-
Notifications
You must be signed in to change notification settings - Fork 88
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
Selecting a cipher based on "signature_algorithms" #193
Conversation
Function credentialCanDecrypt allowed a non-RSA credential when the certificate did not have a key usage.
Now that signature_algorithms extension is used to sign the ServerKeyXchg, this creates a possibility that client and server don't agree on the hash algorithm. When this happens a cipher with a different key-exchange should have been selected. We now skip ciphers when the key-exchange is not possible because of this reason.
core/Network/TLS/Handshake/Server.hs
Outdated
TLS12 -> let -- Build a list of all signature algorithms with at least | ||
-- one hash algorithm common between client and server. | ||
-- May contain duplicates, as it is only used for `elem`. | ||
sigAlgExt = extensionLookup extensionID_SignatureAlgorithms exts >>= extensionDecode False |
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.
The code here is a copy-and-paste.
They should be a function to avoid redundancy.
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.
Yes you're right. I added that function.
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.
LGTM now.
I guess that it's time to release a new version. |
Thanks. I created two new issues for things we could continue further, but I don't plan to work on them immediately. Anyone can self-assign. A new release is a good idea, we have already 70 commits since 1.3.9. |
@vincenthz Let's release a new version. |
This extends #177 so that the "signature_algorithms" extension impacts cipher selection as discussed in #191.
I added a new test case specific to TLS 1.2 where handshake can either succeed or fail depending on client and server
supportedHashSignatures
. This could be further extended to test more TLS 1.2 extensions like "supported_groups" when we implement it.I also added code for DSA certificates, so that the QuickCheck properties can test more combinations.