Merge pull request #5963 from tom-daubney-arm/remove_ssl_compression_new

Remove use of SSL session compression

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.

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  */

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;

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;
 

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 ||
+    if( session->ciphersuite != session_tmp.ciphersuite )
-        session->compression != session_tmp.compression )
     {
         /* 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

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;

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>;

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,7 +4720,6 @@ 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 );