Hi Xuelei, thank you for the comments - my replies are in-line:
On 9/6/2018 2:31 PM, Xuelei Fan wrote:
SSLCipher.java
--------------
line 2159-2164 in the update vs line 1992-1997 in the old file.
The new code is fine, but it takes me a while to analysis the code,
and comparing with the old one. Maybe, we can use the same
implementation code for the same logic for maintenance? Just a very
personal preference. You make the final choice. If you accept it,
please consider other places that compute the nonce value.
Respectfully, I think the way the AES-GCM code sets up the cipher
doesn't match very well with how ChaCha20 does it. Even the RFC itself
says that the nonce construction is different. There's no per-record
nonce_explicit and it's really just a padded sequence number XORed with
the client or server read/write IV. I think the current code follows
the procedure in 7905 closely. Taking the GCM construction will muddy
it a bit, since things like recordIvSize get brought in...for CC20
that's always zero, so why have it at all? It just clutters things IMO.
2180 sequence);
'sn' should be used here. The 'sequence' variable may be different
from the one used for the cipher.
Oh! Good catch. I will fix this.
Otherwise, looks fine to me.
Thanks Xuelei, much appreciated,
--Jamil
Thanks,
Xuelei
On 9/5/2018 9:51 PM, Jamil Nimeh wrote:
Hello all,
This change will add ChaCha20-Poly1305 cipher suites to our TLS 1.2
and TLS 1.3 implementations. A few test cases had to be updated to
reflect the new suites as well.
JBS: https://bugs.openjdk.java.net/browse/JDK-8140466
CSR: https://bugs.openjdk.java.net/browse/JDK-8204192
Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8140466/webrev.01/
Thanks,
--Jamil
