I'm doing a yet another full review pass over the whole document (not just diff as I've usually done; to find issues/inconsistencies that diffs wouldn't necessarily show). So far, I've covered Sections 1 and 2.
Substantial comments: - Section 1.5: I noticed the 1st paragraph nowadays (well, since -00 of the WG draft) allows sending INVALID_IKE_SPI notification inside an existing IKE_SA. This contradicts a MUST NOT in RFC 4306, and I'm not sure if it really brings any benefits? - Section 2.8.1, NO_PROPOSAL_CHOSEN near the end. This is probably left over from old versions, and should be something else now to align with Section 2.25? - Section 2.8.2: (already covered by issue #135) - Section 2.14: "only the first 64 bits of Ni and the first 64 bits of Nr are used in the calculation". This section has two calculations involving Ni/Nr, but this sentence should only apply to the former. Suggested rephrasing: "the calculation" -> "when calculating SKEYSEED (but all bits are used when calculating SK_*)" - Section 2.17: See http://www.ietf.org/mail-archive/web/ipsec/current/msg04673.html http://www.ietf.org/mail-archive/web/ipsec/current/msg04681.html - Section 2.23, last bullet: "MUST NOT be used when MOBIKE is used" This is actually not exactly what Section 3.8 of [MOBIKE] says (it does allow using this for ESP). I would suggest just referring to Section 3.8 of [MOBIKE] for details. - Section 2.23.1: If the responder doesn't find SPD entry for transport mode with the modified traffic selectors, and does a lookup with the original selectors, if it finds an entry for transport mode, can it use it? (And would that screw up the initiator processing of the reply? Unfortunately,this question is relevant for RFC 5555...) - Section 2.25: "A peer that receives a CHILD_SA_NOT_FOUND notification SHOULD silently delete the Child SA": Is this really necessary? I think this notification should only occur in cases where DELETE payload for this Child SA pair has already been sent, and probably has been processed already by the time the CHILD_SA_NOT_FOUND notification is received. - Section 2.25.2, "If a peer receives a request to delete a Child SA when it is currently rekeying the IKE SA, it SHOULD reply as usual, with a Delete payload." I noticed this is different from what RFC 4718 recommended, but this is probably OK, given the other text... (but I still hope this was intentional change :-) Somewhat substantial: - Section 1.3.1: delete sentence "When an IKE SA is not created..." (this section is about CREATE_CHILD_SA, and the IKE_AUTH error messages are already well described in Section 1.2). - Section 1.5 doesn't read well, and needs a rewrite. I can provide text once the substantial issue re: INVALID_IKE_SPI is decided. - Section 2.6, paragraph beginning with "In addition to cookies..", and Section 2.7, last paragraph, are both in very strange places (and basically say the same thing). Suggest combining both, and adding them to the end of paragraph "In the first message.." in 2.6: "When the exchange does not result in the creation of an IKE SA on the responder (due to, for example, COOKIE, INVALID_KE_PAYLOAD, or NO_PROPOSAL_CHOSEN), the responder's SPI will be zero also in the response message. However, if the responder does send a non-zero responder SPI, the initiator should not reject the response for only that reason." - There are two places that have very similar text about INVALID_KE_PAYLOAD: Section 1.3 (for CREATE_CHILD_SA exchange), and Section 2.7 (for IKE_SA_INIT exchange). Especially the latter seems a bit strange, everything else in that section applies to CREATE_CHILD_SA exchanges, too, but this paragraph explicitly applies to IKE_SA_INIT only (although INVALID_KE_PAYLOAD works roughly the same way in CREATE_CHILD_SA, too). Perhaps the text in 2.7 should be moved to 1.2? - Section 2.8, sentence: "Note that, when rekeying..." This is in wrong place; the rest of this paragraph is about IKE_SA rekeying, so it should be moved to the previous paragraph. - Section 2.18, last paragraph: suggest adding "PRF" to the list of things that come from the new exchange. - Section 2.23, paragraph starting: "An initiator can float..." needs some clarifications. Probably "UDP encapsulation/encapsulated" should be "UDP encapsulation of ESP/UDP-encapsulated ESP" (since at other places, the document also talks about IKE packets being UDP encapsulated). - Section 2.23.1: I would strongly suggest using the word "original address" (just like RFC 3947) instead "real address" (since both addresses are very real). - Section 2.23.1, "- Store the original traffic selectors.." (occurs twice) I would suggest using "received" instead of "original" here. Best regards, Pasi _______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
