Subject: [TLS] new error alerts?

From: Dave Garrett <[email-masked]>
Date: Wed, 22 Jul 2015 21:39:45 -0400

Hubert Kairo found quite a few more spots in need of explicit error designations, which have been amended into PR #201.
https://github.com/tlswg/tls13-spec/pull/201

I just noticed one error in the current draft text that was wrong and added a fix for that as well. The Server Hello section said that lack of acceptable group would result in an "insufficient_security" error, which is incorrect. That error is clearly defined to be for lack of acceptable cipher suite. The Negotiated Groups section says lack of acceptable group is a “handshake_failure” error. I changed the text to state the error for suites, as the other is already noted elsewhere. (this change is now in PR #201) This brings up a problem, however: there is no distinct error for lack of group support. The “handshake_failure” is a bit of a catchall, so there's no way for a client to really know what's wrong if this happens. This is also why I don't want to change the definition of the "insufficient_security" error. Clients rely on these being relatively precise in order to show error messages that are hopefully meaningful enough to get them fixed. As such, I'd like to propose adding a new error just for this and renaming the old one to focus precisely on its long defined meaning. While we're at it, a failure of client authentication doesn't have its own error alert code either.

  enum {
       handshake_failure(40),
       unsupported_cipher_suites(71),  /* formerly insufficient_security */
       unsupported_dh_groups(72),  /* new */
       client_authentication_failure(73),  /* new */
       (255)
   } AlertDescription;

Pretty straightforward. Are there any other errors that can't be clearly identified by the returned code? Debugging shouldn't be guesswork. ;)

Dave

---

From: Aaron Zauner <[email-masked]>
Date: Thu, 23 Jul 2015 12:49:06 +0200

Hi Dave,

>        client_authentication_failure(73),  /* new */

I mean I kinda agree that 'insufficent security' is a misleading name,
but as it has been used for decades in TLS I'm a bit hesitant if it's a
good idea to change the name now.

Aaron

---

From: Dave Garrett <[email-masked]>
Date: Thu, 23 Jul 2015 10:52:46 -0400

> good idea to change the name now.

If that's really an issue, then it could just be insufficient_security, insufficient_dh_security, & client_authentication_failure. The name isn't as important as not producing errors without clear meanings.

Dave

---

From: Jeffrey Walton <[email-masked]>
Date: Thu, 23 Jul 2015 11:17:07 -0400

> Pretty straightforward. Are there any other errors that can't be clearly identified by the returned code? Debugging shouldn't be guesswork. ;)
Alert 40 shows up frequently in my debugging experiences. A few things
can cause it. It would be nice to see that one broken out.

Jeff

---

From: Dave Garrett <[email-masked]>
Date: Thu, 23 Jul 2015 14:26:20 -0400

> good idea to change the name now.

Alternate bikesheddy response: what about renaming it to "insufficient_cipher_security" and adding a new "insufficient_dh_security"?

That keeps the legacy naming, but modifies it to include actual specific meaning, rather than a total replacement.

Dave

---

From: Aaron Zauner <[email-masked]>
Date: Thu, 23 Jul 2015 21:31:06 +0200

> That keeps the legacy naming, but modifies it to include actual specific meaning, rather than a total replacement.

Fine with that. Now that I think about it again; I'm also fine with the
original proposal. The thing is 'insufficient security' has a nicer ring
to it than 'unsupported XYZ'.

Aaron

---

From: Dave Garrett <[email-masked]>
Date: Thu, 23 Jul 2015 16:01:12 -0400

> to it than 'unsupported XYZ'.

It's wrong, though. If a server rejects a client connection because the server only supports RC4 and the client doesn't, the correct error for the server to return is "insufficient_security". If you invert the meaning, I guess the server has insufficient security, but it's not the same.

If we're ok with a complete change, then I'll just go with the "unsupported_X" format as there's already an "unsupported_certificate" and "unsupported_extension".

I'll stick a commit for this into my ever growing PR #201 in a bit.

Dave

---

From: Aaron Zauner <[email-masked]>
Date: Thu, 23 Jul 2015 22:18:45 +0200

Dave Garrett wrote:>
> It's wrong, though. If a server rejects a client connection because the server only supports RC4 and the client doesn't, the correct error for the server to return is "insufficient_security". If yo…

Well that was kinda what I was getting at, yea :)

> I'll stick a commit for this into my ever growing PR #201 in a bit.

Fine with me - didn't want to bikeshed here.

Aaron

---

From: Eric Rescorla <[email-masked]>
Date: Fri, 24 Jul 2015 01:31:42 +0200

It's not clear to me that there is consensus that more granular error
codes are a good idea. I'll defer to the chairs on the process question.

-Ekr

On Thu, Jul 23, 2015 at 3:39 AM, Dave Garrett <[email-masked]>
wrote:

