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
