> On Sep 6, 2018, at 5:02 PM, Jamil Nimeh <jamil.j.ni...@oracle.com> wrote: > > 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. This is a sound reason to me. Okay, keep it.
Xuelei > 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 >
