Hi,
On Mon, Feb 13, Willy Tarreau wrote:
> On Fri, Feb 10, 2017 at 10:54:46AM +0100, Emmanuel Hocdet wrote:
> > Hi,
> >
> > > Le 10 févr. 2017 à 10:07, Jarno Huuskonen <[email protected]> a
> > > écrit :
> > >
> > > Hi,
> > >
> > > On Wed, Feb 08, Jarno Huuskonen wrote:
> > >> On Tue, Feb 07, Emmanuel Hocdet wrote:
> > >>> I'm not able to reproduce this crash with current 1.8dev and openssl
> > >>> 1.0.2j.
> > >>
> > >> OK, thanks for checking. I'll try to compile openssl-1.0.2/openssl-1.1.0
> > >> and
> > >> test with those to see if it's specific to openssl that comes w/centos7.
> > >
> > > I just tested with 1.0.1u / 1.1.0c (compiled from source) -> no crash.
> > >
> > >> Do you have access to a centos7 vm (are you able to reproduce with
> > >> openssl that comes w/centos7) ?
> > >
> > > I still get a crash with openssl that comes with centos7. haproxy -vv
> > > reports version as:
> > > Built with OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> > > Running on OpenSSL version : OpenSSL 1.0.1e-fips 11 Feb 2013
> > >
> > > Is commit 405ff31e31eb1cbdc76ba0d93c6db4c7a3fd497a boringssl specific ?
> > >
> >
> > No, it's a cleanup for current openssl versions: using API instead change
> > internal states.
> > I doubt that the problem is directly related to this commit.
>
> Hmmm wait a minute, there's bug in this commit :
>
> @@ -4022,15 +4022,15 @@ static void ssl_sock_shutw(struct connection *conn,
> int
> {
> if (conn->flags & CO_FL_HANDSHAKE)
> return;
> + if (!clean)
> + /* don't sent notify on SSL_shutdown */
> + SSL_CTX_set_quiet_shutdown(conn->xprt_ctx, 1);
>
> Here we call SSL_CTX_set_quiet_shutdown() (which takes an SSL_CTX *)
> instead of SSL_set_quiet_shutdown() which applies to an SSL*.
> Unfortunately conn->xprt_ctx is a void* so there is no warning, and
> who knows where the flag is set. It may overwrite any internal field,
> pointer etc...
>
> Jarno, please try this, I'm pretty sure it will fix the problem for
> you :
Yes, this patch fixes the crash for me.
(also reverting commit 405ff31e31eb1cbdc76ba0d93c6db4c7a3fd497a ->
no crash).
Thanks,
-Jarno
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index 232a497..e7eb5df 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -4177,7 +4177,7 @@ static void ssl_sock_shutw(struct connection *conn, int
> cl
> return;
> if (!clean)
> /* don't sent notify on SSL_shutdown */
> - SSL_CTX_set_quiet_shutdown(conn->xprt_ctx, 1);
> + SSL_set_quiet_shutdown(conn->xprt_ctx, 1);
> /* no handshake was in progress, try a clean ssl shutdown */
> if (SSL_shutdown(conn->xprt_ctx) <= 0) {
> /* Clear openssl global errors stack */
>
> Thanks,
> Willy
--
Jarno Huuskonen
