Skip to content
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

Enable sha384 ciphers #168

Merged
merged 10 commits into from
Dec 17, 2016
Merged

Conversation

vdukhovni
Copy link
Collaborator

This PR implements support for TLS12 ciphersuites with SHA384 (or other, though none yet in use) PRFs.
It also adds some new cipher code points that fill existing gaps.
With this done ECDSA signature algorithms are enabled, and secp384r1 is added to the supported curves. See individual commit logs for more detail.

These functions work with general public keys and are not RSA-specific
This just changes a comment.  These are neither used nor needed in
TLS, and have e.g. been withdrawn from OpenSSL.
With version >= TLS12 the PRF is cipher dependent.  We throw in the
version as an ignored parameter as future-proofing.  The caller
will pass in the protocol version and the PRF from the appropriate
cipher.
This is needed for SHA384 ciphers in TLS12 and later.
Also fix typos:

    cipher_ECDHE_RSA_AES256CBC_SHA384: cipherID
    cipher_ECDHE_RSA_AES256GCM_SHA384: cipherName
@vdukhovni vdukhovni force-pushed the enable-sha384-ciphers branch 3 times, most recently from 620dfe2 to d2a0db1 Compare December 1, 2016 00:25
@ocheron
Copy link
Contributor

ocheron commented Dec 1, 2016

With ECDSA enabled this also fixes #152.

@vdukhovni
Copy link
Collaborator Author

vdukhovni commented Dec 1, 2016

@ocheron: With ECDSA enabled this also fixes #152.

Good to kill two birds with one stone. I was working to fix interoperability with OpenSSL-based stacks such as Postfix, initially because leaving out support for SHA384 ciphersuites meant that OpenSSL servers that preempt the client's cipher list order will choose AES256CBC over AESGCM128, and I wanted to see AEAD used.

I was pleased to find that adding the missing bits was simpler than I expected.

It would be good if a few folks looked at the updated lists for ciphersuite_all and ciphersuite_strong as well as the new ciphersuite_default (which is really a default recommendation, rather than an actual default setting). The commit in question is fc1de62

And yes, I've successfully tested working ECDHE_ECDSA_AES256GCM_SHA384, ... against an OpenSSL server. It would be good to add test cases for this to the test suite, but the master branch does not yet have support for server-side ECDSA credentials (the ECDSA private key code seems to be missing). Perhaps someone can fill in the gap...

This does also require haskell-tls/hs-certificate#80 so that all the usual NIST curves are recognized.

The optional cipherPRFHash field determines the Hash in the PRF
HMAC (which otherwise defaults to SHA256), while the required
cipherHash field determines the Hash used in the Finished message
(and as MAC for non-AEAD ciphers).
Two are AES256GCM-SHA384 variants of existing AES128GCM-SHA256 ciphers:

     DHE-RSA-AES256GCM-SHA384
     ECDHE-ECDSA-AES256GCM-SHA384

Two more are CBC alternatives to GCM for ECDHE-ECDSA with SHA2:

     ECDHE-ECDSA-AES128CBC-SHA256
     ECDHE-ECDSA-AES256CBC-SHA384

Two more are 128/256-bit GCM alternatives to CBC for non-DHE
RSA key-exchange:

     RSA-AES128GCM-SHA256
     RSA-AES256GCM-SHA384

The last two flesh out ECDHE-ECDSA support with CBC+SHA1

     ECDHE-ECDSA-AES128CBC-SHA
     ECDHE-ECDSA-AES256CBC-SHA

We also note that:

     cipher_RSA_3DES_EDE_CBC_SHA1
     cipher_RC4_128_MD5
     cipher_RC4_128_SHA1
     cipher_null_MD5
     cipher_DHE_DSS_RC4_SHA1

are obsolete and possibly non-standard (hintint that they should
not be used).
"ciphersuite_default" is "ciphersuite_all", minus the obsolete
stuff.  Suggest using this as the "Supported" set of ciphers.
Signal ECDSA signatures as part of the TLS12 signature algorithms
extension.

It is not uncommon to find secp384r1 as a server's sole ECDHE curve.
Add it to the supported curve list.
@vdukhovni
Copy link
Collaborator Author

FYI, did a force push to simplify needlessly complex definitions of getPRF and getHash (use guards on the function itself instead of Pattern Guards on case () of). No substantive change, just style...

@vincenthz vincenthz merged commit f4fec81 into haskell-tls:master Dec 17, 2016
@ocheron
Copy link
Contributor

ocheron commented Dec 18, 2016

Now would probably be a good time to change also SimpleClient.hs and replace the variable ciphers with ciphersuite_all:

*Main Data.List Network.TLS.Extra> mapM_ print $ ciphers \\ ciphersuite_all
RSA-rc4-128-md5
*Main Data.List Network.TLS.Extra> mapM_ print $ ciphersuite_all \\ ciphers
ECDHE-ECDSA-AES256GCM-SHA384
ECDHE-RSA-AES256GCM-SHA384
DHE-RSA-AES256GCM-SHA384
ECDHE-ECDSA-AES128CBC-SHA256
ECDHE-ECDSA-AES256CBC-SHA384
ECDHE-RSA-AES128CBC-SHA256
ECDHE-RSA-AES256CBC-SHA384
ECDHE-ECDSA-AES128CBC-SHA
ECDHE-ECDSA-AES256CBC-SHA
ECDHE-RSA-AES128CBC-SHA
RSA-AES128GCM-SHA256
RSA-AES256GCM-SHA384
RSA-AES256-SHA256
RSA-AES128-SHA256

If no objection I would also replace the lists in other debug utilities (for example ciphersuite_all client-side and ciphersuite_default server-side).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants