Early Review of draft-ietf-ipsecme-g-ikev2-08
review-ietf-ipsecme-g-ikev2-08-secdir-early-housley-2023-04-14-00

I 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 authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Document: draft-ietf-ipsecme-g-ikev2-08
Reviewer: Russ Housley
Review Date: 2023-04-05
IETF LC End Date: 2022-04-30
IESG Telechat date: Unknown

Summary: Not Ready

Major Concerns:

Throughout: RFC 7296 says that SK operations are "Encrypted and
Authenticated".  However, several places we see SK{}.  What is being
authenticated and encrypted in such cases.  Similarly, there are 
several places where all of the items in SK{ ... } are optional.
What is being authenticated and encrypted if they are all absent?
Maybe I am missing something, but I think some discussion of the
notation would be helpful.

Section 1: RFC 3740 defines a GSA as:

   A bundling of Security Associations (SAs) that together define how
   a group communicates securely.  The GSA may include a registration
   protocol SA, a rekey protocol SA, and one or more data security
   protocol SAs.

However, this introduction only talks about two aspects of the GSA.
Please state which data security protocol will be used.  I assume it
supports both ESP and AH.

Section 1: The IKE_INTERMEDIATE exchange is discussed later in the
document, but the Introduction does not lay the ground work for it.

Section 2.4.1.1: Please provide some explanatory text prior to:

   DataToAuthenticate = A | P
   GsaRekeyMessage = GenIKEHDR | EncPayload
   GenIKEHDR = [ four octets 0 if using port 4500 ] | AdjustedIKEHDR
   AdjustedIKEHDR =  SPIi | SPIr |  . . . | AdjustedLen
   EncPayload = AdjustedEncPldHdr | IV | InnerPlds | Pad | PadLen | ICV
   AdjustedEncPldHdr = NextPld | C | RESERVED | AdjustedPldLen
   A = AdjustedIKEHDR | AdjustedEncPldHdr
   P = InnerPlds

Section 2.4.1.2: The following seems impossible to implement:

   *  The GCKS must always use IKE fragmentation based on a known
      fragmentation threshold (unspecified in this memo), as there is no
      way to check if fragmentation is needed by first sending
      unfragmented messages and waiting for response.

There is no hint about how to learn the fragmentation threshold, but
the GCKS MUST NOT use fragmentation unless the fragmentation threshold
is known.

Section 4.4.2.1.1: It says: "To specify the details of the signature
algorithm a new attribute Algorithm Identifier (<TBA by IANA>) is
defined."  Shouldn't this simply reference RFC 7427?

Section 4.5.1: Some key wrap algorithms do not have an an explicit IV.
See RFC 3394 for one example.  How is a zero-length IV handled?

Section 5: It says:

   S     A single attribute of this type must be present

   M     Multiple attributes of this type may be present

   []    Attribute is optional

   -     Attribute must not be present

I think these should be MUST, MAY, OPTIONAL, and MUST NOT statements.


Minor Concerns:

Section 1.2: Please add some punctuation between the term being
defined and the definition.  Further, it would be very helpful to
the reader if you say the source of each defined term, instead of
the catch all phrase at the top of the list of defined terms.
Section 2.2 uses "--" for this purpose.

Section 1.2: Please add "CPI", "LKH", and "NULL Authentication" to
the definitions.

Section 2.1.1, 2nd paragraph: I cannot parse it. I think it is trying
to say:

   Section 2.23 of [RFC7296] describes how IKEv2 deals with NATs.
   G-IKEv2 registration doesn't create any unicast IPsec SAs, thus if
   a NAT is present between the GM and the GCKS, there is no unicast
   ESP traffic to encapsulate in UDP.  However, the actions described
   in this section regarding the IKE SA MUST be honored.  If the GM
   and the GCKS use UDP port 848 for the IKE_SA_INIT exchange, they
   MUST behave in the same manner as if they had used UDP port 500.

Section 2.3.2: It says: "When a secure channel is already established
between a GM and the GCKS, ...".  I think it would be more clear to
say: "After the successful completion of the GSA_AUTH exchange, ...".
If there is some detail that is not captured by this alternate wording,
then that detail needs to be explained in this first paragraph of this
section.

Section 2.3.3: The text mixes the terms "GM" and "initiator". It would
be more clear to use one term throughout.

Section 2.3.3: It says:

   In the simplest case when no dependency between transforms exists,
   all Proposals in SAg payload will have the same value in Proposal Num
   field.

This is a contradiction to other text, especially this: "Proposals
of different types having the same value in Proposal Num field are
treated as a set.".  So, if there are no dependencies at all, I would
expect each proposal to have a unique value in Proposal Num field.

Section 2.3.3, last paragraph: It is very hard to understand.  Perhaps:

   Once a GM has received GIKE_REKEY policy during a registration, the
   IKE SA MAY be closed.  By convention, the GCKS closes IKE SA.  The
   GKCS MAY choose to keep the IKE SA open for inband rekeying,
   especially for small groups.  If inband rekeying is used, then the
   initial IKE SA can be rekeyed with the standard IKEv2 mechanism
   described in Section 1.3.2 of [RFC7296].  If for some reason the
   IKE SA is closed before a GIKE_REKEY policy is received during the
   registration process, the GM MUST consider itself excluded from the
   group.  To continue participating in the group, the GM needs to
   re-register.

Section 2.3.4: s/The GCKS may also respond with an INVALID_GROUP_ID/
                /The GCKS SHOULD respond with an INVALID_GROUP_ID/

