Merge remote-tracking branch 'upstream-restricted/pr/360' into mbedtls-2.1-restricted-proposed

Conflicts:
* scripts/config.pl: reconciled parallel edits in a comment.

ChangeLog

@@ -17,6 +17,8 @@ Security
      implementation allowed an offline 2^80 brute force attack on the
      HMAC key of a single, uninterrupted connection (with no
      resumption of the session).
+   * Verify results of RSA private key operations to defend
+     against Bellcore glitch attack.
 
 Features
    * Extend PKCS#8 interface by introducing support for the entire SHA
@@ -51,6 +53,7 @@ Bugfix
      structure. Do not assume that zeroizing a context is a correct way to
      reset it. Found independently by ccli8 on Github.
    * In mbedtls_entropy_free(), properly free the message digest context.
+   * Fix memory leak in RSA self test.
 
 Changes
    * Clarified the documentation of mbedtls_ssl_setup.

include/mbedtls/rsa.h

@@ -220,7 +220,7 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx,
  * \brief          Do an RSA private key operation
  *
  * \param ctx      RSA context
- * \param f_rng    RNG function (Needed for blinding)
+ * \param f_rng    RNG function (used for blinding)
  * \param p_rng    RNG parameter
  * \param input    input buffer
  * \param output   output buffer
@@ -229,6 +229,18 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx,
  *
  * \note           The input and output buffers must be large
  *                 enough (eg. 128 bytes if RSA-1024 is used).
+ *
+ * \note           Blinding is used if and only if a PRNG is provided.
+ *
+ * \note           If blinding is used, both the base of exponentation
+ *                 and the exponent are blinded, providing protection
+ *                 against some side-channel attacks.
+ *
+ * \warning        It is deprecated and a security risk to not provide
+ *                 a PRNG here and thereby prevent the use of blinding.
+ *                 Future versions of the library may enforce the presence
+ *                 of a PRNG.
+ *
  */
 int mbedtls_rsa_private( mbedtls_rsa_context *ctx,
                  int (*f_rng)(void *, unsigned char *, size_t),

library/bignum.c

@@ -1613,7 +1613,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi
     mbedtls_mpi RR, T, W[ 2 << MBEDTLS_MPI_WINDOW_SIZE ], Apos;
     int neg;
 
-    if( mbedtls_mpi_cmp_int( N, 0 ) < 0 || ( N->p[0] & 1 ) == 0 )
+    if( mbedtls_mpi_cmp_int( N, 0 ) <= 0 || ( N->p[0] & 1 ) == 0 )
         return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA );
 
     if( mbedtls_mpi_cmp_int( E, 0 ) < 0 )

library/rsa.c

@@ -403,9 +403,41 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx,
     mbedtls_mpi *DQ = &ctx->DQ;
 #endif
 
+    /* Temporaries holding the initial input and the double
+     * checked result; should be the same in the end. */
+    mbedtls_mpi I, C;
+
     /* Make sure we have private key info, prevent possible misuse */
-    if( ctx->P.p == NULL || ctx->Q.p == NULL || ctx->D.p == NULL )
+#if defined(MBEDTLS_RSA_NO_CRT)
+    if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ||
+        ( f_rng != NULL && mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 ) ||
+        ( f_rng != NULL && mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 ) )
+    {
         return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
+    }
+#else /* ! MBEDTLS_RSA_NO_CRT */
+    if( mbedtls_mpi_cmp_int( &ctx->N, 0 )  == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->E, 0 )  == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->P, 0 )  == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->Q, 0 )  == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) == 0 ||
+        mbedtls_mpi_cmp_int( &ctx->QP, 0 ) == 0 )
+    {
+        return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
+    }
+#endif /* ! MBEDTLS_RSA_NO_CRT */
+
+
+#if defined(MBEDTLS_THREADING_C)
+    if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 )
+        return( ret );
+#endif
+
+    mbedtls_mpi_init( &I );
+    mbedtls_mpi_init( &C );
 
     mbedtls_mpi_init( &T ); mbedtls_mpi_init( &T1 ); mbedtls_mpi_init( &T2 );
     mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); mbedtls_mpi_init( &R );
@@ -421,12 +453,6 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx,
 #endif
     }
 
-
-#if defined(MBEDTLS_THREADING_C)
-    if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 )
-        return( ret );
-#endif
-
     MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &T, input, ctx->len ) );
     if( mbedtls_mpi_cmp_mpi( &T, &ctx->N ) >= 0 )
     {
@@ -434,6 +460,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx,
         goto cleanup;
     }
 
+    MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) );
+
     if( f_rng != NULL )
     {
         /*
@@ -522,6 +550,15 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx,
         MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) );
     }
 
+    /* Verify the result to prevent glitching attacks. */
+    MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E,
+                                          &ctx->N, &ctx->RN ) );
+    if( mbedtls_mpi_cmp_mpi( &C, &I ) != 0 )
+    {
+        ret = MBEDTLS_ERR_RSA_VERIFY_FAILED;
+        goto cleanup;
+    }
+
     olen = ctx->len;
     MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &T, output, olen ) );
 
@@ -544,6 +581,9 @@ cleanup:
 #endif
     }
 
+    mbedtls_mpi_free( &C );
+    mbedtls_mpi_free( &I );
+
     if( ret != 0 )
         return( MBEDTLS_ERR_RSA_PRIVATE_FAILED + ret );
 
