Commit 36ff232c authored by Matt Caswell's avatar Matt Caswell
Browse files

Change the default number of NewSessionTickets we send to 2 



Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5227)
parent 394159da

ssl/ssl_lib.c

+3 −2
Original line number Diff line number Diff line
@@ -591,6 +591,7 @@ int SSL_clear(SSL *s) 
    s->psksession_id = NULL;

    s->psksession_id_len = 0;

    s->hello_retry_request = 0;

    s->sent_tickets = 0;



    s->error = 0;

    s->hit = 0;
@@ -3034,8 +3035,8 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) 
     */

    ret->max_early_data = 0;



    /* By default we send one session ticket automatically in TLSv1.3 */

    ret->num_tickets = 1;

    /* By default we send two session tickets automatically in TLSv1.3 */

    ret->num_tickets = 2;



    ssl_ctx_system_config(ret);

ssl/statem/statem_clnt.c

+0 −8
Original line number Diff line number Diff line
@@ -2590,7 +2590,6 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) 
     * cache.

     */

    if (SSL_IS_TLS13(s) || s->session->session_id_length > 0) {

        int i = s->session_ctx->session_cache_mode;

        SSL_SESSION *new_sess;

        /*

         * We reused an existing session, so we need to replace it with a new
@@ -2603,13 +2602,6 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) 
            goto err;

        }



        if (i & SSL_SESS_CACHE_CLIENT) {

            /*

             * Remove the old session from the cache. We carry on if this fails

             */

            SSL_CTX_remove_session(s->session_ctx, s->session);

        }



        SSL_SESSION_free(s->session);

        s->session = new_sess;

    }

ssl/statem/statem_srvr.c

+15 −8
Original line number Diff line number Diff line
@@ -507,6 +507,9 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) 
        /* Fall through */



    case TLS_ST_SW_KEY_UPDATE:

        st->hand_state = TLS_ST_OK;

        return WRITE_TRAN_CONTINUE;



    case TLS_ST_SW_SESSION_TICKET:

        /* In a resumption we only ever send a maximum of one new ticket.

         * Following an initial handshake we send the number of tickets we have
@@ -708,7 +711,7 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst) 
        return WORK_FINISHED_CONTINUE;



    case TLS_ST_SW_SESSION_TICKET:

        if (SSL_IS_TLS13(s)) {

        if (SSL_IS_TLS13(s) && s->sent_tickets == 0) {

            /*

             * Actually this is the end of the handshake, but we're going

             * straight into writing the session ticket out. So we finish off
@@ -3687,14 +3690,18 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt) 
    sk = NULL;



    /* Save the current hash state for when we receive the CertificateVerify */

    if (SSL_IS_TLS13(s)

            && !ssl_handshake_hash(s, s->cert_verify_hash,

    if (SSL_IS_TLS13(s)) {

        if (!ssl_handshake_hash(s, s->cert_verify_hash,

                                sizeof(s->cert_verify_hash),

                                &s->cert_verify_hash_len)) {

            /* SSLfatal() already called */

            goto err;

        }



        /* Resend session tickets */

        s->sent_tickets = 0;

    }



    ret = MSG_PROCESS_CONTINUE_READING;



 err:
@@ -3989,7 +3996,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) 
        goto err;

    }

    if (SSL_IS_TLS13(s)) {

        ssl_update_cache(s, SSL_SESS_CACHE_SERVER);

        if (!tls_construct_extensions(s, pkt,

                                      SSL_EXT_TLS1_3_NEW_SESSION_TICKET,

                                      NULL, 0)) {
@@ -3997,6 +4003,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt) 
            goto err;

        }

        s->sent_tickets++;

        ssl_update_cache(s, SSL_SESS_CACHE_SERVER);

    }

    EVP_CIPHER_CTX_free(ctx);

    HMAC_CTX_free(hctx);

test/handshake_helper.c

