<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Jamil,<br>
<br>
<div class="moz-cite-prefix">On 19/05/2018 22:40, Jamil Nimeh wrote:<br>
</div>
<blockquote type="cite"
cite="mid:5bd12247-c693-603b-86df-7239a3bab5f3@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
Hi John, comments below:<br>
<br>
<div class="moz-cite-prefix">On 5/19/2018 1:31 AM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com" moz-do-not-send="true">sha.jiang@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:3890b763-aa91-5829-b594-4d8ac00741d1@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Hi Jamil,<br>
</p>
<div class="moz-cite-prefix">On 19/05/2018 07:27, Jamil Nimeh
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a26fe868-2947-9fc8-91ef-76ce805c5e14@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Hi John,<br>
</p>
<p>Yes, the second call must throw IllegalStateException. See
the class description in javax.crypto.Cipher where it talks
about AEAD modes and AAD processing. It states that all AAD
has to be supplied before update and/or doFinal methods are
invoked. This constraint is also talked about in
Cipher.updateAAD's javadoc.<br>
</p>
</blockquote>
I see.<br>
<br>
I was confused by a corner case that passing an empty byte[] (or
a ByteBuffer that has no remaining byte) to the second call on
Cipher.updateAAD().<br>
In fact, in this case, the second call doesn't invoke the
underlying engineUpdateAAD(), so IllegalStateException won't be
thrown.<br>
</blockquote>
I see what you're talking about, but I can't protect against that
(if it even needs protecting). That zero-length check is in
Cipher so my code is never invoked (as you stated). The upside is
the behavior of ChaCha20 in this case would be consistent with any
other AEAD cipher. Changing that behavior (and I'm not convinced
we should change it) is something that goes beyond the scope of
this JEP.<br>
<br>
I think it is safe in this case to not throw ISE since you're not
adding any AAD data, so for ChaCha20 you would not affect the AAD
length or padding, and any buffer that would affect it will most
definitely cause ISE to be thrown. I think we're OK here (as with
any other AEAD cipher).<br>
</blockquote>
Never mind, I have no concern on this point.<br>
<br>
Best regards,<br>
John Jiang<br>
<blockquote type="cite"
cite="mid:5bd12247-c693-603b-86df-7239a3bab5f3@oracle.com"> <br>
Thanks,<br>
--Jamil<br>
<blockquote type="cite"
cite="mid:3890b763-aa91-5829-b594-4d8ac00741d1@oracle.com"> <br>
Best regards,<br>
John Jiang<br>
<br>
<blockquote type="cite"
cite="mid:a26fe868-2947-9fc8-91ef-76ce805c5e14@oracle.com">
<p> </p>
Thanks for the catch on the double-space.<br>
<br>
--Jamil<br>
<br>
<div class="moz-cite-prefix">On 05/18/2018 04:06 PM, <a
class="moz-txt-link-abbreviated"
href="mailto:sha.jiang@oracle.com" moz-do-not-send="true">sha.jiang@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:6cd74c4b-649d-d7b8-4f1d-2d1ddb07a5c4@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Hi Jamil,<br>
<br>
-- ChaCha20Cipher.java<br>
430 } else if (aadDone) {<br>
431 // No AAD updates allowed after the PT/CT
update method is called<br>
432 throw new
IllegalStateException("Attempted to update AAD on " +<br>
433 "Cipher after
plaintext/ciphertext update");<br>
<br>
Please consider the below case,<br>
cipher.updateAAD();<br>
cipher.update();<br>
cipher.updateAAD();<br>
Should the second call on updateAAD() throw
IllegalStateException?<br>
<br>
Minor: Two spaces between "method" and "is" on line 431.<br>
<br>
Best regards,<br>
John Jiang<br>
</p>
<div class="moz-cite-prefix">On 17/05/2018 03:05, Jamil
Nimeh wrote:<br>
</div>
<blockquote type="cite"
cite="mid:f9ed5e08-0ad8-cacb-ac95-b6796984e844@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Round 6.</p>
<p>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.</p>
<p>I also added a few test cases that exercise these code
paths in the ChaCha20Poly1305Parameters.java test case.<br>
</p>
<p><a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.06/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.06/</a></p>
<p>Thanks,</p>
<p>--Jamil<br>
</p>
<br>
<div class="moz-cite-prefix">On 05/04/2018 07:06 AM, Jamil
Nimeh wrote:<br>
</div>
<blockquote type="cite"
cite="mid:d50c2610-42d9-64f0-8a91-796c2588fcf9@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Round 5.</p>
<p>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.<br>
</p>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.04/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.05</a><br>
<br>
Also the CSR has been finalized and can be found here:<br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8198925"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8198925</a><br>
<br>
--Jamil<br>
<br>
<div class="moz-cite-prefix">On 04/27/2018 02:21 PM,
Jamil Nimeh wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e1877cbe-c013-d7ab-a4c3-8c1bc6a4edd4@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<p>Round 4 of updates for ChaCha20 and
ChaCha20-Poly1305, minor stuff mostly:</p>
<ul>
<li>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).</li>
<li>Added an additional test case for
ChaCha20NoReuse</li>
<li>Made accessor methods for ChaCha20ParameterSpec
final and cleaned up the code a bit based on
comments from the field.</li>
</ul>
<p><a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.04/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.04/</a></p>
<p>Thanks!</p>
<p>--Jamil<br>
</p>
<br>
<div class="moz-cite-prefix">On 04/13/2018 11:59 AM,
Jamil Nimeh wrote:<br>
</div>
<blockquote type="cite"
cite="mid:4cdfa771-b222-2986-b1fa-2a67a89b9212@oracle.com">Round
3 of updates for ChaCha20 and ChaCha20-Poly1305: <br>
<br>
* Removed the key field in ChaCha20 and Poly1305
implementations and only retain the key bytes as an
object field (thanks Thomas for catching this) <br>
<br>
* 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. <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.03/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.03/</a>
<br>
<br>
Thanks, <br>
--Jamil <br>
<br>
On 04/10/2018 03:34 PM, Jamil Nimeh wrote: <br>
<blockquote type="cite">Hello everyone, <br>
<br>
This is a quick update to the previous webrev: <br>
<br>
* 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. <br>
<br>
* Unused debug code was removed from the
ChaCha20Cipher.java file <br>
<br>
* 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. <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.02/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.02/</a>
<br>
<br>
Thanks, <br>
<br>
--Jamil <br>
<br>
<br>
On 03/26/2018 12:08 PM, Jamil Nimeh wrote: <br>
<blockquote type="cite">Hello all, <br>
<br>
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. <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8153028/webrev.01/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jnimeh/reviews/8153028/webrev.01/</a>
<br>
<a class="moz-txt-link-freetext"
href="http://openjdk.java.net/jeps/329"
moz-do-not-send="true">http://openjdk.java.net/jeps/329</a>
<br>
<br>
Thanks, <br>
--Jamil <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>