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

post-handshake authentication implicit enablement breaks existing applications #6933

Closed
nmav opened this issue Aug 13, 2018 · 20 comments
Closed
Milestone

Comments

@nmav
Copy link
Contributor

nmav commented Aug 13, 2018

With openssl 1.1.1pre8 the post-handshake authentication is explicitly enabled by openssl when
certificate callbacks are set according to SSL_CTX_set_verify manpage. That's an unexpected behavior for existing TLS1.2 applications (in this particular case it breaks python) which fail because:

  • server-side assumes that SSL_CTX contains information about client certs directly after the handshake and before any application data is exchanged.
  • client applications that only know to handle client cert auth during the handshake, but not when they are sending/receiving application data.
  • client applications that assume that OpenSSL asks for client cert password during the handshake.

A backwards compatible alternative (for tls1.2 apps) would be for applications to explicitly enable
post-handshake authentication via flag in SSL_CTX. That way applications written for tls1.2 will work as intended under tls1.3, while applications which can take advantage of post-handshake authentication will still do, but after explicitly enabling it.

@mattcaswell mattcaswell added this to the 1.1.1 milestone Aug 13, 2018
@tmshort
Copy link
Contributor

tmshort commented Aug 13, 2018

I could see the SSL_VERIFY_POST_HANDSHAKE flag being repurposed for the client-side, but I'm thinking that might be a bad idea (unless I can be convinced otherwise).

@mattcaswell
Copy link
Member

I'm working on a patch that renames SSL_force_post_handshake_auth() to SSL_set_post_handshake_auth(), and it must be called if you want to do PHA, whether or not you have supplied a cert.

mattcaswell added a commit to mattcaswell/openssl that referenced this issue Aug 13, 2018
Having post handshake auth automatically switched on breaks some
applications written for TLSv1.2. This changes things so that an explicit
function call is required for a client to indicate support for
post-handshake auth.

Fixes openssl#6933.
@mattcaswell
Copy link
Member

@nmav - will #6938 fix it?

@tiran
Copy link
Contributor

tiran commented Aug 14, 2018

@mattcaswell @nmav I verified Python's ssl module with PR #6938. PHA extension is no longer send and TLS client cert is available on the remote side.

@nmav
Copy link
Contributor Author

nmav commented Aug 17, 2018

I realized I replied on the PR instead. In case you were waiting for my reply, it looks good to me, and @tiran above verified that python's test suite works well with it!

@notroj
Copy link

notroj commented Sep 10, 2018

Do I understand this change to mean that all clients which previously (automatically) allowed post-handshake client cert exchange will now have to explicitly enable that support?

@mattcaswell
Copy link
Member

Almost right. Previously all clients that had a certificate configured at the time of sending the ClientHello, automatically configured post-handshake certificate support. Now they have to explicitly enable that support.

@notroj
Copy link

notroj commented Sep 10, 2018

So even if SSL_CTX_set_client_cert_cb() is used on the client side - which kind of implicitly signals support for PHA, "I can provide a client cert on demand" - you don't get PHA?

@mattcaswell
Copy link
Member

Correct. It doesn't really signal support for PHA - that function has existed for a very long time: well before TLSv1.3. So having PHA implicitly enabled may result it in being called at unexpected times.

@tmshort
Copy link
Contributor

tmshort commented Sep 10, 2018

Correct. I designed it to be implicit, based on existing configuration; but this caused more trouble than it's worth with existing applications (i.e. they broke), and for ABI compatibility, it was disabled by default. Since this is a new feature 1.1.1, you have to explicitly turn it on. Remember, all previous 1.1.1 releases were beta releases, so the behavior of new features can change until the latest official release.

@notroj
Copy link

notroj commented Sep 10, 2018

I'm not concerned about the behaviour within 1.1.1, but compared to the current API. I would say that a major use case of SSL_CTX_set_client_cert_cb() has always been on-demand provision on client certs in HTTPS configurations where they are not required on initial handshake, so it looks like a major API break.

@tmshort
Copy link
Contributor

tmshort commented Sep 10, 2018

That's still the case. PHA is a TLSv1.3-specific feature. Certificate authentication during the handshake will still occur if PHA is not turned on. If PHA is enabled, then certificate authentication is moved to after the handshake.

@mattcaswell
Copy link
Member

I would say that a major use case of SSL_CTX_set_client_cert_cb() has always been on-demand provision on client certs in HTTPS configurations where they are not required on initial handshake, so it looks like a major API break.

Post-handshake authentication (PHA) has never been a feature - and this opt-in only applies to PHA. In handshake authentication can be done with TLSv1.2, and of course can happen with a renegotiation. But renegotiation is not possible in TLSv1.3. The real change here is the change from the TLSv1.2 way of doing this to the TLSv1.3 way of doing things.

The least breaking way of implementing this IMO is to have it an opt-in.

@tiran
Copy link
Contributor

tiran commented Sep 10, 2018

@notroj I was one of the developers who requested to change PHA for TLS 1.3. PHA broke several tests in CPython's test suit as well applications code. Tests and application relied on the fact that client certs are available after the handshake, when mandatory TLS client cert authentication has been enabled.

@mattcaswell 's change makes TLS client cert auth compatible with TLS 1.2. The new explicit opt-in for PAH makes it possible to use OpenSSL 1.1.1 and TLS 1.3 in application, that haven't been designed for TLS 1.3. IMHO it's a good thing.

@notroj
Copy link

notroj commented Sep 18, 2018

