Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

> Le 25 oct. 2017 à 15:45, Emmanuel Hocdet  a écrit :
> 
> 
> Hi Olivier,
> 
> 
>> Le 25 oct. 2017 à 14:57, Olivier Houchard  a écrit :
>> 
>> On Wed, Oct 25, 2017 at 02:37:58PM +0200, Emmanuel Hocdet wrote:
>>> Hi,
>>> 
>>> . patches serie rebase from master 
>>> . update openssl 1.1.1 api calls with new early callback name
>>> (https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_client_hello_cb.html
>>>  
>>> )
>>> 
>> 
>> That mostly looks like the version I maintained, except :
>> -if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name))
>> +if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name) || 
>> !s->generate_certs)
>> 
>> Shouldn't that be && !s->generate_certs ? Or we'll return 
>> SSL_TLSEXT_ERR_NOACK
>> as soon as we don't generate certificates.
> 
> Indeed, it’s &&, i test with strict-sni and doesn’t see that.
> thanks
> 

patches serie with simplify condition:



0001-MEDIUM-ssl-convert-CBS-BoringSSL-api-usage-to-neutra.patch
Description: Binary data


0002-MINOR-ssl-support-Openssl-1.1.1-early-callback-for-s.patch
Description: Binary data


0003-MINOR-ssl-generated-certificate-is-missing-in-switch.patch
Description: Binary data

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

Hi Olivier,


> Le 25 oct. 2017 à 14:57, Olivier Houchard  a écrit :
> 
> On Wed, Oct 25, 2017 at 02:37:58PM +0200, Emmanuel Hocdet wrote:
>> Hi,
>> 
>> . patches serie rebase from master 
>> . update openssl 1.1.1 api calls with new early callback name
>> (https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_client_hello_cb.html
>>  
>> )
>> 
> 
> That mostly looks like the version I maintained, except :
> - if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name))
> + if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name) || 
> !s->generate_certs)
> 
> Shouldn't that be && !s->generate_certs ? Or we'll return SSL_TLSEXT_ERR_NOACK
> as soon as we don't generate certificates.

Indeed, it’s &&, i test with strict-sni and doesn’t see that.
thanks

Manu

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Olivier Houchard]

Hi Emmanuel,

On Wed, Oct 25, 2017 at 02:37:58PM +0200, Emmanuel Hocdet wrote:
> Hi,
> 
> . patches serie rebase from master 
> . update openssl 1.1.1 api calls with new early callback name
> (https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_client_hello_cb.html 
> )
> 

That mostly looks like the version I maintained, except :
-   if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name))
+   if (!SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name) || 
!s->generate_certs)

Shouldn't that be && !s->generate_certs ? Or we'll return SSL_TLSEXT_ERR_NOACK
as soon as we don't generate certificates.

Regards,

Olivier

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

Hi,. patches serie rebase from master . update openssl 1.1.1 api calls with new early callback name(https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_client_hello_cb.html)Le 4 sept. 2017 à 16:39, Emmanuel Hocdet  a écrit :Hi Emeric, ChristopherIf you can review when you have time. (3) for Christopher.This patches allows to support native multicert selection (RSA/ECDSA) andssl-min-ver/ ssl-max-ver per certificat with openssl 1.1.1 (boringssl is the onlyone to support this until this patch).patches:1) Convert BoringSSL api call (CBS) to ssl-lib independent code.    This is the biggest part and only depend on BoringSSL build (until 2).2) support  openssl 1.1.1 early callback API. It mimic BoringSSL api, and this   is a good news (small patch).   Do we want to push code for openssl 1.1.1 (dev) in haproxy (dev) now?3) Add generated certificate for early switch-ctx.    Historically this part has been skipped (no supported for boringssl).    There are now a ssl_sock_generate_certificate_from_conn func, but i don’t    understand how this take a real/generated cert.    Christopher, can you take a look?Manu

0001-MEDIUM-ssl-convert-CBS-BoringSSL-api-usage-to-neutra.patch
Description: Binary data


0002-MINOR-ssl-support-Openssl-1.1.1-early-callback-for-s.patch
Description: Binary data


0003-MINOR-ssl-generated-certificate-is-missing-in-switch.patch
Description: Binary data

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

Hi Willy,

> Le 5 sept. 2017 à 10:11, Willy Tarreau  a écrit :
> 
> Hi Manu,
> 
> On Mon, Sep 04, 2017 at 04:39:45PM +0200, Emmanuel Hocdet wrote:
>> Hi Emeric, Christopher
>> 
>> If you can review when you have time. (3) for Christopher.
>> 
>> This patches allows to support native multicert selection (RSA/ECDSA) and
>> ssl-min-ver/ ssl-max-ver per certificat with openssl 1.1.1 (boringssl is the 
>> only
>> one to support this until this patch).
>> 
>> patches:
>> 1) Convert BoringSSL api call (CBS) to ssl-lib independent code.
>>This is the biggest part and only depend on BoringSSL build (until 2).
>> 
>> 2) support  openssl 1.1.1 early callback API. It mimic BoringSSL api, and 
>> this
>>   is a good news (small patch).
>>   Do we want to push code for openssl 1.1.1 (dev) in haproxy (dev) now?
> 
> I suspect it will be mandatory in order to support TLS early-data (0-RTT).
> So I think it will be nice to have it before the release. However given that
> both Christopher and Emeric are heavily loaded on the multi-threading part,
> I suggest that we postpone the patchset review until the multi-thread stuff
> gets merged. As you say, the patch is small so it will be easy to review and
> apply, and/or revert in case of issues so it's not a big deal to merge it
> late in the cycle.

