Skip to main content

Last Call Review of draft-ietf-tls-ecdhe-psk-aead-03
review-ietf-tls-ecdhe-psk-aead-03-secdir-lc-kaduk-2017-05-18-00

Request Review of draft-ietf-tls-ecdhe-psk-aead
Requested revision No specific revision (document currently at 05)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2017-05-18
Requested 2017-05-04
Authors John Preuß Mattsson , Daniel Migault
I-D last updated 2017-05-18
Completed reviews Genart Last Call review of -03 by Dan Romascanu (diff)
Secdir Last Call review of -03 by Benjamin Kaduk (diff)
Genart Telechat review of -04 by Dan Romascanu (diff)
Assignment Reviewer Benjamin Kaduk
State Completed
Request Last Call review on draft-ietf-tls-ecdhe-psk-aead by Security Area Directorate Assigned
Reviewed revision 03 (document currently at 05)
Result Has nits
Completed 2017-05-18
review-ietf-tls-ecdhe-psk-aead-03-secdir-lc-kaduk-2017-05-18-00
Hi all,

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.

This document is ready with nits.

Essentially, we are filling in a gap in the TLS (< 1.3) ciphersuite
space, thought of as a cross product of key exchange and
cipher+mac/AEAD -- we have some of the combinations (PSK with ECDHE
but no AES-[GC]CM, PSK with AES-GCM but only non-EC DH, etc.) but
not quite this one.

That said, it seems a little silly to only partially fill the gap
(by omitting the AES_256_CCM* cipher suites), even though there is
not currently demand for them.

This document is just assembling pieces that were already specified
elsewhere, so it need not contain much detail itself, which is fine.
That said, I think section 3 should probably state explicitly which
pieces it uses, instead of a vague reference of being "based on RFC
4279".  So, "The ServerKeyExchange and ClientKeyExchange messages
from RFC 5489 for ECDHE_PSK are used, and the premaster secret is
computed in the same manner as for ECDHE_PSK key exchange in RFC
5489."  (I am not sure why RFC 4279 is cited in the current text; it
does not cover ECDHE_PSK.)

The premaster_secret structure so used basically ends up putting the
ECDH output first followed by the static PSK; with the pre-TLS 1.2
PRF, that would give the ECDH shared secret to md5 and the PSK to
sha1, which is perhaps another reason to not use these with pre-1.2
worth mentioning (in addition to the AEAD availability).

The security considerations largely refer to those of the other
documents providing the pieces that are combined together here;
those referenced security considerations sections are more than
adequate here, as this document itself does not really do anything
particularly novel.  That said, if we are going to reiterate the
entropy requirement for PSKs inline, we probably ought to also
reiterate the nonce-reuse considerations for GCM and CCM.  The
relevant constructions help, but there are still ways to mess up and
reuse a nonce when doing crypto in parallel, if I remember the
GCM/TLS document's security considerations correctly.


Some other editorial nits follow.

I see we're already discussing "perfect forward secrecy" vs.
"forward secrecy"; my preference is for just "forward secrecy" (and
I may have been one of the more active voices in causing TLS 1.3 to
switch), but in the grand scheme of things it doesn't really matter.

In section 2, the RFC 7525 reference that "AEAD algorithms [...] are
strongly recommended" is scoped to just (D)TLS, so the text here
should probably also list that qualification.

In section 4, "... make use of the authenticated encryption with
additional data (AEAD) defined in TLS 1.2" seems to make AEAD into
something of a unique object, as opposed to a class of things with
multiple possible instantiations.  I would consider something like
"... (AEAD) concept".

In section 4, "these cipher suites MUST NOT be negotiated in TLS
versions prior to 1.2" should probably clarify that "these" cipher
suites are the new ones specified by this document.

s/Servers,\nwhich/Servers\nthat/

Does the last paragraph of section 5 want a note "RFC Editor please
remove this paragraph"?

Section 6, third paragraph, s/by using short PSK/by using a short
PSK/.  Also, s/by a human and thus/by a human which thus/, maybe?

Last paragraph page 4, comma after "i.e.".


-Ben