-
Notifications
You must be signed in to change notification settings - Fork 88
Custom private key #282
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
base: main
Are you sure you want to change the base?
Custom private key #282
Conversation
fe7ab89 to
aa451de
Compare
Rename to CustomPrivateKey
Co-authored-by: Cory Benfield <cbenfield@apple.com>
aa451de to
8a254cd
Compare
7d16034 to
574eaa0
Compare
Motivation: The new tests should be run Modifications: The new tests are missing the `@Test` macros Result: The new tests will have the `@Test` macros
Motivation: Code should be clean Modifications: Removed `@inlinable` from protocol definition Result: protocol will not have `@inlinable`
Motivation: Default implementation of signAsynchronously should be inlinable Modifications: adds `@inlinable` to default signAsynchronously implementation. Result: default signAsynchronously implementation will be `@inlinable`
Motivation: `signAsynchronously` should not it is not mandatory to implement Modifications: Updated documentation Result: `signAsynchronously` has better docs
574eaa0 to
4d583ad
Compare
| /// preference or security of the contained algorithms. | ||
| var supportedSignatureAlgorithms: [Certificate.SignatureAlgorithm] { get } | ||
|
|
||
| /// Use the private key to sign the provided bytes with a given signature algorithm. |
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.
How is an async only private key supposed to implement this function? Should it throw or block instead?
Should this be separate protocols for async and sync keys?
some documentation/guidance for conforming types and callers of this function would be good.
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.
That was my original implementation. I believe the concern was it could lead to too much api duplication.
I will add documentation to guide the developer to throw an error in the case of unsupported synchronous signing.
Motivation:
In some cases a developer may want to sign a certificate using a method other than a private key. For example: if a private key is protected by hardware which signs asynchronously.
Modifications:
CustomPrivateKeyprotocolasyncinitializers forCertificateandCertificateSigningRequest.CustomPrivateKeycan now back aCertificate.PrivateKeyCertificate.Signatureinitializer publicResult:
Developers can now sign a certificate with greater flexibility.
Alternatives Considered:
Implementations
Pass the
CustomPrivateKeyintoCertificateandCertificateSigningRequestinitializers directly.There is concern this could add too much duplication of api.
Various names for the protocol:
Certificate.PrivateKeyProtocolandCertificate.AsyncPrivateKeyProtocolCertificate.Signer/Certificate.AsyncSignerCertificate.SignatureProvider/Certificate.AsyncSignatureProvider