commit d5ba50e239fdb117172ba8162db607c5ad189577
author Gilles Peskine <Gilles.Peskine@arm.com> Fri Dec 10 21:33:21 2021 +0100
committer Gilles Peskine <Gilles.Peskine@arm.com> Sat Dec 11 14:59:45 2021 +0100
tree d523ff5a1c67700a03f45c07b2e18b43d443a70e
parent 476b157dc44cc9c94b9accb5255f020d62510a17

Zeroize local MAC variables

Zeroize local MAC variables used for CBC+HMAC cipher suites. In encryption,
this is just good hygiene but probably not needed for security since the
data protected by the MAC that could leak is about to be transmitted anyway.
In DTLS decryption, this could be a security issue since an adversary could
learn the MAC of data that they were trying to inject. At least with
encrypt-then-MAC, the adversary could then easily inject a datagram with
a corrected packet. TLS would still be safe since the receiver would close
the connection after the bad MAC.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

ChangeLog.d/ssl-mac-zeroize.txt

diff --git a/ChangeLog.d/ssl-mac-zeroize.txt b/ChangeLog.d/ssl-mac-zeroize.txt
new file mode 100644
index 0000000..b49c7ac
--- /dev/null
+++ b/ChangeLog.d/ssl-mac-zeroize.txt
@@ -0,0 +1,5 @@
+Security
+   * Zeroize intermediate variables used to calculate the MAC in CBC cipher
+     suites. This hardens the library in case stack memory leaks through a
+     memory disclosure vulnerabilty, which could formerly have allowed a
+     man-in-the-middle to inject fake ciphertext into a DTLS connection.

library/ssl_msg.c

diff --git a/library/ssl_msg.c b/library/ssl_msg.c
index f7e40b1..2d06fdd 100644
--- a/library/ssl_msg.c
+++ b/library/ssl_msg.c
@@ -685,6 +685,7 @@
         rec->data_len += transform->maclen;
         post_avail -= transform->maclen;
         auth_done++;
+        mbedtls_platform_zeroize( mac, transform->maclen );
     }
 #endif /* MBEDTLS_SSL_SOME_SUITES_USE_MAC */
 
@@ -939,6 +940,7 @@
             rec->data_len += transform->maclen;
             post_avail -= transform->maclen;
             auth_done++;
+            mbedtls_platform_zeroize( mac, transform->maclen );
         }
 #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */
     }
@@ -1222,13 +1224,20 @@
                                    transform->maclen );
 
             /* Compare expected MAC with MAC at the end of the record. */
+            ret = 0;
             if( mbedtls_ct_memcmp( data + rec->data_len, mac_expect,
                                               transform->maclen ) != 0 )
             {
                 MBEDTLS_SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
-                return( MBEDTLS_ERR_SSL_INVALID_MAC );
+                ret = MBEDTLS_ERR_SSL_INVALID_MAC;
+                goto hmac_failed_etm_enabled;
             }
             auth_done++;
+
+        hmac_failed_etm_enabled:
+            mbedtls_platform_zeroize( mac_expect, transform->maclen );
+            if( ret != 0 )
+                return( ret );
         }
 #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */
 
@@ -1420,7 +1429,7 @@
         if( ret != 0 )
         {
             MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ct_hmac", ret );
-            return( ret );
+            goto hmac_failed_etm_disabled;
         }
 
         mbedtls_ct_memcpy_offset( mac_peer, data,
@@ -1443,6 +1452,12 @@
             correct = 0;
         }
         auth_done++;
+
+    hmac_failed_etm_disabled:
+        mbedtls_platform_zeroize( mac_peer, transform->maclen );
+        mbedtls_platform_zeroize( mac_expect, transform->maclen );
+        if( ret != 0 )
+            return( ret );
     }
 
     /*