Re: [tsvwg] draft-ietf-tsvwg-natsupp-12 -- comments on the draft

[Michael Tuexen <[e-mail]>]

> On 18. Jul 2018, at 18:19, G Fairhurst <[e-mail]> wrote:
> 
> 
> Thanks for this revision, but I still do have a number of concerns that I think need to be acted upon before a WGLC. The editors may like to address some/all of these before the IETF TSVWG meeting... If people have opinions please.
Hi Gorry,

thanks for the feedback. See my responses in-line.

Best regards
Michael
> 
> Gorry
> 
> ---
> I think the use of "NAPT" seems odd to me. I think in most cases the document is talking about adding NAPT functions (that I know of as an ALG) to support SCTP NAT traversal.
Sometimes NAT means "NAT device". So a NAT device can provide NAPT functionality in the sense
that it can handle local port number collisions.

I've changed the text such that "NAT device" is used when talking about a device.
> ---
> 
> I think this text is not quite correct:
> "This document describes an SCTP specific variant NAT and specific
>   packets and procedures to help NATs provide similar features of NAPT
>   in the single-point and multi-point traversal scenario.  An SCTP
>   implementation supporting this extension will follow these procedures
>   to assure that in both single-homed and multi-homed cases a NAT will
>   maintain the proper state without needing to change port numbers."
> 
> I think this text may be a better start:
> "  This document  specifies procedures allow a NAT to support SCTP
>  by providing similar features
>  to those provided by a NAPT for TCP and other supported protocols "
> 
>  The document also specifies a set of data formats for SCTP packets
>   and a set of SCTP endpoint procedures to support NAT traversal. An SCTP
>   implementation supporting these procedures
>   can assure that in both single-homed and multi-homed cases a NAT will
>   maintain the proper state without needing to change port numbers."
> ---
Taken.
> There are several text note that say something like:
> ' [NOTE:
> 
>      ASSIGNMENT OF M-BIT TO BE CONFIRMED BY IANA.
> 
>   ]"
> 
> (I do not understand from that *what* you wish the RFC-ED to insert here. I think we want the RFC-ED to remove this commnent, and if we keep thois, we should say so.)
> 
> Actually, I really do we need to actually retain the comment now, as we move to WGLC - could we just remove?
The reason we have this is that in the past some WG wanted a note making clear what needs
to be changed if IANA does not assign code points as used in the ID.
I think the point was that IANA is assigning the numbers, no the authors/editors of the ID.
In reality, IANA always assigned as we requested except for one case. In that
case IANA's assignment would have resulted in problems and they changed it
to the suggested value without any major discussion. We just had to make
clear why IANA's choice was problematic.

So in the above case it is a marker that if IANA does not assign 0x02 as the M-bit
we have to change the figure.

I'm changing all these notes to a consistent format for now.

If you want me to remove them all together, I have no problem with that...
> ---
> Nits:
> 
> "NAT box" to "NAT device"
Changed. (NAT box was use the be consistent with middle box.)
> "NAT which has" to ""NAT that has"
Changed.
> "can't" to "is unable to" (or similar)
Changed.
> "i.  e." to "i.e."
Changed.
> - is "end point" one word?
At least "endpoint" is used in RFC 4960. So I changed all occurrences of "end point" to "endpoint".
> 
> ---
> To me, NAPT includes port translation of the IP payload, and NAT only changes IP addresses. Is this your interpretation, and has this been used consistently?
To be precisely, a middle box providing NAPT functionality can handle local port number collision by
translating them. However, they do not always translate the port numbers.

So I think we agree on the semantic. I tried to use this terminology with this interpretation.
> ---
> I am unclear what is intended by the para with this statement:
> "When considering this feature it is possible to have multiple levels
>   of support. "
> - This para needs to be rewritten, does the table refer to support of the new mechanisms in this document? or do I misunderstand?
> are the features just what is described in one of the sections, or something else. I was pretty confused.
> What does this mean:
> "This is because for the most part of the current situation".
Changed to

This assumes that the NAT device does not handle SCTP packets at all and
all SCTP packets sent externally from behind a NAT device are discarded by
the NAT.

> - what situation?
Assume you have endpoint A communicating to endpoint B via a middlebox M. At one time is was
requested that we describe what happens if A, B, or M do or do not support the extension
defined in this document. This gives the 8 combinations described in the table.
> ---
> "This document uses the following terms"
> - I think this should also use the terms defined by the IETF BEHAVE work. Please be consistent with these and say so in the terminology.
I think we are consistent. Do you have an example where we are not?
> ---
> 
> I am not a huge fan of a construct that uses MUST followed by a bullet list, as in:
> "The endpoint MUST do the following:"
> - I would strongly advise you to use "MUST" instead in each individual bullet, so that the requirement is clear.
Changed.
> 
> ---
> The meaning of this requirement appears ambiguous:
> "if it does not discard the incoming ERROR chunk."
> If I understamd this, you could write:
> "if the NAT device does not discard the incoming ERROR chunk, then the server MUST validate that the peer of the SCTP association supports the
>      dynamic address extension".
Changed to
It MUST validate that the peer of the SCTP association supports
the dynamic address extension. If the peer does not support it, the NAT Device
MUST discard the incoming ERROR chunk.
> ---
> Section 4.1 and 4.2
> - I think (maybe I am wrong) this is all background? and therefore could be subsections of section 1?
Well, Section 4.1 defines the scenarios we are considering. Not sure if this fits well in the Introduction.
4.2 is a motivation for 4.3. So I would prefer to keep it together.
> ---
> The english in section 4 reads like a paper, there are many loose terms here, that need to be tightened.
> e.g. (but please check the entire text):
> "Furthermore we are
>   focusing in this section on the single point traversal scenario."
> - who is we, and what does this mean?
> "The modification of SCTP packets sent to the public Internet is easy."
> - what does that mean?
> "It may also be necessary to establish some state in the NAT
>   box to handle incoming packets, which is discussed later."?
I have clarified the text.
> ---
> Section 4.3 needs to be renamed to align with BEHAVE terms.
Where is the text not aligned with BEHAVE terms?
> ---
> There are several examples of requirements that do not say how to realise them, this is a problem. I suspect we could simply be missing cross-references to the SCTP base spec to tell the NAT implementer where the appropriate method is defined.
> "A NAT box MUST support IP reassembly of received fragmented SCTP packets."
Not sure what you want the document to say.

