TLS 1.3 options available with OpenSSL 1.1.1

[Dirkjan Bussink]

Hi all,

With the release of OpenSSL 1.1.1, TLS 1.3 is now also available. It already is working fine in my testing with HAProxy 1.8, there is however one issue. Currently there is no way to control the ciphers for TLS 1.3 from HAProxy, as according to the OpenSSL documentation, ciphers are handled by a separate method for TLS 1.3 compared to TLS 1.2 and earlier:

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_cipher_list.html

SSL_CTX_set_cipher_list() sets the list of available ciphers (TLSv1.2 and below) for ctx using the control string str.

SSL_CTX_set_ciphersuites() is used to configure the available TLSv1.3 ciphersuites for ctx.


Before I jump into writing code for this, I’m wondering what the approach is that HAProxy wants to take here. Should a similar options as todays `ciphers` option be made available in HAProxy to control the TLS 1.3 ciphers? If so, what should that be named?

Or is another approach preferred here? For example by still using the `ciphers` configuration setting, but by then filtering out ciphers that start with `TLS13` and set those separate with `SSL_CTX_set_ciphersuites`?

Cheers,

Dirkjan Bussink

[Lukas Tribus]

Hi Dirkjan,

Post by Dirkjan Bussink
Hi all,
https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_cipher_list.html
SSL_CTX_set_cipher_list() sets the list of available ciphers (TLSv1.2 and below) for ctx using the control string str.
SSL_CTX_set_ciphersuites() is used to configure the available TLSv1.3 ciphersuites for ctx.
Before I jump into writing code for this, I’m wondering what the approach is that HAProxy wants to take here. Should a similar options as todays `ciphers` option be made available in HAProxy to control the TLS 1.3 ciphers? If so, what should that be named?
Or is another approach preferred here? For example by still using the `ciphers` configuration setting, but by then filtering out ciphers that start with `TLS13` and set those separate with `SSL_CTX_set_ciphersuites`?

