From e6060c5d877e78cfab04cca4f603340540206994 Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Thu, 16 Nov 2017 17:42:52 +0100 Subject: [PATCH] MINOR: SSL: Store the ASN1 representation of client sessions. Instead of storing the SSL_SESSION pointer directly in the struct server, store the ASN1 representation, otherwise, session resumption is broken with TLS 1.3, when multiple outgoing connections want to use the same session. --- include/types/server.h | 6 ++++- src/ssl_sock.c | 53 +++++++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/include/types/server.h b/include/types/server.h index adedca43cd..d22f05ac62 100644 --- a/include/types/server.h +++ b/include/types/server.h @@ -274,7 +274,11 @@ struct server { char *sni_expr; /* Temporary variable to store a sample expression for SNI */ struct { SSL_CTX *ctx; - SSL_SESSION **reused_sess; + struct { + unsigned char *ptr; + int size; + int allocated_size; + } * reused_sess; char *ciphers; /* cipher suite to use if non-null */ int options; /* ssl options */ struct tls_version_filter methods; /* ssl methods */ diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 25f3ecaaff..c652d0adbb 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -3855,19 +3855,35 @@ static int sh_ssl_sess_store(unsigned char *s_id, unsigned char *data, int data_ static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess) { struct connection *conn = SSL_get_app_data(ssl); + struct server *s; - /* check if session was reused, if not store current session on server for reuse */ - if (objt_server(conn->target)->ssl_ctx.reused_sess[tid]) { - SSL_SESSION_free(objt_server(conn->target)->ssl_ctx.reused_sess[tid]); - objt_server(conn->target)->ssl_ctx.reused_sess[tid] = NULL; + s = objt_server(conn->target); + + if (!(s->ssl_ctx.options & SRV_SSL_O_NO_REUSE)) { + int len; + unsigned char *ptr; + + len = i2d_SSL_SESSION(sess, NULL); + if (s->ssl_ctx.reused_sess[tid].ptr && s->ssl_ctx.reused_sess[tid].allocated_size >= len) { + ptr = s->ssl_ctx.reused_sess[tid].ptr; + } else { + free(s->ssl_ctx.reused_sess[tid].ptr); + ptr = s->ssl_ctx.reused_sess[tid].ptr = malloc(len); + s->ssl_ctx.reused_sess[tid].allocated_size = len; + } + if (s->ssl_ctx.reused_sess[tid].ptr) { + s->ssl_ctx.reused_sess[tid].size = i2d_SSL_SESSION(sess, + &ptr); + } + } else { + free(s->ssl_ctx.reused_sess[tid].ptr); + s->ssl_ctx.reused_sess[tid].ptr = NULL; } - if (!(objt_server(conn->target)->ssl_ctx.options & SRV_SSL_O_NO_REUSE)) - objt_server(conn->target)->ssl_ctx.reused_sess[tid] = SSL_get1_session(conn->xprt_ctx); - - return 1; + return 0; } + /* SSL callback used on new session creation */ int sh_ssl_sess_new_cb(SSL *ssl, SSL_SESSION *sess) { @@ -4433,7 +4449,7 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) /* Initiate SSL context for current server */ if (!srv->ssl_ctx.reused_sess) { - if ((srv->ssl_ctx.reused_sess = calloc(1, global.nbthread*sizeof(SSL_SESSION*))) == NULL) { + if ((srv->ssl_ctx.reused_sess = calloc(1, global.nbthread*sizeof(*srv->ssl_ctx.reused_sess))) == NULL) { Alert("Proxy '%s', server '%s' [%s:%d] out of memory.\n", curproxy->id, srv->id, srv->conf.file, srv->conf.line); @@ -4922,10 +4938,15 @@ static int ssl_sock_init(struct connection *conn) } SSL_set_connect_state(conn->xprt_ctx); - if (objt_server(conn->target)->ssl_ctx.reused_sess) { - if(!SSL_set_session(conn->xprt_ctx, objt_server(conn->target)->ssl_ctx.reused_sess[tid])) { - SSL_SESSION_free(objt_server(conn->target)->ssl_ctx.reused_sess[tid]); - objt_server(conn->target)->ssl_ctx.reused_sess[tid] = NULL; + if (objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr) { + const unsigned char *ptr = objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr; + SSL_SESSION *sess = d2i_SSL_SESSION(NULL, &ptr, objt_server(conn->target)->ssl_ctx.reused_sess[tid].size); + if(sess && !SSL_set_session(conn->xprt_ctx, sess)) { + SSL_SESSION_free(sess); + free(objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr); + objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr = NULL; + } else if (sess) { + SSL_SESSION_free(sess); } } @@ -5260,9 +5281,9 @@ int ssl_sock_handshake(struct connection *conn, unsigned int flag) ERR_clear_error(); /* free resumed session if exists */ - if (objt_server(conn->target) && objt_server(conn->target)->ssl_ctx.reused_sess[tid]) { - SSL_SESSION_free(objt_server(conn->target)->ssl_ctx.reused_sess[tid]); - objt_server(conn->target)->ssl_ctx.reused_sess[tid] = NULL; + if (objt_server(conn->target) && objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr) { + free(objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr); + objt_server(conn->target)->ssl_ctx.reused_sess[tid].ptr = NULL; } /* Fail on all other handshake errors */