Skip to content

Commit 4ffc546

Browse files
euanheuanh
authored
ContainerRegistry: Reject invalid image tags and digests (#142)
Motivation ---------- Cherry pick of #140. `ImageReference` does not check for illegal characters in parsed image digests and tags. This means that `containertool` will send illegal image names to the registry. The registry will reject them, but the error message might not explain why, so a generic error message will be printed. Runtimes reject illegal image references immediately, without sending them to the registry. Some desktop runtimes accept local image names which the registry will reject; other runtimes reject these names even for local names. `containertool` now also rejects them. Modifications ------------- * Check validity of tags and digests when parsing image names * Change the low-level API functions to accept `Digest` or `Reference` instead of `String`. Result ------ It is impossible to create a `Repository` object containing a malformed tag or digest, because the constructor checks the string value. It is impossible to send a malformed name to the registry because the API wrappers only accept `Digest` or `Reference (Digest | Tag)` objects. Fixes #139 Test Plan --------- Existing tests continue to pass. New tests exercise additional checks which were previously missing. Removed tests which checked tags which seemed to be accepted by some desktop runtimes, but which were not accepted by registries.
1 parent 0658594 commit 4ffc546

File tree

10 files changed

+247
-93
lines changed

10 files changed

+247
-93
lines changed

‎Sources/ContainerRegistry/Blobs.swift‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,7 @@ extension RegistryClient {
6565
extension HTTPField.Name { static let dockerContentDigest = Self("Docker-Content-Digest")! }
6666

6767
public extension RegistryClient {
68-
func blobExists(repository: ImageReference.Repository, digest: String) async throws -> Bool {
69-
precondition(digest.count > 0)
70-
68+
func blobExists(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws -> Bool {
7169
do {
7270
let _ = try await executeRequestThrowing(
7371
.head(repository, path: "blobs/\(digest)"),
@@ -84,10 +82,8 @@ public extension RegistryClient {
8482
/// - digest: Digest of the blob.
8583
/// - Returns: The downloaded data.
8684
/// - Throws: If the blob download fails.
87-
func getBlob(repository: ImageReference.Repository, digest: String) async throws -> Data {
88-
precondition(digest.count > 0, "digest must not be an empty string")
89-
90-
return try await executeRequestThrowing(
85+
func getBlob(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws -> Data {
86+
try await executeRequestThrowing(
9187
.get(repository, path: "blobs/\(digest)", accepting: ["application/octet-stream"]),
9288
decodingErrors: [.notFound]
9389
)
@@ -106,10 +102,10 @@ public extension RegistryClient {
106102
/// in the registry as plain blobs with MIME type "application/octet-stream".
107103
/// This function attempts to decode the received data without reference
108104
/// to the MIME type.
109-
func getBlob<Response: Decodable>(repository: ImageReference.Repository, digest: String) async throws -> Response {
110-
precondition(digest.count > 0, "digest must not be an empty string")
111-
112-
return try await executeRequestThrowing(
105+
func getBlob<Response: Decodable>(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws
106+
-> Response
107+
{
108+
try await executeRequestThrowing(
113109
.get(repository, path: "blobs/\(digest)", accepting: ["application/octet-stream"]),
114110
decodingErrors: [.notFound]
115111
)

‎Sources/ContainerRegistry/ImageReference.swift‎

Lines changed: 127 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
//
1313
//===----------------------------------------------------------------------===//
1414

15-
import RegexBuilder
16-
1715
// https://github.com/distribution/distribution/blob/v2.7.1/reference/reference.go
1816
// Split the image reference into a registry and a name part.
1917
func splitReference(_ reference: String) throws -> (String?, String) {
@@ -30,29 +28,43 @@ func splitReference(_ reference: String) throws -> (String?, String) {
3028
}
3129

3230
// Split the name into repository and tag parts
33-
// distribution/distribution defines regular expressions which validate names but these seem to be very strict
34-
// and reject names which clients accept
35-
func splitName(_ name: String) throws -> (String, String) {
31+
// distribution/distribution defines regular expressions which validate names
32+
// Some clients, such as docker CLI, accept names which violate these regular expressions for local images, but those images cannot be pushed.
33+
// Other clients, such as podman CLI, reject names which violate these regular expressions even for local images
34+
func parseName(_ name: String) throws -> (ImageReference.Repository, any ImageReference.Reference) {
3635
let digestSplit = name.split(separator: "@", maxSplits: 1, omittingEmptySubsequences: false)
37-
if digestSplit.count == 2 { return (String(digestSplit[0]), String(digestSplit[1])) }
36+
if digestSplit.count == 2 {
37+
return (
38+
try ImageReference.Repository(String(digestSplit[0])),
39+
try ImageReference.Digest(String(digestSplit[1]))
40+
)
41+
}
3842

3943
let tagSplit = name.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false)
40-
if tagSplit.count == 0 { throw ImageReference.ValidationError.unexpected("unexpected error") }
44+
if tagSplit.count == 0 {
45+
throw ImageReference.ValidationError.unexpected("unexpected error")
46+
}
4147

42-
if tagSplit.count == 1 { return (name, "latest") }
48+
if tagSplit.count == 1 {
49+
return (try ImageReference.Repository(name), try ImageReference.Tag("latest"))
50+
}
4351

4452
// assert splits == 2
45-
return (String(tagSplit[0]), String(tagSplit[1]))
53+
return (
54+
try ImageReference.Repository(String(tagSplit[0])),
55+
try ImageReference.Tag(String(tagSplit[1]))
56+
)
4657
}
4758

4859
/// ImageReference points to an image stored on a container registry
60+
/// This type is not found in the API - it is the reference string given by the user
4961
public struct ImageReference: Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
5062
/// The registry which contains this image
5163
public var registry: String
5264
/// The repository which contains this image
5365
public var repository: Repository
54-
/// The tag identifying the image.
55-
public var reference: String
66+
/// The tag or digest identifying the image.
67+
public var reference: Reference
5668

5769
public enum ValidationError: Error {
5870
case unexpected(String)
@@ -65,18 +77,18 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
6577
/// - Throws: If `reference` cannot be parsed as an image reference.
6678
public init(fromString reference: String, defaultRegistry: String = "localhost:5000") throws {
6779
let (registry, remainder) = try splitReference(reference)
68-
let (repository, reference) = try splitName(remainder)
80+
let (repository, reference) = try parseName(remainder)
6981
self.registry = registry ?? defaultRegistry
7082
if self.registry == "docker.io" {
7183
self.registry = "index.docker.io" // Special case for docker client, there is no network-level redirect
7284
}
7385
// As a special case, official images can be referred to by a single name, such as `swift` or `swift:slim`.
7486
// moby/moby assumes that these names refer to images in `library`: `library/swift` or `library/swift:slim`.
7587
// This special case only applies when using Docker Hub, so `example.com/swift` is not expanded `example.com/library/swift`
76-
if self.registry == "index.docker.io" && !repository.contains("/") {
88+
if self.registry == "index.docker.io" && !repository.value.contains("/") {
7789
self.repository = try Repository("library/\(repository)")
7890
} else {
79-
self.repository = try Repository(repository)
91+
self.repository = repository
8092
}
8193
self.reference = reference
8294
}
@@ -87,19 +99,19 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
8799
/// - registry: The registry which stores the image data.
88100
/// - repository: The repository within the registry which holds the image.
89101
/// - reference: The tag identifying the image.
90-
init(registry: String, repository: Repository, reference: String) {
102+
init(registry: String, repository: Repository, reference: Reference) {
91103
self.registry = registry
92104
self.repository = repository
93105
self.reference = reference
94106
}
95107

108+
public static func == (lhs: ImageReference, rhs: ImageReference) -> Bool {
109+
"\(lhs)" == "\(rhs)"
110+
}
111+
96112
/// Printable description of an ImageReference in a form which can be understood by a runtime
97113
public var description: String {
98-
if reference.starts(with: "sha256") {
99-
return "\(registry)/\(repository)@\(reference)"
100-
} else {
101-
return "\(registry)/\(repository):\(reference)"
102-
}
114+
"\(registry)/\(repository)\(reference.separator)\(reference)"
103115
}
104116

105117
/// Printable description of an ImageReference in a form suitable for debugging.
@@ -149,3 +161,98 @@ extension ImageReference {
149161
}
150162
}
151163
}
164+
165+
extension ImageReference {
166+
/// Reference refers to an image in a repository. It can either be a tag or a digest.
167+
public protocol Reference: Sendable, CustomStringConvertible, CustomDebugStringConvertible {
168+
var separator: String { get }
169+
}
170+
171+
/// Tag is a human-readable name for an image.
172+
public struct Tag: Reference, Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
173+
var value: String
174+
175+
public enum ValidationError: Error, Equatable {
176+
case emptyString
177+
case invalidReferenceFormat(String)
178+
case tooLong(String)
179+
}
180+
181+
public init(_ rawValue: String) throws {
182+
guard rawValue.count > 0 else {
183+
throw ValidationError.emptyString
184+
}
185+
186+
guard rawValue.count <= 128 else {
187+
throw ValidationError.tooLong(rawValue)
188+
}
189+
190+
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
191+
let regex = /[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}/
192+
if try regex.wholeMatch(in: rawValue) == nil {
193+
throw ValidationError.invalidReferenceFormat(rawValue)
194+
}
195+
196+
value = rawValue
197+
}
198+
199+
public static func == (lhs: Tag, rhs: Tag) -> Bool {
200+
lhs.value == rhs.value
201+
}
202+
203+
public var separator: String = ":"
204+
205+
public var description: String {
206+
"\(value)"
207+
}
208+
209+
/// Printable description in a form suitable for debugging.
210+
public var debugDescription: String {
211+
"Tag(\(value))"
212+
}
213+
}
214+
215+
/// Digest identifies a specific blob by the hash of the blob's contents.
216+
public struct Digest: Reference, Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
217+
var value: String
218+
219+
public enum ValidationError: Error, Equatable {
220+
case emptyString
221+
case invalidReferenceFormat(String)
222+
case tooLong(String)
223+
}
224+
225+
public init(_ rawValue: String) throws {
226+
guard rawValue.count > 0 else {
227+
throw ValidationError.emptyString
228+
}
229+
230+
if rawValue.count > 7 + 64 {
231+
throw ValidationError.tooLong(rawValue)
232+
}
233+
234+
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
235+
let regex = /sha256:[a-fA-F0-9]{64}/
236+
if try regex.wholeMatch(in: rawValue) == nil {
237+
throw ValidationError.invalidReferenceFormat(rawValue)
238+
}
239+
240+
value = rawValue
241+
}
242+
243+
public static func == (lhs: Digest, rhs: Digest) -> Bool {
244+
lhs.value == rhs.value
245+
}
246+
247+
public var separator: String = "@"
248+
249+
public var description: String {
250+
"\(value)"
251+
}
252+
253+
/// Printable description in a form suitable for debugging.
254+
public var debugDescription: String {
255+
"Digest(\(value))"
256+
}
257+
}
258+
}

‎Sources/ContainerRegistry/Manifests.swift‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
public extension RegistryClient {
16-
func putManifest(repository: ImageReference.Repository, reference: String, manifest: ImageManifest) async throws
16+
func putManifest(
17+
repository: ImageReference.Repository,
18+
reference: any ImageReference.Reference,
19+
manifest: ImageManifest
20+
) async throws
1721
-> String
1822
{
1923
// See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests
20-
precondition("\(reference)".count > 0, "reference must not be an empty string")
21-
2224
let httpResponse = try await executeRequestThrowing(
2325
// All blob uploads have Content-Type: application/octet-stream on the wire, even if mediatype is different
2426
.put(
@@ -42,11 +44,11 @@ public extension RegistryClient {
4244
.absoluteString
4345
}
4446

45-
func getManifest(repository: ImageReference.Repository, reference: String) async throws -> ImageManifest {
47+
func getManifest(repository: ImageReference.Repository, reference: any ImageReference.Reference) async throws
48+
-> ImageManifest
49+
{
4650
// See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
47-
precondition(reference.count > 0, "reference must not be an empty string")
48-
49-
return try await executeRequestThrowing(
51+
try await executeRequestThrowing(
5052
.get(
5153
repository,
5254
path: "manifests/\(reference)",
@@ -60,10 +62,11 @@ public extension RegistryClient {
6062
.data
6163
}
6264

63-
func getIndex(repository: ImageReference.Repository, reference: String) async throws -> ImageIndex {
64-
precondition(reference.count > 0, "reference must not be an empty string")
65-
66-
return try await executeRequestThrowing(
65+
func getIndex(repository: ImageReference.Repository, reference: any ImageReference.Reference) async throws
66+
-> ImageIndex
67+
{
68+
// See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
69+
try await executeRequestThrowing(
6770
.get(
6871
repository,
6972
path: "manifests/\(reference)",

‎Sources/ContainerRegistry/RegistryClient+ImageConfiguration.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ extension RegistryClient {
2121
/// - Throws: If the blob cannot be decoded as an `ImageConfiguration`.
2222
///
2323
/// Image configuration records are stored as blobs in the registry. This function retrieves the requested blob and tries to decode it as a configuration record.
24-
public func getImageConfiguration(forImage image: ImageReference, digest: String) async throws -> ImageConfiguration
24+
public func getImageConfiguration(forImage image: ImageReference, digest: ImageReference.Digest) async throws
25+
-> ImageConfiguration
2526
{
2627
try await getBlob(repository: image.repository, digest: digest)
2728
}

‎Sources/containertool/Extensions/Errors+CustomStringConvertible.swift‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,31 @@ extension ContainerRegistry.ImageReference.Repository.ValidationError: Swift.Cus
6060
}
6161
}
6262
}
63+
64+
extension ContainerRegistry.ImageReference.Tag.ValidationError: Swift.CustomStringConvertible {
65+
/// A human-readable string describing an image reference validation error
66+
public var description: String {
67+
switch self {
68+
case .emptyString:
69+
return "Invalid reference format: tag cannot be empty"
70+
case .tooLong(let rawValue):
71+
return "Invalid reference format: tag (\(rawValue)) is too long"
72+
case .invalidReferenceFormat(let rawValue):
73+
return "Invalid reference format: tag (\(rawValue)) contains invalid characters"
74+
}
75+
}
76+
}
77+
78+
extension ContainerRegistry.ImageReference.Digest.ValidationError: Swift.CustomStringConvertible {
79+
/// A human-readable string describing an image reference validation error
80+
public var description: String {
81+
switch self {
82+
case .emptyString:
83+
return "Invalid reference format: digest cannot be empty"
84+
case .tooLong(let rawValue):
85+
return "Invalid reference format: digest (\(rawValue)) is too long"
86+
case .invalidReferenceFormat(let rawValue):
87+
return "Invalid reference format: digest (\(rawValue)) is not a valid digest"
88+
}
89+
}
90+
}

‎Sources/containertool/Extensions/RegistryClient+CopyBlobs.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension RegistryClient {
2323
/// - destRepository: The repository on this registry to which the blob should be copied.
2424
/// - Throws: If the copy cannot be completed.
2525
func copyBlob(
26-
digest: String,
26+
digest: ImageReference.Digest,
2727
fromRepository sourceRepository: ImageReference.Repository,
2828
toClient destClient: RegistryClient,
2929
toRepository destRepository: ImageReference.Repository
@@ -39,6 +39,6 @@ extension RegistryClient {
3939
log("Layer \(digest): pushing")
4040
let uploaded = try await destClient.putBlob(repository: destRepository, data: blob)
4141
log("Layer \(digest): done")
42-
assert(digest == uploaded.digest)
42+
assert("\(digest)" == uploaded.digest)
4343
}
4444
}

‎Sources/containertool/Extensions/RegistryClient+Layers.swift‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ extension RegistryClient {
2626
return try await getManifest(repository: image.repository, reference: image.reference)
2727
} catch {
2828
// Try again, treating the top level object as an index.
29-
// This could be more efficient if the exception thrown by getManfiest() included the data it was unable to parse
29+
// This could be more efficient if the exception thrown by getManifest() included the data it was unable to parse
3030
let index = try await getIndex(repository: image.repository, reference: image.reference)
3131
guard let manifest = index.manifests.first(where: { $0.platform?.architecture == architecture }) else {
3232
throw "Could not find a suitable base image for \(architecture)"
3333
}
3434
// The index should not point to another index; if it does, this call will throw a final error to be handled by the caller.
35-
return try await getManifest(repository: image.repository, reference: manifest.digest)
35+
return try await getManifest(
36+
repository: image.repository,
37+
reference: ImageReference.Digest(manifest.digest)
38+
)
3639
}
3740
}
3841

0 commit comments

Comments
 (0)