Last Call Review of draft-ietf-tram-turnbis-27
review-ietf-tram-turnbis-27-secdir-lc-wood-2019-07-07-00

Request Review of draft-ietf-tram-turnbis
Requested rev. no specific revision (document currently at 29)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2019-05-28
Requested 2019-05-14
Draft last updated 2019-07-07
Completed reviews Secdir Last Call review of -27 by Christopher Wood (diff)
Tsvart Last Call review of -25 by Joseph Touch (diff)
Genart Last Call review of -25 by Vijay Gurbani (diff)
Genart Telechat review of -27 by Vijay Gurbani (diff)
Assignment Reviewer Christopher Wood
State Completed
Review review-ietf-tram-turnbis-27-secdir-lc-wood-2019-07-07
Posted at https://mailarchive.ietf.org/arch/msg/secdir/XL_2Ibl2mLP4yYQxy0QP9x_Axlo
Reviewed rev. 27 (document currently at 29)
Review result Has Nits
Review completed: 2019-07-07

Review
review-ietf-tram-turnbis-27-secdir-lc-wood-2019-07-07

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

The summary of the review is: Ready with nits

Summary:

In general, the document is well written and clearly founded in operational experience. The security considerations are thorough, providing examples where necessary to highlight important problem areas. It draws a clear line between issues best addressed by applications outside of TURN, and provides sufficient detail for those issues in scope. My comments and questions are, hopefully, mostly nits.

General comments:

- TURN server authentication in the case of (D)TLS is underspecified. Are servers assumed to have WebPKI certificates, OOB-trusted raw public keys, or something else? Is there a preferred form of authentication? Relatedly, in Section 3.2, how do clients authenticate the server?
- Section 3.7: Could TURN servers not chunk data from stream-oriented transports (TCP or TLS) to a preferred MTU size before relaying to peers? Specifically, there are likely some cases wherein the server could deal with the client data messages larger than the recommended 500B limit. On that point, should servers even accept data larger than this recommended size?
- Section 3.9: There may be cases where the TLS connection post TCP connection establishment fails. It would therefore seems prudent to not declare victory for one connection over the other until TLS connects, too.
- Section 3 could benefit from a subsection on replays and the nonce role. In particular, as later discussed in the security considerations, some of these attacks are not new to TURN and should therefore be dealt with by the application protocol (SRTP). This section might also describe nonce rotation policies with more specificity, as this is underspecified in the document.
- Section 3 could also benefit from discussion about cleartext versus encryption transports between clients and servers. Encrypting the nonce, username, realm, etc., among other things, has obvious benefits.
- Why are SOFTWARE and FINGERPRINT attributes recommended? It seems like clients should be discouraged from sending these if anything, especially if not used to make allocation decisions on the server.
- Section 5: When servers receive data that exceeds an allocation’s bandwidth quota, servers silently discard this data. This means there’s no allocation-based flow control mechanism between client and server beyond what’s provided by the transport protocol, right? This seems fine, though perhaps some discussion of why this design decision was made would be helpful. For example, I could imagine explicit signals from servers to clients that indicate server state would reveal information about other allocations on the server, which might be problematic.
- Section 7.2 suggests that servers can redirect client allocation requests to other servers. Can this create a loop, wherein two TURN servers redirect to one another? Moreover, is it acceptable for one TURN server to redirect to an unrelated TURN server? (It should be made clear here that these responses are authenticated, as otherwise it would be possible for an on-path adversary to redirect allocation requests to a server it owns.)
- Section 21.1.2: Use of (D)TLS doesn’t help against dictionary attacks much, since presumably there’s low entropy in usernames and passwords alike. Thus, I question whether this is a “stronger” mitigation.
- Section 12.1.6: “username” and “realm” are not considered sensitive? They seem sensitive to me.
- As an extension, it seems possible to improve on what’s in STUN. For example, it may be worthwhile, here or elsewhere, to update STUN’s long term credential key derivation process (MD5(username + realm + password)) to something a bit more modern. This is quite likely out of scope, though in the context of client authentication it seems worth mentioning that TURN is limited to the mechanisms provided by STUN.

Nits and other comments:

- Section 2: “message-digest” is undefined in the Nonce definition. 
- Section 3: It’s probably worth citing RFC8446 as the recommended version of TLS.
- Section 3.4: It might be worth mentioning that use of (D)TLS for the client-to-server transport mitigates the need for Send and Data authentication. 
- Section 3.4: What does “proper security” mean?
- Figure 4: Adding another message exchange wherein a channel message is sent without a prior ChannelBind request would be useful to highlight this dependency and expected behavior from clients and servers.
- Section 3.6: Another benefit of this user space design decision is use of (D)TLS links. 
- Section 5: Where did the 40 second request buffer timeout come from? Adding some details might help.
- Section 6: “secure hash” is undefined, though presumably what is meant is a cryptographic hash with collision resistance. It would be good to clarify this requirement.
- Section 7.4: What is the retry behavior if allocation requests timeout?
- Section 12.5: The STUN requirement for 4-byte alignment should be cited when discussing the TCP and TLS padding requirement.
- Section 15: Typo “DON’T FRAGMENT”.