Subject: [tsvwg] Magnus AD review of draft-ietf-tsvwg-natsupp-22

From: Magnus Westerlund <[email-masked]>
Date: Wed, 10 Mar 2021 10:17:11 +0000

Hi,

So this is the promised AD review from me. For chairs and authors this has
been updated adjust for the answers I have received so I try to clarify my
issues. 

Also sorry about the long delay in reviewing it. It was not acceptable to
delay this so much. 

1. My first issue is related to the fork lift upgrade this SCTP NAT
functionality appear to require. So in the introduction there is this table
that discuss what occurs if the two end-points support this style of SCTP
NAT or not. I think part of this is as of a consequence of this document
changing the definition of how to identify an SCTP Association. In RFC 4960
an association is defined as:

   o  SCTP association: A protocol relationship between SCTP endpoints,

      composed of the two SCTP endpoints and protocol state information

      including Verification Tags and the currently active set of

      Transmission Sequence Numbers (TSNs), etc.  An association can be

      uniquely identified by the transport addresses used by the

      endpoints in the association.  Two SCTP endpoints MUST NOT have

      more than one SCTP association between them at any given time.

Note that the VTAG is not defined as being part of uniquely identifying an
association when receiving a packet. This means that a non-upgrade host
might react to the case where two clients on the internal side of NAT. I
think this change is not sufficiently explained and made clear. Because it
might have impact on other SCTP extensions, thus we might even have to
consider if this should make this document be one that update RFC 4960 (or
the bis version) to make this clear.

So the specified NAT state 

          +---------+--------+----------+--------+-----------+

   NAT    |  Int    |  Int   |   Rem    |   Rem  |    Int    |

          |  VTag   |  Port  |   VTag   |   Port |    Addr   |

          +---------+--------+----------+--------+-----------+

          |  1234   |  4321  |    5678  |  10123 |  10.0.0.1 |

          +---------+--------+----------+--------+-----------+

Do have the advantage of making this NAT providing endpoint independent
mapping which has benefits when it comes to handle remote peer multihoming
and also peer to peer, as the external mapped address can be learned
separately and the external port will be the client's port the INIT
collision case is made simpler. When it comes to how many SCTP Association
it can at most support, each internal + external port number combinations
has a maximum of 32-bit of VTAGs in both direction. The practical limit is
less as both client and server need to select a unique which  will result in
more collisions when the number of used VTAG become fairly large. But I
think this number is more than high enough. 

The downside of this I think is that it changes this SCTP association
definition which will have impact on both implementations and likely also
firewalls. I am uncertain if this complications have been sufficiently
evaluated. Can the WG please clarify what consideration has been done into
these aspects? 

I would also like some clarification on what is meant with limited
capabilities per Table 1. Assume the NAT Support this functionality. What
happens in these cases?

      +===============+==============+=============+===============+
      | Internal Host | NAT Function | Remote Host | Communication |
      +===============+==============+=============+===============+
      |    Support    |   Support    |  No Support |    Limited    |
      +---------------+--------------+-------------+---------------+
      |   No Support  |   Support    |   Support   |    Limited    |
      +---------------+--------------+-------------+---------------+
      |   No Support  |   Support    |  No Support |    Limited    |
      +---------------+--------------+-------------+---------------+

Especially for cases where there are not large number of SCTP Associations,
so the risk for collision is low. It would be desirable that it would work
to get an association established. 

To be concreate, I do think the direction is reasonable here. However, I
think each of these limited cases needs to be walked through and explained
what is expected to occur in each and what limitations the supporting node
and NAT need to do to handle. Also the change to the SCTP association
definition needs to be made clear. 

2. The next significant issue I struggle with is to understand how complex
the NAT's per packet handling is. With the current definition for all
packets the NAT need to read the common header to find the ports and the
VTAG. As that is in a fixed location that I think have small impact on the
NAT. What I try to determine is what it means for the NAT to process those
chunks it is expected to process such as INIT, INIT-ACK, ASCONF, ASCONF-ACK
and ABORT. So maybe I don't know RFC 4960 well enough, but it was not clear
to me that in all cases the relevant chunks for the NAT would be ensured to
always be first among the chunks, even if some bundling was done. If it
can't be guaranteed that the NAT only need to lock at the first chunk, then
I think this is a potential issue. As that increases both the amount of
processing and a question if it can be done in all implementations. So, is
it guaranteed that these chunks will be first so that a fixed offset
processing will work?

