BUG/MAJOR: ssl: OpenSSL context is stored in non-reserved memory slot

We never saw unexplicated crash with SSL, so I suppose that we are
luck, or the slot 0 is always reserved. Anyway the usage of the macro
SSL_get_app_data() and SSL_set_app_data() seem wrong. This patch change
the deprecated functions SSL_get_app_data() and SSL_set_app_data()
by the new functions SSL_get_ex_data() and SSL_set_ex_data(), and
it reserves the slot in the SSL memory space.

For information, this is the two declaration which seems wrong or
incomplete in the OpenSSL ssl.h file. We can see the usage of the
slot 0 whoch is hardcoded, but never reserved.

   #define SSL_set_app_data(s,arg)     (SSL_set_ex_data(s,0,(char *)arg))
   #define SSL_get_app_data(s)      (SSL_get_ex_data(s,0))

This patch must be backported at least in 1.8, maybe in other versions.
This commit is contained in:
Thierry FOURNIER 2018-06-17 21:37:05 +02:00 committed by Willy Tarreau
parent 16ff050478
commit 28962c9941

View File

@ -260,6 +260,7 @@ struct ssl_capture {
}; };
struct pool_head *pool_head_ssl_capture = NULL; struct pool_head *pool_head_ssl_capture = NULL;
static int ssl_capture_ptr_index = -1; static int ssl_capture_ptr_index = -1;
static int ssl_app_data_index = -1;
static int ssl_pkey_info_index = -1; static int ssl_pkey_info_index = -1;
@ -824,7 +825,7 @@ static int ssl_tlsext_ticket_key_cb(SSL *s, unsigned char key_name[16], unsigned
int i; int i;
int ret = -1; /* error by default */ int ret = -1; /* error by default */
conn = SSL_get_app_data(s); conn = SSL_get_ex_data(s, ssl_app_data_index);
ref = objt_listener(conn->target)->bind_conf->keys_ref; ref = objt_listener(conn->target)->bind_conf->keys_ref;
HA_RWLOCK_RDLOCK(TLSKEYS_REF_LOCK, &ref->lock); HA_RWLOCK_RDLOCK(TLSKEYS_REF_LOCK, &ref->lock);
@ -1388,7 +1389,7 @@ out:
void ssl_sock_infocbk(const SSL *ssl, int where, int ret) void ssl_sock_infocbk(const SSL *ssl, int where, int ret)
{ {
struct connection *conn = SSL_get_app_data(ssl); struct connection *conn = SSL_get_ex_data(ssl, ssl_app_data_index);
BIO *write_bio; BIO *write_bio;
(void)ret; /* shut gcc stupid warning */ (void)ret; /* shut gcc stupid warning */
@ -1426,7 +1427,7 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
int err, depth; int err, depth;
ssl = X509_STORE_CTX_get_ex_data(x_store, SSL_get_ex_data_X509_STORE_CTX_idx()); ssl = X509_STORE_CTX_get_ex_data(x_store, SSL_get_ex_data_X509_STORE_CTX_idx());
conn = SSL_get_app_data(ssl); conn = SSL_get_ex_data(ssl, ssl_app_data_index);
conn->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE; conn->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
@ -1574,7 +1575,7 @@ void ssl_sock_msgcbk(int write_p, int version, int content_type, const void *buf
/* test heartbeat received (write_p is set to 0 /* test heartbeat received (write_p is set to 0
for a received record) */ for a received record) */
if ((content_type == TLS1_RT_HEARTBEAT) && (write_p == 0)) { if ((content_type == TLS1_RT_HEARTBEAT) && (write_p == 0)) {
struct connection *conn = SSL_get_app_data(ssl); struct connection *conn = SSL_get_ex_data(ssl, ssl_app_data_index);
const unsigned char *p = buf; const unsigned char *p = buf;
unsigned int payload; unsigned int payload;
@ -1902,7 +1903,7 @@ static int
ssl_sock_generate_certificate_from_conn(struct bind_conf *bind_conf, SSL *ssl) ssl_sock_generate_certificate_from_conn(struct bind_conf *bind_conf, SSL *ssl)
{ {
unsigned int key; unsigned int key;
struct connection *conn = SSL_get_app_data(ssl); struct connection *conn = SSL_get_ex_data(ssl, ssl_app_data_index);
conn_get_to_addr(conn); conn_get_to_addr(conn);
if (conn->flags & CO_FL_ADDR_TO_SET) { if (conn->flags & CO_FL_ADDR_TO_SET) {
@ -2106,7 +2107,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
int allow_early = 0; int allow_early = 0;
int i; int i;
conn = SSL_get_app_data(ssl); conn = SSL_get_ex_data(ssl, ssl_app_data_index);
s = objt_listener(conn->target)->bind_conf; s = objt_listener(conn->target)->bind_conf;
if (s->ssl_conf.early_data) if (s->ssl_conf.early_data)
@ -3890,7 +3891,7 @@ static int sh_ssl_sess_store(unsigned char *s_id, unsigned char *data, int data_
/* SSL callback used when a new session is created while connecting to a server */ /* SSL callback used when a new session is created while connecting to a server */
static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess) static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess)
{ {
struct connection *conn = SSL_get_app_data(ssl); struct connection *conn = SSL_get_ex_data(ssl, ssl_app_data_index);
struct server *s; struct server *s;
s = objt_server(conn->target); s = objt_server(conn->target);
@ -4387,7 +4388,7 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
return ok; return ok;
ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
conn = SSL_get_app_data(ssl); conn = SSL_get_ex_data(ssl, ssl_app_data_index);
/* We're checking if the provided hostnames match the desired one. The /* We're checking if the provided hostnames match the desired one. The
* desired hostname comes from the SNI we presented if any, or if not * desired hostname comes from the SNI we presented if any, or if not
@ -4962,7 +4963,7 @@ static int ssl_sock_init(struct connection *conn)
} }
/* set connection pointer */ /* set connection pointer */
if (!SSL_set_app_data(conn->xprt_ctx, conn)) { if (!SSL_set_ex_data(conn->xprt_ctx, ssl_app_data_index, conn)) {
SSL_free(conn->xprt_ctx); SSL_free(conn->xprt_ctx);
conn->xprt_ctx = NULL; conn->xprt_ctx = NULL;
if (may_retry--) { if (may_retry--) {
@ -5021,7 +5022,7 @@ static int ssl_sock_init(struct connection *conn)
} }
/* set connection pointer */ /* set connection pointer */
if (!SSL_set_app_data(conn->xprt_ctx, conn)) { if (!SSL_set_ex_data(conn->xprt_ctx, ssl_app_data_index, conn)) {
SSL_free(conn->xprt_ctx); SSL_free(conn->xprt_ctx);
conn->xprt_ctx = NULL; conn->xprt_ctx = NULL;
if (may_retry--) { if (may_retry--) {
@ -8964,6 +8965,7 @@ static void __ssl_sock_init(void)
#if (OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL && !defined LIBRESSL_VERSION_NUMBER) #if (OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL && !defined LIBRESSL_VERSION_NUMBER)
sctl_ex_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_sctl_free_func); sctl_ex_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_sctl_free_func);
#endif #endif
ssl_app_data_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
ssl_capture_ptr_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_capture_free_func); ssl_capture_ptr_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_capture_free_func);
ssl_pkey_info_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL); ssl_pkey_info_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL);
sample_register_fetches(&sample_fetch_keywords); sample_register_fetches(&sample_fetch_keywords);