If a NAT device supports UDP and should handle fragmented UDP packet, isn't
it a requirement in this case for the NAT device to support IP-reassembly?
See REQ-14 of https://tools.ietf.org/html/rfc4787#section-11

Since it might happen that an SCTP stack has to fragment SCTP packets,
the requirement of support for reassembly was made.

I don't think that IP reassembly is defined in an SCTP specification, but
in the IP specifications. Since I don't see them referenced in REQ-14,
I'm not sure what to do here...
> 
> ---
> 
> Section 6.6 is inherently IPv4, and I think thoought is needed to help avoid silly review comments:
> "Handling of Fragmented SCTP Packets" to "Handling ofFragmented SCTP Packets"
> >> Is there an RFC that decsribes in-network reassembly of IPv4 fragments?
I don't know. See REQ-14 of https://tools.ietf.org/html/rfc4787#section-11
> >> If you allow this, then you place requirements on all IP fragments passing through this NAT, that you should state.
> >> You also need to discuss timer actions, MSL, etc.
> >> How do you deal with fragments that set "DF"?
> >> Do you propose or not to re-assemble IPv6 fragments?
> >> There are also security issues in performing reassembly!
Isn't all of this not specific to SCTP and the same for UDP?
> 
> After all this, I really wonder if this is a GOOD thing to include in the NAT function.  Can you justify why this is an important function?
If a NAT device does not support reassembly, an SCTP association using (for some amount of time) IP fragmentation,
can't work. This can happen because the INIT or INIT-ACK is too large or the PMTU shrunk and the endpoint
has to retransmit DATA chunks which do not fit in an SCTP packet with the smaller MTU.
> ------------
> 
> Please rephrase this:
> "The suggested value for the T bit is 0x01 and for the M bit is 0x02."
> as something like:
> "IANA is requested to assign the value of 0x01 for theT bit and the value of 0x02 for theM bit."
> ---
> Please rephrase this (multiple):
> "It is suggested to use the values given below."
> as
> "IANA is requested to assign the following values:"
OK. All occurrences of "suggested" replaced by "requested".
> ---
> "More than one host behind a NAT may pick".
> - could this be.
> "Multiple endpoints behind a NAT device could use"
Changed to "NAT device could select"
> ---
> Why is there a section 6.1 subsection, this text seems to belong in section 6 as an overview of everything in section 6?
Move the text of Subsection 6.1 directly under Section 6.
> ---
> Section 6.2
> - I retain my comment that I think this section should differentiate NAT and endpoint actions.
> In this case it should somehow be labeled as an endpoint mechanism. e.g.,
> "Considerations when an endpoint sets up an SCTP association"
Changed to "Association Setup Considerations for Endpoints"
> ---
> In section 6.2, there are a set of requirements:
> e.g, "Every association MUST initially be set up single-homed." , etc, etc.
> I am unclear. Are these changes to the base spec, or clarifications of what the base spec says, either way I would not feel comfortable without some sort of reference back to the base spec.
I think this it is clear what it means. No address parameters in the INIT/INIT-ACK.
Not sure what to reference to...
> ---
> I think the first part of section 6.3 refers to what NATs provide, I think you need to make a subsection title including "NAT" and place the first part in this.
> e.g.
> "6.3.1 NAT Handling of Internal Port Number and VTAG Collisions"
> I think the last 3 para  of section 6.3 refers to what NATs provide, I think you need to make a subsection title including "NAT" and place the first part in this.
> "6.3.2 SCTP Endpoint Handling of Internal Port Number and VTAG Collisions"
> ---
OK. Done.
> I really think that section 6.4 can and should also be sub-divided in the same way.
Done.
> ---
> I really think that section 6.5 can and should also be sub-divided in the same way.
Done.
> ---
> In section 6.5, this also states:
> "Upon reception of this ERROR chunk by an SCTP endpoint the receiver SHOULD take the following actions"
> - what happens if it does not? (please say).
> ---
> Section 6.6 is clearly just "NAT" ? - if I understand, can this be added to the title?
Correct and added to the title.
> ---
> Section 6.7 is clearly just "SCTP Endpoint" ? - if I understand, can this be added to the title?
Correct and added to the title.

These changes are in -13. I'll work through 63 Maxims comments next. Most likely discuss some with him
F2F in Montreal.

Best regards
Michael
> ---
>