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 /ssl | |
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 'ssl')
-rw-r--r-- | ssl/ssl_err.c | 2 | ||||
-rw-r--r-- | ssl/ssl_locl.h | 1 | ||||
-rw-r--r-- | ssl/ssl_sess.c | 175 | ||||
-rw-r--r-- | ssl/statem/extensions.c | 30 |
4 files changed, 117 insertions, 91 deletions
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); - } - - /*- - * 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); + 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; |