-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[Collections] Signing (part 5): validate signature #3252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is part 1 of a series of PRs to support package collection signing. Modifications: - New `PackageCollectionsSigning` module - Add `Certificate` type - Add EC and RSA key types
Following example apple/swift-crypto#67
This is part 2 of a series of PRs to support package collection signing. Depends on [part 1](swiftlang#3241) Modifications: Add support for signing with EC and RSA keys
- Introduce `CertificatePolicy` protocol (more on this in part 3) - `PackageCollectionsSigning` - given a package collection, certificate and its private key, generate a "signed" collection; and reverse that process to validate collection signature.
This is part 3 of a series of PRs to support package collection signing. Depends on [part 2](swiftlang#3242) Modifications: - Certificate policies to validate collection signing certificates. OCSP support on non-Apple platforms to come in part 4. - Wire up `PackageCollectionSigning` with certificate policies.
This is part 4 of a series of PRs to support package collection signing. Depends on [part 3](swiftlang#3245) Modifications: - Add PackageCollectionsSigningLibc module - Add OCSP support for non-Apple platforms through PackageCollectionsSigningLibc
This continues [part 4](swiftlang#3250) but doesn't necessarily depend on it. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
@swift-ci please smoke test |
certChainPaths: [URL], | ||
certPrivateKeyPath: URL, | ||
certPolicyKey: CertificatePolicyKey, | ||
jsonEncoder: JSONEncoder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: are we passing a concrete JSONEncoder to save the need to create a new one every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we passing a concrete JSONEncoder to save the need to create a new one every time?
No, the collection is serialized and embedded inside the signature, and upon checking the signature we deserialize the collection and compare it with the one received. In previous implementations we were comparing JSON/Data
so we had to make sure we use the same encoder/decoder, but now that we are comparing SignedCollection
we can probably get away with that.
/// - callback: The callback to invoke when the result is available. | ||
func validate(signedCollection: PackageCollectionModel.V1.SignedCollection, | ||
certPolicyKey: CertificatePolicyKey, | ||
jsonDecoder: JSONDecoder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question as above. seems a bit unnatural to have it in the public API vs. a private static decoder/encoder at the implementation level (imo). fwiw, I would feel differently if this was a "TopLevelEncoder" (which does not exist in the stdlib) that reflect the format can be either JSON or something else, but since its always JSON it feels like this should be an impl detail rather than exposed in the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed the "part 5" commit (e23c7e7) only, looks good to me. one question regarding API design
This is part 4 of a series of PRs to support package collection signing on Apple platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on Apple platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on Apple platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on **Apple** platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on **Apple** platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on **Apple** platforms. Originally swiftlang#3252. Depends on swiftlang#3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider`
This is part 4 of a series of PRs to support package collection signing on **Apple** platforms. Originally #3252. Depends on #3269. Modifications: - Add `PackageCollectionsSigning` dependency to `PackageCollections` module - Verify collection signature in `JSONPackageCollectionProvider` - Don't cache CertificatePolicy - Add support for additionalTrustedRootCerts - Custom certificate policy by collection source - Throw cannotVerifySignature when no trusted roots configured - Feature flag
This continues part 4 but doesn't necessarily depend on it.
Modifications:
PackageCollectionsSigning
dependency toPackageCollections
moduleJSONPackageCollectionProvider