diff --git a/Package.swift b/Package.swift index 140b9fee0..b55d5c714 100644 --- a/Package.swift +++ b/Package.swift @@ -307,6 +307,15 @@ let package = Package( ], path: "Sources/Services/ContainerImagesService/Client" ), + .testTarget( + name: "ContainerImagesServiceTests", + dependencies: [ + .product(name: "ContainerizationOCI", package: "containerization"), + "ContainerImagesService", + "ContainerImagesServiceClient", + "ContainerXPC", + ] + ), .executableTarget( name: "container-network-vmnet", dependencies: [ diff --git a/Sources/Services/ContainerAPIService/Client/ClientImage.swift b/Sources/Services/ContainerAPIService/Client/ClientImage.swift index 3516d0ef9..1f095fcb5 100644 --- a/Sources/Services/ContainerAPIService/Client/ClientImage.swift +++ b/Sources/Services/ContainerAPIService/Client/ClientImage.swift @@ -271,6 +271,10 @@ extension ClientImage { request.set(key: .insecureFlag, value: insecure) request.set(key: .maxConcurrentDownloads, value: Int64(maxConcurrentDownloads)) + if let authorization = try await Self.registryAuthorization(for: reference) { + request.set(key: .registryAuthorization, value: authorization) + } + var progressUpdateClient: ProgressUpdateClient? if let progressUpdate { progressUpdateClient = await ProgressUpdateClient(for: progressUpdate, request: request) @@ -284,6 +288,35 @@ extension ClientImage { return image } + /// Resolve registry credentials from the keychain and return the + /// `Authorization` header value to forward to the images helper. + /// + /// The lookup runs in this client process, which for `container image + /// pull`/`push` and `container build` is always the `container` CLI binary + /// (`com.apple.container.cli`). That is the same code identity that + /// `container registry login` uses when it writes the keychain item, so the + /// item's access control accepts this read. The images helper runs under a + /// different identity (`com.apple.container.container-core-images`) and is + /// rejected by that access control, so it must not read the keychain itself. + /// + /// Returns `nil` when no stored credentials exist for the reference's host. + private static func registryAuthorization(for reference: String) async throws -> String? { + guard let host = try Reference.parse(reference).resolvedDomain else { + return nil + } + let keychain = KeychainHelper(securityDomain: Constants.keychainID) + let auth: Authentication + do { + auth = try keychain.lookup(hostname: host) + } catch let err as KeychainHelper.Error { + guard case .keyNotFound = err else { + throw ContainerizationError(.internalError, message: "error querying keychain for \(host)", cause: err) + } + return nil + } + return try await auth.token() + } + public static func delete(reference: String, garbageCollect: Bool = false) async throws { let client = newXPCClient() let request = newRequest(.imageDelete) @@ -395,6 +428,10 @@ extension ClientImage { try request.set(platform: platform) + if let authorization = try await Self.registryAuthorization(for: reference) { + request.set(key: .registryAuthorization, value: authorization) + } + var progressUpdateClient: ProgressUpdateClient? if let progressUpdate { progressUpdateClient = await ProgressUpdateClient(for: progressUpdate, request: request) diff --git a/Sources/Services/ContainerImagesService/Client/ImageServiceXPCKeys.swift b/Sources/Services/ContainerImagesService/Client/ImageServiceXPCKeys.swift index c087f99f1..566d2c0e8 100644 --- a/Sources/Services/ContainerImagesService/Client/ImageServiceXPCKeys.swift +++ b/Sources/Services/ContainerImagesService/Client/ImageServiceXPCKeys.swift @@ -38,6 +38,9 @@ public enum ImagesServiceXPCKeys: String { case maxConcurrentDownloads case forceLoad case rejectedMembers + /// Pre-resolved registry `Authorization` header value, read from the + /// keychain on the client (CLI) side and forwarded to the images helper. + case registryAuthorization /// ContentStore case digest diff --git a/Sources/Services/ContainerImagesService/Server/ImagesService.swift b/Sources/Services/ContainerImagesService/Server/ImagesService.swift index 21a3e5146..a30cadbac 100644 --- a/Sources/Services/ContainerImagesService/Server/ImagesService.swift +++ b/Sources/Services/ContainerImagesService/Server/ImagesService.swift @@ -79,7 +79,7 @@ public actor ImagesService { return try await imageStore.list().map { $0.description.fromCZ } } - public func pull(reference: String, platform: Platform?, insecure: Bool, progressUpdate: ProgressUpdateHandler?, maxConcurrentDownloads: Int = 3) async throws + public func pull(reference: String, platform: Platform?, insecure: Bool, auth: Authentication?, progressUpdate: ProgressUpdateHandler?, maxConcurrentDownloads: Int = 3) async throws -> ImageDescription { self.log.debug( @@ -103,7 +103,7 @@ public actor ImagesService { ) } - let img = try await Self.withAuthentication(ref: reference) { auth in + let img = try await Self.withAuthentication(ref: reference, auth: auth) { auth in try await self.imageStore.pull( reference: reference, platform: platform, insecure: insecure, auth: auth, progress: ContainerizationProgressAdapter.handler(from: progressUpdate), maxConcurrentDownloads: maxConcurrentDownloads) @@ -114,7 +114,7 @@ public actor ImagesService { return img.description.fromCZ } - public func push(reference: String, platform: Platform?, insecure: Bool, progressUpdate: ProgressUpdateHandler?) async throws { + public func push(reference: String, platform: Platform?, insecure: Bool, auth: Authentication?, progressUpdate: ProgressUpdateHandler?) async throws { self.log.debug( "ImagesService: enter", metadata: [ @@ -135,7 +135,7 @@ public actor ImagesService { ) } - try await Self.withAuthentication(ref: reference) { auth in + try await Self.withAuthentication(ref: reference, auth: auth) { auth in try await self.imageStore.push( reference: reference, platform: platform, insecure: insecure, auth: auth, progress: ContainerizationProgressAdapter.handler(from: progressUpdate)) } @@ -412,25 +412,20 @@ extension ImagesService { extension ImagesService { private static func withAuthentication( - ref: String, _ body: @Sendable @escaping (_ auth: Authentication?) async throws -> T? + ref: String, auth: Authentication?, _ body: @Sendable @escaping (_ auth: Authentication?) async throws -> T? ) async throws -> T? { - var authentication: Authentication? let ref = try Reference.parse(ref) guard let host = ref.resolvedDomain else { throw ContainerizationError(.invalidArgument, message: "no host specified in image reference: \(ref)") } - authentication = Self.authenticationFromEnv(host: host) - if let authentication { - return try await body(authentication) - } - let keychain = KeychainHelper(securityDomain: Constants.keychainID) - do { - authentication = try keychain.lookup(hostname: host) - } catch let err as KeychainHelper.Error { - guard case .keyNotFound = err else { - throw ContainerizationError(.internalError, message: "error querying keychain for \(host)", cause: err) - } - } + // Environment credentials take precedence, preserving prior behavior. + // Keychain credentials are resolved on the client (CLI) side and passed + // in as `auth`. This helper must not read the keychain itself: its code + // identity (`com.apple.container.container-core-images`) differs from the + // writer's (`com.apple.container.cli`), so the keychain item's access + // control would reject the read and securityd would fail it with + // errSecInteractionNotAllowed in this non-interactive process. + let authentication = Self.authenticationFromEnv(host: host) ?? auth do { return try await body(authentication) } catch let err as RegistryClient.Error { @@ -459,6 +454,19 @@ extension ImagesService { } } +/// An `Authentication` backed by a pre-resolved `Authorization` header value. +/// +/// Registry credentials are read from the keychain on the client (CLI) side and +/// the resulting header is forwarded to the images helper over XPC, so the +/// helper never reads the keychain itself. +struct ResolvedAuthentication: Authentication { + let authorization: String + + func token() async throws -> String { + authorization + } +} + extension ImageDescription { public var toCZ: Containerization.Image.Description { .init(reference: self.reference, descriptor: self.descriptor) diff --git a/Sources/Services/ContainerImagesService/Server/ImagesServiceHarness.swift b/Sources/Services/ContainerImagesService/Server/ImagesServiceHarness.swift index e88b2368f..3ac3c4b20 100644 --- a/Sources/Services/ContainerImagesService/Server/ImagesServiceHarness.swift +++ b/Sources/Services/ContainerImagesService/Server/ImagesServiceHarness.swift @@ -33,6 +33,19 @@ public struct ImagesServiceHarness: Sendable { self.service = service } + /// Build the registry `Authentication` forwarded from the CLI over XPC, or + /// nil when the request carried no credentials. + /// + /// Registry credentials are read from the keychain on the CLI side (the + /// `com.apple.container.cli` identity that wrote them) and forwarded here as + /// an `Authorization` header value. The images helper must not read the + /// keychain itself: its distinct code identity is rejected by the item's + /// access control, and securityd fails the read with + /// errSecInteractionNotAllowed (-25308) in this non-interactive process. + static func authentication(from message: XPCMessage) -> Authentication? { + message.string(key: .registryAuthorization).map { ResolvedAuthentication(authorization: $0) as Authentication } + } + @Sendable public func pull(_ message: XPCMessage) async throws -> XPCMessage { let ref = message.string(key: .imageReference) @@ -49,10 +62,11 @@ public struct ImagesServiceHarness: Sendable { } let insecure = message.bool(key: .insecureFlag) let maxConcurrentDownloads = message.int64(key: .maxConcurrentDownloads) + let auth = Self.authentication(from: message) let progressUpdateService = ProgressUpdateService(message: message) let imageDescription = try await service.pull( - reference: ref, platform: platform, insecure: insecure, progressUpdate: progressUpdateService?.handler, maxConcurrentDownloads: Int(maxConcurrentDownloads)) + reference: ref, platform: platform, insecure: insecure, auth: auth, progressUpdate: progressUpdateService?.handler, maxConcurrentDownloads: Int(maxConcurrentDownloads)) let imageData = try JSONEncoder().encode(imageDescription) let reply = message.reply() @@ -75,9 +89,10 @@ public struct ImagesServiceHarness: Sendable { platform = try JSONDecoder().decode(ContainerizationOCI.Platform.self, from: platformData) } let insecure = message.bool(key: .insecureFlag) + let auth = Self.authentication(from: message) let progressUpdateService = ProgressUpdateService(message: message) - try await service.push(reference: ref, platform: platform, insecure: insecure, progressUpdate: progressUpdateService?.handler) + try await service.push(reference: ref, platform: platform, insecure: insecure, auth: auth, progressUpdate: progressUpdateService?.handler) let reply = message.reply() return reply diff --git a/Tests/ContainerImagesServiceTests/RegistryAuthForwardingTests.swift b/Tests/ContainerImagesServiceTests/RegistryAuthForwardingTests.swift new file mode 100644 index 000000000..f013b1732 --- /dev/null +++ b/Tests/ContainerImagesServiceTests/RegistryAuthForwardingTests.swift @@ -0,0 +1,60 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerImagesServiceClient +import ContainerXPC +import ContainerizationOCI +import Testing + +@testable import ContainerImagesService + +/// Regression coverage for reading registry credentials in the CLI and forwarding +/// them to the images helper over XPC (commit "Read registry credentials in the +/// CLI, not the images helper"). +/// +/// The bug was that the `container-core-images` helper read the keychain itself +/// and failed with errSecInteractionNotAllowed (-25308), because its code identity +/// differs from the CLI that wrote the item. The fix forwards the resolved +/// `Authorization` header over XPC. These tests pin that the helper consumes the +/// forwarded credential rather than touching the keychain. +@Suite +struct RegistryAuthForwardingTests { + private let authorization = "Basic dXNlcjpwYXNz" // base64("user:pass") + + /// The wrapper must hand back the forwarded `Authorization` value unchanged, + /// including its scheme prefix, so the registry handshake sees what the CLI read. + @Test func resolvedAuthenticationReturnsAuthorizationVerbatim() async throws { + let auth = ResolvedAuthentication(authorization: authorization) + #expect(try await auth.token() == authorization) + } + + /// A request carrying the credential must decode into an `Authentication` + /// whose token matches it — the helper consumes it instead of reading the keychain. + @Test func harnessDecodesForwardedAuthorization() async throws { + let message = XPCMessage(route: ImagesServiceXPCRoute.imagePull.rawValue) + message.set(key: .registryAuthorization, value: authorization) + + let auth = try #require(ImagesServiceHarness.authentication(from: message)) + #expect(try await auth.token() == authorization) + } + + /// No forwarded credential means anonymous access (e.g. public image pulls), + /// so no `Authentication` is produced. + @Test func harnessReturnsNilWithoutForwardedAuthorization() { + let message = XPCMessage(route: ImagesServiceXPCRoute.imagePull.rawValue) + #expect(ImagesServiceHarness.authentication(from: message) == nil) + } +}