Re: [TLS] secdir review of draft-ietf-tls-ecdhe-psk-aead-03

Benjamin Kaduk <kaduk@mit.edu> Fri, 19 May 2017 16:27 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: tls@ietfa.amsl.com
Delivered-To: tls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 61EFA129526; Fri, 19 May 2017 09:27:36 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.202
X-Spam-Level:
X-Spam-Status: No, score=-4.202 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rUKWvGG-H7b0; Fri, 19 May 2017 09:27:34 -0700 (PDT)
Received: from dmz-mailsec-scanner-2.mit.edu (dmz-mailsec-scanner-2.mit.edu [18.9.25.13]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9E4E7129447; Fri, 19 May 2017 09:27:33 -0700 (PDT)
X-AuditID: 1209190d-fd7ff700000023f1-de-591f1cf45586
Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) (using TLS with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by dmz-mailsec-scanner-2.mit.edu (Symantec Messaging Gateway) with SMTP id B6.96.09201.4FC1F195; Fri, 19 May 2017 12:27:32 -0400 (EDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id v4JGRU5P007560; Fri, 19 May 2017 12:27:31 -0400
Received: from kduck.kaduk.org (24-107-191-124.dhcp.stls.mo.charter.com [24.107.191.124]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id v4JGRPmR013013 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Fri, 19 May 2017 12:27:29 -0400
Date: Fri, 19 May 2017 11:27:25 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: Daniel Migault <daniel.migault@ericsson.com>
Cc: The IESG <iesg@ietf.org>, secdir@ietf.org, draft-ietf-tls-ecdhe-psk-aead.all@ietf.org, tls <tls@ietf.org>, "ietf@ietf.org" <ietf@ietf.org>
Message-ID: <20170519162725.GM39245@kduck.kaduk.org>
References: <20170519043827.GL39245@kduck.kaduk.org> <CADZyTkncMTsTQt6C2S+Z0mw+30uc38bfrTSCOvjWRPn_dJkDLQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CADZyTkncMTsTQt6C2S+Z0mw+30uc38bfrTSCOvjWRPn_dJkDLQ@mail.gmail.com>
User-Agent: Mutt/1.7.1 (2016-10-04)
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrLIsWRmVeSWpSXmKPExsUixCmqrftFRj7SYOEGZYsp0/ewWbxZtonJ YsaficwWzzbOZ7H4sPAhi8Wn812MDmwev75eZfNYsuQnUwBTFJdNSmpOZllqkb5dAlfGnOZP 7AVrHCtONW1gb2BsNeli5OSQEDCRON3/lgnEFhJYzCSxb7EnhL2RUeLXvPQuRi4g+yqTxNpf J1hAEiwCqhLzJh1hBLHZBFQkGrovM4PYIgIGEi8n7GQDsZkFFjBKzPltCGILC7hIrOhbAlbD C7Rs79zdUMuqJY5cmMgGEReUODnzCQtEr5bEjX8vgWo4gGxpieX/OEDCnAKBElM+zAdbKyqg LPH38D2WCYwCs5B0z0LSPQuhewEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJdI73czBK91JTS TYzgUJbk3cH4767XIUYBDkYlHt6EX3KRQqyJZcWVuYcYJTmYlER5HQ8DhfiS8lMqMxKLM+KL SnNSiw8xSnAwK4nwMojKRwrxpiRWVqUW5cOkpDlYlMR5xTUaI4QE0hNLUrNTUwtSi2CyMhwc ShK8G6WBGgWLUtNTK9Iyc0oQ0kwcnCDDeYCGvwOp4S0uSMwtzkyHyJ9iVJQS5z0EkhAASWSU 5sH1glKNRPb+mleM4kCvCPOqAROPEA8wTcF1vwIazAQ0uPmBNMjgkkSElFQDY8GJ5y4qF7be uHe8Qtfk+Pfp166E8p07ey9CUC3v0OfjAr3Xdko82Ph5/2Y534mt9w46znyynOGxl/SUvs2t asd3yc4Q8ddVdUpd0LhJ+viBwxW7dv/zsJmrWLT7emSly/XMcOntIlW23G7ftfau3DSjKK7j 53L2oG0WvSIBP3fWRubOPVQR/FWJpTgj0VCLuag4EQBhJEK1EAMAAA==
Archived-At: <https://mailarchive.ietf.org/arch/msg/tls/ADPTY1HqLlNAyYz44YAwdR3aRVQ>
Subject: Re: [TLS] secdir review of draft-ietf-tls-ecdhe-psk-aead-03
X-BeenThere: tls@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "This is the mailing list for the Transport Layer Security working group of the IETF." <tls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tls>, <mailto:tls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tls/>
List-Post: <mailto:tls@ietf.org>
List-Help: <mailto:tls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tls>, <mailto:tls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 19 May 2017 16:27:36 -0000

On Fri, May 19, 2017 at 11:55:35AM -0400, Daniel Migault wrote:
> Hi Benjamin,
> 
> Thank you for the review. Please find my comments inline and let me know if
> you agree with the proposed text. I believe the only point not addressed
> concerns the addition of CCM-256 which has been remove after discussion
> during the WGLC.
> 
> Thanks you for the review,
> 
> Yours,
> Daniel
> 
> On Fri, May 19, 2017 at 12:38 AM, Benjamin Kaduk <kaduk@mit.edu> wrote:
> 
> > 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.
> >
> > MGLT: TLS_ECDHE_PSK_WITH_AES_256_CCM_8_SHA256 and
> TLS_ECDHE_PSK_WITH_AES_256_CCM_SHA384  were mentioned in the 01 and removed
> after  the WGLC. [0] The issue was whether or not introducing new ciphers
> not supported by TLS1.3. In 01, we though of adding the code point for
> TLS1.2 and then specify that TLS1.3 was only implementing a **subset** of
> the cipher suites proposed.In case of needs these could still be supported
> later by TLS1.3. The consensus seems to not introduce ciphers that would
> not be handled by TLS1.3. If the WG decides otherwise, these could still be
> added.
> 
> [0]
> https://mailarchive.ietf.org/arch/msg/tls/M442CwmUMxrYJR8FjCh3h-a69o4/?qid=6e4713be1d71bae6718c6e6e6c4b8007

That seems like a reasonable position for the WG to take, given how
close TLS 1.3 is to publication.  (It would also be reasonable to
define the full cross-product for TLS 1.2 only and have TLS 1.3 just
use a subset, but I have no real preference.)

> 
> > 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.)
> >
> 
> MGLT: I agree we coudl be more explicit, here are the changes I propose. I
> believe it address your purpose.
> 
> OLD:
> 
> Messages and pre-master secret construction in this
> document are based on [RFC4279 <https://tools.ietf.org/html/rfc4279>].
> 
> NEW:
> 
> Messages and pre-master secret construction in this
> document are defined in [RFC5489]. The ServerKeyExchange
> and ClientKeyExchange messages and premaster secret is
> computed as for  the ECDHE_PSK key exchange.

That basically works for me, though I'd tweak the phrasing slightly
for clarity/grammar:

NEW2:

Messages and pre-master secret construction in this
document are defined in [RFC5489]. The ServerKeyExchange
and ClientKeyExchange messages are used and the premaster secret is
computed as for the ECDHE_PSK key exchange.


> 
> > 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).
> >
> 
> MGLT: I believe the following text address your concern, however I am
> wondering if we are not going beyond the scope of expected considerations.

It perhaps is and perhaps is not worth mentioning, yes.

> In addition, it is worth noting that TLS1.0 <xref target="RFC2246"/> and
> TL1.2 <xref target="RFC4346"/> splits the premaster in two parts. The PRF
> results of mixing the two pseudorandom streams with distinct hash functions

"results of" is an unusual phrasing; "results from" might be more
familiar.

> (MD5 and SHA-1) by exclusive-ORing them together. In the case of ECDHE_PSK
> authentication, the PSK and pre-master are treated by distinct hash
> function with distinct properties. This may introduce vulnerabilities over
> the expected security provided by the constructed pre-master. As such
> TLS1.0 and TLS1.1 SHOULD NOT be used with ECDHE_PSK.

The general tenor of this text addresses my concern, yes, but feel
free to use editorial discretion and not bother putting it in, if
you don't think it's useful.  (BTW, I think we are still TLS1.0 and
TLS1.1 MUST NOT be used with these ciphersuites, so maybe the SHOULD
NOT in the last sentence is not quite what is intended.)

> 
> > 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.
> >
> 
> MGLT: I believe the following text address your comments.
> 
>  <t>GCM or CCM encryption - even of different clear text - re-using a
> nonce with a same key undermines the security of GCM and CCM. As a
> result, GCM and CCM MUST only be used with system guaranteeing nonce
> uniqueness <xref target="RFC5116"/>.</t>

Sure, sounds good.  (Grammar nit: "a system".)

> 
> >
> > Some other editorial nits follow.
> >
> 
> > 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.
> >
> 
> MGLT: I believe the following text address your concern:
> 
> OLD text:
> <t>AEAD algorithms that combine encryption and integrity protection are
> strongly recommended for <xref target="RFC7525" /> and non-AEAD algorithms
> are forbidden to use in TLS 1.3 <xref target="I-D.ietf-tls-tls13"/>.
> 
> NEW text:
> <t>AEAD algorithms that combine encryption and integrity protection are
> strongly recommended for (D)TLS <xref target="RFC7525" /> and non-AEAD
> algorithms
> are forbidden to use in TLS 1.3 <xref target="I-D.ietf-tls-tls13"/>.

Yes, thanks.

> > 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.
> >
> > MGLT: I believe the following text address your comment:
> 
> OLD:
> these cipher suites MUST NOT be negotiated in TLS  versions prior to 1.2.
> 
> NEW:
> the cipher suites defined in this document MUST NOT be negotiated in TLS
> versions prior to 1.2.
> 
>  OLD:
> Clients MUST NOT offer these cipher suites
> 
> NEW:
> Clients MUST NOT offer these cipher suites defined in this document

I think I only intended to comment about the first one, but there is
no harm in changing both.  Thanks!


-Ben