[tsvwg] Martin's AD Review of draft-ietf-tsvwg-natsupp-22

[Martin Duke <[e-mail]>]

This draft is so nice ADs reviewed it twice :-)

I broadly agree with Magnus's comments, and have some further ones that may
overlap a bit:

1) The draft does not really explain how the endpoint discovers it is
behind a NAT and therefore should conduct these NAT-friendly actions. This
might be a priori knowledge (via the new socket API), or maybe it receives
a private IPv4 address, or is it reliant on receiving ABORTs with M flags?
All three? There ought to be something about discovery here. If we're
reliant on M-flags, endpoints should in general send the disable-restart
parameter in case they're behind a NAT, yes? (Cue Magnus's point about this
draft updating 4960). It it's all based on socket API calls, I have real
questions about how this would work in practice -- are all applications
that use an SCTP socket going to ask the user if they're behind a NAT?

2) A brief summary of restart would be useful. I went to 4960 and found the
relevant text scattered through the document. It was hard to quickly get a
full understanding of this mechanism, which is especially relevant here. It
is not clear to me what an endpoint does differently when restart is
disabled, as Magnus said: is it that it never tears down an existing
connection based on the new exchange?

Sec 6.3.2, on a second reading, at least allowed me to understand (I think)
exactly what the problem is. It would be nice to have a clear and concise
explanation of the restart ambiguity earlier in the document.

3) In Sec 4.3, it would be great to have numbered figures. But I will
struggle through without them:
(a) " For an incoming packet containing an INIT chunk a table lookup is
made only based on the addresses and port numbers." Why are we looking up
addresses? The state table doesn't have them! The diagram doesn't say
anything about looking up addresses so I think this is just a typo?

(b) " The processing of outgoing SCTP packets containing no INIT chunks" It
is not clear to me what happens if the internal address is different on
these packets. The internal address is in the state table but is not one of
the lookup keys, IIUC. So if all else matches, does this packet go through,
or as part of the lookup does it check the internal IP and drop if there is
no match?

(c) Is it sufficient for the binding table to have one entry for restart
support, or should this be separated out per-direction?

4) In Sec 6.2.2, it seems it would be wise for the endpoint to change its
port, if ephemeral -- particularly if it's enabled restart.

5) Sec 6.3.1 is awkwardly worded because the truth table for what fields
match and what entries there already are more complicated than simply
checking all fields . If the first instance of a port-pair has restart
disabled, then the NAT need not store the Vtags; any other instance must be
rejected. Similarly, if there is already a restart-disabled instance on
that port pair, and an enabled instance arrives, the NAT MUST reject it.

6) Does this draft implicitly assume that there is a single External
Address? If there is more than one, some of the design elements no longer
work: each binding entry needs to record the external address. Also, the
requirement in 6.5 that NATs MUST handle fragmentation becomes a DoS
vector: the NAT has to reassemble packets, to have the information
necessary to rewrite all those fragment addresses (as the necessary vtag is
in only one of the fragments).

NITS:
- Please update the Responsible AD in the shepherd writeup

- Sec 1. s/single IPv4 address/single external IPv4 address

- Sec 6. s/then the these/then these

- Sec 8.4. Why does the remote address change in the example? Is this a
typo?

Thanks
Martin