Merge pull request #3403 from piotr-now/sca_memmove

Add mbedtls_platform_memmove() as a secured memcmp()

include/mbedtls/error.h

@@ -86,7 +86,7 @@
  * CHACHA20  3                  0x0051-0x0055
  * POLY1305  3                  0x0057-0x005B
  * CHACHAPOLY 2 0x0054-0x0056
- * PLATFORM  3  0x0070-0x0072   0x0071-0x0071
+ * PLATFORM  4  0x0070-0x0072   0x0071-0x0071 0x0076-0x0076
  *
  * High-level module nr (3 bits - 0x0...-0x7...)
  * Name      ID  Nr of Errors

include/mbedtls/nist_kw.h

@@ -133,6 +133,7 @@ void mbedtls_nist_kw_free( mbedtls_nist_kw_context *ctx );
  *
  * \return          \c 0 on success.
  * \return          \c MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA for invalid input length.
+ * \return          \c MBEDTLS_ERR_PLATFORM_ALLOC_FAILED in case of a memory allocation failure.
  * \return          cipher-specific error code on failure of the underlying cipher.
  */
 int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx, mbedtls_nist_kw_mode_t mode,

include/mbedtls/platform.h

@@ -42,6 +42,7 @@
 #define MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED     -0x0070 /**< Hardware accelerator failed */
 #define MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED -0x0072 /**< The requested feature is not supported by the platform */
 #define MBEDTLS_ERR_PLATFORM_FAULT_DETECTED      -0x0071 /**< A hardware fault was detected in a critical path. As a security precaution this should be treated as a potential physical attack */
+#define MBEDTLS_ERR_PLATFORM_ALLOC_FAILED        -0x0076 /**< Memory allocation failed */
 
 #if defined(MBEDTLS_PLATFORM_C)
 

include/mbedtls/platform_util.h

@@ -198,6 +198,22 @@ void *mbedtls_platform_memset( void *ptr, int value, size_t num );
  */
 void *mbedtls_platform_memcpy( void *dst, const void *src, size_t num );
 
+/**
+ * \brief       Secure memmove
+ *
+ *              This is a constant-time version of memmove(). It is based on
+ *              the double use of the mbedtls_platform_memcpy() function secured
+ *              against side-channel attacks.
+ *
+ * \param dst   Destination buffer where the data is being moved to.
+ * \param src   Source buffer where the data is being moved from.
+ * \param num   The length of the buffers in bytes.
+ *
+ * \return      0 if the operation was successful
+ * \return      #MBEDTLS_ERR_PLATFORM_ALLOC_FAILED if a memory allocation failed
+ */
+int mbedtls_platform_memmove( void *dst, const void *src, size_t num );
+
 /**
  * \brief       Secure memcmp
  *

library/bignum.c

@@ -558,7 +558,7 @@ static int mpi_write_hlp( mbedtls_mpi *X, int radix,
         length++;
     } while( mbedtls_mpi_cmp_int( X, 0 ) != 0 );
 
-    memmove( *p, p_end, length );
+    MBEDTLS_MPI_CHK( mbedtls_platform_memmove( *p, p_end, length ) );
     *p += length;
 
 cleanup:

library/error.c

@@ -843,6 +843,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen )
         mbedtls_snprintf( buf, buflen, "PLATFORM - The requested feature is not supported by the platform" );
     if( use_ret == -(MBEDTLS_ERR_PLATFORM_FAULT_DETECTED) )
         mbedtls_snprintf( buf, buflen, "PLATFORM - A hardware fault was detected in a critical path. As a security precaution this should be treated as a potential physical attack" );
+    if( use_ret == -(MBEDTLS_ERR_PLATFORM_ALLOC_FAILED) )
+        mbedtls_snprintf( buf, buflen, "PLATFORM - Memory allocation failed" );
 #endif /* MBEDTLS_PLATFORM_C */
 
 #if defined(MBEDTLS_POLY1305_C)

