Subject: [TLS] PR#625: Change alert requirements

From: Eric Rescorla <[email-masked]>
Date: Mon, 5 Sep 2016 11:02:57 -0700

PR: https://github.com/tlswg/tls13-spec/pull/625

Currently the TLS spec requires implementations to send alerts under various
fatal conditions. However, many stacks actually don't send alerts but
instead
just terminate the connection. Several people have argued that we should
relax
the requirement.

At the September 2015 interim there was consensus to instead encourage
sending alerts and require that if you send an alert, you send a specific
one.
I've finally gotten around to producing a PR that does this (link above).
This
PR:

- Harmonizes all the language around alert sending (though perhaps I missed
  a couple of places)
- Tries to make which alerts to send clearer in the alert descriptions to
avoid
  having to specify individually how to handle every decision.
- Relaxes the requirement as listed above.

Note that these are to some extent orthogonal changes; even if we decide to
continue mandating sending alerts, that should be listed in one location not
scattered around the spec.

I know that there wasn't universal consensus on relaxing the requirement to
send, so I'll await WG discussion and the chairs decision on how to handle
this PR.

-Ekr

---

From: Sean Turner <[email-masked]>
Date: Tue, 6 Sep 2016 17:33:41 -0400

All,

The chairs would like to get some eyes on this PR by this Friday (Sept 9th) so that we can draw it to close.

Thanks,

J&S

> On Sep 05, 2016, at 14:02, Eric Rescorla <[email-masked]> wrote:
> 
> fatal conditions. However, many stacks actually don't send alerts but instead
> just terminate the connection. Several people have argued that we should relax
> the requirement.
> 
> sending alerts and require that if you send an alert, you send a specific one.
> I've finally gotten around to producing a PR that does this (link above). This
> PR:
> 
> - Tries to make which alerts to send clearer in the alert descriptions to avoid
> send, so I'll await WG discussion and the chairs decision on how to handle this PR. 
> 
> -Ekr
> 
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls

---

From: Watson Ladd <[email-masked]>
Date: Tue, 6 Sep 2016 14:59:57 -0700

> The chairs would like to get some eyes on this PR by this Friday (Sept 9th) so that we can draw it to close.

So I'd like to know if there are places this PR weakens the
requirement beyond what common practice is. Why is the unexpected
message alert removed, for instance? Do we always want to permit just
terminating as an option?

-- 
"Man is born free, but everywhere he is in chains".
--Rousseau.

---

From: Hubert Kario <[email-masked]>
Date: Wed, 07 Sep 2016 19:05:31 +0200

On Monday, 5 September 2016 11:02:57 CEST Eric Rescorla wrote:
> fatal conditions. However, many stacks actually don't send alerts

the only popular stack I found that does not seem to send alerts is the 
schannel from Microsoft

F5, FortiOS, OpenSSL, NSS, GnuTLS, Java, mbedTLS, botan, axtls, Go 
implementation of TLS, all send alert messages

> scattered around the spec.

In my opinion, putting the information how to handle errors in processing 
given message near the definition of a message makes it easier for 
implementers to implement and makes it less likely that they'll miss error 
handling.

-- 
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: Eric Rescorla <[email-masked]>
Date: Wed, 7 Sep 2016 10:19:02 -0700

> the only popular stack I found that does not seem to send alerts is the

Well, that's a fairly popular stack.

> implementation of TLS, all send alert messages

My understanding is that this is situation-dependent and that some systems
do not send alerts all the time.

> implementers to implement and makes it less likely that they'll miss error

Fair enough. I think this is a question of editor discretion.

-Ekr

---

From: Hubert Kario <[email-masked]>
Date: Wed, 07 Sep 2016 19:54:38 +0200

On Wednesday, 7 September 2016 10:19:02 CEST Eric Rescorla wrote:
> > > Currently the TLS spec requires implementations to send alerts under
> > 
> > various
> > 

just pointing out that "one does not many make" :)

> do not send alerts all the time.

