Hi,
ok that is an good point that if we have more values in the structure
than we use this can make some confusion.
I was only looking from the coding point of view and saw that if i can
use the same structure for booth cases this
can reduce the coding overhead. But i can fully understand your point.
Gruß Thomas
On 4/10/2018 11:37 PM, Jamil Nimeh wrote:
Hello Thomas, et al.,
On 3/26/2018 1:49 PM, Jamil Nimeh wrote:
Hi Thomas, thanks for the feedback
1. Were there guidelines? Not really, though I looked at other
parameter definitions in com.sun.crypto.provider and tried to
follow along the same lines that they do. One thing that should
be changed is the LINE_SEP assignment shouldn't be an explicit
getProperty call. I noticed most are doing
System.lineSeparator() so I'll change my implementation to match
that. None of these params appear to stringify as json, so I'll
probably keep things consistent with the other parameter output.
2. You make a fair point with respect to a null SecureRandom. I can
make that spec change.
3. Let me think on this one - I shied away from
ChaCha20ParameterSpec for AEAD mode only because you have this
nonce field that is set but gets ignored. But making
ChaCha20ParameterSpec an IvParameterSpec potentially runs into
the same issue were it used for a ChaCha20-Poly1305 cipher. If I
had to choose between the two I think I'd go with allowing
ChaCha20ParameterSpec to be used with CC20-P1305 rather than
making it a subclass of IvParameterSpec. Doing the former helps
from a type safety perspective that you couldn't use a
ChaCha20ParameterSpec with other Ciphers that require an
IvParameterSpec. I know I had some discussions early on in the
design where we talked about this, I need to refresh my memory as
to why we didn't allow it.
Finally getting back to #3. Took me a while to find early discussions
on this. The primary objection to ChaCha20ParameterSpec being used
with ChaCha20-Poly1305 (as opposed to plain old ChaCha20) has to do
with the configurable block counter. You have this parameter that is
not used, and consumption of this type of AlgorithmParameterSpec then
leaves it to documentation to define what happens (is it ignored?
Used despite what the spec says? Set to some default value regardless
of what the caller sets there?). Using IvParameterSpec with
ChaCha20-Poly1305 is more clear because it only allows the caller what
they need to get CC20/P1305 going, the nonce. Respectfully, I would
like to keep this as-is.
--Jamil
On 3/26/2018 12:45 PM, Thomas Lußnig wrote:
Hi Jamil,
1) where there any guidelines about how the engineToString should be
formatted ?
I ask because i wondering why we need two new lines with access to
the System property.
If it is represented as single line json no need to line break would
be needed.
Gruß Thomas
/** * Creates a formatted string describing the parameters. * *
@return a string representation of the ChaCha20 parameters. */
@Override protected String engineToString() { String LINE_SEP =
System.getProperty("line.separator"); HexDumpEncoder encoder = new
HexDumpEncoder(); StringBuilder sb = new StringBuilder(LINE_SEP +
"nonce:" + LINE_SEP + "[" + encoder.encodeBuffer(nonce) + "]");
return sb.toString(); }
2) I do not think it is an good idea to say no secureRandom=null will cause IV
to be null.
I see here the risk of weak implementations. I would suggest to throw an
Exception to
enforce secure usages. If someone really want an insecure IV he can provide
am SecureRandom
implementation retuning 0 only or an matching IV.
* @param random a {@code SecureRandom} implementation. If {@code null}
* is used for the random object, then a nonce consisting of all
* zero bytes will be used. Otherwise a random nonce will be
* used.
3) If ChaCha20ParameterSpec would extends IvParameterSpec if would be valid for
booth modes in engineInit.
Even if the counter is not needed.
As an alternative i would allow ChaCha20ParameterSpec also for AEAD mode.
Grup Thomas
On 3/26/2018 9: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
