Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EAP-FAST failing in generation of ClientHello with 1.1.1-pre1 #5359

Closed
jmalinen opened this issue Feb 13, 2018 · 20 comments
Closed

EAP-FAST failing in generation of ClientHello with 1.1.1-pre1 #5359

jmalinen opened this issue Feb 13, 2018 · 20 comments

Comments

@jmalinen
Copy link

It looks like something in OpenSSL 1.1.1-pre1 broke EAP-FAST support. SSL_connect() is now failing on the client side when trying to prepare ClientHello (debug logs from wpa_supplicant):

EAP-FAST: No PAC found - starting provisioning
EAP-FAST: Enabling authenticated provisioning TLS cipher suites
OpenSSL: cipher suites: DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA:RC4-SHA
SSL: (where=0x10 ret=0x1)
SSL: (where=0x1001 ret=0x1)
SSL: SSL_connect:before SSL initialization
OpenSSL: TX ver=0x0 content_type=256 (TLS header info/)
OpenSSL: Message - hexdump(len=5): 15 03 01 00 02
OpenSSL: TX ver=0x304 content_type=21 (alert/)
OpenSSL: Message - hexdump(len=2): 02 50
SSL: (where=0x4008 ret=0x250)
SSL: SSL3 alert: write (local SSL3 detected an error):fatal:internal error
EAP: Status notification: local TLS alert (param=internal error)
SSL: (where=0x1002 ret=0xffffffff)
SSL: SSL_connect:error in error
OpenSSL: openssl_handshake - SSL_connect error:141A90B5:SSL routines:ssl_cipher_list_to_bytes:no ciphers available

This is what this looks like in a successful run (from a build with OpenSSL 1.1.0f):

EAP-FAST: No PAC found - starting provisioning
EAP-FAST: Enabling authenticated provisioning TLS cipher suites
OpenSSL: cipher suites: DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA:RC4-SHA
SSL: (where=0x10 ret=0x1)
SSL: (where=0x1001 ret=0x1)
SSL: SSL_connect:before SSL initialization
OpenSSL: TX ver=0x0 content_type=256 (TLS header info/)
OpenSSL: Message - hexdump(len=5): 16 03 01 00 63
OpenSSL: TX ver=0x303 content_type=22 (handshake/client hello)
OpenSSL: Message - hexdump(len=99): 01 00 00 5f 03 03 0d 86 19 ca 1e 86 c9 9c ee 4e 8e ab 67 4c 13 0d 13 ec 0d 70 78 49 4e 8e c2 76 74 47 85 e1 40 4d 00 00 0a 00 39 00 33 00 35 00 2f 00 ff 01 00 00 2c 00 0d 00 20 00 1e 06 01 06 02 06 03 05 01 05 02 05 03 04 01 04 02 04 03 03 01 03 02 03 03 02 01 02 02 02 03 00 16 00 00 00 17 00 00
SSL: (where=0x1001 ret=0x1)
SSL: SSL_connect:SSLv3/TLS write client hello
SSL: (where=0x1002 ret=0xffffffff)
SSL: SSL_connect:error in SSLv3/TLS write client hello
SSL: SSL_connect - want more data

@kroeckx
Copy link
Member

kroeckx commented Feb 13, 2018

@richsalz
Copy link
Contributor

Have you configured your own list of ciphers? IF so, did you disable TLS 1.3? If not, that's an error.
See this for details: https://www.openssl.org/blog/blog/2018/02/08/tlsv1.3/

@richsalz
Copy link
Contributor

@kroeckx hah ;)

@jmalinen
Copy link
Author

I read an earlier TLSv1.3 blog post, but did not realize that there would be additional requirement to explicitly disable v1.3 in such a case. I do actually disable various TLS versions with older OpenSSL versions, but that was for another issue and that code was apparently conditional on OPENSSL_VERSION_NUMBER, so it did not get included here. I did even go through the CHANGES document before opening this issue, but somehow managed to miss the connection between the TLS v1.3 item and the EAP-FAST needs to change the ciphersuite list.

Adding an explicit SSL_set_options(ssl, SSL_OP_NO_TLSv1_3) does indeed resolve this. I think that it would have been nicer if OpenSSL were to still allow TLS v1.2 to be used in this type of case where all TLS protocols were enabled and the selected cipher suites would work with v1.2, but not v1.3.. Anyway, now that I know what to search for, the first CHANGES file entry does explain this very specific case and implies that 1.1.1 is not automatically backwards compatible with the past on this front.

@kroeckx
Copy link
Member

kroeckx commented Feb 14, 2018

So can we close this issue?

@richsalz
Copy link
Contributor

Yes, or should we use it as a hook to rediscuss the compatibility issue? Right now, if you set your own ciphers and don't set any of the SSL options, your application will break. I think that's pretty wrong.
When phrased this way, I think we should reconsider the policy. @mattcaswell and others?

@mattcaswell
Copy link
Member

So I think we discussed this before and came to the current policy as the least-bad option.

Options I can think of are:

  1. Don't check if we have TLSv1.3 capable ciphers until we actually negotiate a TLSv1.3 connection and then fail

  2. If we have no TLSv1.3 capable ciphers then automatically disable TLSv1.3

  3. Do what we do now: Check early whether we are configured to allow TLSv1.3 but have no TLSv1.3 capable ciphers and bail out if so (regardless of whether TLSv1.3 is actually negotiated)

We actually originally had option 1 (more by accident than design). The problem with this approach is that everything seems to be working just fine until you actually negotiate TLSv1.3 and then you get a failure. To start with at least, successful TLSv1.3 connections may be few and far between - which means you end up getting spurious failures which are difficult to track down what the problem is (this was why we decided to move to option 3).

