Skip to content

Commit

Permalink
RSA: Use hashlen as the hash input size as documented
Browse files Browse the repository at this point in the history 
Where hashlen was previously ignored when the hash length could be
inferred from an md_alg parameter, the two must now match.

Adapt the existing tests accordingly. Adapt the sample programs accordingly.

This commit does not add any negative testing.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
  • Loading branch information
@gilles-peskine-arm
gilles-peskine-arm committed Jun 22, 2021
1 parent 9dbbc29 commit 6e3187b
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 56 deletions.
20 changes: 12 additions & 8 deletions library/rsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1799,7 +1799,8 @@ static int rsa_rsassa_pss_sign( mbedtls_rsa_context *ctx,
if( md_info == NULL )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );

hashlen = mbedtls_md_get_size( md_info );
if( hashlen != mbedtls_md_get_size( md_info ) )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
}

md_info = mbedtls_md_info_from_type( (mbedtls_md_type_t) ctx->hash_id );
Expand Down Expand Up @@ -1934,14 +1935,13 @@ int mbedtls_rsa_rsassa_pss_sign( mbedtls_rsa_context *ctx,
* Parameters:
* - md_alg: Identifies the hash algorithm used to generate the given hash;
* MBEDTLS_MD_NONE if raw data is signed.
* - hashlen: Length of hash in case hashlen is MBEDTLS_MD_NONE.
* - hashlen: Length of hash. Must match md_alg if that's not NONE.
* - hash: Buffer containing the hashed message or the raw data.
* - dst_len: Length of the encoded message.
* - dst: Buffer to hold the encoded message.
*
* Assumptions:
* - hash has size hashlen if md_alg == MBEDTLS_MD_NONE.
* - hash has size corresponding to md_alg if md_alg != MBEDTLS_MD_NONE.
* - hash has size hashlen.
* - dst points to a buffer of size at least dst_len.
*
*/
Expand All @@ -1966,7 +1966,8 @@ static int rsa_rsassa_pkcs1_v15_encode( mbedtls_md_type_t md_alg,
if( mbedtls_oid_get_oid_by_md( md_alg, &oid, &oid_size ) != 0 )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );

hashlen = mbedtls_md_get_size( md_info );
if( hashlen != mbedtls_md_get_size( md_info ) )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );

/* Double-check that 8 + hashlen + oid_size can be used as a
* 1-byte ASN.1 length encoding and that there's no overflow. */
Expand Down Expand Up @@ -2031,6 +2032,8 @@ static int rsa_rsassa_pkcs1_v15_encode( mbedtls_md_type_t md_alg,
* TAG-NULL + LEN [ NULL ] ]
* TAG-OCTET + LEN [ HASH ] ]
*/
if( 0x08 + oid_size + hashlen >= 0x80 )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
*p++ = MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED;
*p++ = (unsigned char)( 0x08 + oid_size + hashlen );
*p++ = MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED;
Expand Down Expand Up @@ -2212,7 +2215,8 @@ int mbedtls_rsa_rsassa_pss_verify_ext( mbedtls_rsa_context *ctx,
if( md_info == NULL )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );

hashlen = mbedtls_md_get_size( md_info );
if( hashlen != mbedtls_md_get_size( md_info ) )
return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA );
}

md_info = mbedtls_md_info_from_type( mgf1_hash_id );
Expand Down Expand Up @@ -2683,7 +2687,7 @@ int mbedtls_rsa_self_test( int verbose )
}

