Subject: [TLS] Server-side missing_extension MUSTs

From: David Benjamin <[email-masked]>
Date: Wed, 13 Jul 2016 01:58:46 +0000

Hey folks,

I would like to remove the missing_extension MUSTs on the server side. Full
justification in the PR.
https://github.com/tlswg/tls13-spec/pull/544

On the client, it is perfectly feasible to mandate a particular alert
value. The check is very straight-forward. On the server, however, this is
a mistake. Servers do not necessarily have full information if not all
advertised ciphers are known, and a natural implementation of the
negotiation algorithm will not output this case. Even without this clause,
the handshake is already required to fail, so there is no risk of invalid
clients being deployed.

Adding more complexity to an already hairy negotiation algorithm (the
pseudocode I mentioned is incomplete) just to diagnose what is an invalid
ClientHello anyway is not worth it. It buys too little for the complexity
cost.

David

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 12 Jul 2016 19:51:22 -0700

I generally agree with David here.

-Ekr

P.S. Back in Seattle, we had rough consensus to change the alert
requirements [0] so that
you didn't have to send alerts, but if you sent an alert, you had to send
alert X. That's been
on the TODO list for a while but expect a PR soon.

[0] https://github.com/tlswg/tls13-spec/issues/254

On Tue, Jul 12, 2016 at 6:58 PM, David Benjamin <[email-masked]>
wrote:

> Hey folks,
>
> I would like to remove the missing_extension MUSTs on the server side.
> Full justification in the PR.
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>
>

---

From: Dave Garrett <[email-masked]>
Date: Tue, 12 Jul 2016 23:10:17 -0400

> ClientHello anyway is not worth it. It buys too little for the complexity

Reporting errors sufficiently is not an inconsequential issue. TLS has many interop issues with servers which just stare blankly back when something goes wrong, or fail in ways which do not indicate why. This general topic has come up as a problem in discussions here before. We need to specify how to fail appropriately such that when it happens, it can actually be diagnosed and fixed easily. The alternative is people sticking with the status quo forever. The only way this adds complexity is if you assume you're only writing the client and server for themselves, not expecting to interop with unknown peers.

First of all, as to not having "full information", I don't think anyone expects a server to be expected to pick the exact error alert if it can't even parse the cipher suites in question. When a server just skips over an unknown suite, if a missing_extension was technically correct for a server which could've understood it, then it doesn't really matter; the lack of that extension isn't what's causing the failure, as it never gets to that point.

To respond to your pseudocode in the PR, I'd say that explaining the expectation is simpler with a try/catch model: just assume that the required extensions are present, do whatever you do, and throw a missing_extension exception to trigger said fatal alert if at any point that fails to be true. (if you're using a language that does not have exceptions, you can of course do the same thing, just differently; this model just makes stating things simple)

The current wording in the draft has "and no alternative cipher suite is available" added in there, as selecting another suite that doesn't care about such a fault was agreed to be passable because requiring further validation after an acceptable suite+variables was already found was agreed to be excessive. However, we do not mean to imply that you should go _looking_ for an alternative if you're evaluating a suite and the required extension for it is missing. I would generally advise against adding workarounds for implementation flaws you've yet to actually encounter, as all you do is make it harder to detect and fix them when first created.

Dave

---

From: Dave Garrett <[email-masked]>
Date: Tue, 12 Jul 2016 23:22:08 -0400

On Tuesday, July 12, 2016 10:51:22 pm Eric Rescorla wrote:
> P.S. Back in Seattle, we had rough consensus to change the alert requirements [0] so that
> you didn't have to send alerts, but if you sent an alert, you had to send alert X. That's been

I am aware that discussion ended up on accepting sloppy error reporting as acceptable. Changing the disputed missing_extension alerts to SHOULDs would be an acceptable compromise if that decision is still actually going through.

Dave

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 Jul 2016 14:18:28 +1000

> I am aware that discussion ended up on accepting sloppy error reporting as acceptable. Changing the disputed missing_extension alerts to SHOULDs would be an acceptable compromise if that decision i…

"sloppy" is a bit of a mischaracterization.  There are good reasons
for this decision.

