From 4019f0e914416ca6c80bfad51509404265b5bf45 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 12 Sep 2019 22:05:59 +0200 Subject: [PATCH] Immediately reject 0-size signature buffer when signing 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. --- library/psa_crypto.c | 8 +++++++- tests/suites/test_suite_psa_crypto.data | 8 ++++++++ ...st_suite_psa_crypto_se_driver_hal_mocks.function | 13 +++++++++++-- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ac2eae6670..c53d15b011 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -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 ) @@ -3422,7 +3428,7 @@ exit: 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. */ diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 16edd382ab..9df4b43be8 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -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 diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal_mocks.function b/tests/suites/test_suite_psa_crypto_se_driver_hal_mocks.function index e3641789fa..e6b3f7b1fe 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal_mocks.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal_mocks.function @@ -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 == @@ -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; @@ -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 ); @@ -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 ) ); @@ -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 );