On 1/21/19 3:37 PM, Dirkjan Bussink wrote: > Hi all, > >> On 21 Jan 2019, at 02:01, Emeric Brun <eb...@haproxy.com> 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