It allows implementations to be structured differently.  In
particular, if there are two relevant error conditions, different
implementations could discover them in different orders.
MUST-strength requirements on alerting for one of those conditions
means that you have to order your checking in a particular way;
MUST-strength requirements on multiple concurrent conditions are
basically impossible to meet.

It also allows an implementation to decide that abrupt cessation of
communications is preferable to generating a report.  That can be
important for DoS mitigation.

In the end, we concluded that you shouldn't lie about error
conditions; if you determine that there is a missing extension and you
decide to send an alert, then it has to be the missing_extension
alert.

David's PR goes to the core of the first of my reasons here.  The ways
in which servers decide on the things like version and cipher suite
when presented with a ClientHello is in practice hugely different
between implementations.  That's healthy, but it does mean that we
need flexibility.

---

From: David Benjamin <[email-masked]>
Date: Wed, 13 Jul 2016 05:23:53 +0000

I'm also curious what cases were you envisioning missing_extension to be
useful. I spend a lot of my time triaging TLS-related bugs in Chrome, so
I'm certainly not blind to TLS handshake errors being difficult to
diagnose, especially on the client. But I don't believe such an alert would
ever have helped me. Perhaps I am missing something?

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. Such a client is completely failing to
implement the protocol, and in a way that nothing would work. Difficulties
usually arise not here, but with more complex cases. Where we have interop
against some peers, but not others. Where all protocol messages are legal
but, due to different configurations on either end, no set of parameters
could be found. Where the connection would have succeeded but for, say, a
recent RC4 removal.

If we wish to address the diagnosis problem, we should think about those,
and also how to simplify negotiation so the kinds of errors are less
complex and reporting them is simpler, rather than get distracted by
trivialities.

David

On Wed, Jul 13, 2016 at 12:18 AM Martin Thomson <[email-masked]>
wrote:

> > I am aware that discussion ended up on accepting sloppy error reporting
> as acceptable. Changing the disputed missing_extension alerts to SHOULDs
> would be an acceptable compromise if that decision is still actually going
> through.
>

---

From: Dave Garrett <[email-masked]>
Date: Wed, 13 Jul 2016 03:31:52 -0400

> ever have helped me. Perhaps I am missing something?

I've triaged TLS-related bugs in Firefox (to be fair, not as much as of late), which largely made me come to the conclusion that stricter and more precise error handling is sorely needed. I make no claim that this particular error in this particular case is the focal point on which anything practical I can come up with at the moment hinges, however I have seen too much nonsense to not advocate for rigorous preventive action. When we leave holes and generic responses in specs, the incomprehensible litany of implementations inevitably results in interactions that we are not capable of predicting.

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).

> implement the protocol, and in a way that nothing would work.

