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

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

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]

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