Skip to content

Commit

Permalink
Immediately reject 0-size signature buffer when signing
Browse files Browse the repository at this point in the history
In psa_asymmetric_sign, immediately reject an empty signature buffer.
This can never be right.

Add test cases (one RSA and one ECDSA).

Change the SE HAL mock tests not to use an empty signature buffer.
  • Loading branch information
gilles-peskine-arm committed Sep 12, 2019
1 parent f916894 commit 4019f0e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
8 changes: 7 additions & 1 deletion library/psa_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -3347,6 +3347,12 @@ psa_status_t psa_asymmetric_sign( psa_key_handle_t handle,
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */

*signature_length = signature_size;
/* Immediately reject a zero-length signature buffer. This guarantees
* that signature must be a valid pointer. (On the other hand, the hash
* buffer can in principle be empty since it doesn't actually have
* to be a hash.) */
if( signature_size == 0 )
return( PSA_ERROR_BUFFER_TOO_SMALL );

status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_SIGN, alg );
if( status != PSA_SUCCESS )
Expand Down Expand Up @@ -3422,7 +3428,7 @@ psa_status_t psa_asymmetric_sign( psa_key_handle_t handle,
if( status == PSA_SUCCESS )
memset( signature + *signature_length, '!',
signature_size - *signature_length );
else if( signature_size != 0 )
else
memset( signature, '!', signature_size );
/* If signature_size is 0 then we have nothing to do. We must not call
* memset because signature may be NULL in this case. */
Expand Down
8 changes: 8 additions & 0 deletions tests/suites/test_suite_psa_crypto.data
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,14 @@ PSA sign: deterministic ECDSA SECP256R1 SHA-256, output buffer too small
depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C
sign_fail:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_CURVE_SECP256R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_DETERMINISTIC_ECDSA( PSA_ALG_SHA_256 ):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b":63:PSA_ERROR_BUFFER_TOO_SMALL

PSA sign: RSA PKCS#1 v1.5 SHA-256, empty output buffer
depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_SHA256_C
sign_fail:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256):"ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad":0:PSA_ERROR_BUFFER_TOO_SMALL

PSA sign: deterministic ECDSA SECP256R1 SHA-256, empty output buffer
depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C
sign_fail:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_CURVE_SECP256R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_DETERMINISTIC_ECDSA( PSA_ALG_SHA_256 ):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b":0:PSA_ERROR_BUFFER_TOO_SMALL

PSA sign: deterministic ECDSA SECP256R1, invalid hash algorithm (0)
depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECDSA_DETERMINISTIC
sign_fail:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_CURVE_SECP256R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_DETERMINISTIC_ECDSA( 0 ):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b":72:PSA_ERROR_INVALID_ARGUMENT
Expand Down
13 changes: 11 additions & 2 deletions tests/suites/test_suite_psa_crypto_se_driver_hal_mocks.function
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ void mock_generate( int mock_alloc_return_value,
psa_set_key_lifetime( &attributes, lifetime );
psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT );
psa_set_key_type( &attributes, PSA_KEY_TYPE_RAW_DATA );
psa_set_key_bits( &attributes, 8 );
TEST_ASSERT( psa_generate_key( &attributes, &handle ) == expected_result );
TEST_ASSERT( mock_allocate_data.called == 1 );
TEST_ASSERT( mock_generate_data.called ==
Expand Down Expand Up @@ -482,6 +483,8 @@ void mock_sign( int mock_sign_return_value, int expected_result )
psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
const uint8_t key_material[3] = {0xfa, 0xca, 0xde};
psa_algorithm_t algorithm = PSA_ALG_ECDSA(PSA_ALG_SHA_256);
const uint8_t hash[1] = {'H'};
uint8_t signature[1] = {'S'};
size_t signature_length;

mock_sign_data.return_value = mock_sign_return_value;
Expand Down Expand Up @@ -512,7 +515,9 @@ void mock_sign( int mock_sign_return_value, int expected_result )
key_material, sizeof( key_material ),
&handle ) );

TEST_ASSERT( psa_asymmetric_sign( handle, algorithm, NULL, 0, NULL, 0,
TEST_ASSERT( psa_asymmetric_sign( handle, algorithm,
hash, sizeof( hash ),
signature, sizeof( signature ),
&signature_length)
== expected_result );
TEST_ASSERT( mock_sign_data.called == 1 );
Expand All @@ -538,6 +543,8 @@ void mock_verify( int mock_verify_return_value, int expected_result )
psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
const uint8_t key_material[3] = {0xfa, 0xca, 0xde};
psa_algorithm_t algorithm = PSA_ALG_ECDSA(PSA_ALG_SHA_256);
const uint8_t hash[1] = {'H'};
const uint8_t signature[1] = {'S'};

mock_verify_data.return_value = mock_verify_return_value;
memset( &driver, 0, sizeof( driver ) );
Expand Down Expand Up @@ -567,7 +574,9 @@ void mock_verify( int mock_verify_return_value, int expected_result )
key_material, sizeof( key_material ),
&handle ) );

TEST_ASSERT( psa_asymmetric_verify( handle, algorithm, NULL, 0, NULL, 0)
TEST_ASSERT( psa_asymmetric_verify( handle, algorithm,
hash, sizeof( hash ),
signature, sizeof( signature ) )
== expected_result );
TEST_ASSERT( mock_verify_data.called == 1 );

Expand Down

0 comments on commit 4019f0e

Please sign in to comment.