Merge pull request #3016 from jack-fortanix/jack/parse-rsa-crt-2.16

Backport 2.16: Parse RSA parameters DP, DQ and QP from PKCS1 private keys

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 <JBennett@incomsystems.biz> 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 <JBennett@incomsystems.biz> via #3008.
+   * Fix an unchecked call to mbedtls_md() in the x509write module.
 
 = mbed TLS 2.16.4 branch released 2020-01-15
 

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 )

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 );