Section 2.4: Please add some punctuation between the type of group
maintenance channel and the description of that channel.  Section 2.2
uses "--" for this purpose.

Section 2.4.1: It says:

   Since this rekey does not require a response and it sends
   to multiple GMs, G-IKEv2 rekeying MUST NOT use IKE SA windowing
   mechanism, described in Section 2.3 of [RFC7296].

I was not able to get the meaning on first reading.  Perhaps:

   Since the Rekey message does not require a response and it is sent
   to multiple GMs, the GCKS MUST NOT use the IKE SA windowing
   mechanism described in Section 2.3 of [RFC7296] for the Rekey
   message.

Section 2.4.1.4: It says:

   When a group member receives the Rekey Message from the GCKS it
   decrypts the message using the current KEK, validates its
   authenticity using the key retrieved in a previous G-IKEv2 exchange
   if AUTH payload is present, verifies the Message ID, and processes
   the GSA and KD payloads.

It is unclear which part of the processing depends on the presence of
the AUTH payload.  Please reword to be clear which parts are always
done and which parts are done only if the AUTH payload is present.

Section 3.3: It says:

   ...  In particular, the GM's task
   is to find a way to decrypt at least one of the SA_KEY attributes
   using either the GSK_w or the keys from the WRAP_KEY attributes that
   are present in the same message or were receives in previous
   messages.

This is really a transition sentence to prepare the reader for the
following paragraphs. On my first reading, it sounds more like a
challenge or goal.  I suggest:

   ...  The GM decrypts at least one of the SA_KEY attributes using
   either the GSK_w or the keys from the WRAP_KEY attributes that
   are present in the same message or were received in previous
   messages as described below.

Section 3.4: The text talks about "first bits", which is ambiguous.
Perhaps "most significant bits" or "leftmost bits in Network Byte
Order".

Section 4: I find the structure of this section difficult.  The
discussion mixes the payloads that are reused from IKEv2 and the new
ones that are defined for G-IKEv2.  I think it would be easier to
follow if the ones that are reused from IKEv2 were discussed in a
subsection and then a separate subsection talked about the new
payloads.  The identifiers for the new payloads have already been
assigned by IANA, but the IANA registries point to draft-yeung-g-ikev2.

Section 4.5: I find the term "Key Packets" confusing.  I suggest a
better term might be "Wrapped Keys".  Similarly, the use of "packet"
should be changed in "Group Key Packet" and "Member Key Packet".

Section 6.1: Its says: "Below are possible scenarios involving using
PPK."  Some of them do not use PPK.  Please reword.

Section 7.2.1: I understand that implicit authentication doesn't
provide source origin authentication, but there is a bit more that
can be said here.  Can the GM determine that each message came from
the same entity, even if the GM cannot be sure that entity is the
GCKS? 


Nits:

All: Some places the document uses "Data Security SAs" and other
places it uses "Data-Security SAs".  Please pick one.

Section 1: It says: "... that allows to perform a group key ...".
What entity does it allow to perform group key management?  I think
is should say: "... that allows the GCKS to perform a group key ...".

Section 1: It says: "... (see Appendix A of [RFC7296]."  Please add a
closing parenthesis.

Section 1: It says: "Large and small groups may use different sets of
these protocols."  I think that "protocols" is the wrong word.  I think
that "exchanges" is a better word.

Section 1.2: s/[RFC4301], its extension/[RFC4301], and its extension/

Section 1.2: s/good understanding/an understanding/

Section 2.1: s/However some/However, some/

Section 2.1.1: s/As IKEv2 extension G-IKEv2/As IKEv2 extension, G-IKEv2/

Table 1 caption: s/the protocol/G-IKEv2/

Section 2.3: s/The registration protocol consists/Registration consists/

Section 2.3.3: s/Section 2.5)/Section 2.5/

Section 2.3.3: s/AEAD and non-AEAD transforms must not be combined in/
                /AEAD and non-AEAD transforms not be combined in/

Section 2.3.3: It says: "... those of them that must be used ...".
Please reword.  The use of the word "must" here caused me to miss the
meaning on first reading.  Perhaps:

   Use of the same value in the Proposal Num field of different
   proposals indicates that the GM expects these proposals to be
   used in conjunction with each other.

Section 2.3.3: s/SHOULD NOT close IKE SA/SHOULD NOT close the IKE SA/

Section 2.3.4: s/Data-Securirt/Data-Security/

Section 2.3.4: s/GCKS may have no need/GCKS may have no further need/

Section 2.4.1: s/Rekey securely, usually using/Rekey, usually using/

Section 2.4.1.1: s/The chunk A lasts from/The chunk a starts with/

Section 2.4.1.1: s/to the last octet/and continues to the last octet/

Section 2.4.1.2: s/ messages, however when/ messages; however, when/

Section 2.4.1.3: s/must have the Message ID 0/MUST use Message ID of 0/

Section 3.2: s/individual GMs' keys/keys for each GM./

Section 3.3: s/GCKS's key management method/
              /key management method use by the GCKS/

Section 4: The punctuation needs corrections.  I suggest:

   ... However, the processing of some payloads are
   different.  Several new payloads are defined: Group Identification
   (IDg), Group Security Association (GSA), and Key Download (KD).

Section 4.4.2.1: s/Method (AUTH)and/Method (AUTH) and/

Section 4.5: s/a keys and/keys and/

Section 7.1: s/in [RFC7296] section 5 Security Considerations/
              /in the Section 5 of [RFC7296]/


Note: I did not review the Appendix.