@@ -705,7 +745,7 @@ int mbedtls_rsa_rsaes_pkcs1_v15_encrypt( mbedtls_rsa_context *ctx,
         return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
 
     olen = ctx->len;
-    
+
     // first comparison checks for overflow
     if( ilen + 11 < ilen || olen < ilen + 11 )
         return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
@@ -1167,11 +1207,6 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx,
     size_t nb_pad, olen, oid_size = 0;
     unsigned char *p = sig;
     const char *oid = NULL;
-    unsigned char *sig_try = NULL, *verif = NULL;
-    size_t i;
-    unsigned char diff;
-    volatile unsigned char diff_no_optimize;
-    int ret;
 
     if( mode == MBEDTLS_RSA_PRIVATE && ctx->padding != MBEDTLS_RSA_PKCS_V15 )
         return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
@@ -1237,42 +1272,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx,
     if( mode == MBEDTLS_RSA_PUBLIC )
         return( mbedtls_rsa_public(  ctx, sig, sig ) );
 
-    /*
+    return( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig ) );
-     * In order to prevent Lenstra's attack, make the signature in a
-     * temporary buffer and check it before returning it.
-     */
-    sig_try = mbedtls_calloc( 1, ctx->len );
-    if( sig_try == NULL )
-        return( MBEDTLS_ERR_MPI_ALLOC_FAILED );
-
-    verif   = mbedtls_calloc( 1, ctx->len );
-    if( verif == NULL )
-    {
-        mbedtls_free( sig_try );
-        return( MBEDTLS_ERR_MPI_ALLOC_FAILED );
-    }
-
-    MBEDTLS_MPI_CHK( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig_try ) );
-    MBEDTLS_MPI_CHK( mbedtls_rsa_public( ctx, sig_try, verif ) );
-
-    /* Compare in constant time just in case */
-    for( diff = 0, i = 0; i < ctx->len; i++ )
-        diff |= verif[i] ^ sig[i];
-    diff_no_optimize = diff;
-
-    if( diff_no_optimize != 0 )
-    {
-        ret = MBEDTLS_ERR_RSA_PRIVATE_FAILED;
-        goto cleanup;
-    }
-
-    memcpy( sig, sig_try, ctx->len );
-
-cleanup:
-    mbedtls_free( sig_try );
-    mbedtls_free( verif );
-
-    return( ret );
 }
 #endif /* MBEDTLS_PKCS1_V15 */
 
@@ -1792,7 +1792,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( verbose != 0 )
@@ -1806,7 +1807,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( verbose != 0 )
@@ -1819,7 +1821,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( memcmp( rsa_decrypted, rsa_plaintext, len ) != 0 )
@@ -1827,7 +1830,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( verbose != 0 )
@@ -1845,7 +1849,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( verbose != 0 )
@@ -1857,7 +1862,8 @@ int mbedtls_rsa_self_test( int verbose )
         if( verbose != 0 )
             mbedtls_printf( "failed\n" );
 
-        return( 1 );
+        ret = 1;
+        goto cleanup;
     }
 
     if( verbose != 0 )

scripts/config.pl

@@ -76,6 +76,11 @@ Options
 
 EOU
 
+# The following options are disabled instead of enabled with "full".
+# Notes:
+# - MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 and
+#   MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION could be enabled if the
+#   respective tests were adapted
 my @excluded = qw(
 MBEDTLS_DEPRECATED_REMOVED
 MBEDTLS_HAVE_SSE2
@@ -87,6 +92,7 @@ MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES
 MBEDTLS_NO_PLATFORM_ENTROPY
 MBEDTLS_REMOVE_ARC4_CIPHERSUITES
 MBEDTLS_SSL_HW_RECORD_ACCEL
+MBEDTLS_RSA_NO_CRT
 MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3
 MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
 MBEDTLS_ZLIB_SUPPORT

tests/scripts/all.sh

@@ -478,6 +478,23 @@ make test
 msg "test: !MBEDTLS_SSL_RENEGOTIATION - ssl-opt.sh (ASan build)" # ~ 6 min
 if_build_succeeded tests/ssl-opt.sh
 
+msg "build: Default + RSA_NO_CRT (ASan build)" # ~ 6 min
+cleanup
+cp "$CONFIG_H" "$CONFIG_BAK"
+scripts/config.pl set MBEDTLS_RSA_NO_CRT
+CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan .
+make
+
+msg "test: RSA_NO_CRT - main suites (inc. selftests) (ASan build)" # ~ 50s
+make test
+
+msg "test: RSA_NO_CRT - RSA-related part of ssl-opt.sh (ASan build)" # ~ 5s
+tests/ssl-opt.sh -f RSA
+
+msg "test: RSA_NO_CRT - RSA-related part of compat.sh (ASan build)" # ~ 3 min
+tests/compat.sh -t RSA
+
+
 msg "build: cmake, full config, clang" # ~ 50s
 cleanup
 cp "$CONFIG_H" "$CONFIG_BAK"

tests/suites/test_suite_pk.function

@@ -43,16 +43,19 @@ int mbedtls_rsa_decrypt_func( void *ctx, int mode, size_t *olen,
                        const unsigned char *input, unsigned char *output,
                        size_t output_max_len )
 {
-    return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, NULL, NULL, mode, olen,
+    return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx,
-                               input, output, output_max_len ) );
+                                       rnd_std_rand, NULL, mode, olen,
+                                       input, output, output_max_len ) );
 }
 int mbedtls_rsa_sign_func( void *ctx,
                    int (*f_rng)(void *, unsigned char *, size_t), void *p_rng,
                    int mode, mbedtls_md_type_t md_alg, unsigned int hashlen,
                    const unsigned char *hash, unsigned char *sig )
 {
-    return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, f_rng, p_rng, mode,
+    ((void) f_rng);
-                            md_alg, hashlen, hash, sig ) );
+    ((void) p_rng);
+    return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, rnd_std_rand, NULL, mode,
+                                    md_alg, hashlen, hash, sig ) );
 }
 size_t mbedtls_rsa_key_len_func( void *ctx )
 {