commit 5c8434cf524d33a57c89bedd1a2ad2750b3a40c9
author Manuel Pégourié-Gonnard <mpg@elzevir.fr> Mon Oct 28 13:46:11 2013 +0100
committer Paul Bakker <p.j.bakker@polarssl.org> Mon Jul 07 14:10:07 2014 +0200
tree 335b8e3422eb99960482776ef28d111a9efbfc1f
parent 79f1ff84edc219a6b3506df93763583086322e5e

Safer buffer comparisons in the SSL modules

include/polarssl/ssl.h

diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 6383c0e..0900a2c 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -1136,6 +1136,20 @@
 void ssl_optimize_checksum( ssl_context *ssl, int ciphersuite );
 int ssl_get_ciphersuite_min_version( const int ciphersuite_id );
 
+/* constant-time buffer comparison */
+static inline int safer_memcmp( const void *a, const void *b, size_t n )
+{
+    size_t i;
+    const unsigned char *A = (const unsigned char *) a;
+    const unsigned char *B = (const unsigned char *) b;
+    unsigned char diff = 0;
+
+    for( i = 0; i < n; i++ )
+        diff |= A[i] ^ B[i];
+
+    return( diff );
+}
+
 #ifdef __cplusplus
 }
 #endif

library/ssl_cli.c

diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 4ebee35..fc82002 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -341,11 +341,13 @@
     }
     else
     {
+        /* Check verify-data in constant-time. The length OTOH is no secret */
         if( len    != 1 + ssl->verify_data_len * 2 ||
             buf[0] !=     ssl->verify_data_len * 2 ||
-            memcmp( buf + 1, ssl->own_verify_data,  ssl->verify_data_len ) != 0 ||
-            memcmp( buf + 1 + ssl->verify_data_len,
-                    ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
+            safer_memcmp( buf + 1,
+                          ssl->own_verify_data, ssl->verify_data_len ) != 0 ||
+            safer_memcmp( buf + 1 + ssl->verify_data_len,
+                          ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
         {
             SSL_DEBUG_MSG( 1, ( "non-matching renegotiated connection field" ) );
 

library/ssl_srv.c

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index d166986..ea1d63e 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -106,9 +106,11 @@
     }
     else
     {
+        /* Check verify-data in constant-time. The length OTOH is no secret */
         if( len    != 1 + ssl->verify_data_len ||
             buf[0] !=     ssl->verify_data_len ||
-            memcmp( buf + 1, ssl->peer_verify_data, ssl->verify_data_len ) != 0 )
+            safer_memcmp( buf + 1, ssl->peer_verify_data,
+                          ssl->verify_data_len ) != 0 )
         {
             SSL_DEBUG_MSG( 1, ( "non-matching renegotiated connection field" ) );
 

library/ssl_tls.c

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 15cb3bc..a8cc501 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1547,8 +1547,8 @@
     SSL_DEBUG_BUF( 4, "computed mac", ssl->in_msg + ssl->in_msglen,
                    ssl->transform_in->maclen );
 
-    if( memcmp( tmp, ssl->in_msg + ssl->in_msglen,
-                     ssl->transform_in->maclen ) != 0 )
+    if( safer_memcmp( tmp, ssl->in_msg + ssl->in_msglen,
+                      ssl->transform_in->maclen ) != 0 )
     {
 #if defined(POLARSSL_SSL_DEBUG_ALL)
         SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
@@ -2886,7 +2886,7 @@
         return( POLARSSL_ERR_SSL_BAD_HS_FINISHED );
     }
 
-    if( memcmp( ssl->in_msg + 4, buf, hash_len ) != 0 )
+    if( safer_memcmp( ssl->in_msg + 4, buf, hash_len ) != 0 )
     {
         SSL_DEBUG_MSG( 1, ( "bad finished message" ) );
         return( POLARSSL_ERR_SSL_BAD_HS_FINISHED );