TLSv1.3: Update libcrypto to support TLSv1.3 style IVs#1940
TLSv1.3: Update libcrypto to support TLSv1.3 style IVs#1940mattcaswell wants to merge 6 commits intoopenssl:masterfrom
Conversation
When initialising a cipher you are supposed to be able to provide the key and IV in seperate calls to EVP_CipherInitEx. However ChaCha20 ignores the IV if you do it this way.
We need to add the ability to handle a sequence number, as well as forcing in-place encryption (the only supported type for TLS). Also the TLS implementation does not return a separate tag. It is just on the end of the ciphertext.
crypto/evp/e_aes.c
Outdated
| gctx->taglen = -1; | ||
| gctx->iv_gen = 0; | ||
| gctx->tls_aad_len = -1; | ||
| gctx->use_seq_no = 0; |
There was a problem hiding this comment.
perhaps remove all the "=0" assignments and start off with a memset(0)?
There was a problem hiding this comment.
GH won't let me comment on line 1296, but since you're there put a blank line after the decl/assignment and before the switch.
Windows doesn't seem to be able to cope with the \n line endings otherwise. This was a pre-existing bug in evp_test.c, and was only causing an issue in the X25519 tests at the end of evptests.txt which have multi-line keys. For some reason this didn't actually fail until we tried to add more tests onto the end of the file, which is why we didn't notice it before.
|
Two new commits added: one to address the feedback comments, the other to address an AppVeyor failure. The AppVeyor issue was actually a pre-existing bug related to the multi-line X25519 keys that were already at the end of evptests.txt. The bug didn't hit until we tried to add more tests. It seems Windows can't cope very well with Unix style \n when opening a file in "r" mode. Opening "rb" sorts it. @richsalz please reconfirm? |
|
Rather than continue to route all these things into But the TLS code instead uses TLS-specific ctrl hooks which make the AEADs aware of TLS-specific nonce construction. Having this parallel logic means your code is harder to test and understand. The usual standalone AEAD tests don't cover the TLS stuff because it's using different code. The ChaCha20-Poly1305 version of this logic, in particular, has not held up well: This PR adds a new TLS-specific ctrl hook. And because you export it in the public header, you are committing to exposing this in Instead, I would suggest the TLS code be a normal consumer of libcrypto and use the same AEAD API as everything else. All the business around different versions having different nonce constructions is all TLS stuff and belongs in libssl. |
|
+1 reconfirm. |
|
@davidben - I think that's a very fair point. I'm closing this while I have a rethink. |
Checklist
Description of change
This is an update to the existing TLS GCM cipher in libcrypto to be able to support TLSv1.3 style IVs. evp_tests.txt is also updated with test vectors from https://www.ietf.org/id/draft-thomson-tls-tls13-vectors-01.txt
I had to make a few tweaks to evp_test.c to be able to support these test vectors. Part of that was splitting the setting of the key and the IV into separate steps. This revealed a bug in ChaCha20 where this didn't work properly, so I fixed that as well.