Re: [IPsec] Mahesh Jethanandani's No Objection on draft-ietf-ipsecme-ikev2-auth-announce-09: (with COMMENT)

[Valery Smyslov <[e-mail]>]

Hi Mahesh,

 

please, see inline.

 

HI Valery,

 

Thanks for responding to my comments. 

 

Some of these comments are generated by a tool we use, and as it says, you
should feel free to ignore them if they are not applicable. Please see
inline for the remaining.





On Apr 11, 2024, at 12:56 AM, Valery Smyslov <[e-mail]
<mailto:[e-mail]> > wrote:

 

Hi Mahesh,

thank you for your comments, please see inline.




Mahesh Jethanandani has entered the following ballot position for
draft-ietf-ipsecme-ikev2-auth-announce-09: No Objection

When responding, please keep the subject line intact and reply to all email
addresses included in the To and CC lines. (Feel free to cut this
introductory
paragraph, however.)


Please refer to
https://www.ietf.org/about/groups/iesg/statements/handling-ballot-
positions/
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ipsecme-ikev2-auth-announce/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks to Reese Enghardt for the General Area Review Team (Gen-ART) review
to Rifaat for the SECDIR review, and to Marc for the ARTART review.

Section 3.1, paragraph 14



  If the responder has sent any CERTREQ payload in the IKE_SA_INIT,
  then it MUST re-send the same payload(s) in the IKE_INTERMEDIATE
  response containing the SUPPORTED_AUTH_METHODS notification if any of
  the included Announcements has a non-zero Cert Link field (see
  Section 3.2.2 and Section 3.2.3).  This requirement allows peers to
  have a list of Announcements and a list of CAs in the same message,
  which simplifies their linking (note, that this requirement is always
  fulfilled for the IKE_SA_INIT and IKE_AUTH exchanges).  However, if
  for any reason the responder doesn't re-send CERTREQ payload(s) in
  the IKE_INTERMEDIATE exchange, then the initiator MUST NOT abort
  negotiation.  Instead, the initiator MAY either link the
  Announcements to the CAs received in the IKE_SA_INIT response, or MAY
  ignore the Announcements containing links to CAs.


I am a little puzzled by the MUST at the beginning of the paragraph which
insists that CERTREQ payload should be sent in IKE_INTERMEDIATE response
and
the MUST NOT/MAY at the bottom of the paragraph, which seems to be ok with
not
re-sending the CERTREQ payload. Is it possible that the CERTREQ payloads are
not re-send and at the same time they do not fit in IKE_SA_INIT (without
being
fragmented)?


Good point, thank you. We can s/MUST/SHOULD.

The idea is to make initiator's task of linking auth announcements to CAs
simpler,
by always having them in one message. On the other hand, responder may
have its own considerations about re-sending CERTREQ in the
IKE_INTERMEDIATE.




The IANA review of this document seems to not have concluded yet.


Hmm, from my understanding, the IANA has already reviewed the draft...

 

You are right. In most cases, IANA will take a look at the IANA
Considerations section, and say they understand the request. I on the other
hand, tend to err on the side of giving more information than less. For
example, in this case what does RFCXXXX refer to? A short note to the RFC
Editor (with another note to say please remove it before publication), would
inform them that RFCXXXX refers to the RFC number that will be assigned to
this document. 

 

         Well, I see your point, but do you think that the current text
would confuse the IANA and the RFC Editor?

         The current text is:

 

   This document defines a new Notify Message Type in the "IKEv2 Notify

   Message Status Types" registry referencing this RFC:

 

     <TBA>       SUPPORTED_AUTH_METHODS    [RFCXXXX]





          It seems to me that the current text is clear enough that RFCXXXX
is this RFC. 

         Do you think more instructions for the RFC Editor are needed?





No reference entries found for these items, which were mentioned in the
text:
[RFCXXXX].


I believe the RFC Editor will change XXXX this to the appropriate value.




Possible DOWNREF from this Standards Track doc to [IKEV2-IANA]. If so, the
IESG
needs to approve it.


