Tidy up attribute management inside psa_crypto

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;

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

library/psa_crypto_storage.h

@@ -35,9 +35,14 @@ extern "C" {
 #include <stdint.h>
 #include <string.h>
 
-/* 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.
  *

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

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,

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

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() );