Commit 32305f88 authored by Matt Caswell's avatar Matt Caswell
Browse files

Always call the new_session_cb when issuing a NewSessionTicket in TLSv1.3 



Conceptually in TLSv1.3 there can be multiple sessions associated with a
single connection. Each NewSessionTicket issued can be considered a
separate session. We can end up issuing multiple NewSessionTickets on a
single connection at the moment (e.g. in a post-handshake auth scenario).
Each of those issued tickets should have the new_session_cb called, it
should go into the session cache separately and it should have a unique
id associated with it (so that they can be found individually in the
cache).

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5644)
parent 51cf8ba0

ssl/ssl_sess.c

+7 −1
Original line number Diff line number Diff line
@@ -417,7 +417,13 @@ int ssl_get_new_session(SSL *s, int session) 
    s->session = NULL;



    if (session) {

        if (!ssl_generate_session_id(s, ss)) {

        if (SSL_IS_TLS13(s)) {

            /*

             * We generate the session id while constructing the

             * NewSessionTicket in TLSv1.3.

             */

            ss->session_id_length = 0;

        } else if (!ssl_generate_session_id(s, ss)) {

            /* SSLfatal() already called */

            SSL_SESSION_free(ss);

            return 0;

ssl/statem/statem_srvr.c

+4 −1
Original line number Diff line number Diff line
@@ -3691,6 +3691,10 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) 
    } age_add_u;



    if (SSL_IS_TLS13(s)) {

        if (!ssl_generate_session_id(s, s->session)) {

            /* SSLfatal() already called */

            goto err;

        }

        if (ssl_randbytes(s, age_add_u.age_add_c, sizeof(age_add_u)) <= 0) {

            SSLfatal(s, SSL_AD_INTERNAL_ERROR,

                     SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET,
@@ -3776,7 +3780,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) 
                 SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET, ERR_R_INTERNAL_ERROR);

        goto err;

    }

    sess->session_id_length = 0; /* ID is irrelevant for the ticket */



    slen = i2d_SSL_SESSION(sess, NULL);

    if (slen == 0 || slen > slen_full) {

ssl/t1_lib.c

+4 −3
Original line number Diff line number Diff line
@@ -1409,7 +1409,7 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick, 
    OPENSSL_free(sdec);

    if (sess) {

        /* Some additional consistency checks */

        if (slen != 0 || sess->session_id_length != 0) {

        if (slen != 0) {

            SSL_SESSION_free(sess);

            return SSL_TICKET_NO_DECRYPT;

        }
@@ -1419,9 +1419,10 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick, 
         * structure. If it is empty set length to zero as required by

         * standard.

         */

        if (sesslen)

        if (sesslen) {

            memcpy(sess->session_id, sess_id, sesslen);

            sess->session_id_length = sesslen;

        }

        *psess = sess;

        if (renew_ticket)

            return SSL_TICKET_SUCCESS_RENEW;

test/sslapitest.c

+9 −4
Original line number Diff line number Diff line
@@ -1021,15 +1021,20 @@ static int execute_test_session(int maxprot, int use_int_cache, 
        goto end;



    if (use_ext_cache) {

        if (!TEST_int_eq(new_called, 0)

                || !TEST_int_eq(remove_called, 0))

        if (!TEST_int_eq(remove_called, 0))

            goto end;



        if (maxprot == TLS1_3_VERSION) {

            if (!TEST_int_eq(get_called, 0))

            /*

             * Every time we issue a NewSessionTicket we are creating a new

             * session for next time in TLSv1.3

             */

            if (!TEST_int_eq(new_called, 1)

                    || !TEST_int_eq(get_called, 0))

                goto end;

        } else {

            if (!TEST_int_eq(get_called, 1))

            if (!TEST_int_eq(new_called, 0)

                    || !TEST_int_eq(get_called, 1))

                goto end;

        }

    }