Skip to content

Commit

Permalink
Parse RSA CRT parameters from PKCS1 private keys
Browse files Browse the repository at this point in the history
Currently they are ignored in the serialized key and then regenerated
from P/Q/D. But that exposes key loading to a side channel attack on
the modular inversion and GCD bignum functions when QP is computed.
[And probably similar issues for DP/DQ during the division step but no
attack there has been published yet.]

Backport of ARMmbed/mbed-crypto#352
  • Loading branch information
jack-fortanix committed Jan 22, 2020
1 parent e860574 commit 43d1579
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
27 changes: 21 additions & 6 deletions mbedtls-sys/vendor/library/pkparse.c
Original file line number Diff line number Diff line change
Expand Up @@ -768,14 +768,29 @@ 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 )
/* Import DP */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len,
MBEDTLS_ASN1_INTEGER ) ) != 0 ||
( ret = mbedtls_mpi_read_binary( &rsa->DP, p, len ) ) != 0 )
goto cleanup;
p += len;

/* Check optional parameters */
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 )
/* Import DQ */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len,
MBEDTLS_ASN1_INTEGER ) ) != 0 ||
( ret = mbedtls_mpi_read_binary( &rsa->DQ, p, len ) ) != 0 )
goto cleanup;
p += len;

/* Import QP */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len,
MBEDTLS_ASN1_INTEGER ) ) != 0 ||
( ret = mbedtls_mpi_read_binary( &rsa->QP, p, len ) ) != 0 )
goto cleanup;
p += len;

/* Complete the RSA private key */
if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 )
goto cleanup;

if( p != end )
Expand Down
8 changes: 6 additions & 2 deletions mbedtls-sys/vendor/library/rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ static int rsa_check_context( mbedtls_rsa_context const *ctx, int is_priv,
int mbedtls_rsa_complete( mbedtls_rsa_context *ctx )
{
int ret = 0;
int have_N, have_P, have_Q, have_D, have_E;
int have_N, have_P, have_Q, have_D, have_E, have_DP, have_DQ, have_QP;
int n_missing, pq_missing, d_missing, is_pub, is_priv;

RSA_VALIDATE_RET( ctx != NULL );
Expand All @@ -258,6 +258,10 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx )
have_Q = ( mbedtls_mpi_cmp_int( &ctx->Q, 0 ) != 0 );
have_D = ( mbedtls_mpi_cmp_int( &ctx->D, 0 ) != 0 );
have_E = ( mbedtls_mpi_cmp_int( &ctx->E, 0 ) != 0 );
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 );


/*
* Check whether provided parameters are enough
Expand Down Expand Up @@ -324,7 +328,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 );
Expand Down

0 comments on commit 43d1579

Please sign in to comment.