Hi Valery,

Thanks for the feedback! I'll clarify that the TCP Originator is the same as 
the original initiator of the first IKE SA, as well as fixing the typographical 
errors.

If anyone else has more feedback, please chime in! I'll wait a day or so before 
updating the draft, to batch any other changes that should be made.

Thanks,
Tommy

> On Feb 7, 2017, at 5:54 AM, Valery Smyslov <sva...@gmail.com> wrote:
> 
> Hi Tommy,
> 
> thank you for the quick update. A couple nits:
> 
> 1. Section 1.2:
> s/the the/the
> 
> 2. Section 1.2:
>       The TCP  
>       Originator MUST be the same as the "Original Initiator", or the  
>       Initiator of the first IKE SA exchange for a given IKE session.
> 
> Again, this is confusing. In RFC7296 the term " Original Initiator" is 
> defined as:
>    The "original initiator
>     always refers to the party who initiated the exchange that resulted
>     in the current IKE SA.  In other words, if the "original responder"
>     starts rekeying the IKE SA, that party becomes the "original
>     initiator" of the new IKE SA."
> 
> "the Initiator of the first IKE SA exchange for a given IKE session" is also
> wrong, since IKE session means IKE SA and it can be a rekey of previous
> IKE SA when entities swap their original roles.
> 
> This is now what we want it to mean, so an additional clarification is needed.
> Something along the lines:
> 
>    The TCP Originator MUST be the same as the entity who originally
>    initiated the first IKE SA (in a series of possibly several rekeyed
>    IKE SAs).
> 
> Otherwise it looks good.
> 
> Regards,
> Valery.
> 
>> Hi Valery,
>> 
>> Thanks so much for your comments! I have posted a new version of the draft 
>> here:
>> https://tools.ietf.org/html/draft-ietf-ipsecme-tcp-encaps-06
>> 
>> Responses inline.
>> 
>> Best,
>> Tommy
>> 
>>> On Feb 2, 2017, at 4:13 AM, Valery Smyslov <sva...@gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> here is my review of draft-ietf-ipsecme-tcp-encaps-05.
>>> The draft is in a good shape, but a bit of final polishing is required.
>>> 
>>> 1. The terms "tunnel", "tunneled" are used throughout the document
>>>   when referring to ESP SA. I think it is technically incorrect, since
>>>   ESP supports both tunnel and transport modes, so the document
>>>   should use more appropriate terms like "IPsec SA", "ESP packets" etc.
>> 
>> Great point. I have removed the references to tunnel, and replaced with SA, 
>> generally.
>> 
>>> 
>>> 2. Section 4.
>>>   This value is
>>>  only sent once, by the Initiator only, at the beginning of any stream
>>>  of IKE and ESP messages.
>>> 
>>>  If other framing protocols are used within TCP to further encapsulate
>>>  or encrypt the stream of IKE and ESP messages, the Stream Prefix must
>>>  be at the start of the Initiator's IKE and ESP message stream within
>>>  the added protocol layer [Appendix A].
>>> 
>>> Using "Initiator" is wrong here, since the TCP stream may be re-established
>>> after the peers rekeyed IKE SA and changed their roles (so that original
>>> Initiator becomes Responder). It either must be "original Initiator"
>>> (with all explanations who it is, as in Section 6) or, preferably,
>>> "originator of TCP stream" (or smth like that).
>>> 
>>> Actually, "Initiator" is used in a lot of places throughout the document
>>> and in most cases it means "original Initiator of the first IKE SA in a 
>>> series
>>> of possible successive rekeys of this SA". It is good to look through
>>> all these uses and either clarify this term in all places it is used, or add
>>> some para in the beginning of the draft, e.g. like it is done in RFC4555:
>>> 
>>>  In this document, the term "initiator" means the party who originally
>>>  initiated the first IKE_SA (in a series of possibly several rekeyed
>>>  IKE_SAs); "responder" is the other peer.  During the lifetime of the
>>>  IKE_SA, both parties may initiate INFORMATIONAL or CREATE_CHILD_SA
>>>  exchanges; in this case, the terms "exchange initiator" and "exchange
>>>  responder" are used.  The term "original initiator" (which in [IKEv2]
>>>  refers to the party who started the latest IKE_SA rekeying) is not
>>>  used in this document.
>>> 
>>> I also noticed that both "Initiator" and "initiator" are used in the 
>>> document
>>> (as well as "Responder" and "responder"). It's better to use one form
>>> (preferably with capital letter, like in RFC7296). RFC Editor will change
>>> them to one form in any case, so you can ease her work a bit.
>> 
>> I've switched to use the capitalized version of Initiator and Responder.
>> 
>>> 
>>> I'd also suggest to introduce some term for the party, that creates
>>> TCP session (e.g. "TCP originator") and use it where appropriate,
>>> so that IKE roles are clearly distinct from TCP roles. In this case
>>> you can get rid of "Initiator" in most places replacing it with "TCP 
>>> originator".
>> 
>> I added the terms "TCP Originator" and "TCP Responder" in the terminology 
>> section, with an
> explanation of
>> this distinction. I've used the new terms where appropriate throughout the 
>> document.
>> 
>>> 
>>> 3. Section 6.
>>>  When the IKE initiator uses TCP encapsulation for its negotiation,...
>>> 
>>> "its negotiation" is confusing here, since TCP encapsulation is not 
>>> negotiated.
>>> I suggest removing "for its negotiation" completely (or change to "for IKE 
>>> SA negotiation").
>> 
>> Removed the phrase.
>> 
>>> 
>>> 4. Section 6.
>>>  Once all
>>>  SAs have been deleted,...
>>> 
>>> "all SAs" is confusing. Later in this section it is stated that multiple 
>>> IKE SAs
>>> MUST NOT share a single TCP connection. Is this a leftover from an early 
>>> design?
>> 
>> Switched to "Once the SA has been..."
>> 
>>> 
>>> 5. Section 6.
>>>  If the connection is being used to resume a previous IKE session...
>>> 
>>> For clarity:  s/connection/TCP connection
>> 
>> Switched to "if a TCP connection".
>> 
>>> 
>>> 6. Section 6.
>>>  A responder SHOULD at any given time send packets for an IKE SA and
>>>  its Child SAs over only one TCP connection.
>>> 
>>> Why only Responder? What about Initiator? I think this requirement
>>> is valid for both.
>> 
>> Slightly modified the phrasing. The paragraph above describes the 
>> Initiator/Originator behavior.
>> 
>>> 
>>> 7. Section 6.
>>>  In order to be considered valid for choosing a TCP
>>>  connection, an IKE message successfully decrypt and be authenticated,
>>>  not be a retransmission of a previously received message, and be
>>>  within the expected window for IKE message IDs.  Similarly, an ESP
>>>  message must pass authentication checks and be decrypted, not be a
>>>  replay of a previous message.
>>> 
>>> "an IKE message successfully decrypt" - is it OK with the grammar?
>>> Shouldn't it be: "an IKE message must be successfully decrypted"?
>> 
>> Thanks for catching! Fixed.
>> 
>>> 
>>> What about ES, I think that it is a good thing to add
>>> a requirement, that ESP window size MUST be set to 1 if TCP
>>> encapsulation is in use. Larger windows are useless with TCP,
>>> since there is no packet reordering. On the other hand, setting
>>> window size to 1 will effectively block an attack vector that was described
>>> by Tero, when an attacker sends old packet with fake TCP header.
>>> If window size is 1, then this packet will be dropped immediately
>>> without any changes in the ESP logic.
>> 
>> Added this recommendation to the security considerations.
>> 
>>> 
>>> 8. Section 6.
>>>   Multiple IKE SAs MUST NOT share a single TCP connection.
>>> 
>>> Please add clarification: "unless one of them is a rekey of another,
>>> in which case two SAs sharing a single TCP connection coexist for
>>> a short period of time" (or smth similar).
>> 
>> Added the clarification.
>> 
>>> 
>>> Regards,
>>> Valery.
>>> 
> 
> _______________________________________________
> IPsec mailing list
> IPsec@ietf.org
> https://www.ietf.org/mailman/listinfo/ipsec

_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec

Reply via email to