Re: [TLS] Server-side missing_extension MUSTs

Hubert Kario <hkario@redhat.com> Wed, 13 July 2016 16:36 UTC

Return-Path: <hkario@redhat.com>
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 7CB6E12D137 for <tls@ietfa.amsl.com>; Wed, 13 Jul 2016 09:36:40 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -8.209
X-Spam-Level:
X-Spam-Status: No, score=-8.209 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-1.287, SPF_HELO_PASS=-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 zFwFnb_vCGLg for <tls@ietfa.amsl.com>; Wed, 13 Jul 2016 09:36:38 -0700 (PDT)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 568CB126FDC for <tls@ietf.org>; Wed, 13 Jul 2016 09:36:38 -0700 (PDT)
Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6A4613453EF; Wed, 13 Jul 2016 16:30:53 +0000 (UTC)
Received: from pintsize.usersys.redhat.com (ovpn-204-99.brq.redhat.com [10.40.204.99]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6DGUodG010889 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 13 Jul 2016 12:30:52 -0400
From: Hubert Kario <hkario@redhat.com>
To: David Benjamin <davidben@chromium.org>
Date: Wed, 13 Jul 2016 18:30:41 +0200
Message-ID: <1638525.WFeDikGCus@pintsize.usersys.redhat.com>
User-Agent: KMail/4.14.10 (Linux/4.5.7-202.fc23.x86_64; KDE/4.14.20; x86_64; ; )
In-Reply-To: <CAF8qwaBiToiAdH+e-ei2LZiSa6UY4OZ+th_iMS3-SuKfKSB-5w@mail.gmail.com>
References: <CAF8qwaAAw6zA9jRPMQ5MXqHptBtsarhNPcH6KJzzSE-h1XiFDg@mail.gmail.com> <3111426.zLZNlvgK9Q@pintsize.usersys.redhat.com> <CAF8qwaBiToiAdH+e-ei2LZiSa6UY4OZ+th_iMS3-SuKfKSB-5w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart3321673.oJgfTBGyNN"; micalg="pgp-sha512"; protocol="application/pgp-signature"
X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 13 Jul 2016 16:30:53 +0000 (UTC)
Archived-At: <https://mailarchive.ietf.org/arch/msg/tls/7Peg1oUHwsVjwq6t8fOq-7ZAX-E>
Cc: tls@ietf.org
Subject: Re: [TLS] Server-side missing_extension MUSTs
X-BeenThere: tls@ietf.org
X-Mailman-Version: 2.1.17
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: Wed, 13 Jul 2016 16:36:40 -0000

On Wednesday 13 July 2016 15:58:24 David Benjamin wrote:
> On Wed, Jul 13, 2016 at 11:06 AM Hubert Kario <hkario@redhat.com> wrote:
> 
> > On Wednesday 13 July 2016 14:43:58 David Benjamin wrote:
> > > To be clear, I am not at all opposed to useful errors or strict policing
> > of
> > > what the peer sends. That's all great. Indeed the linked test below makes
> > > use of more specific error codes than TLS provides. But I would like such
> > > things to:
> > >
> > > 1. Be useful, either for debugging or because it rejects an invalid
> > > handshake that would otherwise go unnoticed.
> > > 2. Come out naturally out of what the implementation would do anyway, to
> > > keep complexity down.
> > >
> > > I'm also quite okay with budging on #2 where #1 is strong. Complexity is
> > > the currency we pay for adding things. But I do not think this particular
> > > error code passes #1 (the special-case is not useful and such a peer
> > would
> > > not be able to handshake against correct implementations anyway), so I'm
> > > not willing to sacrifice #2 for it.
> >
> > Then I fail to see how missing_extension does not flow out of correct
> > implementation (and from general RFC recommendations).
> >
> > You need to parse the Client Hello, and then sanity check it (length of
> > session ID, validity of ciphers, lack of duplicate extensions, etc.).
> > Making sure that mandatory extensions are in place in the same code
> > seems to me like a obvious place to do it.
> >
> 
> The extension is not mandatory. Whether it is mandatory or not depends on
> the cipher suite selected. The cipher suite selected depends on the
> contents (or lack of) the extension. This makes checking things somewhat
> circular.

not really, if there is a cipher suite that uses ECDHE key exchange,
the extension must be there

so sure, the check is not as simple as 

if client_hello.client_version >= (3, 4):
   assert len(x for x in client_hello.extesnions if x.ext_type == supported_groups) == 1

but rather something like:

if client_hello.client_version >= (3, 4) and any(x in client_hello.cipher_suites for x in Ciphers.ecdheKex):
   assert len([x for x in client_hello.extensions if x.ext_type == ExtensionType.supported_groups]) == 1

