From 8844055b0ee81f44e5b4f1cf4b189ba4bec5f39d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 Jul 2019 14:16:13 +0100 Subject: [PATCH] Remove compression field from SSL session if compression disabled --- include/mbedtls/ssl.h | 2 + include/mbedtls/ssl_internal.h | 15 +++++++ library/ssl_cache.c | 3 +- library/ssl_cli.c | 6 ++- library/ssl_srv.c | 9 ++-- library/ssl_tls.c | 67 ++++++++++++++++++++++++---- tests/suites/test_suite_ssl.function | 4 ++ 7 files changed, 91 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index edb0a4699..eef01f683 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -939,7 +939,9 @@ struct mbedtls_ssl_session #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) int ciphersuite; /*!< chosen ciphersuite */ #endif /* MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ +#if defined(MBEDTLS_ZLIB_SUPPORT) int compression; /*!< chosen compression */ +#endif /* MBEDTLS_ZLIB_SUPPORT */ size_t id_len; /*!< session id length */ unsigned char id[32]; /*!< session identifier */ unsigned char master[48]; /*!< the master secret */ diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 9b8e21fd0..6f7773016 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1780,4 +1780,19 @@ MBEDTLS_ALWAYS_INLINE static inline void mbedtls_ssl_pend_fatal_alert( ssl->pending_fatal_alert_msg = message; } +/* + * Getter functions for fields in SSL session. + */ + +static inline int mbedtls_ssl_session_get_compression( + mbedtls_ssl_session const *session ) +{ +#if defined(MBEDTLS_ZLIB_SUPPORT) + return( session->compression ); +#else + ( (void) session ); + return( MBEDTLS_SSL_COMPRESS_NULL ); +#endif +} + #endif /* ssl_internal.h */ diff --git a/library/ssl_cache.c b/library/ssl_cache.c index bcc2f59d1..90e2a81ca 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -86,7 +86,8 @@ int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session ) if( mbedtls_ssl_session_get_ciphersuite( session ) != mbedtls_ssl_session_get_ciphersuite( &entry->session ) || - session->compression != entry->session.compression || + mbedtls_ssl_session_get_compression( session ) != + mbedtls_ssl_session_get_compression( &entry->session ) || session->id_len != entry->session.id_len ) { continue; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 003aa10c9..cdb35f3c5 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1837,7 +1837,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( n == 0 || mbedtls_ssl_get_renego_status( ssl ) != MBEDTLS_SSL_INITIAL_HANDSHAKE || mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ) != i || - ssl->session_negotiate->compression != comp || + mbedtls_ssl_session_get_compression( ssl->session_negotiate ) != comp || ssl->session_negotiate->id_len != n || memcmp( ssl->session_negotiate->id, buf + 35, n ) != 0 ) { @@ -1868,7 +1868,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) ssl->session_negotiate->ciphersuite = i; #endif /* MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ +#if defined(MBEDTLS_ZLIB_SUPPORT) ssl->session_negotiate->compression = comp; +#endif ssl->session_negotiate->id_len = n; memcpy( ssl->session_negotiate->id, buf + 35, n ); } @@ -1932,7 +1934,9 @@ server_picked_valid_suite: MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } +#if defined(MBEDTLS_ZLIB_SUPPORT) ssl->session_negotiate->compression = comp; +#endif ext = buf + 40 + n; diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 573f32769..bcc105254 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1802,8 +1802,8 @@ read_record_header: MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, compression", buf + comp_offset + 1, comp_len ); - ssl->session_negotiate->compression = MBEDTLS_SSL_COMPRESS_NULL; #if defined(MBEDTLS_ZLIB_SUPPORT) + ssl->session_negotiate->compression = MBEDTLS_SSL_COMPRESS_NULL; for( i = 0; i < comp_len; ++i ) { if( buf[comp_offset + 1 + i] == MBEDTLS_SSL_COMPRESS_DEFLATE ) @@ -1812,13 +1812,13 @@ read_record_header: break; } } -#endif /* See comments in ssl_write_client_hello() */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) ssl->session_negotiate->compression = MBEDTLS_SSL_COMPRESS_NULL; #endif +#endif /* MBEDTLS_ZLIB_SUPPORT */ /* Do not parse the extensions if the protocol is SSLv3 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) @@ -2881,12 +2881,13 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) 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 ); + *p++ = (unsigned char)( + mbedtls_ssl_session_get_compression( ssl->session_negotiate ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: %s", mbedtls_ssl_get_ciphersuite_name( ciphersuite ) ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, compress alg.: 0x%02X", - ssl->session_negotiate->compression ) ); + mbedtls_ssl_session_get_compression( ssl->session_negotiate ) ) ); /* Do not write the extensions if the protocol is SSLv3 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d04602027..95a5e9c2c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -9327,6 +9327,12 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_co #define SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT 0 #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +#if defined(MBEDTLS_ZLIB_SUPPORT) +#define SSL_SERIALIZED_SESSION_CONFIG_COMPRESSION 1 +#else +#define SSL_SERIALIZED_SESSION_CONFIG_COMPRESSION 0 +#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + #define SSL_SERIALIZED_SESSION_CONFIG_TIME_BIT 0 #define SSL_SERIALIZED_SESSION_CONFIG_CRT_BIT 1 #define SSL_SERIALIZED_SESSION_CONFIG_CLIENT_TICKET_BIT 2 @@ -9335,6 +9341,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_co #define SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT 5 #define SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT 6 #define SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT 7 +#define SSL_SERIALIZED_SESSION_CONFIG_COMPRESSION_BIT 8 #define SSL_SERIALIZED_SESSION_CONFIG_BITFLAG \ ( (uint16_t) ( \ @@ -9345,6 +9352,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer( const mbedtls_ssl_co ( SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC << SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT ) | \ ( SSL_SERIALIZED_SESSION_CONFIG_ETM << SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT ) | \ ( SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT ) | \ + ( SSL_SERIALIZED_SESSION_CONFIG_COMPRESSION << SSL_SERIALIZED_SESSION_CONFIG_COMPRESSION_BIT ) | \ ( SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT << SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT ) ) ) static unsigned char ssl_serialized_session_header[] = { @@ -9450,12 +9458,28 @@ static int ssl_session_save( const mbedtls_ssl_session *session, /* * Basic mandatory fields */ - used += 2 /* ciphersuite */ - + 1 /* compression */ - + 1 /* id_len */ - + sizeof( session->id ) - + sizeof( session->master ) - + 4; /* verify_result */ + { + size_t const ciphersuite_len = 2; +#if defined(MBEDTLS_ZLIB_SUPPORT) + size_t const compression_len = 1; +#else + size_t const compression_len = 0; +#endif + size_t const id_len_len = 1; + size_t const id_len = 32; + size_t const master_len = 48; + size_t const verif_result_len = 4; + + size_t const basic_len = + ciphersuite_len + + compression_len + + id_len_len + + id_len + + master_len + + verif_result_len; + + used += basic_len; + } if( used <= buf_len ) { @@ -9464,7 +9488,10 @@ static int ssl_session_save( const mbedtls_ssl_session *session, *p++ = (unsigned char)( ( ciphersuite >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( ciphersuite ) & 0xFF ); - *p++ = (unsigned char)( session->compression & 0xFF ); +#if defined(MBEDTLS_ZLIB_SUPPORT) + *p++ = (unsigned char)( + mbedtls_ssl_session_get_compression( session ) ); +#endif *p++ = (unsigned char)( session->id_len & 0xFF ); memcpy( p, session->id, 32 ); @@ -9662,9 +9689,29 @@ static int ssl_session_load( mbedtls_ssl_session *session, /* * Basic mandatory fields */ + { + size_t const ciphersuite_len = 2; +#if defined(MBEDTLS_ZLIB_SUPPORT) + size_t const compression_len = 1; +#else + size_t const compression_len = 0; +#endif + size_t const id_len_len = 1; + size_t const id_len = 32; + size_t const master_len = 48; + size_t const verif_result_len = 4; - if( 2 + 1 + 1 + 32 + 48 + 4 > (size_t)( end - p ) ) - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + size_t const basic_len = + ciphersuite_len + + compression_len + + id_len_len + + id_len + + master_len + + verif_result_len; + + if( basic_len > (size_t)( end - p ) ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } ciphersuite = ( p[0] << 8 ) | p[1]; p += 2; @@ -9679,7 +9726,9 @@ static int ssl_session_load( mbedtls_ssl_session *session, } #endif +#if defined(MBEDTLS_ZLIB_SUPPORT) session->compression = *p++; +#endif session->id_len = *p++; memcpy( session->id, p, 32 ); diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 7d7845e77..2c4a6b4b2 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -286,7 +286,9 @@ static int ssl_populate_session( mbedtls_ssl_session *session, #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) session->ciphersuite = 0xabcd; #endif +#if defined(MBEDTLS_ZLIB_SUPPORT) session->compression = 1; +#endif session->id_len = sizeof( session->id ); memset( session->id, 66, session->id_len ); memset( session->master, 17, sizeof( session->master ) ); @@ -716,7 +718,9 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file ) #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) TEST_ASSERT( original.ciphersuite == restored.ciphersuite ); #endif +#if defined(MBEDTLS_ZLIB_SUPPORT) TEST_ASSERT( original.compression == restored.compression ); +#endif TEST_ASSERT( original.id_len == restored.id_len ); TEST_ASSERT( memcmp( original.id, restored.id, sizeof( original.id ) ) == 0 );