diff --git a/ChangeLog b/ChangeLog index f03b83d09..a8f9a79b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,11 +2,6 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 2.16.X branch released XXXX-XX-XX -Bugfix - * Allow loading symlinked certificates. Fixes #3005. Reported and fixed - by Jonathan Bennett via #3008. - * Fix an unchecked call to mbedtls_md() in the x509write module. - Security * Fix potential memory overread when performing an ECDSA signature operation. The overread only happens with cryptographically low @@ -14,8 +9,16 @@ Security unless the RNG is broken, and could result in information disclosure or denial of service (application crash or extra resource consumption). Found by Auke Zeilstra and Peter Schwabe, using static analysis. + * 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. + * Fix an unchecked call to mbedtls_md() in the x509write module. = mbed TLS 2.16.4 branch released 2020-01-15 diff --git a/library/pkparse.c b/library/pkparse.c index ae210bca6..0ae24027f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -768,14 +768,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 af1a87859..09fd379fd 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -249,6 +249,9 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) { int ret = 0; int have_N, have_P, have_Q, have_D, have_E; +#if !defined(MBEDTLS_RSA_NO_CRT) + int have_DP, have_DQ, have_QP; +#endif int n_missing, pq_missing, d_missing, is_pub, is_priv; RSA_VALIDATE_RET( ctx != NULL ); @@ -259,6 +262,12 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) have_D = ( mbedtls_mpi_cmp_int( &ctx->D, 0 ) != 0 ); have_E = ( mbedtls_mpi_cmp_int( &ctx->E, 0 ) != 0 ); +#if !defined(MBEDTLS_RSA_NO_CRT) + have_DP = ( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) != 0 ); + have_DQ = ( mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) != 0 ); + have_QP = ( mbedtls_mpi_cmp_int( &ctx->QP, 0 ) != 0 ); +#endif + /* * Check whether provided parameters are enough * to deduce all others. The following incomplete @@ -324,7 +333,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 );