Definitely not some by string matching, openssl could have done
exactly that, they choose to make a new API call instead, and they
expect applications to introduce new configuration knobs or use the
generic configuration interface SSL_CONF, so no, let's not get crazy
with string magic here and expose the API as-is to the users
(SSL_CTX_set_ciphersuites() or the generic SSL_CONF().

I assume we don't have to change anything regarding groups/curves,
although they implemented the new SSL_CTX_set1_groups() call, but if I
understand correctly SSL_CTX_set1_curves_list() works just as well
with TLSv1.3, right?



Thanks,
Lukas

[Dirkjan Bussink]

Hi Lukas,

Post by Lukas Tribus
Definitely not some by string matching, openssl could have done
exactly that, they choose to make a new API call instead, and they
expect applications to introduce new configuration knobs or use the
generic configuration interface SSL_CONF, so no, let's not get crazy
with string magic here and expose the API as-is to the users
(SSL_CTX_set_ciphersuites() or the generic SSL_CONF().

So with a new API call, does that mean adding for example a `ciphersuites` option that works similar to `ciphers` today that it accepts a string and then calls `SSL_CTX_set_ciphersuites`? I can see if I can create a patch that does that (and ideally would be possible to backport to 1.8 as well, since I would like to be able to run TLS 1.3 then with 1.8 which works perfectly fine apart from a lack of tuning for this).

Post by Lukas Tribus
I assume we don't have to change anything regarding groups/curves,
although they implemented the new SSL_CTX_set1_groups() call, but if I
understand correctly SSL_CTX_set1_curves_list() works just as well
with TLSv1.3, right?

Yes, these work with TLSv1.3 still, but according to https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set1_curves_list.html there is preference for the groups functions (but not something I think needs to be addressed in the same patch / change?):

---

The curve functions are synonyms for the equivalently named group functions and are identical in every respect. They exist because, prior to TLS1.3, there was only the concept of supported curves. In TLS1.3 this was renamed to supported groups, and extended to include Diffie Hellman groups. The group functions should be used in preference.

---


Cheers,

Dirkjan

[Lukas Tribus]

Hello Dirkjan,

Post by Dirkjan Bussink
So with a new API call, does that mean adding for example a `ciphersuites`
option that works similar to `ciphers` today that it accepts a string and then
calls `SSL_CTX_set_ciphersuites`?

Yes, that's what I'd have in mind. Sufficiently #ifdef that everything
still builds the same and documented of course so that people
understand what is what. Let's wait for Manu and Emeric's feedback
though, before investing your time.

Post by Dirkjan Bussink
The curve functions are synonyms for the equivalently named group functions
and are identical in every respect.

So there is no technical reason to implement the new API call, it's
just to go with the new naming convention. No need for any action
then, we don't have to blow up our code with additional #ifdef mess
for the sake of using better looking names in API calls.


Thanks for looking into this!

cheers,
lukas

[Dirkjan Bussink]

Hi all,

I took the liberty of writing up a patch with what this could look like. I have named the option `ciphersuites` and also added the documentation for it as well. I have attached the patch to this email.

I think if TLSv <= 1.2 and TLSv1.3 ciphers are handled separately, this is good reason
to add a new keyword to manage both at a same line on an haproxy configuration file's line .

This is what my patch does indeed.

I've just realized that it may be the openssl's response to an issue we faced on earlier version of
openssl1.1.1 dev branch where forcing cipher suite on a SSL_CTX broke TLSv1.2 handshakes if
no TLSv1.3 ciphers were specified in this list.
Doing this, managing differently TLS <= v1.2 and 1.3 ciphers permits the user to not face regression issues
upgrading to v1.1.1 when suites where forced in configuration because openssl-1.1.1 kept default
TVSv1.3 ciphers.

Yeah, without the configurations setting it uses the 1.3 defaults which are already good safe defaults.

So i'm convinced we need to handle this new TLSv1.3 cipher suite with a new config keyword, but I
don't know how we should name it. I think it will be a mistake to make appear 1.3 in the new name because
there is no warranty that next TLS versions will specify specific cipher lists. Openssl's
API make the choice of "ciphersuites" ... perhaps a the right choice.

That’s what I did :). Hopefully it looks somewhat sensible.

Did any of you check how this is endled on "openssl s_client" command line?

From the help for this command:

-cipher val Specify TLSv1.2 and below cipher list to be used
-ciphersuites val Specify TLSv1.3 ciphersuites to be used

So it does the same thing as my patch does.

Cheers,

Dirkjan

[Emmanuel Hocdet]

Hi Emeric, Lukas, Dirkjan

Hi Lukas, Dirkjan,

Post by Lukas Tribus
Hello Dirkjan,

Post by Dirkjan Bussink
So with a new API call, does that mean adding for example a `ciphersuites`
option that works similar to `ciphers` today that it accepts a string and then
calls `SSL_CTX_set_ciphersuites`?

Yes, that's what I'd have in mind. Sufficiently #ifdef that everything
still builds the same and documented of course so that people
understand what is what. Let's wait for Manu and Emeric's feedback
though, before investing your time.

I think if TLSv <= 1.2 and TLSv1.3 ciphers are handled separately, this is good reason
to add a new keyword to manage both at a same line on an haproxy configuration file's line .
I've just realized that it may be the openssl's response to an issue we faced on earlier version of
openssl1.1.1 dev branch where forcing cipher suite on a SSL_CTX broke TLSv1.2 handshakes if
no TLSv1.3 ciphers were specified in this list.
Doing this, managing differently TLS <= v1.2 and 1.3 ciphers permits the user to not face regression issues
upgrading to v1.1.1 when suites where forced in configuration because openssl-1.1.1 kept default
TVSv1.3 ciphers.

Same deal with boringssl, TLSv <= 1.2 ciphers configuration and TLSv1.3 ciphers are segregated.
https://boringssl.googlesource.com/boringssl/+/abbbee10ad4739648bcbf36a5ac52f23263a67dd%5E!/

So i'm convinced we need to handle this new TLSv1.3 cipher suite with a new config keyword, but I
don't know how we should name it. I think it will be a mistake to make appear 1.3 in the new name because
there is no warranty that next TLS versions will specify specific cipher lists. Openssl's
API make the choice of "ciphersuites" ... perhaps a the right choice.

Following the API name should be the right choice.

++
Manu

[Dirkjan Bussink]

Hi all,

Post by Emmanuel Hocdet
Same deal with boringssl, TLSv <= 1.2 ciphers configuration and TLSv1.3 ciphers are segregated.
https://boringssl.googlesource.com/boringssl/+/abbbee10ad4739648bcbf36a5ac52f23263a67dd%5E!/

This reminded me to double check with BoringSSL and LibreSSL how they handle this. Neither has this new method, so I have updated the conditional used in the patch to exclude these two as well.

Cheers,

Dirkjan

[Emmanuel Hocdet]

Post by Dirkjan Bussink
Hi all,

Post by Emmanuel Hocdet
Same deal with boringssl, TLSv <= 1.2 ciphers configuration and TLSv1.3 ciphers are segregated.
https://boringssl.googlesource.com/boringssl/+/abbbee10ad4739648bcbf36a5ac52f23263a67dd%5E!/

This reminded me to double check with BoringSSL and LibreSSL how they handle this. Neither has this new method, so I have updated the conditional used in the patch to exclude these two as well.

It’s not necessary, BoringSSL and LibreSSL have, at best, OPENSSL_VERSION_NUMBER set to 1.1.0 for API compatibilité.

++
Manu

[Dirkjan Bussink]

Hi all,

Post by Emmanuel Hocdet
It’s not necessary, BoringSSL and LibreSSL have, at best, OPENSSL_VERSION_NUMBER set to 1.1.0 for API compatibilité.

Looking at LibreSSL, it’s defining this (in their latest Git code):

src/lib/libcrypto/opensslv.h:#define OPENSSL_VERSION_NUMBER 0x20000000L

I also see this conditional used in other places to explicitly exclude BoringSSL and LibreSSL, so that’s why I thought it would be needed here as well.

[Dirkjan Bussink]

Hi all,

Given all the critical security issue and that you all were busy with that, I suspect this didn’t get much additional eyes. Now that that fix is out the door, I’m wondering if there’s any feedback or further input for the OpenSSL 1.1.1 patches I wrote?

Cheers,

Dirkjan

Post by Dirkjan Bussink
Hi all,

Post by Emmanuel Hocdet
It’s not necessary, BoringSSL and LibreSSL have, at best, OPENSSL_VERSION_NUMBER set to 1.1.0 for API compatibilité.

src/lib/libcrypto/opensslv.h:#define OPENSSL_VERSION_NUMBER 0x20000000L
I also see this conditional used in other places to explicitly exclude BoringSSL and LibreSSL, so that’s why I thought it would be needed here as well.
--
Cheers,
Dirkjan

[Dirkjan Bussink]

Hi Emeric,

Could you precise in the old "ciphers" description that this applies only for TLSv <= 1.2. (and add a ref to the new keyword for TLSv1.3)

I have updated the documentation. Hopefully this is good then. Would it be possible to also backport this to 1.8 so that we could deploy a future 1.8 stable version with OpenSSL 1.1.1 in the future?

Cheers,

Dirkjan

[Lukas Tribus]

Post by Emmanuel Hocdet
Hi Emeric,

Could you precise in the old "ciphers" description that this applies only for TLSv <= 1.2. (and add a ref to the new keyword for TLSv1.3)

I have updated the documentation. Hopefully this is good then. Would it be possible to also backport this to 1.8 so that we could deploy a future 1.8 stable version with OpenSSL 1.1.1 in the future?

There is one space too much in the beginning of the penultimate line
in the doc of "ssl-default-server-ciphersuites" (--> <--TLSv1.2 and
earlier, please check).

I agree we should backport this to 1.8. As far as I can see, this
change is as safe as it could be (as everything is #ifdef'ed and
exludes boringssl and libressl), and frankly we are gonna need this in
1.8.


To'ing Willy.


Thanks for this,
Lukas

[Dirkjan Bussink]

Hi Lukas,

Post by Lukas Tribus
There is one space too much in the beginning of the penultimate line
in the doc of "ssl-default-server-ciphersuites" (--> <--TLSv1.2 and
earlier, please check).

Updated in the attached patch!

Cheers,

Dirkjan

[Willy Tarreau]

Post by Dirkjan Bussink
I have updated the documentation. Hopefully this is good then. Would it be
possible to also backport this to 1.8 so that we could deploy a future 1.8
stable version with OpenSSL 1.1.1 in the future?

Indeed, 1.8 is supposed to support openssl-1.1.1 so it should be backported.
I'm OK to merge this patch.

Now applied to master. I've added a note to the commit message to mention
the need to backport it to 1.8 as well.

Thanks guys!
Willy