Merge pull request #201 from gilles-peskine-arm/psa-se_driver-set_key_slot_number

Add slot number attribute
diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h
index 6dfaa13..130ce75 100644
--- a/include/psa/crypto_extra.h
+++ b/include/psa/crypto_extra.h
@@ -104,6 +104,79 @@
     return( attributes->core.policy.alg2 );
 }
 
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+
+/** Retrieve the slot number where a key is stored.
+ *
+ * A slot number is only defined for keys that are stored in a secure
+ * element.
+ *
+ * This information is only useful if the secure element is not entirely
+ * managed through the PSA Cryptography API. It is up to the secure
+ * element driver to decide how PSA slot numbers map to any other interface
+ * that the secure element may have.
+ *
+ * \param[in] attributes        The key attribute structure to query.
+ * \param[out] slot_number      On success, the slot number containing the key.
+ *
+ * \retval #PSA_SUCCESS
+ *         The key is located in a secure element, and \p *slot_number
+ *         indicates the slot number that contains it.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ *         The caller is not permitted to query the slot number.
+ *         Mbed Crypto currently does not return this error.
+ * \retval #PSA_ERROR_INVALID_ARGUMENT
+ *         The key is not located in a secure element.
+ */
+psa_status_t psa_get_key_slot_number(
+    const psa_key_attributes_t *attributes,
+    psa_key_slot_number_t *slot_number );
+
+/** Choose the slot number where a key is stored.
+ *
+ * This function declares a slot number in the specified attribute
+ * structure.
+ *
+ * A slot number is only meaningful for keys that are stored in a secure
+ * element. It is up to the secure element driver to decide how PSA slot
+ * numbers map to any other interface that the secure element may have.
+ *
+ * \note Setting a slot number in key attributes for a key creation can
+ *       cause the following errors when creating the key:
+ *       - #PSA_ERROR_NOT_SUPPORTED if the selected secure element does
+ *         not support choosing a specific slot number.
+ *       - #PSA_ERROR_NOT_PERMITTED if the caller is not permitted to
+ *         choose slot numbers in general or to choose this specific slot.
+ *       - #PSA_ERROR_INVALID_ARGUMENT if the chosen slot number is not
+ *         valid in general or not valid for this specific key.
+ *       - #PSA_ERROR_ALREADY_EXISTS if there is already a key in the
+ *         selected slot.
+ *
+ * \param[out] attributes       The attribute structure to write to.
+ * \param slot_number           The slot number to set.
+ */
+static inline void psa_set_key_slot_number(
+    psa_key_attributes_t *attributes,
+    psa_key_slot_number_t slot_number )
+{
+    attributes->core.flags |= MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER;
+    attributes->slot_number = slot_number;
+}
+
+/** Remove the slot number attribute from a key attribute structure.
+ *
+ * This function undoes the action of psa_set_key_slot_number().
+ *
+ * \param[out] attributes       The attribute structure to write to.
+ */
+static inline void psa_clear_key_slot_number(
+    psa_key_attributes_t *attributes )
+{
+    attributes->core.flags &= ~MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER;
+}
+
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+
 /**@}*/
 
 /**
diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h
index f95eaeb..69cdaba 100644
--- a/include/psa/crypto_se_driver.h
+++ b/include/psa/crypto_se_driver.h
@@ -134,10 +134,17 @@
                                           void *persistent_data,
                                           psa_key_lifetime_t lifetime);
 
+#if defined(__DOXYGEN_ONLY__) || !defined(MBEDTLS_PSA_CRYPTO_SE_C)
+/* Mbed Crypto with secure element support enabled defines this type in
+ * crypto_types.h because it is also visible to applications through an
+ * implementation-specific extension.
+ * For the PSA Cryptography specification, this type is only visible
+ * via crypto_se_driver.h. */
 /** An internal designation of a key slot between the core part of the
  * PSA Crypto implementation and the driver. The meaning of this value
  * is driver-dependent. */
 typedef uint64_t psa_key_slot_number_t;
