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

From: Tero Kivinen <[email-masked]>
Date: Sun, 20 Mar 2022 16:23:06 +0200

I was doing the review of the draft-ietf-ipsecme-rfc8229bis while
doing the shepherd writeup, and here are my comments to the draft.

In section 7.5:
----------------------------------------------------------------------
   If a NAT is detected due to the SHA-1 digests not matching the
   expected values, no change should be made for encapsulation of
   subsequent IKE or ESP packets, since TCP encapsulation inherently
   supports NAT traversal.  Implementations MAY use the information that
   a NAT is present to influence keep-alive timer values.

   If a NAT is detected, implementations need to handle transport mode
   TCP and UDP packet checksum fixup as defined for UDP encapsulation in
   [RFC3948].
----------------------------------------------------------------------

Is bit confusing, as you first say that no change is done, but then
you say you still need to implement checksum fixup.

Perhaps

----------------------------------------------------------------------
   If a NAT is detected due to the SHA-1 digests not matching the
   expected values, no change should be made for encapsulation of
   subsequent IKE or ESP packets, since TCP encapsulation inherently
   supports NAT traversal, but the in the decapsulation the
   implementations need to handle transport mode TCP and UDP packet
   checksum fixup as defined for UDP encapsulation in [RFC3948].
   Implementations MAY use the information that a NAT is present to
   influence keep-alive timer values.
----------------------------------------------------------------------

In secton 7.7:

----------------------------------------------------------------------
                       If
       this functionality is needed, implementations should create
       several IKE SAs over TCP and assign a corresponding DSCP value to
       each of them.
----------------------------------------------------------------------

I think we should be clear that we need to run several IKE SAs over
separate TCP connections:

----------------------------------------------------------------------
       If this functionality is needed, implementations should create
       several IKE SAs each over separate TCP connection and assign a
       corresponding DSCP value to each of them.
----------------------------------------------------------------------

In section 8.1 we are missing description what happens to the old TCP
connection in case the mobike detects change of network. Of course it
the mobike detects that IP-address changed and the old IP-addresses
are no longer usable, the old TCP connection needs to be closed, but
in some cases mobike might get new network before loosing connection
to the previous network, and we need to define how the TCP connection
is handled in that case.

I.e., I would assume that if implementation is using TCP encapsulation
and then mobike detects new network and notices that UDP works over
that it should close the unused TCP connection after mobike connection
has finished updating.

Same when moving from one TCP connection to another.

Now the question what does the responder do if the other end was using
TCP connection and mobike, and then responder gets update to new
IP-address, but does not receive close to the TCP (for example other
end might have lost its access to the previous network).

I assume it should close the TCP connection after it has verified that
other has moved all traffic to the new connection (either UDP or TCP).

Also note that as described in the RFC 4555 section 3.5 the mobike
requires retransmit of all outstanding IKE exchanges after the address
update, and we should most likely make a note of that here too.

I.e. note that RFC4555 has following sentence:
----------------------------------------------------------------------
   o  If there are outstanding IKEv2 requests (requests for which the
      initiator has not yet received a reply), continues retransmitting
      them using the addresses in the IKE_SA (the new addresses).
----------------------------------------------------------------------

This should be done even when moving from TCP to TCP.

In section 11 security considarations there is text saying:
----------------------------------------------------------------------
   Since TCP provides reliable, in-order delivery of ESP messages, the
   ESP anti-replay window size SHOULD be set to 1.  See [RFC4303] for a
   complete description of the ESP anti-replay window.  This increases
   the protection of implementations against replay attacks.
----------------------------------------------------------------------

I am not sure if that is good advice. There are cases where ESP
packets might still be sent out of order in case one TCP connection is
teared down and new one is created, and of course if using MOBIKE with
TCP this should not be used, as MOBIKE may move the traffic from TCP
to UDP at any time.

Other case where packets might be sent out of order is when reordering
happens inside the node, for example because it has multiple hardware
encryption cores, and it sends big packet to first core, and small
packet to second core, and the second core finished before the the
first one, and the packes then come out of order.

I am not sure if setting the window size to 1 actually offers any more
protection than having normal anti-replay window size.

I would simply delete the whole paragarph.

And the final sentence is definiately untrue. Using replay window of 1
DOES NOT change the protection against replay attacks. Replays are
detected regardless of the window size if the anti-replay protection
is enabled.

In section B.4 I think we are missing second MOBIKE update, i.e. after
step 6, there should another INFORMATIONAL, HDR, SK {
N(UPDATE_SA_ADDRESS), ... } message and reply that. This is because if
the MOBIKE detected it needed to send its update over mutliple
addresses (or transport method like in this case), it will use the
retransmissions of the first one to detect which path work, and after
that finishes, it will immediately start new one to make sure that
information is in sync in both ends. For example the NAT_DETECTION_*
payloads might change when moving from UDP to TCP (or from one TCP to
another TCP).

This is described in the RFC4555 section 3.5 as follows:
----------------------------------------------------------------------
   o  If a new address change occurs while waiting for the response,
      starts again from the first step (and ignores responses to this
      UPDATE_SA_ADDRESSES request).
----------------------------------------------------------------------

So adding another UPDATE_SA_ADDRESSES there between steps 6 and 7
should be done.
-- 
[email-masked]

---

From: "Valery Smyslov" <[email-masked]>
Date: Tue, 22 Mar 2022 11:32:34 +0300

Hi Tero,

thank you for this review.

> doing the shepherd writeup, and here are my comments to the draft.

