Re: [IPsec] Please review draft-ietf-ipsecme-chacha20-poly1305

[Yoav Nir <[e-mail]>]

Hi, Valery.

Thanks for the review. See my reply inline.


> On Apr 21, 2015, at 7:19 PM, Valery Smyslov <[e-mail]> wrote:
> 
> Hi,
> 
> this is my review of draft-ietf-ipsecme-chacha20-poly1305-02.
> 
> I think that the draft is in a good shape. A few issues need to be resolved.
> 
> 
> 
> Technical issues.
> 
> 1. For the question raised in the draft:
> 
>   TBD: do we want an extra 32 bits as salt for the nonce like
>   in GCM, or keep the salt (=SenderID) at zero?
> 
> I prefer to follow GCM-like approach, i.e. to take these 32 bits
> from the KEYMAT. It is cryptographically sound and is good
> from evaluation point of view - you know where these bits come from.

I thought so as well. In the meantime, the TLS working group is discussing the same thing for TLS 1.3, and they are proposing to get rid of the salt (or IV) for AES-GCM as well as ChaCha20. 

http://www.ietf.org/mail-archive/web/tls/current/msg15884.html

The TLS working group is not making decisions for us, but I think it would be a shame to arrive at different conclusions where the cases are very similar. More technically, I use a single crypto library for both TLS and IPsec, and having a “salt" parameter that is always set to zero for TLS makes me sad.

> I understand that it would require some adaptation
> for multi-sender case, like in RFC6054, but AFAIK currently
> Many-to-Many IPsec SAs are rare and look exotic.
> And as RFC6054 states, the nonce is not transferred in packet,
> so it is not the best place for SenderID (unlike explicit IV).

As RFC 6045 says in section 3, the sender ID is carved out of the explicit IV, not out of the invisible salt. So that part of my question made no sense anyway. We either want the salt to protect against something, or we don’t. It’s not reserved for sender ID.

> 2. Section 4.
> 
>  When negotiating the ChaCha20-Poly1305 algorithm for use in IKE or
>  IPsec, the value xxx (TBA by IANA) should be used in the transform
>  substructure of the SA payload as the ENCR (type 1) transform ID.  As
>  with other AEAD algorithms, INTEG (type 3) transform substructures
>  SHOULD NOT be specified unless at least one of the ENCR transforms is
>  non-AEAD.
> 
> The last sentence is wrong. Section 3.3 of RFC7296 states:
> 
>  Combined-mode ciphers include
>  both integrity and encryption in a single encryption algorithm, and
>  MUST either offer no integrity algorithm or a single integrity
>  algorithm of "NONE", with no integrity algorithm being the
>  RECOMMENDED method.
> 
> So INTEG either MUST NOT be present in the proposal substructure
> containing ChaCha20-Poly1305 or MUST be NONE. And RFC7296
> requires not to mix AEAD and non-AEAD algorithms in the same proposal:
> 
>  If an initiator wants to propose both combined-
>  mode ciphers and normal ciphers, it must include two proposals: one
>  will have all the combined-mode ciphers, and the other will have all
>  the normal ciphers with the integrity algorithms.

Thanks. I forgot about this restriction. Will fix in the next draft.


> 
> Editorial issues.
> 
> 1. Section 2, first bullet.
> 
>  o  The Initialization Vector (IV) is 64-bit, and is used as part of
>     the nonce.  The IV MUST be unique for each SA but does not need to
>     be unpredictable.  The use of a counter or an LFSR is RECOMMENDED.
> 
> The IV MUST be unique for each invocation (ESP packet or IKEv2 message),
> not for each SA.

Unique for each invocation *with the same key, no?  And keys are different from one SA to another.


> 2. Section 2, last but one bullet:
> 
>     *  The length of the additional data in octets (as a 64-bit
>        little-endian integer).
> 
> "additional data" is a bit vague, please use "Authenticated Additional Data"
> (or "AAD") for clarity.

OK.

> 3. Informative References
> 
>  [FIPS-46]  National Institute of Standards and Technology, "Data
>             Encryption Standard", FIPS PUB 46-2, December 1993,
>             <http://www.itl.nist.gov/fipspubs/fip46-2.htm>.
> 
> The url is incorrect, it leads to the NIST home page.
> I believe the correct url is
> http://csrc.nist.gov/publications/fips/archive/fips46-3/fips46-3.pdf
> (and it is FIPS 46-3, that superceeded FIPS 46-2).

It worked when I first wrote the draft. Will fix.

> 
> Minor nits (subjective).
> 
> 1. Section 1.
> 
> I think the main problem with 3DES is not that it is significantly slower
> than AES, but that it has blocksize of 64 bits, that is considered
> loo small for high-speed networks, when the possibility of birthday attack
> leads to necessity to frequently rekey.

It’s hard to make that case. The blocksize is 64 bits. So it’s prudent to not use more than, say, a billion blocks. A billion blocks is 64 Gb. There are very few real tunnels that run that kind of throughput in under a minute. OTOH it’s no problem at all to run a CreateChildSA every minute, or even every five seconds. So I think there are very few cases that *can’t* use 3DES.


> 2. Section 2.
> 
> From the description of how ESP packet is constructed it is not
> absolutely clear what parts of the nonce are explicitely included in
> the packet, and what aren't. I guess it is 64 bit IV that is included
> and the higher 32 bits (SenderID) - that aren't, but it is not explicitely
> stated. I'd prefer to it to be spelled out.

OK. I’ll fix that when we decide what to do with the salt.


> 3. Section 3.
> 
> The last sentence is too long and refers to different sections in different
> documents (this draft and RFC5282). That makes it a bit difficult to follow.
> Could you, please, split it into several shorter sentences.

I guess I can split that.

> 4. Section 5.
> 
> The terms "64-bit cipher", "128-bit cipher" and "256-bit cipher"
> look like a jargon (and could be mixed up with key length by naive reader).
> I'd prefer to use "block cipher with 64-bit block" etc.

I think I’ll just drop everything after "Counters and LFSRs are both acceptable ways of generating unique nonces.” Encrypting counters gets the job done, but is a very inefficient way of generating unique IV. Especially if it means bringing DES into the mix.


> 5. Section 6.
> 
> I think it is worth to instruct IANA that tis document must be referenced
> in both ESP and IKEv2 columns of IANA registry.
> However it is not strictly necessary since IANA will ask this question anyway.

OK.

Yoav