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> 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. > >> 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. > >> 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. > >> ------------------------------------------------------------------------------- >> 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. > >> 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
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
