From 5c8434cf524d33a57c89bedd1a2ad2750b3a40c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= <mpg@elzevir.fr>
Date: Mon, 28 Oct 2013 13:46:11 +0100
Subject: [PATCH] Safer buffer comparisons in the SSL modules

---
 include/polarssl/ssl.h | 14 ++++++++++++++
 library/ssl_cli.c      |  8 +++++---
 library/ssl_srv.c      |  4 +++-
 library/ssl_tls.c      |  6 +++---
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 6383c0e6af..0900a2c673 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -1136,6 +1136,20 @@ int ssl_write_finished( ssl_context *ssl );
 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
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 4ebee3529b..fc82002309 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -341,11 +341,13 @@ static int ssl_parse_renegotiation_info( ssl_context *ssl,
     }
     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" ) );
 
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index d1669864c0..ea1d63eaa4 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -106,9 +106,11 @@ static int ssl_parse_renegotiation_info( ssl_context *ssl,
     }
     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" ) );
 
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 15cb3bc2b0..a8cc501aa4 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1547,8 +1547,8 @@ static int ssl_decrypt_buf( ssl_context *ssl )
     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 @@ int ssl_parse_finished( ssl_context *ssl )
         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 );