From ccae4a1582efcad311d095a8e6832b2b67d5ed05 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 11 Mar 2016 17:44:01 +0300 Subject: [PATCH] Allow different protocol version when trying to reuse a session We now send the highest supported version by the client, even if the session uses an older version. This fixes 2 problems: - When you try to reuse a session but the other side doesn't reuse it and uses a different protocol version the connection will fail. - When you're trying to reuse a session with an old version you might be stuck trying to reuse the old version while both sides support a newer version Signed-off-by: Kurt Roeckx Reviewed-by: Viktor Dukhovni GH: #852, MR: #2452 --- include/openssl/ssl.h | 1 + ssl/methods.c | 166 ++++++--------------------------------- ssl/ssl_err.c | 2 + ssl/ssl_locl.h | 12 ++- ssl/ssl_sess.c | 19 +---- ssl/statem/statem_clnt.c | 13 ++- ssl/statem/statem_lib.c | 41 +++++++++- 7 files changed, 87 insertions(+), 167 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 87d9e11acc..fc7dab06fd 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2511,6 +2511,7 @@ void ERR_load_SSL_strings(void); # define SSL_R_SSL_SESSION_ID_CONFLICT 302 # define SSL_R_SSL_SESSION_ID_CONTEXT_TOO_LONG 273 # define SSL_R_SSL_SESSION_ID_HAS_BAD_LENGTH 303 +# define SSL_R_SSL_SESSION_VERSION_MISMATCH 210 # define SSL_R_TLS_CLIENT_CERT_REQ_WITH_ANON_CIPHER 232 # define SSL_R_TLS_HEARTBEAT_PEER_DOESNT_ACCEPT 365 # define SSL_R_TLS_HEARTBEAT_PENDING 366 diff --git a/ssl/methods.c b/ssl/methods.c index d66edff988..e576502c63 100644 --- a/ssl/methods.c +++ b/ssl/methods.c @@ -116,59 +116,34 @@ * TLS/SSLv3 methods */ -static const SSL_METHOD *tls1_get_method(int ver) -{ - if (ver == TLS_ANY_VERSION) - return TLS_method(); -#ifndef OPENSSL_NO_TLS1_2 - if (ver == TLS1_2_VERSION) - return tlsv1_2_method(); -#endif -#ifndef OPENSSL_NO_TLS1_1 - if (ver == TLS1_1_VERSION) - return tlsv1_1_method(); -#endif -#ifndef OPENSSL_NO_TLS1 - if (ver == TLS1_VERSION) - return tlsv1_method(); -#endif -#ifndef OPENSSL_NO_SSL3 - if (ver == SSL3_VERSION) - return (sslv3_method()); - else -#endif - return NULL; -} - IMPLEMENT_tls_meth_func(TLS_ANY_VERSION, 0, 0, TLS_method, ossl_statem_accept, - ossl_statem_connect, tls1_get_method, TLSv1_2_enc_data) + ossl_statem_connect, TLSv1_2_enc_data) #ifndef OPENSSL_NO_TLS1_2_METHOD IMPLEMENT_tls_meth_func(TLS1_2_VERSION, 0, SSL_OP_NO_TLSv1_2, tlsv1_2_method, ossl_statem_accept, - ossl_statem_connect, tls1_get_method, TLSv1_2_enc_data) + ossl_statem_connect, TLSv1_2_enc_data) #endif #ifndef OPENSSL_NO_TLS1_1_METHOD IMPLEMENT_tls_meth_func(TLS1_1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1_1, tlsv1_1_method, ossl_statem_accept, - ossl_statem_connect, tls1_get_method, TLSv1_1_enc_data) + ossl_statem_connect, TLSv1_1_enc_data) #endif #ifndef OPENSSL_NO_TLS1_METHOD IMPLEMENT_tls_meth_func(TLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1, tlsv1_method, ossl_statem_accept, - ossl_statem_connect, tls1_get_method, TLSv1_enc_data) + ossl_statem_connect, TLSv1_enc_data) #endif #ifndef OPENSSL_NO_SSL3_METHOD -IMPLEMENT_ssl3_meth_func(sslv3_method, ossl_statem_accept, ossl_statem_connect, - tls1_get_method) +IMPLEMENT_ssl3_meth_func(sslv3_method, ossl_statem_accept, ossl_statem_connect) #endif @@ -176,41 +151,18 @@ IMPLEMENT_ssl3_meth_func(sslv3_method, ossl_statem_accept, ossl_statem_connect, * TLS/SSLv3 server methods */ -static const SSL_METHOD *tls1_get_server_method(int ver) -{ - if (ver == TLS_ANY_VERSION) - return TLS_server_method(); -#ifndef OPENSSL_NO_TLS1_2 - if (ver == TLS1_2_VERSION) - return tlsv1_2_server_method(); -#endif -#ifndef OPENSSL_NO_TLS1_1 - if (ver == TLS1_1_VERSION) - return tlsv1_1_server_method(); -#endif -#ifndef OPENSSL_NO_TLS1 - if (ver == TLS1_VERSION) - return tlsv1_server_method(); -#endif -#ifndef OPENSSL_NO_SSL3 - if (ver == SSL3_VERSION) - return (sslv3_server_method()); -#endif - return NULL; -} - IMPLEMENT_tls_meth_func(TLS_ANY_VERSION, 0, 0, TLS_server_method, ossl_statem_accept, ssl_undefined_function, - tls1_get_server_method, TLSv1_2_enc_data) + TLSv1_2_enc_data) #ifndef OPENSSL_NO_TLS1_2_METHOD IMPLEMENT_tls_meth_func(TLS1_2_VERSION, 0, SSL_OP_NO_TLSv1_2, tlsv1_2_server_method, ossl_statem_accept, ssl_undefined_function, - tls1_get_server_method, TLSv1_2_enc_data) + TLSv1_2_enc_data) #endif #ifndef OPENSSL_NO_TLS1_1_METHOD @@ -218,7 +170,7 @@ IMPLEMENT_tls_meth_func(TLS1_1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1_1, tlsv1_1_server_method, ossl_statem_accept, ssl_undefined_function, - tls1_get_server_method, TLSv1_1_enc_data) + TLSv1_1_enc_data) #endif #ifndef OPENSSL_NO_TLS1_METHOD @@ -226,13 +178,13 @@ IMPLEMENT_tls_meth_func(TLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1, tlsv1_server_method, ossl_statem_accept, ssl_undefined_function, - tls1_get_server_method, TLSv1_enc_data) + TLSv1_enc_data) #endif #ifndef OPENSSL_NO_SSL3_METHOD IMPLEMENT_ssl3_meth_func(sslv3_server_method, ossl_statem_accept, - ssl_undefined_function, tls1_get_server_method) + ssl_undefined_function) #endif @@ -240,41 +192,18 @@ IMPLEMENT_ssl3_meth_func(sslv3_server_method, * TLS/SSLv3 client methods */ -static const SSL_METHOD *tls1_get_client_method(int ver) -{ - if (ver == TLS_ANY_VERSION) - return TLS_client_method(); -#ifndef OPENSSL_NO_TLS1_2 - if (ver == TLS1_2_VERSION) - return tlsv1_2_client_method(); -#endif -#ifndef OPENSSL_NO_TLS1_1 - if (ver == TLS1_1_VERSION) - return tlsv1_1_client_method(); -#endif -#ifndef OPENSSL_NO_TLS1 - if (ver == TLS1_VERSION) - return tlsv1_client_method(); -#endif -#ifndef OPENSSL_NO_SSL3 - if (ver == SSL3_VERSION) - return (sslv3_client_method()); -#endif - return NULL; -} - IMPLEMENT_tls_meth_func(TLS_ANY_VERSION, 0, 0, TLS_client_method, ssl_undefined_function, ossl_statem_connect, - tls1_get_client_method, TLSv1_2_enc_data) + TLSv1_2_enc_data) #ifndef OPENSSL_NO_TLS1_2_METHOD IMPLEMENT_tls_meth_func(TLS1_2_VERSION, 0, SSL_OP_NO_TLSv1_2, tlsv1_2_client_method, ssl_undefined_function, ossl_statem_connect, - tls1_get_client_method, TLSv1_2_enc_data) + TLSv1_2_enc_data) #endif #ifndef OPENSSL_NO_TLS1_1_METHOD @@ -282,7 +211,7 @@ IMPLEMENT_tls_meth_func(TLS1_1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1_1, tlsv1_1_client_method, ssl_undefined_function, ossl_statem_connect, - tls1_get_client_method, TLSv1_1_enc_data) + TLSv1_1_enc_data) #endif #ifndef OPENSSL_NO_TLS1_METHOD @@ -290,41 +219,26 @@ IMPLEMENT_tls_meth_func(TLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_TLSv1, tlsv1_client_method, ssl_undefined_function, ossl_statem_connect, - tls1_get_client_method, TLSv1_enc_data) + TLSv1_enc_data) #endif #ifndef OPENSSL_NO_SSL3_METHOD IMPLEMENT_ssl3_meth_func(sslv3_client_method, ssl_undefined_function, - ossl_statem_connect, tls1_get_client_method) + ossl_statem_connect) #endif /* * DTLS methods */ -static const SSL_METHOD *dtls1_get_method(int ver) -{ - if (ver == DTLS_ANY_VERSION) - return DTLS_method(); -#ifndef OPENSSL_NO_DTLS1 - else if (ver == DTLS1_VERSION) - return dtlsv1_method(); -#endif -#ifndef OPENSSL_NO_DTLS1_2 - else if (ver == DTLS1_2_VERSION) - return dtlsv1_2_method(); -#endif - else - return NULL; -} #ifndef OPENSSL_NO_DTLS1_METHOD IMPLEMENT_dtls1_meth_func(DTLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_DTLSv1, dtlsv1_method, ossl_statem_accept, ossl_statem_connect, - dtls1_get_method, DTLSv1_enc_data) + DTLSv1_enc_data) #endif #ifndef OPENSSL_NO_DTLS1_2_METHOD @@ -332,41 +246,25 @@ IMPLEMENT_dtls1_meth_func(DTLS1_2_VERSION, 0, SSL_OP_NO_DTLSv1_2, dtlsv1_2_method, ossl_statem_accept, ossl_statem_connect, - dtls1_get_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) #endif IMPLEMENT_dtls1_meth_func(DTLS_ANY_VERSION, 0, 0, DTLS_method, ossl_statem_accept, ossl_statem_connect, - dtls1_get_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) /* * DTLS server methods */ -static const SSL_METHOD *dtls1_get_server_method(int ver) -{ - if (ver == DTLS_ANY_VERSION) - return DTLS_server_method(); -#ifndef OPENSSL_NO_DTLS1 - else if (ver == DTLS1_VERSION) - return dtlsv1_server_method(); -#endif -#ifndef OPENSSL_NO_DTLS1_2 - else if (ver == DTLS1_2_VERSION) - return dtlsv1_2_server_method(); -#endif - else - return NULL; -} - #ifndef OPENSSL_NO_DTLS1_METHOD IMPLEMENT_dtls1_meth_func(DTLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_DTLSv1, dtlsv1_server_method, ossl_statem_accept, ssl_undefined_function, - dtls1_get_server_method, DTLSv1_enc_data) + DTLSv1_enc_data) #endif #ifndef OPENSSL_NO_DTLS1_2_METHOD @@ -374,42 +272,26 @@ IMPLEMENT_dtls1_meth_func(DTLS1_2_VERSION, 0, SSL_OP_NO_DTLSv1_2, dtlsv1_2_server_method, ossl_statem_accept, ssl_undefined_function, - dtls1_get_server_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) #endif IMPLEMENT_dtls1_meth_func(DTLS_ANY_VERSION, 0, 0, DTLS_server_method, ossl_statem_accept, ssl_undefined_function, - dtls1_get_server_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) /* * DTLS client methods */ -static const SSL_METHOD *dtls1_get_client_method(int ver) -{ - if (ver == DTLS_ANY_VERSION) - return DTLS_client_method(); -#ifndef OPENSSL_NO_DTLS1 - else if (ver == DTLS1_VERSION || ver == DTLS1_BAD_VER) - return dtlsv1_client_method(); -#endif -#ifndef OPENSSL_NO_DTLS1_2 - else if (ver == DTLS1_2_VERSION) - return dtlsv1_2_client_method(); -#endif - else - return NULL; -} - #ifndef OPENSSL_NO_DTLS1_METHOD IMPLEMENT_dtls1_meth_func(DTLS1_VERSION, SSL_METHOD_NO_SUITEB, SSL_OP_NO_DTLSv1, dtlsv1_client_method, ssl_undefined_function, ossl_statem_connect, - dtls1_get_client_method, DTLSv1_enc_data) + DTLSv1_enc_data) #endif #ifndef OPENSSL_NO_DTLS1_2_METHOD @@ -417,14 +299,14 @@ IMPLEMENT_dtls1_meth_func(DTLS1_2_VERSION, 0, SSL_OP_NO_DTLSv1_2, dtlsv1_2_client_method, ssl_undefined_function, ossl_statem_connect, - dtls1_get_client_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) #endif IMPLEMENT_dtls1_meth_func(DTLS_ANY_VERSION, 0, 0, DTLS_client_method, ssl_undefined_function, ossl_statem_connect, - dtls1_get_client_method, DTLSv1_2_enc_data) + DTLSv1_2_enc_data) #if OPENSSL_API_COMPAT < 0x10100000L diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 88f6c73cfe..d0cadc60f0 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -634,6 +634,8 @@ static ERR_STRING_DATA SSL_str_reasons[] = { "ssl session id context too long"}, {ERR_REASON(SSL_R_SSL_SESSION_ID_HAS_BAD_LENGTH), "ssl session id has bad length"}, + {ERR_REASON(SSL_R_SSL_SESSION_VERSION_MISMATCH), + "ssl session version mismatch"}, {ERR_REASON(SSL_R_TLS_CLIENT_CERT_REQ_WITH_ANON_CIPHER), "tls client cert req with anon cipher"}, {ERR_REASON(SSL_R_TLS_HEARTBEAT_PEER_DOESNT_ACCEPT), diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 64f4ae9373..4a2b52d19e 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -542,7 +542,6 @@ struct ssl_method_st { int (*ssl_pending) (const SSL *s); int (*num_ciphers) (void); const SSL_CIPHER *(*get_cipher) (unsigned ncipher); - const struct ssl_method_st *(*get_ssl_method) (int version); long (*get_timeout) (void); const struct ssl3_enc_method *ssl3_enc; /* Extra SSLv3/TLS stuff */ int (*ssl_version) (void); @@ -1761,7 +1760,7 @@ extern const SSL3_ENC_METHOD DTLSv1_2_enc_data; #define SSL_METHOD_NO_SUITEB (1U<<1) # define IMPLEMENT_tls_meth_func(version, flags, mask, func_name, s_accept, \ - s_connect, s_get_meth, enc_data) \ + s_connect, enc_data) \ const SSL_METHOD *func_name(void) \ { \ static const SSL_METHOD func_name##_data= { \ @@ -1789,7 +1788,6 @@ const SSL_METHOD *func_name(void) \ ssl3_pending, \ ssl3_num_ciphers, \ ssl3_get_cipher, \ - s_get_meth, \ tls1_default_timeout, \ &enc_data, \ ssl_undefined_void_function, \ @@ -1799,7 +1797,7 @@ const SSL_METHOD *func_name(void) \ return &func_name##_data; \ } -# define IMPLEMENT_ssl3_meth_func(func_name, s_accept, s_connect, s_get_meth) \ +# define IMPLEMENT_ssl3_meth_func(func_name, s_accept, s_connect) \ const SSL_METHOD *func_name(void) \ { \ static const SSL_METHOD func_name##_data= { \ @@ -1827,7 +1825,6 @@ const SSL_METHOD *func_name(void) \ ssl3_pending, \ ssl3_num_ciphers, \ ssl3_get_cipher, \ - s_get_meth, \ ssl3_default_timeout, \ &SSLv3_enc_data, \ ssl_undefined_void_function, \ @@ -1838,7 +1835,7 @@ const SSL_METHOD *func_name(void) \ } # define IMPLEMENT_dtls1_meth_func(version, flags, mask, func_name, s_accept, \ - s_connect, s_get_meth, enc_data) \ + s_connect, enc_data) \ const SSL_METHOD *func_name(void) \ { \ static const SSL_METHOD func_name##_data= { \ @@ -1866,7 +1863,6 @@ const SSL_METHOD *func_name(void) \ ssl3_pending, \ ssl3_num_ciphers, \ ssl3_get_cipher, \ - s_get_meth, \ dtls1_default_timeout, \ &enc_data, \ ssl_undefined_void_function, \ @@ -1996,6 +1992,8 @@ __owur int ssl3_handshake_write(SSL *s); __owur int ssl_allow_compression(SSL *s); +__owur int ssl_version_supported(const SSL *s, int version); + __owur int ssl_set_client_hello_version(SSL *s); __owur int ssl_check_version_downgrade(SSL *s); __owur int ssl_set_version_bound(int method_version, int version, int *bound); diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 3f030a76eb..70e2683ee4 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -872,19 +872,9 @@ int SSL_SESSION_up_ref(SSL_SESSION *ss) int SSL_set_session(SSL *s, SSL_SESSION *session) { int ret = 0; - const SSL_METHOD *meth; - if (session != NULL) { - meth = s->ctx->method->get_ssl_method(session->ssl_version); - if (meth == NULL) - meth = s->method->get_ssl_method(session->ssl_version); - if (meth == NULL) { - SSLerr(SSL_F_SSL_SET_SESSION, SSL_R_UNABLE_TO_FIND_SSL_METHOD); - return (0); - } - - if (meth != s->method) { - if (!SSL_set_ssl_method(s, meth)) + if (s->ctx->method != s->method) { + if (!SSL_set_ssl_method(s, s->ctx->method)) return (0); } @@ -896,9 +886,8 @@ int SSL_set_session(SSL *s, SSL_SESSION *session) } else { SSL_SESSION_free(s->session); s->session = NULL; - meth = s->ctx->method; - if (meth != s->method) { - if (!SSL_set_ssl_method(s, meth)) + if (s->ctx->method != s->method) { + if (!SSL_set_ssl_method(s, s->ctx->method)) return (0); } ret = 1; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index bafb90a9dc..73f54bcb96 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -817,7 +817,8 @@ int tls_construct_client_hello(SSL *s) goto err; } - if ((sess == NULL) || (sess->ssl_version != s->version) || + if ((sess == NULL) || + !ssl_version_supported(s, sess->ssl_version) || /* * In the case of EAP-FAST, we can have a pre-shared * "ticket" without a session ID. @@ -1126,12 +1127,22 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) } } + s->session->ssl_version = s->version; s->session->session_id_length = session_id_len; /* session_id_len could be 0 */ memcpy(s->session->session_id, PACKET_data(&session_id), session_id_len); } + /* Session version and negotiated protocol version should match */ + if (s->version != s->session->ssl_version) { + al = SSL_AD_PROTOCOL_VERSION; + + SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_SSL_SESSION_VERSION_MISMATCH); + goto f_err; + } + c = ssl_get_cipher_by_char(s, cipherchars); if (c == NULL) { /* unknown cipher */ diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index a24060e47f..8fcc23246e 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -787,6 +787,44 @@ static int ssl_method_error(const SSL *s, const SSL_METHOD *method) return 0; } +/* + * ssl_version_supported - Check that the specified `version` is supported by + * `SSL *` instance + * + * @s: The SSL handle for the candidate method + * @version: Protocol version to test against + * + * Returns 1 when supported, otherwise 0 + */ +int ssl_version_supported(const SSL *s, int version) +{ + const version_info *vent; + const version_info *table; + + switch (s->method->version) { + default: + /* Version should match method version for non-ANY method */ + return version_cmp(s, version, s->version) == 0; + case TLS_ANY_VERSION: + table = tls_version_table; + break; + case DTLS_ANY_VERSION: + table = dtls_version_table; + break; + } + + for (vent = table; + vent->version != 0 && version_cmp(s, version, vent->version) <= 0; + ++vent) { + if (vent->cmeth != NULL && + version_cmp(s, version, vent->version) == 0 && + ssl_method_error(s, vent->cmeth()) == 0) { + return 1; + } + } + return 0; +} + /* * ssl_check_version_downgrade - In response to RFC7507 SCSV version * fallback indication from a client check whether we're using the highest @@ -976,7 +1014,6 @@ int ssl_choose_client_version(SSL *s, int version) * versions they don't want. If not, then easy to fix, just return * ssl_method_error(s, s->method) */ - s->session->ssl_version = s->version; return 0; case TLS_ANY_VERSION: table = tls_version_table; @@ -999,7 +1036,7 @@ int ssl_choose_client_version(SSL *s, int version) if (err != 0) return err; s->method = method; - s->session->ssl_version = s->version = version; + s->version = version; return 0; }