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

["Konda, Tirumaleswar Reddy" <[e-mail]>]

Hi Adam,

Thanks for the review. Please see inline

> -----Original Message-----
> From: tram <[e-mail]> On Behalf Of Adam Roach via
> Datatracker
> Sent: Wednesday, July 10, 2019 11:20 AM
> To: The IESG <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: [tram] Adam Roach's Yes on draft-ietf-tram-turnbis-27: (with
> COMMENT)
> 
> 
> 
> 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.

Yes, Ben and Christopher raised similar point as part of Secdir review.  I proposed to update Security Considerations section as follows:
SOFWATE attribute can reveal the specific software version of the TURN client and server to eavesdropper and it might possibly allow attacks against vulnerable software that is known to contain security holes. If it is important to prevent an eavesdropper from learning the software version, 
TURN can be run over (D)TLS.

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

The check is only to extract the UDP source and destination ports. I will update the line as follows:
It also verifies that the IP packet in the ICMP packet payload contains a UDP header, and if a UDP header is present, the server extracts the UDP source and destination port information.

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

Good point, added the following note:
Note that the channel number range is not backwards compatible with RFC5766, and cannot be used in environments where such compatibility is required.

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

We are re-using the reason phrase defined in https://tools.ietf.org/html/draft-ietf-tram-stunbis-21#section-14.8. It says "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)."  

Cheers,
-Tiru

> 
> 
> _______________________________________________
> tram mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tram