Last Call Review of draft-ietf-tram-stun-dtls-03
review-ietf-tram-stun-dtls-03-secdir-lc-josefsson-2014-06-26-00
| Request | Review of | draft-ietf-tram-stun-dtls |
|---|---|---|
| Requested revision | No specific revision (document currently at 05) | |
| Type | Last Call Review | |
| Team | Security Area Directorate (secdir) | |
| Deadline | 2014-06-24 | |
| Requested | 2014-06-12 | |
| Authors | Marc Petit-Huguenin , Gonzalo Salgueiro | |
| Draft last updated | 2014-06-26 | |
| Completed reviews |
Genart Last Call review of -03
by
Roni Even
(diff)
Secdir Last Call review of -03 by Simon Josefsson (diff) Opsdir Last Call review of -03 by Carlos Pignataro (diff) |
|
| Assignment | Reviewer | Simon Josefsson |
| State | Completed | |
| Review |
review-ietf-tram-stun-dtls-03-secdir-lc-josefsson-2014-06-26
|
|
| Reviewed revision | 03 (document currently at 05) | |
| Result | Has Nits | |
| Completed | 2014-06-26 |
review-ietf-tram-stun-dtls-03-secdir-lc-josefsson-2014-06-26-00
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