From 398aee57421706ffdb7c90d9d4397a0f1adc1bd0 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Tue, 13 Oct 2020 14:35:45 +0200
Subject: [PATCH 1/7] Rework psa_copy_key_material

There's no need for calling export-and-import when the key is
guaranteed to have been stored in export representation.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index ddb03fc32..5a5dc8d4e 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2179,26 +2179,16 @@ exit:
 static psa_status_t psa_copy_key_material( const psa_key_slot_t *source,
                                            psa_key_slot_t *target )
 {
-    psa_status_t status;
-    uint8_t *buffer = NULL;
-    size_t buffer_size = 0;
-    size_t length;
-
-    buffer_size = PSA_KEY_EXPORT_MAX_SIZE( source->attr.type,
-                                           psa_get_key_slot_bits( source ) );
-    buffer = mbedtls_calloc( 1, buffer_size );
-    if( buffer == NULL )
-        return( PSA_ERROR_INSUFFICIENT_MEMORY );
-    status = psa_internal_export_key( source, buffer, buffer_size, &length, 0 );
+    psa_status_t status = psa_allocate_buffer_to_slot( target,
+                                                       source->data.key.bytes );
     if( status != PSA_SUCCESS )
-        goto exit;
-    target->attr.type = source->attr.type;
-    status = psa_import_key_into_slot( target, buffer, length );
+        return( status );
 
-exit:
-    mbedtls_platform_zeroize( buffer, buffer_size );
-    mbedtls_free( buffer );
-    return( status );
+    memcpy( target->data.key.data, source->data.key.data, source->data.key.bytes );
+    target->attr.type = source->attr.type;
+    target->attr.bits = source->attr.bits;
+
+    return( PSA_SUCCESS );
 }
 
 psa_status_t psa_copy_key( psa_key_handle_t source_handle,

From 0452476eacbc4989a337942b697194dd77b8dbd7 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Tue, 13 Oct 2020 17:43:44 +0200
Subject: [PATCH 2/7] Implement, plug in and test validate_key driver entry
 point

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c                          |   3 +-
 library/psa_crypto_driver_wrappers.c          |  28 +++++
 library/psa_crypto_driver_wrappers.h          |   9 ++
 tests/include/test/drivers/keygen.h           |   7 +-
 tests/src/drivers/keygen.c                    | 112 +++++++++++++++++-
 ...test_suite_psa_crypto_driver_wrappers.data |  19 +++
 ..._suite_psa_crypto_driver_wrappers.function |  34 ++++++
 7 files changed, 208 insertions(+), 4 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 5a5dc8d4e..ddb2faa3c 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -977,6 +977,7 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
                                        size_t data_length )
 {
     psa_status_t status = PSA_SUCCESS;
+    size_t bit_size;
 
     /* zero-length keys are never supported. */
     if( data_length == 0 )
@@ -984,7 +985,7 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
 
     if( key_type_is_raw_bytes( slot->attr.type ) )
     {
-        size_t bit_size = PSA_BYTES_TO_BITS( data_length );
+        bit_size = PSA_BYTES_TO_BITS( data_length );
 
         /* Ensure that the bytes-to-bits conversion hasn't overflown. */
         if( data_length > SIZE_MAX / 8 )
diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c
index f19f55920..2bda2a6cb 100644
--- a/library/psa_crypto_driver_wrappers.c
+++ b/library/psa_crypto_driver_wrappers.c
@@ -410,6 +410,34 @@ psa_status_t psa_driver_wrapper_generate_key( const psa_key_attributes_t *attrib
 #endif /* PSA_CRYPTO_DRIVER_PRESENT */
 }
 
+psa_status_t psa_driver_wrapper_validate_key( const psa_key_attributes_t *attributes,
+                                              const uint8_t *data,
+                                              size_t data_length,
+                                              size_t *bits )
+{
+#if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT)
+    psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+    /* Try accelerators in turn */
+#if defined(PSA_CRYPTO_DRIVER_TEST)
+    status = test_transparent_validate_key( attributes,
+                                            data,
+                                            data_length,
+                                            bits );
+    /* Declared with fallback == true */
+    if( status != PSA_ERROR_NOT_SUPPORTED )
+        return( status );
+#endif /* PSA_CRYPTO_DRIVER_TEST */
+
+    return( PSA_ERROR_NOT_SUPPORTED );
+#else /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */
+    (void) attributes;
+    (void) data;
+    (void) data_length;
+    (void) bits;
+    return( PSA_ERROR_NOT_SUPPORTED );
+#endif /* PSA_CRYPTO_DRIVER_PRESENT */
+}
+
 /*
  * Cipher functions
  */
diff --git a/library/psa_crypto_driver_wrappers.h b/library/psa_crypto_driver_wrappers.h
index 0db15d6c3..b0b483bb5 100644
--- a/library/psa_crypto_driver_wrappers.h
+++ b/library/psa_crypto_driver_wrappers.h
@@ -43,9 +43,18 @@ psa_status_t psa_driver_wrapper_verify_hash( psa_key_slot_t *slot,
                                              const uint8_t *signature,
                                              size_t signature_length );
 
+/*
+ * Key handling functions
+ */
+
 psa_status_t psa_driver_wrapper_generate_key( const psa_key_attributes_t *attributes,
                                               psa_key_slot_t *slot );
 
+psa_status_t psa_driver_wrapper_validate_key( const psa_key_attributes_t *attributes,
+                                              const uint8_t *data,
+                                              size_t data_length,
+                                              size_t *bits );
+
 /*
  * Cipher functions
  */
diff --git a/tests/include/test/drivers/keygen.h b/tests/include/test/drivers/keygen.h
index b72c65c78..e5a5e4700 100644
--- a/tests/include/test/drivers/keygen.h
+++ b/tests/include/test/drivers/keygen.h
@@ -1,5 +1,5 @@
 /*
- * Test driver for generating keys.
+ * Test driver for generating and verifying keys.
  */
 /*  Copyright The Mbed TLS Contributors
  *  SPDX-License-Identifier: Apache-2.0
@@ -57,5 +57,10 @@ psa_status_t test_opaque_generate_key(
     const psa_key_attributes_t *attributes,
     uint8_t *key, size_t key_size, size_t *key_length );
 
+psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attributes,
+                                           const uint8_t *data,
+                                           size_t data_length,
+                                           size_t *bits);
+
 #endif /* PSA_CRYPTO_DRIVER_TEST */
 #endif /* PSA_CRYPTO_TEST_DRIVERS_KEYGEN_H */
diff --git a/tests/src/drivers/keygen.c b/tests/src/drivers/keygen.c
index f15a4bc9a..84fc98a37 100644
--- a/tests/src/drivers/keygen.c
+++ b/tests/src/drivers/keygen.c
@@ -1,6 +1,6 @@
 /*
- * Test driver for generating keys.
- * Currently only supports generating ECC keys.
+ * Test driver for generating and verifying keys.
+ * Currently only supports generating and verifying ECC keys.
  */
 /*  Copyright The Mbed TLS Contributors
  *  SPDX-License-Identifier: Apache-2.0
@@ -122,4 +122,112 @@ psa_status_t test_opaque_generate_key(
     return( PSA_ERROR_NOT_SUPPORTED );
 }
 
+psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attributes,
+                                           const uint8_t *data,
+                                           size_t data_length,
+                                           size_t *bits)
+{
+    ++test_driver_keygen_hooks.hits;
+
+    if( test_driver_keygen_hooks.forced_status != PSA_SUCCESS )
+        return( test_driver_keygen_hooks.forced_status );
+
+#if defined(MBEDTLS_ECP_C)
+    psa_key_type_t type = psa_get_key_type( attributes );
+    if ( PSA_KEY_TYPE_IS_ECC( type ) )
+    {
+        // Code mostly copied from psa_load_ecp_representation
+        psa_ecc_family_t curve = PSA_KEY_TYPE_ECC_GET_FAMILY( type );
+        mbedtls_ecp_group_id grp_id;
+        mbedtls_ecp_keypair ecp;
+        psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+
+        if( *bits == 0 )
+        {
+            // Attempt auto-detect of curve bit size
+            size_t curve_size = data_length;
+
+            if( PSA_KEY_TYPE_IS_PUBLIC_KEY( type ) &&
+                PSA_KEY_TYPE_ECC_GET_FAMILY( type ) != PSA_ECC_FAMILY_MONTGOMERY )
+            {
+                /* A Weierstrass public key is represented as:
+                 * - The byte 0x04;
+                 * - `x_P` as a `ceiling(m/8)`-byte string, big-endian;
+                 * - `y_P` as a `ceiling(m/8)`-byte string, big-endian.
+                 * So its data length is 2m+1 where n is the key size in bits.
+                 */
+                if( ( data_length & 1 ) == 0 )
+                    return( PSA_ERROR_INVALID_ARGUMENT );
+                curve_size = data_length / 2;
+
+                /* Montgomery public keys are represented in compressed format, meaning
+                 * their curve_size is equal to the amount of input. */
+
+                /* Private keys are represented in uncompressed private random integer
+                 * format, meaning their curve_size is equal to the amount of input. */
+            }
+
+            grp_id = mbedtls_ecc_group_of_psa( curve, curve_size );
+        }
+        else
+        {
+            grp_id = mbedtls_ecc_group_of_psa( curve,
+                PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ) );
+        }
+
+        const mbedtls_ecp_curve_info *curve_info =
+            mbedtls_ecp_curve_info_from_grp_id( grp_id );
+
+        if( attributes->domain_parameters_size != 0 )
+            return( PSA_ERROR_NOT_SUPPORTED );
+        if( grp_id == MBEDTLS_ECP_DP_NONE || curve_info == NULL )
+            return( PSA_ERROR_NOT_SUPPORTED );
+
+        *bits = curve_info->bit_size;
+
+        mbedtls_ecp_keypair_init( &ecp );
+
+        status = mbedtls_to_psa_error(
+                    mbedtls_ecp_group_load( &ecp.grp, grp_id ) );
+        if( status != PSA_SUCCESS )
+            goto ecp_exit;
+
+        /* Load the key material. */
+        if( PSA_KEY_TYPE_IS_PUBLIC_KEY( type ) )
+        {
+            /* Load the public value. */
+            status = mbedtls_to_psa_error(
+                mbedtls_ecp_point_read_binary( &ecp.grp, &ecp.Q,
+                                               data,
+                                               data_length ) );
+            if( status != PSA_SUCCESS )
+                goto ecp_exit;
+
+            /* Check that the point is on the curve. */
+            status = mbedtls_to_psa_error(
+                mbedtls_ecp_check_pubkey( &ecp.grp, &ecp.Q ) );
+        }
+        else
+        {
+            /* Load and validate the secret value. */
+            status = mbedtls_to_psa_error(
+                mbedtls_ecp_read_key( ecp.grp.id,
+                                      &ecp,
+                                      data,
+                                      data_length ) );
+        }
+
+ecp_exit:
+        mbedtls_ecp_keypair_free( &ecp );
+        return( status );
+    }
+    return( PSA_ERROR_NOT_SUPPORTED );
+#else
+    (void) data;
+    (void) data_length;
+    (void) bits;
+    return( PSA_ERROR_NOT_SUPPORTED );
+#endif /* MBEDTLS_ECP_C */
+}
+
 #endif /* MBEDTLS_PSA_CRYPTO_DRIVERS && PSA_CRYPTO_DRIVER_TEST */
diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.data b/tests/suites/test_suite_psa_crypto_driver_wrappers.data
index 7abc25692..1f1ee39cd 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.data
+++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.data
@@ -40,6 +40,25 @@ generate_key:PSA_ERROR_NOT_SUPPORTED:"":PSA_SUCCESS
 generate_key through transparent driver: error
 generate_key:PSA_ERROR_GENERIC_ERROR:"":PSA_ERROR_GENERIC_ERROR
 
+validate key through transparent driver: good private key
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+validate_key:PSA_SUCCESS:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":PSA_SUCCESS
+
+validate key through transparent driver: good public key
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+validate_key:PSA_SUCCESS:PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_FAMILY_SECP_R1):"04dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320":PSA_SUCCESS
+
+validate key through transparent driver: fallback private key
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+validate_key:PSA_ERROR_NOT_SUPPORTED:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":PSA_SUCCESS
+
+validate key through transparent driver: fallback public key
+depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+validate_key:PSA_ERROR_NOT_SUPPORTED:PSA_KEY_TYPE_ECC_PUBLIC_KEY(PSA_ECC_FAMILY_SECP_R1):"04dea5e45d0ea37fc566232a508f4ad20ea13d47e4bf5fa4d54a57a0ba012042087097496efc583fed8b24a5b9be9a51de063f5a00a8b698a16fd7f29b5485f320":PSA_SUCCESS
+
+validate key through transparent driver: error
+validate_key:PSA_ERROR_GENERIC_ERROR:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":PSA_ERROR_GENERIC_ERROR
+
 PSA symmetric encrypt: AES-CTR, 16 bytes, good
 depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR
 cipher_encrypt:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee22e409f96e93d7e117393172a":"8f9408fe80a81d3e813da3c7b0b2bd32":0:PSA_SUCCESS:PSA_SUCCESS
diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
index 951670d56..3cecbfc67 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function
+++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
@@ -184,6 +184,40 @@ exit:
 }
 /* END_CASE */
 
+/* BEGIN_CASE depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED */
+void validate_key( int force_status_arg,
+                   int key_type_arg,
+                   data_t *key_input,
+                   int expected_status_arg )
+{
+    psa_status_t force_status = force_status_arg;
+    psa_status_t expected_status = expected_status_arg;
+    psa_key_type_t key_type = key_type_arg;
+    psa_key_handle_t handle = 0;
+    psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
+    psa_status_t actual_status;
+    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+
+    psa_set_key_type( &attributes,
+                      key_type );
+    psa_set_key_bits( &attributes, 0 );
+    psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT );
+
+    test_driver_keygen_hooks.forced_status = force_status;
+
+    PSA_ASSERT( psa_crypto_init( ) );
+
+    actual_status = psa_import_key( &attributes, key_input->x, key_input->len, &handle );
+    TEST_EQUAL( test_driver_keygen_hooks.hits, 1 );
+    TEST_EQUAL( actual_status, expected_status );
+exit:
+    psa_reset_key_attributes( &attributes );
+    psa_destroy_key( handle );
+    PSA_DONE( );
+    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+}
+/* END_CASE */
+
 /* BEGIN_CASE */
 void cipher_encrypt( int alg_arg, int key_type_arg,
                      data_t *key, data_t *iv,

From f7cebd4a2b636059e97061aa18d095a2731b23a3 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Tue, 13 Oct 2020 20:27:40 +0200
Subject: [PATCH 3/7] Add internal helper function to load prevalidated key
 material

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c      | 24 +++++++++++++++++-------
 library/psa_crypto_core.h | 21 +++++++++++++++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index ddb2faa3c..afe09af02 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -969,6 +969,19 @@ static psa_status_t psa_allocate_buffer_to_slot( psa_key_slot_t *slot,
     return( PSA_SUCCESS );
 }
 
+psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
+                                              const uint8_t* data,
+                                              size_t data_length )
+{
+    psa_status_t status = psa_allocate_buffer_to_slot( slot,
+                                                       data_length );
+    if( status != PSA_SUCCESS )
+        return( status );
+
+    memcpy( slot->data.key.data, data, data_length );
+    return( PSA_SUCCESS );
+}
+
 /** Import key data into a slot. `slot->attr.type` must have been set
  * previously. This function assumes that the slot does not contain
  * any key material yet. On failure, the slot content is unchanged. */
@@ -1001,13 +1014,10 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
             return( status );
 
         /* Allocate memory for the key */
-        status = psa_allocate_buffer_to_slot( slot, data_length );
+        status = psa_copy_key_material_into_slot( slot, data, data_length );
         if( status != PSA_SUCCESS )
             return( status );
 
-        /* copy key into allocated buffer */
-        memcpy( slot->data.key.data, data, data_length );
-
         /* Write the actual key size to the slot.
          * psa_start_key_creation() wrote the size declared by the
          * caller, which may be 0 (meaning unspecified) or wrong. */
@@ -2180,12 +2190,12 @@ exit:
 static psa_status_t psa_copy_key_material( const psa_key_slot_t *source,
                                            psa_key_slot_t *target )
 {
-    psa_status_t status = psa_allocate_buffer_to_slot( target,
-                                                       source->data.key.bytes );
+    psa_status_t status = psa_copy_key_material_into_slot( target,
+                                                           source->data.key.data,
+                                                           source->data.key.bytes );
     if( status != PSA_SUCCESS )
         return( status );
 
-    memcpy( target->data.key.data, source->data.key.data, source->data.key.bytes );
     target->attr.type = source->attr.type;
     target->attr.bits = source->attr.bits;
 
diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h
index 6ee17fce0..4943eb1a7 100644
--- a/library/psa_crypto_core.h
+++ b/library/psa_crypto_core.h
@@ -161,6 +161,27 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
                                        const uint8_t *data,
                                        size_t data_length );
 
+/** Copy key data (in export format) into an empty key slot.
+ *
+ * This function assumes that the slot does not contain
+ * any key material yet. On failure, the slot content is unchanged.
+ *
+ * \param[in,out] slot          Key slot to copy the key into.
+ * \param[in] data              Buffer containing the key material.
+ * \param data_length           Size of the key buffer.
+ *
+ * \retval #PSA_SUCCESS
+ *         The key has been copied successfully.
+ * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
+ *         Not enough memory was available for allocation of the
+ *         copy buffer.
+ * \retval #PSA_ERROR_ALREADY_EXISTS
+ *         There was other key material already present in the slot.
+ */
+psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
+                                              const uint8_t *data,
+                                              size_t data_length );
+
 
 /** Convert an mbed TLS error code to a PSA error code
  *

From 3ea0ce450ff1cf72396b0e0fae64255a9f071a09 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Fri, 23 Oct 2020 11:37:05 +0200
Subject: [PATCH 4/7] Separate 'import' from 'load into slot'

Now that there's a validate_key entry point for drivers, it becomes
much more important to separate the import action (where a key needs
to be validated) from the load action (where a key has been
previously validated, and thus re-validating it would be a waste of
time).

This also exposes why not storing the 'bits' attribute persistently
was a bad idea. The only reason there's a rather large function to
detect bit size is because loading from persistent storage requires
it.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c                 | 262 ++++++++++++++++++++++++---
 library/psa_crypto_core.h            |  42 ++---
 library/psa_crypto_slot_management.c |   6 +-
 3 files changed, 263 insertions(+), 47 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index afe09af02..1901281c5 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -739,7 +739,7 @@ static psa_status_t psa_load_ecp_representation( psa_key_type_t type,
          * - The byte 0x04;
          * - `x_P` as a `ceiling(m/8)`-byte string, big-endian;
          * - `y_P` as a `ceiling(m/8)`-byte string, big-endian.
-         * So its data length is 2m+1 where n is the key size in bits.
+         * So its data length is 2m+1 where m is the curve size in bits.
          */
         if( ( data_length & 1 ) == 0 )
             return( PSA_ERROR_INVALID_ARGUMENT );
@@ -982,12 +982,197 @@ psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
     return( PSA_SUCCESS );
 }
 
