Hi Tero,
thank you for this review.
> 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.
> ----------------------------------------------------------------------
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:
>
> ----------------------------------------------------------------------
> 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.
> ----------------------------------------------------------------------
Changed to your proposed text.
> 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.
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).
> 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).
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).
> 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.
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:
> ----------------------------------------------------------------------
> 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.
Good point.
> 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.
Done.
> 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.
I had hard time trying to recall what was meant by the last sentence, but
failed.
Anyway, the text is deleted.
> 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.
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.
> --
> kivi...@iki.fi
>
> _______________________________________________
> 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
