Wow, good catch. This is something that got left by the wayside,
probably from one of the earliest revisions of the code when we didn't
have an AlgorithmParameters implementation. I'll get this fixed up
today. Shouldn't take too long to get this working. I think it should
return null if it is ChaCha20 since there's no AlgorithmParameters
definition for ChaCha20, and return a ChaCha20Poly1305Parameters for
ChaCha20-Poly1305.
For the latter Cipher, it shouldn't matter how you init the Cipher. We
can return the AP with the nonce value, however it came into being
(random, AP or APS).
--Jamil
On 05/22/2018 10:29 AM, Sean Mullan wrote:
Question on engineGetParameters() in ChaCha20Cipher.java:
231 protected AlgorithmParameters engineGetParameters() {
232 return null;
233 }
Shouldn't this return the params that are passed into engineInit()?
Also, when null is passed in and the params are generated using
default values, this method should return those params. From
CipherSpi.engineInit(int opmode, Key key, SecureRandom):
"The generated parameters can be retrieved using engineGetParameters
or engineGetIV (if the parameter is an IV)."
--Sean
On 5/21/18 2:36 PM, 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
