From e02758c9c88d1200bc20f7d9514de3fc18ec8aa6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 26 Jun 2019 15:31:31 +0100 Subject: [PATCH] Remove ciphersuite from SSL session if single suite hardcoded If MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled, the type mbedtls_ssl_ciphersuite_handle_t is logically a boolean (concretely realized as `unsigned char`), containing the invalid handle and the unique valid handle, which represents the single enabled ciphersuite. The SSL session structure mbedtls_ssl_session contains an instance of mbedtls_ssl_ciphersuite_handle_t which is guaranteed to be valid, and which is hence redundant in any two-valued implementation of mbedtls_ssl_ciphersuite_handle_t. This commit replaces read-uses of mbedtls_ssl_session::ciphersuite_info by a getter functions which, and defines this getter function either by just reading the field from the session structure (in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is disabled), or by returning the single valid ciphersuite handle (in case MBEDTLS_SSL_SINGLE_CIPHERSUITE is enabled) and removing the field from mbedtls_ssl_session in this case. --- include/mbedtls/ssl.h | 2 ++ include/mbedtls/ssl_ciphersuites.h | 15 ++++++++++ library/ssl_cache.c | 5 +++- library/ssl_cli.c | 4 ++- library/ssl_srv.c | 26 +++++++++++----- library/ssl_tls.c | 45 +++++++++++++++++++--------- tests/suites/test_suite_ssl.function | 4 +++ 7 files changed, 78 insertions(+), 23 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index f9b95023e..daa9d746f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -923,7 +923,9 @@ struct mbedtls_ssl_session #if defined(MBEDTLS_HAVE_TIME) mbedtls_time_t start; /*!< starting time */ #endif +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) int ciphersuite; /*!< chosen ciphersuite */ +#endif /* MBEDTLS_SSL_SINGLE_CIPHERSUITE */ int compression; /*!< chosen compression */ size_t id_len; /*!< session id length */ unsigned char id[32]; /*!< session identifier */ diff --git a/include/mbedtls/ssl_ciphersuites.h b/include/mbedtls/ssl_ciphersuites.h index 2f31ceede..c8cfacde2 100644 --- a/include/mbedtls/ssl_ciphersuites.h +++ b/include/mbedtls/ssl_ciphersuites.h @@ -681,6 +681,21 @@ static inline int mbedtls_ssl_ciphersuite_uses_server_signature( } } +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) +static inline int mbedtls_ssl_session_get_ciphersuite( + mbedtls_ssl_session const * session ) +{ + return( session->ciphersuite ); +} +#else /* !MBEDTLS_SSL_SINGLE_CIPHERSUITE */ +static inline int mbedtls_ssl_session_get_ciphersuite( + mbedtls_ssl_session const * session ) +{ + ((void) session); + return( MBEDTLS_SSL_SUITE_ID( MBEDTLS_SSL_SINGLE_CIPHERSUITE ) ); +} +#endif /* MBEDTLS_SSL_SINGLE_CIPHERSUITE */ + const int *mbedtls_ssl_list_ciphersuites( void ); mbedtls_ssl_ciphersuite_handle_t mbedtls_ssl_ciphersuite_from_string( const char *ciphersuite_name ); diff --git a/library/ssl_cache.c b/library/ssl_cache.c index 62a0a2987..bcc2f59d1 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -84,10 +84,13 @@ int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session ) continue; #endif - if( session->ciphersuite != entry->session.ciphersuite || + if( mbedtls_ssl_session_get_ciphersuite( session ) != + mbedtls_ssl_session_get_ciphersuite( &entry->session ) || session->compression != entry->session.compression || session->id_len != entry->session.id_len ) + { continue; + } if( memcmp( session->id, entry->session.id, entry->session.id_len ) != 0 ) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 498bb79d8..f4d51dc99 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1845,7 +1845,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) if( n == 0 || mbedtls_ssl_get_renego_status( ssl ) != MBEDTLS_SSL_INITIAL_HANDSHAKE || - ssl->session_negotiate->ciphersuite != i || + mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ) != i || ssl->session_negotiate->compression != comp || ssl->session_negotiate->id_len != n || memcmp( ssl->session_negotiate->id, buf + 35, n ) != 0 ) @@ -1874,7 +1874,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_HAVE_TIME) ssl->session_negotiate->start = mbedtls_time( NULL ); #endif +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ssl->session_negotiate->ciphersuite = i; +#endif /* MBEDTLS_SSL_SINGLE_CIPHERSUITE */ ssl->session_negotiate->compression = comp; ssl->session_negotiate->id_len = n; memcpy( ssl->session_negotiate->id, buf + 35, n ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 1963672ea..a97bc3a3a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1037,7 +1037,9 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) size_t n; unsigned int ciph_len, sess_len, chal_len; unsigned char *buf, *p; +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) mbedtls_ssl_ciphersuite_handle_t ciphersuite_info; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello v2" ) ); @@ -1256,7 +1258,9 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) if( ssl_ciphersuite_is_match( ssl, cur_info, NULL ) ) { +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ciphersuite_info = cur_info; +#endif goto have_ciphersuite_v2; } @@ -1289,9 +1293,9 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) have_ciphersuite_v2: +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ssl->session_negotiate->ciphersuite = mbedtls_ssl_suite_get_id( ciphersuite_info ); -#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ssl->handshake->ciphersuite_info = ciphersuite_info; #endif @@ -1341,7 +1345,10 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) int extended_ms_seen = 0; #endif int handshake_failure = 0; + +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) mbedtls_ssl_ciphersuite_handle_t ciphersuite_info; +#endif int major, minor; #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ @@ -2175,7 +2182,9 @@ read_record_header: if( ssl_ciphersuite_is_match( ssl, cur_info, acceptable_ec_grp_ids) ) { +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ciphersuite_info = cur_info; +#endif /* MBEDTLS_SSL_SINGLE_CIPHERSUITE */ goto have_ciphersuite; } @@ -2212,9 +2221,9 @@ read_record_header: have_ciphersuite: +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ssl->session_negotiate->ciphersuite = mbedtls_ssl_suite_get_id( ciphersuite_info ); -#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) ssl->handshake->ciphersuite_info = ciphersuite_info; #endif /* MBEDTLS_SSL_SINGLE_CIPHERSUITE */ @@ -2354,7 +2363,7 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, * encrypt-then-MAC response extension back to the client." */ suite = mbedtls_ssl_ciphersuite_from_id( - ssl->session_negotiate->ciphersuite ); + mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ) ); if( suite == MBEDTLS_SSL_CIPHERSUITE_INVALID_HANDLE ) { *olen = 0; @@ -2695,6 +2704,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) mbedtls_time_t t; #endif int ret; + int ciphersuite; size_t olen, ext_len = 0, n; unsigned char *buf, *p; @@ -2844,12 +2854,13 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "%s session has been resumed", mbedtls_ssl_handshake_get_resume( ssl->handshake ) ? "a" : "no" ) ); - *p++ = (unsigned char)( ssl->session_negotiate->ciphersuite >> 8 ); - *p++ = (unsigned char)( ssl->session_negotiate->ciphersuite ); + ciphersuite = mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ); + *p++ = (unsigned char)( ciphersuite >> 8 ); + *p++ = (unsigned char)( ciphersuite ); *p++ = (unsigned char)( ssl->session_negotiate->compression ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: %s", - mbedtls_ssl_get_ciphersuite_name( ssl->session_negotiate->ciphersuite ) ) ); + mbedtls_ssl_get_ciphersuite_name( ciphersuite ) ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, compress alg.: 0x%02X", ssl->session_negotiate->compression ) ); @@ -2898,7 +2909,8 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if ( mbedtls_ssl_ciphersuite_uses_ec( - mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ) ) ) + mbedtls_ssl_ciphersuite_from_id( + mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ) ) ) ) { ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); ext_len += olen; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index afe32be57..ab48cc4db 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1378,24 +1378,24 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) /* Populate transform structure */ ret = ssl_populate_transform( ssl->transform_negotiate, - ssl->session_negotiate->ciphersuite, - ssl->session_negotiate->master, + mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ), + ssl->session_negotiate->master, #if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) - ssl->session_negotiate->encrypt_then_mac, + ssl->session_negotiate->encrypt_then_mac, #endif #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - ssl->session_negotiate->trunc_hmac, + ssl->session_negotiate->trunc_hmac, #endif #endif /* MBEDTLS_SSL_SOME_MODES_USE_MAC */ #if defined(MBEDTLS_ZLIB_SUPPORT) - ssl->session_negotiate->compression, + ssl->session_negotiate->compression, #endif - ssl->handshake->tls_prf, - ssl->handshake->randbytes, - ssl->minor_ver, - mbedtls_ssl_conf_get_endpoint( ssl->conf ), - ssl ); + ssl->handshake->tls_prf, + ssl->handshake->randbytes, + ssl->minor_ver, + mbedtls_ssl_conf_get_endpoint( ssl->conf ), + ssl ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_populate_transform", ret ); @@ -8968,10 +8968,13 @@ uint32_t mbedtls_ssl_get_verify_result( const mbedtls_ssl_context *ssl ) const char *mbedtls_ssl_get_ciphersuite( const mbedtls_ssl_context *ssl ) { + int suite; + if( ssl == NULL || ssl->session == NULL ) return( NULL ); - return mbedtls_ssl_get_ciphersuite_name( ssl->session->ciphersuite ); + suite = mbedtls_ssl_session_get_ciphersuite( ssl->session ); + return( mbedtls_ssl_get_ciphersuite_name( suite ) ); } const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) @@ -9393,8 +9396,10 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, if( used <= buf_len ) { - *p++ = (unsigned char)( ( session->ciphersuite >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( session->ciphersuite ) & 0xFF ); + const int ciphersuite = + mbedtls_ssl_session_get_ciphersuite( session ); + *p++ = (unsigned char)( ( ciphersuite >> 8 ) & 0xFF ); + *p++ = (unsigned char)( ( ciphersuite ) & 0xFF ); *p++ = (unsigned char)( session->compression & 0xFF ); @@ -9532,6 +9537,7 @@ static int ssl_session_load( mbedtls_ssl_session *session, { const unsigned char *p = buf; const unsigned char * const end = buf + len; + int ciphersuite; #if defined(MBEDTLS_HAVE_TIME) uint64_t start; #endif @@ -9578,12 +9584,23 @@ static int ssl_session_load( mbedtls_ssl_session *session, /* * Basic mandatory fields */ + if( 2 + 1 + 1 + 32 + 48 + 4 > (size_t)( end - p ) ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - session->ciphersuite = ( p[0] << 8 ) | p[1]; + ciphersuite = ( p[0] << 8 ) | p[1]; p += 2; +#if !defined(MBEDTLS_SSL_SINGLE_CIPHERSUITE) + session->ciphersuite = ciphersuite; +#else + if( ciphersuite != + MBEDTLS_SSL_SUITE_ID( MBEDTLS_SSL_SINGLE_CIPHERSUITE ) ) + { + return( MBEDTLS_ERR_SSL_VERSION_MISMATCH ); + } +#endif + session->compression = *p++; session->id_len = *p++; diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 8271b232f..9707935ee 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -279,7 +279,9 @@ static int ssl_populate_session( mbedtls_ssl_session *session, #if defined(MBEDTLS_HAVE_TIME) session->start = mbedtls_time( NULL ) - 42; #endif +#if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) session->ciphersuite = 0xabcd; +#endif session->compression = 1; session->id_len = sizeof( session->id ); memset( session->id, 66, session->id_len ); @@ -698,7 +700,9 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file ) #if defined(MBEDTLS_HAVE_TIME) TEST_ASSERT( original.start == restored.start ); #endif +#if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) TEST_ASSERT( original.ciphersuite == restored.ciphersuite ); +#endif TEST_ASSERT( original.compression == restored.compression ); TEST_ASSERT( original.id_len == restored.id_len ); TEST_ASSERT( memcmp( original.id,