+#endif /* __DOXYGEN_ONLY__ || !MBEDTLS_PSA_CRYPTO_SE_C */
 
 /**@}*/
 
diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h
index 9e38e53..fbfe77e 100644
--- a/include/psa/crypto_struct.h
+++ b/include/psa/crypto_struct.h
@@ -322,6 +322,29 @@
  * conditionals. */
 #define PSA_MAX_KEY_BITS 0xfff8
 
+/** A mask of flags that can be stored in key attributes.
+ *
+ * This type is also used internally to store flags in slots. Internal
+ * flags are defined in library/psa_crypto_core.h. Internal flags may have
+ * the same value as external flags if they are properly handled during
+ * key creation and in psa_get_key_attributes.
+ */
+typedef uint16_t psa_key_attributes_flag_t;
+
+#define MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER     \
+    ( (psa_key_attributes_flag_t) 0x0001 )
+
+/* A mask of key attribute flags used externally only.
+ * Only meant for internal checks inside the library. */
+#define MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY (      \
+        MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER |    \
+        0 )
+
+/* A mask of key attribute flags used both internally and externally.
+ * Currently there aren't any. */
+#define MBEDTLS_PSA_KA_MASK_DUAL_USE (          \
+        0 )
+
 typedef struct
 {
     psa_key_type_t type;
@@ -329,7 +352,7 @@
     psa_key_id_t id;
     psa_key_policy_t policy;
     psa_key_bits_t bits;
-    uint16_t flags;
+    psa_key_attributes_flag_t flags;
 } psa_core_key_attributes_t;
 
 #define PSA_CORE_KEY_ATTRIBUTES_INIT {0, 0, 0, {0, 0, 0}, 0, 0}
@@ -337,11 +360,19 @@
 struct psa_key_attributes_s
 {
     psa_core_key_attributes_t core;
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+    psa_key_slot_number_t slot_number;
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     void *domain_parameters;
     size_t domain_parameters_size;
 };
 
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+#define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, 0, NULL, 0}
+#else
 #define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, NULL, 0}
+#endif
+
 static inline struct psa_key_attributes_s psa_key_attributes_init( void )
 {
     const struct psa_key_attributes_s v = PSA_KEY_ATTRIBUTES_INIT;
diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h
index 1944be4..9af4957 100644
--- a/include/psa/crypto_types.h
+++ b/include/psa/crypto_types.h
@@ -244,6 +244,17 @@
  */
 typedef struct psa_key_attributes_s psa_key_attributes_t;
 
+
+#ifndef __DOXYGEN_ONLY__
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+/* Mbed Crypto defines this type in crypto_types.h because it is also
+ * visible to applications through an implementation-specific extension.
+ * For the PSA Cryptography specification, this type is only visible
+ * via crypto_se_driver.h. */
+typedef uint64_t psa_key_slot_number_t;
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+#endif /* !__DOXYGEN_ONLY__ */
+
 /**@}*/
 
 /** \defgroup derivation Key derivation
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 41289c6..5cb88de 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1187,6 +1187,13 @@
         return( status );
 
     attributes->core = slot->attr;
+    attributes->core.flags &= ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY |
+                                MBEDTLS_PSA_KA_MASK_DUAL_USE );
+
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+    if( psa_key_slot_is_external( slot ) )
+        psa_set_key_slot_number( attributes, slot->data.se.slot_number );
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
 
     switch( slot->attr.type )
     {
@@ -1196,7 +1203,7 @@
 #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
             /* TOnogrepDO: reporting the public exponent for opaque keys
              * is not yet implemented. */
-            if( psa_get_se_driver( slot->attr.lifetime, NULL, NULL ) )
+            if( psa_key_slot_is_external( slot ) )
                 break;
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
             status = psa_get_rsa_public_exponent( slot->data.rsa, attributes );
@@ -1212,6 +1219,21 @@
     return( status );
 }
 
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+psa_status_t psa_get_key_slot_number(
+    const psa_key_attributes_t *attributes,
+    psa_key_slot_number_t *slot_number )
+{
+    if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER )
+    {
+        *slot_number = attributes->slot_number;
+        return( PSA_SUCCESS );
+    }
+    else
+        return( PSA_ERROR_INVALID_ARGUMENT );
+}
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+
 #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C)
 static int pk_write_pubkey_simple( mbedtls_pk_context *key,
                                    unsigned char *buf, size_t size )
