RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

[Sean Mullan]

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
>>>>>>
>>>>>
>>>>
>>>
>