-
Notifications
You must be signed in to change notification settings - Fork 88
Expose supported signature algorithms for private keys #281
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
Motivation: When loading a private key from disk as a Certificate.PrivateKey there is no straight forward way to query the supported signature algorithms, e.g., to perform a signature with the key. For signature algorithm negotiation, the raw values defined in RFC can also be useful. Modifications: Add a new property to query the supported signature algorithms for a given key. Add a new property that translates the signature algorithms to the value defined in RFC 8446. Results: Working with certificate private keys and signatures gets easier.
| /// Return a list of all supported signature types for this private key. The ordering is not a comment on the | ||
| /// preference or security of the contained algorithms. | ||
| @inlinable | ||
| public func supportedSignatureAlgorithms(includeLegacyAlgorithms: Bool = false) -> [Certificate.SignatureAlgorithm] |
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.
Instead of "legacy", the term used internally in this repo, and in public API in Swift Crypto, is "insecure".
E.g.
| case insecureSHA1(Insecure.SHA1Digest) |
However, since Swift Certificates' public API has a value for Certificate.SignatureAlgorithm.sha1WithRSAEncryption, that makes no such distinction, I don't know if we want have this flag or not in the API.
WDYT?
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.
I used "legacy" because that's the term used in RFC 8446, but insecure works just as well. It is probably more important for us to be consistent across modules!
Whether we should include such a flag is a good question. Depending on the use case, users might want to filter out that sha1 every time after calling this. Then again, it is probably good to explicit about signature use instead of supporting anything this returns.
I'd be okay with removing that flag and maybe highlighting this in the doc?
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.
I removed the argument for now. And since we don't have an argument anymore, I made this a computed property.
| @available(macOS 10.15, iOS 13, watchOS 6, tvOS 13, macCatalyst 13, visionOS 1.0, *) | ||
| extension Certificate.SignatureAlgorithm { | ||
| /// Map a signature algorithm to the signature scheme value defined in RFC 8446 for TLS 1.3. | ||
| public var rfc8446SignatureSchemeValue: UInt16 { |
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.
IMHO, if we have this we'll probably want to have the inverse, i.e. an init(rfc8446SignatureSchemeValue: UInt16) either as fallible or throwing.
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.
Good point. I'll add this.
simonjbeaumont
left a comment
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.
Thanks for making the changes. This LGTM now. //cc @Lukasa if you have any additional feedback.
Motivation:
When loading a private key from disk as a
Certificate.PrivateKeythere is no straight forward way to query the supported signature algorithms. For signature algorithm negotiation, the raw values defined in RFC are useful as well.Modifications:
Add a new property to query the supported signature algorithms for a given key. Add a new property that translates a signature algorithm to the respective value defined in RFC 8446.
Results:
Working with certificate private keys and signature algorithms gets easier.