[Collections] Add skip signature check option to 'add'

Sources/Commands/SwiftPackageCollectionsTool.swift

@@ -20,8 +20,10 @@ private enum CollectionsError: Swift.Error {
     case invalidArgument(String)
     case invalidVersionString(String)
     case unsigned
+    case cannotVerifySignature
 }
 
+// FIXME: add links to docs in error messages
 extension CollectionsError: CustomStringConvertible {
     var description: String {
         switch self {
@@ -31,6 +33,8 @@ extension CollectionsError: CustomStringConvertible {
             return "Invalid version string '\(versionString)'"
         case .unsigned:
             return "The collection is not signed. If you would still like to add it please rerun 'add' with '--trust-unsigned'."
+        case .cannotVerifySignature:
+            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."
         }
     }
 }
@@ -106,12 +110,15 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
         @Flag(name: .long, help: "Trust the collection even if it is unsigned")
         var trustUnsigned: Bool = false
 
+        @Flag(name: .long, help: "Skip signature check if the collection is signed")
+        var skipSignatureCheck: Bool = false
+
         mutating func run() throws {
             guard let collectionUrl = URL(string: collectionUrl) else {
                 throw CollectionsError.invalidArgument("collectionUrl")
             }
 
-            let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
+            let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl, skipSignatureCheck: self.skipSignatureCheck)
             let collection: PackageCollectionsModel.Collection = try with { collections in
                 do {
                     let userTrusted = self.trustUnsigned
@@ -125,6 +132,8 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
                     }
                 } catch PackageCollectionError.trustConfirmationRequired, PackageCollectionError.untrusted {
                     throw CollectionsError.unsigned
+                } catch PackageCollectionError.cannotVerifySignature {
+                    throw CollectionsError.cannotVerifySignature
                 }
             }
 

Sources/PackageCollections/API.swift

@@ -174,4 +174,8 @@ public enum PackageCollectionError: Equatable, Error {
 
     /// Package collection is not signed and user explicitly marks it untrusted
     case untrusted
+
+    /// There are no trusted root certificates. Signature check cannot be done in this case since it involves validating
+    /// the certificate chain that is used for signing and one requirement is that the root certificate must be trusted.
+    case cannotVerifySignature
 }

Sources/PackageCollections/Model/Collection.swift