+14 −4
Original line number Diff line number Diff line
@@ -1403,7 +1403,7 @@ static HANDSHAKE_RESULT *do_handshake_internal( 
    HANDSHAKE_EX_DATA server_ex_data, client_ex_data;

    CTX_DATA client_ctx_data, server_ctx_data, server2_ctx_data;

    HANDSHAKE_RESULT *ret = HANDSHAKE_RESULT_new();

    int client_turn = 1, client_turn_count = 0;

    int client_turn = 1, client_turn_count = 0, client_wait_count = 0;

    connect_phase_t phase = HANDSHAKE;

    handshake_status_t status = HANDSHAKE_RETRY;

    const unsigned char* tick = NULL;
@@ -1586,10 +1586,20 @@ static HANDSHAKE_RESULT *do_handshake_internal( 
                    ret->result = SSL_TEST_INTERNAL_ERROR;

                    goto err;

                }



                if (client_turn && server.status == PEER_SUCCESS) {

                    /*

                     * The server may finish before the client because the

                     * client spends some turns processing NewSessionTickets.

                     */

                    if (client_wait_count++ >= 2) {

                        ret->result = SSL_TEST_INTERNAL_ERROR;

                        goto err;

                    }

                } else {

                    /* Continue. */

                    client_turn ^= 1;

                }

            }

            break;

        }

    }

test/sslapitest.c

+138 −26
Original line number Diff line number Diff line
@@ -882,10 +882,14 @@ static int execute_test_session(int maxprot, int use_int_cache, 
    SSL *serverssl3 = NULL, *clientssl3 = NULL;

# endif

    SSL_SESSION *sess1 = NULL, *sess2 = NULL;

    int testresult = 0;

    int testresult = 0, numnewsesstick = 1;



    new_called = remove_called = 0;



    /* TLSv1.3 sends 2 NewSessionTickets */

    if (maxprot == TLS1_3_VERSION)

        numnewsesstick = 2;



    if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(),

                                       TLS1_VERSION, TLS_MAX_VERSION,

                                       &sctx, &cctx, cert, privkey)))
@@ -923,7 +927,9 @@ static int execute_test_session(int maxprot, int use_int_cache, 
    if (use_int_cache && !TEST_false(SSL_CTX_add_session(cctx, sess1)))

        goto end;

    if (use_ext_cache

            && (!TEST_int_eq(new_called, 1) || !TEST_int_eq(remove_called, 0)))

            && (!TEST_int_eq(new_called, numnewsesstick)



                || !TEST_int_eq(remove_called, 0)))

        goto end;



    new_called = remove_called = 0;
