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

From: Paul Wouters <[email-masked]>
Date: Sun, 9 Jan 2022 22:42:43 -0500 (EST)

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

---

From: "Valery Smyslov" <[email-masked]>
Date: Mon, 10 Jan 2022 16:38:20 +0300

Hi Paul,

thanks a lot for your review.

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

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

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

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

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

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

> keepalives should be used "as normal".

OK.

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

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

> longer can or wants to resume the session. Does the client close the TCP

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.

>  	The MOBIKE protocol that allows SA's

Why "SA's"? Shouldn't it be " SAs"?

>  	The MOBIKE protocol defines the NO_NATS_ALLOWED

Fixed in the local copy, thank you.

Regards,
Valery.

> _______________________________________________
> IPsec mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ipsec

---

From: Paul Wouters <[email-masked]>
Date: Mon, 10 Jan 2022 09:58:10 -0500 (EST)

> I think that keeping responder stateless is a good idea.

Yes, stateless is good in those cases, but reality is we already used
state to setup the TCP connection. So why drop all that state now. It is
in a way too late for the "state creation savings". Instead, you are now
creating _more_ work for a server that might be under a DoS attack.

> you must already be able to handle this situation.

That's fair. I guess I have to test this on our code base :)

> I'd choose the former as more important.

I am choosing the other way around :)

> Does it address your concern?

I don't really see much difference. If the attacker can setup a TCP
session, surely it can write 15 lines of python to replay the cookie
back as well. The question is really whether the responder under attack
is better of not needing as many TCP 3way handshakes versus keeping some
state. Since the document says to basically ignore cookies as much as
possible, it seems inline to just keep the TCP connection.

> to encourage implementers to make this optimization

I'm looking at the damage here. There is again no good reason to tear
down the TCP connection for an INVALID_KE, so why tell implementations
to do that. Even if simple implementations might do that just because
that's how they do it for UDP (kill entire exchange, start a fresh one)

But I'd like to see us re-using the same TCP when receiving INVALID_KE,
even if it was only for the reduced latency of another 3way handshake.

>          encapsulation as interfaces change.

sure, just slightly different start:

 	Implementations that implement both MOBIKE and TCP
 	encapsulation within the same connection configuration
 	MUST ....

> we would need to update it with this draft...

Fair enough.

> So the correct recommendation for the client would be - act as if this is the IKE_SA_INIT exchange.

Yes, perfect.

Paul

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 11 Jan 2022 00:59:42 +0200

Valery Smyslov writes:
> 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
> 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.

I think it is important not to close the TCP immediately after the
sending out cookie request, as proper initiator will most likely
respond it in one round trip, thus forcing initiator to restart TCP
handshake wastes lots of resources. On the other hand it might be
useful for puzzles, depending on the expected solving time.

Note, that when the responder closes the TCP connection the connection
goes to the TCP TIME_WAIT state, where it will still waste resources
for a minute or so, thus closing TCP session will NOT free resources
immediately, thus closing TCP session too quickly will waste much more
resources than keeping it open for 10 seconds or so.

If responder keeps the TCP session open for 10 seconds, and initiator
comes back during that time with cookie or solved puzzle it can save 6
packets between the peers, and one set of buffers used for TCP
send/receive windows. If initiator does not come back after 10 second
wait, then it only extends the resource use from 60 seconds to 70
seconds or so...
-- 
[email-masked]

---

From: "Valery Smyslov" <[email-masked]>
Date: Tue, 11 Jan 2022 14:58:13 +0300

Hi Tero,

> resources than keeping it open for 10 seconds or so.

Good point.

> wait, then it only extends the resource use from 60 seconds to 70

The latest text I proposed in reply to Paul's comments incorporates this strategy:

The text doesn't specify what "some short period of time" is.
I don't think we should do it (it may be 10 sec as you suggested
or 5 sec or 20 sec).

Is it OK? Or should we probably change SHOULD to MAY here?

Regards,
Valery.

---

From: "Valery Smyslov" <[email-masked]>
Date: Tue, 11 Jan 2022 15:31:06 +0300

Hi Paul,

> creating _more_ work for a server that might be under a DoS attack.

I agree. That's why I added a clarification that TCP state SHOULD be closed
if no repeated requests are received within some short period of time.

> I am choosing the other way around :)

That is that :-)

> possible, it seems inline to just keep the TCP connection.

It should be eventually closed in case the initiator
never comes back. So, the question is - what time we consider
appropriate for waiting the initiator's repeated request?
The text above specify it as "some short period of time"
and you may interpret it at your will. 

> that's how they do it for UDP (kill entire exchange, start a fresh one)

This section is not about how you handle error notifications. It only tells that 
there is no reason to wait for another (probably good) response if
error notification is received, it never comes (unlike UDP, when it is possible).

How you handle error notification is out of scope of this section.

> even if it was only for the reduced latency of another 3way handshake.

You may do it and by no means the draft suggests the opposite.
Tearing down TCP is only advised in case of COOKIE (or PUZZLE)
to keep the statelessness of the responder. And only if 
the client didn't immediately repeat its request.

OK.

> > So the correct recommendation for the client would be - act as if this is the IKE_SA_INIT exchange.

Great :-) 

Regards,
Valery.

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 11 Jan 2022 14:42:26 +0200

> Is it OK? Or should we probably change SHOULD to MAY here?

I think it would be best to explain that valid clients usually return
back almost immediately, so responders should keep tcp connection open
for at least 10 seconds to receive the cookie responses, and after
that SHOULD close the connection after some timeout.

And perhaps add some words that depending on the puzzle difficulty
level the timers might be modified (i.e., if you give puzzle that will
require several minutes to solve, there is no point of waiting any
more than the few seconds, but if you expect initiator to solve the
puzzle in 5 seconds, then waiting 20 seconds is good thing).
-- 
[email-masked]

---

From: "Valery Smyslov" <[email-masked]>
Date: Tue, 11 Jan 2022 15:53:43 +0300

> that SHOULD close the connection after some timeout.

I don't think we should advise any concrete values here, following
RFC 7296 practice which deliberately avoided specifying concrete timeouts
and also because they don't affect interoperability.
But I agree that some explanations can be added and
we can mention concrete values (say 10 secs) as examples.

> puzzle in 5 seconds, then waiting 20 seconds is good thing).

OK, good idea to add some clarifications.

Regards,
Valery.

---

From: "Valery Smyslov" <[email-masked]>
Date: Wed, 19 Jan 2022 21:45:51 +0300

Hi Paul,

a new -02 version of the draft is published. We believe it addressed your comments,
except for one, see below.

> eg "in case routers or network middleboxes do not handle UDP".

After some discussion between the authors we decided to keep the 
original text, because it was in the RFC8229 and caused no problems.

Regards,
Tommy & Valery.

[sniped]

---

From: Paul Wouters <[email-masked]>
Date: Wed, 19 Jan 2022 14:02:31 -0500 (EST)

> original text, because it was in the RFC8229 and caused no problems.

Ok. Let's hope past performance _is_ indicative of future results :)

Paul