-/** Import key data into a slot. `slot->attr.type` must have been set
- * previously. This function assumes that the slot does not contain
- * any key material yet. On failure, the slot content is unchanged. */
-psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
-                                       const uint8_t *data,
-                                       size_t data_length )
+psa_status_t psa_detect_bit_size_in_slot( psa_key_slot_t *slot )
+{
+    if( slot->attr.bits != 0 )
+        return( PSA_SUCCESS );
+
+    if( key_type_is_raw_bytes( slot->attr.type ) )
+    {
+        slot->attr.bits =
+            (psa_key_bits_t) PSA_BYTES_TO_BITS( slot->data.key.bytes );
+        return( PSA_SUCCESS );
+    }
+    else if( PSA_KEY_TYPE_IS_ECC( slot->attr.type ) )
+    {
+        /* Keys are stored in export format, and we are currently
+         * restricted to known curves, so do the reverse lookup based
+         * on data length. */
+        size_t byte_length = slot->data.key.bytes;
+        if( PSA_KEY_TYPE_IS_PUBLIC_KEY( slot->attr.type ) &&
+            PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type ) !=
+                PSA_ECC_FAMILY_MONTGOMERY )
+        {
+            /* A Weierstrass public key is represented as:
+             * - The byte 0x04;
+             * - `x_P` as a `ceiling(m/8)`-byte string, big-endian;
+             * - `y_P` as a `ceiling(m/8)`-byte string, big-endian.
+             * So its data length is 2m+1 where m is the curve size in bits.
+             */
+            if( ( byte_length & 1 ) == 0 )
+                return( PSA_ERROR_BAD_STATE );
+            byte_length = byte_length / 2;
+
+            /* Montgomery public keys are represented in compressed format,
+             * meaning their curve_size is equal to the amount of input. */
+
+            /* Private keys are represented in uncompressed private random
+             * integer format, meaning their curve_size is equal to the
+             * amount of input. */
+        }
+
+        switch( PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type ) )
+        {
+            case PSA_ECC_FAMILY_SECP_R1:
+                switch( byte_length )
+                {
+                    case PSA_BITS_TO_BYTES( 192 ):
+                        slot->attr.bits = 192;
+                        break;
+                    case PSA_BITS_TO_BYTES( 224 ):
+                        slot->attr.bits = 224;
+                        break;
+                    case PSA_BITS_TO_BYTES( 256 ):
+                        slot->attr.bits = 256;
+                        break;
+                    case PSA_BITS_TO_BYTES( 384 ):
+                        slot->attr.bits = 384;
+                        break;
+                    case PSA_BITS_TO_BYTES( 521 ):
+                        slot->attr.bits = 521;
+                        break;
+                    default:
+                        return( PSA_ERROR_BAD_STATE );
+                }
+                break;
+
+            case PSA_ECC_FAMILY_BRAINPOOL_P_R1:
+                switch( byte_length )
+                {
+                    case PSA_BITS_TO_BYTES( 256 ):
+                        slot->attr.bits = 256;
+                        break;
+                    case PSA_BITS_TO_BYTES( 384 ):
+                        slot->attr.bits = 384;
+                        break;
+                    case PSA_BITS_TO_BYTES( 512 ):
+                        slot->attr.bits = 512;
+                        break;
+                    default:
+                        return( PSA_ERROR_BAD_STATE );
+                }
+                break;
+
+            case PSA_ECC_FAMILY_MONTGOMERY:
+                switch( byte_length )
+                {
+                    case PSA_BITS_TO_BYTES( 255 ):
+                        slot->attr.bits = 255;
+                        break;
+                    case PSA_BITS_TO_BYTES( 448 ):
+                        slot->attr.bits = 448;
+                        break;
+                    default:
+                        return( PSA_ERROR_BAD_STATE );
+                }
+                break;
+
+            case PSA_ECC_FAMILY_SECP_K1:
+                switch( byte_length )
+                {
+                    case PSA_BITS_TO_BYTES( 192 ):
+                        slot->attr.bits = 192;
+                        break;
+                    case PSA_BITS_TO_BYTES( 224 ):
+                        slot->attr.bits = 224;
+                        break;
+                    case PSA_BITS_TO_BYTES( 256 ):
+                        slot->attr.bits = 256;
+                        break;
+                    default:
+                        return( PSA_ERROR_BAD_STATE );
+                }
+                break;
+
+            default:
+                return( PSA_ERROR_BAD_STATE );
+        }
+
+        return( PSA_SUCCESS );
+    }
+    else if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
+    {
+        /* There's no easy way of figuring out the RSA bit size from
+         * the data length of the export representation. For now, use
+         * the mbed TLS software implementation to figure it out. */
+        psa_key_attributes_t attributes = {
+            .core = slot->attr
+        };
+        size_t bits;
+        psa_status_t status = psa_driver_wrapper_validate_key(
+                                &attributes,
+                                slot->data.key.data,
+                                slot->data.key.bytes,
+                                &bits );
+        if( status == PSA_SUCCESS )
+            slot->attr.bits = (psa_key_bits_t) bits;
+        if( status != PSA_ERROR_NOT_SUPPORTED )
+            return( status );
+
+        /* If no accelerator was able to figure it out, try software. */
+#if defined(MBEDTLS_RSA_C)
+        mbedtls_rsa_context *rsa = NULL;
+
+        /* Parse input */
+        status = psa_load_rsa_representation( slot->attr.type,
+                                              slot->data.key.data,
+                                              slot->data.key.bytes,
+                                              &rsa );
+        if( status != PSA_SUCCESS )
+        {
+            mbedtls_rsa_free( rsa );
+            mbedtls_free( rsa );
+            return( status );
+        }
+
+        slot->attr.bits = (psa_key_bits_t) PSA_BYTES_TO_BITS(
+            mbedtls_rsa_get_len( rsa ) );
+
+        mbedtls_rsa_free( rsa );
+        mbedtls_free( rsa );
+
+        return( PSA_SUCCESS );
+#else
+        return( PSA_ERROR_NOT_SUPPORTED );
+#endif
+    }
+    else
+        return( PSA_ERROR_NOT_SUPPORTED );
+}
+
+/** Import key data into a slot.
+ *
+ * `slot->type` must have been set previously.
+ * This function assumes that the slot does not contain any key material yet.
+ * On failure, the slot content is unchanged.
+ *
+ * Persistent storage is not affected.
+ *
+ * \param[in,out] slot  The key slot to import data into.
+ *                      Its `type` field must have previously been set to
+ *                      the desired key type.
+ *                      It must not contain any key material yet.
+ * \param[in] data      Buffer containing the key material to parse and import.
+ * \param data_length   Size of \p data in bytes.
+ *
+ * \retval #PSA_SUCCESS
+ * \retval #PSA_ERROR_INVALID_ARGUMENT
+ * \retval #PSA_ERROR_NOT_SUPPORTED
+ * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
+ */
+static psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
+                                              const uint8_t *data,
+                                              size_t data_length )
 {
     psa_status_t status = PSA_SUCCESS;
     size_t bit_size;
@@ -1023,32 +1208,65 @@ psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
          * caller, which may be 0 (meaning unspecified) or wrong. */
         slot->attr.bits = (psa_key_bits_t) bit_size;
     }
-    else if( PSA_KEY_TYPE_IS_ECC( slot->attr.type ) )
+    else if( PSA_KEY_TYPE_IS_ASYMMETRIC( slot->attr.type ) )
     {
+        /* Try validation through accelerators first. */
+        bit_size = slot->attr.bits;
+        psa_key_attributes_t attributes = {
+          .core = slot->attr
+        };
+        status = psa_driver_wrapper_validate_key( &attributes,
+                                                  data,
+                                                  data_length,
+                                                  &bit_size );
+        if( status == PSA_SUCCESS )
+        {
+            /* Key has been validated successfully by an accelerator.
+             * Copy key material into slot. */
+            status = psa_copy_key_material_into_slot( slot, data, data_length );
+            if( status != PSA_SUCCESS )
+                return( status );
+
+            slot->attr.bits = (psa_key_bits_t) bit_size;
+            return( PSA_SUCCESS );
+        }
+        else if( status != PSA_ERROR_NOT_SUPPORTED )
+            return( status );
+
+        /* Key format is not supported by any accelerator, try software fallback
+         * if present. */
+        if( PSA_KEY_TYPE_IS_ECC( slot->attr.type ) )
+        {
 #if defined(MBEDTLS_ECP_C)
-        status = psa_import_ecp_key( slot,
-                                     data, data_length );
+            status = psa_import_ecp_key( slot,
+                                         data, data_length );
 #else
-        /* No drivers have been implemented yet, so without mbed TLS backing
-         * there's no way to do ECP with the current library. */
-        return( PSA_ERROR_NOT_SUPPORTED );
+            /* No drivers have been implemented yet, so without mbed TLS backing
+             * there's no way to do ECP with the current library. */
+            status = PSA_ERROR_NOT_SUPPORTED;
 #endif /* defined(MBEDTLS_ECP_C) */
-    }
-    else if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
-    {
+        }
+        else if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
+        {
 #if defined(MBEDTLS_RSA_C)
-        status = psa_import_rsa_key( slot,
-                                     data, data_length );
+            status = psa_import_rsa_key( slot,
+                                         data, data_length );
 #else
-        /* No drivers have been implemented yet, so without mbed TLS backing
-         * there's no way to do RSA with the current library. */
-        status = PSA_ERROR_NOT_SUPPORTED;
+            /* No drivers have been implemented yet, so without mbed TLS backing
+             * there's no way to do RSA with the current library. */
+            status = PSA_ERROR_NOT_SUPPORTED;
 #endif /* defined(MBEDTLS_RSA_C) */
+        }
+        else
+        {
+            /* Unsupported asymmetric key type */
+            status = PSA_ERROR_NOT_SUPPORTED;
+        }
     }
     else
     {
         /* Unknown key type */
-        return( PSA_ERROR_NOT_SUPPORTED );
+        status = PSA_ERROR_NOT_SUPPORTED;
     }
 
     return( status );
diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h
index 4943eb1a7..2786b7993 100644
--- a/library/psa_crypto_core.h
+++ b/library/psa_crypto_core.h
@@ -137,30 +137,6 @@ static inline void psa_key_slot_clear_bits( psa_key_slot_t *slot,
  */
 psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot );
 
-/** Import key data into a slot.
- *
- * `slot->type` must have been set previously.
- * This function assumes that the slot does not contain any key material yet.
- * On failure, the slot content is unchanged.
- *
- * Persistent storage is not affected.
- *
- * \param[in,out] slot  The key slot to import data into.
- *                      Its `type` field must have previously been set to
- *                      the desired key type.
- *                      It must not contain any key material yet.
- * \param[in] data      Buffer containing the key material to parse and import.
- * \param data_length   Size of \p data in bytes.
- *
- * \retval PSA_SUCCESS
- * \retval PSA_ERROR_INVALID_ARGUMENT
- * \retval PSA_ERROR_NOT_SUPPORTED
- * \retval PSA_ERROR_INSUFFICIENT_MEMORY
- */
-psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
-                                       const uint8_t *data,
-                                       size_t data_length );
-
 /** Copy key data (in export format) into an empty key slot.
  *
  * This function assumes that the slot does not contain
@@ -182,6 +158,24 @@ psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
                                               const uint8_t *data,
                                               size_t data_length );
 
+/** Detect the key bit size for a key in a slot where bit size
+ *  is unset.
+ *
+ * This function assumes that the slot contains key material in
+ * export format.
+ *
+ * \param[in,out] slot  Key slot to detect and set the bit size in.
+ *
+ * \retval #PSA_SUCCESS
+ *         The key bit size was already set, or has been detected
+ *         and set accordingly.
+ * \retval #PSA_ERROR_BAD_STATE
+ *         The size of the key material in the slot doesn't match
+ *         with the declared key type.
+ * \retval #PSA_ERROR_NOT_SUPPORTED
+ *         The key type is unknown to the implementation.
+ */
+psa_status_t psa_detect_bit_size_in_slot( psa_key_slot_t *slot );
 
 /** Convert an mbed TLS error code to a PSA error code
  *
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index b7a3c1338..f33c4f2ef 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -141,7 +141,11 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *slot )
     else
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     {
-        status = psa_import_key_into_slot( slot, key_data, key_data_length );
+        status = psa_copy_key_material_into_slot( slot, key_data, key_data_length );
+        if( status != PSA_SUCCESS )
+            goto exit;
+
+        status = psa_detect_bit_size_in_slot( slot );
     }
 
 exit:

From c4813a6e809a38d42db12804f23585518b725d88 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Fri, 23 Oct 2020 11:45:43 +0200
Subject: [PATCH 5/7] Rename 'keygen' to 'key management'

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 .../drivers/{keygen.h => key_management.h}    | 19 +++++-----
 tests/include/test/drivers/signature.h        |  2 +-
 tests/include/test/drivers/test_driver.h      |  2 +-
 .../drivers/{keygen.c => key_management.c}    | 36 ++++++++++---------
 ..._suite_psa_crypto_driver_wrappers.function | 22 ++++++------
 visualc/VS2010/mbedTLS.vcxproj                |  2 +-
 6 files changed, 44 insertions(+), 39 deletions(-)
 rename tests/include/test/drivers/{keygen.h => key_management.h} (74%)
 rename tests/src/drivers/{keygen.c => key_management.c} (85%)

diff --git a/tests/include/test/drivers/keygen.h b/tests/include/test/drivers/key_management.h
similarity index 74%
rename from tests/include/test/drivers/keygen.h
rename to tests/include/test/drivers/key_management.h
index e5a5e4700..56f3ef82e 100644
--- a/tests/include/test/drivers/keygen.h
+++ b/tests/include/test/drivers/key_management.h
@@ -17,8 +17,8 @@
  *  limitations under the License.
  */
 
-#ifndef PSA_CRYPTO_TEST_DRIVERS_KEYGEN_H
-#define PSA_CRYPTO_TEST_DRIVERS_KEYGEN_H
+#ifndef PSA_CRYPTO_TEST_DRIVERS_KEY_MANAGEMENT_H
+#define PSA_CRYPTO_TEST_DRIVERS_KEY_MANAGEMENT_H
 
 #if !defined(MBEDTLS_CONFIG_FILE)
 #include "mbedtls/config.h"
@@ -36,18 +36,19 @@ typedef struct {
     /* If not PSA_SUCCESS, return this error code instead of processing the
      * function call. */
     psa_status_t forced_status;
-    /* Count the amount of times one of the keygen driver functions is called. */
+    /* Count the amount of times one of the key management driver functions
+     * is called. */
     unsigned long hits;
-} test_driver_keygen_hooks_t;
+} test_driver_key_management_hooks_t;
 
-#define TEST_DRIVER_KEYGEN_INIT { NULL, 0, PSA_ERROR_NOT_SUPPORTED, 0 }
-static inline test_driver_keygen_hooks_t test_driver_keygen_hooks_init( void )
+#define TEST_DRIVER_KEY_MANAGEMENT_INIT { NULL, 0, PSA_ERROR_NOT_SUPPORTED, 0 }
+static inline test_driver_key_management_hooks_t test_driver_key_management_hooks_init( void )
 {
-    const test_driver_keygen_hooks_t v = TEST_DRIVER_KEYGEN_INIT;
+    const test_driver_key_management_hooks_t v = TEST_DRIVER_KEY_MANAGEMENT_INIT;
     return( v );
 }
 
-extern test_driver_keygen_hooks_t test_driver_keygen_hooks;
+extern test_driver_key_management_hooks_t test_driver_key_management_hooks;
 
 psa_status_t test_transparent_generate_key(
     const psa_key_attributes_t *attributes,
@@ -63,4 +64,4 @@ psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attribute
                                            size_t *bits);
 
 #endif /* PSA_CRYPTO_DRIVER_TEST */
-#endif /* PSA_CRYPTO_TEST_DRIVERS_KEYGEN_H */
+#endif /* PSA_CRYPTO_TEST_DRIVERS_KEY_MANAGEMENT_H */
diff --git a/tests/include/test/drivers/signature.h b/tests/include/test/drivers/signature.h
index e41892e77..8abcb111a 100644
--- a/tests/include/test/drivers/signature.h
+++ b/tests/include/test/drivers/signature.h
@@ -36,7 +36,7 @@ typedef struct {
     /* If not PSA_SUCCESS, return this error code instead of processing the
      * function call. */
     psa_status_t forced_status;
-    /* Count the amount of times one of the keygen driver functions is called. */
+    /* Count the amount of times one of the signature driver functions is called. */
     unsigned long hits;
 } test_driver_signature_hooks_t;
 
diff --git a/tests/include/test/drivers/test_driver.h b/tests/include/test/drivers/test_driver.h
index ee5974217..f26b795dd 100644
--- a/tests/include/test/drivers/test_driver.h
+++ b/tests/include/test/drivers/test_driver.h
@@ -23,7 +23,7 @@
 #define PSA_CRYPTO_TEST_DRIVER_LIFETIME 0x7fffff
 
 #include "test/drivers/signature.h"
-#include "test/drivers/keygen.h"
+#include "test/drivers/key_management.h"
 #include "test/drivers/cipher.h"
 #include "test/drivers/size.h"
 
diff --git a/tests/src/drivers/keygen.c b/tests/src/drivers/key_management.c
similarity index 85%
rename from tests/src/drivers/keygen.c
rename to tests/src/drivers/key_management.c
index 84fc98a37..6ca03c6be 100644
--- a/tests/src/drivers/keygen.c
+++ b/tests/src/drivers/key_management.c
@@ -30,30 +30,31 @@
 #include "mbedtls/ecp.h"
 #include "mbedtls/error.h"
 
-#include "test/drivers/keygen.h"
+#include "test/drivers/key_management.h"
 
 #include "test/random.h"
 
 #include <string.h>
 
-test_driver_keygen_hooks_t test_driver_keygen_hooks = TEST_DRIVER_KEYGEN_INIT;
+test_driver_key_management_hooks_t test_driver_key_management_hooks =
+    TEST_DRIVER_KEY_MANAGEMENT_INIT;
 
 psa_status_t test_transparent_generate_key(
     const psa_key_attributes_t *attributes,
     uint8_t *key, size_t key_size, size_t *key_length )
 {
-    ++test_driver_keygen_hooks.hits;
+    ++test_driver_key_management_hooks.hits;
 
-    if( test_driver_keygen_hooks.forced_status != PSA_SUCCESS )
-        return( test_driver_keygen_hooks.forced_status );
+    if( test_driver_key_management_hooks.forced_status != PSA_SUCCESS )
+        return( test_driver_key_management_hooks.forced_status );
 
-    if( test_driver_keygen_hooks.forced_output != NULL )
+    if( test_driver_key_management_hooks.forced_output != NULL )
     {
-        if( test_driver_keygen_hooks.forced_output_length > key_size )
+        if( test_driver_key_management_hooks.forced_output_length > key_size )
             return( PSA_ERROR_BUFFER_TOO_SMALL );
-        memcpy( key, test_driver_keygen_hooks.forced_output,
-                test_driver_keygen_hooks.forced_output_length );
-        *key_length = test_driver_keygen_hooks.forced_output_length;
+        memcpy( key, test_driver_key_management_hooks.forced_output,
+                test_driver_key_management_hooks.forced_output_length );
+        *key_length = test_driver_key_management_hooks.forced_output_length;
         return( PSA_SUCCESS );
     }
 
@@ -62,9 +63,12 @@ psa_status_t test_transparent_generate_key(
     if ( PSA_KEY_TYPE_IS_ECC( psa_get_key_type( attributes ) )
          && PSA_KEY_TYPE_IS_KEY_PAIR( psa_get_key_type( attributes ) ) )
     {
-        psa_ecc_family_t curve = PSA_KEY_TYPE_ECC_GET_FAMILY( psa_get_key_type( attributes ) );
+        psa_ecc_family_t curve = PSA_KEY_TYPE_ECC_GET_FAMILY(
+            psa_get_key_type( attributes ) );
         mbedtls_ecp_group_id grp_id =
-            mbedtls_ecc_group_of_psa( curve, PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ) );
+            mbedtls_ecc_group_of_psa(
+                curve,
+                PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ) );
         const mbedtls_ecp_curve_info *curve_info =
             mbedtls_ecp_curve_info_from_grp_id( grp_id );
         mbedtls_ecp_keypair ecp;
@@ -127,10 +131,10 @@ psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attribute
                                            size_t data_length,
                                            size_t *bits)
 {
-    ++test_driver_keygen_hooks.hits;
+    ++test_driver_key_management_hooks.hits;
 
-    if( test_driver_keygen_hooks.forced_status != PSA_SUCCESS )
-        return( test_driver_keygen_hooks.forced_status );
+    if( test_driver_key_management_hooks.forced_status != PSA_SUCCESS )
+        return( test_driver_key_management_hooks.forced_status );
 
 #if defined(MBEDTLS_ECP_C)
     psa_key_type_t type = psa_get_key_type( attributes );
@@ -154,7 +158,7 @@ psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attribute
                  * - The byte 0x04;
                  * - `x_P` as a `ceiling(m/8)`-byte string, big-endian;
                  * - `y_P` as a `ceiling(m/8)`-byte string, big-endian.
-                 * So its data length is 2m+1 where n is the key size in bits.
+                 * So its data length is 2m+1 where m is the curve size in bits.
                  */
                 if( ( data_length & 1 ) == 0 )
                     return( PSA_ERROR_INVALID_ARGUMENT );
diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
index 3cecbfc67..a0140d2cb 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function
+++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
@@ -132,7 +132,7 @@ void generate_key( int force_status_arg,
     psa_status_t actual_status;
     uint8_t actual_output[PSA_KEY_EXPORT_ECC_KEY_PAIR_MAX_SIZE(256)] = {0};
     size_t actual_output_length;
-    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+    test_driver_key_management_hooks = test_driver_key_management_hooks_init();
 
     psa_set_key_type( &attributes,
                       PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP_R1 ) );
@@ -142,18 +142,18 @@ void generate_key( int force_status_arg,
 
     if( fake_output->len > 0 )
     {
-        expected_output = test_driver_keygen_hooks.forced_output = fake_output->x;
-        expected_output_length = test_driver_keygen_hooks.forced_output_length =
+        expected_output = test_driver_key_management_hooks.forced_output = fake_output->x;
+        expected_output_length = test_driver_key_management_hooks.forced_output_length =
             fake_output->len;
     }
 
-    test_driver_keygen_hooks.hits = 0;
-    test_driver_keygen_hooks.forced_status = force_status;
+    test_driver_key_management_hooks.hits = 0;
+    test_driver_key_management_hooks.forced_status = force_status;
 
     PSA_ASSERT( psa_crypto_init( ) );
 
     actual_status = psa_generate_key( &attributes, &handle );
-    TEST_EQUAL( test_driver_keygen_hooks.hits, 1 );
+    TEST_EQUAL( test_driver_key_management_hooks.hits, 1 );
     TEST_EQUAL( actual_status, expected_status );
 
     if( actual_status == PSA_SUCCESS )
@@ -180,7 +180,7 @@ exit:
     psa_reset_key_attributes( &attributes );
     psa_destroy_key( handle );
     PSA_DONE( );
-    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+    test_driver_key_management_hooks = test_driver_key_management_hooks_init();
 }
 /* END_CASE */
 
@@ -196,25 +196,25 @@ void validate_key( int force_status_arg,
     psa_key_handle_t handle = 0;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
     psa_status_t actual_status;
-    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+    test_driver_key_management_hooks = test_driver_key_management_hooks_init();
 
     psa_set_key_type( &attributes,
                       key_type );
     psa_set_key_bits( &attributes, 0 );
     psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT );
 
-    test_driver_keygen_hooks.forced_status = force_status;
+    test_driver_key_management_hooks.forced_status = force_status;
 
     PSA_ASSERT( psa_crypto_init( ) );
 
     actual_status = psa_import_key( &attributes, key_input->x, key_input->len, &handle );
-    TEST_EQUAL( test_driver_keygen_hooks.hits, 1 );
+    TEST_EQUAL( test_driver_key_management_hooks.hits, 1 );
     TEST_EQUAL( actual_status, expected_status );
 exit:
     psa_reset_key_attributes( &attributes );
     psa_destroy_key( handle );
     PSA_DONE( );
-    test_driver_keygen_hooks = test_driver_keygen_hooks_init();
+    test_driver_key_management_hooks = test_driver_key_management_hooks_init();
 }
 /* END_CASE */
 
diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj
index 3e9d14a29..3f1be2105 100644
--- a/visualc/VS2010/mbedTLS.vcxproj
+++ b/visualc/VS2010/mbedTLS.vcxproj
@@ -239,7 +239,7 @@
     <ClInclude Include="..\..\tests\include\test\psa_helpers.h" />
     <ClInclude Include="..\..\tests\include\test\random.h" />
     <ClInclude Include="..\..\tests\include\test\drivers\cipher.h" />
-    <ClInclude Include="..\..\tests\include\test\drivers\keygen.h" />
+    <ClInclude Include="..\..\tests\include\test\drivers\key_management.h" />
     <ClInclude Include="..\..\tests\include\test\drivers\signature.h" />
     <ClInclude Include="..\..\tests\include\test\drivers\size.h" />
     <ClInclude Include="..\..\tests\include\test\drivers\test_driver.h" />

From 162ec8758fc8ff5a08b65f79ca985442aa6af193 Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Fri, 23 Oct 2020 12:03:08 +0200
Subject: [PATCH 6/7] Detecting bit size is no longer required

Storage format has been changed to always store the key's bit size

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c                 | 168 ---------------------------
 library/psa_crypto_core.h            |  19 ---
 library/psa_crypto_slot_management.c |   2 -
 3 files changed, 189 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 1901281c5..74b98714a 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -982,174 +982,6 @@ psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
     return( PSA_SUCCESS );
 }
 
-psa_status_t psa_detect_bit_size_in_slot( psa_key_slot_t *slot )
-{
-    if( slot->attr.bits != 0 )
-        return( PSA_SUCCESS );
-
-    if( key_type_is_raw_bytes( slot->attr.type ) )
-    {
-        slot->attr.bits =
-            (psa_key_bits_t) PSA_BYTES_TO_BITS( slot->data.key.bytes );
-        return( PSA_SUCCESS );
-    }
-    else if( PSA_KEY_TYPE_IS_ECC( slot->attr.type ) )
-    {
-        /* Keys are stored in export format, and we are currently
-         * restricted to known curves, so do the reverse lookup based
-         * on data length. */
-        size_t byte_length = slot->data.key.bytes;
-        if( PSA_KEY_TYPE_IS_PUBLIC_KEY( slot->attr.type ) &&
-            PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type ) !=
-                PSA_ECC_FAMILY_MONTGOMERY )
-        {
-            /* A Weierstrass public key is represented as:
-             * - The byte 0x04;
-             * - `x_P` as a `ceiling(m/8)`-byte string, big-endian;
-             * - `y_P` as a `ceiling(m/8)`-byte string, big-endian.
-             * So its data length is 2m+1 where m is the curve size in bits.
-             */
-            if( ( byte_length & 1 ) == 0 )
-                return( PSA_ERROR_BAD_STATE );
-            byte_length = byte_length / 2;
-
-            /* Montgomery public keys are represented in compressed format,
-             * meaning their curve_size is equal to the amount of input. */
-
-            /* Private keys are represented in uncompressed private random
-             * integer format, meaning their curve_size is equal to the
-             * amount of input. */
-        }
-
-        switch( PSA_KEY_TYPE_ECC_GET_FAMILY( slot->attr.type ) )
-        {
-            case PSA_ECC_FAMILY_SECP_R1:
-                switch( byte_length )
-                {
-                    case PSA_BITS_TO_BYTES( 192 ):
-                        slot->attr.bits = 192;
-                        break;
-                    case PSA_BITS_TO_BYTES( 224 ):
-                        slot->attr.bits = 224;
-                        break;
-                    case PSA_BITS_TO_BYTES( 256 ):
-                        slot->attr.bits = 256;
-                        break;
-                    case PSA_BITS_TO_BYTES( 384 ):
-                        slot->attr.bits = 384;
-                        break;
-                    case PSA_BITS_TO_BYTES( 521 ):
-                        slot->attr.bits = 521;
-                        break;
-                    default:
-                        return( PSA_ERROR_BAD_STATE );
-                }
-                break;
-
-            case PSA_ECC_FAMILY_BRAINPOOL_P_R1:
-                switch( byte_length )
-                {
-                    case PSA_BITS_TO_BYTES( 256 ):
-                        slot->attr.bits = 256;
-                        break;
-                    case PSA_BITS_TO_BYTES( 384 ):
-                        slot->attr.bits = 384;
-                        break;
-                    case PSA_BITS_TO_BYTES( 512 ):
-                        slot->attr.bits = 512;
-                        break;
-                    default:
-                        return( PSA_ERROR_BAD_STATE );
-                }
-                break;
-
-            case PSA_ECC_FAMILY_MONTGOMERY:
-                switch( byte_length )
-                {
-                    case PSA_BITS_TO_BYTES( 255 ):
-                        slot->attr.bits = 255;
-                        break;
-                    case PSA_BITS_TO_BYTES( 448 ):
-                        slot->attr.bits = 448;
-                        break;
-                    default:
-                        return( PSA_ERROR_BAD_STATE );
-                }
-                break;
-
-            case PSA_ECC_FAMILY_SECP_K1:
-                switch( byte_length )
-                {
-                    case PSA_BITS_TO_BYTES( 192 ):
-                        slot->attr.bits = 192;
-                        break;
-                    case PSA_BITS_TO_BYTES( 224 ):
-                        slot->attr.bits = 224;
-                        break;
-                    case PSA_BITS_TO_BYTES( 256 ):
-                        slot->attr.bits = 256;
-                        break;
-                    default:
-                        return( PSA_ERROR_BAD_STATE );
-                }
-                break;
-
-            default:
-                return( PSA_ERROR_BAD_STATE );
-        }
-
-        return( PSA_SUCCESS );
-    }
-    else if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
-    {
-        /* There's no easy way of figuring out the RSA bit size from
-         * the data length of the export representation. For now, use
-         * the mbed TLS software implementation to figure it out. */
-        psa_key_attributes_t attributes = {
-            .core = slot->attr
-        };
-        size_t bits;
-        psa_status_t status = psa_driver_wrapper_validate_key(
-                                &attributes,
-                                slot->data.key.data,
-                                slot->data.key.bytes,
-                                &bits );
-        if( status == PSA_SUCCESS )
-            slot->attr.bits = (psa_key_bits_t) bits;
-        if( status != PSA_ERROR_NOT_SUPPORTED )
-            return( status );
-
-        /* If no accelerator was able to figure it out, try software. */
-#if defined(MBEDTLS_RSA_C)
-        mbedtls_rsa_context *rsa = NULL;
-
-        /* Parse input */
-        status = psa_load_rsa_representation( slot->attr.type,
-                                              slot->data.key.data,
-                                              slot->data.key.bytes,
-                                              &rsa );
-        if( status != PSA_SUCCESS )
-        {
-            mbedtls_rsa_free( rsa );
-            mbedtls_free( rsa );
-            return( status );
-        }
-
-        slot->attr.bits = (psa_key_bits_t) PSA_BYTES_TO_BITS(
-            mbedtls_rsa_get_len( rsa ) );
-
-        mbedtls_rsa_free( rsa );
-        mbedtls_free( rsa );
-
-        return( PSA_SUCCESS );
-#else
-        return( PSA_ERROR_NOT_SUPPORTED );
-#endif
-    }
-    else
-        return( PSA_ERROR_NOT_SUPPORTED );
-}
-
 /** Import key data into a slot.
  *
  * `slot->type` must have been set previously.
diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h
index 2786b7993..8d1f1bb28 100644
--- a/library/psa_crypto_core.h
+++ b/library/psa_crypto_core.h
@@ -158,25 +158,6 @@ psa_status_t psa_copy_key_material_into_slot( psa_key_slot_t *slot,
                                               const uint8_t *data,
                                               size_t data_length );
 
-/** Detect the key bit size for a key in a slot where bit size
- *  is unset.
- *
- * This function assumes that the slot contains key material in
- * export format.
- *
- * \param[in,out] slot  Key slot to detect and set the bit size in.
- *
- * \retval #PSA_SUCCESS
- *         The key bit size was already set, or has been detected
- *         and set accordingly.
- * \retval #PSA_ERROR_BAD_STATE
- *         The size of the key material in the slot doesn't match
- *         with the declared key type.
- * \retval #PSA_ERROR_NOT_SUPPORTED
- *         The key type is unknown to the implementation.
- */
-psa_status_t psa_detect_bit_size_in_slot( psa_key_slot_t *slot );
-
 /** Convert an mbed TLS error code to a PSA error code
  *
  * \note This function is provided solely for the convenience of
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index f33c4f2ef..5140772e0 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -144,8 +144,6 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *slot )
         status = psa_copy_key_material_into_slot( slot, key_data, key_data_length );
         if( status != PSA_SUCCESS )
             goto exit;
-
-        status = psa_detect_bit_size_in_slot( slot );
     }
 
 exit:

From 40120f6b7606b1ab08b4848321c7b487431a83fe Mon Sep 17 00:00:00 2001
From: Steven Cooreman <steven.cooreman@silabs.com>
Date: Thu, 29 Oct 2020 11:42:22 +0100
Subject: [PATCH 7/7] Address review comments

* zero key buffer on failure
* readability improvements
* psa_finish_key_creation adjustment after removing import_key_into_slot

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
---
 library/psa_crypto.c               | 57 +++++++++---------------------
 library/psa_crypto_storage.h       |  5 +--
 tests/src/drivers/key_management.c |  6 +++-
 3 files changed, 25 insertions(+), 43 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 74b98714a..79ecf80a3 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1039,6 +1039,8 @@ static psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
          * psa_start_key_creation() wrote the size declared by the
          * caller, which may be 0 (meaning unspecified) or wrong. */
         slot->attr.bits = (psa_key_bits_t) bit_size;