Disabling PHA support in all clients by default still looks like a major error to me. Disabling this support will break the relatively common HTTPS configuration for sites using client cert auth, where per-location (URI-path-based) authentication is required. This is true unless and until we go around and re-enable it again in every HTTPS client, i.e. reverse this default the very hard way. So far as I can tell, enabling PHA by default is completely harmless in HTTPS clients which configure certificates via SSL_CTX_use_certificate*.

Nobody seems willing to explain exactly what broke in Python here, but if "my test suite" broke is sufficient motivation for changing the default, then I'm unhappy to report that "my test suite also broke" - the Apache httpd test suite relies heavily on doing client cert auth post-initial handshake (since it is a complex and important mod_ssl feature) - and is thoroughly broken because of this change. More details here: http://mail-archives.apache.org/mod_mbox/httpd-dev/201809.mbox/%3C20180918140817.GA26207@redhat.com%3E

@tmshort
Copy link
Contributor

tmshort commented Sep 18, 2018

If those test suites were added for TLSv1.3 support in 1.1.1, then I don't think you can make the argument that this update broke those tests; as they were invalid once 1.1.1 (and TLSv1.3) was officially released.

PHA is a new feature on TLSv1.3, and while I'd personally prefer that it be "automatic", I'll deal with it not being on by default. In TLSv1.2 you had to explicitly start a new handshake/renegotiate in order to do certificate authentication based on URL, and many clients had broken TLS stacks that did not support secure renegotiation. Yes, secure renegotiation is not possible TLSv1.3 (one could argue that removing this broke those unit tests), and something else, TLSv1.3-specific, will have to be done.

@kaduk
Copy link
Contributor

kaduk commented Sep 18, 2018

Agreed, the reference point for comparisons here should be OpenSSL 1.1.0, not 1.1.1preN.

@tmshort
Copy link
Contributor

tmshort commented Sep 18, 2018

Rather than complaining it doesn't work, I would suggest creating a PR (in either in Perl to make it more compatible with OpenSSL 1.1.1/TLSv1.3, or OpenSSL) that solves the problem at hand, and then the merits of the problem and solution can be discussed.

@nmav
Copy link
Contributor Author

nmav commented Sep 19, 2018

Let me chime in since I recommended making pha opt-in; I'm not very familiar with the code base so I'll be a little abstract. Openssl handles tls1.x (x<=2) authentication (rehandshake) transparently and servers could rely on openssl clients re-authenticating when asked. When going to TLS1.3 re-authentication via PHA will not be transparent for existing clients (I guess browsers will be updated, but I believe Joe worries about other clients like curl, etc.)

What was the issue with the transparent PHA used in 1.1.1pre releases? The python issue was because the previous behavior made clients send their certificate during the session (post-handshake) but not during handshake. That meant that clients (in test suites) which were expected to be authenticated during handshake were seeing an unauthenticated session instead. That was solved by making PHA being opt-in for client apps.

One question is whether both concerns can be addressed, so both apache and clients are happy (on the high level it looks probable, by the pre-behavior and sending certificate during handshake if requested). However, what I think is the main question raised by Joe, is mainly whether the transparent re-authentication for clients is something worth of retaining. The benefit is that clients support re-authentication even under TLS1.3 the same way as before; the drawback is enabling an unwanted feature to clients who may never use it.

(IMHO if both concerns can be addressed, having transparency is a good thing as it simplifies clients, though it enables a feature they may have never asked which has proven to be problematic in the past; my recommendation was mainly driven by the latter concern, however the transparency benefits were not obvious to me at the time)

@notroj
Copy link

notroj commented Sep 19, 2018

If those test suites were added for TLSv1.3 support in 1.1.1

They were not, these are long-standing tests for mod_ssl which have worked for multiple versions of OpenSSL and SSL/TLS until now.

pghmcfc added a commit to pghmcfc/p5-net-ssleay that referenced this issue Sep 19, 2018
TLS 1.3 removed renegotiation in favor of rekeying and post handshake
authentication (PHA). With PHA, a server can request a client certificate from
a client at some point after the handshake. The feature is commonly used by
HTTP servers for conditional and path specific TLS client auth. For example, a
server can decide to require a cert based on HTTP method and/or path. A client
must announce support for PHA during the handshake.

Apache mod_ssl uses PHA:
https://github.com/apache/httpd/blob/trunk/modules/ssl/ssl_engine_kernel.c#L1207

As of OpenSSL ticket openssl/openssl#6933, TLS 1.3
clients no longer send the PHA TLS extension by default. For on-demand auth,
PHA extension must be enabled with SSL_CTX_set_post_handshake_auth(),
https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_post_handshake_auth.html .

This function is needed for the Apache httpd upstream test suite:
https://bugzilla.redhat.com/show_bug.cgi?id=1630391 .
h-vn pushed a commit to radiator-software/p5-net-ssleay that referenced this issue Sep 20, 2018
TLS 1.3 removed renegotiation in favor of rekeying and post handshake
authentication (PHA). With PHA, a server can request a client certificate from
a client at some point after the handshake. The feature is commonly used by
HTTP servers for conditional and path specific TLS client auth. For example, a
server can decide to require a cert based on HTTP method and/or path. A client
must announce support for PHA during the handshake.

Apache mod_ssl uses PHA:
https://github.com/apache/httpd/blob/trunk/modules/ssl/ssl_engine_kernel.c#L1207

As of OpenSSL ticket openssl/openssl#6933, TLS 1.3
clients no longer send the PHA TLS extension by default. For on-demand auth,
PHA extension must be enabled with SSL_CTX_set_post_handshake_auth(),
https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_post_handshake_auth.html .

This function is needed for the Apache httpd upstream test suite:
https://bugzilla.redhat.com/show_bug.cgi?id=1630391 .
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

No branches or pull requests

6 participants