RFR: JDK-8140466: ChaCha20-Poly1305 TLS cipher suites

Xuelei Fan Xuelei.Fan at Oracle.Com
Fri Sep 7 00:08:27 UTC 2018


> On Sep 6, 2018, at 5:02 PM, Jamil Nimeh <jamil.j.nimeh at 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
> 




More information about the security-dev mailing list