Changed to:

   If a NAT is detected due to the SHA-1 digests not matching the
   expected values, no change should be made for encapsulation of
   subsequent IKE or ESP packets, since TCP encapsulation inherently
   supports NAT traversal.  However, for the transport mode IPsec SAs
   implementations need to handle TCP and UDP packet checksum fixup
   during decapsulation, as defined for UDP encapsulation in [RFC3948].
   Implementations MAY use the information that a NAT is present to
   influence keep-alive timer values.

> In secton 7.7:

Changed to your proposed text.

> Same when moving from one TCP connection to another.

The following text is added:

   If the TCP transport was used for the previous network connection,
   old TCP connection SHOULD be closed by the Initiator once MOBIKE
   finishes migration to a new connection (either TCP or UDP).

> other has moved all traffic to the new connection (either UDP or TCP).

I believe this is already covered by 7.1 and no new text is needed.
In particular:

   A TCP Responder MAY close a TCP connection that it
   perceives as idle and extraneous (one previously used for IKE and ESP
   messages that has been replaced by a new connection).

> I.e. note that RFC4555 has following sentence:

I think this is also already covered by 7.2, second clause 
(I slightly changed the text to make it more generic):

   o  If a new TCP connection for the IKE SA is established while the
      exchange Initiator is waiting for a response, the Initiator MUST
      retransmit its request over this connection and continue to wait
      for a response.

> In section 11 security considarations there is text saying:

Good point.

> I would simply delete the whole paragarph.

Done.

> detected regardless of the window size if the anti-replay protection

I had hard time trying to recall what was meant by the last sentence, but failed.
Anyway, the text is deleted.

> This is described in the RFC4555 section 3.5 as follows:

Added. BTW, this exchange was missing in the RFC 8229 too.

I also tried to fix idnits complaints about the abstract and added
a para into the Introduction section that references RFC 8229.

The updated version is on github waiting for my co-author review:
https://github.com/smyslov/rfc8229bis/pull/8/files

Regards,
Valery.

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

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 22 Mar 2022 14:18:02 +0200

>    influence keep-alive timer values.

Looks good to me.

>    finishes migration to a new connection (either TCP or UDP).

That looks ok also.

>    messages that has been replaced by a new connection).

Yes, that should cover it, so no need to add anything else.

>       retransmit its request over this connection and continue to wait

I think that partially covers it (of course there might be multiple
request, not just one, as the window size might be larger than one). I
was just thinking repeating it here as this situation is much more
common with mobike, than just normal case. Also you might need to
retransmit the old requests over the new connection before you have
space in the window to actually send address update for mobike.

So having few words here for mobike case would be useful too.
Especially pointing out that this is not specific to the TCP
encapsulation, this is generic thing that is done when using mobike
regardless whether you use TCP or not.. 
-- 
[email-masked]

---

From: Paul Wouters <[email-masked]>
Date: Tue, 22 Mar 2022 08:50:48 -0400 (EDT)

> regardless whether you use TCP or not..

There was some talk from I think Antony and Valery that we should have
a clarification document on some MSGID issues that is a superset of the
TCP problems.

Paul

---

From: Tero Kivinen <[email-masked]>
Date: Tue, 22 Mar 2022 15:00:38 +0200

> a clarification document on some MSGID issues that is a superset of the

The mobike is written in such way that it uses the latest IKEv2
message as a probe and uses that latest message to find the working
connection between devices. This just so that we can use the normal
message id properties, i.e., every single request has unique message
id (along with initiator flag) and has singe response and if there are
retransmissions those requests and responses are all bitwise
identical.

So mebike will retransmit its latest IKEv2 request over different
paths, even when it knows the information there might already be
stale, as the for example the IP adress might have changed in address
update. When it finds a working path, it will immediately restart
address update to make update the other end with proper latest
information.

I have no idea what message id issues you are referencing to...
-- 
[email-masked]

---

From: "Valery Smyslov" <[email-masked]>
Date: Tue, 22 Mar 2022 16:11:30 +0300

> request, not just one, as the window size might be larger than one). 

That's true, but I believe the current text covers it. Note,
that this text is for the exchange initiator, so if multiple
exchanges are initiated for an SA, then this requirement is applied
for each of them and each outstanding request will be retransmitted.

> I was just thinking repeating it here as this situation is much more
> common with mobike, than just normal case. 

Actually, I'm not sure. TCP connections can be teared 
down (e.g. as result of attack) and reestablished quite 
frequently and each time it happens this requirement
must be followed.

> Also you might need to

OK.

> regardless whether you use TCP or not..

Probably we can just reference RFC 4555. 

How about adding the following para
(I also quotet here a part of a previous para, since I noticed that RFC 4555 in fact
doesn't contain normative language on these actions and thus we cannot use
word "requires", more neutral tone is needed):

   [...] Section 3.5
   of [RFC4555] states that a new INFORMATIONAL exchange with the
   UPDATE_SA_ADDRESSES notify is initiated in case the address (or
   transport) is changed while waiting for a response.

   Section 3.5 of [RFC4555] also states that once an IKE SA is switched
   to a new IP address, all outstanding requests in this SA are
   immediately retransmitted using this address.  See also Section 7.2.

Is it OK?

Regards,
Valery.

---

From: Tero Kivinen <[email-masked]>
Date: Wed, 23 Mar 2022 11:03:17 +0200

>    immediately retransmitted using this address.  See also Section 7.2.

Perfect.
-- 
[email-masked]