RFR: ChaCha20 and ChaCha20/Poly1305 Cipher implementations

[Thomas Lußnig]

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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180411/b18f9cc9/attachment.htm>