@@ -1408,6 +1430,15 @@
                                      data_length, 1 ) );
 }
 
+#if defined(static_assert)
+static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0,
+               "One or more key attribute flag is listed as both external-only and dual-use" );
+static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0,
+               "One or more key attribute flag is listed as both internal-only and dual-use" );
+static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0,
+               "One or more key attribute flag is listed as both internal-only and external-only" );
+#endif
+
 /** Validate that a key policy is internally well-formed.
  *
  * This function only rejects invalid policies. It does not validate the
@@ -1467,6 +1498,11 @@
     if( psa_get_key_bits( attributes ) > PSA_MAX_KEY_BITS )
         return( PSA_ERROR_NOT_SUPPORTED );
 
+    /* Reject invalid flags. These should not be reachable through the API. */
+    if( attributes->core.flags & ~ ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY |
+                                     MBEDTLS_PSA_KA_MASK_DUAL_USE ) )
+        return( PSA_ERROR_INVALID_ARGUMENT );
+
     return( PSA_SUCCESS );
 }
 
@@ -1510,7 +1546,7 @@
     if( status != PSA_SUCCESS )
         return( status );
 
-    status = psa_internal_allocate_key_slot( handle, p_slot );
+    status = psa_get_empty_key_slot( handle, p_slot );
     if( status != PSA_SUCCESS )
         return( status );
     slot = *p_slot;
@@ -1523,6 +1559,13 @@
 
     slot->attr = attributes->core;
 
+    /* Erase external-only flags from the internal copy. To access
+     * external-only flags, query `attributes`. Thanks to the check
+     * in psa_validate_key_attributes(), this leaves the dual-use
+     * flags and any internal flag that psa_get_empty_key_slot()
+     * may have set. */
+    slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY;
+
 #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
@@ -1539,6 +1582,10 @@
      * we can roll back to a state where the key doesn't exist. */
     if( *p_drv != NULL )
     {
+        /* Choosing a slot number is not supported yet. */
+        if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER )
+            return( PSA_ERROR_NOT_SUPPORTED );
+
         status = psa_find_se_slot_for_key( attributes, *p_drv,
                                            &slot->data.se.slot_number );
         if( status != PSA_SUCCESS )
diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h
index fbfb6da..edf3ab6 100644
--- a/library/psa_crypto_core.h
+++ b/library/psa_crypto_core.h
@@ -56,14 +56,21 @@
         /* EC public key or key pair */
         mbedtls_ecp_keypair *ecp;
 #endif /* MBEDTLS_ECP_C */
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
         /* Any key type in a secure element */
         struct se
         {
             psa_key_slot_number_t slot_number;
         } se;
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     } data;
 } psa_key_slot_t;
 
