Le 15/10/2015 16:50, Christopher Faulet a écrit :
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,
After our discussion on this bug, I reworked my patch to use
SSL_CTX_set_ex_data/SSL_CTX_get_ex_data functions.
Willy, I let you choose the patch you prefer :)
PS: I do some checks and AFAIK, it works. But a double-check will not to
be too much...
--
Christopher Faulet
>From 171bb9ca0522c12d2f4f9a105c557e01c0011ecc Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@qualys.com>
Date: Thu, 15 Oct 2015 15:29:34 +0200
Subject: [PATCH] BUG: ssl: Fix conditions to release SSL_CTX object when a
connection is closed
When a SSL connection is closed, if its associated SSL_CTX object was generated
and if it was not cached[1], then it must be freed[2].
But, we must check that it was generated. And this check is buggy when multiple
certificates are used on the same bind line. We check that the SSL_CTX object is
not the default one (ssl_ctx != bind_conf->default_ctx). But it is not enough to
determine if it was generated or not. We should also 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 erroneously released when a connection
is closed.
This commit fix the bug. Now, when a SSL_CTX object is generated, we mark it
using SSL_CTX_set_ex_data function. Then when the connection is closed, we check
the presence of this mark using SSL_CTX_get_ex_data functon. If the SSL_CTX
object is marked and not cached (because ssl_ctx_lru_tree is NULL), it is freed.
More information on this bug can be found on the HAProxy mailing-list[3]
[1] This happens when the cache to store generated SSL_CTX object does not exist
(ssl_ctx_lru_tree == NULL) because the 'tune.ssl.ssl-ctx-cache-size' option is
set to 0. This cache is also NULL when the dynamic generation of SSL
certificates is used on no listener.
[2] Cached SSL_CTX objects are released when the cache is destroyed (during
HAProxy shutdown) or when one of them is evicted from the cache.
[3] https://www.mail-archive.com/haproxy@formilux.org/msg19937.html
---
src/ssl_sock.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 5319532..35a3edf 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -153,8 +153,10 @@ static char *x509v3_ext_values[X509V3_EXT_SIZE] = {
};
/* LRU cache to store generated certificate */
-static struct lru64_head *ssl_ctx_lru_tree = NULL;
-static unsigned int ssl_ctx_lru_seed = 0;
+static struct lru64_head *ssl_ctx_lru_tree = NULL;
+static unsigned int ssl_ctx_lru_seed = 0;
+static int gen_ssl_ctx_ptr_index = -1;
+static int is_gen_ssl_ctx = 1;
#endif // SSL_CTRL_SET_TLSEXT_HOSTNAME
#if (defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP)
@@ -1128,6 +1130,9 @@ ssl_sock_do_create_cert(const char *servername, unsigned int serial,
}
#endif
end:
+ if (!SSL_CTX_set_ex_data(ssl_ctx, gen_ssl_ctx_ptr_index, &is_gen_ssl_ctx))
+ goto mkcert_error;
+
return ssl_ctx;
mkcert_error:
@@ -3124,11 +3129,10 @@ 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);
- }
+ SSL_CTX *ctx = SSL_get_SSL_CTX(conn->xprt_ctx);
+ int *flag = SSL_CTX_get_ex_data(ctx, gen_ssl_ctx_ptr_index);
+ if (!ssl_ctx_lru_tree && flag != NULL && *flag == is_gen_ssl_ctx)
+ SSL_CTX_free(ctx);
#endif
SSL_free(conn->xprt_ctx);
conn->xprt_ctx = NULL;
@@ -5392,6 +5396,11 @@ static void __ssl_sock_init(void)
#ifndef OPENSSL_NO_DH
ssl_dh_ptr_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL);
#endif
+
+#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
+ gen_ssl_ctx_ptr_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL);
+#endif
+
}
__attribute__((destructor))
--
2.4.3
