Commit 5d61491c authored by Matt Caswell's avatar Matt Caswell
Browse files

Fix new_session_cb calls in TLSv1.3 



If a new_session_cb is set then it was only ever getting invoked if !s->hit
is true. This is sensible for <=TLSv1.2 but does not work for TLSv1.3.

Fixes #4045

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarBen Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4068)
parent c2908538

ssl/ssl_lib.c

+4 −3
Original line number Diff line number Diff line
@@ -3156,10 +3156,11 @@ void ssl_update_cache(SSL *s, int mode) 
        return;



    i = s->session_ctx->session_cache_mode;

    if ((i & mode) && (!s->hit)

        && ((i & SSL_SESS_CACHE_NO_INTERNAL_STORE)

    if ((i & mode) != 0

        && (!s->hit || SSL_IS_TLS13(s))

        && ((i & SSL_SESS_CACHE_NO_INTERNAL_STORE) != 0

            || SSL_CTX_add_session(s->session_ctx, s->session))

        && (s->session_ctx->new_session_cb != NULL)) {

        && s->session_ctx->new_session_cb != NULL) {

        SSL_SESSION_up_ref(s->session);

        if (!s->session_ctx->new_session_cb(s, s->session))

            SSL_SESSION_free(s->session);

ssl/statem/statem_clnt.c

+6 −6
Original line number Diff line number Diff line
@@ -2462,6 +2462,12 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) 
         * We reused an existing session, so we need to replace it with a new

         * one

         */

        if ((new_sess = ssl_session_dup(s->session, 0)) == 0) {

            al = SSL_AD_INTERNAL_ERROR;

            SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);

            goto f_err;

        }



        if (i & SSL_SESS_CACHE_CLIENT) {

            /*

             * Remove the old session from the cache. We carry on if this fails
@@ -2469,12 +2475,6 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) 
            SSL_CTX_remove_session(s->session_ctx, s->session);

        }



        if ((new_sess = ssl_session_dup(s->session, 0)) == 0) {

            al = SSL_AD_INTERNAL_ERROR;

            SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);

            goto f_err;

        }



        SSL_SESSION_free(s->session);

        s->session = new_sess;

    }

ssl/statem/statem_lib.c

+6 −1
Original line number Diff line number Diff line
@@ -1028,6 +1028,11 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs) 
            s->ctx->stats.sess_accept_good++;

            s->handshake_func = ossl_statem_accept;

        } else {

            /*

             * In TLSv1.3 we update the cache as part of processing the

             * NewSessionTicket

             */

            if (!SSL_IS_TLS13(s))

                ssl_update_cache(s, SSL_SESS_CACHE_CLIENT);

            if (s->hit)

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