RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

Jamil Nimeh jamil.j.nimeh at oracle.com
Tue May 22 04:07:32 UTC 2018 

Hi John, comments in-line

On 5/21/2018 8:30 PM, sha.jiang at oracle.com wrote:
>
> Hi Jamil,
>
> -- ChaCha20Cipher.java
>  497     /**
>  498      * Perform additional initialization actions based on the key 
> and operation
>  499      * type.
>  500      *
>  501      * @param opmode the type of operation to do.  This value 
> must be either
>  502      *      {@code Cipher.ENCRYPT_MODE} or {@code 
> Cipher.DECRYPT_MODE}
>  503      * @param key a 256-bit key suitable for ChaCha20
>  504      * @param newNonce the new nonce value for this initialization.
>  505      *
>  506      * @throws UnsupportedOperationException if the {@code 
> opmode} parameter
>  507      *      is {@code Cipher.WRAP_MODE} or {@code Cipher.UNWRAP_MODE}
>  508      *      (currently unsupported).
>  509      * @throws InvalidKeyException if the {@code opmode} 
> parameter is not
>  510      *      {@code Cipher.ENCRYPT_MODE} or {@code 
> Cipher.DECRYPT_MODE}, or
>  511      *      if the key format is not {@code RAW}.
>  512      */
>  513     private void init(int opmode, Key key, byte[] newNonce)
>  514             throws InvalidKeyException {
>  515         if ((opmode == Cipher.WRAP_MODE) || (opmode == 
> Cipher.UNWRAP_MODE)) {
>  516             throw new UnsupportedOperationException(
>  517                     "WRAP_MODE and UNWRAP_MODE are not currently 
> supported");
>  518         } else if ((opmode != Cipher.ENCRYPT_MODE) &&
>  519                 (opmode != Cipher.DECRYPT_MODE)) {
>  520             throw new InvalidKeyException("Unknown opmode: " + 
> opmode);
>  521         }
> With my understanding, WRAP_MODE and UNWRAP_MODE can be allowed, 
> though they are not supported yet. But the doc on param opmode stats 
> the allowed modes are only ENCRYPT_MODE and DECRYPT_MODE.
> In fact, the doc on param opmode in method engineInit(int opmode, Key 
> key, SecureRandom random) implies that WRAP_MODE can be accepted.
>
JN: Given that Cipher.init and CipherSpi.init both state that UOE should 
be thrown if WRAP/UNWRAP_MODE is provided and the underlying CipherSpi 
does not support those modes, you should NOT accept those values for the 
opmode parameter.  If and when we do support those modes of operation, 
then I will loosen the check and allow it.  But you shouldn't allow a 
Cipher object to be initialized with a mode that you know isn't 
supported.  I would much rather see things behave in a fail-fast manner 
where it is plausible, and this is one of those cases.

I will clean up the method header comments for engineInit to reflect the 
change I made down in the private init common method.
>
>  242      * @param opmode the type of operation to do.  This value may 
> not be
>  243      *      {@code Cipher.DECRYPT_MODE} or {@code Cipher.UNWRAP} 
> mode because
>  244      *      it must generate random parameters like the nonce.
> BTW, Cipher.UNWRAP or Cipher.UNWRAP_MODE?
>
It should be UNWRAP_MODE, good catch.
>
>
> Best regards,
> John Jiang
>
> On 22/05/2018 02:36, 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

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

More information about the security-dev mailing list