Traversal Using Relays around NAT (TURN): Relay Extensions to Session Traversal Utilities for NAT (STUN)
draft-ietf-tram-turnbis-29

Note: This ballot was opened for revision 27 and is now closed.

Alissa Cooper Yes

Adam Roach Yes

Comment (2019-07-09 for -27)
Thanks for all the work everyone did to update the TURN specification. I am
balloting "Yes," as I think these are important changes to have published. I
do have a small handful of comments that, while they don't rise to the level
of a DISCUSS, are nonetheless rather important to consider before progressing
this draft to an RFC.

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

§5:

>  The server SHOULD
>  include the SOFTWARE attribute in all Allocate and Refresh responses
>  (either success or failure) and MAY include it in other responses or
>  indications.

The Security Considerations section should probably include some mention of this
behavior, and the trade-offs between allowing clients to know which software
(and version) the server is running against providing this information to
potential attackers who can use it to determine what software vulnerabilities
may exist in that specific server and version.

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

§11.5:

>   It also verifies that the IP
>   packet in the ICMP packet payload contains a UDP header.

It's not clear what is being asked of implementations here. Is this just a
check of the protocol field, or is it more involved than that? Is the
intention that the server also check that the checksum is valid (i.e., matches
the computed checksum or is zero)? If so, please spell it out more precisely.
If this language is intended to imply even more rigorous checks, then it needs
a lot more text.

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

§12:

>     0x5000-0xFFFF: Reserved (For DTLS-SRTP multiplexing collision
>     avoidance, see [RFC7983].

Given that values through 0x7FFF were permitted by the previous version of the
protocol, it's unclear what what recovery path this document anticipates for
legacy clients that might request channels in the 0x5000 - 0x7FFE range. The
server behavior is well defined (the server sends a 400 response), but this
seems to represent a fundamental backwards incompatibility, given that the
client will take this to be a final request error and presumably abandon the
attempt to use the TURN server.

If the assumption is that no existing deployed legacy clients use channel
numbers greater than 0x4FFF, please include that as a note. If the intention is
instead that we're breaking backwards compatibility in a limited fashion, please
include that as a note. If the rationale is some explanation other than these
two, please include it as a note.

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

§18.12:

>  Reason Phrase:  The recommended reason phrases for error codes 440
>     and 508 are explained in Section 19.  The reason phrase MUST be a
>     UTF-8 [RFC3629] encoded sequence of less than 128 characters
>     (which can be as long as 509 bytes when encoding them or 763 bytes
>     when decoding them).

This doesn't make a lot of sense to me. If arbitrary sequences of 128 UTF-8
characters are allowed, then the on-the-wire encoding might be as long as 512
bytes (rather than 509), even if this would only result from the use of unusual
strings (e.g., 128 consecutive emoji). I'm even more lost by the indication of
"763 bytes," as I can't figure out what "decoding" operation is being indicated
here, nor can I figure out what kind of decoded sequence would use 5.96 bytes
per character. Please double-check your numbers, and add more text indicating
what is meant by "encoding" and "decoding" in this sentence.

Also, nit: "...fewer than 128 characters..."

Magnus Westerlund Yes

Ignas Bagdonas No Objection

Deborah Brungard No Objection

Roman Danyliw (was Discuss) No Objection

Comment (2019-07-22 for -28)
Thank you for addressing my DISCUSS and COMMENT items.

Benjamin Kaduk (was Discuss) No Objection

Comment (2019-07-22 for -28)
Thank you for addressing my Discuss points!

Suresh Krishnan No Objection

Comment (2019-07-10 for -27)
* Section 3.7

I think an informative reference to [draft-ietf-intarea-frag-fragile] would be very useful here. It is a BCP that covers this issue in great detail and has specific recommendations for different audiences.

* Section 13.2 Page 53

"If the packet is being sent to the peer"

The term "the packet" is ambiguous here. This could potentially refer to the ICMPv6 packet being sent or the original packet that caused the error. Can you please clarify which one you mean here?

* References

I do have a better citation for the Fragmentation Considered Harmful paper if you want to use it.

   [Kent]     Kent, C. and J. Mogul, ""Fragmentation Considered
              Harmful", In Proc. SIGCOMM '87 Workshop on Frontiers in
              Computer Communications Technology, DOI
              10.1145/55483.55524", August 1987,
              <http://www.hpl.hp.com/techreports/Compaq-DEC/
              WRL-87-3.pdf>.

Warren Kumari No Objection

Mirja Kühlewind (was Discuss) No Objection

Comment (2019-07-26 for -28)
Thanks for addressing my discuss!

OLD COMMENTS
----
Some other technical comments/questions:

1) Sec 3.7: 
"or use UDP fragmentation [I-D.ietf-tsvwg-udp-options]."
I believe the possibility to use UDP fragmentation was brought up by the TSV-ART review (Thanks Joe!). However, I would like to mention that this can only be used if supported by both endpoints and that should probably also be remarked here. The next sentence in the draft indicated this by saying "until UDP fragmentation support is available", however, this actually seem to be editorially a bit misplaced there and could explain more. See also this text in draft-ietf-tsvwg-udp-options:

"FRAG needs to be used with extreme care because it will present
   incorrect datagram boundaries to a legacy receiver, unless encoded
   as LITE data (see Section 5.8)."

Also note that draft-ietf-tsvwg-udp-options is still under development and we don't have much deployment experience with it yet. 

And further, in the same section. There is also draft-ietf-tsvwg-datagram-plpmtud on "Packetization Layer Path MTU Discovery for Datagram Transports". Please also be aware that there is an extensive TSV-ART for draft-ietf-tram-stun-pmtud. Both might impact the final content of this section.

2) sec 11.5:
"When the server receives an ICMP packet, the server verifies that the
   type is either 3 or 11 for an ICMPv4 [RFC0792] packet or either 1, 2,
   or 3 for an ICMPv6 [RFC4443] packet."
Restricting to a set of known types, doesn't seem to support future extensibility very well...

3) sec 12.5:
"Over TCP and TLS-over-TCP, the ChannelData message MUST be padded to
   a multiple of four bytes in order to ensure the alignment of
   subsequent messages."
Not exactly sure why this is useful...? Is this to align with STUN and therefore make processing somehow easier? Is that really needed. And exception should be easy to implement and should save some bytes which is the as I understood it the whole purpose of channels, no?

4) 12.6:
"Note that if
   the Length field in the ChannelData message is 0, then there will be
   no data in the UDP datagram, but the UDP datagram is still formed and
   sent."
Can you maybe add some more text and explain why this is useful?

5) sec 15:
RFC6824 will soon be obsoleted by draft-ietf-mptcp-rfc6824bis and please s/TCP multi-path/Multipath TCP/.

6) Just a thought looking at section 14 and 16: It could have been nice to provide an ECN feedback field from the server to the client in case a ECN marked packet is received from the peer... however, I guess that future work at this point in the process...

7) sec 18.13: Maybe I missed this because I reviewed this doc over 3 days, but is only the ICMP Attribute send to the client or is the actual ICMP packets or as much as possible of that packet includes as well?

8) sec 23:
"Response: TURN will no longer be needed once there are no longer any
   NATs.  Unfortunately, as of the date of publication of this document,
   it no longer seems very likely that NATs will go away any time soon.
   However, the need for TURN will also decrease as the number of NATs
   with the mapping property of Endpoint-Independent Mapping [RFC4787]
   increases."
Yes... so you don't think that IPv6 will be any help here?


Editorial comments:

1) Sec 6:
"The relayed transport address MUST be unique across all
   allocations, so it can be used to uniquely identify the allocation.

   Both the relayed transport address and the 5-tuple MUST be unique
   across all allocations, so either one can be used to uniquely
   identify the allocation, [...]"
These two sentences seem quite redundant. The first one was added in this draft. The second one was already there in RFC5766.

2) sec 7.1:
"Since this specification only
   allows UDP between the server and the peers, it is RECOMMENDED that [...]"
Wordings ("only allows") seems weird to me given use of other proposals is at least to some extend discussed.

Nits:
sec 7.1.: s/the client pick a currently unused transport address/the client picks a currently unused transport address/

Barry Leiba No Objection

Alexey Melnikov No Objection

Martin Vigoureux No Objection

Éric Vyncke No Objection

Comment (2019-07-07 for -27)
Thank you all for the work put into this clear and well-written document. I also appreciate the fact that TURN server can be used to proxy between IPv4 and IPv6; on this topic, this specific use case could probably be described early in the document rather then in section 5 (e.g. when discussing the transport protocol between client and server or even earlier).

For my own curiosity, isn't TURN scope broader than plain NAT: can it also be useful in the absence of NAT if inbound 'connection' are blocked by security policy ?


== COMMENTS ==

-- Section 2 --

Please use the new boilerplate RFC 8174 ;-)

-- Section 3.1 --

Is there any reason why MPTCP is not specified for the communication between TURN client and TURN server? There is a very short explanation in section 15 "TCP multi-path is not used by both SIP and WebRTC protocols [RFC7478] for media and non-media data" but it does not address the use of MPTCP between TURN client/server.

-- Section 3.7 --

The 500 bytes guideline to avoid fragmentation, is there any data backing the sentence "...will generally avoid IP fragmentation." ?

In the same section, the text about 'DF bit' should be clear that it is obviously for IPv4 (it is indicated 2 paragraphs below).

-- Section 3.9 --

The TCP/UDP/DTLS discussion of 'happy eyeball' is confusing between "use the first TCP connection that is established" and "if connections are established on both IP address families..." Which sentence should be used? Why plain RFC 8305 is not enough?

-- Section 7.2 --

What is the expected server behavior when receiving a DONT-FRAGMENT when the REQUESTED-ADDRESS-FAMILY is IPv6 ? The STUN document appears to use DONT-FRAGMENT only when receiving traffic from the peers.

-- Section 9 --

About the permission based on the peer IP address, the text is about UDP, but what about ICMP messages? (obvioulsy their source could be any router on the path) The text should refer to section 11.5