@@ -100,13 +100,17 @@ extension PackageCollectionsModel {
         /// Indicates if the source is explicitly trusted or untrusted by the user
         public var isTrusted: Bool?
 
+        /// Indicates if the source can skip signature validation
+        public var skipSignatureCheck: Bool
+
         /// The source's absolute file system path, if its URL is of 'file' scheme.
         let absolutePath: AbsolutePath?
 
-        public init(type: CollectionSourceType, url: URL, isTrusted: Bool? = nil) {
+        public init(type: CollectionSourceType, url: URL, isTrusted: Bool? = nil, skipSignatureCheck: Bool = false) {
             self.type = type
             self.url = url
             self.isTrusted = isTrusted
+            self.skipSignatureCheck = skipSignatureCheck
 
             if url.scheme?.lowercased() == "file", let absolutePath = try? AbsolutePath(validating: url.path) {
                 self.absolutePath = absolutePath
@@ -192,8 +196,12 @@ extension PackageCollectionsModel {
         /// Details about the certificate used to generate the signature
         public let certificate: Certificate
 
-        public init(certificate: Certificate) {
+        /// Indicates if the signature has been validated. This is set to false if signature check didn't take place.
+        public let isVerified: Bool
+
+        public init(certificate: Certificate, isVerified: Bool) {
             self.certificate = certificate
+            self.isVerified = isVerified
         }
 
         public struct Certificate: Equatable, Codable {

Sources/PackageCollections/PackageCollections.swift

@@ -140,8 +140,9 @@ public struct PackageCollections: PackageCollectionsProtocol {
                 self.refreshCollectionFromSource(source: source, trustConfirmationProvider: trustConfirmationProvider) { collectionResult in
                     switch collectionResult {
                     case .failure(let error):
-                        // Don't delete the source if we are either pending user confirmation or have recorded user's preference
-                        if let error = error as? PackageCollectionError, error == .trustConfirmationRequired || error == .untrusted {
+                        // Don't delete the source if we are either pending user confirmation or have recorded user's preference.
+                        // It is also possible that we can't verify signature (yet) due to config issue, which user can fix and we retry later.
+                        if let error = error as? PackageCollectionError, error == .trustConfirmationRequired || error == .untrusted || error == .cannotVerifySignature {
                             return callback(.failure(error))
                         }
                         // Otherwise remove source since it fails to be fetched

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

@@ -10,6 +10,7 @@
 
 import Basics
 import Dispatch
+import struct Foundation.Data
 import struct Foundation.Date
 import class Foundation.JSONDecoder
 import struct Foundation.URL
@@ -49,14 +50,9 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
         if let absolutePath = source.absolutePath {
             do {
                 let fileContents = try localFileSystem.readFileContents(absolutePath)
-                let collection: JSONModel.Collection = try fileContents.withData { data in
-                    do {
-                        return try self.decoder.decode(JSONModel.Collection.self, from: data)
-                    } catch {
-                        throw Errors.invalidJSON(error)
-                    }
+                return fileContents.withData { data in
+                    self.decodeAndRunSignatureCheck(source: source, data: data, callback: callback)
                 }
-                return callback(self.makeCollection(from: collection, source: source, signature: nil))
             } catch {
                 return callback(.failure(error))
             }
@@ -97,33 +93,43 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                             return callback(.failure(Errors.invalidResponse("Body is empty")))
                         }
 
-                        do {
-                            // parse json and construct result
-                            do {
-                                // This fails if "signature" is missing
-                                let signature = try JSONModel.SignedCollection.signature(from: body, using: self.decoder)
-                                // TODO: Check collection's signature
-                                // If signature is
-                                //      a. valid: process the collection; set isSigned=true
-                                //      b. invalid: includes expired cert, untrusted cert, signature-payload mismatch => return error
-                                let collection = try JSONModel.SignedCollection.collection(from: body, using: self.decoder)
-                                callback(self.makeCollection(from: collection, source: source, signature: Model.SignatureData(from: signature)))
-                            } catch {
-                                // Collection is not signed
-                                guard let collection = try response.decodeBody(JSONModel.Collection.self, using: self.decoder) else {
-                                    return callback(.failure(Errors.invalidResponse("Invalid body")))
-                                }
-                                callback(self.makeCollection(from: collection, source: source, signature: nil))
-                            }
-                        } catch {
-                            callback(.failure(Errors.invalidJSON(error)))
-                        }
+                        self.decodeAndRunSignatureCheck(source: source, data: body, callback: callback)
                     }
                 }
             }
         }
     }
 
+    private func decodeAndRunSignatureCheck(source: Model.CollectionSource,
+                                            data: Data,
+                                            callback: @escaping (Result<Model.Collection, Error>) -> Void) {
+        do {
+            // This fails if "signature" is missing
+            let signature = try JSONModel.SignedCollection.signature(from: data, using: self.decoder)
+            if source.skipSignatureCheck {
+                // Don't validate signature but set isVerified=false
+                let collection = try JSONModel.SignedCollection.collection(from: data, using: self.decoder)
+                callback(self.makeCollection(from: collection, source: source, signature: Model.SignatureData(from: signature, isVerified: false)))
+            } else {
+                // TODO: Signature validator should throw "cannot verify" error on non-Apple platforms
+                // if there are no trusted root certs set up, in which case we should throw PackageCollectionError.cannotVerifySignature
+
+                // TODO: Check collection's signature
+                // If signature is
+                //      a. valid: process the collection; set isSigned=true
+                //      b. invalid: includes expired cert, untrusted cert, signature-payload mismatch => return error
+                let collection = try JSONModel.SignedCollection.collection(from: data, using: self.decoder)
+                callback(self.makeCollection(from: collection, source: source, signature: Model.SignatureData(from: signature, isVerified: true)))
+            }
+        } catch {
+            // Collection is not signed
+            guard let collection = try? self.decoder.decode(JSONModel.Collection.self, from: data) else {
+                return callback(.failure(Errors.invalidJSON(error)))
+            }
+            callback(self.makeCollection(from: collection, source: source, signature: nil))
+        }
+    }
+
     private func makeCollection(from collection: JSONModel.Collection, source: Model.CollectionSource, signature: Model.SignatureData?) -> Result<Model.Collection, Error> {
         if let errors = self.validator.validate(collection: collection)?.errors() {
             return .failure(MultipleErrors(errors))
@@ -380,8 +386,9 @@ extension Model.License {
 }
 
 extension Model.SignatureData {
-    fileprivate init(from: JSONModel.Signature) {
+    fileprivate init(from: JSONModel.Signature, isVerified: Bool) {
         self.certificate = .init(from: from.certificate)
+        self.isVerified = isVerified
     }
 }
 

Sources/PackageCollections/Storage/FilePackageCollectionsSourcesStorage.swift

@@ -187,6 +187,7 @@ private enum StorageModel {
         let type: String
         let value: String
         let isTrusted: Bool?
+        let skipSignatureCheck: Bool?
     }
 
     enum SourceType: String {
@@ -204,7 +205,7 @@ private extension Model.CollectionSource {
 
         switch from.type {
         case StorageModel.SourceType.json.rawValue:
-            self.init(type: .json, url: url, isTrusted: from.isTrusted)
+            self.init(type: .json, url: url, isTrusted: from.isTrusted, skipSignatureCheck: from.skipSignatureCheck ?? false)
         default:
             throw SerializationError.unknownType(from.type)
         }
@@ -213,7 +214,8 @@ private extension Model.CollectionSource {
     func source() -> StorageModel.Source {
         switch self.type {
         case .json:
-            return .init(type: StorageModel.SourceType.json.rawValue, value: self.url.absoluteString, isTrusted: self.isTrusted)
+            return .init(type: StorageModel.SourceType.json.rawValue, value: self.url.absoluteString,
+                         isTrusted: self.isTrusted, skipSignatureCheck: self.skipSignatureCheck)
         }
     }
 }

Tests/PackageCollectionsTests/JSONPackageCollectionProviderTests.swift

@@ -132,6 +132,69 @@ class JSONPackageCollectionProviderTests: XCTestCase {
             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 testGoodSigned_skipSignatureCheck() 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
+            let provider = JSONPackageCollectionProvider(httpClient: httpClient)
+            // Skip signature check
+            let source = PackageCollectionsModel.CollectionSource(type: .json, url: url, skipSignatureCheck: true)
+            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!
+            XCTAssertFalse(signature.isVerified)
             XCTAssertEqual("Sample Subject", signature.certificate.subject.commonName)
             XCTAssertEqual("Sample Issuer", signature.certificate.issuer.commonName)
         }

Tests/PackageCollectionsTests/PackageCollectionsSourcesStorageTest.swift

@@ -86,7 +86,16 @@ final class PackageCollectionsSourcesStorageTest: XCTestCase {
             source.isTrusted = !(source.isTrusted ?? false)
             _ = try tsc_await { callback in storage.update(source: source, callback: callback) }
             let listAfter = try tsc_await { callback in storage.list(callback: callback) }
-            XCTAssertEqual(source.isTrusted, listAfter.first!.isTrusted, "item should match")
+            XCTAssertEqual(source.isTrusted, listAfter.first!.isTrusted, "isTrusted should match")
+        }
+
+        do {
+            let list = try tsc_await { callback in storage.list(callback: callback) }
+            var source = list.first!
+            source.skipSignatureCheck = !source.skipSignatureCheck
+            _ = try tsc_await { callback in storage.update(source: source, callback: callback) }
+            let listAfter = try tsc_await { callback in storage.list(callback: callback) }
+            XCTAssertEqual(source.skipSignatureCheck, listAfter.first!.skipSignatureCheck, "skipSignatureCheck should match")
         }
     }
 

Tests/PackageCollectionsTests/PackageCollectionsTests.swift

@@ -831,7 +831,8 @@ final class PackageCollectionsTests: XCTestCase {
                         certificate: PackageCollectionsModel.SignatureData.Certificate(
                             subject: .init(userID: nil, commonName: nil, organizationalUnit: nil, organization: nil),
                             issuer: .init(userID: nil, commonName: nil, organizationalUnit: nil, organization: nil)
-                        )
+                        ),
+                        isVerified: true
                     )
                     callback(.success(PackageCollectionsModel.Collection(source: source, name: "", overview: nil, keywords: nil, packages: [], createdAt: Date(), createdBy: nil, signature: signature)))
                 }

Tests/PackageCollectionsTests/Utility.swift

@@ -88,7 +88,8 @@ func makeMockCollections(count: Int = Int.random(in: 50 ... 100), maxPackages: I
                 certificate: PackageCollectionsModel.SignatureData.Certificate(
                     subject: .init(userID: nil, commonName: "subject-\(collectionIndex)", organizationalUnit: nil, organization: nil),
                     issuer: .init(userID: nil, commonName: "issuer-\(collectionIndex)", organizationalUnit: nil, organization: nil)
-                )
+                ),
+                isVerified: true
             )
         }