Merge pull request #367 from mpg/fix-rsa-complete

Fix pk_parse_key()'s use of rsa_complete()
This commit is contained in:
Gilles Peskine 2020-02-19 10:23:03 +01:00 committed by GitHub
commit f841eab48f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 129 additions and 57 deletions

View File

@ -678,6 +678,32 @@ int mbedtls_pk_parse_subpubkey( unsigned char **p, const unsigned char *end,
} }
#if defined(MBEDTLS_RSA_C) #if defined(MBEDTLS_RSA_C)
/*
* Wrapper around mbedtls_asn1_get_mpi() that rejects zero.
*
* The value zero is:
* - never a valid value for an RSA parameter
* - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete().
*
* Since values can't be omitted in PKCS#1, passing a zero value to
* rsa_complete() would be incorrect, so reject zero values early.
*/
static int asn1_get_nonzero_mpi( unsigned char **p,
const unsigned char *end,
mbedtls_mpi *X )
{
int ret;
ret = mbedtls_asn1_get_mpi( p, end, X );
if( ret != 0 )
return( ret );
if( mbedtls_mpi_cmp_int( X, 0 ) == 0 )
return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT );
return( 0 );
}
/* /*
* Parse a PKCS#1 encoded private RSA key * Parse a PKCS#1 encoded private RSA key
*/ */
@ -730,46 +756,36 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa,
} }
/* Import N */ /* Import N */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
MBEDTLS_ASN1_INTEGER ) ) != 0 || ( ret = mbedtls_rsa_import( rsa, &T, NULL, NULL,
( ret = mbedtls_rsa_import_raw( rsa, p, len, NULL, 0, NULL, 0, NULL, NULL ) ) != 0 )
NULL, 0, NULL, 0 ) ) != 0 )
goto cleanup; goto cleanup;
p += len;
/* Import E */ /* Import E */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
MBEDTLS_ASN1_INTEGER ) ) != 0 || ( ret = mbedtls_rsa_import( rsa, NULL, NULL, NULL,
( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, NULL, 0, NULL, &T ) ) != 0 )
NULL, 0, p, len ) ) != 0 )
goto cleanup; goto cleanup;
p += len;
/* Import D */ /* Import D */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
MBEDTLS_ASN1_INTEGER ) ) != 0 || ( ret = mbedtls_rsa_import( rsa, NULL, NULL, NULL,
( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, NULL, 0, &T, NULL ) ) != 0 )
p, len, NULL, 0 ) ) != 0 )
goto cleanup; goto cleanup;
p += len;
/* Import P */ /* Import P */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
MBEDTLS_ASN1_INTEGER ) ) != 0 || ( ret = mbedtls_rsa_import( rsa, NULL, &T, NULL,
( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, p, len, NULL, 0, NULL, NULL ) ) != 0 )
NULL, 0, NULL, 0 ) ) != 0 )
goto cleanup; goto cleanup;
p += len;
/* Import Q */ /* Import Q */
if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
MBEDTLS_ASN1_INTEGER ) ) != 0 || ( ret = mbedtls_rsa_import( rsa, NULL, NULL, &T,
( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, p, len, NULL, NULL ) ) != 0 )
NULL, 0, NULL, 0 ) ) != 0 )
goto cleanup; goto cleanup;
p += len;
#if !defined(MBEDTLS_RSA_NO_CRT) #if !defined(MBEDTLS_RSA_NO_CRT) && !defined(MBEDTLS_RSA_ALT)
/* /*
* The RSA CRT parameters DP, DQ and QP are nominally redundant, in * 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 * that they can be easily recomputed from D, P and Q. However by
@ -782,28 +798,42 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa,
*/ */
/* Import DP */ /* Import DP */
if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DP ) ) != 0) if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_mpi_copy( &rsa->DP, &T ) ) != 0 )
goto cleanup; goto cleanup;
/* Import DQ */ /* Import DQ */
if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->DQ ) ) != 0) if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_mpi_copy( &rsa->DQ, &T ) ) != 0 )
goto cleanup; goto cleanup;
/* Import QP */ /* Import QP */
if( ( ret = mbedtls_asn1_get_mpi( &p, end, &rsa->QP ) ) != 0) if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_mpi_copy( &rsa->QP, &T ) ) != 0 )
goto cleanup; goto cleanup;
#else #else
/* Verify existance of the CRT params */ /* Verify existance of the CRT params */
if( ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ||
( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 ) ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 )
goto cleanup; goto cleanup;
#endif #endif
/* Complete the RSA private key */ /* rsa_complete() doesn't complete anything with the default
if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 ) * implementation but is still called:
* - for the benefit of alternative implementation that may want to
* pre-compute stuff beyond what's provided (eg Montgomery factors)
* - as is also sanity-checks the key
*
* Furthermore, we also check the public part for consistency with
* mbedtls_pk_parse_pubkey(), as it includes size minima for example.
*/
if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 ||
( ret = mbedtls_rsa_check_pubkey( rsa ) ) != 0 )
{
goto cleanup; goto cleanup;
}
if( p != end ) if( p != end )
{ {

View File

@ -1072,33 +1072,84 @@ Parse EC Key #15 (SEC1 DER, secp256k1, SpecifiedECDomain)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256K1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256K1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_keyfile_ec:"data_files/ec_prv.specdom.der":"NULL":0 pk_parse_keyfile_ec:"data_files/ec_prv.specdom.der":"NULL":0
Key ASN1 (Incorrect first tag) Key ASN1 (No data)
pk_parse_key:"":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (First tag not Sequence)
pk_parse_key:"020100":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, incorrect version tag) Key ASN1 (RSAPrivateKey, incorrect version tag)
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"300100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"300100":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, version tag missing) Key ASN1 (RSAPrivateKey, version tag missing)
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"3000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"3000":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, invalid version) Key ASN1 (RSAPrivateKey, invalid version)
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"3003020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"3003020101":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct version, incorrect tag) Key ASN1 (RSAPrivateKey, correct version, incorrect tag)
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"300402010000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"300402010000":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, values present, length mismatch) Key ASN1 (RSAPrivateKey, correct format+values, minimal modulus size (128 bit))
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"301c02010002010102010102010102010102010102010102010102010100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":0
Key ASN1 (RSAPrivateKey, values present, check_privkey fails) Key ASN1 (RSAPrivateKey, correct format, modulus too small (127 bit))
depends_on:MBEDTLS_RSA_C depends_on:MBEDTLS_RSA_C
pk_parse_key:"301b020100020102020101020101020101020101020101020101020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"30630201000211007c8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct format, modulus even)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857002030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct format, d == 0)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"30630201000211007c8ab070369ede72920e5a51523c8571020301000102110000000000000000000000000000000000020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct format, d == p == q == 0)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c8571020301000102110000000000000000000000000000000000020900000000000000000002090000000000000000000209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, trailing garbage)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3064020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c00":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, n wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100FF1100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, e wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c8571FF030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, d wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c85710203010001FF11009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, p wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201FF0900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, q wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61FF0900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, dp wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a211FF09009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, dq wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401FF0813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (RSAPrivateKey, correct values, qp wrong tag)
depends_on:MBEDTLS_RSA_C
pk_parse_key:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b7221FF08052b93d01747a87c":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (ECPrivateKey, empty parameters) Key ASN1 (ECPrivateKey, empty parameters)
depends_on:MBEDTLS_ECP_C depends_on:MBEDTLS_ECP_C
pk_parse_key:"30070201010400a000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT pk_parse_key:"30070201010400a000":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT

View File

@ -113,23 +113,14 @@ exit:
} }
/* END_CASE */ /* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_RSA_C */ /* BEGIN_CASE */
void pk_parse_key( data_t * buf, char * result_str, int result ) void pk_parse_key( data_t * buf, int result )
{ {
mbedtls_pk_context pk; mbedtls_pk_context pk;
unsigned char output[2000];
((void) result_str);
mbedtls_pk_init( &pk ); mbedtls_pk_init( &pk );
memset( output, 0, 2000 ); TEST_ASSERT( mbedtls_pk_parse_key( &pk, buf->x, buf->len, NULL, 0 ) == result );
TEST_ASSERT( mbedtls_pk_parse_key( &pk, buf->x, buf->len, NULL, 0 ) == ( result ) );
if( ( result ) == 0 )
{
TEST_ASSERT( 1 );
}
exit: exit:
mbedtls_pk_free( &pk ); mbedtls_pk_free( &pk );