Commit 1fcb4e4d authored by Benjamin Kaduk's avatar Benjamin Kaduk Committed by Ben Kaduk
Browse files

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

ssl/ssl_lib.c

+32 −15
Original line number Diff line number Diff line
@@ -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,13 +3219,16 @@ 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));

    }

}

}



const SSL_METHOD *SSL_CTX_get_ssl_method(SSL_CTX *ctx)

{

ssl/ssl_sess.c

+12 −7
Original line number Diff line number Diff line
@@ -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) 
                                             &copy);



        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);

            }

        }

    }

ssl/statem/statem_clnt.c

+3 −2
Original line number Diff line number Diff line
@@ -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->ctx->stats.sess_miss, 1, &discard,

                              s->ctx->lock);

            if (!ssl_get_new_session(s, 0)) {

                goto f_err;

            }

ssl/statem/statem_lib.c

+15 −7
Original line number Diff line number Diff line
@@ -111,7 +111,7 @@ int tls_setup_handshake(SSL *s) 
            return 0;

        }

        if (SSL_IS_FIRST_HANDSHAKE(s)) {

            s->ctx->stats.sess_accept++;

            CRYPTO_atomic_add(&s->ctx->stats.sess_accept, 1, &i, s->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 +128,19 @@ 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++;

            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->ctx->stats.sess_connect, 1, &discard,

                              s->ctx->lock);

        else

            s->ctx->stats.sess_connect_renegotiate++;

            CRYPTO_atomic_add(&s->ctx->stats.sess_connect_renegotiate, 1,

                              &discard, s->ctx->lock);



        /* mark client_random uninitialized */

        memset(s->s3->client_random, 0, sizeof(s->s3->client_random));
@@ -991,6 +995,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 +1032,8 @@ 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++;

            CRYPTO_atomic_add(&s->ctx->stats.sess_accept_good, 1, &discard,

                              s->ctx->lock);

            s->handshake_func = ossl_statem_accept;

        } else {

            /*
@@ -1037,10 +1043,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->ctx->stats.sess_hit, 1, &discard,

                                  s->ctx->lock);



            s->handshake_func = ossl_statem_connect;

            s->ctx->stats.sess_connect_good++;

            CRYPTO_atomic_add(&s->ctx->stats.sess_connect_good, 1, &discard,

                              s->ctx->lock);

        }



        if (s->info_callback != NULL)