I think that referencing IANA registries is not a DOWNREF.

 

This is an example of the tool trying to figure out where is the reference
(possibly because of the square brackets). You can ignore it.


          OK.





Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

* Term "his"; alternatives might be "they", "them", "their"



Paul Wouters is definitely "he" :-)

 

Another case of the tool giving a false positive. But in general the idea is
to flag use of his, her etc. You get the picture. 


          Sure.





----------------------------------------------------------------------------
---
NIT
----------------------------------------------------------------------------
---

All comments below are about very minor potential issues that you may choose
to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so
there
will likely be some false positives. There is no need to let me know what
you
did with these suggestions.

Section 1, paragraph 3

s/each peer uses/each peer use/


I think the current text is correct.




Section 3, paragraph 1



  particular trust anchors.  Upon receiving this information the peer
  may take it into consideration while selecting an algorithm for its
  authentication if several alternatives are available.


This sentence does not parse for me. When it says, "the peer may take it
into
consideration while ...", I seem to be missing what needs to be taken into
consideration.


Perhaps:

NEW:

  The receiving party may take this information into consideration when
selecting an algorithm for its
  authentication if several alternatives are available.

Is this better?

 

Yes, and thanks.

 

All the comments following this are from the tool, so feel free to ignore.


          Some of them were useful, thank you.

 

         Regards,

         Valery.





Section 3.2, paragraph 6



  If more authentication methods are defined in future, the
  corresponding documents must describe the semantics of the
  announcements for these methods.  Implementations MUST ignore
  announcements which semantics they don't understand.


s/announcements which semantics/announcements whose semantics/


OK.




Reference [RFC2409] to RFC2409, which was obsoleted by RFC4306 (this may be
on
purpose).


On purpose.




Section 1, paragraph 2



or implementations, especially if so called hybrid schemes are used (e.g. se
                                 ^^^^^^^^^

The expression "so-called" is usually spelled with a hyphen.


Fixed (caused by my native language experience - in Russian no hyphen is
used in this case).




Section 3.1, paragraph 6



E exchange, defined in [RFC9242] (i.e. the responder has received and is
goin
                                     ^^

It seems like there are too many consecutive spaces here.


This is a result of xml2rfc conversion. There are no extra spaces in the
xml.




Section 3.1, paragraph 8



st to be sent in. This would allow to use IKE fragmentation [RFC7383] for
lon
                                  ^^^^^^

Did you mean "using"? Or maybe you should add a pronoun? In active voice,
"allow" + "to" takes an object, usually a pronoun.


OK, s/to use/using




"I", paragraph 6



field, and the Notify Message Type is set to <TBA by IANA>. The Notification
                                   ^^^^^^

You have used the passive voice repeatedly in nearby sentences. To make your
writing clearer and easier to read, consider using active voice.


Not that I disagree with you (and actually, as a non-native speaker, I
really appreciate these comments),
but in this case I'd rather leave it to the RFC Editor.




Section 3.2, paragraph 2



uthentication methods are defined in future, the corresponding documents
must
                                 ^^^^^^^^^

The phrase "in future" is British English. Did you mean: "in the future"?


Fixed (and I will try to remember this particular difference between British
English and American English).




Section 3.2, paragraph 6



ormat is used. This format allows to link the announcement with a particular
                                 ^^^^^^^

Did you mean "linking"? Or maybe you should add a pronoun? In active voice,
"allow" + "to" takes an object, usually a pronoun.


OK, s/to link/linking




Section 8.1, paragraph 5



th-pq-composite-sigs-13>. Appendix A. Examples of Announcing Supported

Authe



                                    ^^

It seems like there are too many consecutive spaces here.


It is xml2rfc which is at fault :-)




Section 8.2, paragraph 5



1), SIGNATURE(RSASSA-PSS:2), SIGNATURE(ECDSA:3)))} IKE_AUTH HDR,

SK {IDi, CE



                                     ^

It appears that a white space is missing.


Not sure where it is missing...

Regards,
Valery.

 


Mahesh Jethanandani

[e-mail] <mailto:[e-mail]>