From 3be08e301146fc449315b7061d086edddb186850 Mon Sep 17 00:00:00 2001 From: Benjamin Kaduk Date: Wed, 18 Oct 2017 08:05:57 -0500 Subject: Provide SSL_CTX.stats.sess_accept for switched ctxs We currently increment the SSL_CTX stats.sess_accept field in tls_setup_handshake(), which is invoked from the state machine well before ClientHello processing would have had a chance to switch the SSL_CTX attached to the SSL object due to a provided SNI value. However, stats.sess_accept_good is incremented in tls_finish_handshake(), and uses the s->ctx.stats field (i.e., the new SSL_CTX that was switched to as a result of SNI processing). This leads to the confusing (nonsensical) situation where stats.sess_accept_good is larger than stats.sess_accept, as the "sess_accept" value was counted on the s->session_ctx. In order to provide some more useful numbers, increment s->ctx.stats.sess_accept after SNI processing if the SNI processing changed s->ctx to differ from s->session_ctx. To preserve the property that any given accept is counted only once, make the corresponding decrement to s->session_ctx.stats.sess_accept when doing so. Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/4549) --- ssl/statem/extensions.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index e16b75f683..3153d3b376 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -832,7 +832,7 @@ static int init_server_name(SSL *s, unsigned int context) static int final_server_name(SSL *s, unsigned int context, int sent, int *al) { - int ret = SSL_TLSEXT_ERR_NOACK; + int ret = SSL_TLSEXT_ERR_NOACK, discard; int altmp = SSL_AD_UNRECOGNIZED_NAME; int was_ticket = (SSL_get_options(s) & SSL_OP_NO_TICKET) == 0; @@ -849,6 +849,19 @@ static int final_server_name(SSL *s, unsigned int context, int sent, s->session->ext.hostname = NULL; } + /* + * If we switched contexts (whether here or in the client_hello callback), + * move the sess_accept increment from the session_ctx to the new + * context, to avoid the confusing situation of having sess_accept_good + * exceed sess_accept (zero) for the new context. + */ + if (SSL_IS_FIRST_HANDSHAKE(s) && s->ctx != s->session_ctx) { + CRYPTO_atomic_add(&s->ctx->stats.sess_accept, 1, &discard, + s->ctx->lock); + CRYPTO_atomic_add(&s->session_ctx->stats.sess_accept, -1, &discard, + s->session_ctx->lock); + } + /* * If we're expecting to send a ticket, and tickets were previously enabled, * and now tickets are disabled, then turn off expected ticket. -- cgit v1.2.3