[Collections] Check for collections that are required to be signed

Sources/Commands/SwiftPackageCollectionsTool.swift

@@ -22,6 +22,7 @@ private enum CollectionsError: Swift.Error {
     case unsigned
     case cannotVerifySignature
     case invalidSignature
+    case missingSignature
 }
 
 // FIXME: add links to docs in error messages
@@ -38,6 +39,8 @@ extension CollectionsError: CustomStringConvertible {
             return "The collection's signature cannot be verified due to missing configuration. Please refer to documentations on how to set up trusted root certificates or rerun 'add' with '--skip-signature-check'."
         case .invalidSignature:
             return "The collection's signature is invalid. If you would still like to add it please rerun 'add' with '--skip-signature-check'."
+        case .missingSignature:
+            return "The collection is missing required signature, which means it might have been compromised. Please contact the collection's authors and alert them of the issue."
         }
     }
 }
@@ -139,6 +142,8 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
                     throw CollectionsError.cannotVerifySignature
                 } catch PackageCollectionError.invalidSignature {
                     throw CollectionsError.invalidSignature
+                } catch PackageCollectionError.missingSignature {
+                    throw CollectionsError.missingSignature
                 }
             }
 

Sources/PackageCollections/API.swift

@@ -180,4 +180,6 @@ public enum PackageCollectionError: Equatable, Error {
     case cannotVerifySignature
 
     case invalidSignature
+
+    case missingSignature
 }

Sources/PackageCollections/PackageCollections+CertificatePolicy.swift

@@ -17,20 +17,30 @@ 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 {
-    static let sourceCertPolicies: [String: CertificatePolicyConfig] = [:]
+    private static let defaultSourceCertPolicies: [String: CertificatePolicyConfig] = [:]
 
-    static func certificatePolicyKey(for source: Model.CollectionSource) -> CertificatePolicyKey? {
-        // Certificate policy is associated to a collection host
-        source.url.host.flatMap { self.sourceCertPolicies[$0]?.certPolicyKey }
-    }
+    private let sourceCertPolicies: [String: CertificatePolicyConfig]
 
-    static var allRootCerts: [String]? {
-        let allRootCerts = Self.sourceCertPolicies.values
+    var allRootCerts: [String]? {
+        let allRootCerts = self.sourceCertPolicies.values
             .compactMap { $0.base64EncodedRootCerts }
             .flatMap { $0 }
         return allRootCerts.isEmpty ? nil : allRootCerts
     }
 
+    init(sourceCertPolicies: [String: CertificatePolicyConfig]? = nil) {
+        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? {
+        // Certificate policy is associated to a collection host
+        source.certPolicyConfigKey.flatMap { self.sourceCertPolicies[$0]?.certPolicyKey }
+    }
+
     struct CertificatePolicyConfig {
         let certPolicyKey: CertificatePolicyKey
 
@@ -39,3 +49,9 @@ internal struct PackageCollectionSourceCertificatePolicy {
         let base64EncodedRootCerts: [String]?
     }
 }
+
+private extension Model.CollectionSource {
+    var certPolicyConfigKey: String? {
+        self.url.host
+    }
+}

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

@@ -34,10 +34,12 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
     private let decoder: JSONDecoder
     private let validator: JSONModel.Validator
     private let signatureValidator: PackageCollectionSignatureValidator
+    private let sourceCertPolicy: PackageCollectionSourceCertificatePolicy
 
     init(configuration: Configuration = .init(),
          httpClient: HTTPClient? = nil,
          signatureValidator: PackageCollectionSignatureValidator? = nil,
+         sourceCertPolicy: PackageCollectionSourceCertificatePolicy = PackageCollectionSourceCertificatePolicy(),
          fileSystem: FileSystem = localFileSystem,
          diagnosticsEngine: DiagnosticsEngine) {
         self.configuration = configuration
@@ -47,10 +49,11 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
         self.validator = JSONModel.Validator(configuration: configuration.validator)
         self.signatureValidator = signatureValidator ?? PackageCollectionSigning(
             trustedRootCertsDir: configuration.trustedRootCertsDir ?? fileSystem.dotSwiftPM.appending(components: "config", "trust-root-certs").asURL,
-            additionalTrustedRootCerts: PackageCollectionSourceCertificatePolicy.allRootCerts,
+            additionalTrustedRootCerts: sourceCertPolicy.allRootCerts,
             callbackQueue: DispatchQueue.global(),
             diagnosticsEngine: diagnosticsEngine
         )
+        self.sourceCertPolicy = sourceCertPolicy
     }
 
     func get(_ source: Model.CollectionSource, callback: @escaping (Result<Model.Collection, Error>) -> Void) {
@@ -109,7 +112,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                             return callback(.failure(Errors.invalidResponse("Body is empty")))
                         }
 
-                        let certPolicyKey = PackageCollectionSourceCertificatePolicy.certificatePolicyKey(for: source) ?? .default
+                        let certPolicyKey = self.sourceCertPolicy.certificatePolicyKey(for: source) ?? .default
                         self.decodeAndRunSignatureCheck(source: source, data: body, certPolicyKey: certPolicyKey, callback: callback)
                     }
                 }
@@ -149,7 +152,11 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                 }
             }
         } catch {
-            // Collection is not signed
+            // Bad: collection is supposed to be signed but it isn't
+            guard !self.sourceCertPolicy.mustBeSigned(source: source) else {
+                return callback(.failure(PackageCollectionError.missingSignature))
+            }
+            // Collection is unsigned
             guard let collection = try? self.decoder.decode(JSONModel.Collection.self, from: data) else {
                 return callback(.failure(Errors.invalidJSON(error)))
             }

Tests/PackageCollectionsSigningTests/CertificatePolicyTests.swift

@@ -319,16 +319,46 @@ class CertificatePolicyTests: XCTestCase {
 
             #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
             // The Apple root certs come preinstalled on Apple platforms and they are automatically trusted
-            let policy = DefaultCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
-                                                  callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
-            XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+
+            // Subject user ID matches
+            do {
+                let policy = DefaultCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
+                                                      callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+            }
+            // Subject user ID does not match
+            do {
+                let mismatchSubjectUserID = "\(expectedSubjectUserID)-2"
+                let policy = DefaultCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: mismatchSubjectUserID,
+                                                      callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                XCTAssertThrowsError(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) }) { error in
+                    guard CertificatePolicyError.subjectUserIDMismatch == error as? CertificatePolicyError else {
+                        return XCTFail("Expected CertificatePolicyError.subjectUserIDMismatch")
+                    }
+                }
+            }
             #else
             // On other platforms we have to specify `trustedRootCertsDir` so the Apple root cert is trusted
             try withTemporaryDirectory { tmp in
                 try localFileSystem.copy(from: rootCAPath, to: tmp.appending(components: "AppleIncRoot.cer"))
-                let policy = DefaultCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
-                                                      callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
-                XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+
+                // Subject user ID matches
+                do {
+                    let policy = DefaultCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
+                                                          callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                    XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+                }
+                // Subject user ID does not match
+                do {
+                    let mismatchSubjectUserID = "\(expectedSubjectUserID)-2"
+                    let policy = DefaultCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: mismatchSubjectUserID,
+                                                          callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                    XCTAssertThrowsError(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) }) { error in
+                        guard CertificatePolicyError.subjectUserIDMismatch == error as? CertificatePolicyError else {
+                            return XCTFail("Expected CertificatePolicyError.subjectUserIDMismatch")
+                        }
+                    }
+                }
             }
             #endif
         }
