Hi Paul, thanks a lot for your review.
> I have reviewed the changed between draft-ietf-ipsecme-rfc8229bis and > RFC 8229. I agree with most of these changes. I have some comments > below. If others want to compare the draft with the RFC, see: > > https://nohats.ca/draft-ietf-ipsecme-rfc8229bis-01-from-rfc8229.diff.html > > > > > that may block IKE negotiation over UDP. > > I would say: > > that may not transport IKE negotiation over UDP. > > Blocking sounds like an active administrative action. Most networks just > accidentally happen to not pass UDP. I think both cases are possible. How about: that may block IKE negotiation over UDP either deliberately or inadvertently. > I might also change "for traversing network middleboxes" to be more neutral, > eg "in case routers or network middleboxes do not handle UDP". OK. So, the whole sentence (assuming my proposal above) would look like: This document describes a method to transport Internet Key Exchange Protocol (IKE) and IPsec packets over a TCP connection in case routers or network middleboxes do not handle UDP either deliberately or inadvertently. Is it OK? > o if the Responder chooses to send Cookie request (possibly along > with Puzzle request), then the TCP connection that the IKE_SA_INIT > request message was received over SHOULD be closed, so that the > Responder remains stateless at least until the Cookie (or Puzzle > Solution) is returned. Note that if this TCP connection is > closed, the Responder MUST NOT include the Initiator's TCP port > into the Cookie calculation (*), since the Cookie will be returned > over a new TCP connection with a different port. > > I am not sure this is a good idea. Tearing down and requiring a new 3way > handshake just to deliver the cookie seems useless. What is wrong with > using the existing TCP stream to reply? The responder does use an existing TCP connection to reply, but once it replies with cookie request, it should close this connection. If it keeps this connection, then it keeps TCP state until the client resends IKE_SA_INIT request (if ever) and thus thwarts the idea of being stateless. This is probably not so important for cookies, because the client has already proved during TCP handshake, that its IP is a real IP address, but it is more important in case of puzzles. But in both cases I think that keeping responder stateless is a good idea. > This might also make the code more complex, as currently, the TCP state > is kept during the entire negotiation. No, RFC 8229 allows the responder to close TCP session at any time (Section 6). Moreover, an attacker may reset TCP connection on its will. In this case the TCP originator would restore it. So, no additional code complexity, you must already be able to handle this situation. > Additionally, a NAT router could give the client a different IP address > for a new TCP stream, making the COOKIE invalid. Yes, it is possible and this situation is covered in the draft. But if we choose between keeping the responder stateless and the possibility of occasional failure of cookie verification, I'd choose the former as more important. I think we can slightly change this recommendation: o if the Responder chooses to send Cookie request (possibly along with Puzzle request), then the TCP connection that the IKE_SA_INIT request message was received over SHOULD be closed after the Responder sends its reply and no repeated requests are received within some short period of time, so that the Responder remains stateless. Note that if this TCP connection is closed, the Responder MUST NOT include the Initiator's TCP port into the Cookie calculation (*), since the Cookie will be returned over a new TCP connection with a different port. Does it address your concern? > Thus, in case of TCP encapsulation, an Initiator SHOULD NOT wait for > additional messages in case it receives error notification from the > Responder in the IKE_SA_INIT exchange. > > This is true, but the code handlling this might not be aware of the > transport used. I'd rather see "an Initiator MAY skip the waiting time > for additional messages" so that this advice becomes "a good idea" and > not a "RFC violation" if not done. Hmm, I think that "SHOULD NOT wait" leaves you a possibility to wait if you have good reasons for it, without being accused in "RFC violation" :-) I have no problems with MAY in general, but I think that it's better to encourage implementers to make this optimization (which MAY does not). > 7.6. Keep-Alives and Dead Peer Detection > > This section tells us to not send NAT keepalives. It also tells us to > not rely on TCP keepalives. That left me puzzled on how to ensure the > peer is alive until I remembered that NAT keepalives are not the same > as IKE keepalives. I would briefly mention in this section that IKE > keepalives should be used "as normal". OK. > Implementations that implement > both MOBIKE and TCP encapsulation MUST support dynamically enabling > and disabling TCP encapsulation as interfaces change. > > I'm not sure of the MUST here. Nice for sure, but perhaps the > implementation supports MOBIKE or TCP and not both at once ? > Perhaps restate as "if MOBIKE and TCP encapsulation are allowed for a > configuration, the implementation MUST support ...... OK. So, the text would look like: Implementations that implement both MOBIKE and TCP encapsulation and both are allowed for a configuration MUST support dynamically enabling and disabling TCP encapsulation as interfaces change. > In case of TCP the NO_NATS_ALLOWED > notification SHOULD be ignored because TCP generally has no problems > with NAT boxes. > > I would re-state it as: > > In case of TCP the NO_NATS_ALLOWED notification MUST be ignored. > > Although, re-reading why NO_NATS_ALLOWED was introduced, I think in > general this payload should be retired entirely, both for UDP and TCP. > (disclosure: libreswan completely ignores it) I think "MUST be ignored" is quite a radical change to RFC 4555. I prefer the current text since it leaves a possibility to not ignore this notification , thus be compliant with RFC 4555. Otherwise we would need to update it with this draft... > 8.3. IKEv2 Session Resumption > > This section says that if a TCP encap session was suspended, that the > client MUST use TCP to resume this. I don't understand why. It is likely > that a resuming client has moved networks, and might be on a network > that would properly support ESP or ESPinUDP. The resumption is really > mostly meant to skip over DH and AUTH phases as these are costly. I see > no reason to tie the transport to this. If I missed a valid reason > for this to be needed, clarify what needs to happen when the server no > longer can or wants to resume the session. Does the client close the TCP > and go back to UDP? I believe you are correct. The idea was to speed up SA resumption (we know that this responder didn't support UDP, but worked with UDP), however you are right that the situation may change. So the correct recommendation for the client would be - act as if this is the IKE_SA_INIT exchange. > NITS: > > This document updates specification for > change to: > This document updates the specification for > > > MOBIKE protocol, that allows IKEv2 SA > change to: > The MOBIKE protocol that allows SA's Why "SA's"? Shouldn't it be " SAs"? > New INFORMATIONAL exchange MUST NOT bestarted in this situation. > change to: > A new INFORMATIONAL exchange MUST NOT bestarted in this situation. > (or maybe say something like "the INFORMATIONAL exchange is then > retransmitted over TCP") > > > Since switching from UDP to TCP happens can occur during a single > INFORMATIONAL message exchange, > change to: > Since switching from UDP to TCP can happen during a single > INFORMATIONAL message exchange, > > MOBIKE protocol defined the NO_NATS_ALLOWED > change to: > The MOBIKE protocol defines the NO_NATS_ALLOWED Fixed in the local copy, thank you. 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
