Re: [IPsec] Secdir early review of draft-ietf-ipsecme-g-ikev2-08

[Russ Housley <[e-mail]>]

> On Apr 18, 2023, at 10:31 AM, Valery Smyslov <[e-mail]> wrote:
> 
> Hi Russ,
> 
> thank you for your review. Please see inline.
> 
>> -----Original Message-----
>> From: Russ Housley via Datatracker [mailto:[e-mail]]
>> Sent: Friday, April 14, 2023 3:56 PM
>> To: [e-mail]
>> Cc: [e-mail]; [e-mail]
>> Subject: Secdir early review of draft-ietf-ipsecme-g-ikev2-08
>> 
>> Reviewer: Russ Housley
>> Review result: Not Ready
>> 
>> 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.
> 
> SK{...} denotes the "Encrypted payload" (Section 3.14 of RFC 7296).
> An empty Encrypted Payload is used in RFC 7296 in several cases - 
> when peer should send something to complete the exchange, but 
> there is really nothing meaningful to send. In reality the content
> of the "empty" payload is not null - it contains the mandatory Pad Length
> field and an optional padding (for counter-based encryption algorithms
> there is no padding). So, when sending SK{}, a peer actually sends 
> one encrypted zero byte (for counter-based modes) and authenticates
> the whole message including the IKE header.
> 
> There is nothing new in G-IKEv2 with this regard comparing to RFC 7296.
> I don't think any clarification about notation is needed - we have dozens of IKEv2 
> extension RFCs which already use this notation with no clarification and in addition 
> readers are supposed to be familiar with RFC 7296. Or should we say this explicitly?

Thanks.  I would say near the beginning that the conventions from  RFC 7296 are used here.  That would have caused me to look more carefully.

>> 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.
> 
> That's true. I've added a clarification:
> 
>          This specification relies on ESP and AH as protocols for Data-Security SAs.

That resolves my concern.

>> Section 1: The IKE_INTERMEDIATE exchange is discussed later in the
>> document, but the Introduction does not lay the ground work for it.
> 
> The introduction is mostly concerned with group key management
> architecture and terminology, since they a bit different from what 
> IPsec folks are accustomed to. IKE_INTERMEDIATE is not specific
> to group key management, so it is not mentioned there. 
> It is mentioned later to make clear that besides IKE_SA_INIT+GSA_AUTH 
> other exchanges, defined as IKEv2 extensions, can also be used.
> 
> If I missed your point, then can you please elaborate?

I think the Introduction needs to says that IKE_INTERMEDIATE can be used and provide a reference.  I was a bit surprised when it came up much later in the document.

>> 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
> 
> This notation follows a similar one from RFC 7296 (Section 2.15)
> and RFC 9242 (3.3.2). 
> 
> We can add some text before this, for example:
> 
> 	The input to the authentication algorithm that computes 
> 	the content of the AUTH payload can be described as:
> 
> Is it enough?

Yes. That resolves my concern.

>> 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.
> 
> It is assumed that fragmentation threshold in this case is pre-configured by administrator.
> 
> Should we clarify this? For example:
> 
>   *  The GCKS must always use IKE fragmentation based on a pre-configured
>      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.
> 
> Is it enough?

This helps, but it does not fully resolve my concern. Based on this, an implementer is supposed to provide a configuration setting for the fragmentation threshold. Some guidance about the value is needed.  If it is too small, then you needlessly fragment.  If too big, some packets will get dropped.

>> 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?
> 
> No, it is irrelevant here. Here the draft discusses the way the GCKS informs the GMs about the authentication
> method it will use for multicast rekey messages. This is accomplished via the newly defined 
> IKEv2 Transform Type "Authentication Method (AUTH)" (not to be mixed with "Authentication Payload (AUTH)"). 
> In case this transform specifies a digital signature algorithm as an authentication method, a newly defined
> IKEv2 Transform Attribute "Algorithm Identifier" inside this transform further specifies which digital signature
> algorithm to use. IKEv2 transforms and attributes and their use in IKEv2 are defined 
> in Sections 3.3.1 and 3.3.5 of RFC 7296.
> 
> RFC 7427 is referenced later in this para, when the content of this attribute is discussed.

