diff --git a/ChangeLog b/ChangeLog index 2a993b99b..0dac49721 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,13 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 2.7.X branch released XXXX-XX-XX +Security + * To avoid a side channel vulnerability when parsing an RSA private key, + read all the CRT parameters from the DER structure rather than + reconstructing them. Found by Alejandro Cabrera Aldaya and Billy Bob + Brumley. Reported and fix contributed by Jack Lloyd. + ARMmbed/mbed-crypto#352 + Bugfix * Allow loading symlinked certificates. Fixes #3005. Reported and fixed by Jonathan Bennett via #3008. diff --git a/library/pkparse.c b/library/pkparse.c index ec9b55f8c..3bad0ce6d 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -754,14 +754,40 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa, goto cleanup; p += len; - /* Complete the RSA private key */ - if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 ) - goto cleanup; +#if !defined(MBEDTLS_RSA_NO_CRT) + /* + * The RSA CRT parameters DP, DQ and QP are nominally redundant, in + * that they can be easily recomputed from D, P and Q. However by + * parsing them from the PKCS1 structure it is possible to avoid + * recalculating them which both reduces the overhead of loading + * RSA private keys into memory and also avoids side channels which + * can arise when computing those values, since all of D, P, and Q + * are secret. See https://eprint.iacr.org/2020/055 for a + * description of one such attack. + */ - /* Check optional parameters */ + /* Import DP */ + if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0) + goto cleanup; + + /* Import DQ */ + if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0) + goto cleanup; + + /* Import QP */ + if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0) + goto cleanup; + +#else + /* Verify existance of the CRT params */ if( ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 ) + goto cleanup; +#endif + + /* Complete the RSA private key */ + if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 ) goto cleanup; if( p != end ) diff --git a/library/rsa.c b/library/rsa.c index 4b3cc0213..98c529f00 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -251,6 +251,12 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) const int have_D = ( mbedtls_mpi_cmp_int( &ctx->D, 0 ) != 0 ); const int have_E = ( mbedtls_mpi_cmp_int( &ctx->E, 0 ) != 0 ); +#if !defined(MBEDTLS_RSA_NO_CRT) + const int have_DP = ( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) != 0 ); + const int have_DQ = ( mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) != 0 ); + const int have_QP = ( mbedtls_mpi_cmp_int( &ctx->QP, 0 ) != 0 ); +#endif + /* * Check whether provided parameters are enough * to deduce all others. The following incomplete @@ -316,7 +322,7 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) */ #if !defined(MBEDTLS_RSA_NO_CRT) - if( is_priv ) + if( is_priv && ! ( have_DP && have_DQ && have_QP ) ) { ret = mbedtls_rsa_deduce_crt( &ctx->P, &ctx->Q, &ctx->D, &ctx->DP, &ctx->DQ, &ctx->QP );