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

New ciphers for TLS 1.2 and TLS 1.3 #287

Merged
merged 12 commits into from
Oct 10, 2018
Merged

New ciphers for TLS 1.2 and TLS 1.3 #287

merged 12 commits into from
Oct 10, 2018

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Sep 23, 2018

Adds the following:

  • AES CCM ciphers for TLS 1.3
  • ChaCha20-Poly1305 ciphers for TLS 1.2 (RFC 7905 section 2) and TLS 1.3

The nonce construction of TLS 1.3 is applied to TLS 1.2 when bulkExplicitIV is 0 instead of 8, which I believe is the correct condition.

I also remove cipherIDtoCipher13 so that custom ciphers can be used with TLS 1.3 too.

@kazu-yamamoto kazu-yamamoto self-requested a review October 10, 2018 05:29
@@ -163,17 +162,18 @@ handshakeClient' cparams ctx groups mcrand = do
return $ Just $ toExtensionRaw $ KeyShareClientHello [ent]
| otherwise = return Nothing

sessionHash sdata = case cipherIDtoCipher13 (sessionCipher sdata) of
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for removing cipherIDtoCipher13 . 👍

@kazu-yamamoto kazu-yamamoto self-requested a review October 10, 2018 06:05
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. I will merge this.

@kazu-yamamoto
Copy link
Collaborator

I have confirmed that tls-simpleclient works well for TLS 1.3.

kazu-yamamoto added a commit to kazu-yamamoto/hs-tls that referenced this pull request Oct 10, 2018
@kazu-yamamoto kazu-yamamoto merged commit b0454ae into haskell-tls:master Oct 10, 2018
@ocheron ocheron deleted the ccm13-chacha20-poly1305 branch October 12, 2018 04:41
@ocheron
Copy link
Contributor Author

ocheron commented Oct 12, 2018

Thanks for your tests.

I think it will be possible to merge TLS13 record modules with the ones used for previous versions. In the end, there are very few differences:

  • TLS13 has the additional wrapping of encrypted records to AppData
  • the content length which is included in the AAD is also different

That's all I see. The compression step can be put back since it is a no-op.

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.

2 participants