Re: [IPsec] Editorial changes to RFC5996

Hi Yaron,

> Hi Valery,
>
> Sorry for being the Bad Guy on this. Your #6 does not seem editorial to

I think that current text is not aligned with RFC4301.
We may leave it as is or try to find other form that
would not appear so misaligned.

> 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.

What about #8 - it's just a question from me. From my feeling it must
be uppercase, but I might be wrong. We may leave it as is.

> 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."

Yes, your text is for RFC5996bis, while I made my notes a while ago
and the text was for RFC5996. Of course your variant is better.

Valery.

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