From 4cb004573a28fe5f8f8d95dc9407e0fe9df6f14c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 26 Jun 2018 18:07:56 +0100 Subject: [PATCH] Remove TLSv1.3 tickets from the client cache as we use them Tickets are supposed to be single use so we remove them from the cache on use. Fixes #6377 Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/6601) --- ssl/statem/statem_clnt.c | 1 + ssl/statem/statem_lib.c | 19 ++++++++++++++----- test/sslapitest.c | 5 +++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 26be9cb6b8..88c343761f 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2682,6 +2682,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) goto err; } s->session->session_id_length = sess_len; + s->session->not_resumable = 0; /* This is a standalone message in TLSv1.3, so there is no more to read */ if (SSL_IS_TLS13(s)) { diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 91d304e2b4..61fc3caa1c 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1068,12 +1068,21 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop) dtls1_start_timer(s); } } else { - /* - * In TLSv1.3 we update the cache as part of processing the - * NewSessionTicket - */ - if (!SSL_IS_TLS13(s)) + if (SSL_IS_TLS13(s)) { + /* + * We encourage applications to only use TLSv1.3 tickets once, + * so we remove this one from the cache. + */ + if ((s->session_ctx->session_cache_mode + & SSL_SESS_CACHE_CLIENT) != 0) + SSL_CTX_remove_session(s->session_ctx, s->session); + } else { + /* + * In TLSv1.3 we update the cache as part of processing the + * NewSessionTicket + */ ssl_update_cache(s, SSL_SESS_CACHE_CLIENT); + } if (s->hit) CRYPTO_atomic_add(&s->session_ctx->stats.sess_hit, 1, &discard, s->session_ctx->lock); diff --git a/test/sslapitest.c b/test/sslapitest.c index 6e08795f2d..598b02a056 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -944,11 +944,12 @@ 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. + * resumed. Since we attempted a resume we should also have removed the + * old ticket from the cache so that we try to only use tickets once. */ if (use_ext_cache && (!TEST_int_eq(new_called, 1) - || !TEST_int_eq(remove_called, 0))) + || !TEST_int_eq(remove_called, 1))) goto end; } else { /*