RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

[sha.jiang at oracle.com]

Hi Jamil,

-- ChaCha20Cipher.java
  430         } else if (aadDone) {
  431             // No AAD updates allowed after the PT/CT update 
method  is called
  432             throw new IllegalStateException("Attempted to update 
AAD on " +
  433                     "Cipher after plaintext/ciphertext update");

Please consider the below case,
cipher.updateAAD();
cipher.update();
cipher.updateAAD();
Should the second call on updateAAD() throw IllegalStateException?

Minor: Two spaces between "method" and "is" on line 431.

Best regards,
John Jiang

On 17/05/2018 03:05, 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
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180519/62c6c323/attachment.htm>