From a84e5c9aa8e50af2bcb445ab30a0e9c19e72f60b Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 1 Sep 2016 08:40:54 -0400 Subject: [PATCH] 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 Reviewed-by: Richard Levitte Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/1529) --- crypto/err/openssl.txt | 1 + include/openssl/sslerr.h | 1 + ssl/ssl_err.c | 2 + ssl/ssl_locl.h | 1 + ssl/ssl_sess.c | 175 +++++++++++++-------------- ssl/statem/extensions.c | 30 +++++ test/README.ssltest.md | 5 + test/handshake_helper.c | 10 +- test/handshake_helper.h | 2 + test/ssl-tests/06-sni-ticket.conf | 16 +++ test/ssl-tests/06-sni-ticket.conf.in | 6 +- test/ssl_test.c | 14 +++ test/ssl_test_ctx.c | 27 +++++ test/ssl_test_ctx.h | 9 ++ test/ssl_test_ctx_test.c | 6 +- test/ssl_test_ctx_test.conf | 3 + 16 files changed, 213 insertions(+), 95 deletions(-) diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 11a1365f72..58b901965b 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1071,6 +1071,7 @@ SSL_F_SSL_DO_CONFIG:391:ssl_do_config SSL_F_SSL_DO_HANDSHAKE:180:SSL_do_handshake SSL_F_SSL_DUP_CA_LIST:408:SSL_dup_CA_list SSL_F_SSL_ENABLE_CT:402:SSL_enable_ct +SSL_F_SSL_GENERATE_SESSION_ID:547:ssl_generate_session_id SSL_F_SSL_GET_NEW_SESSION:181:ssl_get_new_session SSL_F_SSL_GET_PREV_SESSION:217:ssl_get_prev_session SSL_F_SSL_GET_SERVER_CERT_INDEX:322:* diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 91f6d21fb4..1df0dfa713 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -148,6 +148,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_SSL_DO_HANDSHAKE 180 # define SSL_F_SSL_DUP_CA_LIST 408 # define SSL_F_SSL_ENABLE_CT 402 +# define SSL_F_SSL_GENERATE_SESSION_ID 547 # define SSL_F_SSL_GET_NEW_SESSION 181 # define SSL_F_SSL_GET_PREV_SESSION 217 # define SSL_F_SSL_GET_SERVER_CERT_INDEX 322 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 0ce7f271f3..3eb89a3810 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -208,6 +208,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_DO_HANDSHAKE, 0), "SSL_do_handshake"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_DUP_CA_LIST, 0), "SSL_dup_CA_list"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_ENABLE_CT, 0), "SSL_enable_ct"}, + {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GENERATE_SESSION_ID, 0), + "ssl_generate_session_id"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GET_NEW_SESSION, 0), "ssl_get_new_session"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GET_PREV_SESSION, 0), diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 2766462222..960182ed12 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2095,6 +2095,7 @@ __owur CERT *ssl_cert_new(void); __owur CERT *ssl_cert_dup(CERT *cert); void ssl_cert_clear_certs(CERT *c); void ssl_cert_free(CERT *c); +__owur int ssl_generate_session_id(SSL *s, SSL_SESSION *ss); __owur int ssl_get_new_session(SSL *s, int session); __owur int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al); __owur SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket); diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index dcdf4f6e02..98d6106f85 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -305,16 +305,94 @@ static int def_generate_session_id(SSL *ssl, unsigned char *id, return 0; } +int ssl_generate_session_id(SSL *s, SSL_SESSION *ss) +{ + unsigned int tmp; + GEN_SESSION_CB cb = def_generate_session_id; + + switch (s->version) { + case SSL3_VERSION: + case TLS1_VERSION: + case TLS1_1_VERSION: + case TLS1_2_VERSION: + case TLS1_3_VERSION: + case DTLS1_BAD_VER: + case DTLS1_VERSION: + case DTLS1_2_VERSION: + ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; + break; + default: + SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, SSL_R_UNSUPPORTED_SSL_VERSION); + return 0; + } + + /*- + * If RFC5077 ticket, use empty session ID (as server). + * Note that: + * (a) ssl_get_prev_session() does lookahead into the + * ClientHello extensions to find the session ticket. + * When ssl_get_prev_session() fails, statem_srvr.c calls + * ssl_get_new_session() in tls_process_client_hello(). + * At that point, it has not yet parsed the extensions, + * however, because of the lookahead, it already knows + * whether a ticket is expected or not. + * + * (b) statem_clnt.c calls ssl_get_new_session() before parsing + * ServerHello extensions, and before recording the session + * ID received from the server, so this block is a noop. + */ + if (s->ext.ticket_expected) { + ss->session_id_length = 0; + return 1; + } + + /* Choose which callback will set the session ID */ + CRYPTO_THREAD_read_lock(s->lock); + CRYPTO_THREAD_read_lock(s->session_ctx->lock); + if (s->generate_session_id) + cb = s->generate_session_id; + else if (s->session_ctx->generate_session_id) + cb = s->session_ctx->generate_session_id; + CRYPTO_THREAD_unlock(s->session_ctx->lock); + CRYPTO_THREAD_unlock(s->lock); + /* Choose a session ID */ + memset(ss->session_id, 0, ss->session_id_length); + tmp = (int)ss->session_id_length; + if (!cb(s, ss->session_id, &tmp)) { + /* The callback failed */ + SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, + SSL_R_SSL_SESSION_ID_CALLBACK_FAILED); + return 0; + } + /* + * Don't allow the callback to set the session length to zero. nor + * set it higher than it was. + */ + if (tmp == 0 || tmp > ss->session_id_length) { + /* The callback set an illegal length */ + SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, + SSL_R_SSL_SESSION_ID_HAS_BAD_LENGTH); + return 0; + } + ss->session_id_length = tmp; + /* Finally, check for a conflict */ + if (SSL_has_matching_session_id(s, ss->session_id, + (unsigned int)ss->session_id_length)) { + SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, SSL_R_SSL_SESSION_ID_CONFLICT); + return 0; + } + + return 1; +} + int ssl_get_new_session(SSL *s, int session) { /* This gets used by clients and servers. */ - unsigned int tmp; SSL_SESSION *ss = NULL; - GEN_SESSION_CB cb = def_generate_session_id; if ((ss = SSL_SESSION_new()) == NULL) - return (0); + return 0; /* If the context has a default timeout, use it */ if (s->session_ctx->session_timeout == 0) @@ -326,96 +404,11 @@ int ssl_get_new_session(SSL *s, int session) s->session = NULL; if (session) { - if (s->version == SSL3_VERSION) { - ss->ssl_version = SSL3_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == TLS1_VERSION) { - ss->ssl_version = TLS1_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == TLS1_1_VERSION) { - ss->ssl_version = TLS1_1_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == TLS1_2_VERSION) { - ss->ssl_version = TLS1_2_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == TLS1_3_VERSION) { - ss->ssl_version = TLS1_3_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == DTLS1_BAD_VER) { - ss->ssl_version = DTLS1_BAD_VER; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == DTLS1_VERSION) { - ss->ssl_version = DTLS1_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else if (s->version == DTLS1_2_VERSION) { - ss->ssl_version = DTLS1_2_VERSION; - ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH; - } else { - SSLerr(SSL_F_SSL_GET_NEW_SESSION, SSL_R_UNSUPPORTED_SSL_VERSION); + if (!ssl_generate_session_id(s, ss)) { SSL_SESSION_free(ss); - return (0); + return 0; } - /*- - * If RFC5077 ticket, use empty session ID (as server). - * Note that: - * (a) ssl_get_prev_session() does lookahead into the - * ClientHello extensions to find the session ticket. - * When ssl_get_prev_session() fails, statem_srvr.c calls - * ssl_get_new_session() in tls_process_client_hello(). - * At that point, it has not yet parsed the extensions, - * however, because of the lookahead, it already knows - * whether a ticket is expected or not. - * - * (b) statem_clnt.c calls ssl_get_new_session() before parsing - * ServerHello extensions, and before recording the session - * ID received from the server, so this block is a noop. - */ - if (s->ext.ticket_expected) { - ss->session_id_length = 0; - goto sess_id_done; - } - - /* Choose which callback will set the session ID */ - CRYPTO_THREAD_read_lock(s->lock); - CRYPTO_THREAD_read_lock(s->session_ctx->lock); - if (s->generate_session_id) - cb = s->generate_session_id; - else if (s->session_ctx->generate_session_id) - cb = s->session_ctx->generate_session_id; - CRYPTO_THREAD_unlock(s->session_ctx->lock); - CRYPTO_THREAD_unlock(s->lock); - /* Choose a session ID */ - memset(ss->session_id, 0, ss->session_id_length); - tmp = (int)ss->session_id_length; - if (!cb(s, ss->session_id, &tmp)) { - /* The callback failed */ - SSLerr(SSL_F_SSL_GET_NEW_SESSION, - SSL_R_SSL_SESSION_ID_CALLBACK_FAILED); - SSL_SESSION_free(ss); - return (0); - } - /* - * Don't allow the callback to set the session length to zero. nor - * set it higher than it was. - */ - if (tmp == 0 || tmp > ss->session_id_length) { - /* The callback set an illegal length */ - SSLerr(SSL_F_SSL_GET_NEW_SESSION, - SSL_R_SSL_SESSION_ID_HAS_BAD_LENGTH); - SSL_SESSION_free(ss); - return (0); - } - ss->session_id_length = tmp; - /* Finally, check for a conflict */ - if (SSL_has_matching_session_id(s, ss->session_id, - (unsigned int)ss->session_id_length)) { - SSLerr(SSL_F_SSL_GET_NEW_SESSION, SSL_R_SSL_SESSION_ID_CONFLICT); - SSL_SESSION_free(ss); - return (0); - } - - sess_id_done: if (s->ext.hostname) { ss->ext.hostname = OPENSSL_strdup(s->ext.hostname); if (ss->ext.hostname == NULL) { @@ -443,7 +436,7 @@ int ssl_get_new_session(SSL *s, int session) if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) ss->flags |= SSL_SESS_FLAG_EXTMS; - return (1); + return 1; } /*- diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 2991310124..4e8dc70756 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -824,6 +824,7 @@ static int final_server_name(SSL *s, unsigned int context, int sent, { int ret = SSL_TLSEXT_ERR_NOACK; int altmp = SSL_AD_UNRECOGNIZED_NAME; + int was_ticket = (SSL_get_options(s) & SSL_OP_NO_TICKET) == 0; if (s->ctx != NULL && s->ctx->ext.servername_cb != 0) ret = s->ctx->ext.servername_cb(s, &altmp, @@ -833,6 +834,35 @@ static int final_server_name(SSL *s, unsigned int context, int sent, ret = s->session_ctx->ext.servername_cb(s, &altmp, s->session_ctx->ext.servername_arg); + /* + * If we're expecting to send a ticket, and tickets were previously enabled, + * and now tickets are disabled, then turn off expected ticket. + * Also, if this is not a resumption, create a new session ID + */ + if (ret == SSL_TLSEXT_ERR_OK && s->ext.ticket_expected + && was_ticket && (SSL_get_options(s) & SSL_OP_NO_TICKET) != 0) { + s->ext.ticket_expected = 0; + if (!s->hit) { + SSL_SESSION* ss = SSL_get_session(s); + + if (ss != NULL) { + OPENSSL_free(ss->ext.tick); + ss->ext.tick = NULL; + ss->ext.ticklen = 0; + ss->ext.tick_lifetime_hint = 0; + ss->ext.tick_age_add = 0; + ss->ext.tick_identity = 0; + if (!ssl_generate_session_id(s, ss)) { + ret = SSL_TLSEXT_ERR_ALERT_FATAL; + altmp = SSL_AD_INTERNAL_ERROR; + } + } else { + ret = SSL_TLSEXT_ERR_ALERT_FATAL; + altmp = SSL_AD_INTERNAL_ERROR; + } + } + } + switch (ret) { case SSL_TLSEXT_ERR_ALERT_FATAL: *al = altmp; 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 @@ -56,6 +56,12 @@ typedef enum { SSL_TEST_COMPRESSION_YES } 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 @@ -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