[IPsec] Review of draft-ietf-ipsecme-rfc8229bis

[Paul Wouters <[e-mail]>]

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 might also change "for traversing network middleboxes" to be more neutral,
eg "in case routers or network middleboxes do not handle UDP".


 	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?

This might also make the code more complex, as currently, the TCP state
is kept during the entire negotiation.

Additionally, a NAT router could give the client a different IP address
for a new TCP stream, making the COOKIE invalid.

 	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.

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


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


 	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)


 	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?





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


 	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