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

[Michael Tuexen <[e-mail]>]

> On 1. Apr 2021, at 02:31, Martin Duke <[e-mail]> wrote:
> 
> 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?
Hi Martin,

thanks for the review and the comments. I'll prepare a document update which will
try to address the comments from you and Magnus. I'll report the specific changes
for the above issues.

Best regards
Michael
> 
> Thanks
> Martin