Hi Mike, thank you for your review, please see inline.
> Mike Bishop has entered the following ballot position for > draft-ietf-ipsecme-ikev2-qr-alt-08: 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-qr-alt/ > > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Thanks for working on this document. It's in good shape; I have a few minor > issues and several nits that can be incorporated at the discretion of the > author and the responsible AD. > > In Section 3.1, given the MUST abort, I'm guessing the "should" is not a > SHOULD. Consider "should treat this as" => "treats this as" (or "MUST treat > this as a fatal error and abort...") Changed to: ... then the initiator MUST treat this as a fatal and abort the IKE SA establishment. > In Section 4, the terms "QR" and "CRQC" need to be introduced with an > expansion > on first use, or better yet, simply expanded and the abbreviation not used. Good catch. I changed: s/QR protection/quantum secure (the sentence is changed too based on your comment below) I also expanded CRQC in the Introduction, where the expanded term was used, but no abbreviation was provided. > ===NITS=== > > Section 1: > > "it is the IPsec traffic the one that mostly needs to be protected." => "it is > the IPsec traffic that most needs to be protected." This text was also commented by Gorry and after some discussion with him we changed it to: At the time this extension was being developed, the consensus in the IPsecME WG was that the IPsec traffic was more important to be protected than the IKE traffic. > "This allows to leverage fresh" => Either "This allows implementations to > leverage fresh" or "This allows the use of fresh" Done. > "The initiator that" => "An initiator that" Fixed, thanks. > CURRENT: If the responder is configured with one of the PPKs which IDs were > sent by the initiator and this PPK matches the initiator's one (based on the > information from the PPK Confirmation field), CONSIDER: If the responder > supports any of the PPKs which the initiator proposed, based on the PPK ID and > PPK Confirmation fields, This text was also discussed with Gorry and now changed to: 1. If the responder is configured with a PPK, which ID was among IDs sent by the initiator, and this PPK matches the initiator's one (based on the information from the PPK Confirmation field) ... If it is not too difficult to parse, I'd rather keep it, as it is gives more detailed instructions to implementers. > "returns PPK identity" => "returns a PPK identity" Fixed, thanks. > CURRENT: If the responder does not have any of the PPKs which IDs were sent > by > the initiator or it has some of proposed PPKs, but their values mismatch the > initiator's ones (based on the information from the PPK Confirmation field), > and using PPK is mandatory for the responder, then it MUST return > AUTHENTICATION_FAILED notification and abort creating the IKE SA. > CONSIDER: If > the responder cannot use any of the PPKs proposed by the initiator, based on > the PPK ID and PPK Confirmation fields, it MAY return an > AUTHENTICATION_FAILED > notification and abort creating the IKE SA if configured to require PPK usage. Hmm... This would change MUST --> MAY. I don't think this is an editorial change. Or is it just a typo in the proposed text? Anyway, unless you think that the current text is too difficult to parse, I'd rather keep it because it first lists all the conditions (no appropriate PPK and the local policy requirement) and after that indicates what to do. >From my point of view this is formally the preferred way to give instructions >to implementers. > CURRENT: If the responder does not have any of the PPKs which IDs were sent > by > the initiator or it has some of proposed PPKs, but their values mismatch the > initiator's ones (based on the information from the PPK Confirmation field), > and using PPK is optional for the responder, then it does not include any > PPK_IDENTITY notification to the response. CONSIDER: If the responder cannot > use any of the PPKs proposed by the initiator but it is not configured to > require PPK usage, it does not include any PPK_IDENTITY notification to the > response. This text is already changed a bit based on Gorry's comments: 3. If the responder does not have any PPKs proposed by the initiator or it has some of the proposed PPKs, but their values mismatch the initiator's ones (based on the information from the PPK Confirmation field), and using PPK is optional for the responder, then it does not include any PPK_IDENTITY notification to the response. As above, unless you this this is too difficult to parse, I'd rather to keep the text as it is more detailed. > "abort creating IKE SA" => "abort creating the IKE SA" Fixed, thanks. > "selects PPK" => "selects a PPK" Ditto. > Section 3.1.1: > > "applying PPK" => "applying the PPK" > > "with PPK" => "with the PPK" All fixed (oh, those articles... :-(). > Section 3.2: "supports use PPKs" => "supports using PPKs" OK. > Similar language suggestions from Section 3.1. > > Section 3.2.1: "resulted" => "resulting" OK. > Section 4: > - "Compared to use PPKs in IKE_AUTH this" => "Compared to using PPKs in > IKE_AUTH, this" or "Compared to the use of PPKs in IKE_AUTH, this" Already changed to Unlike using PPKs in IKE_AUTH ... based on Andy's comment. > - "since PPK > is used in authentication too, that gives this exchange a QR protection even > against active attacker." => "since the PPK is used in authentication too, > this > exchange is protected even against an active attacker." Changed to (based on your comment above): ... since the PPK is used in authentication too, this exchange is quantum secure even against active attacker. > Throughout: "Note, that" => "Note that" Done. All changes are made in my local copy. Regards, Valery. _______________________________________________ IPsec mailing list -- [email protected] To unsubscribe send an email to [email protected]
