Re: [IPsec] Editorial changes to RFC5996

[Yaron Sheffer <[e-mail]>]

Hi Valery,

Sorry for being the Bad Guy on this. Your #6 does not seem editorial to 
me. Similarly, #8 (adding new RFC 2119 language) is not editorial. I 
would suggest to implement #6 only if it is critical to interoperability 
or security, and to forgo #8.

By the way, your correction #2 still does not do it IMHO. The sentence 
refers to RFC 5996. So:

"IKEv2 as stated in RFC 4306 was a change to the IKE protocol that was 
not backward compatible. RFC 5996 revised RFC 4306 to provide a 
clarification of IKEv2, making minimum changes to the IKEv2 protocol. 
The current document slightly revises RFC 5996 to make it suitable for 
progression to Internet Standard."

Thanks,
	Yaron

On 2013-10-18 16:52, Valery Smyslov wrote:
> Hi all,
>
> as RFC5996 is revised, I suggest to make some editorial changes
> to improve its clarity and to fix some small issues that I found.
>
> 1. Section 1.6 Requirements Terminology is placed far from the begining
>     of the document and all the requirements words, along with terms
>     from RFC4301 etc. are used before they are formally introduced.
>     I don't think it is appropriate for standards track document.
>     I suggest to move this section before Introduction.
>
> 2. Page 5:
>    "IKEv2 was a change to the IKE protocol that was not backward
>    compatible.  In contrast, the current document not only provides a
>    clarification of IKEv2, but makes minimum changes to the IKE
>    protocol."
>
> For me this piece sounds strange. I think it should look like:
>
>    "IKEv2 was a change to the IKE protocol that was not backward
>    compatible.  In contrast, the current document only provides a
>    clarification of IKEv2, and makes minimum changes to the IKEv2
>    protocol."
>
> 3. Page 10.
>    "HDR contains the Security Parameter Indexes (SPIs), version numbers,
>    and flags of various sorts."
>
> Not mentioned Message ID, Exchange Type (and Next Payload and length,
> but they are not very important here). I suggest to change:
>
>    "HDR contains the Security Parameter Indexes (SPIs), version numbers,
>    Type of Exchange, Message ID and flags of various sorts."
>
> 4. Page 11.
>    "At this point in the negotiation, each party can generate SKEYSEED,
>    from which all keys are derived for that IKE SA."
>
> Term SKEYSEED pops up from nowhere. No reference is gived and
> even no word is explained what is it all about. First-time readers will
> definitely be puzzled. I suggest to change:
>
>    "At this point in the negotiation, each party can generate a quantity
>     called SKEYSEED (see section 2.14), from which all keys are derived
>     for that IKE SA."
>
> 5. Page 14, 15 and 16
>    "The responder replies (using the same Message ID to respond) with the
>    accepted offer in an SA payload, and a Diffie-Hellman value in the
>    KEr payload if KEi was included in the request and the selected
>    cryptographic suite includes that group."
>
>    "The responder replies (using the same Message ID to respond) with the
>    accepted offer in an SA payload, and a Diffie-Hellman value in the
>    KEr payload if the selected cryptographic suite includes that group."
>
>    "The responder replies (using the same Message ID to respond) with the
>    accepted offer in an SA payload, and a Diffie-Hellman value in the
>    KEr payload if KEi was included in the request and the selected
>    cryptographic suite includes that group."
>
> All three sentencies look like they were copy-pasted and all three
> lacks mention Nonce Payload. I think it should be explicitely
> mentioned here, as it was mentioned in descriptions of Initiator's message,
> above each of this sentencies.
>
> And I also think that words in parentheses here are superfluous,
> as this requirement is comon for all exchanges, not only for
> CREATE_CHILD_SA,
> and stated several times in the document. So, I suggest to change:
>
>    "The responder replies with the accepted offer in an SA payload,
>     nonce in the Nr payload and a Diffie-Hellman value in the
>    KEr payload if KEi was included in the request and the selected
>    cryptographic suite includes that group."
>
>    "The responder replies with the accepted offer in an SA payload,
>     nonce in the Nr payload and a Diffie-Hellman value in the
>    KEr payload if the selected cryptographic suite includes that group."
>
>    "The responder replies with the accepted offer in an SA payload,
>     nonce in the Nr payload and a Diffie-Hellman value in the
>    KEr payload if KEi was included in the request and the selected
>    cryptographic suite includes that group."
>
> 6. Page 19.
>     "The recipient of this notification cannot tell
>    whether the SPI is for AH or ESP, but this is not important because
>    the SPIs are supposed to be different for the two."
>
> Why "is supposed to be different"? RFC4301 clearly states:
>     "For a unicast SA, the SPI can be used by itself to specify an SA,
> or it may be
>       used in conjunction with the IPsec protocol type."
>
> So I suggest to change as follows:
>
>     "The recipient of this notification cannot tell
>    whether the SPI is for AH or ESP, but this is not important because
>     in many cases the SPIs will be different for the two."
>
> And some words should be added for the case when SPIs are not
> different for AH and ESP. I have no good suggestion here.
>
> 7. Page 31.
> Phrase "(see Section 2.6)" actually references the same section. It
> should either
> be removed or corrected.
>
> 8. Page 31.
>     "However, if the responder sends a non-zero
>    responder SPI, the initiator should not reject the response for only
>    that reason."
>
> Should here "should not" be "SHOULD NOT"?
>
> 9. Page 49.
>     "There are two types of EAP authentication (described in
>    Section 2.16), and each type uses different values in the AUTH
>    computations shown above.  If the EAP method is key-generating,
>    substitute master session key (MSK) for the shared secret in the
>    computation.  For non-key-generating methods, substitute SK_pi and
>    SK_pr, respectively, for the shared secret in the two AUTH
>    computations."
>
> Isn't this para superfluous here? Its content belongs to EAP authentication
> and in fact it is described in more details two pages below.
>
> 10. Page 51.
>     "As described in Section 2.2, when EAP is used, each pair of IKE SA
>    initial setup messages will have their message numbers incremented;
>    the first pair of AUTH messages will have an ID of 1, the second will
>    be 2, and so on."
>
> Should be:
>
>     "As described in Section 2.2, when EAP is used, each pair of IKE SA
>    initial setup messages will have their message numbers incremented;
>    the first pair of IKE_AUTH messages will have an ID of 1, the second
> will
>    be 2, and so on."
>
> 11. Page 52:
>     "A single CHILD_SA negotiation may result in multiple Security
>    Associations."
>
> Should be:
>
>    "A single CREATE_CHILD_SA negotiation may result in multiple Security
>    Associations."
>
>
>
> That's all for now.
> Valery.
>
>
>
>
>
>
>
> _______________________________________________
> IPsec mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ipsec