Re: [tram] Secdir last call review of draft-ietf-tram-turnbis-27

[Benjamin Kaduk <[e-mail]>]

Chris, thanks for the review.  Some more questions/comments inline as I
prepare to ballot on this document...

On Mon, Jul 08, 2019 at 02:18:26PM +0000, Konda, Tirumaleswar Reddy wrote:
> Hi Christopher,
> 
> Thanks for the detailed review. Please see inline
> 
> > -----Original Message-----
> > From: tram <[e-mail]> On Behalf Of Christopher Wood via
> > Datatracker
> > Sent: Monday, July 8, 2019 1:59 AM
> > To: [e-mail]
> > Cc: [e-mail]; [e-mail]; [e-mail]
> > 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]

That helps, but it would still be nice to give some indication of what
certificate  PKI(s) are expected to be used.  Is anything other than the
Web PKI under consideration?

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

Is the editor's copy available for viewing somewhere?  It would be good to
see this (and other changes) in context.

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

There's not much discussion about potential information  leakage via (e.g.)
USERNAME/USERHASH, and really just a claim that the peer address is the
"primary protocol content".

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

These mechanisms  are partial mitigations but can still be susceptible to
cross-connection correlation attacks.

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

At least the latter seems to give some indication that many other users are
using the server at the  time (so that it's overloaded), though not
information about *which* users that is.

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

Where is this specified?

Thanks,

Ben

> > 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
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/tram