[tsvwg] Review of draft-ietf-tsvwg-natsupp-16

[Gorry Fairhurst <[e-mail]>]

I've reviewed this ID again in preparation for a WGLC and have one 
question that I suggest you address and some comments that I would like 
to see resolved before a WGLC, especially comments that concern the 
clarity of the RFC2119 normative language.

My question concerns section 6.6 on the Handling of Fragmented SCTP Packets:

This says /A NAT box MUST support IP reassembly of received fragmented SCTP
    packets.  The fragments may arrive in any order./

- I understand this, but more explanation is needed here and a reference 
is needed to the section of the SCTP base spec that requires this. This 
can not just be stated, what are the implications?

- I'm curious whether the next sentence actually says also what the NAT 
does if fragementation can not be performed, i.e. it returns an ICMP 
message. In fact, if that happens what does SCTP do? because
the data being sent can not be refragmented by SCTP? Please explain what 
happens.

Best wishes,

Gorry

(as TSVWG Document Shepherd)

-------------

My editorial (hopefully) comments are:

---

The first use of /Stream Control Transmission Protocol/ in the 
introduction does not define SCTP, which I suggest would be useful, 
although obvious from the abstract!

---

/The Verification Tag that the host holding the External-Address/

- could you cite the SCTP base spec after /Verification Tag/ so that a 
stupid reader knows where this is defined?

---

/The public address assigned to the NAT box which it uses/
- I think this needs to be ", which" or "that". You will know how to fix.

---

/Whereas for UDP and TCP this can be done

    very efficiently, for SCTP the checksum (CRC32c) over the entire
    packet needs to be recomputed. /

- could you cite an RFC to explain the CRC32c.

---

/The modification of SCTP packets sent to the public Internet is easy./

- Easy is often regarded as a word expressing opinion and to avoid in 
RFCs. Please choose another explanation, or simplify the sentence.

---

/the packet has to be delivered to/to which the packet has to be delivered/

---

/ In this case the INIT MUST be dropped by

    the NAT and an ABORT MUST be sent back to the SCTP host with the
    M-Bit set and an appropriate error cause (see Section 5.1.1 for the
    format)./
- It may not be clear that INIT and ABORT are special terms in SCTP,
should this say /an ABORT chunk/, and /an INIT chunk/?
- Is /the SCTP host/the SCTP host that originated the packet/ or similar?
- What does /sent back/ mean? - Is this an /SCTP packet must be sent to 
the source.../
===

/the INIT SHOULD be dropped/

- Is this the SCTP packet containing the INIT chunk?

---

/and an ABORT SHOULD be sent back to the SCTP host/

- Is this /an SCTP packet containing the ABORT chunk SHOULD/?

- Is /the SCTP host/the SCTP host that originated the packet/ or similar?

---

In 5.1:

/that are modified by this document./

- I'd prefer to more rigorous in the language. Please say  /that include 
the new fields specified by this document/ or something more.

---

In 6.1:

/An SCTP endpoint may be behind two NATs providing redundancy/

- Replace /may/could/ because this needs top be clearly not about what 
is allowed. I am also concerned that this is not clear whether the two 
NATs are successive, or in parallel. Please add text to explain.

---

/and possibly modified handling procedures from those specified in 
[RFC4960]./

- Unclear to me what was intended by /possibly/ do you mean in some 
instances they do require, or that they could or could not require for 
some viewpoint? Please remove possibly and replace the language.

---

/this can't be used/this cannot be used/

---

/There MUST NOT be any IPv4 Address parameter, IPv6 Address parameter, 
or Supported Address Types parameter in the INIT-chunk. /

- To me this was not completely clear, would you agree with:

/The INIT-chunk MUST NOT contain an IPv4 Address parameter, IPv6 Address 
parameter, or Supported Address Types parameter. /

---

/But this is very unlikely./However, this is unlikely./

---

/can control the 16-bit Natted Port/ XXX

---

/However, in this unlikely event the NAT box MUST send an ABORT chunk
    with the M-bit set if the collision is triggered by an INIT or INIT-
    ACK chunk or send an ERROR chunk with the M-bit set if the collision
    is triggered by an ASCONF chunk./

- I have a strong preference for every RFC2119 declaration to be 
self-describing. /This/ is not clear, please restructure to explicitly 
say the unlikely event in the same sentence.

