Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-10-08 Thread Willy Tarreau
On Mon, Oct 08, 2018 at 04:35:55PM +0200, Emeric Brun wrote:
> > 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



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-10-08 Thread Emeric Brun
Hi Dirkjan, Lukas 
On 10/08/2018 09:54 AM, Dirkjan Bussink wrote:
> Hi Lukas,
> 
>> On 7 Oct 2018, at 14:18, Lukas Tribus  wrote:
>>
>> 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
> 


> 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.

R,
Emeric



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-10-08 Thread Dirkjan Bussink
Hi Lukas,

> On 7 Oct 2018, at 14:18, Lukas Tribus  wrote:
> 
> 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



0001-Add-support-for-ciphersuites-option-for-TLSv1.3.patch
Description: Binary data


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-10-07 Thread Lukas Tribus
On Sat, 6 Oct 2018 at 13:03, Dirkjan Bussink  wrote:
>
> Hi Emeric,
>
> > On 24 Sep 2018, at 15:33, Emeric Brun  wrote:
> >
> > Seems good for me except for documentation:
> >
> > 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



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-10-06 Thread Dirkjan Bussink
Hi Emeric,

> On 24 Sep 2018, at 15:33, Emeric Brun  wrote:
> 
> Seems good for me except for documentation:
> 
> 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



0001-Add-support-for-ciphersuites-option-for-TLSv1.3.patch
Description: Binary data


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-24 Thread Emeric Brun
Hi Dirkjan,

On 09/24/2018 11:55 AM, Dirkjan Bussink wrote:
> 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
> 
>> On 14 Sep 2018, at 14:28, Dirkjan Bussink  wrote:
>>
>> Hi all,
>>
>>> On 14 Sep 2018, at 14:15, Emmanuel Hocdet  wrote:
>>>
>>> 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  0x2000L
>>
>> 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
> 

Seems good for me except for documentation:

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)

R,
Emeric



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-24 Thread 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

> On 14 Sep 2018, at 14:28, Dirkjan Bussink  wrote:
> 
> Hi all,
> 
>> On 14 Sep 2018, at 14:15, Emmanuel Hocdet  wrote:
>> 
>> 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   0x2000L
> 
> 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




Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread Dirkjan Bussink
Hi all,

> On 14 Sep 2018, at 14:15, Emmanuel Hocdet  wrote:
> 
> 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 0x2000L

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


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread Emmanuel Hocdet


> Le 14 sept. 2018 à 14:01, Dirkjan Bussink  a écrit :
> 
> Hi all,
> 
>> On 14 Sep 2018, at 12:18, Emmanuel Hocdet  wrote:
>> 
>> 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





Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread Dirkjan Bussink
Hi all,

> On 14 Sep 2018, at 12:18, Emmanuel Hocdet  wrote:
> 
> 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



0001-Add-support-for-ciphersuites-option-for-TLS-1.3.patch
Description: Binary data


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread Emmanuel Hocdet


Hi Emeric, Lukas, Dirkjan

> Le 14 sept. 2018 à 11:12, Emeric Brun  a écrit :
> 
> Hi Lukas, Dirkjan,
> 
> On 09/13/2018 10:17 PM, Lukas Tribus wrote:
>> Hello Dirkjan,
>> 
>> 
>> On Thu, 13 Sep 2018 at 16:44, Dirkjan Bussink  wrote:
>>> 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





Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread 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. 


> On 14 Sep 2018, at 11:12, Emeric Brun  wrote:
> 
> 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 valSpecify 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



0001-Add-support-for-ciphersuites-option-for-TLS-1.3.patch
Description: Binary data


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-14 Thread Emeric Brun
Hi Lukas, Dirkjan,

On 09/13/2018 10:17 PM, Lukas Tribus wrote:
> Hello Dirkjan,
> 
> 
> On Thu, 13 Sep 2018 at 16:44, Dirkjan Bussink  wrote:
>> 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.

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.

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

> 
> 
>> 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.

Perfectly agreed.

> 
> Thanks for looking into this!
> 
> cheers,
> lukas
> 
R,
Emeric



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-13 Thread Lukas Tribus
Hello Dirkjan,


On Thu, 13 Sep 2018 at 16:44, Dirkjan Bussink  wrote:
> 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.


> 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



Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-13 Thread Dirkjan Bussink
Hi Lukas,

> On 13 Sep 2018, at 16:14, Lukas Tribus  wrote:
> 
> 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). 

> 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


Re: TLS 1.3 options available with OpenSSL 1.1.1

2018-09-13 Thread Lukas Tribus
Hi Dirkjan,



On Thu, 13 Sep 2018 at 15:35, Dirkjan Bussink  wrote:
>
> 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`?

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



TLS 1.3 options available with OpenSSL 1.1.1

2018-09-13 Thread 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