[Collections] Support multiple cert pinning configs per domain (#3530)

yim-lee · web-flow · commit 73c47c094293 · 2021-06-15T10:13:45.000-07:00
diff --git a/Sources/PackageCollections/PackageCollections+CertificatePolicy.swift b/Sources/PackageCollections/PackageCollections+CertificatePolicy.swift
@@ -17,33 +17,36 @@ import TSCBasic
 /// that are more restrictive. For example, a source may want to require that all their package
 /// collections be signed using certificate that belongs to certain subject user ID.
 internal struct PackageCollectionSourceCertificatePolicy {
-    private static let defaultSourceCertPolicies: [String: CertificatePolicyConfig] = [
-        "developer.apple.com": CertificatePolicyConfig(
+    private static let defaultSourceCertPolicies: [String: [CertificatePolicyConfig]] = [
+        "developer.apple.com": [CertificatePolicyConfig(
             certPolicyKey: .appleDistribution(subjectUserID: "XLVRDL8TZV"),
             base64EncodedRootCerts: ["MIIEuzCCA6OgAwIBAgIBAjANBgkqhkiG9w0BAQUFADBiMQswCQYDVQQGEwJVUzETMBEGA1UEChMKQXBwbGUgSW5jLjEmMCQGA1UECxMdQXBwbGUgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkxFjAUBgNVBAMTDUFwcGxlIFJvb3QgQ0EwHhcNMDYwNDI1MjE0MDM2WhcNMzUwMjA5MjE0MDM2WjBiMQswCQYDVQQGEwJVUzETMBEGA1UEChMKQXBwbGUgSW5jLjEmMCQGA1UECxMdQXBwbGUgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkxFjAUBgNVBAMTDUFwcGxlIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDkkakJH5HbHkdQ6wXtXnmELes2oldMVeyLGYne+Uts9QerIjAC6Bg++FAJ039BqJj50cpmnCRrEdCju+QbKsMflZ56DKRHi1vUFjczy8QPTc4UadHJGXL1XQ7Vf1+b8iUDulWPTV0N8WQ1IxVLFVkds5T39pyez1C6wVhQZ48ItCD3y6wsIG9wtj8BMIy3Q88PnT3zK0koGsj+zrW5DtleHNbLPbU6rfQPDgCSC7EhFi501TwN22IWq6NxkkdTVcGvL0Gz+PvjcM3mo0xFfh9Ma1CWQYnEdGILEINBhzOKgbEwWOxaBDKMaLOPHd5lc/9nXmW8Sdh2nzMUZaF3lMktAgMBAAGjggF6MIIBdjAOBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUK9BpR5R2Cf70a40uQKb3R01/CF4wHwYDVR0jBBgwFoAUK9BpR5R2Cf70a40uQKb3R01/CF4wggERBgNVHSAEggEIMIIBBDCCAQAGCSqGSIb3Y2QFATCB8jAqBggrBgEFBQcCARYeaHR0cHM6Ly93d3cuYXBwbGUuY29tL2FwcGxlY2EvMIHDBggrBgEFBQcCAjCBthqBs1JlbGlhbmNlIG9uIHRoaXMgY2VydGlmaWNhdGUgYnkgYW55IHBhcnR5IGFzc3VtZXMgYWNjZXB0YW5jZSBvZiB0aGUgdGhlbiBhcHBsaWNhYmxlIHN0YW5kYXJkIHRlcm1zIGFuZCBjb25kaXRpb25zIG9mIHVzZSwgY2VydGlmaWNhdGUgcG9saWN5IGFuZCBjZXJ0aWZpY2F0aW9uIHByYWN0aWNlIHN0YXRlbWVudHMuMA0GCSqGSIb3DQEBBQUAA4IBAQBcNplMLXi37Yyb3PN3m/J20ncwT8EfhYOFG5k9RzfyqZtAjizUsZAS2L70c5vu0mQPy3lPNNiiPvl4/2vIB+x9OYOLUyDTOMSxv5pPCmv/K/xZpwUJfBdAVhEedNO3iyM7R6PVbyTi69G3cN8PReEnyvFteO3ntRcXqNx+IjXKJdXZD9Zr1KIkIxH3oayPc4FgxhtbCS+SsvhESPBgOJ4V9T0mZyCKM2r3DYLP3uujL/lTaltkwGMzd/c6ByxW69oPIQ7aunMZT7XZNn/Bh1XZp5m5MkL72NVxnn6hUrcbvZNCJBIqxw8dtk2cXmPIS4AXUKqK1drk/NAJBzewdXUh"]
-        ),
+        )],
     ]
 
-    private let sourceCertPolicies: [String: CertificatePolicyConfig]
+    private let sourceCertPolicies: [String: [CertificatePolicyConfig]]
 
-    var allRootCerts: [String]? {
+    var allRootCerts: Set<String>? {
         let allRootCerts = self.sourceCertPolicies.values
-            .compactMap { $0.base64EncodedRootCerts }
+            .flatMap { configs in configs.compactMap { $0.base64EncodedRootCerts } }
             .flatMap { $0 }
-        return allRootCerts.isEmpty ? nil : allRootCerts
+        return allRootCerts.isEmpty ? nil : Set(allRootCerts)
     }
 
-    init(sourceCertPolicies: [String: CertificatePolicyConfig]? = nil) {
+    init(sourceCertPolicies: [String: [CertificatePolicyConfig]]? = nil) {
+        guard sourceCertPolicies?.values.first(where: { $0.isEmpty }) == nil else {
+            preconditionFailure("CertificatePolicyConfig array must not be empty")
+        }
         self.sourceCertPolicies = sourceCertPolicies ?? Self.defaultSourceCertPolicies
     }
 
     func mustBeSigned(source: Model.CollectionSource) -> Bool {
         source.certPolicyConfigKey.map { self.sourceCertPolicies[$0] != nil } ?? false
     }
 
-    func certificatePolicyKey(for source: Model.CollectionSource) -> CertificatePolicyKey? {
+    func certificatePolicyKeys(for source: Model.CollectionSource) -> [CertificatePolicyKey]? {
         // Certificate policy is associated to a collection host
-        source.certPolicyConfigKey.flatMap { self.sourceCertPolicies[$0]?.certPolicyKey }
+        source.certPolicyConfigKey.flatMap { self.sourceCertPolicies[$0]?.map { $0.certPolicyKey } }
     }
 
     struct CertificatePolicyConfig {
diff --git a/Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift b/Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift
@@ -33,6 +33,8 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
     static let isSignatureCheckSupported = false
     #endif
 
+    static let defaultCertPolicyKeys: [CertificatePolicyKey] = [.default]
+
     private let configuration: Configuration
     private let diagnosticsEngine: DiagnosticsEngine
     private let httpClient: HTTPClient
@@ -54,7 +56,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
         self.validator = JSONModel.Validator(configuration: configuration.validator)
         self.signatureValidator = signatureValidator ?? PackageCollectionSigning(
             trustedRootCertsDir: configuration.trustedRootCertsDir ?? fileSystem.swiftPMConfigDirectory.appending(component: "trust-root-certs").asURL,
-            additionalTrustedRootCerts: sourceCertPolicy.allRootCerts,
+            additionalTrustedRootCerts: sourceCertPolicy.allRootCerts.map { Array($0) },
             callbackQueue: .sharedConcurrent,
             diagnosticsEngine: diagnosticsEngine
         )
@@ -75,7 +77,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
             do {
                 let fileContents = try localFileSystem.readFileContents(absolutePath)
                 return fileContents.withData { data in
-                    self.decodeAndRunSignatureCheck(source: source, data: data, certPolicyKey: .default, callback: callback)
+                    self.decodeAndRunSignatureCheck(source: source, data: data, certPolicyKeys: Self.defaultCertPolicyKeys, callback: callback)
                 }
             } catch {
                 return callback(.failure(error))
@@ -129,8 +131,8 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                             return callback(.failure(JSONPackageCollectionProviderError.invalidResponse(source.url, "Body is empty")))
                         }
 
-                        let certPolicyKey = self.sourceCertPolicy.certificatePolicyKey(for: source) ?? .default
-                        self.decodeAndRunSignatureCheck(source: source, data: body, certPolicyKey: certPolicyKey, callback: callback)
+                        let certPolicyKeys = self.sourceCertPolicy.certificatePolicyKeys(for: source) ?? Self.defaultCertPolicyKeys
+                        self.decodeAndRunSignatureCheck(source: source, data: body, certPolicyKeys: certPolicyKeys, callback: callback)
                     }
                 }
             }
@@ -139,7 +141,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
 
     private func decodeAndRunSignatureCheck(source: Model.CollectionSource,
                                             data: Data,
-                                            certPolicyKey: CertificatePolicyKey,
+                                            certPolicyKeys: [CertificatePolicyKey],
                                             callback: @escaping (Result<Model.Collection, Error>) -> Void) {
         do {
             // This fails if collection is not signed (i.e., no "signature")
@@ -154,17 +156,26 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                 }
 
                 // Check the signature
-                self.signatureValidator.validate(signedCollection: signedCollection, certPolicyKey: certPolicyKey) { result in
-                    switch result {
-                    case .failure(let error):
-                        self.diagnosticsEngine.emit(warning: "The signature of package collection [\(source)] is invalid: \(error)")
-                        if PackageCollectionSigningError.noTrustedRootCertsConfigured == error as? PackageCollectionSigningError {
-                            callback(.failure(PackageCollectionError.cannotVerifySignature))
-                        } else {
-                            callback(.failure(PackageCollectionError.invalidSignature))
+                let signatureResults = ThreadSafeArrayStore<Result<Void, Error>>()
+                certPolicyKeys.forEach { certPolicyKey in
+                    self.signatureValidator.validate(signedCollection: signedCollection, certPolicyKey: certPolicyKey) { result in
+                        let count = signatureResults.append(result)
+                        if count == certPolicyKeys.count {
+                            if signatureResults.compactMap({ $0.success }).first != nil {
+                                callback(self.makeCollection(from: signedCollection.collection, source: source, signature: Model.SignatureData(from: signedCollection.signature, isVerified: true)))
+                            } else {
+                                guard let error = signatureResults.compactMap({ $0.failure }).first else {
+                                    fatalError("Expected at least one package collection signature validation failure but got none")
+                                }
+
+                                self.diagnosticsEngine.emit(warning: "The signature of package collection [\(source)] is invalid: \(error)")
+                                if PackageCollectionSigningError.noTrustedRootCertsConfigured == error as? PackageCollectionSigningError {
+                                    callback(.failure(PackageCollectionError.cannotVerifySignature))
+                                } else {
+                                    callback(.failure(PackageCollectionError.invalidSignature))
+                                }
+                            }
                         }
-                    case .success:
-                        callback(self.makeCollection(from: signedCollection.collection, source: source, signature: Model.SignatureData(from: signedCollection.signature, isVerified: true)))
                     }
                 }
             }
diff --git a/Sources/PackageCollectionsSigning/Certificate/CertificatePolicy.swift b/Sources/PackageCollectionsSigning/Certificate/CertificatePolicy.swift
@@ -762,7 +762,7 @@ struct AppleDeveloperCertificatePolicy: CertificatePolicy {
     }
 }
 
-public enum CertificatePolicyKey: Equatable, Hashable {
+public enum CertificatePolicyKey: Hashable {
     case `default`(subjectUserID: String?)
     case appleDistribution(subjectUserID: String?)
 
diff --git a/Tests/PackageCollectionsTests/JSONPackageCollectionProviderTests.swift b/Tests/PackageCollectionsTests/JSONPackageCollectionProviderTests.swift
@@ -652,7 +652,81 @@ class JSONPackageCollectionProviderTests: XCTestCase {
             let signatureValidator = MockCollectionSignatureValidator(["Sample Package Collection"])
             // Collections from www.test.com must be signed
             let sourceCertPolicy = PackageCollectionSourceCertificatePolicy(
-                sourceCertPolicies: ["www.test.com": .init(certPolicyKey: CertificatePolicyKey.default, base64EncodedRootCerts: nil)]
+                sourceCertPolicies: ["www.test.com": [.init(certPolicyKey: CertificatePolicyKey.default, base64EncodedRootCerts: nil)]]
+            )
+            let provider = JSONPackageCollectionProvider(httpClient: httpClient, signatureValidator: signatureValidator,
+                                                         sourceCertPolicy: sourceCertPolicy, diagnosticsEngine: DiagnosticsEngine())
+            let source = PackageCollectionsModel.CollectionSource(type: .json, url: url)
+            let collection = try tsc_await { callback in provider.get(source, callback: callback) }
+
+            XCTAssertEqual(collection.name, "Sample Package Collection")
+            XCTAssertEqual(collection.overview, "This is a sample package collection listing made-up packages.")
+            XCTAssertEqual(collection.keywords, ["sample package collection"])
+            XCTAssertEqual(collection.createdBy?.name, "Jane Doe")
+            XCTAssertEqual(collection.packages.count, 2)
+            let package = collection.packages.first!
+            XCTAssertEqual(package.repository, .init(url: "https://www.example.com/repos/RepoOne.git"))
+            XCTAssertEqual(package.summary, "Package One")
+            XCTAssertEqual(package.keywords, ["sample package"])
+            XCTAssertEqual(package.readmeURL, URL(string: "https://www.example.com/repos/RepoOne/README")!)
+            XCTAssertEqual(package.license, .init(type: .Apache2_0, url: URL(string: "https://www.example.com/repos/RepoOne/LICENSE")!))
+            XCTAssertEqual(package.versions.count, 1)
+            let version = package.versions.first!
+            XCTAssertEqual(version.summary, "Fixed a few bugs")
+            let manifest = version.manifests.values.first!
+            XCTAssertEqual(manifest.packageName, "PackageOne")
+            XCTAssertEqual(manifest.targets, [.init(name: "Foo", moduleName: "Foo")])
+            XCTAssertEqual(manifest.products, [.init(name: "Foo", type: .library(.automatic), targets: [.init(name: "Foo", moduleName: "Foo")])])
+            XCTAssertEqual(manifest.toolsVersion, ToolsVersion(string: "5.1")!)
+            XCTAssertEqual(manifest.minimumPlatformVersions, [SupportedPlatform(platform: .macOS, version: .init("10.15"))])
+            XCTAssertEqual(version.verifiedCompatibility?.count, 3)
+            XCTAssertEqual(version.verifiedCompatibility!.first!.platform, .macOS)
+            XCTAssertEqual(version.verifiedCompatibility!.first!.swiftVersion, SwiftLanguageVersion(string: "5.1")!)
+            XCTAssertEqual(version.license, .init(type: .Apache2_0, url: URL(string: "https://www.example.com/repos/RepoOne/LICENSE")!))
+            XCTAssertNotNil(version.createdAt)
+            XCTAssertTrue(collection.isSigned)
+            let signature = collection.signature!
+            XCTAssertTrue(signature.isVerified)
+            XCTAssertEqual("Sample Subject", signature.certificate.subject.commonName)
+            XCTAssertEqual("Sample Issuer", signature.certificate.issuer.commonName)
+        }
+    }
+
+    func testRequiredSigningMultiplePoliciesGood() throws {
+        fixture(name: "Collections") { directoryPath in
+            let path = directoryPath.appending(components: "JSON", "good_signed.json")
+            let url = URL(string: "https://www.test.com/collection.json")!
+            let data = Data(try localFileSystem.readFileContents(path).contents)
+
+            let handler: HTTPClient.Handler = { request, _, completion in
+                XCTAssertEqual(request.url, url, "url should match")
+                switch request.method {
+                case .head:
+                    completion(.success(.init(statusCode: 200,
+                                              headers: .init([.init(name: "Content-Length", value: "\(data.count)")]))))
+                case .get:
+                    completion(.success(.init(statusCode: 200,
+                                              headers: .init([.init(name: "Content-Length", value: "\(data.count)")]),
+                                              body: data)))
+                default:
+                    XCTFail("method should match")
+                }
+            }
+
+            var httpClient = HTTPClient(handler: handler)
+            httpClient.configuration.circuitBreakerStrategy = .none
+            httpClient.configuration.retryStrategy = .none
+
+            // Mark collection as having valid signature
+            let signatureValidator = MockCollectionSignatureValidator(certPolicyKeys: [CertificatePolicyKey.default(subjectUserID: "test")])
+            // Collections from www.test.com must be signed
+            let sourceCertPolicy = PackageCollectionSourceCertificatePolicy(
+                sourceCertPolicies: [
+                    "www.test.com": [
+                        .init(certPolicyKey: CertificatePolicyKey.default, base64EncodedRootCerts: nil),
+                        .init(certPolicyKey: CertificatePolicyKey.default(subjectUserID: "test"), base64EncodedRootCerts: nil),
+                    ],
+                ]
             )
             let provider = JSONPackageCollectionProvider(httpClient: httpClient, signatureValidator: signatureValidator,
                                                          sourceCertPolicy: sourceCertPolicy, diagnosticsEngine: DiagnosticsEngine())
@@ -721,7 +795,7 @@ class JSONPackageCollectionProviderTests: XCTestCase {
             let signatureValidator = MockCollectionSignatureValidator()
             // Collections from www.test.com must be signed
             let sourceCertPolicy = PackageCollectionSourceCertificatePolicy(
-                sourceCertPolicies: ["www.test.com": .init(certPolicyKey: CertificatePolicyKey.default, base64EncodedRootCerts: nil)]
+                sourceCertPolicies: ["www.test.com": [.init(certPolicyKey: CertificatePolicyKey.default, base64EncodedRootCerts: nil)]]
             )
             let provider = JSONPackageCollectionProvider(httpClient: httpClient, signatureValidator: signatureValidator,
                                                          sourceCertPolicy: sourceCertPolicy, diagnosticsEngine: DiagnosticsEngine())
diff --git a/Tests/PackageCollectionsTests/PackageCollectionSourceCertificatePolicyTests.swift b/Tests/PackageCollectionsTests/PackageCollectionSourceCertificatePolicyTests.swift
@@ -17,24 +17,30 @@ import PackageCollectionsSigning
 final class PackageCollectionSourceCertificatePolicyTests: XCTestCase {
     func testCustomData() {
         let sourceCertPolicy = PackageCollectionSourceCertificatePolicy(sourceCertPolicies: [
-            "package-collection-1": PackageCollectionSourceCertificatePolicy.CertificatePolicyConfig(
-                certPolicyKey: CertificatePolicyKey.default,
-                base64EncodedRootCerts: ["root-cert-1a", "root-cert-1b"]
-            ),
-            "package-collection-2": PackageCollectionSourceCertificatePolicy.CertificatePolicyConfig(
+            "package-collection-1": [
+                PackageCollectionSourceCertificatePolicy.CertificatePolicyConfig(
+                    certPolicyKey: CertificatePolicyKey.default,
+                    base64EncodedRootCerts: ["root-cert-1a", "root-cert-1b"]
+                ),
+                PackageCollectionSourceCertificatePolicy.CertificatePolicyConfig(
+                    certPolicyKey: .default(subjectUserID: "test"),
+                    base64EncodedRootCerts: ["root-cert-1c"]
+                ),
+            ],
+            "package-collection-2": [PackageCollectionSourceCertificatePolicy.CertificatePolicyConfig(
                 certPolicyKey: CertificatePolicyKey.default,
                 base64EncodedRootCerts: ["root-cert-2"]
-            ),
+            )],
         ])
         let source1 = Model.CollectionSource(type: .json, url: URL(string: "https://package-collection-1")!)
         let unsignedSource = Model.CollectionSource(type: .json, url: URL(string: "https://package-collection-unsigned")!)
 
-        XCTAssertEqual(["root-cert-1a", "root-cert-1b", "root-cert-2"], sourceCertPolicy.allRootCerts?.sorted())
+        XCTAssertEqual(["root-cert-1a", "root-cert-1b", "root-cert-1c", "root-cert-2"], sourceCertPolicy.allRootCerts?.sorted())
 
         XCTAssertTrue(sourceCertPolicy.mustBeSigned(source: source1))
         XCTAssertFalse(sourceCertPolicy.mustBeSigned(source: unsignedSource))
 
-        XCTAssertEqual(CertificatePolicyKey.default, sourceCertPolicy.certificatePolicyKey(for: source1))
-        XCTAssertNil(sourceCertPolicy.certificatePolicyKey(for: unsignedSource))
+        XCTAssertEqual([.default, .default(subjectUserID: "test")], sourceCertPolicy.certificatePolicyKeys(for: source1))
+        XCTAssertNil(sourceCertPolicy.certificatePolicyKeys(for: unsignedSource))
     }
 }
diff --git a/Tests/PackageCollectionsTests/Utility.swift b/Tests/PackageCollectionsTests/Utility.swift
@@ -174,10 +174,12 @@ struct MockMetadataProvider: PackageMetadataProvider {
 
 struct MockCollectionSignatureValidator: PackageCollectionSignatureValidator {
     let collections: Set<String>
+    let certPolicyKeys: Set<CertificatePolicyKey>
     let hasTrustedRootCerts: Bool
 
-    init(_ collections: Set<String> = [], hasTrustedRootCerts: Bool = true) {
+    init(_ collections: Set<String> = [], certPolicyKeys: Set<CertificatePolicyKey> = [], hasTrustedRootCerts: Bool = true) {
         self.collections = collections
+        self.certPolicyKeys = certPolicyKeys
         self.hasTrustedRootCerts = hasTrustedRootCerts
     }
 
@@ -188,7 +190,7 @@ struct MockCollectionSignatureValidator: PackageCollectionSignatureValidator {
             return callback(.failure(PackageCollectionSigningError.noTrustedRootCertsConfigured))
         }
 
-        if self.collections.contains(signedCollection.collection.name) {
+        if self.collections.contains(signedCollection.collection.name) || self.certPolicyKeys.contains(certPolicyKey) {
             callback(.success(()))
         } else {
             callback(.failure(PackageCollectionSigningError.invalidSignature))

Original file line number	Diff line number	Diff line change
`@@ -762,7 +762,7 @@ struct AppleDeveloperCertificatePolicy: CertificatePolicy {`
`762`	`762`	`}`
`763`	`763`	`}`
`764`	`764`
`765`		`-public enum CertificatePolicyKey: Equatable, Hashable {`
	`765`	`+public enum CertificatePolicyKey: Hashable {`
`766`	`766`	case `default`(subjectUserID: String?)
`767`	`767`	`case appleDistribution(subjectUserID: String?)`
`768`	`768`