Hi Christopher, Thanks for the detailed review. Please see inline > -----Original Message----- > From: tram <tram-bounces@xxxxxxxx> On Behalf Of Christopher Wood via > Datatracker > Sent: Monday, July 8, 2019 1:59 AM > To: secdir@xxxxxxxx > Cc: draft-ietf-tram-turnbis.all@xxxxxxxx; ietf@xxxxxxxx; tram@xxxxxxxx > Subject: [tram] Secdir last call review of draft-ietf-tram-turnbis-27 > > This email originated from outside of the organization. Do not click links or > open attachments unless you recognize the sender and know the content is > safe. > > Reviewer: Christopher Wood > Review result: Has Nits > > 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? Server certificate validation is discussed in https://tools.ietf.org/html/draft-ietf-tram-stunbis-21#section-6.2.3. I have modified the text as follows: If (D)TLS transport is used between the TURN client and the TURN server, the cipher suites, server certificate validation and authentication of TURN server are discussed in Section 6.2.3 of [I-D.ietf-tram-stunbis] >- 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 ? - Please see https://tools.ietf.org/html/draft-ietf-tram-turnbis-27#section-15 for TCP-to-UDP relay. If the PMTU is not known, and on legacy or otherwise unusual networks, 500 byte limit for application data is recommended. > 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. - Agreed, Eric (as part of ISEG review) suggested to simplify the text. I have updated the text as follows: o For TCP or TLS-over-TCP, the results of the Happy Eyeballs procedure [RFC8305] are used by the TURN client for sending its TURN messages to the server. > 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. It is discussed in Section 5 in the specification, the server should expire the nonce at least once every hour during the lifetime of the allocation. >- Section 3 could also benefit from > discussion about cleartext versus encryption transports between clients and > servers. It is discussed in https://tools.ietf.org/html/draft-ietf-tram-turnbis-27#section-21.1.6. > 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. Username may not be the user identity, Please see https://tools.ietf.org/html/rfc7635), It could be an ephemeral and unique key identifier. Further, username can be anonymized (please see https://tools.ietf.org/html/draft-ietf-tram-stunbis-21#section-14.4). SOFTWARE and FINGERPRINT attributes are defined in the STUN specification, see https://tools.ietf.org/html/draft-ietf-tram-stunbis-21. - 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? Yes, UDP does not include a congestion control mechanism. > 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. - The errors 486 (Allocation Quota Exceeded) or 508 (Insufficient Capacity) do not reveal information of which other users are using the TURN server. > 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? The client detect and stop the loop, it is similar to HTTP redirection. > 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.) It is already discussed in https://tools.ietf.org/html/draft-ietf-tram-stunbis-21#section-10 > - 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. The section only discusses "offline" dictionary attack, How is offline dictionary attack possible with (D)TLS ? >- 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. As mentioned previously, username may not be the end-user real identity and username can be anonymized. > > Nits and other comments: > > - Section 2: “message-digest” is undefined in the Nonce definition. Thanks, replaced "message-digest" with "server response" > - Section 3: It’s probably worth citing RFC8446 as the recommended version > of TLS. The draft says guidance given in [RFC7525] MUST be followed to avoid attacks on (D)TLS. RFC7525 says TLS 1.3 resolves the vulnerabilities found in previous TLS versions. >- 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. https://tools.ietf.org/html/draft-ietf-tram-turnbis-27#section-21.1.4 discusses this issue in detail. > - Section 3.4: What does “proper security” mean? Thanks, replaced "proper security" with "end-to-end security". >- 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. https://tools.ietf.org/html/draft-ietf-tram-turnbis-27#section-12.6 explains that If the ChannelData message is received on a channel that is not bound to any peer, then the message is silently discarded. > - Section 3.6: Another benefit of > this user space design decision is use of (D)TLS links. - Good point, updated line to say: for example, to allow a TURN server to be integrated into a peer-to-peer application so that one peer can offer NAT traversal services to another peer and to use (D)TLS to secure the TURN connection. Section 5: Where did > the 40 second request buffer timeout come from? It is coming from the STUN specification (see https://tools.ietf.org/html/draft-ietf-tram-stunbis-21#section-6.3.1) > 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. Yes, replaced with " cryptographic hash". > - Section 7.4: What is the retry behavior if allocation > requests timeout? The retry behavior is discussed as follows: o (Request timed out): There is either a problem with the server, or a problem reaching the server with the chosen transport. The client considers the current transaction as having failed but MAY choose to retry the Allocate request using a different transport (e.g., TCP instead of UDP). >- Section 12.5: The STUN requirement for 4-byte > alignment should be cited when discussing the TCP and TLS padding > requirement. Done. > - Section 15: Typo “DON’T FRAGMENT”. Fixed. Cheers, -Tiru > > > _______________________________________________ > tram mailing list > tram@xxxxxxxx > https://www.ietf.org/mailman/listinfo/tram
