Merge pull request #125 from ARMmbed/key_slot_index-fix

Fix off-by-one errors in key slot index limits

library/psa_crypto.c

@@ -82,6 +82,8 @@
 
 
 
+#define ARRAY_LENGTH( array ) ( sizeof( array ) / sizeof( *( array ) ) )
+
 /* Implementation that should never be optimized out by the compiler */
 static void mbedtls_zeroize( void *v, size_t n )
 {
@@ -343,10 +345,13 @@ static psa_status_t mbedtls_to_psa_error( int ret )
 static psa_status_t psa_get_key_slot( psa_key_slot_t key,
                                       key_slot_t **p_slot )
 {
-    if( key == 0 || key > PSA_KEY_SLOT_COUNT )
+    /* 0 is not a valid slot number under any circumstance. This
+     * implementation provides slots number 1 to N where N is the
+     * number of available slots. */
+    if( key == 0 || key > ARRAY_LENGTH( global_data.key_slots ) )
         return( PSA_ERROR_INVALID_ARGUMENT );
 
-    *p_slot = &global_data.key_slots[key];
+    *p_slot = &global_data.key_slots[key - 1];
     return( PSA_SUCCESS );
 }
 
@@ -3471,7 +3476,7 @@ psa_status_t psa_generate_key( psa_key_slot_t key,
 void mbedtls_psa_crypto_free( void )
 {
     psa_key_slot_t key;
-    for( key = 1; key < PSA_KEY_SLOT_COUNT; key++ )
+    for( key = 1; key <= PSA_KEY_SLOT_COUNT; key++ )
         psa_destroy_key( key );
     mbedtls_ctr_drbg_free( &global_data.ctr_drbg );
     mbedtls_entropy_free( &global_data.entropy );

tests/suites/test_suite_psa_crypto.data

@@ -1,6 +1,9 @@
 PSA init/deinit
 init_deinit:
 
+PSA fill 250 slots
+fill_slots:250
+
 PSA import/export raw: 0 bytes
 import_export:"":PSA_KEY_TYPE_RAW_DATA:PSA_ALG_CBC_BASE | PSA_ALG_BLOCK_CIPHER_PAD_NONE:PSA_KEY_USAGE_EXPORT:0:0:PSA_SUCCESS:1
 

tests/suites/test_suite_psa_crypto.function

@@ -447,6 +447,63 @@ void init_deinit( )
 }
 /* END_CASE */
 
+/* BEGIN_CASE */
+void fill_slots( int max_arg )
+{
+    /* Fill all the slots until we run out of memory or out of slots,
+     * or until some limit specified in the test data for the sake of
+     * implementations with an essentially unlimited number of slots.
+     * This test assumes that available slots are numbered from 1. */
+
+    psa_key_slot_t slot;
+    psa_key_slot_t max = 0;
+    psa_key_policy_t policy;
+    uint8_t exported[sizeof( max )];
+    size_t exported_size;
+    psa_status_t status;
+
+    TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS );
+
+    psa_key_policy_init( &policy );
+    psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_EXPORT, 0 );
+
+    for( max = 1; max <= (size_t) max_arg; max++ )
+    {
+        status = psa_set_key_policy( max, &policy );
+        /* Stop filling slots if we run out of memory or out of
+         * available slots. */
+        TEST_ASSERT( status == PSA_SUCCESS ||
+                     status == PSA_ERROR_INSUFFICIENT_MEMORY ||
+                     status == PSA_ERROR_INVALID_ARGUMENT );
+        if( status != PSA_SUCCESS )
+            break;
+        status = psa_import_key( max, PSA_KEY_TYPE_RAW_DATA,
+                                 (uint8_t*) &max, sizeof( max ) );
+        /* Since psa_set_key_policy succeeded, we know that the slot
+         * number is valid. But we may legitimately run out of memory. */
+        TEST_ASSERT( status == PSA_SUCCESS ||
+                     status == PSA_ERROR_INSUFFICIENT_MEMORY );
+        if( status != PSA_SUCCESS )
+            break;
+    }
+    /* `max` is now the first slot number that wasn't filled. */
+    max -= 1;
+
+    for( slot = 1; slot <= max; slot++ )
+    {
+        TEST_ASSERT( psa_export_key( slot,
+                                     exported, sizeof( exported ),
+                                     &exported_size ) == PSA_SUCCESS );
+        TEST_ASSERT( exported_size == sizeof( slot ) );
+        TEST_ASSERT( memcmp( exported, &slot, sizeof( slot ) ) == 0 );
+    }
+
+exit:
+    /* Do not destroy the keys. mbedtls_psa_crypto_free() should do it. */
+    mbedtls_psa_crypto_free( );
+}
+/* END_CASE */
+
 /* BEGIN_CASE */
 void import( data_t *data, int type, int expected_status_arg )
 {