@@ -938,11 +944,11 @@ static int execute_test_session(int maxprot, int use_int_cache, 
    if (maxprot == TLS1_3_VERSION) {

        /*

         * In TLSv1.3 we should have created a new session even though we have

         * resumed. The original session should also have been removed.

         * resumed.

         */

        if (use_ext_cache

                && (!TEST_int_eq(new_called, 1)

                    || !TEST_int_eq(remove_called, 1)))

                    || !TEST_int_eq(remove_called, 0)))

            goto end;

    } else {

        /*
@@ -972,7 +978,8 @@ static int execute_test_session(int maxprot, int use_int_cache, 
        goto end;



    if (use_ext_cache

            && (!TEST_int_eq(new_called, 1) || !TEST_int_eq(remove_called, 0)))

            && (!TEST_int_eq(new_called, numnewsesstick)

                || !TEST_int_eq(remove_called, 0)))

        goto end;



    new_called = remove_called = 0;
@@ -1072,7 +1079,7 @@ static int execute_test_session(int maxprot, int use_int_cache, 
    if (use_ext_cache) {

        SSL_SESSION *tmp = sess2;



        if (!TEST_int_eq(new_called, 1)

        if (!TEST_int_eq(new_called, numnewsesstick)

                || !TEST_int_eq(remove_called, 0)

                || !TEST_int_eq(get_called, 0))

            goto end;
@@ -1105,10 +1112,6 @@ static int execute_test_session(int maxprot, int use_int_cache, 
            goto end;



        if (maxprot == TLS1_3_VERSION) {

            /*

             * 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;
@@ -1181,6 +1184,101 @@ static int test_session_with_both_cache(void) 
#endif

}



SSL_SESSION *sesscache[9];



static int new_cachesession_cb(SSL *ssl, SSL_SESSION *sess)

{

    sesscache[new_called++] = sess;



    return 1;

}



static int test_tickets(int idx)

{

    SSL_CTX *sctx = NULL, *cctx = NULL;

    SSL *serverssl = NULL, *clientssl = NULL;

    int testresult = 0, i;

    size_t j;



    /* idx is the test number, but also the number of tickets we want */



    new_called = 0;



    if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(),

                                       TLS1_VERSION, TLS_MAX_VERSION, &sctx,

                                       &cctx, cert, privkey))

            || !TEST_true(SSL_CTX_set_num_tickets(sctx, idx)))

        goto end;



    SSL_CTX_set_session_cache_mode(cctx, SSL_SESS_CACHE_CLIENT

                                         | SSL_SESS_CACHE_NO_INTERNAL_STORE);

    SSL_CTX_sess_set_new_cb(cctx, new_cachesession_cb);



    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,

                                          &clientssl, NULL, NULL)))

        goto end;



    SSL_force_post_handshake_auth(clientssl);



    if (!TEST_true(create_ssl_connection(serverssl, clientssl,

                                                SSL_ERROR_NONE))

               /* Check we got the number of tickets we were expecting */

            || !TEST_int_eq(idx, new_called))

        goto end;



    /* After a post-handshake authentication we should get new tickets issued */

    SSL_set_verify(serverssl, SSL_VERIFY_PEER, NULL);

    if (!TEST_true(SSL_verify_client_post_handshake(serverssl)))

        goto end;



    /* Start handshake on the server and client */

    if (!TEST_int_eq(SSL_do_handshake(serverssl), 1)

            || !TEST_int_le(SSL_read(clientssl, NULL, 0), 0)

            || !TEST_int_le(SSL_read(serverssl, NULL, 0), 0)

            || !TEST_true(create_ssl_connection(serverssl, clientssl,

                                                SSL_ERROR_NONE))

            || !TEST_int_eq(idx * 2, new_called))

        goto end;



    SSL_CTX_sess_set_new_cb(cctx, NULL);

    SSL_shutdown(clientssl);

    SSL_shutdown(serverssl);

    SSL_free(serverssl);

    SSL_free(clientssl);

    serverssl = clientssl = NULL;



    /* Test that we can resume with all the tickets we got given */

    for (i = 0; i < new_called; i++) {

        if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,

                                              &clientssl, NULL, NULL))

                || !TEST_true(SSL_set_session(clientssl, sesscache[i]))

                || !TEST_true(create_ssl_connection(serverssl, clientssl,

                                                    SSL_ERROR_NONE))

                || !TEST_true(SSL_session_reused(clientssl)))

            goto end;



        SSL_shutdown(clientssl);

        SSL_shutdown(serverssl);

        SSL_free(serverssl);

        SSL_free(clientssl);

        serverssl = clientssl = NULL;

        SSL_SESSION_free(sesscache[i]);

        sesscache[i] = NULL;

    }



    testresult = 1;



 end:

    SSL_free(serverssl);

    SSL_free(clientssl);

    for (j = 0; j < OSSL_NELEM(sesscache); j++)

        SSL_SESSION_free(sesscache[j]);

    SSL_CTX_free(sctx);

    SSL_CTX_free(cctx);



    return testresult;

}



#define USE_NULL            0

#define USE_BIO_1           1

#define USE_BIO_2           2
@@ -1198,7 +1296,6 @@ static int test_session_with_both_cache(void) 
# define TOTAL_CONN_FAIL_SSL_SET_BIO_TESTS       0

#endif





#define TOTAL_SSL_SET_BIO_TESTS TOTAL_NO_CONN_SSL_SET_BIO_TESTS \

                                + TOTAL_CONN_SUCCESS_SSL_SET_BIO_TESTS \

                                + TOTAL_CONN_FAIL_SSL_SET_BIO_TESTS
@@ -1933,10 +2030,13 @@ static int test_early_data_read_write(int idx) 
        goto end;



    /*

     * Make sure we process the NewSessionTicket. This arrives post-handshake.

     * We attempt a read which we do not expect to return any data.

     * Make sure we process the two NewSessionTickets. These arrive

     * post-handshake. We attempt reads which we do not expect to return any

     * data.

     */

    if (!TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf), &readbytes)))

    if (!TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf), &readbytes))

            || !TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf),

                           &readbytes)))

        goto end;



    /* Server should be able to write normal data */
