> On Apr 12, 2024, at 12:52 AM, Valery Smyslov <s...@elvis.ru> wrote: > > 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 <s...@elvis.ru >> <mailto:s...@elvis.ru>> 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- >>> <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/ >>> <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?
The current text is fine. As you mention, IANA has already processed the request. Thanks. >> >> >>> 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 >>> <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 >>> <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 > mjethanand...@gmail.com <mailto:mjethanand...@gmail.com> Mahesh Jethanandani mjethanand...@gmail.com
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
