From ea453cb111d75e22c464a84827f0863fc2f16144 Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:50:36 +0100 Subject: [PATCH] Preserve registry authentication realm query parameters (#1362) * Preserve registry authentication realm query parameters Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> * Keep encoded registry realm query parameters unchanged Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> --------- Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> --- Sources/tart/OCI/Registry.swift | 35 +++++-- Tests/TartTests/RegistryRealmTests.swift | 116 +++++++++++++++++++++++ 2 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 Tests/TartTests/RegistryRealmTests.swift diff --git a/Sources/tart/OCI/Registry.swift b/Sources/tart/OCI/Registry.swift index 4a9ebe0..906b7b3 100644 --- a/Sources/tart/OCI/Registry.swift +++ b/Sources/tart/OCI/Registry.swift @@ -360,12 +360,9 @@ class Registry { ) async throws -> (AsyncThrowingStream, HTTPURLResponse) { var urlComponents = urlComponents - if urlComponents.queryItems == nil && !parameters.isEmpty { - urlComponents.queryItems = [] - } - urlComponents.queryItems?.append(contentsOf: parameters.map { key, value -> URLQueryItem in + Self.updateQueryItems(parameters.map { key, value in URLQueryItem(name: key, value: value) - }) + }, in: &urlComponents) var request = URLRequest(url: urlComponents.url!) request.httpMethod = method.rawValue @@ -387,6 +384,30 @@ class Registry { return (channel, response) } + // Preserve the exact encoding of existing query values, even when the + // caller supplies no additional parameters. URLComponents.queryItems decodes + // then re-encodes retained items, which changes values such as alpha%2Bbeta. + private static func updateQueryItems( + _ items: [URLQueryItem], + in components: inout URLComponents, + replacingExisting: Bool = false + ) { + guard !items.isEmpty else { return } + + var additions = URLComponents() + additions.queryItems = items + + var existing = components.percentEncodedQueryItems ?? [] + if replacingExisting { + let replacementNames = items.map { $0.name } + existing.removeAll { item in + guard let name = item.name.removingPercentEncoding else { return false } + return replacementNames.contains(name) + } + } + components.percentEncodedQueryItems = existing + (additions.percentEncodedQueryItems ?? []) + } + private func auth(response: HTTPURLResponse) async throws { // Process WWW-Authenticate header guard let wwwAuthenticateRaw = response.value(forHTTPHeaderField: "WWW-Authenticate") else { @@ -423,7 +444,7 @@ class Registry { // >[...] using the service and scope values from the WWW-Authenticate header. // // [1]: https://docs.docker.com/registry/spec/auth/token/ - authenticateURL.queryItems = ["scope", "service"].compactMap { key in + let challengeQueryItems: [URLQueryItem] = ["scope", "service"].compactMap { key in if let value = wwwAuthenticate.kvs[key] { return URLQueryItem(name: key, value: value) } else { @@ -431,6 +452,8 @@ class Registry { } } + Self.updateQueryItems(challengeQueryItems, in: &authenticateURL, replacingExisting: true) + var headers: Dictionary = Dictionary() if let (user, password) = try lookupCredentials() { diff --git a/Tests/TartTests/RegistryRealmTests.swift b/Tests/TartTests/RegistryRealmTests.swift new file mode 100644 index 0000000..d28529b --- /dev/null +++ b/Tests/TartTests/RegistryRealmTests.swift @@ -0,0 +1,116 @@ +import Foundation +import Network +import XCTest +@testable import tart + +final class RegistryRealmTests: XCTestCase { + func testPreservesRealmQueryParameters() async throws { + try await checkAuthentication(mode: "preserve") + } + + func testChallengeReplacesOnlyItsOwnQueryParameters() async throws { + try await checkAuthentication(mode: "replace") + } + + func testRealmWithoutQueryParameters() async throws { + try await checkAuthentication(mode: "plain") + } + + func testPreservesOriginalPercentEncodingAndReplacesEncodedChallengeKeys() async throws { + try await checkAuthentication(mode: "encoded") + } + + private func checkAuthentication(mode: String) async throws { + let server = try RealmTokenServer(mode: mode) + defer { server.stop() } + let port = try await server.start() + let registry = try Registry(host: "127.0.0.1:\(port)", namespace: "org/repo", insecure: true, credentialsProviders: []) + try await registry.ping() + } +} + +private final class RealmTokenServer { + private let listener: NWListener + private let queue = DispatchQueue(label: "registry-realm-test") + private let mode: String + + init(mode: String) throws { + self.mode = mode + let parameters = NWParameters.tcp + parameters.requiredLocalEndpoint = .hostPort(host: "127.0.0.1", port: .any) + listener = try NWListener(using: parameters) + } + + func start() async throws -> UInt16 { + try await withCheckedThrowingContinuation { continuation in + listener.stateUpdateHandler = { state in + switch state { + case .ready: + self.listener.stateUpdateHandler = nil + continuation.resume(returning: self.listener.port!.rawValue) + case .failed(let error): + self.listener.stateUpdateHandler = nil + continuation.resume(throwing: error) + default: + break + } + } + listener.newConnectionHandler = { connection in + connection.start(queue: self.queue) + self.receive(connection, buffered: Data()) + } + listener.start(queue: queue) + } + } + + func stop() { listener.cancel() } + + private func receive(_ connection: NWConnection, buffered: Data) { + connection.receive(minimumIncompleteLength: 1, maximumLength: 65536) { data, _, complete, error in + let received = buffered + (data ?? Data()) + guard received.range(of: Data("\r\n\r\n".utf8)) != nil else { + if error != nil || complete { connection.cancel() } + else { self.receive(connection, buffered: received) } + return + } + let request = String(decoding: received, as: UTF8.self) + let target = request.split(separator: " ")[1] + let url = URL(string: "http://127.0.0.1:\(self.listener.port!.rawValue)\(target)")! + var status = "200 OK" + var extraHeaders = "" + var body = "{}" + if url.path == "/v2" || url.path == "/v2/" { + if !request.lowercased().contains("authorization: bearer test-token\r\n") { + let query: String + switch self.mode { + case "plain": query = "" + case "encoded": query = "?tenant=alpha%2Bbeta&escape=%7e%2f&mode=one&mode=two&empty=" + default: query = "?tenant=alpha%26beta&mode=one&mode=two&empty=" + } + let stale = self.mode == "replace" ? "&scope=old&service=old" : + (self.mode == "encoded" ? "&sc%6Fpe=old&service=old" : "") + let realm = "http://127.0.0.1:\(self.listener.port!.rawValue)/token\(query)\(stale)" + status = "401 Unauthorized" + extraHeaders = "WWW-Authenticate: Bearer realm=\"\(realm)\",service=\"registry\",scope=\"repository:org/repo:pull\"\r\n" + } + } else if url.path == "/token" { + let receivedURL = URLComponents(url: url, resolvingAgainstBaseURL: false)! + let items = receivedURL.queryItems ?? [] + let values = { (name: String) in items.filter { $0.name == name }.map { $0.value ?? "" } } + let expectedRaw = "tenant=alpha%2Bbeta&escape=%7e%2f&mode=one&mode=two&empty=&" + let encodedValid = receivedURL.percentEncodedQuery?.hasPrefix(expectedRaw) == true && + values("tenant") == ["alpha+beta"] && values("escape") == ["~/"] + let normalValid = values("tenant") == ["alpha&beta"] + let valid = values("service") == ["registry"] && values("scope") == ["repository:org/repo:pull"] && + (self.mode == "plain" || ((self.mode == "encoded" ? encodedValid : normalValid) && + values("mode") == ["one", "two"] && values("empty") == [""])) + status = valid ? "200 OK" : "400 Bad Request" + body = valid ? "{\"token\":\"test-token\"}" : "{\"error\":\"missing or changed realm parameters\"}" + } else { + status = "404 Not Found" + } + let response = "HTTP/1.1 \(status)\r\n\(extraHeaders)Content-Type: application/json\r\nContent-Length: \(body.utf8.count)\r\nConnection: close\r\n\r\n\(body)" + connection.send(content: Data(response.utf8), completion: .contentProcessed { _ in connection.cancel() }) + } + } +}