[tram] Adam Roach's Yes on draft-ietf-tram-turnbis-27: (with COMMENT)

[Adam Roach via Datatracker <[e-mail]>]

Adam Roach has entered the following ballot position for
draft-ietf-tram-turnbis-27: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tram-turnbis/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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..."