> Hubert Kairo found quite a few more spots in need of explicit error
> designations, which have been amended into PR #201.
> I just noticed one error in the current draft text that was wrong and
> added a fix for that as well. The Server Hello section said that lack of
> acceptable group would result in an "insufficient_security" error, which is
> incorrect. That error is clearly defined to be for lack of acceptable
> cipher suite. The Negotiated Groups section says lack of acceptable group
> is a “handshake_failure” error. I changed the text to state the error for
> suites, as the other is already noted elsewhere. (this change is now in PR
> #201) This brings up a problem, however: there is no distinct error for
> lack of group support. The “handshake_failure” is a bit of a catchall, so
> there's no way for a client to really know what's wrong if this happens.
> This is also why I don't want to change the definition of the
> "insufficient_security" error. Clients rely on these being relatively
> precise in order to show error messages that are hopefully meaningful
> enough to get them fixed. As such, I'd like to propose adding a new error
> just for this and renaming the old one to focus precisely on its long
> defined meaning. While we're at it, a failure of client authentication
> doesn't have its own error alert code either.
>
>   enum {
>        handshake_failure(40),
> Pretty straightforward. Are there any other errors that can't be clearly
> identified by the returned code? Debugging shouldn't be guesswork. ;)
>
>
> Dave
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>

---

From: Andrei Popov <[email-masked]>
Date: Fri, 24 Jul 2015 02:52:59 +0000

Rather than renaming and otherwise modifying the meaning of the existing alerts, would it be better to define new, more granular alerts in 1.3? We can’t ascribe new meanings to alerts generated by the code we’ve shipped in the past.

From: TLS [mailto:[email-masked]] On Behalf Of Eric Rescorla
Sent: Friday, July 24, 2015 1:32 AM
To: Dave Garrett
Cc: [email-masked]
Subject: Re: [TLS] new error alerts?

-Ekr

On Thu, Jul 23, 2015 at 3:39 AM, Dave Garrett <[email-masked]<mailto:[email-masked]>> wrote:
Hubert Kairo found quite a few more spots in need of explicit error designations, which have been amended into PR #201.
https://github.com/tlswg/tls13-spec/pull/201

  enum {
       handshake_failure(40),

Dave
_______________________________________________
TLS mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/tls

---

From: Dave Garrett <[email-masked]>
Date: Fri, 24 Jul 2015 01:09:25 -0400

> Rather than renaming and otherwise modifying the meaning of the existing alerts, would it be better to define new, more granular alerts in 1.3? We can’t ascribe new meanings to alerts generated by…

I'm not proposing changing the meaning of existing alerts. At most, the Negotiated FF-DH draft would need to be updated/fixed.

I'm proposing renaming "insufficient_security" to "unsupported_cipher_suites", which is explicitly what it's been for since TLS 1.0. There isn't a specific error defined for lack of a supported group yet. RFC 4492 just says "fatal handshake failure alert". The Negotiated FF-DH draft has "insufficient_security" for unsupported group. _That_ does change the meaning, as previously it was explicitly defined for cipher issues only. What I want is to add a new "unsupported_groups" alert to use instead. (both here and there) The "client_authentication_failure" alert suggestion is to pull that out of the "handshake_failure" catchall.

I just want to clarify the existing alert, not reuse it for a related but distinctly different alert, and not lump stuff into a catchall that we can't debug. ;)

Dave

---

From: Andrei Popov <[email-masked]>
Date: Fri, 24 Jul 2015 05:50:31 +0000

> I'm proposing renaming "insufficient_security" to "unsupported_cipher_suites", which is explicitly what it's been for since TLS 1.0.

Not quite. Insufficient_security alert is defined as follows:
" Returned instead of handshake_failure when a negotiation has
   failed specifically because the server requires ciphers more
   secure than those supported by the client.  This message is always
   fatal."

This is a very narrow and specific definition. The server says "I know all the cipher suites the client advertises, and consider them too weak". By contrast, unsupported_cipher_suites means something like "I don't have a cipher suite in common with the client". The latter can happen when the client's cipher suites are more secure than the server's.

> What I want is to add a new "unsupported_groups" alert to use instead. (both here and there) The "client_authentication_failure" alert suggestion is to pull that out of the "handshake_failure" catchall.

I have absolutely no problem with the new alerts; my concern is about redefining existing alerts (and, for that matter, redefining existing cipher suites:)).

Cheers,

Andrei

-----Original Message-----
From: Dave Garrett [mailto:[email-masked]] 
Sent: Friday, July 24, 2015 7:09 AM
To: Andrei Popov; Eric Rescorla
Cc: [email-masked]
Subject: Re: [TLS] new error alerts?

Dave

---

From: Dave Garrett <[email-masked]>
Date: Fri, 24 Jul 2015 12:15:45 -0400

> This is a very narrow and specific definition. The server says "I know all the cipher suites the client advertises, and consider them too weak". By contrast, unsupported_cipher_suites means somethi…

Then if we wish to keep this as narrow as written, we can just have a separate one for unsupported with no judgment on strength:

insufficient_security(71),  // unchanged
unsupported_cipher_suites(72),  // new
unsupported_groups(73),  // new
client_authentication_failure(74),  // new

e.g. RC4 gets insufficient_security & Camellia gets unsupported_cipher_suites

Sounds good to me, if we prefer this.

Dave

---

From: Andrei Popov <[email-masked]>
Date: Fri, 24 Jul 2015 23:26:18 +0000

Yes, this sounds good to me too.

Cheers,

Andrei

-----Original Message-----
From: Dave Garrett [mailto:[email-masked]] 
Sent: Friday, July 24, 2015 6:16 PM
To: Andrei Popov
Cc: Eric Rescorla; [email-masked]
Subject: Re: [TLS] new error alerts?

insufficient_security(71),  // unchanged unsupported_cipher_suites(72),  // new unsupported_groups(73),  // new client_authentication_failure(74),  // new

Dave

---

From: Aaron Zauner <[email-masked]>
Date: Sat, 25 Jul 2015 05:17:20 +0200

> Yes, this sounds good to me too.

+1.

Aaron