which points out that some messages (both valid and invalid) may not be 
handled as well as they should

while making a protocol definition that is precise and allows for 
interoperability is the primary objective of this work group, I think we can 
all agree that helping developers actually implement it correctly and with no 
bugs (a la heartbleed) is not something we should completely ignore

In my opinion, precise definition of error handling does that (even if 
implementation decides not to send alerts at all).
-- 
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: Andrei Popov <[email-masked]>
Date: Wed, 7 Sep 2016 18:19:54 +0000

> > the only popular stack I found that does not seem to send alerts is 
> > the schannel from Microsoft

To clarify, schannel does generate alerts per RFC, but the HTTP stack (which actually owns the socket) sees no value in sending them.

Cheers,

Andrei

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 7 Sep 2016 11:20:29 -0700

On Wed, Sep 7, 2016 at 11:19 AM, Andrei Popov <[email-masked]>
wrote:

> To clarify, schannel does generate alerts per RFC, but the HTTP stack
> (which actually owns the socket) sees no value in sending them.
>

My impression is that this sometimes applies to Chrome as well.

-Ekr

---

From: [email-masked] (Martin Rex)
Date: Wed, 7 Sep 2016 20:45:14 +0200 (CEST)

> (which actually owns the socket) sees no value in sending them.

"Pillows don't hit people, people do." ;-)

When operating with a transport-less (i.e. opaque-PDUs-only) API,
it is somewhat unusal to having an API call _fail_ *and* return output
parameters & transport requests at the same time.  Higher layer programmers
(and the exception-style programming) often do not expect having to
deal with both.

