RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

Sean Mullan sean.mullan at oracle.com
Thu May 31 12:55:16 UTC 2018 

Looks fine to me.

--Sean

On 5/30/18 5:39 PM, Jamil Nimeh wrote:
> Hopefully the final set of updates before committing this feature:
> 
>   * Updated the ChaChaEngine implementations in ChaCha20Cipher.java to
>     properly conform to Cipher's specification when the output buffer
>     used in doUpdate is too small.  Instead of throwing
>     IndexOutOfBoundsException like it was doing, it will now throw a
>     ShortBufferException per the spec.
> 
> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.09
> 
> Thanks,
> 
> --Jamil
> 
> 
> On 05/22/2018 01:43 PM, Jamil Nimeh wrote:
>> A couple updates:
>>
>>   * Touched up the comments surrounding init/wrap/unwrap methods. 
>>     None of these affect public javadocs.
>>   * Added an implementation for engineGetParameters.  This was
>>     something that just got forgotten from the initial development of
>>     the cipher when there were no AlgorithmParameter implementations.
>>       o There are a couple tests added to ChaCha20Poly1305ParamTest to
>>         exercise these new code paths.
>>
>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.08
>>
>> Thanks,
>>
>> --Jamil
>>
>>
>> On 5/21/2018 11:36 AM, Jamil Nimeh wrote:
>>>
>>> And the fun just keeps going!  Round 7.
>>>
>>> This revision removes wrap/unwrap support for ChaCha20 and 
>>> ChaCha20-Poly1305 until there is a standardized key wrapping approach 
>>> for these ciphers (similar to how AES has its own key wrapping scheme 
>>> in RFC 3394).
>>>
>>> Initializing the cipher with WRAP/UNWRAP mode will throw 
>>> UnsupportedOperationException and likewise the wrap/unwrap methods 
>>> will throw IllegalStateException.
>>>
>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.07/
>>>
>>> Thanks,
>>>
>>> --Jamil
>>>
>>>
>>> On 05/16/2018 12:05 PM, Jamil Nimeh wrote:
>>>>
>>>> Round 6.
>>>>
>>>> This brings ChaCha20/ChaCha20-Poly1305 into conformance with 
>>>> Cipher's specification when forms of init that take 
>>>> AlgorithmParameters or AlgorithmParameterSpec are used. Previously, 
>>>> a non-null AP or APS object was required.  In order to better 
>>>> conform to the specification, if a null AP or APS is provided for 
>>>> these ciphers, a random nonce will be generated and the counter will 
>>>> be set to 1, just as is currently done with valid forms of init that 
>>>> don't take an AP or APS object (e.g. Cipher.init(int, Key, 
>>>> SecureRandom) ).  Per the spec in Cipher, this is only true for 
>>>> ENCRYPT_MODE and will throw InvalidKeyException when done in 
>>>> DECRYPT_MODE.
>>>>
>>>> I also added a few test cases that exercise these code paths in the 
>>>> ChaCha20Poly1305Parameters.java test case.
>>>>
>>>> http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.06/
>>>>
>>>> Thanks,
>>>>
>>>> --Jamil
>>>>
>>>>
>>>> On 05/04/2018 07: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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

More information about the security-dev mailing list