Hi,
Here is a proper patch to fix the recent bug reported on haproxy 1.6.0
when SNI is used.
Willy, I didn't wait your reply to speed-up the code review. But if
there is any problem with this patch, let me know.
Regards,
--
Christopher Faulet
>From c89e1256113aa36826b00706094ccde98490684d Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Thu, 15 Oct 2015 15:29:34 +0200
Subject: [PATCH] BUG: ssl: Fix conditions to release SSL_CTX when a SSL
connection is closed
When a SSL connection is closed, if a SSL certificate was generated and if there
is no cache to store it, it must be released. But, we must be sure that this is
a generated certificate and not one of the default ones.
This check was buggy when multiple certificates were used on the same bind
line. We checked that the certificate is not the default one (ssl_ctx !=
bind_conf->default_ctx). But we should also have to check it against SNI
certificates (bind_conf->sni_ctx and bind_conf->sni_wc_ctx). This bug was
introducted with the commit d2cab92 and it leads to a segfault in certain
circumstances. A SNI certificate can be released when a connection is closed and
then reused for a new one.
This commit fix the bug. Now, when a SSL certificate is generated, we set the
flag 'CO_FL_DYN_SSL_CTX' on the corresponding connection. So, when a connection
is closed, if this flag is set and if there is no cache to store the generated
SSL certificates, we release it. This way to do is speedier than to compare a
SSL certifacte to the default ones.
---
include/types/connection.h | 5 ++++-
src/ssl_sock.c | 10 +++++++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/types/connection.h b/include/types/connection.h
index dfbff6a..070d779 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -122,7 +122,10 @@ enum {
/* This connection may not be shared between clients */
CO_FL_PRIVATE = 0x10000000,
- /* unused : 0x20000000, 0x40000000 */
+ /* A dynamically generated SSL certificate was used for this connection */
+ CO_FL_DYN_SSL_CTX = 0x20000000,
+
+ /* unused : 0x40000000 */
/* This last flag indicates that the transport layer is used (for instance
* by logs) and must not be cleared yet. The last call to conn_xprt_close()
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 5319532..2829af8 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1232,6 +1232,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s)
ctx = ssl_sock_get_generated_cert(serial, s);
if (ctx) {
/* switch ctx */
+ conn->flags |= CO_FL_DYN_SSL_CTX;
SSL_set_SSL_CTX(ssl, ctx);
return SSL_TLSEXT_ERR_OK;
}
@@ -1271,6 +1272,9 @@ 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 */
+ struct connection *conn = (struct connection *)SSL_get_app_data(ssl);
+
+ conn->flags |= CO_FL_DYN_SSL_CTX;
SSL_set_SSL_CTX(ssl, ctx);
return SSL_TLSEXT_ERR_OK;
}
@@ -3124,11 +3128,11 @@ 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)) {
+ if ((conn->flags & CO_FL_DYN_SSL_CTX) && !ssl_ctx_lru_tree) {
SSL_CTX *ctx = SSL_get_SSL_CTX(conn->xprt_ctx);
- if (ctx != objt_listener(conn->target)->bind_conf->default_ctx)
- SSL_CTX_free(ctx);
+ SSL_CTX_free(ctx);
}
+ conn->flags &= ~CO_FL_DYN_SSL_CTX,
#endif
SSL_free(conn->xprt_ctx);
conn->xprt_ctx = NULL;
--
2.4.3
