RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

Tue May 8 18:13:01 UTC 2018

Okay, let me fix the code and the CSR and make them match.  I'll put the 
CSR back in draft and make a note in the comments about what is 
changing.  This is an innocuous change, but if Sean feels that code-wise 
the final class should not have final methods, then making the CSR match 
the fixed code sounds like the right way to go.

--Jamil

On 5/8/2018 11:03 AM, Roger Riggs wrote:
> 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>