We have to assume that some small minority of implementors are not acting in good faith to maintain ideal security, but rather are creating something just functional enough for their specific purpose (possibly a larger minority with IoT). If someone kludges an ad hoc client/server pair that skips the groups extension to go straight to key share, they could get it to work fine. (they're only separate because we are, ourselves, kludging in new features into an old extension, whilst maintaining backwards compatibility) If someone else then has to get this to work with a normal server, they're going to have to let this slide. It is a hell of a lot easier (from a psychological & political perspective) to just kludge in a special case instead of being forced to explicitly cancel out error checks/reporting that come directly from the spec. The more this stuff is left unspecified, the more risk we get of kludge-related breakage/exploits.

As a rule, I never assume something is impossible; you can always be surprised by the interactions of several kludges and a few lazy, sloppy, stupid, or malicious people. This is a 20 year old mess; paranoia is required.

> complex and reporting them is simpler, rather than get distracted by

I agree we should simplify negotiation, wholeheartedly. That is most certainly a better way to do things. I preferred we just require groups and key share for all TLS 1.3+ hellos, always, without exception. Much simpler, but pure-PSK (which I'd prefer was not in the spec) is the special case that complicates things here, and merging the two systems into one simpler one was rejected. (one always mandatory dh/psk key specification extension that is always used requires far less thought to sanity check) Just because a better system isn't used, doesn't mean I won't advocate for stop-loss on what we do go with, however.

To restate what I said in my other reply, downgrading the relevant MUSTs to SHOULDs would be something I'd be ok with. Servers MUST give an appropriate error when aborting, and SHOULD give the most relevant one they can provide at that time.

I do not claim that this here specifically is of top-tier importance, but I will generally resist all attempts to water-down constraints we had previously maintained consensus for.

Dave

---

From: Hubert Kario <[email-masked]>
Date: Wed, 13 Jul 2016 14:12:39 +0200

On Wednesday 13 July 2016 05:23:53 David Benjamin wrote:
> 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.

-- 
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

---

From: David Benjamin <[email-masked]>
Date: Wed, 13 Jul 2016 14:02:50 +0000

On Wed, Jul 13, 2016 at 3:31 AM Dave Garrett <[email-masked]> 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).

> Two, because you can write a strict negative test case that actually

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.

---

From: David Benjamin <[email-masked]>
Date: Wed, 13 Jul 2016 14:43:58 +0000

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.

---

From: Hubert Kario <[email-masked]>
Date: Wed, 13 Jul 2016 17:06:26 +0200

> 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.

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™

---

From: David Benjamin <[email-masked]>
Date: Wed, 13 Jul 2016 15:58:24 +0000

> 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.

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.

> suddenly and you end up writing Yet Another Workaround for Broken

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.

David

---

From: Hubert Kario <[email-masked]>
Date: Wed, 13 Jul 2016 18:30:41 +0200

> contents (or lack of) the extension. This makes checking things somewhat

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
 
> non-syntax-error cases), but I have no objections.

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

> 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

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 13 Jul 2016 10:01:13 -0700

Reinforcing what David says here:
It's natural to pick the cipher suite first and then look for the key_share
extension, so if, for instance, you pick a PSK-only cipher suite, then you
wouldn't look for the key_share.

Obviously, you could add a check that said that if an EC cipher suite was
advertised, then you had to look for key shares even if you picked one, but
it's not a check you otherwise need.

-Ekr

---

From: Dave Garrett <[email-masked]>
Date: Wed, 13 Jul 2016 13:28:18 -0400

> wouldn't look for the key_share.

Agreed. That's why I'm ok with the current "no alternative cipher suite is available" qualification. If the extension never comes up, then not giving a specific error for it is allowed.

On Wednesday, July 13, 2016 10:43:58 am David Benjamin wrote:
> what the peer sends. 
[...]
> Complexity is the currency we pay for adding things.

I very much agree. Our debate hinges on risk assessment, which gets admittedly hard when talking about unknown future implementations. ;)

Essentially, the design philosophy I and Hubert are advocating involves mandatory validation of inputs by all implementations such that we focus on avoiding divergence from what we all agree to in the spec, rather than always try and use our imagination to enumerate each individual screw up that could be made.

Dave

---

From: Martin Thomson <[email-masked]>
Date: Thu, 14 Jul 2016 09:17:06 +1000

> it's not a check you otherwise need.

Though you would miss an EC cipher suite that you didn't know about.
And as far as the client is concerned, any cipher suite that the
server didn't pick is potentially one that it didn't know about.

---

From: Hubert Kario <[email-masked]>
Date: Thu, 14 Jul 2016 13:37:08 +0200

> server didn't pick is potentially one that it didn't know about.

yes, but it's also a ciphersuite no other client will negotiate with that server

and that's why we don't say that server should complain that an extension
it does know about was sent despite there are no EC ciphers from its point of
view

you, as server, act on what you know and understand, and you check it
fastidiously, everything else you MUST completely ignore

ie. if you know about supported_groups extension you MUST check that the length
of extension matches exactly the length of array (minus the 2 bytes for length),
and that length is an even number but you MUST ignore any identifiers you
don't know about if they appear in the array

so server should check if, and only if, there are EC ciphers it knows about* in
the client hello, then it should check for the presence of the extension
and abort if it is missing

 * either ones it can negotiate now, or in general, but that's unimportant
   (I'd say the latter solution would be "cleaner")
-- 
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