---

/In the other two cases, the source and destination address and port 
numbers MUST be swapped./

- I have a strong preference for every RFC2119 declaration to be 
self-describing. /other cases/ is not clear, please restructure to 
explicitly state what.

---

/The NAT MUST do the following when processing an

    INIT:

    o  If the INIT is destined to an external address and port for which
       the NAT has no outbound connection, allow the INIT creating an
       internal mapping table.

    o  If the INIT matches the external address and port of an already
       existing connection, validate that the external server supports
       the Disable Restart feature, if it does allow the INIT to be
       forwarded.

    o  If the external server does not support the Disable Restart
       extension the NAT MUST send an ABORT with the M-bit set./

- I have a strong preference to clarify the RFC2119 declaration.
Please restructure as somnething like this:

/The NAT performs following when processing an INIT:


    o  If the INIT is destined to an external address and port for which
       the NAT has no outbound connection, the NAT MUST allow the INIT 
creating an
       internal mapping table.

    o  If the INIT matches the external address and port of an already
       existing connection, the NAT MUST validate that the external 
server supports
       the Disable Restart feature, if it does allow the INIT to be
       forwarded.

    o  If the external server does not support the Disable Restart
       extension the NAT MUST send an ABORT with the M-bit set./
---

/... error cause MUST be sent back./
... error cause MUST be sent to the SCTP host that originated the packet./
- or similar.
----
/Please note that
    such a packet containing an ERROR chunk SHOULD NOT be sent if the
    received packet contains an ABORT, SHUTDOWN-COMPLETE or INIT-ACK
    chunk. /
- Please remove /Please not that/ from an RFC2119 clause, this can not 
be a note.

---
/In such a
    case the NAT MUST send an ERROR chunk with the error cause code set
    to 'VTag and Port Number Collision' (see Section 5.2.1)./
- I have a strong preference for every RFC2119 declaration to be
self-describing. /in such a case/If the NAT discovers an Internal Port 
Number and Verification
Tag collision.../
---
/The endpoint MUST do the
    following:

    o  Validate that the verification tag is reflected by looking at the
       VTag that would have been included in the outgoing packet.
    o  Validate that the peer of the SCTP association supports the
       dynamic address extension, if it does not discard the incoming
       ERROR chunk.
    o  If the association is attempting to add an address (i.e. following
       the procedures in Section 6.7) then the endpoint MUST-NOT consider
       the address part of the association and SHOULD make no further
       attempt to add the address (i.e. cancel any ASCONF timers and
       remove any record of the path), since the NAT has a VTag collision
       and the association cannot easily create a new VTag (as it would
       if the error occurred when sending an INIT).
    o  If the endpoint has no other path, i.  e. the procedure was
       executed due to missing a state in the NAT, then the endpoint MUST
       abort the association.  This would occur only if the local NAT
       restarted and accepted a new association before attempting to
       repair the missing state (Note that this is no different than what
       happens to all TCP connections when a NAT looses its state)./
- Is not yet clear what is required, suggest:
/
/The endpoint processing is:

    o  The endpoint MUST validate that the verification tag is reflected 
by looking at the
       VTag that would have been included in the outgoing packet.
    o  The endpoint MUST validate that the peer of the SCTP association 
supports the
       dynamic address extension, if it does not it MUST discard the 
incoming
       ERROR chunk.
    o  If the association is attempting to add an address (i.e. following
       the procedures in Section 6.7) then the endpoint MUST NOT consider
       the address part of the association and SHOULD make no further
       attempt to add the address (i.e. cancel any ASCONF timers and
       remove any record of the path), since the NAT has a VTag collision
       and the association cannot easily create a new VTag (as it would
       if the error occurred when sending an INIT).
    o  If the endpoint has no other path, i.e. the procedure was
       executed due to missing a state in the NAT, then the endpoint MUST
       abort the association.  This would occur only if the local NAT
       restarted and accepted a new association before attempting to
       repair the missing state (Note that this is no different than what
       happens to all TCP connections when a NAT looses its state)./
/

----
/7.5.  Peer-to-Peer Communication

    If two hosts are behind NATs,/
- Is that each behind a NAT or two behind the same NAT, I assume the former.
---

Please note the recommendations in the yang review.