but my point is that it doesn't require any information not available
just after the message was parsed
 
> If it were simply mandatory, by all means, mandate a missing_extension
> alert. I still do not think it'd be very useful (I'm more interested in
> non-syntax-error cases), but I have no objections.

it is "simply mandatory" if client hello includes ciphers that use ECDHE

> > In other words, you shouldn't delay checking if a particular extension
> > is present until the time you want to use it. If there is any chance
> > (or possible configuration) in which you would end up using it, you
> > should complain that the extension is missing or it is malformed.
> >
> > This way when you start to depend on it, things won't start breaking
> > suddenly and you end up writing Yet Another Workaround for Broken
> > Implementations™
> >
> 
> I still do not see how returning handshake_failure (or whatever the "no
> common cipher" error is) for this case instead of missing_extension will
> cause broken implementations to come up.

there are implementations that send incorrect version number to negotiate
tls version they want to negotiate

i.e. they send (3, 2) when they want to negotiate TLS 1.0

in case of TLS 1.3, returning missing_extension makes it clear that
the client hello sent by such an implementation was incomplete for TLS 1.3
and it wouldn't work with *any* correct TLS 1.3 implementation

unlike a handshake_failure which generally means that the server configuration
is incompatible with the client proposed parameters

> > > On Wed, Jul 13, 2016 at 10:02 AM David Benjamin <davidben@chromium.org>
> > > wrote:
> > >
> > > > On Wed, Jul 13, 2016 at 3:31 AM Dave Garrett <davemgarrett@gmail.com>
> > > > wrote:
> > > >
> > > >> To be clear though, completely mandatory extensions, at least as we're
> > > >> doing them here, are a new thing. TLS 1.2 relied on separate messages
> > for
> > > >> stuff, but we're front-loading everything into the ClientHello to get
> > > >> reliable 1RTT (with the exception of HRR).
> > > >
> > > >
> > > > On Wed, Jul 13, 2016 at 8:12 AM Hubert Kario <hkario@redhat.com>
> > wrote:
> > > >
> > > >> On Wednesday 13 July 2016 05:23:53 David Benjamin wrote:
> > > >> > I don't believe an implementation which fails to send
> > supported_groups,
> > > >> > etc., in 1.3 would ever leave a developer's workstation. It would
> > not
> > > >> > interoperate with anything.
> > > >>
> > > >> it would interoperate with itself, and for some deployments that's
> > enough
> > > >> of a passing grade... (Even if you do interoperatbility testing you
> > > >> do not check all possible permutations of features and settings)
> > > >>
> > > >> I wholeheartedly agree with Dave here, error definitions should be
> > strict
> > > >> (both on the when and what to do). One, because it allows to better
> > > >> diagnose (in general, maybe not in this specific situation) problems.
> > > >> Two, because you can write a strict negative test case that actually
> > > >> checks for it.
> > > >
> > > >
> > > >
> > > >
> > https://boringssl.googlesource.com/boringssl/+/1c256544dda26e4042c1af082580a1b87c9a690f/ssl/test/runner/runner.go#5646
> > > > Also something I have plenty of experience with. We're obsessive about
> > > > adding this kind of test in BoringSSL[0]. :-)
> > > >
> > > > One doesn't need this alert to write a test for curve negotiation. Just
> > > > test that the handshake hits the usual codepath for there not being a
> > > > common cipher.
> > > >
> > > > This semi-mandatory extension isn't new in TLS 1.3, depending on your
> > 1.2
> > > > behavior. BoringSSL will already refuse to select an ECDHE cipher if
> > > > supported_curves is missing. 1.2 does not require this, but we opted
> > to do
> > > > it for simplicity[1].
> > > >
> > > > David
> > > >
> > > > [0] If anyone wants to try, I'm sure there is an
> > implementation-agnostic
> > > > TLS protocol test suite one could extract out of that. It would take
> > some
> > > > wrangling as we cheat and condition on BoringSSL error codes, but the
> > nice
> > > > thing is I've already built up a large battery of tests here.
> > > >
> > > > [1] Previously we always picking our most preferred curve if the
> > extension
> > > > was missing, like OpenSSL. But now our most preferred curve is X25519,
> > not
> > > > P-256. It did not seem worth kludging that when we could just decline
> > the
> > > > cipher. No one's noticed, so I would say that worked.
> > > >
> >
> > --
> > Regards,
> > Hubert Kario
> > Senior Quality Engineer, QE BaseOS Security team
> > Web: www.cz.redhat.com
> > Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic

-- 
Regards,
Hubert Kario
Senior Quality Engineer, QE BaseOS Security team
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic