From 98435ddf849f2fe16e27f0e93ff67a7108e814d2 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Fri, 8 Jan 2021 19:19:40 +0100
Subject: [PATCH] Allow loading wrapped keys even when SE support is compiled
 in

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c                          | 55 +++++++++++--------
 library/psa_crypto_slot_management.c          | 51 +++++++++++++----
 ...test_suite_psa_crypto_slot_management.data | 11 +++-
 3 files changed, 81 insertions(+), 36 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 7ea2a1a9a..e0ed35f0a 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2325,34 +2325,45 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes,
     if( status != PSA_SUCCESS )
         goto exit;
 
-#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
-    if( driver != NULL )
+    if( psa_key_lifetime_is_external( psa_get_key_lifetime( attributes ) ) )
     {
-        const psa_drv_se_t *drv = psa_get_se_driver_methods( driver );
-        /* The driver should set the number of key bits, however in
-         * case it doesn't, we initialize bits to an invalid value. */
-        size_t bits = PSA_MAX_KEY_BITS + 1;
-        if( drv->key_management == NULL ||
-            drv->key_management->p_import == NULL )
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+        if( driver != NULL )
         {
-            status = PSA_ERROR_NOT_SUPPORTED;
+            const psa_drv_se_t *drv = psa_get_se_driver_methods( driver );
+            /* The driver should set the number of key bits, however in
+             * case it doesn't, we initialize bits to an invalid value. */
+            size_t bits = PSA_MAX_KEY_BITS + 1;
+            if( drv->key_management == NULL ||
+                drv->key_management->p_import == NULL )
+            {
+                status = PSA_ERROR_NOT_SUPPORTED;
+                goto exit;
+            }
+            status = drv->key_management->p_import(
+                psa_get_se_driver_context( driver ),
+                slot->data.se.slot_number, attributes, data, data_length,
+                &bits );
+            if( status != PSA_SUCCESS )
+                goto exit;
+            if( bits > PSA_MAX_KEY_BITS )
+            {
+                status = PSA_ERROR_NOT_SUPPORTED;
+                goto exit;
+            }
+            slot->attr.bits = (psa_key_bits_t) bits;
+        }
+        else
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+        {
+            /* Importing a key with external lifetime through the driver wrapper
+             * interface is not yet supported. Return as if this was an invalid
+             * lifetime. */
+            status = PSA_ERROR_INVALID_ARGUMENT;
             goto exit;
         }
-        status = drv->key_management->p_import(
-            psa_get_se_driver_context( driver ),
-            slot->data.se.slot_number, attributes, data, data_length,
-            &bits );
-        if( status != PSA_SUCCESS )
-            goto exit;
-        if( bits > PSA_MAX_KEY_BITS )
-        {
-            status = PSA_ERROR_NOT_SUPPORTED;
-            goto exit;
-        }
-        slot->attr.bits = (psa_key_bits_t) bits;
     }
     else
-#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     {
         status = psa_import_key_into_slot( slot, data, data_length );
         if( status != PSA_SUCCESS )
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 4c4ad0331..f30b75ada 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -247,25 +247,42 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *slot )
     if( status != PSA_SUCCESS )
         goto exit;
 
-#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
     if( psa_key_lifetime_is_external( slot->attr.lifetime ) )
     {
-        psa_se_key_data_storage_t *data;
-        if( key_data_length != sizeof( *data ) )
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+        const psa_drv_se_t *drv;
+        psa_drv_se_context_t *drv_context;
+        if( psa_get_se_driver( slot->attr.lifetime, &drv, &drv_context ) )
         {
-            status = PSA_ERROR_STORAGE_FAILURE;
-            goto exit;
+            psa_se_key_data_storage_t *data;
+            if( key_data_length != sizeof( *data ) )
+            {
+                status = PSA_ERROR_STORAGE_FAILURE;
+                goto exit;
+            }
+            data = (psa_se_key_data_storage_t *) key_data;
+            memcpy( &slot->data.se.slot_number, &data->slot_number,
+                    sizeof( slot->data.se.slot_number ) );
+        }
+        else
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+        {
+            /* A key that is successfully loaded from storage with an
+             * external lifetime, but doesn't belong to an SE driver,
+             * must be a PSA driver-associated key which we can just
+             * load like an internal key. */
+            if ( key_data == NULL )
+            {
+                status = PSA_ERROR_STORAGE_FAILURE;
+                goto exit;
+            }
+
+            status = psa_copy_key_material_into_slot( slot, key_data, key_data_length );
         }
-        data = (psa_se_key_data_storage_t *) key_data;
-        memcpy( &slot->data.se.slot_number, &data->slot_number,
-                sizeof( slot->data.se.slot_number ) );
     }
     else
-#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     {
         status = psa_copy_key_material_into_slot( slot, key_data, key_data_length );
-        if( status != PSA_SUCCESS )
-            goto exit;
     }
 
 exit:
@@ -345,7 +362,14 @@ psa_status_t psa_validate_key_location( psa_key_lifetime_t lifetime,
 #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
         psa_se_drv_table_entry_t *driver = psa_get_se_driver_entry( lifetime );
         if( driver == NULL )
+        {
+#if defined(MBEDTLS_PSA_CRYPTO_DRIVERS)
+            /* Key location for external keys gets checked by the wrapper */
+            return( PSA_SUCCESS );
+#else
             return( PSA_ERROR_INVALID_ARGUMENT );
+#endif
+        }
         else
         {
             if (p_drv != NULL)
@@ -354,7 +378,12 @@ psa_status_t psa_validate_key_location( psa_key_lifetime_t lifetime,
         }
 #else
         (void) p_drv;
+#if defined(MBEDTLS_PSA_CRYPTO_DRIVERS)
+        /* Key location for external keys gets checked by the wrapper */
+        return( PSA_SUCCESS );
+#else
         return( PSA_ERROR_INVALID_ARGUMENT );
+#endif
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     }
     else
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data
index 396cdfb53..ece8f1442 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.data
+++ b/tests/suites/test_suite_psa_crypto_slot_management.data
@@ -107,10 +107,15 @@ Open failure: non-existent identifier
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:1:PSA_ERROR_DOES_NOT_EXIST
 
-Create failure: invalid lifetime
-create_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT
+Create failure: invalid lifetime for a persistent key
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:0x7fffffff:1:PSA_ERROR_INVALID_ARGUMENT
 
-Create failure: invalid key id (0)
+Create failure: invalid lifetime for a volatile key
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:0x7fffff00:0:PSA_ERROR_INVALID_ARGUMENT
+
+Create failure: invalid key id (0) for a persistent key
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 create_fail:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_ERROR_INVALID_HANDLE