diff --git a/ChangeLog.d/remove_ssl_session_compression.txt b/ChangeLog.d/remove_ssl_session_compression.txt new file mode 100644 index 000000000..dc59f1c9d --- /dev/null +++ b/ChangeLog.d/remove_ssl_session_compression.txt @@ -0,0 +1,5 @@ +Removals + * Remove compression property from SSL session struct. + MBEDTLS_SSL_COMPRESS_NULL is now the only supported + compression option and can be used for compatibility + reasons. Changes requested in #4223. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 5099cfb6b..d3881b205 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1159,7 +1159,6 @@ struct mbedtls_ssl_session mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< starting time */ #endif int MBEDTLS_PRIVATE(ciphersuite); /*!< chosen ciphersuite */ - int MBEDTLS_PRIVATE(compression); /*!< chosen compression */ size_t MBEDTLS_PRIVATE(id_len); /*!< session id length */ unsigned char MBEDTLS_PRIVATE(id)[32]; /*!< session identifier */ unsigned char MBEDTLS_PRIVATE(master)[48]; /*!< the master secret */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 76b31ef72..eefd89dd9 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7322,7 +7322,6 @@ static mbedtls_tls_prf_types tls_prf_get_type( mbedtls_ssl_tls_prf_cb *tls_prf ) * - [in] ciphersuite * - [in] master * - [in] encrypt_then_mac - * - [in] compression * - [in] tls_prf: pointer to PRF to use for key derivation * - [in] randbytes: buffer holding ServerHello.random + ClientHello.random * - [in] tls_version: TLS version @@ -7998,7 +7997,6 @@ unsigned int mbedtls_ssl_tls12_get_preferred_hash_for_sig_alg( * struct { * uint64 start_time; * uint8 ciphersuite[2]; // defined by the standard - * uint8 compression; // 0 or 1 * uint8 session_id_len; // at most 32 * opaque session_id[32]; * opaque master[48]; // fixed length in the standard @@ -8046,7 +8044,6 @@ static size_t ssl_tls12_session_save( const mbedtls_ssl_session *session, * Basic mandatory fields */ used += 2 /* ciphersuite */ - + 1 /* compression */ + 1 /* id_len */ + sizeof( session->id ) + sizeof( session->master ) @@ -8057,8 +8054,6 @@ static size_t ssl_tls12_session_save( const mbedtls_ssl_session *session, MBEDTLS_PUT_UINT16_BE( session->ciphersuite, p, 0 ); p += 2; - *p++ = MBEDTLS_BYTE_0( session->compression ); - *p++ = MBEDTLS_BYTE_0( session->id_len ); memcpy( p, session->id, 32 ); p += 32; @@ -8202,14 +8197,12 @@ static int ssl_tls12_session_load( mbedtls_ssl_session *session, /* * Basic mandatory fields */ - if( 2 + 1 + 1 + 32 + 48 + 4 > (size_t)( end - p ) ) + if( 2 + 1 + 32 + 48 + 4 > (size_t)( end - p ) ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); session->ciphersuite = ( p[0] << 8 ) | p[1]; p += 2; - session->compression = *p++; - session->id_len = *p++; memcpy( session->id, p, 32 ); p += 32; diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index 240520854..8a109698c 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1413,7 +1413,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE || #endif ssl->session_negotiate->ciphersuite != i || - ssl->session_negotiate->compression != comp || ssl->session_negotiate->id_len != n || memcmp( ssl->session_negotiate->id, buf + 35, n ) != 0 ) { @@ -1423,7 +1422,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->session_negotiate->start = mbedtls_time( NULL ); #endif ssl->session_negotiate->ciphersuite = i; - ssl->session_negotiate->compression = comp; ssl->session_negotiate->id_len = n; memcpy( ssl->session_negotiate->id, buf + 35, n ); } @@ -1486,8 +1484,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) } #endif - if( comp != MBEDTLS_SSL_COMPRESS_NULL - ) + if( comp != MBEDTLS_SSL_COMPRESS_NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); mbedtls_ssl_send_alert_message( @@ -1496,7 +1493,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - ssl->session_negotiate->compression = comp; ext = buf + 40 + n; diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 4e18e6285..bc32327c1 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1298,7 +1298,10 @@ read_record_header: buf + ciph_offset + 2, ciph_len ); /* - * Check the compression algorithms length and pick one + * Check the compression algorithm's length. + * The list contents are ignored because implementing + * MBEDTLS_SSL_COMPRESS_NULL is mandatory and is the only + * option supported by Mbed TLS. */ comp_offset = ciph_offset + 2 + ciph_len; @@ -1317,12 +1320,6 @@ 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; - /* See comments in ssl_write_client_hello() */ -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - ssl->session_negotiate->compression = MBEDTLS_SSL_COMPRESS_NULL; -#endif /* * Check the extension length */ @@ -2180,8 +2177,7 @@ static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl ) if( ret != 0 ) goto exit; - if( session->ciphersuite != session_tmp.ciphersuite || - session->compression != session_tmp.compression ) + if( session->ciphersuite != session_tmp.ciphersuite ) { /* Mismatch between cached and negotiated session */ goto exit; @@ -2331,12 +2327,12 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_PUT_UINT16_BE( ssl->session_negotiate->ciphersuite, p, 0 ); p += 2; - *p++ = MBEDTLS_BYTE_0( ssl->session_negotiate->compression ); + *p++ = MBEDTLS_BYTE_0( MBEDTLS_SSL_COMPRESS_NULL ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: %s", mbedtls_ssl_get_ciphersuite_name( ssl->session_negotiate->ciphersuite ) ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, compress alg.: 0x%02X", - (unsigned int) ssl->session_negotiate->compression ) ); + (unsigned int) MBEDTLS_SSL_COMPRESS_NULL ) ); /* * First write extensions, then the total length diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 91c205801..505f8dda8 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1461,7 +1461,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, * ... */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - if( p[0] != 0 ) + if( p[0] != MBEDTLS_SSL_COMPRESS_NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) ); fatal_alert = MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 701d044f3..4319ec3ca 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1779,7 +1779,7 @@ static int ssl_tls13_write_server_hello_body( mbedtls_ssl_context *ssl, * ... */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); - *p++ = 0x0; + *p++ = MBEDTLS_SSL_COMPRESS_NULL; /* ... * Extension extensions<6..2^16-1>; diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 60f6e0d27..4e291ce08 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1733,7 +1733,6 @@ static int ssl_tls12_populate_session( mbedtls_ssl_session *session, #endif session->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; session->ciphersuite = 0xabcd; - session->compression = 1; session->id_len = sizeof( session->id ); memset( session->id, 66, session->id_len ); memset( session->master, 17, sizeof( session->master ) ); @@ -4721,12 +4720,11 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file, #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( tls_version == MBEDTLS_SSL_VERSION_TLS1_2 ) { - TEST_ASSERT( original.compression == restored.compression ); TEST_ASSERT( original.id_len == restored.id_len ); TEST_ASSERT( memcmp( original.id, - restored.id, sizeof( original.id ) ) == 0 ); + restored.id, sizeof( original.id ) ) == 0 ); TEST_ASSERT( memcmp( original.master, - restored.master, sizeof( original.master ) ) == 0 ); + restored.master, sizeof( original.master ) ) == 0 ); #if defined(MBEDTLS_X509_CRT_PARSE_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)