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: Rich Salz <rsalz@openssl.org> (Merged from https://github.com/openssl/openssl/pull/5644)
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 1873237..5e44d4c 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c
@@ -417,7 +417,13 @@ 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;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 50be825..5542a78 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c
@@ -3691,6 +3691,10 @@ } 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 @@ 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) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 596fdd4..796e9d4 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -1409,7 +1409,7 @@ 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 @@ * 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; + sess->session_id_length = sesslen; + } *psess = sess; if (renew_ticket) return SSL_TICKET_SUCCESS_RENEW;