From 88ec2381d6af23935c491272375cea85d942d894 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 13:51:16 +0100 Subject: [PATCH 01/24] Add configuration options for verification and blinding This commit defines some configuration options to control the mandatory use of blinding and verification in RSA private key operations. --- include/mbedtls/config.h | 72 +++++++++++++++++++++++++++++++++++++++- include/mbedtls/rsa.h | 35 ++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index c4b8995c1..1ce92c5a1 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -970,16 +970,86 @@ */ #define MBEDTLS_PKCS1_V21 +/** + * \def MBEDTLS_RSA_FORCE_BLINDING + * + * Force the use of blinding in RSA private key operations. + * This makes these operations fail when the caller doesn't + * provide a PRNG. + * + * Comment this macro to allow RSA private key operations + * without blinding. + * + * \warning Disabling this can be a security risk! + * Blinding RSA private key operations is a way + * to prevent statistical timing attacks as in + * [P. Kocher ', Timing Attacks on Implementations + * of Diffie-Hellman, RSA, DSS, and Other Systems] + * + * \note Disabling this does not mean that blinding + * will never be used, but instead makes private + * key operations fail if, perhaps unintentionally, + * the user failed to call them with a PRNG. + * + * \note For more on the use of blinding in RSA + * private key operations, see the documentation + * of \c mbedtls_rsa_private. + */ +#define MBEDTLS_RSA_FORCE_BLINDING + /** * \def MBEDTLS_RSA_NO_CRT * - * Do not use the Chinese Remainder Theorem for the RSA private operation. + * Do not use the Chinese Remainder Theorem + * for the RSA private operation. * * Uncomment this macro to disable the use of CRT in RSA. * */ //#define MBEDTLS_RSA_NO_CRT +/** + * \def MBEDTLS_RSA_FORCE_CRT_VERIFICATION + * + * Force verification of results of RSA private key operations + * when RSA-CRT is used. + * + * Comment this macro to disable RSA-CRT verification. + * + * \warning Disabling this can be a security risk! + * Omitting verification makes the RSA-CRT + * signing vulnerable to the Bellcore + * glitch attack leading to private key + * compromise if an attacker can cause a + * glitch in a certain timeframe during + * the signing operation. Uncomment only + * if you're sure that glitches are out of + * your attack model. + */ +#define MBEDTLS_RSA_FORCE_CRT_VERIFICATION + +/** + * \def MBEDTLS_RSA_FORCE_VERIFICATION + * + * Force verification of results of any RSA private key + * operation regardless of the algorithm used. + * + * Uncomment this to enable unconditional RSA verification. + * + * \note This is to prevent the RSA signing operation + * (regardless of the particular algorithm chosen) + * from potential future glitch attacks. We are + * currently not aware of any such for our default + * implementation, therefore disabling the option + * by default. + * + * \note Enabling it comes at the cost of roughly an + * additional public key operation at the end of + * signing (low compared to private key operations), + * as well as minor memory consumption. + */ +//#define MBEDTLS_RSA_FORCE_VERIFICATION + /** * \def MBEDTLS_SELF_TEST * diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index 54653dfdc..e34fea0f2 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -63,6 +63,15 @@ #define MBEDTLS_RSA_SALT_LEN_ANY -1 +/* + * RSA configuration + */ +#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) || \ + ( ! defined(MBEDTLS_RSA_NO_CRT) && \ + defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION ) ) +#define MBEDTLS_RSA_REQUIRE_VERIFICATION +#endif + /* * The above constants may be used even if the RSA module is compile out, * eg for alternative (PKCS#11) RSA implemenations in the PK layers. @@ -220,7 +229,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 +238,30 @@ 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 Enabling and disabling of blinding: + * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING + * is disabled, blinding is disabled. + * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING + * is enabled, the function fails. + * + * \note If blinding is used, both the base of exponentation + * and the exponent are blinded, preventing both statistical + * timing and power analysis attacks. + * + * \note Depending on the way RSA is implemented, a failure + * in the computation can lead to disclosure of the private + * key if the wrong result is passed to attacker - e.g., + * implementing RSA through CRT is vulnerable to the + * Bellcore glitch attack. + * + * As a remedy, the user can force double checking the + * result of the private key operation through the option + * MBEDTLS_RSA_FORCE_VERIFICATION. If verification is + * to be enabled only when RSA-CRT is used (as controlled + * by the configuration option MBEDTLS_RSA_NO_CRT), the + * option MBEDTLS_RSA_FORCE_CRT_VERIFICATION can be used. + * */ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, int (*f_rng)(void *, unsigned char *, size_t), From 5bc8729b9e7738d8f9a32e96b8e1fb2f597e3609 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 15:09:31 +0100 Subject: [PATCH 02/24] Correct memory leak in RSA self test The RSA self test didn't free the RSA context on failure. --- library/rsa.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 122bc1360..c8090044a 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1772,7 +1772,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1786,7 +1787,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1799,7 +1801,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 ) @@ -1807,7 +1810,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1825,7 +1829,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1837,7 +1842,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) From a540068a56efcadb6cf05b7a197021aa7c4788b5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 16:43:15 +0100 Subject: [PATCH 03/24] Modify PK test suite to provide PRNG to RSA signature function To prepare for the option of mandatory blinding, this commit changes the PK test suite to always call signature functions with a PRNG. --- tests/suites/test_suite_pk.function | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 5fa8a693a..33453ac6f 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -43,7 +43,7 @@ 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, rnd_std_rand, NULL, mode, olen, input, output, output_max_len ) ); } int mbedtls_rsa_sign_func( void *ctx, @@ -51,7 +51,9 @@ int mbedtls_rsa_sign_func( void *ctx, 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); + ((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 ) From 06811ced27d809610cfde1db85dd138452f40436 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 15:10:34 +0100 Subject: [PATCH 04/24] Put configuration options for RSA blinding and verification to work. --- library/rsa.c | 132 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 24 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index c8090044a..d3feeba88 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -398,24 +398,68 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, { int ret; size_t olen; - mbedtls_mpi T, T1, T2; + + /* Temporary holding the result */ + mbedtls_mpi T; + + /* Temporaries holding P-1, Q-1 and the + * exponent blinding factor, respectively. */ mbedtls_mpi P1, Q1, R; -#if defined(MBEDTLS_RSA_NO_CRT) - mbedtls_mpi D_blind; - mbedtls_mpi *D = &ctx->D; -#else + +#if !defined(MBEDTLS_RSA_NO_CRT) + /* Temporaries holding the results mod p resp. mod q. */ + mbedtls_mpi TP, TQ; + + /* Temporaries holding the blinded exponents for + * the mod p resp. mod q computation (if used). */ mbedtls_mpi DP_blind, DQ_blind; + + /* Pointers to actual exponents to be used - either the unblinded + * or the blinded ones, depending on the presence of a PRNG. */ mbedtls_mpi *DP = &ctx->DP; mbedtls_mpi *DQ = &ctx->DQ; +#else + /* Temporary holding the blinded exponent (if used). */ + mbedtls_mpi D_blind; + + /* Pointer to actual exponent to be used - either the unblinded + * or the blinded one, depending on the presence of a PRNG. */ + mbedtls_mpi *D = &ctx->D; +#endif + +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + /* Temporaries holding input mod p resp. mod q. */ + mbedtls_mpi IP, IQ; + + /* Temporaries holding double check results mod p resp. mod q; + * should in the end have the same values as IP and IQ. */ + mbedtls_mpi CP, CQ; + + /* Comparison results */ + int check = 0; +#endif + +#if defined(MBEDTLS_RSA_FORCE_BLINDING) + if( f_rng == NULL ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); #endif /* Make sure we have private key info, prevent possible misuse */ if( ctx->P.p == NULL || ctx->Q.p == NULL || ctx->D.p == NULL ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); - mbedtls_mpi_init( &T ); mbedtls_mpi_init( &T1 ); mbedtls_mpi_init( &T2 ); - mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); mbedtls_mpi_init( &R ); +#if defined(MBEDTLS_THREADING_C) + if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) + return( ret ); +#endif + /* MPI Initialization */ + + mbedtls_mpi_init( &T ); + + mbedtls_mpi_init( &P1 ); + mbedtls_mpi_init( &Q1 ); + mbedtls_mpi_init( &R ); if( f_rng != NULL ) { @@ -427,12 +471,17 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif } - -#if defined(MBEDTLS_THREADING_C) - if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) - return( ret ); +#if !defined(MBEDTLS_RSA_NO_CRT) + mbedtls_mpi_init( &TP ); mbedtls_mpi_init( &TQ ); #endif +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + mbedtls_mpi_init( &IP ); mbedtls_mpi_init( &IQ ); + mbedtls_mpi_init( &CP ); mbedtls_mpi_init( &CQ ); +#endif + + /* End of MPI initialization */ + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &T, input, ctx->len ) ); if( mbedtls_mpi_cmp_mpi( &T, &ctx->N ) >= 0 ) { @@ -440,6 +489,11 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, goto cleanup; } +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IP, &T, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IQ, &T, &ctx->Q ) ); +#endif + if( f_rng != NULL ) { /* @@ -498,24 +552,25 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* * Faster decryption using the CRT * - * T1 = input ^ dP mod P - * T2 = input ^ dQ mod Q + * TP = input ^ dP mod P + * TQ = input ^ dQ mod Q */ - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &T1, &T, DP, &ctx->P, &ctx->RP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &T2, &T, DQ, &ctx->Q, &ctx->RQ ) ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &TP, &T, DP, &ctx->P, &ctx->RP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &TQ, &T, DQ, &ctx->Q, &ctx->RQ ) ); /* - * T = (T1 - T2) * (Q^-1 mod P) mod P + * T = (TP - TQ) * (Q^-1 mod P) mod P */ - MBEDTLS_MPI_CHK( mbedtls_mpi_sub_mpi( &T, &T1, &T2 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T1, &T, &ctx->QP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T1, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_mpi( &T, &TP, &TQ ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &TP, &T, &ctx->QP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &TP, &ctx->P ) ); /* - * T = T2 + T * Q + * T = TQ + T * Q */ - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T1, &T, &ctx->Q ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &T, &T2, &T1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &TP, &T, &ctx->Q ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &T, &TQ, &TP ) ); #endif /* MBEDTLS_RSA_NO_CRT */ if( f_rng != NULL ) @@ -528,6 +583,23 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } + /* If requested by the config, verify the result to prevent glitching attacks. + * For that, check the two prime moduli separately. */ +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CP, &T, &ctx->E, &ctx->P, &ctx->RP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CQ, &T, &ctx->E, &ctx->Q, &ctx->RQ ) ); + + check |= mbedtls_mpi_cmp_mpi( &CP, &IP ); + check |= mbedtls_mpi_cmp_mpi( &CQ, &IQ ); + + if( check != 0 ) + { + /* Verification failed */ + ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; + goto cleanup; + } +#endif /* MBEDTLS_RSA_REQUIRE_VERIFICATION */ + olen = ctx->len; MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &T, output, olen ) ); @@ -537,8 +609,9 @@ cleanup: return( MBEDTLS_ERR_THREADING_MUTEX_ERROR ); #endif - mbedtls_mpi_free( &T ); mbedtls_mpi_free( &T1 ); mbedtls_mpi_free( &T2 ); - mbedtls_mpi_free( &P1 ); mbedtls_mpi_free( &Q1 ); mbedtls_mpi_free( &R ); + mbedtls_mpi_free( &P1 ); + mbedtls_mpi_free( &Q1 ); + mbedtls_mpi_free( &R ); if( f_rng != NULL ) { @@ -550,6 +623,17 @@ cleanup: #endif } + mbedtls_mpi_free( &T ); + +#if !defined(MBEDTLS_RSA_NO_CRT) + mbedtls_mpi_free( &TP ); mbedtls_mpi_free( &TQ ); +#endif + +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + mbedtls_mpi_free( &IP ); mbedtls_mpi_free( &IQ ); + mbedtls_mpi_free( &CP ); mbedtls_mpi_free( &CQ ); +#endif + if( ret != 0 ) return( MBEDTLS_ERR_RSA_PRIVATE_FAILED + ret ); From b624b85b04e3b335ba6e03f1d06d7c5167bf7843 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 May 2017 09:00:08 +0100 Subject: [PATCH 05/24] Adapt ChangeLog --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index 08edd7796..b6ab9665a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Fix memory leak in RSA self test. + +Security + * Add option for mandatory use of blinding in RSA private key operations. + * Add options for verification of RSA private key operations to defend + against Bellcore glitch attack. + = mbed TLS 2.x.x branch released xxxx-xx-xx Security From 9f4e670b14b41ac2978469852acae943f8a2b19c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Jun 2017 10:23:19 +0100 Subject: [PATCH 06/24] Correct documentation for RSA_FORCE_BLINDING option --- include/mbedtls/config.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 1ce92c5a1..d54f0c382 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -987,9 +987,12 @@ * of Diffie-Hellman, RSA, DSS, and Other Systems] * * \note Disabling this does not mean that blinding - * will never be used, but instead makes private - * key operations fail if, perhaps unintentionally, - * the user failed to call them with a PRNG. + * will never be used: if a PRNG is provided, + * blinding will be in place. Instead, disabling this + * option may result in private key operations being + * performed in a way potentially leaking sensitive + * information through side-channels when no PRNG + * is supplied by the user. * * \note For more on the use of blinding in RSA * private key operations, see the documentation From c6075cc5acccf5bdd105a31300da7957a16e7ce3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:45:35 +0100 Subject: [PATCH 07/24] Don't use CRT for signature verification If CRT is not used, the helper fields CRT are not assumed to be present in the RSA context structure, so do the verification directly in this case. If CRT is used, verification could be done using CRT, but we're sticking to ordinary verification for uniformity. --- library/rsa.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index d3feeba88..0c5bc4fb5 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -428,15 +428,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - /* Temporaries holding input mod p resp. mod q. */ - mbedtls_mpi IP, IQ; - - /* Temporaries holding double check results mod p resp. mod q; - * should in the end have the same values as IP and IQ. */ - mbedtls_mpi CP, CQ; - - /* Comparison results */ - int check = 0; + /* Temporaries holding the initial input and the double + * checked result; should be the same in the end. */ + mbedtls_mpi I, C; #endif #if defined(MBEDTLS_RSA_FORCE_BLINDING) @@ -476,8 +470,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - mbedtls_mpi_init( &IP ); mbedtls_mpi_init( &IQ ); - mbedtls_mpi_init( &CP ); mbedtls_mpi_init( &CQ ); + mbedtls_mpi_init( &I ); + mbedtls_mpi_init( &C ); #endif /* End of MPI initialization */ @@ -490,8 +484,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, } #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IP, &T, &ctx->P ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IQ, &T, &ctx->Q ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) ); #endif if( f_rng != NULL ) @@ -583,18 +576,11 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } - /* If requested by the config, verify the result to prevent glitching attacks. - * For that, check the two prime moduli separately. */ + /* If requested by the config, verify the result to prevent glitching attacks. */ #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CP, &T, &ctx->E, &ctx->P, &ctx->RP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CQ, &T, &ctx->E, &ctx->Q, &ctx->RQ ) ); - - check |= mbedtls_mpi_cmp_mpi( &CP, &IP ); - check |= mbedtls_mpi_cmp_mpi( &CQ, &IQ ); - - if( check != 0 ) + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, &ctx->N, &ctx->RN ) ); + if( mbedtls_mpi_cmp_mpi( &C, &I ) != 0 ) { - /* Verification failed */ ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; goto cleanup; } @@ -630,8 +616,8 @@ cleanup: #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - mbedtls_mpi_free( &IP ); mbedtls_mpi_free( &IQ ); - mbedtls_mpi_free( &CP ); mbedtls_mpi_free( &CQ ); + mbedtls_mpi_free( &C ); + mbedtls_mpi_free( &I ); #endif if( ret != 0 ) @@ -1245,11 +1231,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 ); From 43f94721ab4e331517b71e678d9c5a72b6834958 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:50:00 +0100 Subject: [PATCH 08/24] Add quick-check for presence of relevant parameters in rsa_private --- library/rsa.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 0c5bc4fb5..9b7d346c2 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -425,7 +425,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Pointer to actual exponent to be used - either the unblinded * or the blinded one, depending on the presence of a PRNG. */ mbedtls_mpi *D = &ctx->D; -#endif +#endif /* MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) /* Temporaries holding the initial input and the double @@ -438,9 +438,24 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); #endif - /* Make sure we have private key info, prevent possible misuse */ - if( ctx->P.p == NULL || ctx->Q.p == NULL || ctx->D.p == NULL ) + /* Sanity-check that all relevant fields are at least set, + * but don't perform a full keycheck. */ + if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ) + { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + } +#if !defined(MBEDTLS_RSA_NO_CRT) + if( 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 ) @@ -1294,7 +1309,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_public( ctx, sig, sig ) ); /* * In order to prevent Lenstra's attack, make the signature in a From cc209ca56d0592404f5019a03f4887e383f956d0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:51:03 +0100 Subject: [PATCH 09/24] Remove signature verification from rsa_rsassa_pkcs1_v15_sign This verification path is redundant now that verification is uniformly done in rsa_private. --- library/rsa.c | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 9b7d346c2..680df0d8e 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1311,42 +1311,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, if( mode == MBEDTLS_RSA_PUBLIC ) return( mbedtls_rsa_public( ctx, 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 ); + return( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig ) ); } #endif /* MBEDTLS_PKCS1_V15 */ From 936f72c641c0953cc288d01de30a2dd811b5f8ac Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 10:56:10 +0100 Subject: [PATCH 10/24] Disable MBEDTLS_RSA_FORCE_BLINDING by default This commit disables the new MBEDTLS_RSA_FORCE_BLINDING option by default to preserve backwards compatibility. Further, it deprecates disabling to prepare for a future release in which blinding will be unconditionally enforced. --- include/mbedtls/config.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d54f0c382..741ce416a 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -980,6 +980,11 @@ * Comment this macro to allow RSA private key operations * without blinding. * + * \deprecated Disabling this option is deprecated and only + * disabled by default for backwards compatibility. + * Future versions of Mbed TLS will remove this + * option and enforce blinding unconditionally. + * * \warning Disabling this can be a security risk! * Blinding RSA private key operations is a way * to prevent statistical timing attacks as in @@ -998,7 +1003,7 @@ * private key operations, see the documentation * of \c mbedtls_rsa_private. */ -#define MBEDTLS_RSA_FORCE_BLINDING +//#define MBEDTLS_RSA_FORCE_BLINDING /** * \def MBEDTLS_RSA_NO_CRT From 6ac972d815107812be6df8ab591e475208709720 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 10:57:48 +0100 Subject: [PATCH 11/24] Style correction in test_suite_pk.function --- tests/suites/test_suite_pk.function | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 33453ac6f..a6372c52a 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -43,8 +43,9 @@ 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, rnd_std_rand, NULL, mode, olen, - input, output, output_max_len ) ); + return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, + 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, @@ -107,7 +108,8 @@ void mbedtls_pk_check_pair( char *pub_file, char *prv_file, int ret ) if( mbedtls_pk_get_type( &prv ) == MBEDTLS_PK_RSA ) { TEST_ASSERT( mbedtls_pk_setup_rsa_alt( &alt, mbedtls_pk_rsa( prv ), - mbedtls_rsa_decrypt_func, mbedtls_rsa_sign_func, mbedtls_rsa_key_len_func ) == 0 ); + mbedtls_rsa_decrypt_func, mbedtls_rsa_sign_func, + mbedtls_rsa_key_len_func ) == 0 ); TEST_ASSERT( mbedtls_pk_check_pair( &pub, &alt ) == ret ); } #endif From a988a2702ab402e119502f9759347b12d91c0ee4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 11:32:04 +0100 Subject: [PATCH 12/24] Emit deprecation warning if MBEDTLS_RSA_FORCE_BLINDING is not set --- library/rsa.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/rsa.c b/library/rsa.c index 680df0d8e..88257aa57 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -66,6 +66,13 @@ #define mbedtls_free free #endif +#if !defined(MBEDTLS_RSA_FORCE_BLINDING) && \ + defined(MBEDTLS_DEPRECATED_WARNING) +#warning Not enforcing blinding checks for RSA private key operations\ + is deprecated. Please uncomment MBEDTLS_RSA_FORCE_BLINDING\ + in config.h to enforce blinding checks. +#endif + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0; From 8d1dd1b5b9ffd1e615d1dea6524c8ea53a13216a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:02:24 +0100 Subject: [PATCH 13/24] Fix bug in mbedtls_mpi_exp_mod Calling `mbedtls_mpi_exp_mod` with a freshly initialized exponent MPI `N`, i.e. `N.p == NULL`, would lead to a null-pointer dereference. --- library/bignum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index 8b9082cdc..e9ac56505 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1614,7 +1614,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 ) From 2c9f027e32f3fc83ccb3d24d132a77a711bd141b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:04:13 +0100 Subject: [PATCH 14/24] Don't require P,Q if CRT is not used Previously, verification used P,Q regardless of whether CRT was used in the computation, but this has changed in the meantime. --- library/rsa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 88257aa57..11ba2019a 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -448,15 +448,15 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } #if !defined(MBEDTLS_RSA_NO_CRT) - if( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 || + if( 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 ) { From 7c0f17d1155d8a3e0fd52f831ecc84ce11673f2e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:49:46 +0100 Subject: [PATCH 15/24] Add `MBEDTLS_RSA_NO_CRT` to options unaffected by `config.pl full` The effect of `config.pl full` on 'negative' options such as `NO_PLATFORM_ENTROPY` is usually inverted, but `MBEDTLS_RSA_NO_CRT` was not included in the list of such options. This commit adds it. --- scripts/config.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/config.pl b/scripts/config.pl index 2757f17fe..e2760b15c 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -17,7 +17,7 @@ # # Full usage description provided below. # -# Things that shouldn't be enabled with "full". +# The following options are disabled instead of enabled with "full". # # MBEDTLS_TEST_NULL_ENTROPY # MBEDTLS_DEPRECATED_REMOVED @@ -30,6 +30,7 @@ # 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 # - this could be enabled if the respective tests were adapted @@ -85,6 +86,7 @@ MBEDTLS_ECP_DP_M383_ENABLED MBEDTLS_ECP_DP_M511_ENABLED MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES MBEDTLS_NO_PLATFORM_ENTROPY +MBEDTLS_RSA_NO_CRT MBEDTLS_REMOVE_ARC4_CIPHERSUITES MBEDTLS_SSL_HW_RECORD_ACCEL MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 From d5ba5effaa30addc721f27f65b15a97af3f33248 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 12:53:51 +0100 Subject: [PATCH 16/24] Add ASan build-and-test run for MBEDTLS_RSA_NO_CRT in all.sh --- tests/scripts/all.sh | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7c33c5c2c..5fe9191cc 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -326,6 +326,22 @@ OPENSSL_CMD="$OPENSSL_LEGACY" tests/compat.sh -m 'ssl3' msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min 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, C99" # ~ 50s cleanup cp "$CONFIG_H" "$CONFIG_BAK" @@ -572,4 +588,3 @@ rm -rf "$OUT_OF_SOURCE_DIR" msg "Done, cleaning up" cleanup - From a6f55394137487b7298ab929202d70b5f210c7c2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 12:56:28 +0100 Subject: [PATCH 17/24] Adapt version_features.c to new config options --- library/version_features.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/version_features.c b/library/version_features.c index 9f97c7bc3..f7fa041c4 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -345,9 +345,18 @@ static const char *features[] = { #if defined(MBEDTLS_PKCS1_V21) "MBEDTLS_PKCS1_V21", #endif /* MBEDTLS_PKCS1_V21 */ +#if defined(MBEDTLS_RSA_FORCE_BLINDING) + "MBEDTLS_RSA_FORCE_BLINDING", +#endif /* MBEDTLS_RSA_FORCE_BLINDING */ #if defined(MBEDTLS_RSA_NO_CRT) "MBEDTLS_RSA_NO_CRT", #endif /* MBEDTLS_RSA_NO_CRT */ +#if defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION) + "MBEDTLS_RSA_FORCE_CRT_VERIFICATION", +#endif /* MBEDTLS_RSA_FORCE_CRT_VERIFICATION */ +#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) + "MBEDTLS_RSA_FORCE_VERIFICATION", +#endif /* MBEDTLS_RSA_FORCE_VERIFICATION */ #if defined(MBEDTLS_SELF_TEST) "MBEDTLS_SELF_TEST", #endif /* MBEDTLS_SELF_TEST */ From 2fdffe0da0bf74cb94682730fe2db6b0ba8472fa Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 29 Sep 2017 15:19:28 +0100 Subject: [PATCH 18/24] Check exactly for the RSA context fields required in rsa_private Previously, the code was also checking for the presence of D for RSA-CRT, which is not needed in this case. --- library/rsa.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 11ba2019a..d866c7aa3 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -447,14 +447,19 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ +#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 ) + mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } -#if !defined(MBEDTLS_RSA_NO_CRT) - if( mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || +#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 || @@ -462,7 +467,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } -#endif /* MBEDTLS_RSA_NO_CRT */ +#endif /* ! MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_THREADING_C) if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) From 4e1be398f64170a10495561e91ccc27aa31f94a3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 2 Oct 2017 15:56:48 +0100 Subject: [PATCH 19/24] Remove FORCE_VERIFICATION and FORCE_BLINDING --- include/mbedtls/config.h | 77 -------------------------------------- include/mbedtls/rsa.h | 31 +++------------ library/rsa.c | 22 ----------- library/version_features.c | 9 ----- 4 files changed, 5 insertions(+), 134 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 741ce416a..52556262a 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -970,41 +970,6 @@ */ #define MBEDTLS_PKCS1_V21 -/** - * \def MBEDTLS_RSA_FORCE_BLINDING - * - * Force the use of blinding in RSA private key operations. - * This makes these operations fail when the caller doesn't - * provide a PRNG. - * - * Comment this macro to allow RSA private key operations - * without blinding. - * - * \deprecated Disabling this option is deprecated and only - * disabled by default for backwards compatibility. - * Future versions of Mbed TLS will remove this - * option and enforce blinding unconditionally. - * - * \warning Disabling this can be a security risk! - * Blinding RSA private key operations is a way - * to prevent statistical timing attacks as in - * [P. Kocher ', Timing Attacks on Implementations - * of Diffie-Hellman, RSA, DSS, and Other Systems] - * - * \note Disabling this does not mean that blinding - * will never be used: if a PRNG is provided, - * blinding will be in place. Instead, disabling this - * option may result in private key operations being - * performed in a way potentially leaking sensitive - * information through side-channels when no PRNG - * is supplied by the user. - * - * \note For more on the use of blinding in RSA - * private key operations, see the documentation - * of \c mbedtls_rsa_private. - */ -//#define MBEDTLS_RSA_FORCE_BLINDING - /** * \def MBEDTLS_RSA_NO_CRT * @@ -1016,48 +981,6 @@ */ //#define MBEDTLS_RSA_NO_CRT -/** - * \def MBEDTLS_RSA_FORCE_CRT_VERIFICATION - * - * Force verification of results of RSA private key operations - * when RSA-CRT is used. - * - * Comment this macro to disable RSA-CRT verification. - * - * \warning Disabling this can be a security risk! - * Omitting verification makes the RSA-CRT - * signing vulnerable to the Bellcore - * glitch attack leading to private key - * compromise if an attacker can cause a - * glitch in a certain timeframe during - * the signing operation. Uncomment only - * if you're sure that glitches are out of - * your attack model. - */ -#define MBEDTLS_RSA_FORCE_CRT_VERIFICATION - -/** - * \def MBEDTLS_RSA_FORCE_VERIFICATION - * - * Force verification of results of any RSA private key - * operation regardless of the algorithm used. - * - * Uncomment this to enable unconditional RSA verification. - * - * \note This is to prevent the RSA signing operation - * (regardless of the particular algorithm chosen) - * from potential future glitch attacks. We are - * currently not aware of any such for our default - * implementation, therefore disabling the option - * by default. - * - * \note Enabling it comes at the cost of roughly an - * additional public key operation at the end of - * signing (low compared to private key operations), - * as well as minor memory consumption. - */ -//#define MBEDTLS_RSA_FORCE_VERIFICATION - /** * \def MBEDTLS_SELF_TEST * diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index e34fea0f2..bc2f810ae 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -63,15 +63,6 @@ #define MBEDTLS_RSA_SALT_LEN_ANY -1 -/* - * RSA configuration - */ -#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) || \ - ( ! defined(MBEDTLS_RSA_NO_CRT) && \ - defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION ) ) -#define MBEDTLS_RSA_REQUIRE_VERIFICATION -#endif - /* * The above constants may be used even if the RSA module is compile out, * eg for alternative (PKCS#11) RSA implemenations in the PK layers. @@ -239,28 +230,16 @@ 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 Enabling and disabling of blinding: - * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING - * is disabled, blinding is disabled. - * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING - * is enabled, the function fails. + * \note Blinding is used if and onlf if a PRNG is provided. * * \note If blinding is used, both the base of exponentation * and the exponent are blinded, preventing both statistical * timing and power analysis attacks. * - * \note Depending on the way RSA is implemented, a failure - * in the computation can lead to disclosure of the private - * key if the wrong result is passed to attacker - e.g., - * implementing RSA through CRT is vulnerable to the - * Bellcore glitch attack. - * - * As a remedy, the user can force double checking the - * result of the private key operation through the option - * MBEDTLS_RSA_FORCE_VERIFICATION. If verification is - * to be enabled only when RSA-CRT is used (as controlled - * by the configuration option MBEDTLS_RSA_NO_CRT), the - * option MBEDTLS_RSA_FORCE_CRT_VERIFICATION can be used. + * \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, diff --git a/library/rsa.c b/library/rsa.c index d866c7aa3..de684b39c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -66,13 +66,6 @@ #define mbedtls_free free #endif -#if !defined(MBEDTLS_RSA_FORCE_BLINDING) && \ - defined(MBEDTLS_DEPRECATED_WARNING) -#warning Not enforcing blinding checks for RSA private key operations\ - is deprecated. Please uncomment MBEDTLS_RSA_FORCE_BLINDING\ - in config.h to enforce blinding checks. -#endif - /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0; @@ -434,16 +427,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, mbedtls_mpi *D = &ctx->D; #endif /* MBEDTLS_RSA_NO_CRT */ -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) /* Temporaries holding the initial input and the double * checked result; should be the same in the end. */ mbedtls_mpi I, C; -#endif - -#if defined(MBEDTLS_RSA_FORCE_BLINDING) - if( f_rng == NULL ) - return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); -#endif /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ @@ -496,10 +482,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, mbedtls_mpi_init( &TP ); mbedtls_mpi_init( &TQ ); #endif -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) mbedtls_mpi_init( &I ); mbedtls_mpi_init( &C ); -#endif /* End of MPI initialization */ @@ -510,9 +494,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, goto cleanup; } -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) ); -#endif if( f_rng != NULL ) { @@ -604,14 +586,12 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, } /* If requested by the config, verify the result to prevent glitching attacks. */ -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) 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; } -#endif /* MBEDTLS_RSA_REQUIRE_VERIFICATION */ olen = ctx->len; MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &T, output, olen ) ); @@ -642,10 +622,8 @@ cleanup: mbedtls_mpi_free( &TP ); mbedtls_mpi_free( &TQ ); #endif -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) mbedtls_mpi_free( &C ); mbedtls_mpi_free( &I ); -#endif if( ret != 0 ) return( MBEDTLS_ERR_RSA_PRIVATE_FAILED + ret ); diff --git a/library/version_features.c b/library/version_features.c index f7fa041c4..9f97c7bc3 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -345,18 +345,9 @@ static const char *features[] = { #if defined(MBEDTLS_PKCS1_V21) "MBEDTLS_PKCS1_V21", #endif /* MBEDTLS_PKCS1_V21 */ -#if defined(MBEDTLS_RSA_FORCE_BLINDING) - "MBEDTLS_RSA_FORCE_BLINDING", -#endif /* MBEDTLS_RSA_FORCE_BLINDING */ #if defined(MBEDTLS_RSA_NO_CRT) "MBEDTLS_RSA_NO_CRT", #endif /* MBEDTLS_RSA_NO_CRT */ -#if defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION) - "MBEDTLS_RSA_FORCE_CRT_VERIFICATION", -#endif /* MBEDTLS_RSA_FORCE_CRT_VERIFICATION */ -#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) - "MBEDTLS_RSA_FORCE_VERIFICATION", -#endif /* MBEDTLS_RSA_FORCE_VERIFICATION */ #if defined(MBEDTLS_SELF_TEST) "MBEDTLS_SELF_TEST", #endif /* MBEDTLS_SELF_TEST */ From 2dec5e8b00d25f2fd6946172eb3b30177a4b124e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 3 Oct 2017 07:49:52 +0100 Subject: [PATCH 20/24] Correct outdated comment --- library/rsa.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index de684b39c..56f434563 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -585,8 +585,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } - /* If requested by the config, verify the result to prevent glitching attacks. */ - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, &ctx->N, &ctx->RN ) ); + /* 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; From 7bba968afcb9d2a352d2e39cc9eae5a338d94c53 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 26 Oct 2017 11:53:26 +0100 Subject: [PATCH 21/24] Adapt ChangeLog --- ChangeLog | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index b6ab9665a..2f1f0557c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,12 +2,8 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx -Bugfix - * Fix memory leak in RSA self test. - Security - * Add option for mandatory use of blinding in RSA private key operations. - * Add options for verification of RSA private key operations to defend + * Verify results of RSA private key operations to defend against Bellcore glitch attack. = mbed TLS 2.x.x branch released xxxx-xx-xx From 2412061a5a55410e8fffc583b3ce3a2f0dfc067d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 26 Oct 2017 11:53:35 +0100 Subject: [PATCH 22/24] Correct typo and improve documentation --- include/mbedtls/rsa.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index bc2f810ae..54a1f2520 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -230,11 +230,11 @@ 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 onlf if a PRNG is provided. + * \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, preventing both statistical - * timing and power analysis attacks. + * 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. From 63073aa3d389500251fcda9bcb0eb3e9d4774f3d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 27 Nov 2017 15:33:18 +0000 Subject: [PATCH 23/24] Don't require P,Q in rsa_private in case of non-blinded non-CRT For non-CRT, P and Q are only used for the purpose of blinding the exponent. --- library/rsa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 56f434563..35ace85c5 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -437,8 +437,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, 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 || - mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->Q, 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 ); } From 6f486a6fb5c7311a8d07913778b53f128ec37cd8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Mar 2018 13:31:44 +0000 Subject: [PATCH 24/24] Fix merge error --- tests/scripts/all.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 140a90f09..e60530fd7 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -881,8 +881,5 @@ rm -rf "$OUT_OF_SOURCE_DIR" msg "Done, cleaning up" cleanup -<<<<<<< HEAD -======= final_report ->>>>>>> development-restricted