@@ -3392,9 +3492,10 @@ static int test_custom_exts(int tst) 
                || (tst == 2 && snicb != 1))

            goto end;

    } else {

        /* In this case there 2 NewSessionTicket messages created */

        if (clntaddnewcb != 1

                || clntparsenewcb != 4

                || srvaddnewcb != 4

                || clntparsenewcb != 5

                || srvaddnewcb != 5

                || srvparsenewcb != 1)

            goto end;

    }
@@ -3438,10 +3539,13 @@ static int test_custom_exts(int tst) 
                || srvparsenewcb != 2)

            goto end;

    } else {

        /* No Certificate message extensions in the resumption handshake */

        /*

         * No Certificate message extensions in the resumption handshake,

         * 2 NewSessionTickets in the initial handshake, 1 in the resumption

         */

        if (clntaddnewcb != 2

                || clntparsenewcb != 7

                || srvaddnewcb != 7

                || clntparsenewcb != 8

                || srvaddnewcb != 8

                || srvparsenewcb != 2)

            goto end;

    }
@@ -4205,14 +4309,16 @@ static struct info_cb_states_st { 
        {SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TRFIN"},

        {SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_HANDSHAKE_START, NULL},

        {SSL_CB_LOOP, "TWST"}, {SSL_CB_HANDSHAKE_DONE, NULL},

        {SSL_CB_EXIT, NULL}, {SSL_CB_ALERT, NULL},

        {SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "PINIT "},

        {SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TRCH"}, {SSL_CB_LOOP, "TWSH"},

        {SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWEE"}, {SSL_CB_LOOP, "TWFIN"},

        {SSL_CB_LOOP, "TED"}, {SSL_CB_EXIT, NULL}, {SSL_CB_LOOP, "TED"},

        {SSL_CB_LOOP, "TRFIN"}, {SSL_CB_HANDSHAKE_DONE, NULL},

        {SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "TWST"},

        {SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL}, {0, NULL},

        {SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_EXIT, NULL},

        {SSL_CB_ALERT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},

        {SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TRCH"},

        {SSL_CB_LOOP, "TWSH"}, {SSL_CB_LOOP, "TWCCS"}, {SSL_CB_LOOP, "TWEE"},

        {SSL_CB_LOOP, "TWFIN"}, {SSL_CB_LOOP, "TED"}, {SSL_CB_EXIT, NULL},

        {SSL_CB_LOOP, "TED"}, {SSL_CB_LOOP, "TRFIN"},

        {SSL_CB_HANDSHAKE_DONE, NULL}, {SSL_CB_HANDSHAKE_START, NULL},

        {SSL_CB_LOOP, "TWST"}, {SSL_CB_HANDSHAKE_DONE, NULL},

        {SSL_CB_EXIT, NULL}, {0, NULL},

    }, {

        /* TLSv1.3 client followed by resumption */

        {SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "PINIT "},
@@ -4223,6 +4329,9 @@ static struct info_cb_states_st { 
        {SSL_CB_EXIT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},

        {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},

        {SSL_CB_HANDSHAKE_DONE, NULL},  {SSL_CB_EXIT, NULL},

        {SSL_CB_HANDSHAKE_START, NULL}, {SSL_CB_LOOP, "SSLOK "},

        {SSL_CB_LOOP, "SSLOK "}, {SSL_CB_LOOP, "TRST"},

        {SSL_CB_HANDSHAKE_DONE, NULL},  {SSL_CB_EXIT, NULL},

        {SSL_CB_ALERT, NULL}, {SSL_CB_HANDSHAKE_START, NULL},

        {SSL_CB_LOOP, "PINIT "}, {SSL_CB_LOOP, "TWCH"}, {SSL_CB_EXIT, NULL},

        {SSL_CB_LOOP, "TWCH"}, {SSL_CB_LOOP, "TRSH"},  {SSL_CB_LOOP, "TREE"},
@@ -4856,6 +4965,9 @@ int setup_tests(void) 
    ADD_TEST(test_session_with_only_int_cache);

    ADD_TEST(test_session_with_only_ext_cache);

    ADD_TEST(test_session_with_both_cache);

#ifndef OPENSSL_NO_TLS1_3

    ADD_ALL_TESTS(test_tickets, 3);

#endif

    ADD_ALL_TESTS(test_ssl_set_bio, TOTAL_SSL_SET_BIO_TESTS);

    ADD_TEST(test_ssl_bio_pop_next_bio);

    ADD_TEST(test_ssl_bio_pop_ssl_bio);