[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Tue Jul 17 09:15:32 UTC 2018
The branch master has been updated
via 03cdf559145bff263692c9516cac3c6456c77f2c (commit)
via 5f26ddff7ee2914782e312621023e10af356de18 (commit)
via 04d7814a8038e01dbeb9fd7721d40c1824f553a8 (commit)
via 84475ccb70da709c9a0035561429a34700b565d9 (commit)
from 57fd517066418472b3280a975823405fb8f2f43d (commit)
- Log -----------------------------------------------------------------
commit 03cdf559145bff263692c9516cac3c6456c77f2c
Author: Matt Caswell <matt at openssl.org>
Date: Mon Jul 16 16:58:23 2018 +0100
Test that a failed resumption issues the correct number of tickets
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6722)
commit 5f26ddff7ee2914782e312621023e10af356de18
Author: Matt Caswell <matt at openssl.org>
Date: Mon Jul 16 16:57:36 2018 +0100
Always issue new tickets when using TLSv1.3 stateful tickets
Previously we were failing to issue new tickets if a resumption attempt
failed.
Fixes #6654
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6722)
commit 04d7814a8038e01dbeb9fd7721d40c1824f553a8
Author: Matt Caswell <matt at openssl.org>
Date: Thu Jul 5 17:19:03 2018 +0100
Improve testing of stateful tickets
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6722)
commit 84475ccb70da709c9a0035561429a34700b565d9
Author: Matt Caswell <matt at openssl.org>
Date: Mon Jul 16 14:57:35 2018 +0100
Don't remove sessions from the cache during PHA in TLSv1.3
If we issue new tickets due to post-handshake authentication there is no
reason to remove previous tickets from the cache. The code that did that
only removed the last session anyway - so if more than one ticket got
issued then those other tickets are still valid.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6722)
-----------------------------------------------------------------------
Summary of changes:
ssl/statem/extensions_srvr.c | 3 +-
ssl/statem/statem_srvr.c | 9 ---
test/sslapitest.c | 183 +++++++++++++++++++++++++++++++++----------
3 files changed, 144 insertions(+), 51 deletions(-)
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index ab38a4f..f5ab5bb 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1014,6 +1014,8 @@ static SSL_TICKET_STATUS tls_get_stateful_ticket(SSL *s, PACKET *tick,
{
SSL_SESSION *tmpsess = NULL;
+ s->ext.ticket_expected = 1;
+
switch (PACKET_remaining(tick)) {
case 0:
return SSL_TICKET_EMPTY;
@@ -1031,7 +1033,6 @@ static SSL_TICKET_STATUS tls_get_stateful_ticket(SSL *s, PACKET *tick,
if (tmpsess == NULL)
return SSL_TICKET_NO_DECRYPT;
- s->ext.ticket_expected = 1;
*sess = tmpsess;
return SSL_TICKET_SUCCESS;
}
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 5c59eb8..01b07a9 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -3648,8 +3648,6 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
*/
if (s->post_handshake_auth == SSL_PHA_REQUESTED) {
- int m = s->session_ctx->session_cache_mode;
-
if ((new_sess = ssl_session_dup(s->session, 0)) == 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE,
@@ -3657,13 +3655,6 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
goto err;
}
- if (m & SSL_SESS_CACHE_SERVER) {
- /*
- * 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;
}
diff --git a/test/sslapitest.c b/test/sslapitest.c
index cdac8bc..f435853 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -1233,11 +1233,92 @@ static int post_handshake_verify(SSL *sssl, SSL *cssl)
return 1;
}
-static int test_tickets(int idx)
+static int setup_ticket_text(int stateful, int idx, SSL_CTX **sctx,
+ SSL_CTX **cctx)
+{
+ int sess_id_ctx = 1;
+
+ 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))
+ || !TEST_true(SSL_CTX_set_session_id_context(*sctx,
+ (void *)&sess_id_ctx,
+ sizeof(sess_id_ctx))))
+ return 0;
+
+ if (stateful)
+ SSL_CTX_set_options(*sctx, SSL_OP_NO_TICKET);
+
+ 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);
+
+ return 1;
+}
+
+static int check_resumption(int idx, SSL_CTX *sctx, SSL_CTX *cctx, int succ)
+{
+ SSL *serverssl = NULL, *clientssl = NULL;
+ int i;
+
+ /* Test that we can resume with all the tickets we got given */
+ for (i = 0; i < idx * 2; i++) {
+ new_called = 0;
+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
+ &clientssl, NULL, NULL))
+ || !TEST_true(SSL_set_session(clientssl, sesscache[i])))
+ goto end;
+
+ SSL_force_post_handshake_auth(clientssl);
+
+ if (!TEST_true(create_ssl_connection(serverssl, clientssl,
+ SSL_ERROR_NONE)))
+ goto end;
+
+ /*
+ * Following a successful resumption we only get 1 ticket. After a
+ * failed one we should get idx tickets.
+ */
+ if (succ) {
+ if (!TEST_true(SSL_session_reused(clientssl))
+ || !TEST_int_eq(new_called, 1))
+ goto end;
+ } else {
+ if (!TEST_false(SSL_session_reused(clientssl))
+ || !TEST_int_eq(new_called, idx))
+ goto end;
+ }
+
+ new_called = 0;
+ /* After a post-handshake authentication we should get 1 new ticket */
+ if (succ
+ && (!post_handshake_verify(serverssl, clientssl)
+ || !TEST_int_eq(new_called, 1)))
+ 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;
+ }
+
+ return 1;
+
+ end:
+ SSL_free(clientssl);
+ SSL_free(serverssl);
+ return 0;
+}
+
+static int test_tickets(int stateful, int idx)
{
SSL_CTX *sctx = NULL, *cctx = NULL;
SSL *serverssl = NULL, *clientssl = NULL;
- int testresult = 0, i;
+ int testresult = 0;
size_t j;
/* idx is the test number, but also the number of tickets we want */
@@ -1245,15 +1326,49 @@ static int test_tickets(int idx)
new_called = 0;
do_cache = 1;
- 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)))
+ if (!setup_ticket_text(stateful, idx, &sctx, &cctx))
+ goto end;
+
+ if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
+ &clientssl, NULL, NULL)))
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_connection(serverssl, clientssl,
+ SSL_ERROR_NONE))
+ /* Check we got the number of tickets we were expecting */
+ || !TEST_int_eq(idx, new_called))
+ goto end;
+
+ SSL_shutdown(clientssl);
+ SSL_shutdown(serverssl);
+ SSL_free(serverssl);
+ SSL_free(clientssl);
+ SSL_CTX_free(sctx);
+ SSL_CTX_free(cctx);
+ clientssl = serverssl = NULL;
+ sctx = cctx = NULL;
+
+ /*
+ * Now we try to resume with the tickets we previously created. The
+ * resumption attempt is expected to fail (because we're now using a new
+ * SSL_CTX). We should see idx number of tickets issued again.
+ */
+
+ /* Stop caching sessions - just count them */
+ do_cache = 0;
+
+ if (!setup_ticket_text(stateful, idx, &sctx, &cctx))
+ goto end;
+
+ if (!check_resumption(idx, sctx, cctx, 0))
+ goto end;
+
+ /* Start again with caching sessions */
+ new_called = 0;
+ do_cache = 1;
+
+ if (!setup_ticket_text(stateful, idx, &sctx, &cctx))
+ goto end;
if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
&clientssl, NULL, NULL)))
@@ -1281,37 +1396,12 @@ static int test_tickets(int idx)
/* Stop caching sessions - just count them */
do_cache = 0;
- /* Test that we can resume with all the tickets we got given */
- for (i = 0; i < idx * 2; i++) {
- new_called = 0;
- if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
- &clientssl, NULL, NULL))
- || !TEST_true(SSL_set_session(clientssl, sesscache[i])))
- goto end;
-
- SSL_force_post_handshake_auth(clientssl);
-
- if (!TEST_true(create_ssl_connection(serverssl, clientssl,
- SSL_ERROR_NONE))
- || !TEST_true(SSL_session_reused(clientssl))
- /* Following a resumption we only get 1 ticket */
- || !TEST_int_eq(new_called, 1))
- goto end;
-
- new_called = 0;
- /* After a post-handshake authentication we should get 1 new ticket */
- if (!post_handshake_verify(serverssl, clientssl)
- || !TEST_int_eq(new_called, 1))
- 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;
- }
+ /*
+ * Check we can resume with all the tickets we created. This time around the
+ * resumptions should all be successful.
+ */
+ if (!check_resumption(idx, sctx, cctx, 1))
+ goto end;
testresult = 1;
@@ -1327,6 +1417,16 @@ static int test_tickets(int idx)
return testresult;
}
+
+static int test_stateless_tickets(int idx)
+{
+ return test_tickets(0, idx);
+}
+
+static int test_stateful_tickets(int idx)
+{
+ return test_tickets(1, idx);
+}
#endif
#define USE_NULL 0
@@ -5272,7 +5372,8 @@ int setup_tests(void)
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);
+ ADD_ALL_TESTS(test_stateful_tickets, 3);
+ ADD_ALL_TESTS(test_stateless_tickets, 3);
#endif
ADD_ALL_TESTS(test_ssl_set_bio, TOTAL_SSL_SET_BIO_TESTS);
ADD_TEST(test_ssl_bio_pop_next_bio);
More information about the openssl-commits
mailing list