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