Merge pull request Mbed-TLS#4664 from tom-daubney-arm/rm_truncated_HM…

…AC_ext

Remove truncated HMAC extension

ChangeLog

@@ -32,8 +32,6 @@ API changes
    * Drop support for parsing SSLv2 ClientHello
      (MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO).
    * Drop support for SSLv3 (MBEDTLS_SSL_PROTO_SSL3).
-   * Drop support for compatibility with our own previous buggy
-     implementation of truncated HMAC (MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT).
    * Drop support for TLS record-level compression (MBEDTLS_ZLIB_SUPPORT).
    * Drop support for RC4 TLS ciphersuites.
    * Drop support for single-DES ciphersuites.

ChangeLog.d/rm-truncated-hmac-ext.txt

@@ -0,0 +1,5 @@
+Removals
+   * Remove MBEDTLS_SSL_TRUNCATED_HMAC and also remove
+     MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT config option. Users are better served by
+     using a CCM-8 ciphersuite than a CBC ciphersuite with truncated HMAC.
+     See issue #4341 for more details.

docs/3.0-migration-guide.md

@@ -137,17 +137,16 @@ and relied on that version in order to communicate with peers that are not up
 to date. If one of your peers is in that case, please try contacting them and
 encouraging them to upgrade their software.
 
-Remove support for compatibility with old Mbed TLS's truncated HMAC
--------------------------------------------------------------------
+Remove support for truncated HMAC
+---------------------------------
 
-This doesn't affect people using the default configuration as it was already
-disabled by default.
+This affects users of truncated HMAC, that is, users who called
+`mbedtls_ssl_conf_truncated_hmac( ..., MBEDTLS_SSL_TRUNC_HMAC_ENABLED)`,
+regardless of whether the standard version was used or compatibility version
+(`MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT`).
 
-This only affects TLS users who enabled `MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT` and
-used the Truncated HMAC extension to communicate with peers using old version
-of Mbed TLS. Please consider using a CCM-8 ciphersuite instead of the
-Truncated HMAC extension, or convincing your peer to upgrade their version of
-Mbed TLS.
+The recommended migration path for people who want minimal overhead is to use a
+CCM-8 ciphersuite.
 
 Remove support for TLS record-level compression
 -----------------------------------------------

include/mbedtls/check_config.h

@@ -829,6 +829,10 @@
 #error "MBEDTLS_SSL_TLS1_3_PADDING_GRANULARITY was removed in Mbed TLS 3.0. See https://github.com/ARMmbed/mbedtls/issues/4335"
 #endif
 
+#if defined(MBEDTLS_SSL_TRUNCATED_HMAC) //no-check-names
+#error "MBEDTLS_SSL_TRUNCATED_HMAC was removed in Mbed TLS 3.0. See https://github.com/ARMmbed/mbedtls/issues/4341"
+#endif
+
 /*
  * Avoid warning from -pedantic. This is a convenient place for this
  * workaround since this is included by every single file before the

include/mbedtls/config.h

@@ -1617,15 +1617,6 @@
  */
 #define MBEDTLS_SSL_SERVER_NAME_INDICATION
 
-/**
- * \def MBEDTLS_SSL_TRUNCATED_HMAC
- *
- * Enable support for RFC 6066 truncated HMAC in SSL.
- *
- * Comment this macro to disable support for truncated HMAC in SSL
- */
-#define MBEDTLS_SSL_TRUNCATED_HMAC
-
 /**
  * \def MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH
  *

include/mbedtls/ssl.h

@@ -958,10 +958,6 @@ struct mbedtls_ssl_session
     unsigned char MBEDTLS_PRIVATE(mfl_code);     /*!< MaxFragmentLength negotiated by peer */
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-    int MBEDTLS_PRIVATE(trunc_hmac);             /*!< flag for truncated hmac activation   */
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
     int MBEDTLS_PRIVATE(encrypt_then_mac);       /*!< flag for EtM activation                */
 #endif
@@ -1182,9 +1178,6 @@ struct mbedtls_ssl_config
 #if defined(MBEDTLS_SSL_RENEGOTIATION)
     unsigned int MBEDTLS_PRIVATE(disable_renegotiation) : 1; /*!< disable renegotiation?     */
 #endif
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-    unsigned int MBEDTLS_PRIVATE(trunc_hmac) : 1;    /*!< negotiate truncated hmac?          */
-#endif
 #if defined(MBEDTLS_SSL_SESSION_TICKETS)
     unsigned int MBEDTLS_PRIVATE(session_tickets) : 1;   /*!< use session tickets?           */
 #endif
