Re: RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

Thu, 31 May 2018 05:56:16 -0700 

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

Reply via email to