if( mbedtls_rsa_pkcs1_sign( &rsa, myrand, NULL,
MBEDTLS_MD_SHA1, 0,
MBEDTLS_MD_SHA1, 20,
sha1sum, rsa_ciphertext ) != 0 )
{
if( verbose != 0 )
Expand All @@ -2696,7 +2700,7 @@ int mbedtls_rsa_self_test( int verbose )
if( verbose != 0 )
mbedtls_printf( "passed\n PKCS#1 sig. verify: " );

if( mbedtls_rsa_pkcs1_verify( &rsa, MBEDTLS_MD_SHA1, 0,
if( mbedtls_rsa_pkcs1_verify( &rsa, MBEDTLS_MD_SHA1, 20,
sha1sum, rsa_ciphertext ) != 0 )
{
if( verbose != 0 )
Expand Down
2 changes: 1 addition & 1 deletion programs/pkey/dh_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ int main( void )
}

if( ( ret = mbedtls_rsa_pkcs1_verify( &rsa, MBEDTLS_MD_SHA256,
0, hash, p ) ) != 0 )
32, hash, p ) ) != 0 )
{
mbedtls_printf( " failed\n ! mbedtls_rsa_pkcs1_verify returned %d\n\n", ret );
goto exit;
Expand Down
2 changes: 1 addition & 1 deletion programs/pkey/dh_server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ int main( void )
buf[n + 1] = (unsigned char)( rsa.MBEDTLS_PRIVATE(len) );

if( ( ret = mbedtls_rsa_pkcs1_sign( &rsa, NULL, NULL, MBEDTLS_MD_SHA256,
0, hash, buf + n + 2 ) ) != 0 )
32, hash, buf + n + 2 ) ) != 0 )
{
mbedtls_printf( " failed\n ! mbedtls_rsa_pkcs1_sign returned %d\n\n", ret );
goto exit;
Expand Down
2 changes: 1 addition & 1 deletion programs/pkey/rsa_sign.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ int main( int argc, char *argv[] )
}

if( ( ret = mbedtls_rsa_pkcs1_sign( &rsa, NULL, NULL, MBEDTLS_MD_SHA256,
20, hash, buf ) ) != 0 )
32, hash, buf ) ) != 0 )
{
mbedtls_printf( " failed\n ! mbedtls_rsa_pkcs1_sign returned -0x%0x\n\n", (unsigned int) -ret );
goto exit;
Expand Down
2 changes: 1 addition & 1 deletion programs/pkey/rsa_verify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ int main( int argc, char *argv[] )
}

if( ( ret = mbedtls_rsa_pkcs1_verify( &rsa, MBEDTLS_MD_SHA256,
20, hash, buf ) ) != 0 )
32, hash, buf ) ) != 0 )
{
mbedtls_printf( " failed\n ! mbedtls_rsa_pkcs1_verify returned -0x%0x\n\n", (unsigned int) -ret );
goto exit;
Expand Down
22 changes: 12 additions & 10 deletions tests/suites/test_suite_pk.function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,10 @@ void pk_rsa_verify_ext_test_vec( data_t * message_str, int digest,

if( digest != MBEDTLS_MD_NONE )
{
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ),
message_str->x, message_str->len, hash_result ) == 0 );
hash_len = 0;
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len,
hash_result ) == 0 );
hash_len = mbedtls_md_get_size( md_info );
}
else
{
Expand Down Expand Up @@ -611,7 +612,8 @@ void pk_sign_verify( int type, int parameter, int sign_ret, int verify_ret )
{
mbedtls_pk_context pk;
size_t sig_len;
unsigned char hash[MBEDTLS_MD_MAX_SIZE];
unsigned char hash[32]; // Hard-coded for SHA256
size_t hash_len = sizeof( hash );
unsigned char sig[MBEDTLS_PK_SIGNATURE_MAX_SIZE];
void *rs_ctx = NULL;
#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
Expand All @@ -635,30 +637,30 @@ void pk_sign_verify( int type, int parameter, int sign_ret, int verify_ret )
TEST_ASSERT( pk_genkey( &pk, parameter ) == 0 );

TEST_ASSERT( mbedtls_pk_sign_restartable( &pk, MBEDTLS_MD_SHA256,
hash, sizeof hash, sig, &sig_len,
hash, hash_len, sig, &sig_len,
mbedtls_test_rnd_std_rand, NULL, rs_ctx ) == sign_ret );
if( sign_ret == 0 )
TEST_ASSERT( sig_len <= MBEDTLS_PK_SIGNATURE_MAX_SIZE );
else
sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE;

TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256,
hash, sizeof hash, sig, sig_len ) == verify_ret );
hash, hash_len, sig, sig_len ) == verify_ret );

if( verify_ret == 0 )
{
hash[0]++;
TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256,
hash, sizeof hash, sig, sig_len ) != 0 );
hash, hash_len, sig, sig_len ) != 0 );
hash[0]--;

sig[0]++;
TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256,
hash, sizeof hash, sig, sig_len ) != 0 );
hash, hash_len, sig, sig_len ) != 0 );
sig[0]--;
}

TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash,
TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, hash_len,
sig, &sig_len,
mbedtls_test_rnd_std_rand,
NULL ) == sign_ret );
Expand All @@ -668,7 +670,7 @@ void pk_sign_verify( int type, int parameter, int sign_ret, int verify_ret )
sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE;

TEST_ASSERT( mbedtls_pk_verify_restartable( &pk, MBEDTLS_MD_SHA256,
hash, sizeof hash, sig, sig_len, rs_ctx ) == verify_ret );
hash, hash_len, sig, sig_len, rs_ctx ) == verify_ret );

if( verify_ret == 0 )
{
Expand Down
20 changes: 11 additions & 9 deletions tests/suites/test_suite_pkcs1_v15.function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ void pkcs1_rsassa_v15_sign( int mod, int radix_P, char * input_P, int radix_Q,
data_t * result_str, int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
unsigned char output[128];
mbedtls_rsa_context ctx;
mbedtls_mpi N, P, Q, E;
Expand Down Expand Up @@ -298,13 +299,13 @@ void pkcs1_rsassa_v15_sign( int mod, int radix_P, char * input_P, int radix_Q,
TEST_ASSERT( mbedtls_rsa_complete( &ctx ) == 0 );
TEST_ASSERT( mbedtls_rsa_check_privkey( &ctx ) == 0 );

if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );

TEST_ASSERT( mbedtls_rsa_pkcs1_sign( &ctx, &mbedtls_test_rnd_buffer_rand,
&info, digest, 0, hash_result,
output ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_sign(
&ctx, &mbedtls_test_rnd_buffer_rand, &info,
digest, mbedtls_md_get_size( md_info ), hash_result,
output ) == result );
if( result == 0 )
{

Expand All @@ -326,6 +327,7 @@ void pkcs1_rsassa_v15_verify( int mod, int radix_N, char * input_N,
data_t * result_str, int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
mbedtls_rsa_context ctx;
mbedtls_mpi N, E;
((void) salt);
Expand All @@ -343,10 +345,10 @@ void pkcs1_rsassa_v15_verify( int mod, int radix_N, char * input_N,
TEST_ASSERT( mbedtls_rsa_check_pubkey( &ctx ) == 0 );


if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );
if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, 0, hash_result, result_str->x ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, mbedtls_md_get_size( md_info ), hash_result, result_str->x ) == result );

exit:
mbedtls_mpi_free( &N ); mbedtls_mpi_free( &E );
Expand Down
35 changes: 21 additions & 14 deletions tests/suites/test_suite_pkcs1_v21.function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ void pkcs1_rsassa_pss_sign( int mod, data_t * input_P, data_t * input_Q,
int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
unsigned char output[512];
mbedtls_rsa_context ctx;
mbedtls_test_rnd_buf_info info;
Expand Down Expand Up @@ -152,14 +153,15 @@ void pkcs1_rsassa_pss_sign( int mod, data_t * input_P, data_t * input_Q,
TEST_ASSERT( mbedtls_rsa_complete( &ctx ) == 0 );
TEST_ASSERT( mbedtls_rsa_check_privkey( &ctx ) == 0 );

if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );
if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

if (fixed_salt_length == MBEDTLS_RSA_SALT_LEN_ANY)
{
TEST_ASSERT( mbedtls_rsa_pkcs1_sign( &ctx, &mbedtls_test_rnd_buffer_rand,
&info, digest, 0,hash_result,
output ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_sign(
&ctx, &mbedtls_test_rnd_buffer_rand, &info,
digest, mbedtls_md_get_size( md_info ), hash_result,
output ) == result );
if( result == 0 )
{
ASSERT_COMPARE( output, ctx.len, result_str->x, result_str->len );
Expand All @@ -169,9 +171,10 @@ void pkcs1_rsassa_pss_sign( int mod, data_t * input_P, data_t * input_Q,
info.length = rnd_buf->len;
}

TEST_ASSERT( mbedtls_rsa_rsassa_pss_sign_ext( &ctx, &mbedtls_test_rnd_buffer_rand,
&info, digest, 0, hash_result,
fixed_salt_length, output ) == result );
TEST_ASSERT( mbedtls_rsa_rsassa_pss_sign_ext(
&ctx, &mbedtls_test_rnd_buffer_rand, &info,
digest, mbedtls_md_get_size( md_info ), hash_result,
fixed_salt_length, output ) == result );
if( result == 0 )
{
ASSERT_COMPARE( output, ctx.len, result_str->x, result_str->len );
Expand All @@ -190,6 +193,7 @@ void pkcs1_rsassa_pss_verify( int mod, data_t * input_N, data_t * input_E,
char * salt, data_t * result_str, int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
mbedtls_rsa_context ctx;
mbedtls_mpi N, E;
((void) salt);
Expand All @@ -208,10 +212,10 @@ void pkcs1_rsassa_pss_verify( int mod, data_t * input_N, data_t * input_E,
TEST_ASSERT( mbedtls_rsa_check_pubkey( &ctx ) == 0 );


if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );
if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, 0, hash_result, result_str->x ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, mbedtls_md_get_size( md_info ), hash_result, result_str->x ) == result );

exit:
mbedtls_mpi_free( &N ); mbedtls_mpi_free( &E );
Expand Down Expand Up @@ -248,9 +252,12 @@ void pkcs1_rsassa_pss_verify_ext( int mod, data_t * input_N, data_t * input_E,

if( msg_digest_id != MBEDTLS_MD_NONE )
{
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( msg_digest_id ),
message_str->x, message_str->len, hash_result ) == 0 );
hash_len = 0;
const mbedtls_md_info_t *md_info =
mbedtls_md_info_from_type( msg_digest_id );
TEST_ASSERT( mbedtls_md( md_info,
message_str->x, message_str->len,
hash_result ) == 0 );
hash_len = mbedtls_md_get_size( md_info );
}
else
{
Expand Down
22 changes: 11 additions & 11 deletions tests/suites/test_suite_rsa.function
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ void mbedtls_rsa_pkcs1_sign( data_t * message_str, int padding_mode,
data_t * result_str, int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
unsigned char output[256];
mbedtls_rsa_context ctx;
mbedtls_mpi N, P, Q, E;
Expand All @@ -111,13 +112,13 @@ void mbedtls_rsa_pkcs1_sign( data_t * message_str, int padding_mode,
TEST_ASSERT( mbedtls_rsa_complete( &ctx ) == 0 );
TEST_ASSERT( mbedtls_rsa_check_privkey( &ctx ) == 0 );

if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );

TEST_ASSERT( mbedtls_rsa_pkcs1_sign( &ctx, &mbedtls_test_rnd_pseudo_rand,
&rnd_info, digest, 0, hash_result,
output ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_sign(
&ctx, &mbedtls_test_rnd_pseudo_rand, &rnd_info,
digest, mbedtls_md_get_size( md_info ), hash_result,
output ) == result );
if( result == 0 )
{

Expand All @@ -139,8 +140,8 @@ void mbedtls_rsa_pkcs1_verify( data_t * message_str, int padding_mode,
data_t * result_str, int result )
{
unsigned char hash_result[MBEDTLS_MD_MAX_SIZE];
const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( digest );
mbedtls_rsa_context ctx;

mbedtls_mpi N, E;

mbedtls_mpi_init( &N ); mbedtls_mpi_init( &E );
Expand All @@ -155,11 +156,10 @@ void mbedtls_rsa_pkcs1_verify( data_t * message_str, int padding_mode,
TEST_ASSERT( mbedtls_rsa_get_len( &ctx ) == (size_t) ( mod / 8 ) );
TEST_ASSERT( mbedtls_rsa_check_pubkey( &ctx ) == 0 );

if( md_info != NULL )
TEST_ASSERT( mbedtls_md( md_info, message_str->x, message_str->len, hash_result ) == 0 );

if( mbedtls_md_info_from_type( digest ) != NULL )
TEST_ASSERT( mbedtls_md( mbedtls_md_info_from_type( digest ), message_str->x, message_str->len, hash_result ) == 0 );

TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, 0, hash_result, result_str->x ) == result );
TEST_ASSERT( mbedtls_rsa_pkcs1_verify( &ctx, digest, mbedtls_md_get_size( md_info ), hash_result, result_str->x ) == result );

exit:
mbedtls_mpi_free( &N ); mbedtls_mpi_free( &E );
Expand Down

0 comments on commit 6e3187b

Please sign in to comment.