[openssl-commits] [openssl] master update
kaduk at mit.edu
kaduk at mit.edu
Mon Oct 30 15:28:29 UTC 2017
The branch master has been updated
via 3be08e301146fc449315b7061d086edddb186850 (commit)
via 0e6161bcae451ed17346c51db1ddc61fea5f3ec2 (commit)
via 1fcb4e4d521971caccb61df215541bf55f7ca9a5 (commit)
from ce01b1896b57f3568c5af71e5b2912280171ba6e (commit)
- Log -----------------------------------------------------------------
commit 3be08e301146fc449315b7061d086edddb186850
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date: Wed Oct 18 08:05:57 2017 -0500
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 at openssl.org>
Reviewed-by: Paul Dale <paul.dale at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
commit 0e6161bcae451ed17346c51db1ddc61fea5f3ec2
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date: Tue Oct 17 15:28:42 2017 -0500
Normalize on session_ctx for stats where possible
For client SSL objects and before any callbacks have had a chance
to be called, we can write the stats accesses using the session_ctx,
which makes sense given that these values are all prefixed with
"sess_".
For servers after a client_hello or servername callback has been
called, retain the existing behavior of modifying the statistics
for the current (non-session) context. This has some value,
in that it allows the statistics to be viewed on a per-vhost level.
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Paul Dale <paul.dale at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
commit 1fcb4e4d521971caccb61df215541bf55f7ca9a5
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date: Tue Oct 17 14:46:58 2017 -0500
Use atomics for SSL_CTX statistics
It is expected that SSL_CTX objects are shared across threads,
and as such we are responsible for ensuring coherent data accesses.
Aligned integer accesses ought to be atomic already on all supported
architectures, but we can be formally correct.
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Paul Dale <paul.dale at oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4549)
-----------------------------------------------------------------------
Summary of changes:
ssl/ssl_lib.c | 47 ++++++++++++++++++++++++++++++++---------------
ssl/ssl_sess.c | 19 ++++++++++++-------
ssl/statem/extensions.c | 15 ++++++++++++++-
ssl/statem/statem_clnt.c | 5 +++--
ssl/statem/statem_lib.c | 26 +++++++++++++++++++-------
5 files changed, 80 insertions(+), 32 deletions(-)
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 4435efd..c151e7e 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2180,6 +2180,7 @@ LHASH_OF(SSL_SESSION) *SSL_CTX_sessions(SSL_CTX *ctx)
long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
{
long l;
+ int i;
/* For some cases with ctx == NULL perform syntax checks */
if (ctx == NULL) {
switch (cmd) {
@@ -2234,27 +2235,40 @@ long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
case SSL_CTRL_SESS_NUMBER:
return lh_SSL_SESSION_num_items(ctx->sessions);
case SSL_CTRL_SESS_CONNECT:
- return ctx->stats.sess_connect;
+ return CRYPTO_atomic_read(&ctx->stats.sess_connect, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_CONNECT_GOOD:
- return ctx->stats.sess_connect_good;
+ return CRYPTO_atomic_read(&ctx->stats.sess_connect_good, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_CONNECT_RENEGOTIATE:
- return ctx->stats.sess_connect_renegotiate;
+ return CRYPTO_atomic_read(&ctx->stats.sess_connect_renegotiate, &i,
+ ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_ACCEPT:
- return ctx->stats.sess_accept;
+ return CRYPTO_atomic_read(&ctx->stats.sess_accept, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_ACCEPT_GOOD:
- return ctx->stats.sess_accept_good;
+ return CRYPTO_atomic_read(&ctx->stats.sess_accept_good, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_ACCEPT_RENEGOTIATE:
- return ctx->stats.sess_accept_renegotiate;
+ return CRYPTO_atomic_read(&ctx->stats.sess_accept_renegotiate, &i,
+ ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_HIT:
- return ctx->stats.sess_hit;
+ return CRYPTO_atomic_read(&ctx->stats.sess_hit, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_CB_HIT:
- return ctx->stats.sess_cb_hit;
+ return CRYPTO_atomic_read(&ctx->stats.sess_cb_hit, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_MISSES:
- return ctx->stats.sess_miss;
+ return CRYPTO_atomic_read(&ctx->stats.sess_miss, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_TIMEOUTS:
- return ctx->stats.sess_timeout;
+ return CRYPTO_atomic_read(&ctx->stats.sess_timeout, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_SESS_CACHE_FULL:
- return ctx->stats.sess_cache_full;
+ return CRYPTO_atomic_read(&ctx->stats.sess_cache_full, &i, ctx->lock)
+ ? i : 0;
case SSL_CTRL_MODE:
return (ctx->mode |= larg);
case SSL_CTRL_CLEAR_MODE:
@@ -3205,11 +3219,14 @@ void ssl_update_cache(SSL *s, int mode)
/* auto flush every 255 connections */
if ((!(i & SSL_SESS_CACHE_NO_AUTO_CLEAR)) && ((i & mode) == mode)) {
- if ((((mode & SSL_SESS_CACHE_CLIENT)
- ? s->session_ctx->stats.sess_connect_good
- : s->session_ctx->stats.sess_accept_good) & 0xff) == 0xff) {
+ int *stat, val;
+ if (mode & SSL_SESS_CACHE_CLIENT)
+ stat = &s->session_ctx->stats.sess_connect_good;
+ else
+ stat = &s->session_ctx->stats.sess_accept_good;
+ if (CRYPTO_atomic_read(stat, &val, s->session_ctx->lock)
+ && (val & 0xff) == 0xff)
SSL_CTX_flush_sessions(s->session_ctx, (unsigned long)time(NULL));
- }
}
}
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 9f5b016..c8d1cc3 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -461,7 +461,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
/* This is used only by servers. */
SSL_SESSION *ret = NULL;
- int fatal = 0;
+ int fatal = 0, discard;
int try_session_cache = 0;
TICKET_RETURN r;
@@ -512,7 +512,8 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
}
CRYPTO_THREAD_unlock(s->session_ctx->lock);
if (ret == NULL)
- s->session_ctx->stats.sess_miss++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_miss, 1, &discard,
+ s->session_ctx->lock);
}
if (try_session_cache &&
@@ -524,7 +525,8 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
©);
if (ret != NULL) {
- s->session_ctx->stats.sess_cb_hit++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_cb_hit, 1, &discard,
+ s->session_ctx->lock);
/*
* Increment reference count now if the session callback asks us
@@ -589,7 +591,8 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
}
if (ret->timeout < (long)(time(NULL) - ret->time)) { /* timeout */
- s->session_ctx->stats.sess_timeout++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_timeout, 1, &discard,
+ s->session_ctx->lock);
if (try_session_cache) {
/* session was from the cache, so remove it */
SSL_CTX_remove_session(s->session_ctx, ret);
@@ -617,7 +620,8 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
s->session = ret;
}
- s->session_ctx->stats.sess_hit++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_hit, 1, &discard,
+ s->session_ctx->lock);
s->verify_result = s->session->verify_result;
return 1;
@@ -646,7 +650,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al)
int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
{
- int ret = 0;
+ int ret = 0, discard;
SSL_SESSION *s;
/*
@@ -713,7 +717,8 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
if (!remove_session_lock(ctx, ctx->session_cache_tail, 0))
break;
else
- ctx->stats.sess_cache_full++;
+ CRYPTO_atomic_add(&ctx->stats.sess_cache_full, 1, &discard,
+ ctx->lock);
}
}
}
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index e16b75f..3153d3b 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;
@@ -850,6 +850,19 @@ static int final_server_name(SSL *s, unsigned int context, int sent,
}
/*
+ * 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.
* Also, if this is not a resumption, create a new session ID
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index af42bcb..0a68264 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1266,7 +1266,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
unsigned int compression;
unsigned int sversion;
unsigned int context;
- int protverr;
+ int protverr, discard;
RAW_EXTENSION *extensions = NULL;
#ifndef OPENSSL_NO_COMP
SSL_COMP *comp;
@@ -1430,7 +1430,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
|| (SSL_IS_TLS13(s)
&& s->session->ext.tick_identity
!= TLSEXT_PSK_BAD_IDENTITY)) {
- s->ctx->stats.sess_miss++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_miss, 1, &discard,
+ s->session_ctx->lock);
if (!ssl_get_new_session(s, 0)) {
goto f_err;
}
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index e36f98a..6db8816 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -111,7 +111,9 @@ int tls_setup_handshake(SSL *s)
return 0;
}
if (SSL_IS_FIRST_HANDSHAKE(s)) {
- s->ctx->stats.sess_accept++;
+ /* N.B. s->session_ctx == s->ctx here */
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_accept, 1, &i,
+ s->session_ctx->lock);
} else if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
/* Renegotiation is disabled */
ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
@@ -128,15 +130,20 @@ int tls_setup_handshake(SSL *s)
ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
return 0;
} else {
- s->ctx->stats.sess_accept_renegotiate++;
+ /* N.B. s->ctx may not equal s->session_ctx */
+ CRYPTO_atomic_add(&s->ctx->stats.sess_accept_renegotiate, 1, &i,
+ s->ctx->lock);
s->s3->tmp.cert_request = 0;
}
} else {
+ int discard;
if (SSL_IS_FIRST_HANDSHAKE(s))
- s->ctx->stats.sess_connect++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect, 1, &discard,
+ s->session_ctx->lock);
else
- s->ctx->stats.sess_connect_renegotiate++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect_renegotiate,
+ 1, &discard, s->session_ctx->lock);
/* mark client_random uninitialized */
memset(s->s3->client_random, 0, sizeof(s->s3->client_random));
@@ -991,6 +998,7 @@ unsigned long ssl3_output_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk,
*/
WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs)
{
+ int discard;
void (*cb) (const SSL *ssl, int type, int val) = NULL;
#ifndef OPENSSL_NO_SCTP
@@ -1027,7 +1035,9 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs)
if (s->server) {
ssl_update_cache(s, SSL_SESS_CACHE_SERVER);
- s->ctx->stats.sess_accept_good++;
+ /* N.B. s->ctx may not equal s->session_ctx */
+ CRYPTO_atomic_add(&s->ctx->stats.sess_accept_good, 1, &discard,
+ s->ctx->lock);
s->handshake_func = ossl_statem_accept;
} else {
/*
@@ -1037,10 +1047,12 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs)
if (!SSL_IS_TLS13(s))
ssl_update_cache(s, SSL_SESS_CACHE_CLIENT);
if (s->hit)
- s->ctx->stats.sess_hit++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_hit, 1, &discard,
+ s->session_ctx->lock);
s->handshake_func = ossl_statem_connect;
- s->ctx->stats.sess_connect_good++;
+ CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect_good, 1,
+ &discard, s->session_ctx->lock);
}
if (s->info_callback != NULL)
More information about the openssl-commits
mailing list