Subject: [secdir] Review of draft-ietf-tram-stun-dtls-03

From: Simon Josefsson <[email-masked]>
Date: Thu, 19 Jun 2014 14:10:51 +0200

Hello,

I have reviewed this document, and believe it is ready with nits.

MAJOR:

* The document is silent on whether DTLS cookies is required or not
  for STUN.  DTLS cookies is one of few security differences between
  TLS and DTLS.  The DTLS document says servers SHOULD use cookies,
  but as not requiring this can lead to amplification attacks, I
  suggest adding a MUST here.  For example, add the following to the
  end of section 3:

    Servers implementing this document MUST send DTLS cookies to protect
    against some Denial of Service attacks.

  Of course, I'm not to decide this, and I'm certainly no STUN expert,
  so the authors and/or WG should think about whether mandating DTLS
  cookies makes sense for STUN.

  Note that section 4.6 require DTLS cookies for TURN.  Is this
  difference intentional?  Or did I overlook where this draft requires
  it for STUN too?

* There is a bit of terminology mixup, and I think the origin of the
  mixup is section 1 referring to "TLS-over-UDP" as "DTLS".  That is
  unfortunate for two reasons: 1) DTLS is not bound to UDP; DTLS can
  be used over TCP or other transports, and 2) DTLS is similar but not
  identical to TLS.  DTLS is not the same as TLS run over UDP.
  Quoting section 1:

   TLS-over-UDP (referred to as DTLS [RFC6347]) offers the same security
   advantages as TLS-over-TCP, but without the undesirable latency
   concerns.

  I suggest to rewrite the paragraph into:

   STUN transported over DTLS [RFC6347] over UDP offers the same
   security advantages as TLS-over-TCP, but without the undesirable
   latency concerns.

  Further, in section 3 clarify that the STUN transport "DTLS" is DTLS
  over UDP which isn't otherwise explicit:

  OLD:
   STUN [RFC5389] defines three transports: UDP, TCP, and TLS.  This
   document adds DTLS as a valid transport for STUN.
  NEW:
   STUN [RFC5389] defines three transports: UDP, TCP, and TLS.  This
   document adds DTLS (over UDP) as a valid transport for STUN.

* Cipher suites.  Quoting the document:

   Instead of TLS_RSA_WITH_AES_128_CBC_SHA, which is the default cipher
   suite for STUN over TLS, STUN over DTLS, at a minimum, MUST support
   TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 and MUST support
   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256. STUN over DTLS MUST prefer
   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 and any other Perfect Forward
   Secrecy (PFS) cipher suites over non-PFS cipher suites.

  I think this is good text, but still, some concerns:

   1) The word "support" is not precise.  Does it mean 1) that
      implementers MUST implement it (but not necessarily enable
      them), or that 2) all deployed clients MUST offer at least these
      two ciphersuites, and that all servers MUST accept at least
      these two ciphersuites if the client doesn't offer anything
      else?  I assume the latter was the intent, but it can be read as
      the former.

   2) I don't think "PFS cipher suites" is well-defined in this
      context, as PFS is only briefly discussed in the TLS spec.
      There is a bunch of DHE key exchange algorithms defined for TLS,
      are all of them relevant?  Probably not the anonymous ones?

   3) The requirement to prefer some cipher suites is generally for both
      client and server, but in DTLS they behave differently.  Clients
      list a set of acceptable ciphersuites, and the server picks one.
      Is it permitted for clients to list non-PFS ciphersuites at all?

   4) The text can lead to poor decisions for broken ciphers.  There
      are PFS ciphersuites out there based on broken ciphers, and
      there are non-PFS ciphersuites that are not known to be broken.
      For example, TLS_DHE_DSS_WITH_DES_CBC_SHA is PFS but DES is
      clearly broken.  I don't think you want a server to prefer
      TLS_DHE_DSS_WITH_DES_CBC_SHA over, say,
      TLS_DH_RSA_WITH_AES_256_CBC_SHA256, right?

   5) Since you are explicit about ciphersuites, and appear to worry
      about ciphersuite selection, consider forbidding DES and RC4
      directly since they are known weak cipher.  This would resolve
      the last concern too, I think.

  A straw-man:

   Implementations of STUN over DTLS, and deployed clients and
   servers, MUST support TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 and
   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, MAY support other
   ciphersuites, and SHOULD NOT support any insecure ciphersuites.
   Perfect Forward Secrecy (PFS) cipher suites MUST be preferred over
   non-PFS cipher suites.  Cipher suites with known weaknesses, such
   as those based on (single) DES and RC4, MUST NOT be used.