I actually notice just now that Microsoft Win32 SSPI DeleteSecurityContext()
(Microsoft's incarnation of GSS-API) does not have a final/context-deletion
token output parameter.

In the original (DEC given to IETF) GSS-API design, the "final token"
was not meant to be emitted by a failing context iterator call along with
a fatal error code, but by a _successful_ call to gss_delete_sec_context().

I'm actually confused--where does SChannel return that TLS alert PDU
(and along with with what kind of API return code)?

Admittedly, for applications on top of GSS-API (rfc2743/rfc2744) it is
quite common to _not_ bother conveying the optional(!) context deletion
token that may be produced by GSS-API's gss_delete_sec_context(), and
the GSS-APIv2 spec acknowledged this being a common app behaviour,
and deprecated the context deletion token.

-Martin

---

From: David Benjamin <[email-masked]>
Date: Wed, 07 Sep 2016 18:46:25 +0000

> My impression is that this sometimes applies to Chrome as well.

Chrome doesn't send close_notify. I'm not sure if error alerts get
swallowed or not. I thought they did, but they do seem to make it through
some layers in the stack. Probably whether they manage to get sent depends
on how long higher layers happen to hold on to objects when an error is
reported.

For Chrome, writing to a socket is asynchronous (we have some
callback-based interface which ultimately uses select/poll/whatever +
non-blocking write/send on POSIX), but tearing down a socket or its owners
is synchronous (it's just C++ delete), so we would attempt to post the
write but then immediately cancel everything if it's just before everything
gets torn down. Trying to wait for that write to actually clear (and then
account for timing it out if it takes forever, etc) would be a ton of
trouble and not actually do much useful. I imagine Andrei's story with the
HTTP stack is similar.

David

---

From: Timothy Jackson <[email-masked]>
Date: Wed, 7 Sep 2016 18:57:43 +0000

We probably want to discuss the potential for side-channels introduced by alerting. I’ve seen at least one case where there were two possible classes of errors at a particular state in the protocol. One caused an alert, the other caused the TLS stack to close the channel. Unfortunately, this difference in behavior could give information to an attacker. The only viable solution was to modify the TLS stack to close the channel in both cases.

If we’re going to continue having alerts, I think we would need to be sure that an implementations following the RFC will not wind up in this situation. I suspect that will be difficult at best.

Tim

From: TLS <[email-masked]> on behalf of David Benjamin <[email-masked]>
Date: Wednesday, September 7, 2016 at 11:46 AM
To: Eric Rescorla <[email-masked]>, Andrei Popov <[email-masked]>
Cc: "[email-masked]" <[email-masked]>
Subject: Re: [TLS] PR#625: Change alert requirements

On Wed, Sep 7, 2016 at 2:21 PM Eric Rescorla <[email-masked]<mailto:[email-masked]>> wrote:

Chrome doesn't send close_notify. I'm not sure if error alerts get swallowed or not. I thought they did, but they do seem to make it through some layers in the stack. Probably whether they manage to get sent depends on how long higher layers happen to hold on to objects when an error is reported.

For Chrome, writing to a socket is asynchronous (we have some callback-based interface which ultimately uses select/poll/whatever + non-blocking write/send on POSIX), but tearing down a socket or its owners is synchronous (it's just C++ delete), so we would attempt to post the write but then immediately cancel everything if it's just before everything gets torn down. Trying to wait for that write to actually clear (and then account for timing it out if it takes forever, etc) would be a ton of trouble and not actually do much useful. I imagine Andrei's story with the HTTP stack is similar.

David

---

From: Ilari Liusvaara <[email-masked]>
Date: Wed, 7 Sep 2016 22:24:47 +0300

On Wed, Sep 07, 2016 at 06:57:43PM +0000, Timothy Jackson wrote:
> We probably want to discuss the potential for side-channels introduced by
> alerting. I’ve seen at least one case where there were two possible
> classes of errors at a particular state in the protocol. One caused an
> alert, the other caused the TLS stack to close the channel.
> Unfortunately, this difference in behavior could give information to an
> attacker. The only viable solution was to modify the TLS stack to close
> the channel in both cases.
> 
> If we’re going to continue having alerts, I think we would need to be
> sure that an implementations following the RFC will not wind up in this
> situation. I suspect that will be difficult at best.

IMO, no-alert (not even tried) closes are only acceptable if:

- Association was never created (bad HelloVerifyRequest or
  HelloRetryRequest in DTLS).
- Association has been lost (transport error).
- The association state is corrupt.

The bad alerts that leak info are:

- Depend on secret values and
- Substantial probability to not trigger alert for first cause(s).

IIRC, TLS 1.0-1.2 blockmode had one of those.

Note that if you have an alert which could leak information, very
probably it is a timing attack vector as well. And there just closing
the connection won't help.

-Ilari

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 7 Sep 2016 19:35:11 +0000

I've been reading this.

I think we should get rid of the "abort" concept.  There's a clean shutdown and there's everything else which is an abrupt or unclean closing of the connection.  The "send alert" and "close connection" concepts are separable and I think we should do that.

I think writing things this way will make it more clear.  And then we can bikeshed over which alerts are MAY MUST SHOULD, knowing all along that ECONNRESET means the other side gave up.

---

From: [email-masked] (Martin Rex)
Date: Wed, 7 Sep 2016 22:42:23 +0200 (CEST)

Salz, Rich wrote:
>
> I've been reading this.
> 
> I think we should get rid of the "abort" concept.  There's a clean
> shutdown and there's everything else which is an abrupt or unclean
> closing of the connection.  The "send alert" and "close connection"
> concepts are separable and I think we should do that.
> 
> I think writing things this way will make it more clear.
> And then we can bikeshed over which alerts are MAY MUST SHOULD,
> knowing all along that ECONNRESET means the other side gave up.

For TLS handshake failures, the presence of Alerts on-the-wire can
significantly facilitate troubleshooting.

For terminating application data flows after successful TLS handshakes
an ECONNRESET may actually be preferable to graceful TLS closure alerts
in certain scenarios (but this means that applications will have to
perform proper application-level end-of-data signaling).

If the backend uses a multi-tier architecture (such as a reverse proxy
in a DMZ), then its easier to notice (and process) ECONNRESET than
a TLS closure alert, and cancel processing of the active request
in the backend.

This becomes even more important when the stupid idea of hiding the
content type is not removed from TLSv1.3/TLSv2.0.  Recognizing
ECONNRESET is trivial.  Peeking and recognizing a pending TLS Alert
is doable in TLS up to v1.2, but this is going to be a royal PITA
in an Alert-ContentType-concealing TLSv1.3/TLSv2.0.

-Martin

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 7 Sep 2016 21:57:42 +0000

Sorry, I left off the semi-serious joking indicator on the second paragraph.

---

From: "Salz, Rich" <[email-masked]>
Date: Thu, 8 Sep 2016 16:11:10 +0000

I think an introductory section on "normal and exceptional flow of control" or such would help.  It could also define consistent terminology to be used in the rest of the document.  To take a stab at it:
	Close -- means cleanly close the connection at some point after the necessary alerts have been sent etc
	Teminate -- to close the connection unexpectedly, such as when an one side detects an error.  In these cases, an alert SHOULD be sent before the TCP connection is closed.  (Do we bind/imply TCP?)  I would use this instead of the term abort.

Yes, most of that text is in lines 3422ff.  Put them up earlier.

Then, throughout the rest of the doc, we use things like "if not consistent, terminate the handshake with illegal_parameter"

I can live with the current PR tho.

---

From: Hubert Kario <[email-masked]>
Date: Thu, 08 Sep 2016 20:08:02 +0200

> PR: https://github.com/tlswg/tls13-spec/pull/625

Finally found some time to take a look at it.

So in general I like the change to "abort the handshake with a <type> alert", 
the "send a fatal <type> alert and close the connection" was a bit clunky. I 
was actually planning to do something like this myself.

Few other items carried over from my proposed PR[1]:

---

I still think we should include generic advice for handling parser errors in 
the parser (Presentation Language/Vectors and Constructed Types) section; 
something like this:

   Peers that receive a encoding of a vector in a message with a length that 
   does not match specification MUST abort the connection with a 
   "decode_error" alert unless more specific section states otherwise.

and this:

   Peers that receive messages that doesn't match the expected constructed 
   type in expected message MUST abort the connection with a "decode_error" 
   alert unless more specific section states otherwise.

---

I think it's a bit confusing to have this sentence:

   Servers MUST select the lower of the highest supported server version and
   the version offered by the client in the ClientHello. In particular,
   servers MUST accept ClientHello messages with versions higher than those
   supported and negotiate the highest mutually supported version.

in the Server Hello section. Everything before the very last "and" is more 
applicable to ClientHello.

--- 

I think that this sentence in Handshake Protocol:

   Sending handshake messages in an unexpected order results in an
   "unexpected_message" fatal error.

should use a MUST and be prescribed to the receiving side, for example:

   Peer that receives handshake messages in unexpected order MUST abort the
   handshake with an "unexpected_message" alert.

---

Description what should client do when the ServerHello.cipher_suite is the one 
it did not advertise in ClientHello.cipher_suites is missing.

So is description of how to handle ServerHello.version being higher than 
ClientHello.max_supported_version.

At least, I don't see it in Server Hello section.

---

missing_extension description in Alert Protocol probably should be extended 
from:

   Sent by endpoints that receive a hello message not containing an extension 
   that is mandatory to send for the offered TLS version.

to:

   Sent by endpoints that receive a hello message not containing an extension 
   that is mandatory to send for the offered TLS version or the offered key 
   exchange mechanisms.

(thinking of handling situations where "key_share" or "pre_shared_key" is 
missing)

---

Hello Extensions section doesn't specify how to handle messages with duplicate 
extensions (e.g. second "key_share" extension added in the second ClientHello 
message in connection)

---

Signature Algorithms doesn't say how to handle RSA-SSA signatures with salt 
length that doesn't match hash size.

---

Key Share doesn't say how to handle server_share that doesn't match 
client_shares.

---

Finished section doesn't describe what to do if the contents don't match the 
expected value.

Should it be illegal_parameter or bad_record_mac is more appropriate?

---

Record Layer doesn't describe what to do if a record with zero-length payload 
and handshake or alert type is received.

---

Record Layer includes

   legacy_record_version : (...) This field is deprecated and MUST be ignored
   for all purposes.

Record Layer Protection does not.

---

1 - https://github.com/tlswg/tls13-spec/pull/621
-- 
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: Hannes Tschofenig <[email-masked]>
Date: Fri, 9 Sep 2016 16:35:49 +0200

Hi Ekr,

I read through the text and I think it is an improvement.

I only had one question that is only slightly related to your edits 
because it came up in the interop testing with the Mint implementation.

"
Servers requiring this extension SHOULD respond to a ClientHello
lacking a "server_name" extension by terminating the connection
with a "missing_extension" alert.
"

I personally would find it more useful to have an alert saying 
"missing_server_name_extension" instead of just returning 
"missing_extension" for a number of different extensions since this 
gives the client no chance to fix the problem without human intervention.

Ciao
Hannes

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 9 Sep 2016 18:13:04 -0500

I'm happy to make all alerts fatal.

I also like the state in the pull requests where some cases require that
if an alert is sent, it is a specific alert, while leaving flexibility
in other cases (and preventing us from having to exhaustively enumerate
all possible causes for alerts).

It would also be nice to standardize on using "terminate" for
post-handshake issues and "abort" for during the handshake (or, really,
any concrete split).  It looks like the intent of the pull request is to
make that distinction, but I wasn't really reading with a keen eye and
checking all cases for it.

-Ben

---

From: Martin Thomson <[email-masked]>
Date: Sat, 10 Sep 2016 20:43:33 +1000

On 10 September 2016 at 00:35, Hannes Tschofenig
<[email-masked]> wrote:
> "missing_extension" for a number of different extensions since this gives
> the client no chance to fix the problem without human intervention.

In this case, this is the only use of missing_extension that would be
due to the extension being optional.  In all other cases,
missing_extension is used for a protocol violation: missing
signature_algorithms, for instance, is always a direct result of
problems that are inherent to the ClientHello.  Thus, a well-behaved
implementation can (currently) infer that this is the reason for
missing_extension.

I wouldn't say that this is a great argument, but it's one that could
be made.  Generally, I've given up on TLS error codes being useful, or
even making them useful; we've been stung in the past by being overly
specific about what went wrong.

---

From: Ilari Liusvaara <[email-masked]>
Date: Sat, 10 Sep 2016 17:53:21 +0300

> specific about what went wrong.

Well, regarding usefulness of TLS error codes, the error codes don't
even come close to useful-for-debugging. This isn't because of
information leaking (which AFAICT can only happen in very few places),
but because error codes just are not expressive enough.

Also, if you have a place where you worry about alerts leaking 
information to attacker, watch out, you might very well also have a
timing attack.

The places of TLS 1.3 that I can quickly name that are sensitive,
and so one should take care to only send one kind of alert for
each:

- Deprotect failures (bad_record_mac)
- MAC (finished) failures (decrypt_error)
- key agreement failure (???, my code uses illegal_parameter[1])
- signature failures (internal_error)

Also, note that with some bad cryptographic algorithms, one can't
even let the other side know operation failed. So even just
resetting the connection leaks too much information.

[1] Logic being: It is likely provoked by peer sending invalid
share -> invalid field value -> illegal_parameter.

---

From: Hubert Kario <[email-masked]>
Date: Thu, 15 Sep 2016 16:39:54 +0200

> all possible causes for alerts).

I have one scenario in which a the difference in alert type sent is actually 
quite crucial for properly testing an implementation.

Say you have a server that implements NPN. In case that extension is 
negotiated, client will send EncryptedExtensions between CCS and Finished, 
something that is a big no-no in standard TLS.

Now, if client misbehaves and sends the EncryptedExtensions despite NPN not 
being negotiated, server should respond with an unexpected_message alert. But 
if a server is expecting some message (by mistake) it won't match what it 
expects and by standard definition it should then send decode_error.

The end result of detailed specification is not only easier testing, but also 
higher confidence in test results.

---

From: Sean Turner <[email-masked]>
Date: Mon, 19 Sep 2016 15:50:29 -0400

Thanks for the discussion.  We’re going to ask ekr to merge this one (obviously dealing with the additional input provided during the discussion).

J&S

> On Sep 06, 2016, at 17:33, Sean Turner <[email-masked]> wrote:
> 
> All,
>

---

From: Eric Rescorla <[email-masked]>
Date: Mon, 19 Sep 2016 17:53:35 -0700

Please post a PR for this and see what the WG thinks.

On Fri, Sep 9, 2016 at 7:35 AM, Hannes Tschofenig <[email-masked]

---

From: Eric Rescorla <[email-masked]>
Date: Mon, 19 Sep 2016 18:12:22 -0700

Resolutions below.

>    alert unless more specific section states otherwise.

I added some text in the syntax section.

> applicable to ClientHello.

I am going to leave this because I anticipate adopting the new version
syntax.

>    handshake with an "unexpected_message" alert.

I changed this.

Description what should client do when the ServerHello.cipher_suite is the
> one

Changed..

> At least, I don't see it in Server Hello section.

See above re: version

> (thinking of handling situations where "key_share" or "pre_shared_key" is

Updated a bit.

> Key Share doesn't say how to handle server_share that doesn't match

I'm comfortable leaving these as-is.

> Should it be illegal_parameter or bad_record_mac is more appropriate?

This is specified in the decrypt_error section.

> and handshake or alert type is received.

I'm comfortable leaving these as-is.

> Record Layer Protection does not.

I don't follow.

---

From: Eric Rescorla <[email-masked]>
Date: Mon, 19 Sep 2016 18:12:31 -0700

Merged

On Mon, Sep 19, 2016 at 12:50 PM, Sean Turner <[email-masked]> wrote:

> Thanks for the discussion.  We’re going to ask ekr to merge this one
> (obviously dealing with the additional input provided during the
> discussion).
>
> J&S
>

---

From: Hubert Kario <[email-masked]>
Date: Tue, 20 Sep 2016 12:39:31 +0200

> Resolutions below.

Thanks! I'll go over the post-merge text again later.
 
> This is specified in the decrypt_error section.

on one hand, this give a side channel to detect mismatch between decryption 
error and finished value compare, on the other hand, the timing difference 
between those will be huge anyway...
 
> I'm comfortable leaving these as-is.

so what would be the expected alert description in such situation?

> > ---
> > 
> > Record Layer includes
> > 
> >    legacy_record_version : (...) This field is deprecated and MUST be
> > 
> > ignored
> > 
> >    for all purposes.
> > 

in Record Layer there's the following text:

    legacy_record_version : This value MUST be set to { 3, 1 } for all
    records. This field is deprecated and MUST be ignored for all purposes.

in Record Layer Protection there's the following text:

    legacy_record_version : The legacy_record_version field is identical to
    TLSPlaintext.legacy_record_version and is always { 3, 1 }. Note that the
    handshake protocol including the ClientHello and ServerHello messages
    authenticates the protocol version, so this value is redundant.

which doesn't say if the version can be ignored completely (skipped while 
parsing) or if it should be verified.

Less error-prone solution would be to have the same behaviour specified for 
encrypted and non-encrypted record layer.

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 20 Sep 2016 06:03:52 -0700

On Tue, Sep 20, 2016 at 3:39 AM, Hubert Kario <[email-masked]> wrote:

> > > Finished section doesn't describe what to do if the contents don't
> match
> > > the
> > > expected value.
> > >

> so what would be the expected alert description in such situation?

Probably bad_record_mac, because it's a deprotection failure.

> parsing) or if it should be verified.

These are different fields.

> encrypted and non-encrypted record layer.

The rationale for this is that we need to allow flexibility for the
plaintext messages
because there are stacks which don't conform already, but not for the
encrypted.

Please feel free to file a PR and start a discussion on the list if you
disagree, but
this is out of scope for this PR.

-Ekr