+
+        return( PSA_SUCCESS );
     }
     else if( PSA_KEY_TYPE_IS_ASYMMETRIC( slot->attr.type ) )
     {
@@ -1067,41 +1069,27 @@ static psa_status_t psa_import_key_into_slot( psa_key_slot_t *slot,
 
         /* Key format is not supported by any accelerator, try software fallback
          * if present. */
+#if defined(MBEDTLS_ECP_C)
         if( PSA_KEY_TYPE_IS_ECC( slot->attr.type ) )
         {
-#if defined(MBEDTLS_ECP_C)
-            status = psa_import_ecp_key( slot,
-                                         data, data_length );
-#else
-            /* No drivers have been implemented yet, so without mbed TLS backing
-             * there's no way to do ECP with the current library. */
-            status = PSA_ERROR_NOT_SUPPORTED;
+            return( psa_import_ecp_key( slot, data, data_length ) );
+        }
 #endif /* defined(MBEDTLS_ECP_C) */
-        }
-        else if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
-        {
 #if defined(MBEDTLS_RSA_C)
-            status = psa_import_rsa_key( slot,
-                                         data, data_length );
-#else
-            /* No drivers have been implemented yet, so without mbed TLS backing
-             * there's no way to do RSA with the current library. */
-            status = PSA_ERROR_NOT_SUPPORTED;
-#endif /* defined(MBEDTLS_RSA_C) */
-        }
-        else
+        if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) )
         {
-            /* Unsupported asymmetric key type */
-            status = PSA_ERROR_NOT_SUPPORTED;
+            return( psa_import_rsa_key( slot, data, data_length ) );
         }
+#endif /* defined(MBEDTLS_RSA_C) */
+
+        /* Fell through the fallback as well, so have nothing else to try. */
+        return( PSA_ERROR_NOT_SUPPORTED );
     }
     else
     {
         /* Unknown key type */
-        status = PSA_ERROR_NOT_SUPPORTED;
+        return( PSA_ERROR_NOT_SUPPORTED );
     }
-
-    return( status );
 }
 
 /** Calculate the intersection of two algorithm usage policies.
@@ -1977,22 +1965,11 @@ static psa_status_t psa_finish_key_creation(
         else
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
         {
-            size_t buffer_size =
-                PSA_KEY_EXPORT_MAX_SIZE( slot->attr.type,
-                                         slot->attr.bits );
-            uint8_t *buffer = mbedtls_calloc( 1, buffer_size );
-            size_t length = 0;
-            if( buffer == NULL )
-                return( PSA_ERROR_INSUFFICIENT_MEMORY );
-            status = psa_internal_export_key( slot,
-                                              buffer, buffer_size, &length,
-                                              0 );
-            if( status == PSA_SUCCESS )
-                status = psa_save_persistent_key( &slot->attr,
-                                                  buffer, length );
-
-            mbedtls_platform_zeroize( buffer, buffer_size );
-            mbedtls_free( buffer );
+            /* Key material is saved in export representation in the slot, so
+             * just pass the slot buffer for storage. */
+            status = psa_save_persistent_key( &slot->attr,
+                                              slot->data.key.data,
+                                              slot->data.key.bytes );
         }
     }
 #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */
diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h
index de845a748..3def1b5e4 100644
--- a/library/psa_crypto_storage.h
+++ b/library/psa_crypto_storage.h
@@ -81,9 +81,10 @@ int psa_is_key_present_in_storage( const mbedtls_svc_key_id_t key );
  * This function formats the key data and metadata and saves it to a
  * persistent storage backend. The storage location corresponding to the
  * key slot must be empty, otherwise this function will fail. This function
- * should be called after psa_import_key_into_slot() to ensure the
+ * should be called after loading the key into an internal slot to ensure the
  * persistent key is not saved into a storage location corresponding to an
- * already occupied non-persistent key, as well as validating the key data.
+ * already occupied non-persistent key, as well as ensuring the key data is
+ * validated.
  *
  *
  * \param[in] attr          The attributes of the key to save.
diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c
index 6ca03c6be..9bef4b678 100644
--- a/tests/src/drivers/key_management.c
+++ b/tests/src/drivers/key_management.c
@@ -106,6 +106,10 @@ psa_status_t test_transparent_generate_key(
         {
             *key_length = bytes;
         }
+        else
+        {
+            memset( key, 0, bytes );
+        }
 
         mbedtls_ecp_keypair_free( &ecp );
         return( status );
@@ -146,7 +150,7 @@ psa_status_t test_transparent_validate_key(const psa_key_attributes_t *attribute
         mbedtls_ecp_keypair ecp;
         psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
 
-        if( *bits == 0 )
+        if( psa_get_key_bits( attributes ) == 0 )
         {
             // Attempt auto-detect of curve bit size
             size_t curve_size = data_length;