+/* A mask of key attribute flags used only internally.
+ * Currently there aren't any. */
+#define PSA_KA_MASK_INTERNAL_ONLY (     \
+        0 )
+
 /** Test whether a key slot is occupied.
  *
  * A key slot is occupied iff the key type is nonzero. This works because
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 0734009..fe92148 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -102,7 +102,7 @@
     global_data.key_slots_initialized = 0;
 }
 
-psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle,
+psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle,
                                              psa_key_slot_t **p_slot )
 {
     if( ! global_data.key_slots_initialized )
@@ -228,7 +228,7 @@
     if( status != PSA_SUCCESS )
         return( status );
 
-    status = psa_internal_allocate_key_slot( handle, &slot );
+    status = psa_get_empty_key_slot( handle, &slot );
     if( status != PSA_SUCCESS )
         return( status );
 
diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h
index cde590f..472253d 100644
--- a/library/psa_crypto_slot_management.h
+++ b/library/psa_crypto_slot_management.h
@@ -71,8 +71,8 @@
  * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
  * \retval #PSA_ERROR_BAD_STATE
  */
-psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle,
-                                             psa_key_slot_t **p_slot );
+psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle,
+                                     psa_key_slot_t **p_slot );
 
 /** Test whether a lifetime designates a key in an external cryptoprocessor.
  *
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index b049840..4118d2f 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -19,6 +19,9 @@
 PSA key attributes: lifetime then id
 persistence_attributes:0x1234:3:0x1235:0x1235:3
 
+PSA key attributes: slot number
+slot_number_attribute:
+
 PSA import/export raw: 0 bytes
 import_export:"":PSA_KEY_TYPE_RAW_DATA:PSA_KEY_USAGE_EXPORT:0:0:0:PSA_SUCCESS:1
 
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index 0eb6172..3225bef 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -1113,6 +1113,23 @@
     return( ok );
 }
 
+/* Assert that a key isn't reported as having a slot number. */
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+#define ASSERT_NO_SLOT_NUMBER( attributes )                             \
+    do                                                                  \
+    {                                                                   \
+        psa_key_slot_number_t ASSERT_NO_SLOT_NUMBER_slot_number;        \
+        TEST_EQUAL( psa_get_key_slot_number(                            \
+                        attributes,                                     \
+                        &ASSERT_NO_SLOT_NUMBER_slot_number ),           \
+                    PSA_ERROR_INVALID_ARGUMENT );                       \
+    }                                                                   \
+    while( 0 )
+#else /* MBEDTLS_PSA_CRYPTO_SE_C */
+#define ASSERT_NO_SLOT_NUMBER( attributes )     \
+    ( (void) 0 )
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
+
 /* An overapproximation of the amount of storage needed for a key of the
  * given type and with the given content. The API doesn't make it easy
  * to find a good value for the size. The current implementation doesn't
@@ -1214,6 +1231,46 @@
 }
 /* END_CASE */
 
