diff options
author | Todd Short <tshort@akamai.com> | 2016-09-01 08:40:54 -0400 |
---|---|---|
committer | Pauli <paul.dale@oracle.com> | 2017-10-04 10:21:08 +1000 |
commit | a84e5c9aa8e50af2bcb445ab30a0e9c19e72f60b (patch) | |
tree | 590baea962817312a9b3b1007501abc67c34f256 /test | |
parent | 270a4bba49849de7f928f4fab186205abd132411 (diff) |
Session resume broken switching contexts
When an SSL's context is swtiched from a ticket-enabled context to
a ticket-disabled context in the servername callback, no session-id
is generated, so the session can't be resumed.
If a servername callback changes the SSL_OP_NO_TICKET option, check
to see if it's changed to disable, and whether a session ticket is
expected (i.e. the client indicated ticket support and the SSL had
tickets enabled at the time), and whether we already have a previous
session (i.e. s->hit is set).
In this case, clear the ticket-expected flag, remove any ticket data
and generate a session-id in the session.
If the SSL hit (resumed) and switched to a ticket-disabled context,
assume that the resumption was via session-id, and don't bother to
update the session.
Before this fix, the updated unit-tests in 06-sni-ticket.conf would
fail test #4 (server1 = SNI, server2 = no SNI).
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/1529)
Diffstat (limited to 'test')
-rw-r--r-- | test/README.ssltest.md | 5 | ||||
-rw-r--r-- | test/handshake_helper.c | 10 | ||||
-rw-r--r-- | test/handshake_helper.h | 2 | ||||
-rw-r--r-- | test/ssl-tests/06-sni-ticket.conf | 16 | ||||
-rw-r--r-- | test/ssl-tests/06-sni-ticket.conf.in | 6 | ||||
-rw-r--r-- | test/ssl_test.c | 14 | ||||
-rw-r--r-- | test/ssl_test_ctx.c | 27 | ||||
-rw-r--r-- | test/ssl_test_ctx.h | 9 | ||||
-rw-r--r-- | test/ssl_test_ctx_test.c | 6 | ||||
-rw-r--r-- | test/ssl_test_ctx_test.conf | 3 |
10 files changed, 94 insertions, 4 deletions
diff --git a/test/README.ssltest.md b/test/README.ssltest.md index c4540b47e4..3b4bb564f1 100644 --- a/test/README.ssltest.md +++ b/test/README.ssltest.md @@ -81,6 +81,11 @@ handshake. - Yes - a session ticket is expected - No - a session ticket is not expected +* SessionIdExpected - whether or not a session id is expected + - Ignore - do not check for a session id (default) + - Yes - a session id is expected + - No - a session id is not expected + * ResumptionExpected - whether or not resumption is expected (Resume mode only) - Yes - resumed handshake - No - full handshake (default) diff --git a/test/handshake_helper.c b/test/handshake_helper.c index 3d59abc66b..be96abe3c9 100644 --- a/test/handshake_helper.c +++ b/test/handshake_helper.c @@ -1304,6 +1304,8 @@ static HANDSHAKE_RESULT *do_handshake_internal( handshake_status_t status = HANDSHAKE_RETRY; const unsigned char* tick = NULL; size_t tick_len = 0; + const unsigned char* sess_id = NULL; + unsigned int sess_id_len = 0; SSL_SESSION* sess = NULL; const unsigned char *proto = NULL; /* API dictates unsigned int rather than size_t. */ @@ -1496,8 +1498,10 @@ static HANDSHAKE_RESULT *do_handshake_internal( ret->server_protocol = SSL_version(server.ssl); ret->client_protocol = SSL_version(client.ssl); ret->servername = server_ex_data.servername; - if ((sess = SSL_get0_session(client.ssl)) != NULL) + if ((sess = SSL_get0_session(client.ssl)) != NULL) { SSL_SESSION_get0_ticket(sess, &tick, &tick_len); + sess_id = SSL_SESSION_get_id(sess, &sess_id_len); + } if (tick == NULL || tick_len == 0) ret->session_ticket = SSL_TEST_SESSION_TICKET_NO; else @@ -1505,6 +1509,10 @@ static HANDSHAKE_RESULT *do_handshake_internal( ret->compression = (SSL_get_current_compression(client.ssl) == NULL) ? SSL_TEST_COMPRESSION_NO : SSL_TEST_COMPRESSION_YES; + if (sess_id == NULL || sess_id_len == 0) + ret->session_id = SSL_TEST_SESSION_ID_NO; + else + ret->session_id = SSL_TEST_SESSION_ID_YES; ret->session_ticket_do_not_call = server_ex_data.session_ticket_do_not_call; #ifndef OPENSSL_NO_NEXTPROTONEG diff --git a/test/handshake_helper.h b/test/handshake_helper.h index 2736057729..96c670e387 100644 --- a/test/handshake_helper.h +++ b/test/handshake_helper.h @@ -62,6 +62,8 @@ typedef struct handshake_result { int client_sign_type; /* Client CA names */ STACK_OF(X509_NAME) *client_ca_names; + /* Session id status */ + ssl_session_id_t session_id; } HANDSHAKE_RESULT; HANDSHAKE_RESULT *HANDSHAKE_RESULT_new(void); diff --git a/test/ssl-tests/06-sni-ticket.conf b/test/ssl-tests/06-sni-ticket.conf index ce0f63bc83..a3a9c78f06 100644 --- a/test/ssl-tests/06-sni-ticket.conf +++ b/test/ssl-tests/06-sni-ticket.conf @@ -93,6 +93,7 @@ VerifyMode = Peer [test-1] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = Yes server = 1-sni-session-ticket-server-extra client = 1-sni-session-ticket-client-extra @@ -136,6 +137,7 @@ VerifyMode = Peer [test-2] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = Yes server = 2-sni-session-ticket-server-extra client = 2-sni-session-ticket-client-extra @@ -179,6 +181,7 @@ VerifyMode = Peer [test-3] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = Yes server = 3-sni-session-ticket-server-extra client = 3-sni-session-ticket-client-extra @@ -222,6 +225,7 @@ VerifyMode = Peer [test-4] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 4-sni-session-ticket-server-extra client = 4-sni-session-ticket-client-extra @@ -265,6 +269,7 @@ VerifyMode = Peer [test-5] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 5-sni-session-ticket-server-extra client = 5-sni-session-ticket-client-extra @@ -308,6 +313,7 @@ VerifyMode = Peer [test-6] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 6-sni-session-ticket-server-extra client = 6-sni-session-ticket-client-extra @@ -351,6 +357,7 @@ VerifyMode = Peer [test-7] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 7-sni-session-ticket-server-extra client = 7-sni-session-ticket-client-extra @@ -394,6 +401,7 @@ VerifyMode = Peer [test-8] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 8-sni-session-ticket-server-extra client = 8-sni-session-ticket-client-extra @@ -437,6 +445,7 @@ VerifyMode = Peer [test-9] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 9-sni-session-ticket-server-extra client = 9-sni-session-ticket-client-extra @@ -480,6 +489,7 @@ VerifyMode = Peer [test-10] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 10-sni-session-ticket-server-extra client = 10-sni-session-ticket-client-extra @@ -523,6 +533,7 @@ VerifyMode = Peer [test-11] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 11-sni-session-ticket-server-extra client = 11-sni-session-ticket-client-extra @@ -566,6 +577,7 @@ VerifyMode = Peer [test-12] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 12-sni-session-ticket-server-extra client = 12-sni-session-ticket-client-extra @@ -609,6 +621,7 @@ VerifyMode = Peer [test-13] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 13-sni-session-ticket-server-extra client = 13-sni-session-ticket-client-extra @@ -652,6 +665,7 @@ VerifyMode = Peer [test-14] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 14-sni-session-ticket-server-extra client = 14-sni-session-ticket-client-extra @@ -695,6 +709,7 @@ VerifyMode = Peer [test-15] ExpectedResult = Success ExpectedServerName = server1 +SessionIdExpected = Yes SessionTicketExpected = No server = 15-sni-session-ticket-server-extra client = 15-sni-session-ticket-client-extra @@ -738,6 +753,7 @@ VerifyMode = Peer [test-16] ExpectedResult = Success ExpectedServerName = server2 +SessionIdExpected = Yes SessionTicketExpected = No server = 16-sni-session-ticket-server-extra client = 16-sni-session-ticket-client-extra diff --git a/test/ssl-tests/06-sni-ticket.conf.in b/test/ssl-tests/06-sni-ticket.conf.in index 8725960060..b4f4ffc29d 100644 --- a/test/ssl-tests/06-sni-ticket.conf.in +++ b/test/ssl-tests/06-sni-ticket.conf.in @@ -24,7 +24,8 @@ sub generate_tests() { foreach my $s1 ("SessionTicket", "-SessionTicket") { foreach my $s2 ("SessionTicket", "-SessionTicket") { foreach my $n ("server1", "server2") { - my $result = expected_result($c, $s1, $s2, $n); + my $ticket_result = expected_result($c, $s1, $s2, $n); + my $session_id_result = "Yes"; # always, even with a ticket push @tests, { "name" => "sni-session-ticket", "client" => { @@ -47,7 +48,8 @@ sub generate_tests() { "test" => { "ExpectedServerName" => $n, "ExpectedResult" => "Success", - "SessionTicketExpected" => $result, + "SessionIdExpected" => $session_id_result, + "SessionTicketExpected" => $ticket_result, } }; } diff --git a/test/ssl_test.c b/test/ssl_test.c index 44232dbdf4..dcdd867f43 100644 --- a/test/ssl_test.c +++ b/test/ssl_test.c @@ -143,6 +143,19 @@ static int check_session_ticket(HANDSHAKE_RESULT *result, SSL_TEST_CTX *test_ctx return 1; } +static int check_session_id(HANDSHAKE_RESULT *result, SSL_TEST_CTX *test_ctx) +{ + if (test_ctx->session_id_expected == SSL_TEST_SESSION_ID_IGNORE) + return 1; + if (!TEST_int_eq(result->session_id, test_ctx->session_id_expected)) { + TEST_info("Client SessionIdExpected mismatch, expected %s, got %s\n.", + ssl_session_id_name(test_ctx->session_id_expected), + ssl_session_id_name(result->session_id)); + return 0; + } + return 1; +} + static int check_compression(HANDSHAKE_RESULT *result, SSL_TEST_CTX *test_ctx) { if (!TEST_int_eq(result->compression, test_ctx->compression_expected)) @@ -320,6 +333,7 @@ static int check_test(HANDSHAKE_RESULT *result, SSL_TEST_CTX *test_ctx) ret &= check_servername(result, test_ctx); ret &= check_session_ticket(result, test_ctx); ret &= check_compression(result, test_ctx); + ret &= check_session_id(result, test_ctx); ret &= (result->session_ticket_do_not_call == 0); #ifndef OPENSSL_NO_NEXTPROTONEG ret &= check_npn(result, test_ctx); diff --git a/test/ssl_test_ctx.c b/test/ssl_test_ctx.c index d669d0d81c..569aef0fa7 100644 --- a/test/ssl_test_ctx.c +++ b/test/ssl_test_ctx.c @@ -293,6 +293,32 @@ const char *ssl_session_ticket_name(ssl_session_ticket_t server) IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, compression_expected) +/* SessionIdExpected */ + +static const test_enum ssl_session_id[] = { + {"Ignore", SSL_TEST_SESSION_ID_IGNORE}, + {"Yes", SSL_TEST_SESSION_ID_YES}, + {"No", SSL_TEST_SESSION_ID_NO}, +}; + +__owur static int parse_session_id(SSL_TEST_CTX *test_ctx, const char *value) +{ + int ret_value; + if (!parse_enum(ssl_session_id, OSSL_NELEM(ssl_session_id), + &ret_value, value)) { + return 0; + } + test_ctx->session_id_expected = ret_value; + return 1; +} + +const char *ssl_session_id_name(ssl_session_id_t server) +{ + return enum_name(ssl_session_id, + OSSL_NELEM(ssl_session_id), + server); +} + /* Method */ static const test_enum ssl_test_methods[] = { @@ -577,6 +603,7 @@ static const ssl_test_ctx_option ssl_test_ctx_options[] = { { "ExpectedServerName", &parse_expected_servername }, { "SessionTicketExpected", &parse_session_ticket }, { "CompressionExpected", &parse_test_compression_expected }, + { "SessionIdExpected", &parse_session_id }, { "Method", &parse_test_method }, { "ExpectedNPNProtocol", &parse_test_expected_npn_protocol }, { "ExpectedALPNProtocol", &parse_test_expected_alpn_protocol }, diff --git a/test/ssl_test_ctx.h b/test/ssl_test_ctx.h index 5eff75cfa1..fea6527656 100644 --- a/test/ssl_test_ctx.h +++ b/test/ssl_test_ctx.h @@ -57,6 +57,12 @@ typedef enum { } ssl_compression_t; typedef enum { + SSL_TEST_SESSION_ID_IGNORE = 0, /* Default */ + SSL_TEST_SESSION_ID_YES, + SSL_TEST_SESSION_ID_NO +} ssl_session_id_t; + +typedef enum { SSL_TEST_METHOD_TLS = 0, /* Default */ SSL_TEST_METHOD_DTLS } ssl_test_method_t; @@ -200,6 +206,8 @@ typedef struct { STACK_OF(X509_NAME) *expected_client_ca_names; /* Whether to use SCTP for the transport */ int use_sctp; + /* Whether to expect a session id from the server */ + ssl_session_id_t session_id_expected; } SSL_TEST_CTX; const char *ssl_test_result_name(ssl_test_result_t result); @@ -210,6 +218,7 @@ const char *ssl_servername_name(ssl_servername_t server); const char *ssl_servername_callback_name(ssl_servername_callback_t servername_callback); const char *ssl_session_ticket_name(ssl_session_ticket_t server); +const char *ssl_session_id_name(ssl_session_id_t server); const char *ssl_test_method_name(ssl_test_method_t method); const char *ssl_handshake_mode_name(ssl_handshake_mode_t mode); const char *ssl_ct_validation_name(ssl_ct_validation_t mode); diff --git a/test/ssl_test_ctx_test.c b/test/ssl_test_ctx_test.c index 194919d1f3..33a18428e8 100644 --- a/test/ssl_test_ctx_test.c +++ b/test/ssl_test_ctx_test.c @@ -92,7 +92,9 @@ static int testctx_eq(SSL_TEST_CTX *ctx, SSL_TEST_CTX *ctx2) || !TEST_str_eq(ctx->expected_alpn_protocol, ctx2->expected_alpn_protocol) || !TEST_int_eq(ctx->resumption_expected, - ctx2->resumption_expected)) + ctx2->resumption_expected) + || !TEST_int_eq(ctx->session_id_expected, + ctx2->session_id_expected)) return 0; return 1; } @@ -166,6 +168,7 @@ static int test_good_configuration(void) fixture->expected_ctx->expected_servername = SSL_TEST_SERVERNAME_SERVER2; fixture->expected_ctx->session_ticket_expected = SSL_TEST_SESSION_TICKET_YES; fixture->expected_ctx->compression_expected = SSL_TEST_COMPRESSION_NO; + fixture->expected_ctx->session_id_expected = SSL_TEST_SESSION_ID_IGNORE; fixture->expected_ctx->resumption_expected = 1; fixture->expected_ctx->extra.client.verify_callback = @@ -207,6 +210,7 @@ static const char *bad_configurations[] = { "ssltest_unknown_servername_callback", "ssltest_unknown_session_ticket_expected", "ssltest_unknown_compression_expected", + "ssltest_unknown_session_id_expected", "ssltest_unknown_method", "ssltest_unknown_handshake_mode", "ssltest_unknown_resumption_expected", diff --git a/test/ssl_test_ctx_test.conf b/test/ssl_test_ctx_test.conf index 86d40e5486..c85a4ba91b 100644 --- a/test/ssl_test_ctx_test.conf +++ b/test/ssl_test_ctx_test.conf @@ -75,6 +75,9 @@ SessionTicketExpected = Foo [ssltest_unknown_compression_expected] CompressionExpected = Foo +[ssltest_unknown_session_id_expected] +SessionIdExpected = Foo + [ssltest_unknown_method] Method = TLS2 |