Maybe change the name to avoid the same mistake by others: IKEv2 Transform Type "Authentication Method (AUTHMETH)"

>> 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?
> 
> Generally, there is no problem if IV is not used - the draft says:
> 
>   *  IV (variable) -- Initialization Vector used for encryption.  The
>      size and the content of IV is defined by the employed encryption
>      transform.
> 
> So, if the algorithm doesn't require IV, then this field is absent (implicitly has zero length).
> 
> However, RFC 3394 is not relevant in this case. The draft says:
> 
>   The keys are encrypted using algorithm that is used to encrypt the
>   message the keys are sent in.  It means, that in case of unicast IKE
>   SA (used for GMs registration and rekeying using GSA_INBAND_REKEY)
>   the encryption algorithm will be the one negotiated during the IKE SA
>   establishment, while for a GSA_REKEY message the algorithm will be
>   provided by the GCKS in the Encryption Algorithm transform in the GSA
>   payload when this multicast SA was being established.
> 
> It means that the key wrapping algorithm must be from the 
> IKEv2 encryption transforms, i.e. it is must be among those defined in 
> https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-5
> 
> Actually, there are some algorithms with implicit IV in this registry , but they are prohibited 
> for using in IKEv2 (and thus in G-IKEv2), they can only be used in ESP. With regard
> to multicast ESP SAs, there is further restrictions on using these transforms - 
> Replay protection must be on (which implies that there is the only sender for multicast SA).
> This should be explicitly spelled out in the draft, I'll add a new section.
> 
> Bottom line - there is currently no algorithms defined for use in G-IKEv2 rekey SA without explicit IV
> (and I believe algorithms with no/implicit IV must be prohibited there).

Please say that the algorithm MUST use an explicit IV.

>> 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.
> 
> Perhaps. Let it be so.

Thanks.

>> Minor Concerns:
>> 
>> Section 1.2: Please add some punctuation between the term being
>> defined and the definition.  
>> Section 2.2 uses "--" for this purpose.
> 
> Actually, this is more a tooling issue. I tried to use some xml2rfc v3 syntax,
> since using v3 is generally being promoted by IESG.
> With v3 there is a special tag for definitions and this tag doesn't provide
> any means for specifying a punctuation delimiter between the term
> and its definition. The only option is to make definition starting from 
> new line. I'll add the newline attribute if it's OK with you.

Sure. New line would be fine.

>> 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.
> 
> It is a bit difficult. Same terms are often defined differently in RFC 3740,
> RFC 5374, RFC 4046 and RFC 6407. The problem is also that each of those 
> sources doesn't use terms from the previous RFCs, instead it redefines them :-(
> I tried to pick most appropriate ones (from my POV), sometimes compiled 
> a definitions from different sources or truncated them.
> If more strict definitions are needed, then I'd rather ask for help - 
> currently there seems to be a mess in related RFCs.

I understand.  Thanks for considering it.

>> Section 1.2: Please add "CPI", "LKH", and "NULL Authentication" to
>> the definitions.
> 
> LKH is there (the last term).
> 
> For the CPI I prefer to expand on first (and the only) use in the document.
> 
> With regard to "NULL Authentication" -- the more I think about this, 
> the more I'm sure that referring to "NULL Authentication" in the draft
> was a wrong idea. The draft re-uses the code point for NULL Authentication,
> which is wrong. I'll re-write the related text and use value 0 (currently "Reserved",
> can be renamed to "NONE") to refer to the implicit authentication. 
> Thank you for bringing my attention to this issue.

Thanks.  I think the document will be better with these changes.