Based on the answer I received this is not guaranteed. For INIT and INIT-ACK
it is clear. But for some other cases it is less than clear. I think the
specification should discuss these issues and possibly make recommendations
for updated nodes to ensure ease of implementation. From my perspective it
is not feasible to have to process each packet in completeness to find NAT
state changing packets. Thus, if it can be defined how to easily identify
these so that only a very small subset of packets needs to be processed
fully. And if for example failure case ABORTS results in the NAT state
lingering to the timeout that is likely fine. 

3. Handling of when restart is not turned off. So in all cases where either
hosts don't support this specification the restart will not be turned off.
However, its impact appears different. 

When an internal host does not support the restart disable then the NAT will
have to assume that the internal VTAG is not long term stable, and can't
accept any entry that matches the port combination. That limits the
available number of bindings and drive up collision risk significantly.
Still per destination port there can be at least 63000 clients so it not
non-working. However, one have to ask if making the NAT table endpoint
dependent and track the remote address would reduce the risk for collision
significantly at loss of some of the benefits discussed in 1). At least in
this case the NAT knows immediately if it should ABORT or not. 

In the case where the Internal host supports turning off restart. Then the
NAT can create a mapping and forwards the INIT. But, I am still unclear what
happens if the remote host doesn't support restart disabled. The NAT can
clearly route, but what is the failure case? So is the NAT forced to delay
or abort any association that comes in second before the response has been
received? And for a port pair where at least one entry with VTAGs are
accepted, how does one deal with a response that is non supporting this
specification? 

4. Multihoming of hosts. 

So this supports multihoming of the remote host well and is actually very
efficient in it support. I think the multihoming of the internal host could
be improved. The document lacks a discussion of the potential issue that one
do get a collision for one's client port in a network and NAT that is not
the first one. So are the only mitigation here trying to establish a new
association with a different client port and hope that works better and if
that is established drop the old single homed one? I think this primarily
relates to Section 6.6. However, I think multihoming support for Internal
host are actually more relevant due to virtualization and container
deployments where NATed networking in the environment is fairly common. That
use case was not as important when this work was started.

I have found two very related issues to this. First is the fact that failure
to create a new NAT binding will cause an ABORT. Have there been any
discussion of any method for subsequent path establishments that doesn't
cause the whole association to abort, only the path establishment to fail?
Or should one have an alternative reaction to ABORT under some conditions
when establishing subsequent paths? I think this is highly relevant to have
as one will like to test which combination of internal source address work
with which external as in NATed cases full mesh is far from certain to work.

  "The sender of the packet containing the ASCONF chunk, upon reception

   of a packet containing an ERROR chunk with M bit set, MUST stop

   adding the path to the association."

So this may interpreted as that one will not terminate the association, but
that is not explicit.

The next issue has to do with ASCONF and remote peer receiving the packet.
Looking in Section 5.2 of RFC 5061 the specified processing with a ASCONF
chunk per this NATSUPP specification with a null address will end up in D3)
processing step, resulting in the packet being treated as out of the blue
packet which per RFC 4960. To my understanding that will result in an ABORT
being sent back. So I think this specification is failing to update the
ASCONF processing of RFC 5061 to perform a lookup per VTAG only, and rely on
VTAG + SCTP AUTH. 

So it is likely a reasonable assumption, but the usage of SCTP-AUTH here. Is
that assuming that one actually have key-management in place or that one
uses key 0 based on the initial handshake only that only provides protection
against entities that don't have on path interception capabilities?
Shouldn't the difference between having a properly keyed SCTP AUTH and just
key 0 here be listed as that affect the possibilities to hijack an SCTP
association using the NAT traversal tools?

More detailed comments.

A) Section 4.3: 

   If an outgoing SCTP packet contains an INIT or ASCONF chunk and a
   matching NAT binding table entry is found, the packet is processed as
   a normal outgoing packet.

Is this intending to say: 

For outgoing SCTP packet that has a matching NAT binding table entry, it IP
source address translated and forwarded, even if the SCTP packet contain
INIT or ASCONF chunks? 

Any case when a matching entry would need to be updated? For a internal host
that hasn't disabled restart, the internal tag could change in this
scenario? Does the entry need to be updated or for where restart disabled
has not occurred on simply put in a wild card entry in the internal vtag
field?

B) Section 4.3: Restart procedure disabled, I think one should add a
paragraph to make clear that with restart procedure one mean because I have
trouble figuring out what procedure is actually meant here and what the
endpoint is not to do. Section 5.3.1 is not clear on that either. So there
are signalling for something that is not clearly defined what it is. 

C) Section 5.2.1, 5.2.2, and 5.2.3:

