From c744d99386adda801ea879814e47a08f73a94cc0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 30 Jul 2019 17:26:54 +0200 Subject: [PATCH] Limit keys to 65528 bits 65528 bits is more than any reasonable key until we start supporting post-quantum cryptography. This limit is chosen to allow bit-sizes to be stored in 16 bits, with 65535 left to indicate an invalid value. It's a whole number of bytes, which facilitates some calculations, in particular allowing a key of exactly PSA_CRYPTO_MAX_STORAGE_SIZE to be created but not one bit more. As a resource usage limit, this is arguably too large, but that's out of scope of the current commit. Test that key import, generation and derivation reject overly large sizes. --- include/psa/crypto_struct.h | 6 ++ library/psa_crypto.c | 16 ++- library/psa_crypto_storage.h | 11 +- tests/suites/test_suite_psa_crypto.data | 26 +++++ tests/suites/test_suite_psa_crypto.function | 102 +++++++++++++++++- .../test_suite_psa_crypto_persistent_key.data | 4 +- ...t_suite_psa_crypto_persistent_key.function | 7 +- 7 files changed, 158 insertions(+), 14 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index fea59df37..b37b0b5cc 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -309,6 +309,12 @@ static inline struct psa_key_policy_s psa_key_policy_init( void ) return( v ); } +/* The maximum size of a key in bits. + * This is a whole number of bytes, to facilitate some calculations + * such as the maximum size of key data in storage. + */ +#define PSA_MAX_KEY_BITS 0xfff8 + typedef struct { psa_key_type_t type; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4721f6bfe..4c93dd0ad 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -706,11 +706,14 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot, if( key_type_is_raw_bytes( slot->type ) ) { - /* Ensure that a bytes-to-bit conversion won't overflow. */ + size_t bit_size = PSA_BYTES_TO_BITS( data_length ); + /* Ensure that the bytes-to-bit conversion doesn't overflow. */ if( data_length > SIZE_MAX / 8 ) return( PSA_ERROR_NOT_SUPPORTED ); - status = prepare_raw_data_slot( slot->type, - PSA_BYTES_TO_BITS( data_length ), + /* Ensure that the key is not overly large. */ + if( bit_size > PSA_MAX_KEY_BITS ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = prepare_raw_data_slot( slot->type, bit_size, &slot->data.raw ); if( status != PSA_SUCCESS ) return( status ); @@ -1470,6 +1473,13 @@ static psa_status_t psa_start_key_creation( } slot->type = attributes->core.type; + /* Refuse to create overly large keys. + * Note that this doesn't trigger on import if the attributes don't + * explicitly specify a size (so psa_get_key_bits returns 0), so + * psa_import_key() needs its own checks. */ + if( psa_get_key_bits( attributes ) > PSA_MAX_KEY_BITS ) + return( PSA_ERROR_NOT_SUPPORTED ); + #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* For a key in a secure element, we need to do three things: * create the key file in internal storage, create the diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 8fe20ac32..938cc4f89 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -35,9 +35,14 @@ extern "C" { #include #include -/* Limit the maximum key size to 30kB (just in case someone tries to - * inadvertently store an obscene amount of data) */ -#define PSA_CRYPTO_MAX_STORAGE_SIZE ( 30 * 1024 ) +/* Limit the maximum key size in storage. This should have no effect + * since the key size is limited in memory. */ +#define PSA_CRYPTO_MAX_STORAGE_SIZE ( PSA_BITS_TO_BYTES( PSA_MAX_KEY_BITS ) ) +/* Sanity check: a file size must fit in 32 bits. Allow a generous + * 64kB of metadata. */ +#if PSA_CRYPTO_MAX_STORAGE_SIZE > 0xffff0000 +#error PSA_CRYPTO_MAX_STORAGE_SIZE > 0xffff0000 +#endif /** The maximum permitted persistent slot number. * diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 53f842201..a0e7f7a90 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -314,6 +314,14 @@ PSA import AES: bits=128 wrong depends_on:MBEDTLS_AES_C import:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_KEY_TYPE_AES:128:PSA_ERROR_INVALID_ARGUMENT +PSA import large key: raw, 65528 bits (ok) +depends_on:HAVE_RAM_AVAILABLE_128k +import_large_key:PSA_KEY_TYPE_RAW_DATA:8191:PSA_SUCCESS + +PSA import large key: raw, 65536 bits (not supported) +depends_on:HAVE_RAM_AVAILABLE_128k +import_large_key:PSA_KEY_TYPE_RAW_DATA:8192:PSA_ERROR_NOT_SUPPORTED + PSA import RSA key pair: maximum size exceeded depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_RSA_C import_rsa_made_up:PSA_VENDOR_RSA_MAX_KEY_BITS+8:1:PSA_ERROR_NOT_SUPPORTED @@ -2075,6 +2083,17 @@ PSA key derivation: TLS 1.2 PRF SHA-256, derive key export, 1+41 depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C:!PSA_PRE_1_0_KEY_DERIVATION derive_key_export:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":1:41 +# This test assumes that PSA_MAX_KEY_BITS (currently 65536-8 bits = 8191 bytes +# and not expected to be raised any time soon) is less than the maximum +# output from HKDF-SHA512 (255*64 = 16320 bytes). +PSA key derivation: largest possible key +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C:!PSA_PRE_1_0_KEY_DERIVATION +derive_large_key:PSA_ALG_HKDF(PSA_ALG_SHA_512):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_MAX_KEY_BITS:PSA_SUCCESS + +PSA key derivation: key too large +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C:!PSA_PRE_1_0_KEY_DERIVATION +derive_large_key:PSA_ALG_HKDF(PSA_ALG_SHA_512):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_MAX_KEY_BITS + 1:PSA_ERROR_NOT_SUPPORTED + PSA key agreement setup: ECDH + HKDF-SHA-256: good depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECDH_C:MBEDTLS_MD_C:MBEDTLS_SHA256_C key_agreement_setup:PSA_ALG_KEY_AGREEMENT(PSA_ALG_ECDH, PSA_ALG_HKDF(PSA_ALG_SHA_256)):PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_CURVE_SECP256R1):"c88f01f510d9ac3f70a292daa2316de544e9aab8afe84049c62a9c57862d1433":"04d12dfb5289c8d4f81208b70270398c342296970a0bccb74c736fc7554494bf6356fbf3ca366cc23e8157854c13c58d6aac23f046ada30f8353e74f33039872ab":PSA_SUCCESS @@ -2188,6 +2207,13 @@ generate_key:PSA_KEY_TYPE_RAW_DATA:7:PSA_KEY_USAGE_EXPORT:0:PSA_ERROR_INVALID_AR PSA generate key: raw data, 8 bits generate_key:PSA_KEY_TYPE_RAW_DATA:8:PSA_KEY_USAGE_EXPORT:0:PSA_SUCCESS +PSA generate key: raw data, 65528 bits (ok) +depends_on:HAVE_RAM_AVAILABLE_128k +generate_key:PSA_KEY_TYPE_RAW_DATA:8:PSA_KEY_USAGE_EXPORT:0:PSA_SUCCESS + +PSA generate key: raw data, 65536 bits (not supported) +generate_key:PSA_KEY_TYPE_RAW_DATA:65536:PSA_KEY_USAGE_EXPORT:0:PSA_ERROR_NOT_SUPPORTED + PSA generate key: AES, 128 bits, CTR depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR generate_key:PSA_KEY_TYPE_AES:128:PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:PSA_ALG_CTR:PSA_SUCCESS diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 887ff84d1..8ed7a7d5c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -7,6 +7,13 @@ #include "psa_crypto_helpers.h" +/* Tests that require more than 128kB of RAM plus change have this symbol + * as a dependency. Currently we always define this symbol, so the tests + * are always executed. In the future we should make this conditional + * so that tests that require a lot of memory are skipped on constrained + * platforms. */ +#define HAVE_RAM_AVAILABLE_128k + /** An invalid export length that will never be set by psa_export_key(). */ static const size_t INVALID_EXPORT_LENGTH = ~0U; @@ -556,7 +563,8 @@ static int setup_key_derivation_wrap( psa_key_derivation_operation_t* operation, TEST_ASSERT( ! "Key derivation algorithm not supported" ); } - PSA_ASSERT( psa_key_derivation_set_capacity( operation, capacity ) ); + if( capacity != SIZE_MAX ) + PSA_ASSERT( psa_key_derivation_set_capacity( operation, capacity ) ); return( 1 ); @@ -1237,6 +1245,54 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void import_large_key( int type_arg, int byte_size_arg, + int expected_status_arg ) +{ + psa_key_type_t type = type_arg; + size_t byte_size = byte_size_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_status_t expected_status = expected_status_arg; + psa_key_handle_t handle = 0; + psa_status_t status; + uint8_t *buffer = NULL; + size_t buffer_size = byte_size + 1; + size_t n; + + /* It would be better to skip the test than fail it if the allocation + * fails, but the test framework doesn't support this yet. */ + ASSERT_ALLOC( buffer, buffer_size ); + memset( buffer, 'K', byte_size ); + + PSA_ASSERT( psa_crypto_init( ) ); + + /* Try importing the key */ + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT ); + psa_set_key_type( &attributes, type ); + status = psa_import_key( &attributes, buffer, byte_size, &handle ); + TEST_EQUAL( status, expected_status ); + + if( status == PSA_SUCCESS ) + { + PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + TEST_EQUAL( psa_get_key_type( &attributes ), type ); + TEST_EQUAL( psa_get_key_bits( &attributes ), + PSA_BYTES_TO_BITS( byte_size ) ); + memset( buffer, 0, byte_size + 1 ); + PSA_ASSERT( psa_export_key( handle, buffer, byte_size, &n ) ); + for( n = 0; n < byte_size; n++ ) + TEST_EQUAL( buffer[n], 'K' ); + for( n = byte_size; n < buffer_size; n++ ) + TEST_EQUAL( buffer[n], 0 ); + } + +exit: + psa_destroy_key( handle ); + PSA_DONE( ); + mbedtls_free( buffer ); +} +/* END_CASE */ + /* BEGIN_CASE */ void import_rsa_made_up( int bits_arg, int keypair, int expected_status_arg ) { @@ -4563,6 +4619,50 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void derive_large_key( int alg_arg, + data_t *key_data, data_t *input1, data_t *input2, + int bits_arg, + int expected_status_arg ) +{ + psa_key_handle_t base_handle = 0; + psa_key_handle_t derived_handle = 0; + psa_algorithm_t alg = alg_arg; + size_t bits = bits_arg; + psa_status_t expected_status = expected_status_arg; + psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; + psa_key_attributes_t base_attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_attributes_t derived_attributes = PSA_KEY_ATTRIBUTES_INIT; + + PSA_ASSERT( psa_crypto_init( ) ); + + psa_set_key_usage_flags( &base_attributes, PSA_KEY_USAGE_DERIVE ); + psa_set_key_algorithm( &base_attributes, alg ); + psa_set_key_type( &base_attributes, PSA_KEY_TYPE_DERIVE ); + PSA_ASSERT( psa_import_key( &base_attributes, key_data->x, key_data->len, + &base_handle ) ); + + if( !setup_key_derivation_wrap( &operation, base_handle, alg, + input1->x, input1->len, + input2->x, input2->len, SIZE_MAX ) ) + goto exit; + + psa_set_key_usage_flags( &derived_attributes, PSA_KEY_USAGE_EXPORT ); + psa_set_key_algorithm( &derived_attributes, 0 ); + psa_set_key_type( &derived_attributes, PSA_KEY_TYPE_RAW_DATA ); + psa_set_key_bits( &derived_attributes, bits ); + TEST_EQUAL( psa_key_derivation_output_key( &derived_attributes, &operation, + &derived_handle ), + expected_status ); + +exit: + psa_key_derivation_abort( &operation ); + psa_destroy_key( base_handle ); + psa_destroy_key( derived_handle ); + PSA_DONE( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void key_agreement_setup( int alg_arg, int our_key_type_arg, data_t *our_key_data, diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.data b/tests/suites/test_suite_psa_crypto_persistent_key.data index 925c0f54a..3f40d35c7 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.data +++ b/tests/suites/test_suite_psa_crypto_persistent_key.data @@ -19,10 +19,10 @@ parse_storage_data_check:"505341004b4559":"":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY # Not specific to files, but only run this test in an environment where the maximum size could be reached. Save maximum size persistent raw key depends_on:MBEDTLS_PSA_ITS_FILE_C -save_large_persistent_key:0:PSA_SUCCESS +save_large_persistent_key:PSA_CRYPTO_MAX_STORAGE_SIZE:PSA_SUCCESS Save larger than maximum size persistent raw key, should fail -save_large_persistent_key:1:PSA_ERROR_INSUFFICIENT_STORAGE +save_large_persistent_key:PSA_CRYPTO_MAX_STORAGE_SIZE + 1:PSA_ERROR_NOT_SUPPORTED Persistent key destroy depends_on:MBEDTLS_PK_C:MBEDTLS_PK_PARSE_C:MBEDTLS_RSA_C diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index b76c7330a..61f7f886a 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -96,17 +96,14 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void save_large_persistent_key( int data_too_large, int expected_status ) +void save_large_persistent_key( int data_length_arg, int expected_status ) { psa_key_id_t key_id = 42; psa_key_handle_t handle = 0; uint8_t *data = NULL; - size_t data_length = PSA_CRYPTO_MAX_STORAGE_SIZE; + size_t data_length = data_length_arg; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - if( data_too_large ) - data_length += 1; - ASSERT_ALLOC( data, data_length ); PSA_ASSERT( psa_crypto_init() );