@@ -359,16 +389,46 @@ class CertificatePolicyTests: XCTestCase {
 
             #if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
             // The Apple root certs come preinstalled on Apple platforms and they are automatically trusted
-            let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
-                                                         callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
-            XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+
+            // Subject user ID matches
+            do {
+                let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
+                                                             callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+            }
+            // Subject user ID does not match
+            do {
+                let mismatchSubjectUserID = "\(expectedSubjectUserID)-2"
+                let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: nil, additionalTrustedRootCerts: nil, expectedSubjectUserID: mismatchSubjectUserID,
+                                                             callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                XCTAssertThrowsError(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) }) { error in
+                    guard CertificatePolicyError.subjectUserIDMismatch == error as? CertificatePolicyError else {
+                        return XCTFail("Expected CertificatePolicyError.subjectUserIDMismatch")
+                    }
+                }
+            }
             #else
             // On other platforms we have to specify `trustedRootCertsDir` so the Apple root cert is trusted
             try withTemporaryDirectory { tmp in
                 try localFileSystem.copy(from: rootCAPath, to: tmp.appending(components: "AppleIncRoot.cer"))
-                let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
-                                                             callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
-                XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+
+                // Subject user ID matches
+                do {
+                    let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: expectedSubjectUserID,
+                                                                 callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                    XCTAssertNoThrow(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) })
+                }
+                // Subject user ID does not match
+                do {
+                    let mismatchSubjectUserID = "\(expectedSubjectUserID)-2"
+                    let policy = AppleDeveloperCertificatePolicy(trustedRootCertsDir: tmp.asURL, additionalTrustedRootCerts: nil, expectedSubjectUserID: mismatchSubjectUserID,
+                                                                 callbackQueue: DispatchQueue.global(), diagnosticsEngine: DiagnosticsEngine())
+                    XCTAssertThrowsError(try tsc_await { callback in policy.validate(certChain: certChain, callback: callback) }) { error in
+                        guard CertificatePolicyError.subjectUserIDMismatch == error as? CertificatePolicyError else {
+                            return XCTFail("Expected CertificatePolicyError.subjectUserIDMismatch")
+                        }
+                    }
+                }
             }
             #endif
         }