Note that if the packet will not fit in the ERROR chunk
      or ABORT chunk being sent then the bytes that do not fit are
      truncated.

So how does the NAT determines if this is the cause? Only local MTU? What
are the risk here for the ABORT message is being dropped due to being
oversized? 

D) Section 5.3.2: What are the security risks with this parameter. Per
Section 6.4.1 an internal host could potentially hijack an association using
this. The NAT can't verify the ASCONF authentication. That can only be
expected of the remote host. So it appears that there are some risk that
another SCTP host behind the same NAT could use this to hijack the SCTP
association. If that is possible I think it should be noted.  

E) Section 6.1:

   Every association setup from a host behind a NAT function MUST NOT

   use multiple internal addresses.  The INIT chunk MUST NOT contain an

   IPv4 Address parameter, IPv6 Address parameter, or Supported Address

   Types parameter.  The INIT ACK chunk MUST NOT contain any IPv4

   Address parameter or IPv6 Address parameter using non-global

   addresses.  The INIT chunk and the INIT ACK chunk MUST NOT contain

   any Host Name parameters.

I think this statement is over restricting. For a peer that is behind a NAT
with configured forwarding port(s) they could use IP addresses or host names
that are looked up and having the NAT's external address put into the DNS. I
understand that putting a host name as a client that hasn't had explicit
establishment with assign the port that the client is using with the NAT
could have issues. So I think this is a SHOULD NOT unless the IP address is
know to be publicly reachable or state has been explicitly established in
the NAT(s) prior to the inclusion. That would allow port signalling or
others to follow this specification without being in violation when they
include known working addresses. Are there other reasons for the MUST NOTs?

F) Section 6.2: Handling collisions when restart is not disabled. 

This section does not appear to discuss the case when restart is not
disabled and the VTAGs can't be used for collision detection. 

G) Section 6.2 and 6.2.2.

So based on the above attempt to discuss what occurs when restart is not
disabled. This section recommends new VTAG. Shouldn't a new client port be
recommended as that is what going to resolve the issue in the restart
enabled case. 

H) Section 6.2: Also if the INIT-ACK is dropped due to collision and the
remote peer is not supporting this spec, that error is lost. Any possibility
to send an error message to the internal about the collision in cases where
restart disabled parameter was not part of the response?

I) Section 6.2.1:

If a packet containing
   an INIT chunk or an ASCONF chunk, the source and destination address
   and port numbers MUST be swapped.

This sentence are grammatically in error there is something missing prior to
the comma. Or "containing" would need to be "contains".

J) Section 6.4.1:

   If the NAT function receives a packet from the internal network for
   which it has no NAT binding table entry and the packet contains an
   ASCONF chunk with the VTags parameter, the NAT function MUST update
   its NAT binding table according to the verification tags in the VTags
   parameter and, if present, the Disable Restart parameter.

So if there are no binding table entry, shouldn't it create one? I assume
this is for the case where the SCTP association has been created over the
other path, and then needs to establish an additional path over this other
network. 

K) Section 6.5:

   Therefore, a NAT function MUST be able to handle IP-level fragmented
   SCTP packets.  The fragments MAY arrive in any order.

So that means either reassembly, or if one got the first fragment put in
some state that allows one to find how to forward the rest of the fragments?
So far most if not all NAT implementors have not supported fragmented
packets. Wouldn't it be better to actual be realistic here and figure out if
the sender can determine this and do something about it?

L) Section 7. 

What is default inactivity timer for SCTP? The previous NAT specifications
have had explicit default timers for inactivity tear down. I think this one
should have that too. Sure the YANK model is fine to have the possibility to
change that. Keep alive and discussion of what to do here is interesting. 

M) Section 8: Improving the examples. 

I would recommend using port numbers in the >1023 range. 

Section 8.2. I would recommend that you give the second Host B address from
the example range 198.51.100.0/24 (TEST-NET-2) so that it shows clearly that
Host B is homed into two different network ranges.

N) Section 11. 

I think it should be noted that SCTP NAT in the current definition does
provide quite different filtering properties than NATs for UDP and TCP. 

O) Section 11.

If an endpoint uses SCTP-AUTH, then the ABORT or ERROR with T and M can't be
evaluated by SCTP-AUTH as it isn't signed. This requires discussion about
how one can verify them without SCTP-AUTH or running it at least on the sent
packet?

P) So RFC 5061 is stated in terminology section as needed to understand. I
think it would be good to actually include the reference on the first
reference to ASCONF chunk.

Cheers

Magnus Westerlund