From d1cf6d68cc700c6b839787a0c520bcd9106f962f Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Mon, 26 Jun 2017 13:42:44 +0100
Subject: [PATCH 01/36] Prevent clever optimization to prematurely quit loop in
 safe memcmp

The previous version of `ssl_safer_memcmp` did not qualify the
pointers to the arrays to be compared as volatile, theoretically
opening the possibility for the compiler to notice that the loop
operation `diff |= A[i] ^ B[i]` is pointless if `diff = -1`. This
commit changes this. It also declares the stack variable `diff` as
volatile, to force read and write in every loop; omitting that, the
compiler would still be allowed to get away with reading `A[i]` and
`B[i]` but not doing the XOR and not updating `diff`.
---
 include/polarssl/ssl.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 4a01bbf4c..3a0ac12d2 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -2053,9 +2053,9 @@ int ssl_check_cert_usage( const x509_crt *cert,
 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;
+    volatile const unsigned char *A = (volatile const unsigned char *) a;
+    volatile const unsigned char *B = (volatile const unsigned char *) b;
+    volatile unsigned char diff = 0;
 
     for( i = 0; i < n; i++ )
         diff |= A[i] ^ B[i];

From b2ee6b432e5a05ac12399515fbce710ba03e5c3d Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Mon, 26 Jun 2017 13:52:14 +0100
Subject: [PATCH 02/36] Prevent bounds check bypass through overflow in PSK
 identity parsing

The check `if( *p + n > end )` in `ssl_parse_client_psk_identity` is
unsafe because `*p + n` might overflow, thus bypassing the check. As
`n` is a user-specified value up to 65K, this is relevant if the
library happens to be located in the last 65K of virtual memory.

This commit replaces the check by a safe version.
---
 library/ssl_srv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 146f28310..57ad26299 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -3145,7 +3145,7 @@ static int ssl_parse_client_psk_identity( ssl_context *ssl, unsigned char **p,
     /*
      * Receive client pre-shared key identity name
      */
-    if( *p + 2 > end )
+    if( end - *p < 2 )
     {
         SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
         return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );
@@ -3154,7 +3154,7 @@ static int ssl_parse_client_psk_identity( ssl_context *ssl, unsigned char **p,
     n = ( (*p)[0] << 8 ) | (*p)[1];
     *p += 2;
 
-    if( n < 1 || n > 65535 || *p + n > end )
+    if( n < 1 || n > 65535 || n > (size_t) ( end - *p ) )
     {
         SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
         return( POLARSSL_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );

From f148312db4580e835996ebf8ad60dfd1092a67d5 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:21:30 +0100
Subject: [PATCH 03/36] Zeroize tmp buf on fail in load_file() dhm.c

---
 library/dhm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/library/dhm.c b/library/dhm.c
index 0a4f82028..6109e0a7a 100644
--- a/library/dhm.c
+++ b/library/dhm.c
@@ -532,7 +532,10 @@ static int load_file( const char *path, unsigned char **buf, size_t *n )
     if( fread( *buf, 1, *n, f ) != *n )
     {
         fclose( f );
+
+        polarssl_zeroize( *buf, *n + 1 );
         polarssl_free( *buf );
+
         return( POLARSSL_ERR_DHM_FILE_IO_ERROR );
     }
 

From fa6fa6850e2d8c45a6780eeb866ba32f6b3f785b Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:32:27 +0100
Subject: [PATCH 04/36] Zeroize tmp bufs in entropy.c functions

---
 library/entropy.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/library/entropy.c b/library/entropy.c
index 540a27c57..caff22f42 100644
--- a/library/entropy.c
+++ b/library/entropy.c
@@ -210,7 +210,7 @@ static int entropy_gather_internal( entropy_context *ctx )
         if( ( ret = ctx->source[i].f_source( ctx->source[i].p_source,
                         buf, ENTROPY_MAX_GATHER, &olen ) ) != 0 )
         {
-            return( ret );
+            goto cleanup;
         }
 
         /*
@@ -223,7 +223,10 @@ static int entropy_gather_internal( entropy_context *ctx )
         }
     }
 
-    return( 0 );
+cleanup:
+    polarssl_zeroize( buf, sizeof( buf ) );
+
+    return( ret );
 }
 
 /*
@@ -324,6 +327,8 @@ int entropy_func( void *data, unsigned char *output, size_t len )
     ret = 0;
 
 exit:
+    polarssl_zeroize( buf, sizeof( buf ) );
+
 #if defined(POLARSSL_THREADING_C)
     if( polarssl_mutex_unlock( &ctx->mutex ) != 0 )
         return( POLARSSL_ERR_THREADING_MUTEX_ERROR );
@@ -354,12 +359,15 @@ int entropy_write_seed_file( entropy_context *ctx, const char *path )
     ret = 0;
 
 exit:
+    polarssl_zeroize( buf, sizeof( buf ) );
+
     fclose( f );
     return( ret );
 }
 
 int entropy_update_seed_file( entropy_context *ctx, const char *path )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     unsigned char buf[ ENTROPY_MAX_SEED_SIZE ];
@@ -375,14 +383,16 @@ int entropy_update_seed_file( entropy_context *ctx, const char *path )
         n = ENTROPY_MAX_SEED_SIZE;
 
     if( fread( buf, 1, n, f ) != n )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_ENTROPY_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_ENTROPY_FILE_IO_ERROR;
+    else
+        ret = entropy_update_manual( ctx, buf, n );
 
     fclose( f );
 
-    entropy_update_manual( ctx, buf, n );
+    polarssl_zeroize( buf, sizeof( buf ) );
+
+    if( ret != 0 )
+        return( ret );
 
     return( entropy_write_seed_file( ctx, path ) );
 }

From beb42837ac296459744780d5173be30d88f17316 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:36:30 +0100
Subject: [PATCH 05/36] Zeroize tmp bufs in hmac_drbg.c functions

---
 library/hmac_drbg.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c
index c7904d069..eece38997 100644
--- a/library/hmac_drbg.c
+++ b/library/hmac_drbg.c
@@ -342,11 +342,14 @@ int hmac_drbg_write_seed_file( hmac_drbg_context *ctx, const char *path )
 
 exit:
     fclose( f );
+    polarssl_zeroize( buf, sizeof( buf ) );
+
     return( ret );
 }
 
 int hmac_drbg_update_seed_file( hmac_drbg_context *ctx, const char *path )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     unsigned char buf[ POLARSSL_HMAC_DRBG_MAX_INPUT ];
@@ -365,14 +368,16 @@ int hmac_drbg_update_seed_file( hmac_drbg_context *ctx, const char *path )
     }
 
     if( fread( buf, 1, n, f ) != n )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_HMAC_DRBG_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_HMAC_DRBG_FILE_IO_ERROR;
+    else
+        hmac_drbg_update( ctx, buf, n );
 
     fclose( f );
 
-    hmac_drbg_update( ctx, buf, n );
+    polarssl_zeroize( buf, sizeof( buf ) );
+
+    if( ret != 0 )
+        return( ret );
 
     return( hmac_drbg_write_seed_file( ctx, path ) );
 }

From ff13995812ce1d62e4bd3404b6af821ef92bc05f Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:38:12 +0100
Subject: [PATCH 06/36] Zeroize return buf on failure in pkparse.c

---
 library/pkparse.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/library/pkparse.c b/library/pkparse.c
index 39c51f648..83c93baa4 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -98,7 +98,10 @@ int pk_load_file( const char *path, unsigned char **buf, size_t *n )
     if( fread( *buf, 1, *n, f ) != *n )
     {
         fclose( f );
+
+        polarssl_zeroize( *buf, *n );
         polarssl_free( *buf );
+
         return( POLARSSL_ERR_PK_FILE_IO_ERROR );
     }
 

From dd471788d8dcd17dfb5a4d7667658008f7e056f9 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:43:11 +0100
Subject: [PATCH 07/36] Zeroize tmp bufs in ctr_drbg.c functions

---
 library/ctr_drbg.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c
index 7b315e888..fe7fb27ae 100644
--- a/library/ctr_drbg.c
+++ b/library/ctr_drbg.c
@@ -402,12 +402,11 @@ int ctr_drbg_write_seed_file( ctr_drbg_context *ctx, const char *path )
         goto exit;
 
     if( fwrite( buf, 1, CTR_DRBG_MAX_INPUT, f ) != CTR_DRBG_MAX_INPUT )
-    {
         ret = POLARSSL_ERR_CTR_DRBG_FILE_IO_ERROR;
-        goto exit;
-    }
+    else
+        ret = 0;
 
-    ret = 0;
+    polarssl_zeroize( buf, sizeof( buf ) );
 
 exit:
     fclose( f );
@@ -416,6 +415,7 @@ exit:
 
 int ctr_drbg_update_seed_file( ctr_drbg_context *ctx, const char *path )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     unsigned char buf[ CTR_DRBG_MAX_INPUT ];
@@ -428,20 +428,18 @@ int ctr_drbg_update_seed_file( ctr_drbg_context *ctx, const char *path )
     fseek( f, 0, SEEK_SET );
 
     if( n > CTR_DRBG_MAX_INPUT )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG );
-    }
-
-    if( fread( buf, 1, n, f ) != n )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_CTR_DRBG_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG;
+    else if( fread( buf, 1, n, f ) != n )
+        ret = POLARSSL_ERR_CTR_DRBG_FILE_IO_ERROR;
+    else
+        ctr_drbg_update( ctx, buf, n );
 
     fclose( f );
 
-    ctr_drbg_update( ctx, buf, n );
+    polarssl_zeroize( buf, sizeof( buf ) );
+
+    if( ret != 0 )
+        return( ret );
 
     return( ctr_drbg_write_seed_file( ctx, path ) );
 }

From c381444c7f033cbc5968069ed28795ac6933c091 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:44:50 +0100
Subject: [PATCH 08/36] Zeroize tmp buf in mbedtls_mpi_fill_random()

---
 library/bignum.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/library/bignum.c b/library/bignum.c
index 4829d916b..e4a8dece5 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -1880,6 +1880,8 @@ int mpi_fill_random( mpi *X, size_t size,
     MPI_CHK( mpi_read_binary( X, buf, size ) );
 
 cleanup:
+    polarssl_zeroize( buf, sizeof( buf ) );
+
     return( ret );
 }
 

From a0ae1db2f7635e8a92250c08d9b00338b7c953ff Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:51:22 +0100
Subject: [PATCH 09/36] Zeroize buffers in various modules

---
 library/ssl_tls.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index bae8433fe..645fa32c7 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4140,12 +4140,19 @@ int ssl_set_psk( ssl_context *ssl, const unsigned char *psk, size_t psk_len,
         return( POLARSSL_ERR_SSL_BAD_INPUT_DATA );
     }
 
-    if( ssl->psk != NULL || ssl->psk_identity != NULL )
+    if( ssl->psk != NULL )
     {
+        polarssl_zeroize( ssl->psk, ssl->psk_len );
+
         polarssl_free( ssl->psk );
-        polarssl_free( ssl->psk_identity );
         ssl->psk = NULL;
+        ssl->psk_len = 0;
+    }
+    if( ssl->psk_identity != NULL )
+    {
+        polarssl_free( ssl->psk_identity );
         ssl->psk_identity = NULL;
+        ssl->psk_identity_len = 0;
     }
 
     if( ( ssl->psk = polarssl_malloc( psk_len ) ) == NULL ||

From f4660aaf4c24b5779bb6a5304ab1dfacb729f4ed Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:54:06 +0100
Subject: [PATCH 10/36] Zeroize heap buf on failure in pem.c

---
 library/pem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/library/pem.c b/library/pem.c
index b2c16c292..611c788fd 100644
--- a/library/pem.c
+++ b/library/pem.c
@@ -343,6 +343,7 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer,
     ( defined(POLARSSL_DES_C) || defined(POLARSSL_AES_C) )
         if( pwd == NULL )
         {
+            polarssl_zeroize( buf, len );
             polarssl_free( buf );
             return( POLARSSL_ERR_PEM_PASSWORD_REQUIRED );
         }
@@ -371,10 +372,12 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer,
          */
         if( len <= 2 || buf[0] != 0x30 || buf[1] > 0x83 )
         {
+            polarssl_zeroize( buf, len );
             polarssl_free( buf );
             return( POLARSSL_ERR_PEM_PASSWORD_MISMATCH );
         }
 #else
+        polarssl_zeroize( buf, len );
         polarssl_free( buf );
         return( POLARSSL_ERR_PEM_FEATURE_UNAVAILABLE );
 #endif /* POLARSSL_MD5_C && POLARSSL_CIPHER_MODE_CBC &&

From c0dc5b5d3b2d91d868a3ff7822e46192acc90ed8 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:56:39 +0100
Subject: [PATCH 11/36] Zeroize tmp buf in ctr_drbg_write_seed_file()

---
 library/ctr_drbg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c
index fe7fb27ae..f66064ff4 100644
--- a/library/ctr_drbg.c
+++ b/library/ctr_drbg.c
@@ -406,9 +406,9 @@ int ctr_drbg_write_seed_file( ctr_drbg_context *ctx, const char *path )
     else
         ret = 0;
 
+exit:
     polarssl_zeroize( buf, sizeof( buf ) );
 
-exit:
     fclose( f );
     return( ret );
 }
@@ -428,8 +428,12 @@ int ctr_drbg_update_seed_file( ctr_drbg_context *ctx, const char *path )
     fseek( f, 0, SEEK_SET );
 
     if( n > CTR_DRBG_MAX_INPUT )
-        ret = POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG;
-    else if( fread( buf, 1, n, f ) != n )
+    {
+        fclose( f );
+        return( POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG );
+    }
+
+    if( fread( buf, 1, n, f ) != n )
         ret = POLARSSL_ERR_CTR_DRBG_FILE_IO_ERROR;
     else
         ctr_drbg_update( ctx, buf, n );

From af134da17eac7d2b129288f271db63efb5d78f75 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 10:59:05 +0100
Subject: [PATCH 12/36] Add ChangeLog entry for buf zeroize

---
 ChangeLog | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1672bdf07..1029dc970 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.x released xxxx-xx-xx
+
+Security
+   * Ensure that buffers are cleared after use if they contain sensitive data.
+     Changes were introduced in multiple places in the library.
+
 = mbed TLS 1.3.20 released 2017-06-21
 
 Security

From 1bfa46a45617d7ddb1403900a0f8f53158aac66a Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 11:00:02 +0100
Subject: [PATCH 13/36] Zeroize tmp buffer in entropy_update()

---
 library/entropy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/library/entropy.c b/library/entropy.c
index caff22f42..5a8321b8d 100644
--- a/library/entropy.c
+++ b/library/entropy.c
@@ -166,6 +166,8 @@ static int entropy_update( entropy_context *ctx, unsigned char source_id,
     sha256_update( &ctx->accumulator, p, use_len );
 #endif
 
+    polarssl_zeroize( tmp, sizeof( tmp ) );
+
     return( 0 );
 }
 

From 2d829fb4b367427749727aeabb1bbfbd0f244567 Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <andres.amayagarcia@arm.com>
Date: Wed, 12 Jul 2017 11:01:32 +0100
Subject: [PATCH 14/36] Zeroize buf if mbedtls_base64_decode() fails

---
 library/pem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/library/pem.c b/library/pem.c
index 611c788fd..789a92d51 100644
--- a/library/pem.c
+++ b/library/pem.c
@@ -333,6 +333,7 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer,
 
     if( ( ret = base64_decode( buf, &len, s1, s2 - s1 ) ) != 0 )
     {
+        polarssl_zeroize( buf, len );
         polarssl_free( buf );
         return( POLARSSL_ERR_PEM_INVALID_DATA + ret );
     }

From 27ce0b5ff19f44d89efea491cf7d538ee7354abf Mon Sep 17 00:00:00 2001
From: Ron Eldor <Ron.Eldor@arm.com>
Date: Tue, 5 Sep 2017 15:34:35 +0300
Subject: [PATCH 15/36] Backport 1.3:Set PEM buffer to zero before freeing it

Set PEM buffer to zero before freeing it, to avoid private keys
being leaked to memory after releasing it.
---
 ChangeLog     | 6 ++++++
 library/pem.c | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index a3171d7eb..92f1baee8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS x.x.x branch released xxxx-xx-xx
+
+Security
+   * Set PEM buffer to zero before freeing it, to avoid decoded private keys
+     being leaked to memory after release.
+
 = mbed TLS 1.3.21 branch released 2017-08-10
 
 Security
diff --git a/library/pem.c b/library/pem.c
index b2c16c292..76905b358 100644
--- a/library/pem.c
+++ b/library/pem.c
@@ -389,6 +389,8 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer,
 
 void pem_free( pem_context *ctx )
 {
+    if ( ctx->buf != NULL )
+        polarssl_zeroize( ctx->buf, ctx->buflen );
     polarssl_free( ctx->buf );
     polarssl_free( ctx->info );
 

From 3d98b9744272652609931b451bcde71ec0d1392a Mon Sep 17 00:00:00 2001
From: Andres Amaya Garcia <Andres.AmayaGarcia@arm.com>
Date: Wed, 20 Sep 2017 11:47:49 +0100
Subject: [PATCH 16/36] Modify zeroize internal buffers in md modules

Modify all the following functions to zeroize an internal buffer before
exiting the function. The buffer could potentially contain confidential
data read from a file.

* md2_file()
* md4_file()
* md5_file()
* ripemd160_file()
* sha1_file()
* sha256_file()
* sha512_file()
---
 library/md2.c       | 16 ++++++++--------
 library/md4.c       | 16 ++++++++--------
 library/md5.c       | 16 ++++++++--------
 library/ripemd160.c | 16 ++++++++--------
 library/sha1.c      | 16 ++++++++--------
 library/sha256.c    | 16 ++++++++--------
 library/sha512.c    | 16 ++++++++--------
 7 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/library/md2.c b/library/md2.c
index 2ac7eba61..2d6123f12 100644
--- a/library/md2.c
+++ b/library/md2.c
@@ -217,6 +217,7 @@ void md2( const unsigned char *input, size_t ilen, unsigned char output[16] )
  */
 int md2_file( const char *path, unsigned char output[16] )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     md2_context ctx;
@@ -231,17 +232,16 @@ int md2_file( const char *path, unsigned char output[16] )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         md2_update( &ctx, buf, n );
 
-    md2_finish( &ctx, output );
-    md2_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_MD2_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_MD2_FILE_IO_ERROR;
+    else
+        md2_finish( &ctx, output );
 
+    md2_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/md4.c b/library/md4.c
index 8754d2f52..9c4a9b80a 100644
--- a/library/md4.c
+++ b/library/md4.c
@@ -313,6 +313,7 @@ void md4( const unsigned char *input, size_t ilen, unsigned char output[16] )
  */
 int md4_file( const char *path, unsigned char output[16] )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     md4_context ctx;
@@ -327,17 +328,16 @@ int md4_file( const char *path, unsigned char output[16] )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         md4_update( &ctx, buf, n );
 
-    md4_finish( &ctx, output );
-    md4_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_MD4_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_MD4_FILE_IO_ERROR;
+    else
+        md4_finish( &ctx, output );
 
+    md4_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/md5.c b/library/md5.c
index 8c7ed1fc3..4a0f25191 100644
--- a/library/md5.c
+++ b/library/md5.c
@@ -330,6 +330,7 @@ void md5( const unsigned char *input, size_t ilen, unsigned char output[16] )
  */
 int md5_file( const char *path, unsigned char output[16] )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     md5_context ctx;
@@ -344,17 +345,16 @@ int md5_file( const char *path, unsigned char output[16] )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         md5_update( &ctx, buf, n );
 
-    md5_finish( &ctx, output );
-    md5_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_MD5_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_MD5_FILE_IO_ERROR;
+    else
+        md5_finish( &ctx, output );
 
+    md5_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/ripemd160.c b/library/ripemd160.c
index 2c196f42b..7b5d02e2e 100644
--- a/library/ripemd160.c
+++ b/library/ripemd160.c
@@ -388,6 +388,7 @@ void ripemd160( const unsigned char *input, size_t ilen,
  */
 int ripemd160_file( const char *path, unsigned char output[20] )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     ripemd160_context ctx;
@@ -402,17 +403,16 @@ int ripemd160_file( const char *path, unsigned char output[20] )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         ripemd160_update( &ctx, buf, n );
 
-    ripemd160_finish( &ctx, output );
-    ripemd160_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_RIPEMD160_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_RIPEMD160_FILE_IO_ERROR;
+    else
+        ripemd160_finish( &ctx, output );
 
+    ripemd160_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/sha1.c b/library/sha1.c
index 44de8727e..a5a235b64 100644
--- a/library/sha1.c
+++ b/library/sha1.c
@@ -363,6 +363,7 @@ void sha1( const unsigned char *input, size_t ilen, unsigned char output[20] )
  */
 int sha1_file( const char *path, unsigned char output[20] )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     sha1_context ctx;
@@ -377,17 +378,16 @@ int sha1_file( const char *path, unsigned char output[20] )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         sha1_update( &ctx, buf, n );
 
-    sha1_finish( &ctx, output );
-    sha1_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_SHA1_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_SHA1_FILE_IO_ERROR;
+    else
+        sha1_finish( &ctx, output );
 
+    sha1_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/sha256.c b/library/sha256.c
index 674fdf25e..caae79f9b 100644
--- a/library/sha256.c
+++ b/library/sha256.c
@@ -366,6 +366,7 @@ void sha256( const unsigned char *input, size_t ilen,
  */
 int sha256_file( const char *path, unsigned char output[32], int is224 )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     sha256_context ctx;
@@ -380,17 +381,16 @@ int sha256_file( const char *path, unsigned char output[32], int is224 )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         sha256_update( &ctx, buf, n );
 
-    sha256_finish( &ctx, output );
-    sha256_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_SHA256_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_SHA256_FILE_IO_ERROR;
+    else
+        sha256_finish( &ctx, output );
 
+    sha256_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 
diff --git a/library/sha512.c b/library/sha512.c
index bd607e0ca..5e51f7f0b 100644
--- a/library/sha512.c
+++ b/library/sha512.c
@@ -370,6 +370,7 @@ void sha512( const unsigned char *input, size_t ilen,
  */
 int sha512_file( const char *path, unsigned char output[64], int is384 )
 {
+    int ret = 0;
     FILE *f;
     size_t n;
     sha512_context ctx;
@@ -384,17 +385,16 @@ int sha512_file( const char *path, unsigned char output[64], int is384 )
     while( ( n = fread( buf, 1, sizeof( buf ), f ) ) > 0 )
         sha512_update( &ctx, buf, n );
 
-    sha512_finish( &ctx, output );
-    sha512_free( &ctx );
-
     if( ferror( f ) != 0 )
-    {
-        fclose( f );
-        return( POLARSSL_ERR_SHA512_FILE_IO_ERROR );
-    }
+        ret = POLARSSL_ERR_SHA512_FILE_IO_ERROR;
+    else
+        sha512_finish( &ctx, output );
 
+    sha512_free( &ctx );
+    polarssl_zeroize( buf, sizeof( buf ) );
     fclose( f );
-    return( 0 );
+
+    return( ret );
 }
 #endif /* POLARSSL_FS_IO */
 

From 5f9df9b2ad95ba5220356ae24f34d4d6621a20b8 Mon Sep 17 00:00:00 2001
From: Janos Follath <janos.follath@arm.com>
Date: Wed, 20 Sep 2017 13:46:37 +0100
Subject: [PATCH 17/36] DHM: Add negative tests for parameter checking

A bug in the dhm_check_range() function makes it pass even when the
parameters are not in the range. This commit adds tests for signalling
this problem as well as a couple of other negative tests.
---
 tests/suites/test_suite_dhm.data     | 18 +++++++++++++++---
 tests/suites/test_suite_dhm.function |  7 +++++--
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/tests/suites/test_suite_dhm.data b/tests/suites/test_suite_dhm.data
index f2cdeffa5..5dbe4c8ae 100644
--- a/tests/suites/test_suite_dhm.data
+++ b/tests/suites/test_suite_dhm.data
@@ -1,11 +1,23 @@
 Diffie-Hellman full exchange #1
-dhm_do_dhm:10:"23":10:"5"
+dhm_do_dhm:10:"23":10:"5":0
 
 Diffie-Hellman full exchange #2
-dhm_do_dhm:10:"93450983094850938450983409623":10:"9345098304850938450983409622"
+dhm_do_dhm:10:"93450983094850938450983409623":10:"9345098304850938450983409622":0
 
 Diffie-Hellman full exchange #3
-dhm_do_dhm:10:"93450983094850938450983409623982317398171298719873918739182739712938719287391879381271":10:"9345098309485093845098340962223981329819812792137312973297123912791271"
+dhm_do_dhm:10:"93450983094850938450983409623982317398171298719873918739182739712938719287391879381271":10:"9345098309485093845098340962223981329819812792137312973297123912791271":0
+
+Diffie-Hellman trivial subgroup #1
+dhm_do_dhm:10:"23":10:"1":POLARSSL_ERR_DHM_BAD_INPUT_DATA
+
+Diffie-Hellman trivial subgroup #2
+dhm_do_dhm:10:"23":10:"-1":POLARSSL_ERR_DHM_BAD_INPUT_DATA
+
+Diffie-Hellman small modulus
+dhm_do_dhm:10:"3":10:"5":POLARSSL_ERR_DHM_MAKE_PARAMS_FAILED
+
+Diffie-Hellman zero modulus
+dhm_do_dhm:10:"0":10:"5":POLARSSL_ERR_DHM_BAD_INPUT_DATA
 
 Diffie-Hallman load parameters from file
 dhm_file:"data_files/dhparams.pem":"9e35f430443a09904f3a39a979797d070df53378e79c2438bef4e761f3c714553328589b041c809be1d6c6b5f1fc9f47d3a25443188253a992a56818b37ba9de5a40d362e56eff0be5417474c125c199272c8fe41dea733df6f662c92ae76556e755d10c64e6a50968f67fc6ea73d0dca8569be2ba204e23580d8bca2f4975b3":"02":128
diff --git a/tests/suites/test_suite_dhm.function b/tests/suites/test_suite_dhm.function
index d7cabf464..f21e6e6c1 100644
--- a/tests/suites/test_suite_dhm.function
+++ b/tests/suites/test_suite_dhm.function
@@ -9,7 +9,7 @@
 
 /* BEGIN_CASE */
 void dhm_do_dhm( int radix_P, char *input_P,
-                 int radix_G, char *input_G )
+                 int radix_G, char *input_G, int result )
 {
     dhm_context ctx_srv;
     dhm_context ctx_cli;
@@ -44,7 +44,10 @@ void dhm_do_dhm( int radix_P, char *input_P,
     /*
      * First key exchange
      */
-    TEST_ASSERT( dhm_make_params( &ctx_srv, x_size, ske, &ske_len, &rnd_pseudo_rand, &rnd_info ) == 0 );
+    TEST_ASSERT( dhm_make_params( &ctx_srv, x_size, ske, &ske_len, &rnd_pseudo_rand, &rnd_info ) == result );
+    if ( result != 0 )
+        goto exit;
+
     ske[ske_len++] = 0;
     ske[ske_len++] = 0;
     TEST_ASSERT( dhm_read_params( &ctx_cli, &p, ske + ske_len ) == 0 );

From 77359c93e4646bfa8f3c2dc0700574d6b6936f28 Mon Sep 17 00:00:00 2001
From: Janos Follath <janos.follath@arm.com>
Date: Wed, 20 Sep 2017 15:33:24 +0100
Subject: [PATCH 18/36] DHM: Fix dhm_check_range() always returning 0

Although the variable ret was initialised to an error, the
MBEDTLS_MPI_CHK macro was overwriting it. Therefore it ended up being
0 whenewer the bignum computation was successfull and stayed 0
independently of the actual check.
---
 ChangeLog     |  6 ++++++
 library/dhm.c | 11 +++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a3171d7eb..15e62149c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.x branch released xxxx-xx-xx
+
+Security
+   * Fix dhm_check_range() failing to detect trivial subgroups and potentially
+     leaking 1 bit of the private key. Reported by prashantkspatil.
+
 = mbed TLS 1.3.21 branch released 2017-08-10
 
 Security
diff --git a/library/dhm.c b/library/dhm.c
index 48fba2a73..6f1c51cc0 100644
--- a/library/dhm.c
+++ b/library/dhm.c
@@ -91,6 +91,9 @@ static int dhm_read_bignum( mpi *X,
  *
  * Parameter should be: 2 <= public_param <= P - 2
  *
+ * This means that we need to return an error if
+ *              public_param < 2 or public_param > P-2
+ *
  * For more information on the attack, see:
  *  http://www.cl.cam.ac.uk/~rja14/Papers/psandqs.pdf
  *  http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2005-2643
@@ -98,17 +101,17 @@ static int dhm_read_bignum( mpi *X,
 static int dhm_check_range( const mpi *param, const mpi *P )
 {
     mpi L, U;
-    int ret = POLARSSL_ERR_DHM_BAD_INPUT_DATA;
+    int ret = 0;
 
     mpi_init( &L ); mpi_init( &U );
 
     MPI_CHK( mpi_lset( &L, 2 ) );
     MPI_CHK( mpi_sub_int( &U, P, 2 ) );
 
-    if( mpi_cmp_mpi( param, &L ) >= 0 &&
-        mpi_cmp_mpi( param, &U ) <= 0 )
+    if( mpi_cmp_mpi( param, &L ) < 0 ||
+        mpi_cmp_mpi( param, &U ) > 0 )
     {
-        ret = 0;
+        ret = POLARSSL_ERR_DHM_BAD_INPUT_DATA;
     }
 
 cleanup:

From 55db24ca506cf5831bd31e32d26e5db2cc0264f9 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Tue, 17 Oct 2017 19:01:38 +0200
Subject: [PATCH 19/36] RSA: Fix buffer overflow in PSS signature verification

Fix buffer overflow in RSA-PSS signature verification when the hash is
too large for the key size. Found by Seth Terashima, Qualcomm.

Added a non-regression test and a positive test with the smallest
permitted key size for a SHA-512 hash.
---
 ChangeLog                              |  7 ++++++
 library/rsa.c                          |  2 ++
 tests/data_files/rsa512.key            |  9 ++++++++
 tests/data_files/rsa521.key            |  9 ++++++++
 tests/data_files/rsa522.key            |  9 ++++++++
 tests/data_files/rsa528.key            |  9 ++++++++
 tests/suites/test_suite_pkcs1_v21.data | 32 ++++++++++++++++++++++++++
 7 files changed, 77 insertions(+)
 create mode 100644 tests/data_files/rsa512.key
 create mode 100644 tests/data_files/rsa521.key
 create mode 100644 tests/data_files/rsa522.key
 create mode 100644 tests/data_files/rsa528.key

diff --git a/ChangeLog b/ChangeLog
index a3171d7eb..d2c5f6b36 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.x released xxxx-xx-xx
+
+Security
+   * Fix buffer overflow in RSA-PSS verification when the hash is too
+     large for the key size. Found by Seth Terashima, Qualcomm Product
+     Security Initiative, Qualcomm Technologies Inc.
+
 = mbed TLS 1.3.21 branch released 2017-08-10
 
 Security
diff --git a/library/rsa.c b/library/rsa.c
index ca8f688e6..d531c2607 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1368,6 +1368,8 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
 
     hlen = md_get_size( md_info );
+    if( siglen < hlen + 2 )
+        return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
     slen = siglen - hlen - 1; /* Currently length of salt + padding */
 
     memset( zeros, 0, 8 );
diff --git a/tests/data_files/rsa512.key b/tests/data_files/rsa512.key
new file mode 100644
index 000000000..1fd7987c2
--- /dev/null
+++ b/tests/data_files/rsa512.key
@@ -0,0 +1,9 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIBOwIBAAJBALB20jJQgW+aqwIwfkUrl/DK51mDabQWJOivx5caWaE4kvZLB+qm
+7JKMFgstbsj50N1bY8izrAdntPZciS9WwQ8CAwEAAQJAKYfNcIoB7II6PQmsrhrU
+Z5dZW3fSKNANX7X/A1DwR0DlF8uZnpWsWbYcRoXX7QjvepZqc54wryhW55Wlm6yI
+AQIhAOJIaLjSpbHjzzcJQ7mylxn2WGIlbJPPzJ9OaFZCZQvxAiEAx6OEAvl6JKa6
+6a+N2Wvhtcgb4qqR6UHQGJQYGJz5nP8CIAvgoR6ScAAWZRoOcm+c4DGMrLb6H+ji
+T2tNQkzEz2kBAiEAmw34GStU36STpa6RGJ4+tyZN6jWakDVqf7x+HpfFE1cCIQDc
+KzXIxec2taye4OeIa1v4W/MigMmYE9w93Uw/Qi3azA==
+-----END RSA PRIVATE KEY-----
diff --git a/tests/data_files/rsa521.key b/tests/data_files/rsa521.key
new file mode 100644
index 000000000..0b940aa6e
--- /dev/null
+++ b/tests/data_files/rsa521.key
@@ -0,0 +1,9 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIBPQIBAAJCATG2mGDzy5v4XqNY/fK9KZDxt3qA1qT9+BekPdiWvffdJq+KwCN/
+Um4NM7EFyXH9vU/6ns6Z/EafMez0Kej1YsHDAgMBAAECQCdoYjwdMSHp4kksL5Aa
+0kDc58ni0chy9IgXo+FHjTVmR9DkaZANrwfVvYMJxqYCZo0im1Dw7ZJBUDJQNXnl
+ZokCIRiSk66I24AWa7XGUFvatVwXWi2ACE4QEKqzWQe1mQ24/wIhDHD1TCKpqucA
+XDI+1N7EHs+fN4CfTSWe8FPGiK6q3VM9AiESrKKLi/q011U4KeS8SfR2blDcL2cg
+XFkuQWqxzzLoGOUCIQmgl5E0+Ypwe0zc7NYZFDarf4+ZjqxKQnXCvk0irMHcGQIh
+EVPli6RQb3Gcx7vXJHltzSTno7NElzBDRMBVUlBmVxAJ
+-----END RSA PRIVATE KEY-----
diff --git a/tests/data_files/rsa522.key b/tests/data_files/rsa522.key
new file mode 100644
index 000000000..18fbe70ca
--- /dev/null
+++ b/tests/data_files/rsa522.key
@@ -0,0 +1,9 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIBPgIBAAJCAtMCdT492ij0L02fkshkdCDqb7yXwQ+EmLlmqVPzV2mNZYEGDf4y
+yKuY20vFzirN8MHm5ASnWhMoJVDBqjfTzci/AgMBAAECQU05ffxf7uVg74yC9tKg
+qCa746NpMh3OM+HZrUxiOXv0sJMRXNEPD5HNLtgcNY6MI5NYbUvkOXktnFZpxWYP
+TH7BAiEeFJGs5Z6gRd2v/IbYLMFDHgjqho04INGTOvnyI7lGVKUCIRgJM7moFuoM
+UrKTmJK1uOzauWEykCKgc6BGH6TGZoEWkwIhBzQn2v82qO1ydOYGKRk2w2sa+Yd1
+pH5/kkHqf+m8QjKdAiEQ9eVW+4J30wxD0JyX4b1E/S5UpN5KYNhWX0US+6D3NBsC
+IRxePzdQlutZWg0Cnku3QE1tOLBCFlP7QVVl5FbKcY5H5w==
+-----END RSA PRIVATE KEY-----
diff --git a/tests/data_files/rsa528.key b/tests/data_files/rsa528.key
new file mode 100644
index 000000000..fd463b54d
--- /dev/null
+++ b/tests/data_files/rsa528.key
@@ -0,0 +1,9 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIBRQIBAAJDAOMcJG1GSFmEJh/RdMqz1DVzRGAuzXk8R9vlQlLTe7NQvGNDWbGV
+FVQggORySktnIpG+V8dkj1Finq7yNOhH2ZzGXwIDAQABAkMAsWYyLglQSlwnS4NZ
+L1z4zieTqW3lomWr2+BgxkHbxl2w0Rx4L+Ezp+YK6mhtIQWNkoytPvWJJMS7Jrkg
+agMAHQJBAiIA+F1y5GO0Bv+igsNLXwwtbCqs8hAkavU9W8egt/oDbhzbAiIA6hds
+PZp/s1X7n7dwfmebSs+3vLZFuQfifN8XZLw0CXHNAiEuEzgDQrPdMIN3er96zImI
+rYoUBgabiQ9u/WPFfa4xOU0CIgDDYC089Tfjy72pPgcr2PkpZVhqro5esg/8PI5f
+yxx7TXkCIgCYoE8Y5IxomtL1ub1AQzPe9UyyUGzQB1yWeiloJh6LjxA=
+-----END RSA PRIVATE KEY-----
diff --git a/tests/suites/test_suite_pkcs1_v21.data b/tests/suites/test_suite_pkcs1_v21.data
index a5b382f05..5a66bda09 100644
--- a/tests/suites/test_suite_pkcs1_v21.data
+++ b/tests/suites/test_suite_pkcs1_v21.data
@@ -787,3 +787,35 @@ RSASSA-PSS Signature verify options #13 (MGF1 alg != MSG hash alg, arg wrong)
 depends_on:POLARSSL_SHA256_C
 pkcs1_rsassa_pss_verify_ext:1024:16:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":16:"010001":POLARSSL_MD_NONE:POLARSSL_MD_SHA256:POLARSSL_MD_SHA1:RSA_SALT_LEN_ANY:"c0719e9a8d5d838d861dc6f675c899d2b309a3a65bb9fe6b11e5afcbf9a2c0b1":"7fc506d26ca3b22922a1ce39faaedd273161b82d9443c56f1a034f131ae4a18cae1474271cb4b66a17d9707ca58b0bdbd3c406b7e65bbcc9bbbce94dc45de807b4989b23b3e4db74ca29298137837eb90cc83d3219249bc7d480fceaf075203a86e54c4ecfa4e312e39f8f69d76534089a36ed9049ca9cfd5ab1db1fa75fe5c8":0:POLARSSL_ERR_RSA_INVALID_PADDING
 
+RSASSA-PSS verify ext, 512-bit key, empty salt, good signature
+depends_on:POLARSSL_SHA256_C
+pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369b41624e8afc7971a59a13892f64b07eaa6ec928c160b2d6ec8f9d0dd5b63c8b3ac0767b4f65c892f56c10f":16:"010001":POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:0:"":"ace8b03347da1b9a7a5e94a0d76359bb39c819bb170bef38ea84995ed653446c0ae87ede434cdf9d0cb2d7bf164cf427892363e6855a1d24d0ce5dd72acaf246":0:0
+
+RSASSA-PSS verify ext, 512-bit key, empty salt, bad signature
+depends_on:POLARSSL_SHA256_C
+pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369b41624e8afc7971a59a13892f64b07eaa6ec928c160b2d6ec8f9d0dd5b63c8b3ac0767b4f65c892f56c10f":16:"010001":POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:0:"":"ace8b03347da1b9a7a5e94a0d76359bb39c819bb170bef38ea84995ed653446c0ae87ede434cdf9d0cb2d7bf164cf427892363e6855a1d24d0ce5dd72acaf247":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING
+
+RSASSA-PSS verify ext, 522-bit key, SHA-512, empty salt, good signature
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:522:16:"02d302753e3dda28f42f4d9f92c8647420ea6fbc97c10f8498b966a953f357698d6581060dfe32c8ab98db4bc5ce2acdf0c1e6e404a75a13282550c1aa37d3cdc8bf":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"016752ae0b5dfbade6bbd3dd37868d48c8d741f92dca41c360aeda553204c2212a117b1a3d77e0d3f48723503c46e16c8a64de00f1dee3e37e478417452630859486":0:0
+
+RSASSA-PSS verify ext, 528-bit key, SHA-512, saltlen=64, good signature with saltlen=0
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:528:16:"00e31c246d46485984261fd174cab3d4357344602ecd793c47dbe54252d37bb350bc634359b19515542080e4724a4b672291be57c7648f51629eaef234e847d99cc65f":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:64:"":"a9ad7994ba3a1071124153486924448cc67a5af3a5d34e9261d53770782cc85f58e2edde5f7004652a645e3e9606530eb57de41df7298ae2be9dec69cc0d613ab629":0:POLARSSL_ERR_RSA_INVALID_PADDING
+
+RSASSA-PSS verify ext, 528-bit key, SHA-512, empty salt, good signature
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:528:16:"00e31c246d46485984261fd174cab3d4357344602ecd793c47dbe54252d37bb350bc634359b19515542080e4724a4b672291be57c7648f51629eaef234e847d99cc65f":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"a9ad7994ba3a1071124153486924448cc67a5af3a5d34e9261d53770782cc85f58e2edde5f7004652a645e3e9606530eb57de41df7298ae2be9dec69cc0d613ab629":0:0
+
+RSASSA-PSS verify ext, 528-bit key, SHA-512, saltlen=64, good signature with saltlen=0
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:528:16:"00e31c246d46485984261fd174cab3d4357344602ecd793c47dbe54252d37bb350bc634359b19515542080e4724a4b672291be57c7648f51629eaef234e847d99cc65f":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:64:"":"a9ad7994ba3a1071124153486924448cc67a5af3a5d34e9261d53770782cc85f58e2edde5f7004652a645e3e9606530eb57de41df7298ae2be9dec69cc0d613ab629":0:POLARSSL_ERR_RSA_INVALID_PADDING
+
+RSASSA-PSS verify ext, 512-bit key, SHA-512 (hash too large)
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369b41624e8afc7971a59a13892f64b07eaa6ec928c160b2d6ec8f9d0dd5b63c8b3ac0767b4f65c892f56c10f":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"ace8b03347da1b9a7a5e94a0d76359bb39c819bb170bef38ea84995ed653446c0ae87ede434cdf9d0cb2d7bf164cf427892363e6855a1d24d0ce5dd72acaf246":POLARSSL_ERR_RSA_BAD_INPUT_DATA:POLARSSL_ERR_RSA_BAD_INPUT_DATA
+
+RSASSA-PSS verify ext, 521-bit key, SHA-512, empty salt, bad signature
+depends_on:POLARSSL_SHA512_C
+pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"00471794655837da498cbf27242807b40593a353c707eb22fd2cc5a3259e728ac4f1df676043eeec8e16c1175b3d9ac8cae72ec1d5772dd69de71c5677f19031568e":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING
+

From 511bb84c609e1b39cc9e470f4f9132accb412591 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Tue, 17 Oct 2017 19:02:13 +0200
Subject: [PATCH 20/36] RSA: Fix another buffer overflow in PSS signature
 verification

Fix buffer overflow in RSA-PSS signature verification when the masking
operation results in an all-zero buffer. This could happen at any key size.
---
 ChangeLog                              |  2 ++
 library/rsa.c                          | 21 +++++++++++----------
 tests/suites/test_suite_pkcs1_v21.data |  4 ++++
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d2c5f6b36..9b3dc5c18 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -6,6 +6,8 @@ Security
    * Fix buffer overflow in RSA-PSS verification when the hash is too
      large for the key size. Found by Seth Terashima, Qualcomm Product
      Security Initiative, Qualcomm Technologies Inc.
+   * Fix buffer overflow in RSA-PSS verification when the unmasked
+     data is all zeros.
 
 = mbed TLS 1.3.21 branch released 2017-08-10
 
diff --git a/library/rsa.c b/library/rsa.c
index d531c2607..56fd7920f 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1325,10 +1325,11 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     size_t siglen;
     unsigned char *p;
     unsigned char buf[POLARSSL_MPI_MAX_SIZE];
+    unsigned char *hash_start;
     unsigned char result[POLARSSL_MD_MAX_SIZE];
     unsigned char zeros[8];
     unsigned int hlen;
-    size_t slen, msb;
+    size_t observed_salt_len, msb;
     const md_info_t *md_info;
     md_context_t md_ctx;
 
@@ -1370,7 +1371,7 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     hlen = md_get_size( md_info );
     if( siglen < hlen + 2 )
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
-    slen = siglen - hlen - 1; /* Currently length of salt + padding */
+    hash_start = buf + siglen - hlen - 1;
 
     memset( zeros, 0, 8 );
 
@@ -1385,6 +1386,7 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
         p++;
         siglen -= 1;
     }
+    else
     if( buf[0] >> ( 8 - siglen * 8 + msb ) )
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
 
@@ -1395,25 +1397,24 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
         return( ret );
     }
 
-    mgf_mask( p, siglen - hlen - 1, p + siglen - hlen - 1, hlen, &md_ctx );
+    mgf_mask( p, siglen - hlen - 1, hash_start, hlen, &md_ctx );
 
     buf[0] &= 0xFF >> ( siglen * 8 - msb );
 
-    while( p < buf + siglen && *p == 0 )
+    while( p < hash_start - 1 && *p == 0 )
         p++;
 
-    if( p == buf + siglen ||
+    if( p == hash_start ||
         *p++ != 0x01 )
     {
         md_free( &md_ctx );
         return( POLARSSL_ERR_RSA_INVALID_PADDING );
     }
 
-    /* Actual salt len */
-    slen -= p - buf;
+    observed_salt_len = hash_start - p;
 
     if( expected_salt_len != RSA_SALT_LEN_ANY &&
-        slen != (size_t) expected_salt_len )
+        observed_salt_len != (size_t) expected_salt_len )
     {
         md_free( &md_ctx );
         return( POLARSSL_ERR_RSA_INVALID_PADDING );
@@ -1424,12 +1425,12 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     md_starts( &md_ctx );
     md_update( &md_ctx, zeros, 8 );
     md_update( &md_ctx, hash, hashlen );
-    md_update( &md_ctx, p, slen );
+    md_update( &md_ctx, p, observed_salt_len );
     md_finish( &md_ctx, result );
 
     md_free( &md_ctx );
 
-    if( memcmp( p + slen, result, hlen ) == 0 )
+    if( memcmp( hash_start, result, hlen ) == 0 )
         return( 0 );
     else
         return( POLARSSL_ERR_RSA_VERIFY_FAILED );
diff --git a/tests/suites/test_suite_pkcs1_v21.data b/tests/suites/test_suite_pkcs1_v21.data
index 5a66bda09..b396c642a 100644
--- a/tests/suites/test_suite_pkcs1_v21.data
+++ b/tests/suites/test_suite_pkcs1_v21.data
@@ -819,3 +819,7 @@ RSASSA-PSS verify ext, 521-bit key, SHA-512, empty salt, bad signature
 depends_on:POLARSSL_SHA512_C
 pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"00471794655837da498cbf27242807b40593a353c707eb22fd2cc5a3259e728ac4f1df676043eeec8e16c1175b3d9ac8cae72ec1d5772dd69de71c5677f19031568e":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING
 
+RSASSA-PSS verify ext, all-zero padding, automatic salt length
+depends_on:POLARSSL_SHA256_C
+pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369b41624e8afc7971a59a13892f64b07eaa6ec928c160b2d6ec8f9d0dd5b63c8b3ac0767b4f65c892f56c10f":16:"010001":POLARSSL_MD_NONE:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:RSA_SALT_LEN_ANY:"":"63a35294577c7e593170378175b7df27c293dae583ec2a971426eb2d66f2af483e897bfae5dc20300a9d61a3644e08c3aee61a463690a3498901563c46041056":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING
+

From 7addb7f0a0c3f7b2294780022cf8c84990d9bca0 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Wed, 18 Oct 2017 19:03:42 +0200
Subject: [PATCH 21/36] RSA PSS: fix minimum length check for keys of size 8N+1

The check introduced by the previous security fix was off by one. It
fixed the buffer overflow but was not compliant with the definition of
PSS which technically led to accepting some invalid signatures (but
not signatures made without the private key).
---
 library/rsa.c                          | 7 ++++---
 tests/suites/test_suite_pkcs1_v21.data | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/library/rsa.c b/library/rsa.c
index 56fd7920f..6b720a4f9 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1369,9 +1369,6 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
 
     hlen = md_get_size( md_info );
-    if( siglen < hlen + 2 )
-        return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
-    hash_start = buf + siglen - hlen - 1;
 
     memset( zeros, 0, 8 );
 
@@ -1390,6 +1387,10 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     if( buf[0] >> ( 8 - siglen * 8 + msb ) )
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
 
+    if( siglen < hlen + 2 )
+        return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
+    hash_start = p + siglen - hlen - 1;
+
     md_init( &md_ctx );
     if( ( ret = md_init_ctx( &md_ctx, md_info ) ) != 0 )
     {
diff --git a/tests/suites/test_suite_pkcs1_v21.data b/tests/suites/test_suite_pkcs1_v21.data
index b396c642a..1c4113748 100644
--- a/tests/suites/test_suite_pkcs1_v21.data
+++ b/tests/suites/test_suite_pkcs1_v21.data
@@ -817,7 +817,7 @@ pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369
 
 RSASSA-PSS verify ext, 521-bit key, SHA-512, empty salt, bad signature
 depends_on:POLARSSL_SHA512_C
-pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"00471794655837da498cbf27242807b40593a353c707eb22fd2cc5a3259e728ac4f1df676043eeec8e16c1175b3d9ac8cae72ec1d5772dd69de71c5677f19031568e":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING
+pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"00471794655837da498cbf27242807b40593a353c707eb22fd2cc5a3259e728ac4f1df676043eeec8e16c1175b3d9ac8cae72ec1d5772dd69de71c5677f19031568e":POLARSSL_ERR_RSA_BAD_INPUT_DATA:POLARSSL_ERR_RSA_BAD_INPUT_DATA
 
 RSASSA-PSS verify ext, all-zero padding, automatic salt length
 depends_on:POLARSSL_SHA256_C

From 5d9224e11cb76712f4416da71715fec09aef12e7 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Thu, 19 Oct 2017 15:23:49 +0200
Subject: [PATCH 22/36] RSA PSS: fix first byte check for keys of size 8N+1

For a key of size 8N+1, check that the first byte after applying the
public key operation is 0 (it could have been 1 instead). The code was
incorrectly doing a no-op check instead, which led to invalid
signatures being accepted. Not a security flaw, since you would need the
private key to craft such an invalid signature, but a bug nonetheless.
---
 library/rsa.c                          | 6 +++---
 tests/suites/test_suite_pkcs1_v21.data | 8 ++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/library/rsa.c b/library/rsa.c
index 6b720a4f9..923294f0b 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1376,6 +1376,9 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     //
     msb = mpi_msb( &ctx->N ) - 1;
 
+    if( buf[0] >> ( 8 - siglen * 8 + msb ) )
+        return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
+
     // Compensate for boundary condition when applying mask
     //
     if( msb % 8 == 0 )
@@ -1383,9 +1386,6 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
         p++;
         siglen -= 1;
     }
-    else
-    if( buf[0] >> ( 8 - siglen * 8 + msb ) )
-        return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
 
     if( siglen < hlen + 2 )
         return( POLARSSL_ERR_RSA_BAD_INPUT_DATA );
diff --git a/tests/suites/test_suite_pkcs1_v21.data b/tests/suites/test_suite_pkcs1_v21.data
index 1c4113748..85fb6c0cb 100644
--- a/tests/suites/test_suite_pkcs1_v21.data
+++ b/tests/suites/test_suite_pkcs1_v21.data
@@ -819,6 +819,14 @@ RSASSA-PSS verify ext, 521-bit key, SHA-512, empty salt, bad signature
 depends_on:POLARSSL_SHA512_C
 pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:POLARSSL_MD_SHA512:0:"":"00471794655837da498cbf27242807b40593a353c707eb22fd2cc5a3259e728ac4f1df676043eeec8e16c1175b3d9ac8cae72ec1d5772dd69de71c5677f19031568e":POLARSSL_ERR_RSA_BAD_INPUT_DATA:POLARSSL_ERR_RSA_BAD_INPUT_DATA
 
+RSASSA-PSS verify ext, 521-bit key, SHA-256, empty salt, good signature
+depends_on:POLARSSL_SHA256_C
+pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:0:"41":"009c4941157fa36288e467310b198ab0c615c40963d611ffeef03000549ded809235955ecc57adba44782e9497c004f480ba2b3d58db8335fe0b391075c02c843a6d":0:0
+
+RSASSA-PSS verify ext, 521-bit key, SHA-256, empty salt, flipped-highest-bit signature
+depends_on:POLARSSL_SHA256_C
+pkcs1_rsassa_pss_verify_ext:521:16:"0131b69860f3cb9bf85ea358fdf2bd2990f1b77a80d6a4fdf817a43dd896bdf7dd26af8ac0237f526e0d33b105c971fdbd4ffa9ece99fc469f31ecf429e8f562c1c3":16:"010001":POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:0:"41":"00e11a2403df681c44a1f73f014b6c9ad17847d0b673f7c2a801cee208d10ab5792c10cd0cd495a4b331aaa521409fca7cb1b0d978b3a84cd67e28078b98753e9466":POLARSSL_ERR_RSA_BAD_INPUT_DATA:POLARSSL_ERR_RSA_BAD_INPUT_DATA
+
 RSASSA-PSS verify ext, all-zero padding, automatic salt length
 depends_on:POLARSSL_SHA256_C
 pkcs1_rsassa_pss_verify_ext:512:16:"00b076d23250816f9aab02307e452b97f0cae7598369b41624e8afc7971a59a13892f64b07eaa6ec928c160b2d6ec8f9d0dd5b63c8b3ac0767b4f65c892f56c10f":16:"010001":POLARSSL_MD_NONE:POLARSSL_MD_SHA256:POLARSSL_MD_SHA256:RSA_SALT_LEN_ANY:"":"63a35294577c7e593170378175b7df27c293dae583ec2a971426eb2d66f2af483e897bfae5dc20300a9d61a3644e08c3aee61a463690a3498901563c46041056":POLARSSL_ERR_RSA_INVALID_PADDING:POLARSSL_ERR_RSA_INVALID_PADDING

From 28474f41a0b7eb34e4c0550c588d0aca7126fdfb Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Thu, 19 Oct 2017 17:46:14 +0200
Subject: [PATCH 23/36] RSA PSS: remove redundant check; changelog

Remove a check introduced in the previous buffer overflow fix with keys of
size 8N+1 which the subsequent fix for buffer start calculations made
redundant.

Added a changelog entry for the buffer start calculation fix.
---
 ChangeLog     | 4 ++++
 library/rsa.c | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9b3dc5c18..3de318419 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,10 @@ Security
    * Fix buffer overflow in RSA-PSS verification when the unmasked
      data is all zeros.
 
+Bugfix
+   * Fix some invalid RSA-PSS signatures with keys of size 8N+1 that were
+     accepted. Generating these signatures required the private key.
+
 = mbed TLS 1.3.21 branch released 2017-08-10
 
 Security
diff --git a/library/rsa.c b/library/rsa.c
index 923294f0b..bbb028675 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -1405,8 +1405,7 @@ int rsa_rsassa_pss_verify_ext( rsa_context *ctx,
     while( p < hash_start - 1 && *p == 0 )
         p++;
 
-    if( p == hash_start ||
-        *p++ != 0x01 )
+    if( *p++ != 0x01 )
     {
         md_free( &md_ctx );
         return( POLARSSL_ERR_RSA_INVALID_PADDING );

From 0727ca41b7418378d6d063b8ade62b7437c98d0a Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Wed, 25 Oct 2017 16:07:09 +0100
Subject: [PATCH 24/36] Make mpi_read_binary time constant

This commit modifies mpi_read_binary to always allocate the minimum number of
limbs required to hold the entire buffer provided to the function, regardless of
its content. Previously, leading zero bytes in the input data were detected and
used to reduce memory footprint and time, but this non-constant behavior turned
out to be non-tolerable for the cryptographic applications this function is used
for.
---
 library/bignum.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/library/bignum.c b/library/bignum.c
index 4829d916b..f5130b406 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -678,16 +678,20 @@ cleanup:
 int mpi_read_binary( mpi *X, const unsigned char *buf, size_t buflen )
 {
     int ret;
-    size_t i, j, n;
+    size_t i, j;
+    size_t const limbs = CHARS_TO_LIMBS( buflen );
 
-    for( n = 0; n < buflen; n++ )
-        if( buf[n] != 0 )
-            break;
+    /* Ensure that target MPI has exactly the necessary number of limbs */
+    if( X->n != limbs )
+    {
+        mpi_free( X );
+        mpi_init( X );
+        MPI_CHK( mpi_grow( X, limbs ) );
+    }
 
-    MPI_CHK( mpi_grow( X, CHARS_TO_LIMBS( buflen - n ) ) );
     MPI_CHK( mpi_lset( X, 0 ) );
 
-    for( i = buflen, j = 0; i > n; i--, j++ )
+    for( i = buflen, j = 0; i > 0; i--, j++ )
         X->p[j / ciL] |= ((t_uint) buf[i - 1]) << ((j % ciL) << 3);
 
 cleanup:

From 754663f8c497c43f169638c3903e37b323b7c98f Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Wed, 25 Oct 2017 16:08:19 +0100
Subject: [PATCH 25/36] Fix information leak in ecp_gen_keypair_base

The function ecp_gen_keypair_base did not wipe the stack buffer used to
hold the private exponent before returning. This commit fixes this by not using
a stack buffer in the first place but instead calling mpi_fill_random directly
to acquire the necessary random MPI.
---
 library/ecp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/library/ecp.c b/library/ecp.c
index 79066dc91..f39e7ebe8 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -1854,7 +1854,6 @@ int ecp_gen_keypair( ecp_group *grp, mpi *d, ecp_point *Q,
     {
         /* SEC1 3.2.1: Generate d such that 1 <= n < N */
         int count = 0;
-        unsigned char rnd[POLARSSL_ECP_MAX_BYTES];
 
         /*
          * Match the procedure given in RFC 6979 (deterministic ECDSA):
@@ -1865,8 +1864,7 @@ int ecp_gen_keypair( ecp_group *grp, mpi *d, ecp_point *Q,
          */
         do
         {
-            MPI_CHK( f_rng( p_rng, rnd, n_size ) );
-            MPI_CHK( mpi_read_binary( d, rnd, n_size ) );
+            MPI_CHK( mpi_fill_random( d, n_size, f_rng, p_rng ) );
             MPI_CHK( mpi_shift_r( d, 8 * n_size - grp->nbits ) );
 
             /*

From c2102893af3257a9215176d8cb9c5ee969f39f91 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Wed, 25 Oct 2017 16:09:08 +0100
Subject: [PATCH 26/36] Zeroize stack before returning from mpi_fill_random

---
 library/bignum.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/library/bignum.c b/library/bignum.c
index f5130b406..0a9560734 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -1884,6 +1884,7 @@ int mpi_fill_random( mpi *X, size_t size,
     MPI_CHK( mpi_read_binary( X, buf, size ) );
 
 cleanup:
+    polarssl_zeroize( buf, sizeof( buf ) );
     return( ret );
 }
 

From 825c3db149abc89de130c73f7266f59b844fd926 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Wed, 25 Oct 2017 16:10:07 +0100
Subject: [PATCH 27/36] Adapt ChangeLog

---
 ChangeLog | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index a3171d7eb..360a72db8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.22 branch released xxxx-xx-xx
+
+Security
+   * Make mpi_read_binary constant-time with respect to
+     the input data. Previously, trailing zero bytes were detected
+     and omitted for the sake of saving memory, but potentially
+     leading to slight timing differences.
+     Reported by Marco Macchetti, Kudelski Group.
+   * Wipe stack buffer temporarily holding EC private exponent
+     after keypair generation.
+
 = mbed TLS 1.3.21 branch released 2017-08-10
 
 Security

From 251bab5ceb0b54303e899e1622dd02e1fb2a418c Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Mon, 20 Nov 2017 10:30:08 +0000
Subject: [PATCH 28/36] Fix heap corruption in ssl_decrypt_buf

Previously, MAC validation for an incoming record proceeded as follows:

1) Make a copy of the MAC contained in the record;
2) Compute the expected MAC in place, overwriting the presented one;
3) Compare both.

This resulted in a record buffer overflow if truncated MAC was used, as in this
case the record buffer only reserved 10 bytes for the MAC, but the MAC
computation routine in 2) always wrote a full digest.

For specially crafted records, this could be used to perform a controlled write of
up to 6 bytes past the boundary of the heap buffer holding the record, thereby
corrupting the heap structures and potentially leading to a crash or remote code
execution.

This commit fixes this by making the following change:
1) Compute the expected MAC in a temporary buffer that has the size of the
   underlying message digest.
2) Compare to this to the MAC contained in the record, potentially
   restricting to the first 10 bytes if truncated HMAC is used.

A similar fix is applied to the encryption routine `ssl_encrypt_buf`.
---
 library/ssl_tls.c | 32 ++++++++++++++++----------------
 tests/ssl-opt.sh  | 20 ++++++++++----------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 54867da97..63047aee2 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1141,12 +1141,16 @@ static int ssl_encrypt_buf( ssl_context *ssl )
         defined(POLARSSL_SSL_PROTO_TLS1_2)
         if( ssl->minor_ver >= SSL_MINOR_VERSION_1 )
         {
+            unsigned char mac[SSL_MAC_ADD];
+
             md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 13 );
             md_hmac_update( &ssl->transform_out->md_ctx_enc,
                              ssl->out_msg, ssl->out_msglen );
-            md_hmac_finish( &ssl->transform_out->md_ctx_enc,
-                             ssl->out_msg + ssl->out_msglen );
+            md_hmac_finish( &ssl->transform_out->md_ctx_enc, mac );
             md_hmac_reset( &ssl->transform_out->md_ctx_enc );
+
+            memcpy( ssl->out_msg + ssl->out_msglen, mac,
+                    ssl->transform_out->maclen );
         }
         else
 #endif
@@ -1419,8 +1423,6 @@ static int ssl_encrypt_buf( ssl_context *ssl )
     return( 0 );
 }
 
-#define POLARSSL_SSL_MAX_MAC_SIZE   48
-
 static int ssl_decrypt_buf( ssl_context *ssl )
 {
     size_t i;
@@ -1588,7 +1590,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
 #if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
         if( ssl->session_in->encrypt_then_mac == SSL_ETM_ENABLED )
         {
-            unsigned char computed_mac[POLARSSL_SSL_MAX_MAC_SIZE];
+            unsigned char mac_expect[SSL_MAC_ADD];
             unsigned char pseudo_hdr[13];
 
             SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
@@ -1606,15 +1608,15 @@ static int ssl_decrypt_buf( ssl_context *ssl )
             md_hmac_update( &ssl->transform_in->md_ctx_dec, pseudo_hdr, 13 );
             md_hmac_update( &ssl->transform_in->md_ctx_dec,
                              ssl->in_iv, ssl->in_msglen );
-            md_hmac_finish( &ssl->transform_in->md_ctx_dec, computed_mac );
+            md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
             md_hmac_reset( &ssl->transform_in->md_ctx_dec );
 
             SSL_DEBUG_BUF( 4, "message  mac", ssl->in_iv + ssl->in_msglen,
                                               ssl->transform_in->maclen );
-            SSL_DEBUG_BUF( 4, "computed mac", computed_mac,
+            SSL_DEBUG_BUF( 4, "computed mac", mac_expect,
                                               ssl->transform_in->maclen );
 
-            if( safer_memcmp( ssl->in_iv + ssl->in_msglen, computed_mac,
+            if( safer_memcmp( ssl->in_iv + ssl->in_msglen, mac_expect,
                               ssl->transform_in->maclen ) != 0 )
             {
                 SSL_DEBUG_MSG( 1, ( "message mac does not match" ) );
@@ -1775,15 +1777,13 @@ static int ssl_decrypt_buf( ssl_context *ssl )
 #if defined(POLARSSL_SOME_MODES_USE_MAC)
     if( auth_done == 0 )
     {
-        unsigned char tmp[POLARSSL_SSL_MAX_MAC_SIZE];
+        unsigned char mac_expect[SSL_MAC_ADD];
 
         ssl->in_msglen -= ssl->transform_in->maclen;
 
         ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 );
         ssl->in_hdr[4] = (unsigned char)( ssl->in_msglen      );
 
-        memcpy( tmp, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen );
-
 #if defined(POLARSSL_SSL_PROTO_SSL3)
         if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
         {
@@ -1820,8 +1820,8 @@ static int ssl_decrypt_buf( ssl_context *ssl )
             md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_ctr, 13 );
             md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg,
                              ssl->in_msglen );
-            md_hmac_finish( &ssl->transform_in->md_ctx_dec,
-                             ssl->in_msg + ssl->in_msglen );
+            md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect );
+
             /* Call md_process at least once due to cache attacks */
             for( j = 0; j < extra_run + 1; j++ )
                 md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg );
@@ -1836,11 +1836,11 @@ static int ssl_decrypt_buf( ssl_context *ssl )
             return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
         }
 
-        SSL_DEBUG_BUF( 4, "message  mac", tmp, ssl->transform_in->maclen );
-        SSL_DEBUG_BUF( 4, "computed mac", ssl->in_msg + ssl->in_msglen,
+        SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen );
+        SSL_DEBUG_BUF( 4, "message  mac", ssl->in_msg + ssl->in_msglen,
                        ssl->transform_in->maclen );
 
-        if( safer_memcmp( tmp, ssl->in_msg + ssl->in_msglen,
+        if( safer_memcmp( ssl->in_msg + ssl->in_msglen, mac_expect,
                          ssl->transform_in->maclen ) != 0 )
         {
 #if defined(POLARSSL_SSL_DEBUG_ALL)
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 0f976682b..53bf2acdd 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -504,40 +504,40 @@ run_test    "Truncated HMAC: client default, server default" \
             "$P_SRV debug_level=4" \
             "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \
             0 \
-            -s "dumping 'computed mac' (20 bytes)" \
-            -S "dumping 'computed mac' (10 bytes)"
+            -s "dumping 'expected mac' (20 bytes)" \
+            -S "dumping 'expected mac' (10 bytes)"
 
 run_test    "Truncated HMAC: client disabled, server default" \
             "$P_SRV debug_level=4" \
             "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
              trunc_hmac=0" \
             0 \
-            -s "dumping 'computed mac' (20 bytes)" \
-            -S "dumping 'computed mac' (10 bytes)"
+            -s "dumping 'expected mac' (20 bytes)" \
+            -S "dumping 'expected mac' (10 bytes)"
 
 run_test    "Truncated HMAC: client enabled, server default" \
             "$P_SRV debug_level=4" \
             "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
              trunc_hmac=1" \
             0 \
-            -S "dumping 'computed mac' (20 bytes)" \
-            -s "dumping 'computed mac' (10 bytes)"
+            -S "dumping 'expected mac' (20 bytes)" \
+            -s "dumping 'expected mac' (10 bytes)"
 
 run_test    "Truncated HMAC: client enabled, server disabled" \
             "$P_SRV debug_level=4 trunc_hmac=0" \
             "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
              trunc_hmac=1" \
             0 \
-            -s "dumping 'computed mac' (20 bytes)" \
-            -S "dumping 'computed mac' (10 bytes)"
+            -s "dumping 'expected mac' (20 bytes)" \
+            -S "dumping 'expected mac' (10 bytes)"
 
 run_test    "Truncated HMAC: client enabled, server enabled" \
             "$P_SRV debug_level=4 trunc_hmac=1" \
             "$P_CLI force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \
              trunc_hmac=1" \
             0 \
-            -S "dumping 'computed mac' (20 bytes)" \
-            -s "dumping 'computed mac' (10 bytes)"
+            -S "dumping 'expected mac' (20 bytes)" \
+            -s "dumping 'expected mac' (10 bytes)"
 
 # Tests for Encrypt-then-MAC extension
 

From 4d48bb6ca3513914733ac09e53e09b86c7eff590 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Mon, 20 Nov 2017 10:31:05 +0000
Subject: [PATCH 29/36] Adapt ChangeLog

---
 ChangeLog | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 6a1be9892..f77278b0d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,14 @@ mbed TLS ChangeLog (Sorted per branch, date)
 
 = mbed TLS 1.3.22 branch released 2017-xx-xx
 
+Security
+   * Fix heap corruption in implementation of truncated HMAC extension.
+     When the truncated HMAC extension is enabled and CBC is used,
+     sending a malicious application packet can be used to selectively
+     corrupt 6 bytes on the peer's heap, potentially leading to crash or
+     remote code execution. This can be triggered remotely from either
+     side.
+
 Bugfix
    * Fix memory leak in ssl_set_hostname() when called multiple times.
      Found by projectgus and jethrogb, #836.

From 0a139f9a03c80b9533f7a2e5566afdbf191a611e Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Tue, 21 Nov 2017 17:41:59 +0000
Subject: [PATCH 30/36] Modify debug output

Tests from ssl-opt.sh now expect 'expected mac XXX' and no longer 'computed mac
XXX'.
---
 library/ssl_tls.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 63047aee2..5877e58df 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1159,7 +1159,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
             return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
         }
 
-        SSL_DEBUG_BUF( 4, "computed mac",
+        SSL_DEBUG_BUF( 4, "expected mac",
                        ssl->out_msg + ssl->out_msglen,
                        ssl->transform_out->maclen );
 
@@ -1613,7 +1613,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
 
             SSL_DEBUG_BUF( 4, "message  mac", ssl->in_iv + ssl->in_msglen,
                                               ssl->transform_in->maclen );
-            SSL_DEBUG_BUF( 4, "computed mac", mac_expect,
+            SSL_DEBUG_BUF( 4, "expected mac", mac_expect,
                                               ssl->transform_in->maclen );
 
             if( safer_memcmp( ssl->in_iv + ssl->in_msglen, mac_expect,

From feae81de915ee4857f9a8558be107faa9234d7dd Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Thu, 23 Nov 2017 19:10:48 +0100
Subject: [PATCH 31/36] ChangeLog entry for ssl_parse_client_psk_identity fix

---
 ChangeLog | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1672bdf07..0af86c0c8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.21 released xxxx-xx-xx
+
+Security
+   * Fix unsafe bounds check in ssl_parse_client_psk_identity() when adding
+     64kB to the address of the SSL buffer wraps around.
+
 = mbed TLS 1.3.20 released 2017-06-21
 
 Security

From b662cc1f52893f9797060193add61cd6d0a8b8a7 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Fri, 24 Nov 2017 18:55:19 +0100
Subject: [PATCH 32/36] Avoid uninitialized variable warning in
 entropy_gather_internal

The variable ret was always initialized in entropy_gather_internal,
but `gcc -Werror=maybe-uninitialized` rightfully complained that it
was unable to determine this statically. Therefore, tweak the
problematic case (ctx->source_count == 0) to not use ret in that case.
---
 library/entropy.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/library/entropy.c b/library/entropy.c
index 5a8321b8d..518d98215 100644
--- a/library/entropy.c
+++ b/library/entropy.c
@@ -196,13 +196,11 @@ int entropy_update_manual( entropy_context *ctx,
  */
 static int entropy_gather_internal( entropy_context *ctx )
 {
-    int ret, i;
+    int ret = POLARSSL_ERR_ENTROPY_NO_SOURCES_DEFINED;
+    int i;
     unsigned char buf[ENTROPY_MAX_GATHER];
     size_t olen;
 
-    if( ctx->source_count == 0 )
-        return( POLARSSL_ERR_ENTROPY_NO_SOURCES_DEFINED );
-
     /*
      * Run through our entropy sources
      */

From 1caad0861010577d6a30eebffacd971a7f6f6fbd Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Tue, 28 Nov 2017 13:35:09 +0100
Subject: [PATCH 33/36] add changelog entry

---
 ChangeLog | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1672bdf07..d1b2e407e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 1.3.x released xxxx-xx-xx
+
+Security
+
+   * Tighten should-be-constant-time memcmp against compiler optimizations.
+
 = mbed TLS 1.3.20 released 2017-06-21
 
 Security

From ad951d131d2ca54b9e7f1dc6f3bd957444eaef28 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Wed, 29 Nov 2017 17:51:03 +0000
Subject: [PATCH 34/36] Correct dangerous typo in include/polarssl/ssl.h

The definition of SSL_MAC_ADD depends on the presence of the
configuration option POLARSSL_ARC4_C, which was misspelled as
POLARSSL_RC4_C in ssl.h, leading to a too small buffer and
subsequently to a buffer overflow during record processing.
This commit fixes the typo.
---
 ChangeLog              | 2 ++
 include/polarssl/ssl.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index f77278b0d..67777d4e5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,8 @@ Security
      side.
 
 Bugfix
+   * Fix typo in ssl.h leading to a too small value of SSL_MAC_ADD
+     in case CBC is disabled but ARC4 is enabled.
    * Fix memory leak in ssl_set_hostname() when called multiple times.
      Found by projectgus and jethrogb, #836.
    * Fix usage help in ssl_server2 example. Found and fixed by Bei Lin.
diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h
index 9a3fb8a4b..32c07c2db 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -303,7 +303,7 @@
 #define SSL_COMPRESSION_ADD             0
 #endif
 
-#if defined(POLARSSL_RC4_C) || defined(POLARSSL_CIPHER_MODE_CBC)
+#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_MODE_CBC)
 /* Ciphersuites using HMAC */
 #if defined(POLARSSL_SHA512_C)
 #define SSL_MAC_ADD                 48  /* SHA-384 used for HMAC */

From 3ea75b3a9b16855ab610af4df84acaed8c0ae9a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Mon, 18 Dec 2017 18:04:59 +0100
Subject: [PATCH 35/36] Fix SSLv3 MAC computation

In a previous PR (Fix heap corruption in implementation of truncated HMAC
extension #425) the place where MAC is computed was changed from the end of
the SSL I/O buffer to a local buffer (then (part of) the content of the local
buffer is either copied to the output buffer of compare to the input buffer).

Unfortunately, this change was made only for TLS 1.0 and later, leaving SSL
3.0 in an inconsistent state due to ssl_mac() still writing to the old,
hard-coded location, which, for MAC verification, resulted in later comparing
the end of the input buffer (containing the computed MAC) to the local buffer
(uninitialised), most likely resulting in MAC verification failure, hence no
interop (even with ourselves).

This commit completes the move to using a local buffer by using this strategy
for SSL 3.0 too. Fortunately ssl_mac() was static so it's not a problem to
change its signature.
---
 library/ssl_tls.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index ef8cd203e..1018270f9 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1050,9 +1050,11 @@ int ssl_psk_derive_premaster( ssl_context *ssl, key_exchange_type_t key_ex )
 /*
  * SSLv3.0 MAC functions
  */
-static void ssl_mac( md_context_t *md_ctx, unsigned char *secret,
-                     unsigned char *buf, size_t len,
-                     unsigned char *ctr, int type )
+static void ssl_mac( md_context_t *md_ctx,
+                     const unsigned char *secret,
+                     const unsigned char *buf, size_t len,
+                     const unsigned char *ctr, int type,
+                     unsigned char out[20] )
 {
     unsigned char header[11];
     unsigned char padding[48];
@@ -1077,14 +1079,14 @@ static void ssl_mac( md_context_t *md_ctx, unsigned char *secret,
     md_update( md_ctx, padding, padlen  );
     md_update( md_ctx, header,  11      );
     md_update( md_ctx, buf,     len     );
-    md_finish( md_ctx, buf +    len     );
+    md_finish( md_ctx, out              );
 
     memset( padding, 0x5C, padlen );
     md_starts( md_ctx );
     md_update( md_ctx, secret,    md_size );
     md_update( md_ctx, padding,   padlen  );
-    md_update( md_ctx, buf + len, md_size );
-    md_finish( md_ctx, buf + len          );
+    md_update( md_ctx, out,       md_size );
+    md_finish( md_ctx, out                );
 }
 #endif /* POLARSSL_SSL_PROTO_SSL3 */
 
@@ -1130,10 +1132,15 @@ static int ssl_encrypt_buf( ssl_context *ssl )
 #if defined(POLARSSL_SSL_PROTO_SSL3)
         if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
         {
+            unsigned char mac[20]; /* SHA-1 at most */
+
             ssl_mac( &ssl->transform_out->md_ctx_enc,
                       ssl->transform_out->mac_enc,
                       ssl->out_msg, ssl->out_msglen,
-                      ssl->out_ctr, ssl->out_msgtype );
+                      ssl->out_ctr, ssl->out_msgtype,
+                      mac );
+
+            memcpy( ssl->out_msg + ssl->out_msglen, mac, ssl->transform_out->maclen );
         }
         else
 #endif
@@ -1790,7 +1797,8 @@ static int ssl_decrypt_buf( ssl_context *ssl )
             ssl_mac( &ssl->transform_in->md_ctx_dec,
                       ssl->transform_in->mac_dec,
                       ssl->in_msg, ssl->in_msglen,
-                      ssl->in_ctr, ssl->in_msgtype );
+                      ssl->in_ctr, ssl->in_msgtype,
+                      mac_expect );
         }
         else
 #endif /* POLARSSL_SSL_PROTO_SSL3 */

From 921eb599f6934eee2f82bcba92b1a2b710746f46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Tue, 19 Dec 2017 10:03:46 +0100
Subject: [PATCH 36/36] Fix magic constant in previous commit

---
 library/ssl_tls.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 1018270f9..855872b4f 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1050,11 +1050,12 @@ int ssl_psk_derive_premaster( ssl_context *ssl, key_exchange_type_t key_ex )
 /*
  * SSLv3.0 MAC functions
  */
+#define SSL_MAC_MAX_BYTES   20  /* MD-5 or SHA-1 */
 static void ssl_mac( md_context_t *md_ctx,
                      const unsigned char *secret,
                      const unsigned char *buf, size_t len,
                      const unsigned char *ctr, int type,
-                     unsigned char out[20] )
+                     unsigned char out[SSL_MAC_MAX_BYTES] )
 {
     unsigned char header[11];
     unsigned char padding[48];
@@ -1132,7 +1133,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
 #if defined(POLARSSL_SSL_PROTO_SSL3)
         if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
         {
-            unsigned char mac[20]; /* SHA-1 at most */
+            unsigned char mac[SSL_MAC_MAX_BYTES];
 
             ssl_mac( &ssl->transform_out->md_ctx_enc,
                       ssl->transform_out->mac_enc,