Telechat Review of draft-ietf-ipsecme-rfc7321bis-05
review-ietf-ipsecme-rfc7321bis-05-secdir-telechat-huitema-2017-03-15-00

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

The document revises RFC 7321. It updates the Cryptographic Algorithm 
Implementation Requirements for ESP and AH. The new recommendations
emphasize using authenticated encryption, support for 256 bit keys,
and modern algorithms. The recommendations appear sound, even if there
are some compromises made, in particular in the name of IoT.

This document is ready for publication as a Proposed Standard, with some nits.

My first set of nits concerns the keywords "SHOULD+", "SHOULD-" and "MUST-" that the 
draft is defining. These keywords would make an interesting update to RFC 6919. I 
understand that "SHOULD+" and "MUST-" are already defined and used in RFC 7321, and 
that you have at places to say "RFC 7321 said SHOULD+, but we change that to MUST". 
So maybe you should just note that "RFC 7321 defines these additional keywords".
Also, you never use SHOULD- in the text, so there is no need to define it. You
only MUST- once, to downgrade "AUTH_HMAC_SHA1_96" from MUST to MUST-. You could save 
space in the document by just adding a footnote for the SHA1 entry in the table,
stating that "this MUST means REALLY SHOULD NOT." Or maybe you really want to
update RFC 6919.

The second set of nits contains manual keying. According to the draft, anyone using 
manual keying is punished for doing so, and will only be able to use ENCR_AES_CBC. I 
assume that there is WG consensus on that, but the justification that other algorithms
really require IKEv2 is a bit weak. If there is an API to configure encryption with the
results of an IKEv2 negotiation, it could just as well be used with the results of a
manual configuration. Not sure that the definition of algorithms is the best place to
deprecate manual configuration, even if there are some pretty good reasons to do that.

My last set of nits concerns the relation between this draft and the IANA registry of 
Internet Key Exchange Version 2 (IKEv2) Parameters. I understand that we have five states 
of specification or recommendation:
* MUST implement and MAY use algorithms such as ENCR_AES_GCM_16;
* SHOULD implement and MAY use algorithms such as ENCR_CHACHA20_POLY1305;
* SHOULD implement and MAY use but only for IoT devices, e.g. ENCR_AES_CCM_8;
* SHOULD NOT implement and MUST NOT use, e.g. ENCR_DES; Presumably, the code 
  has to be yanked from existing implementations.
* and then MAY implement and MAY use, e.g. ENCR_CAMELLIA_CBC.
But the fifth category is only specified by default, as "those algorithms that
are listed in the IANA registry but not referenced in the draft." I wonder whether there
is a way to express that clearly in the draft.

-- Christian Huitema