RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

Tue May 8 18:03:45 UTC 2018

Hi,

"final" is a important modifier of the method signature and if the CSR 
and the implementation are
different it might raise a question about which is the correct signature 
when the JCK folks write tests.

It is pretty lightweight process to return the CSR to draft and update 
it and finalize it again.
It should be readily approved.  (or leave the implementation as final).

$.02, Roger

On 5/8/2018 1:58 PM, Sean Mullan wrote:
> On 5/8/18 1:52 PM, Jamil Nimeh wrote:
>> I'll make those fixes.  One question with respect to the final 
>> methods: the CSR had those methods in the description and they were 
>> marked as final.  That CSR is now complete.  Will removing the final 
>> qualifier in the methods be an issue?  In terms of effect on the code 
>> it doesn't matter. It's more of a dot-i/cross-t kind of question.
>
> I don't think so as the behavior is the same either way.
>
> --Sean
>
>>
>> --Jamil
>>
>> On 5/8/2018 10:49 AM, Sean Mullan wrote:
>>> - ChaCha20ParameterSpec.java
>>>
>>> The methods don't need to be final now that the class is final.
>>>
>>> - ChaCha20Poly1305Parameters.java
>>>
>>>  122         if (decodingMethod.equalsIgnoreCase(DEFAULT_FMT)) {
>>>  123             engineInit(encoded);
>>>
>>> The spec for engineInit() says if decodingMethod is null, the 
>>> default encoding should be used, so the code above should be:
>>>
>>>  122         if (decodingMethod == null || 
>>> decodingMethod.equalsIgnoreCase(DEFAULT_FMT)) {
>>>  123             engineInit(encoded);
>>>
>>> Looks good otherwise.
>>>
>>> --Sean
>>>
>>> On 5/4/18 10:06 AM, Jamil Nimeh wrote:
>>>> Round 5.
>>>>
>>>> This adds Sean's comments.  Sean, I was never able to execute a 
>>>> case on init where a half-baked object would fail.  In most cases 
>>>> it would fail in checks in javax.crypto.Cipher before it ever made 
>>>> it down to my code. I'm pretty confident the init sequence is OK.  
>>>> I did move the setting of a few data members toward the end of the 
>>>> init sequence but setting the key and nonce is necessary before 
>>>> creating the initial state, which is then used for generating an 
>>>> authentication key for AEAD mode and generating keystream.
>>>>
>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.05
>>>>
>>>> Also the CSR has been finalized and can be found here:
>>>> https://bugs.openjdk.java.net/browse/JDK-8198925
>>>>
>>>> --Jamil
>>>>
>>>> On 04/27/2018 02:21 PM, Jamil Nimeh wrote:
>>>>>
>>>>> Round 4 of updates for ChaCha20 and ChaCha20-Poly1305, minor stuff 
>>>>> mostly:
>>>>>
>>>>>   * Added words in the description of javax.crypto.Cipher 
>>>>> recommending
>>>>>     callers reinitialize the Cipher to use different nonces after 
>>>>> each
>>>>>     complete encryption or decryption (similar language to what 
>>>>> exists
>>>>>     already for AES-GCM encryption).
>>>>>   * Added an additional test case for ChaCha20NoReuse
>>>>>   * Made accessor methods for ChaCha20ParameterSpec final and cleaned
>>>>>     up the code a bit based on comments from the field.
>>>>>
>>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.04/
>>>>>
>>>>> Thanks!
>>>>>
>>>>> --Jamil
>>>>>
>>>>>
>>>>> On 04/13/2018 11:59 AM, Jamil Nimeh wrote:
>>>>>> Round 3 of updates for ChaCha20 and ChaCha20-Poly1305:
>>>>>>
>>>>>> * Removed the key field in ChaCha20 and Poly1305 implementations 
>>>>>> and only retain the key bytes as an object field (thanks Thomas 
>>>>>> for catching this)
>>>>>>
>>>>>> * Added additional protections against key/nonce reuse. This is a 
>>>>>> behavioral change to ChaCha20 and ChaCha20-Poly1305. Instances of 
>>>>>> these ciphers will no longer allow you to do subsequent 
>>>>>> doUpdate/doFinal calls after the first doFinal without 
>>>>>> re-initializing the cipher with either a new key or nonce. 
>>>>>> Attempting to reuse the cipher without a new initialization will 
>>>>>> throw an IllegalStateException. This is similar to the behavior 
>>>>>> of AES-GCM in encrypt mode, but for ChaCha20 it needs to be done 
>>>>>> for both encrypt and decrypt.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.03/
>>>>>>
>>>>>> Thanks,
>>>>>> --Jamil
>>>>>>
>>>>>> On 04/10/2018 03:34 PM, Jamil Nimeh wrote:
>>>>>>> Hello everyone,
>>>>>>>
>>>>>>> This is a quick update to the previous webrev:
>>>>>>>
>>>>>>> * When using the form of engineInit that does only takes op, key 
>>>>>>> and random, the nonce will always be random even if the random 
>>>>>>> parameter is null.  A default instance of SecureRandom will be 
>>>>>>> used to create the nonce in this case, instead of all zeroes.
>>>>>>>
>>>>>>> * Unused debug code was removed from the ChaCha20Cipher.java file
>>>>>>>
>>>>>>> * ChaCha20Parameters.engineToString no longer obtains the line 
>>>>>>> separator from a System property directly. It calls 
>>>>>>> System.lineSeparator() similar to how other AlgorithmParameter 
>>>>>>> classes in com.sun.crypto.provider do it.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.02/
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> --Jamil
>>>>>>>
>>>>>>>
>>>>>>> On 03/26/2018 12:08 PM, Jamil Nimeh wrote:
>>>>>>>> Hello all,
>>>>>>>>
>>>>>>>> This is a request for review for the ChaCha20 and 
>>>>>>>> ChaCha20-Poly1305 cipher implementations.  Links to the webrev 
>>>>>>>> and the JEP which outlines the characteristics and behavior of 
>>>>>>>> the ciphers are listed below.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.01/
>>>>>>>> http://openjdk.java.net/jeps/329
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> --Jamil
>>>>>>>
>>>>>>
>>>>>
>>>>
>>