Commit 3be08e30 authored Oct 18, 2017 by Benjamin Kaduk Committed by Ben Kaduk Oct 30, 2017

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 <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)

parent 0e6161bc

ssl/statem/extensions.c

+14 −1

Original line number	Diff line number	Diff line
		@@ -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.