Re: haproxy 1.6.0 crashes

[Willy Tarreau]

On Tue, Oct 20, 2015 at 01:59:52PM +0200, Willy Tarreau wrote:
> Then my understanding is that we should instead proceed differently :
>   - the cert is generated. It gets a refcount = 1.
>   - we assign it to the SSL. Its refcount becomes two.
>   - we try to insert it into the tree. The tree will handle its freeing
>     using SSL_CTX_free() during eviction.
>   - if we can't insert into the tree because the tree is disabled, then
>     we have to call SSL_CTX_free() ourselves, then we'd rather do it
>     immediately. It will more closely mimmick the case where the cert
>     is added to the tree and immediately evicted by concurrent activity
>     on the cache.
>   - we never have to call SSL_CTX_free() during ssl_sock_close() because
>     the SSL session only relies on openssl doing the right thing based on
>     the refcount only.
>   - thus we never need to know how the cert was created since the
>     SSL_CTX_free() is either guaranteed or already done for generated
>     certs, and this protects other ones against any accidental call to
>     SSL_CTX_free() without having to track where the cert comes from.

This patch does this, and based on my understanding of your explanations,
it should do the right thing and be safe all the time. What's your opinion ?

Thanks,
Willy


diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 5319532..4eed2ea 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1201,9 +1201,13 @@ ssl_sock_generate_certificate(const char *servername, 
struct bind_conf *bind_con
                        ssl_ctx = ssl_sock_do_create_cert(servername, serial, 
bind_conf, ssl);
                        lru64_commit(lru, ssl_ctx, cacert, 0, (void (*)(void 
*))SSL_CTX_free);
                }
+               SSL_set_SSL_CTX(ssl, ssl_ctx);
        }
-       else
+       else {
                ssl_ctx = ssl_sock_do_create_cert(servername, serial, 
bind_conf, ssl);
+               SSL_set_SSL_CTX(ssl, ssl_ctx);
+               SSL_CTX_free(ssl_ctx);
+       }
        return ssl_ctx;
 }
 
@@ -1271,7 +1275,6 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, 
struct bind_conf *s)
                if (s->generate_certs &&
                    (ctx = ssl_sock_generate_certificate(servername, s, ssl))) {
                        /* switch ctx */
-                       SSL_set_SSL_CTX(ssl, ctx);
                        return SSL_TLSEXT_ERR_OK;
                }
                return (s->strict_sni ?
@@ -3123,13 +3126,6 @@ static int ssl_sock_from_buf(struct connection *conn, 
struct buffer *buf, int fl
 static void ssl_sock_close(struct connection *conn) {
 
        if (conn->xprt_ctx) {
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-               if (!ssl_ctx_lru_tree && objt_listener(conn->target)) {
-                       SSL_CTX *ctx = SSL_get_SSL_CTX(conn->xprt_ctx);
-                       if (ctx != 
objt_listener(conn->target)->bind_conf->default_ctx)
-                               SSL_CTX_free(ctx);
-               }
-#endif
                SSL_free(conn->xprt_ctx);
                conn->xprt_ctx = NULL;
                sslconns--;