MINOR:

* TCP ports != UDP ports.  Typo fix:

OLD:
   By default, STUN over DTLS MUST use port 5349, the same port as STUN
..
   By default, TURN over DTLS uses port 5349, the same port as TURN over
NEW:
   By default, STUN over DTLS MUST use port 5349, the same port number
as STUN ..
   By default, TURN over DTLS uses port 5349, the same port number as
TURN over

* SRV terminology mixup.

OLD:
   SRV records, the service name MUST be set to "turns" and the
   application name to "udp".
...
   SRV records, the service name MUST be set to "stuns" and the
   application name to "udp".
NEW:
   SRV records, the service name MUST be set to "turns" and the
   protocol name to "udp".
...
   SRV records, the service name MUST be set to "stuns" and the
   protocol name to "udp".

/Simon

---

From: Simon Perreault <[email-masked]>
Date: Fri, 20 Jun 2014 08:15:46 -0600

Thanks a lot for your review! The authors will reply and publish a new
revision shortly.

Simon

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Fri, 20 Jun 2014 08:34:37 -0600

Hi Simon,

Thanks for the review.  Please see my responses inline.

>   difference intentional?  Or did I overlook where this draft requires

Yes, you are right, but I do not think that adding this for all the STUN
Usages is right, it should be added on a usage by usage basis.

The Nat Discovery Usage (section 4.1) and NAT Behavior Discovery Usage
(section 4.5) MUST definitively use the cookie in the same way the TURN
Usage is using it.

As for the other Usages (Connectivity Check - 4.2, Media Keep-Alive -
4.3 and SIP Keep-Alive - 4.4), I do not think that the cookie is useful,
based on slide 3 of the following presentation by Martin Thomson:

http://www.ietf.org/proceedings/interim/2012/06/12/rtcweb/slides/slides-interim-2012-rtcweb-2-7.pptx

This said, I'd like some guidance there.

So I will add the same text that is in section 4.6 to sections 4.1 and
4.5.  I will also add to section 4 some text saying that new STUN Usages
should discuss if it is appropriate to use the cookie or not.

>   be used over TCP or other transports,

Well, the same can be said for TLS-over-TCP, e.g. RFC 3436 predates RFC
5389, but RFC 5389 still uses the TLS-over-TCP terminology.

I think what RFC 5389 meant is "in this spec, TLS means TLS-over-TCP",
and in the same way in -stun-dtls we mean "in this spec DTLS means
DTLS-over-UDP".

> and 2) DTLS is similar but not

I agree.

>    security advantages as TLS-over-TCP, but without the undesirable

I would prefer the following, as it establishes DTLS as an alias for
DTLS-over-UDP for the remaining of the spec:

"DTLS-over-UDP (referred to as DTLS [RFC6347]) offers the same security
 advantages as TLS-over-TCP, but without the undesirable latency
 concerns."

>    document adds DTLS (over UDP) as a valid transport for STUN.

With the text I propose above, we do not need this modification.

>    4) The text can lead to poor decisions for broken ciphers.  , There

OK.  I just prepended your first sentence with "Instead of
TLS_RSA_WITH_AES_128_CBC_SHA, which is the default cipher suite for STUN
over TLS, "

>    By default, TURN over DTLS uses port 5349, the same port number as

OK

>    SRV records, the service name MUST be set to "stuns" and the

OK

-- 
Marc Petit-Huguenin
Developer  |  Jive Communications, Inc.
Jive.com  |  [email-masked]