library/nist_kw.c

@@ -222,7 +222,11 @@ int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx,
         }
 
         mbedtls_platform_memcpy( output, NIST_KW_ICV1, KW_SEMIBLOCK_LENGTH );
-        memmove( output + KW_SEMIBLOCK_LENGTH, input, in_len );
+        ret = mbedtls_platform_memmove( output + KW_SEMIBLOCK_LENGTH, input, in_len );
+        if( ret != 0 )
+        {
+            return ret;
+        }
     }
     else
     {
@@ -343,7 +347,12 @@ static int unwrap( mbedtls_nist_kw_context *ctx,
     }
 
     mbedtls_platform_memcpy( A, input, KW_SEMIBLOCK_LENGTH );
-    memmove( output, input + KW_SEMIBLOCK_LENGTH, ( semiblocks - 1 ) * KW_SEMIBLOCK_LENGTH );
+    ret = mbedtls_platform_memmove( output, input + KW_SEMIBLOCK_LENGTH,
+                                       ( semiblocks - 1 ) * KW_SEMIBLOCK_LENGTH );
+    if( ret != 0 )
+    {
+        return ret;
+    }
 
     /* Calculate intermediate values */
     for( t = s; t >= 1; t-- )

library/pk.c

@@ -622,13 +622,18 @@ static int asn1_write_mpibuf( unsigned char **p, unsigned char *start,
                               size_t n_len )
 {
     size_t len = 0;
+    int ret = MBEDTLS_ERR_PLATFORM_FAULT_DETECTED;
 
     if( (size_t)( *p - start ) < n_len )
         return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL );
 
     len = n_len;
     *p -= len;
-    memmove( *p, start, len );
+    ret = mbedtls_platform_memmove( *p, start, len );
+    if( ret != 0 )
+    {
+        return( ret );
+    }
 
     /* ASN.1 DER encoding requires minimal length, so skip leading 0s.
      * Neither r nor s should be 0, but as a failsafe measure, still detect
@@ -690,8 +695,11 @@ static int pk_ecdsa_sig_asn1_from_uecc( unsigned char *sig, size_t *sig_len,
     *--p = MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE;
     len += 2;
 
-    ret = 0;
+    ret = mbedtls_platform_memmove( sig, p, len );
-    memmove( sig, p, len );
+    if( ret != 0 )
+    {
+        return( ret );
+    }
     *sig_len = len;
 
     return( ret );

library/platform_util.c

@@ -38,6 +38,12 @@
 #include "mbedtls/platform.h"
 #include "mbedtls/threading.h"
 
+#if !defined(MBEDTLS_PLATFORM_C)
+#include <stdlib.h>
+#define mbedtls_calloc    calloc
+#define mbedtls_free       free
+#endif
+
 #if defined(MBEDTLS_ENTROPY_HARDWARE_ALT)
 #include "mbedtls/entropy_poll.h"
 #endif
@@ -121,6 +127,23 @@ void *mbedtls_platform_memcpy( void *dst, const void *src, size_t num )
     return( memcpy( (void *) dst, (void *) src, start_offset ) );
 }
 
+int mbedtls_platform_memmove( void *dst, const void *src, size_t num )
+{
+    /* The buffers can have a common part, so we cannot do a copy from a random
+     * location. By using a temporary buffer we can do so, but the cost of it
+     * is using more memory and longer transfer time. */
+    void *tmp = mbedtls_calloc( 1, num );
+    if( tmp != NULL )
+    {
+        mbedtls_platform_memcpy( tmp, src, num );
+        mbedtls_platform_memcpy( dst, tmp, num );
+        mbedtls_free( tmp );
+        return 0;
+    }
+
+    return MBEDTLS_ERR_PLATFORM_ALLOC_FAILED;
+}
+
 int mbedtls_platform_memcmp( const void *buf1, const void *buf2, size_t num )
 {
     volatile const unsigned char *A = (volatile const unsigned char *) buf1;