+/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_SE_C */
+void slot_number_attribute( )
+{
+    psa_key_slot_number_t slot_number = 0xdeadbeef;
+    psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
+
+    /* Initially, there is no slot number. */
+    TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ),
+                PSA_ERROR_INVALID_ARGUMENT );
+
+    /* Test setting a slot number. */
+    psa_set_key_slot_number( &attributes, 0 );
+    PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) );
+    TEST_EQUAL( slot_number, 0 );
+
+    /* Test changing the slot number. */
+    psa_set_key_slot_number( &attributes, 42 );
+    PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) );
+    TEST_EQUAL( slot_number, 42 );
+
+    /* Test clearing the slot number. */
+    psa_clear_key_slot_number( &attributes );
+    TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ),
+                PSA_ERROR_INVALID_ARGUMENT );
+
+    /* Clearing again should have no effect. */
+    psa_clear_key_slot_number( &attributes );
+    TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ),
+                PSA_ERROR_INVALID_ARGUMENT );
+
+    /* Test that reset clears the slot number. */
+    psa_set_key_slot_number( &attributes, 42 );
+    PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) );
+    TEST_EQUAL( slot_number, 42 );
+    psa_reset_key_attributes( &attributes );
+    TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ),
+                PSA_ERROR_INVALID_ARGUMENT );
+}
+/* END_CASE */
+
 /* BEGIN_CASE */
 void import_with_policy( int type_arg,
                          int usage_arg, int alg_arg,
@@ -1246,6 +1303,7 @@
     TEST_EQUAL( psa_get_key_type( &got_attributes ), type );
     TEST_EQUAL( psa_get_key_usage_flags( &got_attributes ), usage );
     TEST_EQUAL( psa_get_key_algorithm( &got_attributes ), alg );
+    ASSERT_NO_SLOT_NUMBER( &got_attributes );
 
     PSA_ASSERT( psa_destroy_key( handle ) );
     test_operations_on_invalid_handle( handle );
@@ -1284,6 +1342,7 @@
     TEST_EQUAL( psa_get_key_type( &got_attributes ), type );
     if( attr_bits != 0 )
         TEST_EQUAL( attr_bits, psa_get_key_bits( &got_attributes ) );
+    ASSERT_NO_SLOT_NUMBER( &got_attributes );
 
     PSA_ASSERT( psa_destroy_key( handle ) );
     test_operations_on_invalid_handle( handle );
@@ -1328,6 +1387,7 @@
         TEST_EQUAL( psa_get_key_type( &attributes ), type );
         TEST_EQUAL( psa_get_key_bits( &attributes ),
                     PSA_BYTES_TO_BITS( byte_size ) );
+        ASSERT_NO_SLOT_NUMBER( &attributes );
         memset( buffer, 0, byte_size + 1 );
         PSA_ASSERT( psa_export_key( handle, buffer, byte_size, &n ) );
         for( n = 0; n < byte_size; n++ )
@@ -1420,6 +1480,7 @@
     PSA_ASSERT( psa_get_key_attributes( handle, &got_attributes ) );
     TEST_EQUAL( psa_get_key_type( &got_attributes ), type );
     TEST_EQUAL( psa_get_key_bits( &got_attributes ), (size_t) expected_bits );
+    ASSERT_NO_SLOT_NUMBER( &got_attributes );
 
     /* Export the key */
     status = psa_export_key( handle,
diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function
index 6ac19a6..9a57464 100644
--- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function
+++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function
@@ -212,6 +212,31 @@
                     psa_get_key_bits( reference_attributes ) );
     }
 
+    {
+        psa_key_slot_number_t actual_slot_number = 0xdeadbeef;
+        psa_key_slot_number_t desired_slot_number = 0xb90cc011;
+        psa_key_lifetime_t lifetime =
+            psa_get_key_lifetime( &actual_attributes );
+        psa_status_t status = psa_get_key_slot_number( &actual_attributes,
+                                                       &actual_slot_number );
+        if( lifetime < MIN_DRIVER_LIFETIME )
+        {
+            /* The key is not in a secure element. */
+            TEST_EQUAL( status, PSA_ERROR_INVALID_ARGUMENT );
+        }
+        else
+        {
+            /* The key is in a secure element. If it had been created
+             * in a specific slot, check that it is reported there. */
+            PSA_ASSERT( status );
+            status = psa_get_key_slot_number( reference_attributes,
+                                              &desired_slot_number );
+            if( status == PSA_SUCCESS )
+            {
+                TEST_EQUAL( desired_slot_number, actual_slot_number );
+            }
+        }
+    }
     ok = 1;
 
 exit:
@@ -485,11 +510,14 @@
     /* Test that the key was created in the expected slot. */
     TEST_ASSERT( ram_slots[min_slot].type == PSA_KEY_TYPE_RAW_DATA );
 
-    /* Test the key attributes and the key data. */
+    /* Test the key attributes, including the reported slot number. */
     psa_set_key_bits( &attributes,
                       PSA_BYTES_TO_BITS( sizeof( key_material ) ) );
+    psa_set_key_slot_number( &attributes, min_slot );
     if( ! check_key_attributes( handle, &attributes ) )
         goto exit;
+
+    /* Test the key data. */
     PSA_ASSERT( psa_export_key( handle,
                                 exported, sizeof( exported ),
                                 &exported_length ) );