Option 2 is also possible. The big problem with this approach is that you think you have enabled TLSv1.3 and are getting all of its benefits but in actual fact you will silently never negotiate it. You can imagine tens of thousands of mis-configured clients and servers out there for years to come which believe they are running the latest OpenSSL version and are therefore using the latest TLS version, but are actually using an out-of-date version of the protocol.

@kroeckx
Copy link
Member

kroeckx commented Feb 14, 2018 via email

@richsalz
Copy link
Contributor

There is nothing known to be wrong with TLS 1.2, why force people off it?
There is something pretty bad about a "dot dot 1" release breaking applications.
I probably agreed with the current behavior but I have changed my mind and I prefer option two.

@mattcaswell
Copy link
Member

There is nothing known to be wrong with TLS 1.2, why force people off it?

Not right now...but what about in the future. I think it is a real problem to have people think they are using TLSv1.3 when they silently never will.

I probably agreed with the current behavior but I have changed my mind and I prefer option two.

I think option 2 is the worst out of all three of them. If we change the current behaviour I would prefer to go with option 1.

@richsalz
Copy link
Contributor

How many year until TLS 1.2 is "risky"? The ciphers it uses are basically the same as 1.3. 1.3 adds 0rtt and not much else cryptographically.

Option 1 is the same as option 3, except the breakage happens at connection time, not object creation time.

Option 2 is the only one, if you don't want to break existing users and if you are not worried about TLS 1.2 being broken in the next five years.

@davidben
Copy link
Contributor

There is also option 4, which is to treat TLS 1.3 ciphers as different animals from TLS 1.2 ciphers (like ECDH curves are different animals from ciphers) and either always enable them or, if they must be configurable, make them configurable via a separate mechanism.

This is what BoringSSL switched to doing very early on, based on experience attempting to deploy TLS 1.3 in existing code.

The ciphers it uses are basically the same as 1.3. 1.3 adds 0rtt and not much else cryptographically.

TLS 1.3 adds quite a lot cryptographically. There is more to crypto than just muttering AES-GCM. The TLS 1.2 handshake uses a number of primitives wrong. The ServerKeyExchange signature, for instance, does not cover anywhere near what it should be covering, which is why we needed the server_random anti-downgrade hack in TLS 1.3 to begin with. It's a workaround for a flaw in TLS 1.2.

@richsalz
Copy link
Contributor

Option 4 is probably not feasible for a dot-dot release. Yes, I glossed over the crypto. When do you think TLS 1.2 will be risky enough to be deprecated?

@davidben
Copy link
Contributor

Why is option 4 not feasible for a dot-dot release? It seems the most compatible option. Compatibility dictates that adding new capabilities (TLS 1.3 ciphers) is generally okay, but changing the semantics of existing ones (cipher configs that, to date, never precluded a higher TLS version) is questionable. Our deployment experience suggested this was the most compatible approach.

The root cause is that TLS 1.3 is the first time that ciphers have been removed in a version of TLS, and so by whitelisting TLS versions, you implicitly whitelist future newer TLS versions that your library may add. If you recall way back when the cipher suite negotiation was being revised from weirdly repurposing TLS 1.2 ciphers (PSK in particular) to new bespoke ones, this was a big part of the motivation. By separating them completely, it becomes much easier to consider them orthogonal, which was the cleanest way to deal with this issue.

(Re TLS 1.2: TLS 1.2 will likely be around for some time, for practical purposes, just as other legacy TLS settings are still around. That's how deployments work. The old things take a long time to fall off the conveyor belt. The immediate corollary here is downgrade protection is absolutely critical, which is why we've been so pushy about not adding another client fallback.)

@mattcaswell
Copy link
Member

Hmmm, option 4 does sound quite attractive. What does the API look like for this in Boring?

@davidben
Copy link
Contributor

Oh, we went the hardcoded behavior route and didn't bother making them separately configurable. There's all of three ciphers.

But there's nothing stopping them from being configurable separately (or even some magic token in the existing cipher language to opt out of default 1.3 behavior, I dunno). We just generally don't add this sort of thing until a need comes up and we better understand what people are trying to do. And if a need never comes up, yay less work. :-)

@kaduk
Copy link
Contributor

kaduk commented Feb 15, 2018

Yeah, option 4 with the ciphers always enabled seems like a fine position, on first read.

@mattcaswell
Copy link
Member

Yeah, option 4 with the ciphers always enabled seems like a fine position, on first read.

Currently working on it. Although I do think we're going to need an API to control which ones are enabled. Our test suite relies on that all over the place at the moment.

@briansniffen
Copy link

I expect the FIPS 140 certification to require that you can disable non-FIPS crypto. Maybe that can be done with just a FIPS switch per session? But if others have similar needs, at least the ability to enable and disable ciphers will be required.

mattcaswell added a commit to mattcaswell/openssl that referenced this issue Mar 14, 2018
With the current mechanism, old cipher strings that used to work in 1.1.0,
may inadvertently disable all TLSv1.3 ciphersuites causing connections to
fail. This is confusing for users.

In reality TLSv1.3 are quite different to older ciphers. They are much
simpler and there are only a small number of them so, arguably, they don't
need the same level of control that the older ciphers have.

This change splits the configuration of TLSv1.3 ciphers from older ones.
By default the TLSv1.3 ciphers are on, so you cannot inadvertently disable
them through your existing config.

Fixes openssl#5359
@mattcaswell
Copy link
Member

So we went with option 4 - now in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants