diff --git a/ChangeLog b/ChangeLog index 0eb0702129..b8ac52b1a9 100644 --- a/ChangeLog +++ b/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. diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index 6b2e54b493..8e34e6269c 100644 --- a/include/mbedtls/rsa.h +++ b/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), diff --git a/library/bignum.c b/library/bignum.c index 25fe8beeb1..165cf87db3 100644 --- a/library/bignum.c +++ b/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 ) diff --git a/library/rsa.c b/library/rsa.c index d2bddf6628..13beba4dfd 100644 --- a/library/rsa.c +++ b/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 ) ); - /* - * 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 */ @@ -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 ) diff --git a/scripts/config.pl b/scripts/config.pl index 1464813586..dcf028110a 100755 --- a/scripts/config.pl +++ b/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 diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 9728bd52ae..f6b59dba17 100755 --- a/tests/scripts/all.sh +++ b/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" diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index f291c2aab7..f76cdce08a 100644 --- a/tests/suites/test_suite_pk.function +++ b/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, - 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, 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, - md_alg, hashlen, hash, sig ) ); + ((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 ) {