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