It will be mandatory to avoid unexpected behavior (see notes on openssl 1.1.1 
API doc),
as I have noticed with boringssl before implement early callback. And yes, 
early-data
should need it and will work per certificate (i already tested it with 
boringssl).

Encouraging Emeric and Christopher for the multithreading part!
++
Manu

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Willy Tarreau]

Hi Manu,

On Mon, Sep 04, 2017 at 04:39:45PM +0200, Emmanuel Hocdet wrote:
> Hi Emeric, Christopher
> 
> If you can review when you have time. (3) for Christopher.
> 
> This patches allows to support native multicert selection (RSA/ECDSA) and
> ssl-min-ver/ ssl-max-ver per certificat with openssl 1.1.1 (boringssl is the 
> only
> one to support this until this patch).
> 
> patches:
> 1) Convert BoringSSL api call (CBS) to ssl-lib independent code.
> This is the biggest part and only depend on BoringSSL build (until 2).
> 
> 2) support  openssl 1.1.1 early callback API. It mimic BoringSSL api, and this
>is a good news (small patch).
>Do we want to push code for openssl 1.1.1 (dev) in haproxy (dev) now?

I suspect it will be mandatory in order to support TLS early-data (0-RTT).
So I think it will be nice to have it before the release. However given that
both Christopher and Emeric are heavily loaded on the multi-threading part,
I suggest that we postpone the patchset review until the multi-thread stuff
gets merged. As you say, the patch is small so it will be easy to review and
apply, and/or revert in case of issues so it's not a big deal to merge it
late in the cycle.

Thanks,
Willy

Re: [PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

link on openssl 1.1.1 API doc:

https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_early_cb.html

NOTES

[…]

"It is also recommended that applications utilize an early callback and not use 
a servername callback, in order to avoid unexpected behavior that occurs due to 
the relative order of processing between things like session resumption and the 
historical servername callback."

> Le 4 sept. 2017 à 16:39, Emmanuel Hocdet  a écrit :
> 
> Hi Emeric, Christopher
> 
> If you can review when you have time. (3) for Christopher.
> 
> This patches allows to support native multicert selection (RSA/ECDSA) and
> ssl-min-ver/ ssl-max-ver per certificat with openssl 1.1.1 (boringssl is the 
> only
> one to support this until this patch).
> 
> patches:
> 1) Convert BoringSSL api call (CBS) to ssl-lib independent code.
>This is the biggest part and only depend on BoringSSL build (until 2).
> 
> 2) support  openssl 1.1.1 early callback API. It mimic BoringSSL api, and this
>   is a good news (small patch).
>   Do we want to push code for openssl 1.1.1 (dev) in haproxy (dev) now?
> 
> 3) Add generated certificate for early switch-ctx.
>Historically this part has been skipped (no supported for boringssl).
>There are now a ssl_sock_generate_certificate_from_conn func, but i don’t
>understand how this take a real/generated cert.
>Christopher, can you take a look?
> 
> ++
> Manu
> 
> 
> <0001-MEDIUM-ssl-convert-CBS-BoringSSL-api-usage-to-neutra.patch><0002-MINOR-ssl-support-Openssl-1.1.1-early-callback-for-s.patch><0003-MINOR-ssl-generated-certificate-is-missing-in-switch.patch>
> 

[PATCH] support Openssl 1.1.1 early callback API for HS

[Emmanuel Hocdet]

Hi Emeric, Christopher

If you can review when you have time. (3) for Christopher.

This patches allows to support native multicert selection (RSA/ECDSA) and
ssl-min-ver/ ssl-max-ver per certificat with openssl 1.1.1 (boringssl is the 
only
one to support this until this patch).

patches:
1) Convert BoringSSL api call (CBS) to ssl-lib independent code.
This is the biggest part and only depend on BoringSSL build (until 2).

2) support  openssl 1.1.1 early callback API. It mimic BoringSSL api, and this
   is a good news (small patch).
   Do we want to push code for openssl 1.1.1 (dev) in haproxy (dev) now?

3) Add generated certificate for early switch-ctx.
Historically this part has been skipped (no supported for boringssl).
There are now a ssl_sock_generate_certificate_from_conn func, but i don’t
understand how this take a real/generated cert.
Christopher, can you take a look?

++
Manu




0001-MEDIUM-ssl-convert-CBS-BoringSSL-api-usage-to-neutra.patch
Description: Binary data


0002-MINOR-ssl-support-Openssl-1.1.1-early-callback-for-s.patch
Description: Binary data


0003-MINOR-ssl-generated-certificate-is-missing-in-switch.patch
Description: Binary data