>> 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.
> 
> This para is already changed as per Gorry proposed. How about (combining his and yours changes):
> 
>          Section 2.23 of [RFC7296] describes how IKEv2 supports paths with NATs.
>          G-IKEv2 registration SA 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. 
>          The behavior of GMs and GCKS MUST NOT depend on the port used to create the initial IKE SA.
>          For example, if the GM and the GCKS used UDP port 848 for the IKE_SA_INIT exchange, they
>          will operate the same as if they had used UDP port 500.
> 
> Is it OK?

Yes. That resolves my concern.

>> 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.
> 
> OK, changed to:
> 
>   After the successful completion of the GSA_AUTH exchange, the GM
>   registration for a group can reuse the established IKE SA.
>   In this scenario the GM will use the GSA_REGISTRATION exchange.
>   Payloads in the exchange are generated and processed as defined in
>   Section 2.3.1.

Yes. That resolves my concern.

>> Section 2.3.3: The text mixes the terms "GM" and "initiator". It would
>> be more clear to use one term throughout.
> 
> Cleaned up (s/initiator/GM).

Thanks.

>> 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.
> 
> There is no contradiction, there is poor wording :-(
> 
> If IKEv2 each Proposal substructure describes transforms for a particular protocol.
> In the case of IKEv2 this would be GIKE_REKEY, AH or ESP. So, the GM will
> send at least two proposals - one for GIKE_REKEY and the other for Data-Security SA,
> let it be ESP. Then, the Proposal Num allows the GM to express, that there
> is a dependency between the transforms for different protocols. For example,
> if the GCKS selects algorithm X for GIKE_REKEY, then it must also select 
> algorithm X for ESP (for example, in case these algorithms are implemented in 
> HSM and it doesn't allow to export derived keys - so you are stick
> to using it  for all protocols). To express this the GM will mark proposal
> for GIKE_REKEY containing transform X and proposal for ESP containing
> transform X with the same Proposal Num. The GM may include
> another proposal for these protocols (say, containing transform Y)
> with a different Proposal Num. 
> Un the simplest case, when no dependency exists, proposals for all protocols 
> will have the same Proposal Num field and each will contain all supported
> transforms for a corresponding protocol. 
> 
> Please, let me know how the wording can be made more clear.
> Currently I made some minor changes for the sake of accuracy:
> 
>   Proposal Num field in Proposal substructure is treated specially in
>   SAg payload: it allows a GM to indicate that algorithms used in Rekey
>   SA and in Data-Security (AH and/or ESP) SAs are dependent.  In
>   particular, Proposals for different protocols having the same value
>   in Proposal Num field are treated as a set, so that if GCKS uses
>   transforms from one of such Proposal for one protocol, then it MUST
>   only use transforms from one of the Proposals with the same value in
>   Proposal Num field for other protocols.  For example, a GM may
>   support algorithms X and Y for both Rekey and Data-Security SAs, but
>   with a restriction that if X is used in Rekey SA, then only X can be
>   used in Data-Security SAs, and the same for Y.  To indicate this the
>   GM sends several Proposals marking those of them that must be used
>   together by putting the same value in their Proposal Num field.  In
>   the simplest case when no dependency between transforms exists, all
>   Proposals in SAg payload will have the same value in Proposal Num
>   field.

Thanks.  This is much more clear.

>> 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.
> 
> OK, will use this text with one correction:
> 
> s/before a GIKE_REKEY policy is received/and no GIKE_REKEY policy is received
> 
> This cannot be "before", since it happens at the time of registration (GSA_AUTH).
> But GCKS may omit sending GIKE_REKEY policy (including keys) to this particular
> GM for some reason (e.g. the GCKS intends to use inband rekey).
> In this case, if IKE SA is closed for any reason, the GM should re-register.

Thanks.  I'm glad you caught my mistake.

>> Section 2.3.4: s/The GCKS may also respond with an INVALID_GROUP_ID/
>>                /The GCKS SHOULD respond with an INVALID_GROUP_ID/
> 
> I don't mind, but in this case in addition: 
> 
> s/it will respond with an AUTHORIZATION_FAILED/it SHOULD respond with an AUTHORIZATION_FAILED

Thanks.  Good improvement.

>> 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.
> 
> See above about the tooling issue. I added an attribute that puts definition on the next line.

Yes. That resolves my concern.

>> 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.
> 
> I prefer:
> 
>    Since the Rekey messages do not require responses and they are sent
>    to multiple GMs, the windowing mechanism described in Section 2.3 
>    of [RFC7296] MUST NOT be used for the Rekey messages.
> 
> Does it work for you?

Yes. That resolves my concern.

>> 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.
> 
> How about the following:
> 
>    When a group member receives the Rekey message from the GCKS it
>    decrypts the message and verifies its integrity using the current KEK. If the AUTH payload is present
>    in the decrypted message, then the GM validates authenticity of the message using the key retrieved 
>    in a previous G-IKEv2 exchange. Then the GM verifies the Message ID, and processes
>    the GSA and KD payloads.

Thanks.  This is much more clear to me.

>> 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.
> 
> I propose:
> 
>    ...  The GM tries to 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.
> 
> (if GM is being excluded from the group using LKH, then it cannot decrypt any SA_KEY,
> but since it doesn't know this, it will try to do it).
> 
> Is it OK?

Yes. That resolves my concern.

>> Section 3.4: The text talks about "first bits", which is ambiguous.
>> Perhaps "most significant bits" or "leftmost bits in Network Byte
>> Order".
> 
> Changed to "leftmost bits" (I believe byte order is irrelevant here since we're talking about strings, not integers).

Okay. That resolves my concern.

>> 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.  
> 
> Hmm, I'm a bit confused. All Section 4 is about new payloads,
> specific to G-IKEv2, with the exception of the G-IKEv2 header,
> which is identical to IKEv2 header. Two payloads (SAg and IDg)
> have the same format as existing IKEv2 payloads, but different semantics.
> 
> The problem with subsections is that in this case the nesting level
> for new payloads will be too deep: currently there exists 4.4.2.1.1.,
> will be one level deeper.
> 
> Perhaps it is sufficient if I add the following text to the beginning of Section 4:
> 
>   G-IKEv2 header (Section 4.1),
>   IDg payload and SAg payload reuse IKEv2 format for the IKEv2 header,
>   IDi/IDr payloads and SA payload respectively.
> 
> Does it help or not?

Maybe I am being confused by this part of the first paragraph:

    ... New
   exchange types GSA_AUTH, GSA_REGISTRATION, GSA_REKEY and
   GSA_INBAND_REKEY are also added.

The addition of "(Section 4.1)" is helpful.

>> The identifiers for the new payloads have already been
>> assigned by IANA, but the IANA registries point to draft-yeung-g-ikev2.
> 
> That's true, they were assigned a long ago.

Yes, but you want the IANA registries to point to this document once it is approved.

>> 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".
> 
> Perhaps "Key Bag"? Or "Key Bundle"?
> 
> By definition Key Packet contains several related keys, 
> and is referred to some substructure, so "Wrapped Key"
> seems to be not a good substitution.
> 
> No changes made so far, waiting for more discussion.

Either "Key Bag" or "Key Bundle" could resolve my concern.  I have a mild preferences for "Key Bag".

>> Section 6.1: Its says: "Below are possible scenarios involving using
>> PPK."  Some of them do not use PPK.  Please reword.
> 
> Changed to:
> 
> 	This is illustrated below.
> 
> I also removed some unnecessary stuff (optional PPK).

This change resolves my concern.

>> 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?
> 
> I don't think it can. With implicit authentication any member of the group
> can impersonate the GCKS. The source IP address can be set to any value,
> since no reply is needed.

Okay. No change needed.

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

Thanks.

>> 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 ...".
> 
> This text is already changed to (by other reviewer's request):
> 
>   This document presents an extension to IKEv2 [RFC7296] called
>   G-IKEv2, which allows performing a group key management.
> 
> Is it OK?

Yes. That resolves my concern.

>> Section 1: It says: "... (see Appendix A of [RFC7296]."  Please add a
>> closing parenthesis.
> 
> Done (also caught by other reviewer).

Okay.

>> 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.
> 
> I prefer "Large and small groups may use different sets of these mechanisms."
> 
> (since GSA_REKEY is not an exchange, it is one-way message).
> 
> Is it better?

Yes. That resolves my concern.

>> Section 1.2: s/[RFC4301], its extension/[RFC4301], and its extension/
> 
> OK.
> 
>> Section 1.2: s/good understanding/an understanding/
> 
> Don't mind.
> 
>> Section 2.1: s/However some/However, some/
> 
> Fixed.
> 
>> Section 2.1.1: s/As IKEv2 extension G-IKEv2/As IKEv2 extension, G-IKEv2/
> 
> Done.
> 
>> Table 1 caption: s/the protocol/G-IKEv2/
> 
> OK.
> 
>> Section 2.3: s/The registration protocol consists/Registration consists/
> 
> Well, let it be so.
> 
>> Section 2.3.3: s/Section 2.5)/Section 2.5/
> 
> Fixed.
> 
>> Section 2.3.3: s/AEAD and non-AEAD transforms must not be combined in/
>>                /AEAD and non-AEAD transforms not be combined in/
> 
> Well, this change is less obvious for my non-native ear, but I trust you here.
> 
>> 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.
> 
> OK, will use this text.
> 
>> Section 2.3.3: s/SHOULD NOT close IKE SA/SHOULD NOT close the IKE SA/
> 
> I cannot find this particular text, probably it is gone in the process of editing.
> Anyway, I got the idea with "the" and fixed whenever found.

Okay.

>> Section 2.3.4: s/Data-Securirt/Data-Security/
> 
> Cannot find, probably fixed earlier.

Okay.

> 
>> Section 2.3.4: s/GCKS may have no need/GCKS may have no further need/
> 
> OK.
> 
>> Section 2.4.1: s/Rekey securely, usually using/Rekey, usually using/
> 
> I understand that the current wording is weird, but shouldn't we emphasis 
> that rekey messages are sent over secure (one-way) channel?
> 
> Perhaps:
> 
>          The GCKS initiates the G-IKEv2 Rekey by sending a protected message to the GMs, 
>          usually using IP multicast.

Thanks. Much better, I think.

>> Section 2.4.1.1: s/The chunk A lasts from/The chunk a starts with/
> 
> OK.
> 
>> Section 2.4.1.1: s/to the last octet/and continues to the last octet/
> 
> Ditto.
> 
>> Section 2.4.1.2: s/ messages, however when/ messages; however, when/
> 
> Done.
> 
>> Section 2.4.1.3: s/must have the Message ID 0/MUST use Message ID of 0/
> 
> Fixed.
> 
>> Section 3.2: s/individual GMs' keys/keys for each GM./
> 
> OK.
> 
>> Section 3.3: s/GCKS's key management method/
>>              /key management method use by the GCKS/
> 
> Sorry, perhaps 
> 
> 	key management method used by the GCKS
> 
> ?

Yes. That is better.

>> 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).
> 
> OK. This text is also addressed in Minor Issues.
> 
>> Section 4.4.2.1: s/Method (AUTH)and/Method (AUTH) and/
> 
> Already fixed.
> 
>> Section 4.5: s/a keys and/keys and/
> 
> Fixed.
> 
>> Section 7.1: s/in [RFC7296] section 5 Security Considerations/
>>              /in the Section 5 of [RFC7296]/
> 
> Done.
> 
>> Note: I did not review the Appendix.
> 
> Thank you for the thorough review.
> 
> The changed made so far are available at: 
> https://github.com/smyslov/G-IKEv2

Russ