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

[Simon Perreault <[e-mail]>]

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

Simon

Le 2014-06-19 06:10, Simon Josefsson a écrit :
> 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
>