@@ -3330,18 +3323,6 @@ int mbedtls_ssl_conf_max_frag_len( mbedtls_ssl_config *conf, unsigned char mfl_c
 void mbedtls_ssl_conf_preference_order( mbedtls_ssl_config *conf, int order );
 #endif /* MBEDTLS_SSL_SRV_C */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-/**
- * \brief          Activate negotiation of truncated HMAC
- *                 (Default: MBEDTLS_SSL_TRUNC_HMAC_DISABLED)
- *
- * \param conf     SSL configuration
- * \param truncate Enable or disable (MBEDTLS_SSL_TRUNC_HMAC_ENABLED or
- *                                    MBEDTLS_SSL_TRUNC_HMAC_DISABLED)
- */
-void mbedtls_ssl_conf_truncated_hmac( mbedtls_ssl_config *conf, int truncate );
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C)
 /**
  * \brief          Enable / Disable session tickets (client only).

library/ssl_cli.c

@@ -559,36 +559,6 @@ static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl,
 }
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-static int ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl,
-                                         unsigned char *buf,
-                                         const unsigned char *end,
-                                         size_t *olen )
-{
-    unsigned char *p = buf;
-
-    *olen = 0;
-
-    if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED )
-        return( 0 );
-
-    MBEDTLS_SSL_DEBUG_MSG( 3,
-        ( "client hello, adding truncated_hmac extension" ) );
-
-    MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 );
-
-    *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF );
-    *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC      ) & 0xFF );
-
-    *p++ = 0x00;
-    *p++ = 0x00;
-
-    *olen = 4;
-
-    return( 0 );
-}
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
 static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl,
                                            unsigned char *buf,
@@ -1304,16 +1274,6 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl )
     ext_len += olen;
 #endif
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-    if( ( ret = ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len,
-                                              end, &olen ) ) != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_truncated_hmac_ext", ret );
-        return( ret );
-    }
-    ext_len += olen;
-#endif
-
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
     if( ( ret = ssl_write_encrypt_then_mac_ext( ssl, p + 2 + ext_len,
                                                 end, &olen ) ) != 0 )
@@ -1479,31 +1439,6 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl,
 }
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl,
-                                         const unsigned char *buf,
-                                         size_t len )
-{
-    if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ||
-        len != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_MSG( 1,
-            ( "non-matching truncated HMAC extension" ) );
-        mbedtls_ssl_send_alert_message(
-            ssl,
-            MBEDTLS_SSL_ALERT_LEVEL_FATAL,
-            MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE );
-        return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO );
-    }
-
-    ((void) buf);
-
-    ssl->session_negotiate->trunc_hmac = MBEDTLS_SSL_TRUNC_HMAC_ENABLED;
-
-    return( 0 );
-}
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
 static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl,
                               const unsigned char *buf,
@@ -2346,19 +2281,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl )
             break;
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-        case MBEDTLS_TLS_EXT_TRUNCATED_HMAC:
-            MBEDTLS_SSL_DEBUG_MSG( 3, ( "found truncated_hmac extension" ) );
-
-            if( ( ret = ssl_parse_truncated_hmac_ext( ssl,
-                            ext + 4, ext_size ) ) != 0 )
-            {
-                return( ret );
-            }
-
-            break;
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
         case MBEDTLS_TLS_EXT_CID:
             MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) );

library/ssl_srv.c

@@ -543,28 +543,6 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl,
 }
 #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl,
-                                         const unsigned char *buf,
-                                         size_t len )
-{
-    if( len != 0 )
-    {
-        MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) );
-        mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
-                                        MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR );
-        return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO );
-    }
-
-    ((void) buf);
-
-    if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_ENABLED )
-        ssl->session_negotiate->trunc_hmac = MBEDTLS_SSL_TRUNC_HMAC_ENABLED;
-
-    return( 0 );
-}
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
 static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl,
                                       const unsigned char *buf,
@@ -1703,16 +1681,6 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl )
                 break;
 #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-            case MBEDTLS_TLS_EXT_TRUNCATED_HMAC:
-                MBEDTLS_SSL_DEBUG_MSG( 3, ( "found truncated hmac extension" ) );
-
-                ret = ssl_parse_truncated_hmac_ext( ssl, ext + 4, ext_size );
-                if( ret != 0 )
-                    return( ret );
-                break;
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
             case MBEDTLS_TLS_EXT_CID:
                 MBEDTLS_SSL_DEBUG_MSG( 3, ( "found CID extension" ) );
@@ -1721,7 +1689,7 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl )
                 if( ret != 0 )
                     return( ret );
                 break;
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
+#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
 
 #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
             case MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC:
@@ -1967,31 +1935,6 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl )
     return( 0 );
 }
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl,
-                                          unsigned char *buf,
-                                          size_t *olen )
-{
-    unsigned char *p = buf;
-
-    if( ssl->session_negotiate->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED )
-    {
-        *olen = 0;
-        return;
-    }
-
-    MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, adding truncated hmac extension" ) );
-
-    *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF );
-    *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC      ) & 0xFF );
-
-    *p++ = 0x00;
-    *p++ = 0x00;
-
-    *olen = 4;
-}
-#endif /* MBEDTLS_SSL_TRUNCATED_HMAC */
-
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
 static void ssl_write_cid_ext( mbedtls_ssl_context *ssl,
                                unsigned char *buf,
@@ -2654,11 +2597,6 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl )
     ext_len += olen;
 #endif
 
-#if defined(MBEDTLS_SSL_TRUNCATED_HMAC)
-    ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len, &olen );
-    ext_len += olen;
-#endif
-
 #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
     ssl_write_cid_ext( ssl, p + 2 + ext_len, &olen );
     ext_len += olen;