Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Lukas, On Thu, Feb 14, 2019 at 06:28:29PM +0100, Lukas Tribus wrote: > Hello, > > > FYI the behavior was also changed on the openssl side (and will be in 1.1.1b): > https://github.com/openssl/openssl/commit/4af5836b55442f31795eff6c8c81ea7a1b8cf94b > > So applications fixes are only necessary for 1.1.1a. Cool, thanks for the heads up! Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hello, FYI the behavior was also changed on the openssl side (and will be in 1.1.1b): https://github.com/openssl/openssl/commit/4af5836b55442f31795eff6c8c81ea7a1b8cf94b So applications fixes are only necessary for 1.1.1a. Lukas
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
śr., 23 sty 2019 o 11:53 Janusz Dziemidowicz napisał(a): > 1.14.2 is current version in Debian testing. Debian seems reluctant to > use "mainline" nginx versions (1.15.x) so 1.14.x might end in Debian > 10. I'll try to file Debian bug report later today. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=920297 -- Janusz Dziemidowicz
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
śr., 23 sty 2019 o 10:41 Lukas Tribus napisał(a): > > I tested all my servers and I've noticed that nginx is broken too. I > > am running nginx 1.14.2 with OpenSSL 1.1.1a The nginx source contains > > exactly the same function as haproxy: > > https://trac.nginx.org/nginx/browser/nginx/src/event/ngx_event_openssl.c?rev=ebf8c9686b8ce7428f975d8a567935ea3722da70#L850 > > > > However, it seems that it might have been fixed in 1.15.2 by this commit: > > https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c > > Thanks for this. It's actually nginx 1.15.4 (September 2018) where > this commit is present. Yes, typed too fast ;) > Are nginx folks aware of the problem? It would probably be wise for > them to backport the fix to their 1.14 tree ... 1.14.2 is current version in Debian testing. Debian seems reluctant to use "mainline" nginx versions (1.15.x) so 1.14.x might end in Debian 10. I'll try to file Debian bug report later today. -- Janusz Dziemidowicz
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Wed, Jan 23, 2019 at 10:40:09AM +0100, Lukas Tribus wrote: > Also, we need a big fat warning that all TLSv1.3 users must upgrade in > the next 1.8 and 1.9 stable version announcement containing this fix. That's a good point, this will also encourage distro maintainers to update their versions. > I have filed a tracking bug for this, which can be closed when backported: > https://github.com/haproxy/haproxy/issues/24 > > Closed or not, the tracking bug makes this easier to find. Thanks! Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Wed, 23 Jan 2019 at 09:52, Willy Tarreau wrote: > > On Wed, Jan 23, 2019 at 12:07:04AM -0800, Dirkjan Bussink wrote: > > Of course, you're right. New version of the patch attached! > > Now merged, thank you! It's obvious, but because the commit message doesn't not explicitly mention it: This must be backported to 1.8. Also, we need a big fat warning that all TLSv1.3 users must upgrade in the next 1.8 and 1.9 stable version announcement containing this fix. I have filed a tracking bug for this, which can be closed when backported: https://github.com/haproxy/haproxy/issues/24 Closed or not, the tracking bug makes this easier to find. > I tested all my servers and I've noticed that nginx is broken too. I > am running nginx 1.14.2 with OpenSSL 1.1.1a The nginx source contains > exactly the same function as haproxy: > https://trac.nginx.org/nginx/browser/nginx/src/event/ngx_event_openssl.c?rev=ebf8c9686b8ce7428f975d8a567935ea3722da70#L850 > > However, it seems that it might have been fixed in 1.15.2 by this commit: > https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c Thanks for this. It's actually nginx 1.15.4 (September 2018) where this commit is present. Are nginx folks aware of the problem? It would probably be wise for them to backport the fix to their 1.14 tree ... > And just for reference, I've found Chrome bug with this problem (as I > am interested when this will get enabled to keep all my systems > updated) https://bugs.chromium.org/p/chromium/issues/detail?id=923685 Thanks, will subscribe to this bug also. Regards, Lukas
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Wed, Jan 23, 2019 at 12:07:04AM -0800, Dirkjan Bussink wrote: > Of course, you're right. New version of the patch attached! Now merged, thank you! Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Willy, On 22 Jan 2019, at 23:17, Willy Tarreau wrote: > > As you can see it will enable this code when SSL_OP_NO_RENEGOTIATION=0, > which is what BoringSSL does and it needs this code to be disabled. Thus > I think it's better to simply do this : > > +#ifndef SSL_OP_NO_RENEGOTIATION > + /* Please note that BoringSSL defines this macro to zero so don't > + * change this to #if and do not assign a default value to this macro! > + */ > Of course, you’re right. New version of the patch attached! Cheers, Dirkjan 0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch Description: Binary data
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Dirkjan,
On Tue, Jan 22, 2019 at 11:07:07PM -0800, Dirkjan Bussink wrote:
> I have adjusted the patch to make it more robust and more match the style of
> how we use other options. How does this look to you?
Unfortunately it does introduce the problem I feared for BoringSSL :
+#if !defined(SSL_OP_NO_RENEGOTIATION) || SSL_OP_NO_RENEGOTIATION == 0
if (where & SSL_CB_HANDSHAKE_START) {
/* Disable renegotiation (CVE-2009-3555) */
if ((conn->flags & (CO_FL_CONNECTED | CO_FL_EARLY_SSL_HS |
CO_FL_EARLY_DATA)) == CO_FL_CONNECTED) {
@@ -1475,6 +1476,7 @@ void ssl_sock_infocbk(const SSL *ssl, int where, int ret)
conn->err_code = CO_ER_SSL_RENEG;
}
}
+#endif
As you can see it will enable this code when SSL_OP_NO_RENEGOTIATION=0,
which is what BoringSSL does and it needs this code to be disabled. Thus
I think it's better to simply do this :
+#ifndef SSL_OP_NO_RENEGOTIATION
+ /* Please note that BoringSSL defines this macro to zero so don't
+* change this to #if and do not assign a default value to this macro!
+*/
Thanks,
Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Willy, > On 22 Jan 2019, at 07:07, Willy Tarreau wrote: > > Hi guys, > > On Tue, Jan 22, 2019 at 03:22:38PM +0100, Emeric Brun wrote: >> I think you can merge this. > > OK. I still find it very fragile in that we usually don't make a > difference between an absent define and the same declared as zero, and > most SSL_OP_* entries are defined this way in ssl_sock.c, but I don't > see that many other options here. I think that the #ifndef at least > deserves a comment indicating that it may also match a zero value to > detect safe implementations so that we are not tempted later to refactor > this and break BoringSSL. > > We can also add a Reported-By to ack Adam's original work on the issue. > > Just let me know if I need to adjust it myself or if anyone wants to take > care of it. I have adjusted the patch to make it more robust and more match the style of how we use other options. How does this look to you? Cheers, Dirkjan 0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch Description: Binary data
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi guys, On Tue, Jan 22, 2019 at 03:22:38PM +0100, Emeric Brun wrote: > I think you can merge this. OK. I still find it very fragile in that we usually don't make a difference between an absent define and the same declared as zero, and most SSL_OP_* entries are defined this way in ssl_sock.c, but I don't see that many other options here. I think that the #ifndef at least deserves a comment indicating that it may also match a zero value to detect safe implementations so that we are not tempted later to refactor this and break BoringSSL. We can also add a Reported-By to ack Adam's original work on the issue. Just let me know if I need to adjust it myself or if anyone wants to take care of it. Thanks, Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Willy, On 1/21/19 6:38 PM, Dirkjan Bussink wrote: > Hi Emeric, > >> On 21 Jan 2019, at 08:06, Emeric Brun wrote: >> >> Interesting, it would be good to skip the check using the same method. >> >> We must stay careful to not put the OP_NO_RENEG flag on the client part >> (when haproxy connects to server), because reneg from server is authorized >> but i think infocbk is called only on frontend/accept side. >> >> so a patch which do: >> >> #ifdef SSL_OP_NO_RENEGOTIATION >> SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION); >> #endif >> >> without condition during init >> >> and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix >> the issue mentionned about keyupdate and will fix the CVE using the clean >> way if the version >> of openssl support. > > I have implemented this and attached the patch for it. What do you think of > this approach? > > Cheers, > > Dirkjan Bussink > I think you can merge this. Thx Dirkjan. R, Emeric
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Willy, On 1/21/19 6:38 PM, Dirkjan Bussink wrote: > Hi Emeric, > >> On 21 Jan 2019, at 08:06, Emeric Brun wrote: >> >> Interesting, it would be good to skip the check using the same method. >> >> We must stay careful to not put the OP_NO_RENEG flag on the client part >> (when haproxy connects to server), because reneg from server is authorized >> but i think infocbk is called only on frontend/accept side. >> >> so a patch which do: >> >> #ifdef SSL_OP_NO_RENEGOTIATION >> SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION); >> #endif >> >> without condition during init >> >> and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix >> the issue mentionned about keyupdate and will fix the CVE using the clean >> way if the version >> of openssl support. > > I have implemented this and attached the patch for it. What do you think of > this approach? > > Cheers, > > Dirkjan Bussink > I think you can merge this R, Emeric
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
> Le 21 janv. 2019 à 19:31, Adam Langley a écrit : > > On Mon, Jan 21, 2019 at 10:16 AM Dirkjan Bussink wrote: >> Ah ok, I recently added support in HAProxy to handle the new >> SSL_CTX_set_ciphersuites option since OpenSSL handles setting TLS 1.3 >> ciphers separate from the regular ones. Are those things that BoringSSL >> would also want to adopt then? > > SSL_CTX_set_ciphersuites is more than a compatibility hack like adding > a dummy #define, and the considerations are more complex. I'm not sure > that we want to allow TLS 1.3 ciphersuite to be configured: the > excessive number of cipher suites prior to TLS 1.3 was a problem, as > was the excessive diversity of configurations. Also, string-based APIs > have historically been expensive because they prevent easy static > analysis. So we could add a dummy SSL_CTX_set_ciphersuites that always > returns zero, but most applications would probably take that to be a > fatal error so that wouldn't be helpful. So SSL_CTX_set_ciphersuites > might be a case where a #ifdef is the best answer. But we'll always > think about such things if asked. > I agree, no need for SSL_CTX_set_ciphersuites. If a security issue appear on cipher i suppose BoringSSL will evolve with a default fix. > (If you happen to know, I would be curious who is using BoringSSL with > HAProxy.) > We used BoringSSL in production since 1.5 year. ++ Manu
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
> Le 21 janv. 2019 à 19:07, Dirkjan Bussink a écrit : > > Hi Manu, > >> On 21 Jan 2019, at 09:49, Emmanuel Hocdet wrote: >> >> Boringssl does not have SSL_OP_NO_RENEGOTIATION and need KeyUpdate to work. >> As workaround, SSL_OP_NO_RENEGOTIATION could be set to 0 in openssl-compat.h. > > Hmm, then we will need a different #define though since we can’t rely own the > constant not being defined in that case to disable the logic. We would need a > separate way to detect this then. Is there a good example of this or should I > change the logic then to version checks instead? And how about LibreSSL in > that case? > > Does BoringSSL need any of the logic in the first place? There’s not really > versions of it, so is the target there always current master or something > else? > No need to change, SSL_OP_NO_RENEGOTIATION is now in Boringssl, thanks Adam, and renegotiation is disabled by default. For LibreSSL, no TLSv1.3, no SSL_OP_NO_RENEGOTIATION. ++ Manu
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Mon, Jan 21, 2019 at 10:16 AM Dirkjan Bussink wrote: > Ah ok, I recently added support in HAProxy to handle the new > SSL_CTX_set_ciphersuites option since OpenSSL handles setting TLS 1.3 ciphers > separate from the regular ones. Are those things that BoringSSL would also > want to adopt then? SSL_CTX_set_ciphersuites is more than a compatibility hack like adding a dummy #define, and the considerations are more complex. I'm not sure that we want to allow TLS 1.3 ciphersuite to be configured: the excessive number of cipher suites prior to TLS 1.3 was a problem, as was the excessive diversity of configurations. Also, string-based APIs have historically been expensive because they prevent easy static analysis. So we could add a dummy SSL_CTX_set_ciphersuites that always returns zero, but most applications would probably take that to be a fatal error so that wouldn't be helpful. So SSL_CTX_set_ciphersuites might be a case where a #ifdef is the best answer. But we'll always think about such things if asked. (If you happen to know, I would be curious who is using BoringSSL with HAProxy.) Cheers AGL -- Adam Langley a...@imperialviolet.org https://www.imperialviolet.org
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Adam, > On 21 Jan 2019, at 10:09, Adam Langley wrote: > > HAProxy isn't a user that we have on our radar, but BoringSSL dislikes > pushing compatibility hacks into downstream projects. (You can always > ask for these things to be included in BoringSSL instead.) Ah ok, I recently added support in HAProxy to handle the new SSL_CTX_set_ciphersuites option since OpenSSL handles setting TLS 1.3 ciphers separate from the regular ones. Are those things that BoringSSL would also want to adopt then? Cheers, Dirkjan Bussink
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Mon, Jan 21, 2019 at 9:49 AM Emmanuel Hocdet wrote: > Boringssl does not have SSL_OP_NO_RENEGOTIATION and need KeyUpdate to work. > As workaround, SSL_OP_NO_RENEGOTIATION could be set to 0 in openssl-compat.h. HAProxy isn't a user that we have on our radar, but BoringSSL dislikes pushing compatibility hacks into downstream projects. (You can always ask for these things to be included in BoringSSL instead.) Thus I've just added SSL_OP_NO_RENEGOTIATION as a no-op to BoringSSL: https://boringssl.googlesource.com/boringssl/+/20f4a043eb8c148d044777b849a1cc1e0168b9ee (Renegotiation is disabled by default in BoringSSL already. Also, there's only the current version of BoringSSL so no need to wait for any releases.) Cheers AGL -- Adam Langley a...@imperialviolet.org https://www.imperialviolet.org
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Manu, > On 21 Jan 2019, at 09:49, Emmanuel Hocdet wrote: > > Boringssl does not have SSL_OP_NO_RENEGOTIATION and need KeyUpdate to work. > As workaround, SSL_OP_NO_RENEGOTIATION could be set to 0 in openssl-compat.h. Hmm, then we will need a different #define though since we can’t rely own the constant not being defined in that case to disable the logic. We would need a separate way to detect this then. Is there a good example of this or should I change the logic then to version checks instead? And how about LibreSSL in that case? Does BoringSSL need any of the logic in the first place? There’s not really versions of it, so is the target there always current master or something else? Cheers, Dirkjan
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi, > Le 21 janv. 2019 à 17:06, Emeric Brun a écrit : > > Interesting, it would be good to skip the check using the same method. > > We must stay careful to not put the OP_NO_RENEG flag on the client part (when > haproxy connects to server), because reneg from server is authorized > but i think infocbk is called only on frontend/accept side. > > so a patch which do: > > #ifdef SSL_OP_NO_RENEGOTIATION > SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION); > #endif > > without condition during init > > and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix > the issue mentionned about keyupdate and will fix the CVE using the clean way > if the version > of openssl support. > Boringssl does not have SSL_OP_NO_RENEGOTIATION and need KeyUpdate to work. As workaround, SSL_OP_NO_RENEGOTIATION could be set to 0 in openssl-compat.h. ++ Manu
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Emeric, > On 21 Jan 2019, at 08:06, Emeric Brun wrote: > > Interesting, it would be good to skip the check using the same method. > > We must stay careful to not put the OP_NO_RENEG flag on the client part (when > haproxy connects to server), because reneg from server is authorized > but i think infocbk is called only on frontend/accept side. > > so a patch which do: > > #ifdef SSL_OP_NO_RENEGOTIATION > SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION); > #endif > > without condition during init > > and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix > the issue mentionned about keyupdate and will fix the CVE using the clean way > if the version > of openssl support. I have implemented this and attached the patch for it. What do you think of this approach? Cheers, Dirkjan Bussink 0001-BUG-MEDIUM-ssl-Fix-handling-of-TLS-1.3-KeyUpdate-mes.patch Description: Binary data
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On 1/21/19 3:37 PM, Dirkjan Bussink wrote: > Hi all, > >> On 21 Jan 2019, at 02:01, Emeric Brun wrote: >> >> Is there a way to check this is a keyupdate message which trigger the >> callback (and not an other)? > > Sadly there is not. I had taken a look at the OpenSSL code and it triggers > the callback without any additional information available at > https://github.com/openssl/openssl/blob/OpenSSL_1_1_1a/ssl/statem/statem_srvr.c#L4059. > > >> And what happen for natural i/o operation, are they return something >> receiving the message or is this completely >> hide by openssl (i.e. what returns a SSL_read/write for instance when a >> keyupdate has just been received) > > This is completely hidden by OpenSSL and as a library user you don’t see this > message happened as it’s transparently updated. > > I think the other option to consider is that since this logic was added, > OpenSSL has gotten support for flags to control renegotiation and prevent > insecure renegotiation so the question is if we need this check still at all. > I think it can be removed (which is also what NGinx did in the commit that > Janusz mentioned in another email in this thread. From the commit message at > https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c: > > --- > > Following 7319:dcab86115261, as long as SSL_OP_NO_RENEGOTIATION is > defined, it is OpenSSL library responsibility to prevent renegotiation, > so the checks are meaningless. > > Additionally, with TLSv1.3 OpenSSL tends to report SSL_CB_HANDSHAKE_START > at various unexpected moments - notably, on KeyUpdate? messages and > when sending tickets. This change prevents unexpected connection > close on KeyUpdate? messages and when finishing handshake with upcoming > early data changes. > > --- Interesting, it would be good to skip the check using the same method. We must stay careful to not put the OP_NO_RENEG flag on the client part (when haproxy connects to server), because reneg from server is authorized but i think infocbk is called only on frontend/accept side. so a patch which do: #ifdef SSL_OP_NO_RENEGOTIATION SSL_set_options(ctx, SSL_OP_NO_RENEGOTIATION); #endif without condition during init and adding #ifndef SSL_OP_NO_RENEGOTIATION arround the CVE check, should fix the issue mentionned about keyupdate and will fix the CVE using the clean way if the version of openssl support. R, Emeric
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi all, > On 21 Jan 2019, at 02:01, Emeric Brun wrote: > > Is there a way to check this is a keyupdate message which trigger the > callback (and not an other)? Sadly there is not. I had taken a look at the OpenSSL code and it triggers the callback without any additional information available at https://github.com/openssl/openssl/blob/OpenSSL_1_1_1a/ssl/statem/statem_srvr.c#L4059. > And what happen for natural i/o operation, are they return something > receiving the message or is this completely > hide by openssl (i.e. what returns a SSL_read/write for instance when a > keyupdate has just been received) This is completely hidden by OpenSSL and as a library user you don’t see this message happened as it’s transparently updated. I think the other option to consider is that since this logic was added, OpenSSL has gotten support for flags to control renegotiation and prevent insecure renegotiation so the question is if we need this check still at all. I think it can be removed (which is also what NGinx did in the commit that Janusz mentioned in another email in this thread. From the commit message at https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c: --- Following 7319:dcab86115261, as long as SSL_OP_NO_RENEGOTIATION is defined, it is OpenSSL library responsibility to prevent renegotiation, so the checks are meaningless. Additionally, with TLSv1.3 OpenSSL tends to report SSL_CB_HANDSHAKE_START at various unexpected moments - notably, on KeyUpdate? messages and when sending tickets. This change prevents unexpected connection close on KeyUpdate? messages and when finishing handshake with upcoming early data changes. --- Cheers, Dirkjan Bussink
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
pon., 21 sty 2019 o 00:10 Adam Langley napisał(a): > No idea, I'm afraid. If you have a server to test, it looks like one > can use OpenSSL 1.1.1's `openssl s_client` tool to send a KeyUpdate > message by writing "K" on a line by itself. I tested all my servers and I've noticed that nginx is broken too. I am running nginx 1.14.2 with OpenSSL 1.1.1a The nginx source contains exactly the same function as haproxy: https://trac.nginx.org/nginx/browser/nginx/src/event/ngx_event_openssl.c?rev=ebf8c9686b8ce7428f975d8a567935ea3722da70#L850 However, it seems that it might have been fixed in 1.15.2 by this commit: https://trac.nginx.org/nginx/changeset/e3ba4026c02d2c1810fd6f2cecf499fc39dde5ee/nginx/src/event/ngx_event_openssl.c It might also be a better approach for haproxy to just use SSL_OP_NO_RENEGOTIATION if possible. Older OpenSSL versions do no have it, but they also don't support TLS 1.3 And just for reference, I've found Chrome bug with this problem (as I am interested when this will get enabled to keep all my systems updated) https://bugs.chromium.org/p/chromium/issues/detail?id=923685 -- Janusz Dziemidowicz
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Adam, On 1/20/19 10:12 PM, Adam Langley wrote: > KeyUpdate messages are a feature of TLS 1.3 that allows the symmetric > keys of a connection to be periodically rotated. It's > mandatory-to-implement in TLS 1.3, but not mandatory to use. Google > Chrome tried enabling KeyUpdate and promptly broke several sites, at > least some of which are using HAProxy. > > The cause is that HAProxy's code to disable TLS renegotiation[1] is > triggering for TLS 1.3 post-handshake messages. But renegotiation has > been removed in TLS 1.3 and post-handshake messages are no longer > abnormal. Thus I'm attaching a patch to only enforce that check when > the version of a TLS connection is <= 1.2. > > Since sites that are using HAProxy with OpenSSL 1.1.1 will break when > Chrome reenables KeyUpdate without this change, I'd like to suggest it > as a candidate for backporting to stable branches. > > [1] https://github.com/haproxy/haproxy/blob/master/src/ssl_sock.c#L1472 > > > Thank you > > AGL Is there a way to check this is a keyupdate message which trigger the callback (and not an other)? And what happen for natural i/o operation, are they return something receiving the message or is this completely hide by openssl (i.e. what returns a SSL_read/write for instance when a keyupdate has just been received) R, Emeric
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Sun, Jan 20, 2019 at 03:08:23PM -0800, Adam Langley wrote: > On Sun, Jan 20, 2019 at 2:41 PM Willy Tarreau wrote: > > Just out of curiosity, if such out-of-band messages are enabled again in > > 1.3, do you think this might have any particular impacts on something like > > kTLS where the TLS stream is deciphered by the kernel ? I don't know how > > such messages can safely be delivered to userland in this case, nor if > > they're needed there at all. > > No idea, I'm afraid. If you have a server to test, it looks like one > can use OpenSSL 1.1.1's `openssl s_client` tool to send a KeyUpdate > message by writing "K" on a line by itself. Oh thanks for the hint, thus we can also use this to test haproxy :-) > If I were to guess about how in-kernel TLS would work, I would think > that the message would be handled internally and user-space wouldn't > need to know anything about it: it just requires rotating the traffic > keys and, potentially, writing a message in reply--both things that the > kernel can probably handle itself. This indeed makes sense. I don't have any such server to test right now but I've been thinking about studying this possibility for later. Thanks! Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Thank you for clarification. Regard Aleks Ursprüngliche Nachricht Von: Adam Langley Gesendet: 21. Jänner 2019 00:12:59 MEZ An: Aleksandar Lazic CC: haproxy@formilux.org, Willy Tarreau , eb...@haproxy.com Betreff: Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used. On Sun, Jan 20, 2019 at 3:04 PM Aleksandar Lazic wrote: > which refers to > https://www.openssl.org/docs/manmaster/man3/SSL_key_update.html > > instead of the suggested Patch? The SSL_key_update function enqueues a KeyUpdate message to be sent. The problem is that if a /client/ of HAProxy sends a KeyUpdate, HAProxy thinks that it's a pre-TLS 1.3 renegotiation message and drops the connection. Thus the patch seeks to address that. HAProxy may also want to do something like send a KeyUpdate for every x MBs of data sent, or y minutes of time elapsed, but that would be a separate feature. (And one needs to be a little cautious because OpenSSL 1.1.1 will only accept 32 KeyUpdate messages per connection.) Cheers AGL
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Sun, Jan 20, 2019 at 3:04 PM Aleksandar Lazic wrote: > which refers to > https://www.openssl.org/docs/manmaster/man3/SSL_key_update.html > > instead of the suggested Patch? The SSL_key_update function enqueues a KeyUpdate message to be sent. The problem is that if a /client/ of HAProxy sends a KeyUpdate, HAProxy thinks that it's a pre-TLS 1.3 renegotiation message and drops the connection. Thus the patch seeks to address that. HAProxy may also want to do something like send a KeyUpdate for every x MBs of data sent, or y minutes of time elapsed, but that would be a separate feature. (And one needs to be a little cautious because OpenSSL 1.1.1 will only accept 32 KeyUpdate messages per connection.) Cheers AGL
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
On Sun, Jan 20, 2019 at 2:41 PM Willy Tarreau wrote: > Just out of curiosity, if such out-of-band messages are enabled again in > 1.3, do you think this might have any particular impacts on something like > kTLS where the TLS stream is deciphered by the kernel ? I don't know how > such messages can safely be delivered to userland in this case, nor if > they're needed there at all. No idea, I'm afraid. If you have a server to test, it looks like one can use OpenSSL 1.1.1's `openssl s_client` tool to send a KeyUpdate message by writing "K" on a line by itself. If I were to guess about how in-kernel TLS would work, I would think that the message would be handled internally and user-space wouldn't need to know anything about it: it just requires rotating the traffic keys and, potentially, writing a message in reply—both things that the kernel can probably handle itself. Cheers AGL -- Adam Langley a...@imperialviolet.org https://www.imperialviolet.org
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi. As far as I understood the keyupdate https://tools.ietf.org/html/rfc8446 4.6.3 which you refer proper isn't it also a option to use https://wiki.openssl.org/index.php/TLS1.3#Renegotiation which refers to https://www.openssl.org/docs/manmaster/man3/SSL_key_update.html instead of the suggested Patch? Best regards Aleks Ursprüngliche Nachricht Von: Willy Tarreau Gesendet: 20. Jänner 2019 23:41:17 MEZ An: Adam Langley CC: haproxy@formilux.org, eb...@haproxy.com Betreff: Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used. Hi Adam, [ccing Emeric] On Sun, Jan 20, 2019 at 01:12:44PM -0800, Adam Langley wrote: > KeyUpdate messages are a feature of TLS 1.3 that allows the symmetric > keys of a connection to be periodically rotated. It's > mandatory-to-implement in TLS 1.3, but not mandatory to use. Google > Chrome tried enabling KeyUpdate and promptly broke several sites, at > least some of which are using HAProxy. > > The cause is that HAProxy's code to disable TLS renegotiation[1] is > triggering for TLS 1.3 post-handshake messages. But renegotiation has > been removed in TLS 1.3 and post-handshake messages are no longer > abnormal. Interesting! > Thus I'm attaching a patch to only enforce that check when > the version of a TLS connection is <= 1.2. I think that it makes sense. I'll wait for Emeric's check regarding any possibly overlooked impact anywhere else if some other parts would assume that this didn't happen anymore. > Since sites that are using HAProxy with OpenSSL 1.1.1 will break when > Chrome reenables KeyUpdate without this change, I'd like to suggest it > as a candidate for backporting to stable branches. Sure! OpenSSL 1.1.1 is supported on 1.9 and 1.8 so this should be backported there. Just out of curiosity, if such out-of-band messages are enabled again in 1.3, do you think this might have any particular impacts on something like kTLS where the TLS stream is deciphered by the kernel ? I don't know how such messages can safely be delivered to userland in this case, nor if they're needed there at all. > [1] https://github.com/haproxy/haproxy/blob/master/src/ssl_sock.c#L1472 > > > Thank you > > AGL > > -- > Adam Langley a...@imperialviolet.org https://www.imperialviolet.org Thanks! Willy
Re: HAProxy with OpenSSL 1.1.1 breaks when TLS 1.3 KeyUpdate is used.
Hi Adam, [ccing Emeric] On Sun, Jan 20, 2019 at 01:12:44PM -0800, Adam Langley wrote: > KeyUpdate messages are a feature of TLS 1.3 that allows the symmetric > keys of a connection to be periodically rotated. It's > mandatory-to-implement in TLS 1.3, but not mandatory to use. Google > Chrome tried enabling KeyUpdate and promptly broke several sites, at > least some of which are using HAProxy. > > The cause is that HAProxy's code to disable TLS renegotiation[1] is > triggering for TLS 1.3 post-handshake messages. But renegotiation has > been removed in TLS 1.3 and post-handshake messages are no longer > abnormal. Interesting! > Thus I'm attaching a patch to only enforce that check when > the version of a TLS connection is <= 1.2. I think that it makes sense. I'll wait for Emeric's check regarding any possibly overlooked impact anywhere else if some other parts would assume that this didn't happen anymore. > Since sites that are using HAProxy with OpenSSL 1.1.1 will break when > Chrome reenables KeyUpdate without this change, I'd like to suggest it > as a candidate for backporting to stable branches. Sure! OpenSSL 1.1.1 is supported on 1.9 and 1.8 so this should be backported there. Just out of curiosity, if such out-of-band messages are enabled again in 1.3, do you think this might have any particular impacts on something like kTLS where the TLS stream is deciphered by the kernel ? I don't know how such messages can safely be delivered to userland in this case, nor if they're needed there at all. > [1] https://github.com/haproxy/haproxy/blob/master/src/ssl_sock.c#L1472 > > > Thank you > > AGL > > -- > Adam Langley a...@imperialviolet.org https://www.imperialviolet.org Thanks! Willy
