Merge pull request #191 from gilles-peskine-arm/psa-se_driver-key_bits

Secure element keys: save the key size

include/psa/crypto_se_driver.h

@@ -833,14 +833,18 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)(
  *
  * \param[in,out] drv_context   The driver context structure.
  * \param[in] key_slot          Slot where the key will be stored
- *                              This must be a valid slot for a key of the chosen
+ *                              This must be a valid slot for a key of the
- *                              type. It must be unoccupied.
+ *                              chosen type. It must be unoccupied.
  * \param[in] lifetime          The required lifetime of the key storage
  * \param[in] type              Key type (a \c PSA_KEY_TYPE_XXX value)
  * \param[in] algorithm         Key algorithm (a \c PSA_ALG_XXX value)
  * \param[in] usage             The allowed uses of the key
  * \param[in] p_data            Buffer containing the key data
  * \param[in] data_length       Size of the `data` buffer in bytes
+ * \param[out] bits             On success, the key size in bits. The driver
+ *                              must determine this value after parsing the
+ *                              key according to the key type.
+ *                              This value is not used if the function fails.
  *
  * \retval #PSA_SUCCESS
  *         Success.
@@ -852,7 +856,8 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_contex
                                                 psa_algorithm_t algorithm,
                                                 psa_key_usage_t usage,
                                                 const uint8_t *p_data,
-                                                size_t data_length);
+                                                size_t data_length,
+                                                size_t *bits);
 
 /**
  * \brief A function that destroys a secure element key and restore the slot to

library/psa_crypto.c

@@ -1035,6 +1035,11 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle )
 /* Return the size of the key in the given slot, in bits. */
 static size_t psa_get_key_slot_bits( const psa_key_slot_t *slot )
 {
+#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
+    if( psa_get_se_driver( slot->lifetime, NULL, NULL ) )
+        return( slot->data.se.bits );
+#endif /* defined(MBEDTLS_PSA_CRYPTO_SE_C) */
+
     if( key_type_is_raw_bytes( slot->type ) )
         return( slot->data.raw.bytes * 8 );
 #if defined(MBEDTLS_RSA_C)
@@ -1140,10 +1145,10 @@ exit:
 }
 #endif /* MBEDTLS_RSA_C */
 
-/** Retrieve the readily-accessible attributes of a key in a slot.
+/** Retrieve the generic attributes of a key in a slot.
  *
- * This function does not compute attributes that are not directly
+ * This function does not retrieve domain parameters, which require
- * stored in the slot, such as the bit size of a transparent key.
+ * additional memory management.
  */
 static void psa_get_key_slot_attributes( psa_key_slot_t *slot,
                                          psa_key_attributes_t *attributes )
@@ -1152,6 +1157,7 @@ static void psa_get_key_slot_attributes( psa_key_slot_t *slot,
     attributes->lifetime = slot->lifetime;
     attributes->policy = slot->policy;
     attributes->type = slot->type;
+    attributes->bits = psa_get_key_slot_bits( slot );
 }
 
 /** Retrieve all the publicly-accessible attributes of a key.
@@ -1164,21 +1170,26 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle,
 
     psa_reset_key_attributes( attributes );
 
-    status = psa_get_transparent_key( handle, &slot, 0, 0 );
+    status = psa_get_key_from_slot( handle, &slot, 0, 0 );
     if( status != PSA_SUCCESS )
         return( status );
 
     psa_get_key_slot_attributes( slot, attributes );
-    attributes->bits = psa_get_key_slot_bits( slot );
 
     switch( slot->type )
     {
 #if defined(MBEDTLS_RSA_C)
         case PSA_KEY_TYPE_RSA_KEY_PAIR:
         case PSA_KEY_TYPE_RSA_PUBLIC_KEY:
+#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->lifetime, NULL, NULL ) )
+                break;
+#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
             status = psa_get_rsa_public_exponent( slot->data.rsa, attributes );
             break;
-#endif
+#endif /* MBEDTLS_RSA_C */
         default:
             /* Nothing else to do. */
             break;
@@ -1489,6 +1500,10 @@ static psa_status_t psa_start_key_creation(
             (void) psa_crypto_stop_transaction( );
             return( status );
         }
+
+        /* TOnogrepDO: validate bits. How to do this depends on the key
+         * creation method, so setting bits might not belong here. */
+        slot->data.se.bits = psa_get_key_bits( attributes );
     }
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
 
@@ -1523,40 +1538,32 @@ static psa_status_t psa_finish_key_creation(
 #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)
     if( slot->lifetime != PSA_KEY_LIFETIME_VOLATILE )
     {
-        uint8_t *buffer = NULL;
+        psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
-        size_t buffer_size = 0;
+        psa_get_key_slot_attributes( slot, &attributes );
-        size_t length = 0;
 
 #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
         if( driver != NULL )
         {
-            buffer = (uint8_t*) &slot->data.se.slot_number;
+            status = psa_save_persistent_key( &attributes,
-            length = sizeof( slot->data.se.slot_number );
+                                              (uint8_t*) &slot->data.se,
+                                              sizeof( slot->data.se ) );
         }
         else
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
         {
-            buffer_size = PSA_KEY_EXPORT_MAX_SIZE( slot->type,
+            size_t buffer_size =
-                                                   psa_get_key_slot_bits( slot ) );
+                PSA_KEY_EXPORT_MAX_SIZE( slot->type,
-            buffer = mbedtls_calloc( 1, buffer_size );
+                                         psa_get_key_bits( &attributes ) );
+            uint8_t *buffer = mbedtls_calloc( 1, buffer_size );
+            size_t length = 0;
             if( buffer == NULL && buffer_size != 0 )
                 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( &attributes, buffer, length );
 
-        if( status == PSA_SUCCESS )
-        {
-            psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
-            psa_get_key_slot_attributes( slot, &attributes );
-            status = psa_save_persistent_key( &attributes, buffer, length );
-        }
-
-#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
-        if( driver == NULL )
-#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
-        {
             if( buffer_size != 0 )
                 mbedtls_platform_zeroize( buffer, buffer_size );
             mbedtls_free( buffer );
@@ -1696,8 +1703,8 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes,
             psa_get_se_driver_context( driver ),
             slot->data.se.slot_number,
             slot->lifetime, slot->type, slot->policy.alg, slot->policy.usage,
-            data, data_length );
+            data, data_length,
-        /* TOnogrepDO: psa_check_key_slot_attributes? */
+            &slot->data.se.bits );
     }
     else
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
@@ -1705,10 +1712,10 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes,
         status = psa_import_key_into_slot( slot, data, data_length );
         if( status != PSA_SUCCESS )
             goto exit;
-        status = psa_check_key_slot_attributes( slot, attributes );
-        if( status != PSA_SUCCESS )
-            goto exit;
     }
+    status = psa_check_key_slot_attributes( slot, attributes );
+    if( status != PSA_SUCCESS )
+        goto exit;
 
     status = psa_finish_key_creation( slot, driver );
 exit:

library/psa_crypto_core.h

@@ -64,6 +64,7 @@ typedef struct
         struct se
         {
             psa_key_slot_number_t slot_number;
+            size_t bits;
         } se;
     } data;
 } psa_key_slot_t;

library/psa_crypto_slot_management.c

@@ -138,13 +138,12 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *p_slot )
 #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
     if( psa_key_lifetime_is_external( p_slot->lifetime ) )
     {
-        if( key_data_length != sizeof( p_slot->data.se.slot_number ) )
+        if( key_data_length != sizeof( p_slot->data.se ) )
         {
             status = PSA_ERROR_STORAGE_FAILURE;
             goto exit;
         }
-        memcpy( &p_slot->data.se.slot_number, key_data,
+        memcpy( &p_slot->data.se, key_data, sizeof( p_slot->data.se ) );
-                sizeof( p_slot->data.se.slot_number ) );
     }
     else
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */

tests/suites/test_suite_psa_crypto_se_driver_hal.function

@@ -62,7 +62,8 @@ static psa_status_t null_import( psa_drv_se_context_t *context,
                                  psa_algorithm_t algorithm,
                                  psa_key_usage_t usage,
                                  const uint8_t *p_data,
-                                 size_t data_length )
+                                 size_t data_length,
+                                 size_t *bits )
 {
     (void) context;
     (void) slot_number;
@@ -71,7 +72,9 @@ static psa_status_t null_import( psa_drv_se_context_t *context,
     (void) algorithm;
     (void) usage;
     (void) p_data;
-    (void) data_length;
+    /* We're supposed to return a key size. Return one that's correct for
+     * plain data keys. */
+    *bits = PSA_BYTES_TO_BITS( data_length );
     return( PSA_SUCCESS );
 }
 
@@ -110,7 +113,8 @@ static psa_status_t ram_import( psa_drv_se_context_t *context,
                                 psa_algorithm_t algorithm,
                                 psa_key_usage_t usage,
                                 const uint8_t *p_data,
-                                size_t data_length )
+                                size_t data_length,
+                                size_t *bits )
 {
     (void) context;
     DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) );
@@ -119,6 +123,7 @@ static psa_status_t ram_import( psa_drv_se_context_t *context,
     ram_slots[slot_number].lifetime = lifetime;
     ram_slots[slot_number].type = type;
     ram_slots[slot_number].bits = PSA_BYTES_TO_BITS( data_length );
+    *bits = PSA_BYTES_TO_BITS( data_length );
     (void) algorithm;
     (void) usage;
     memcpy( ram_slots[slot_number].content, p_data, data_length );
@@ -178,6 +183,41 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context,
 /* Other test helper functions */
 /****************************************************************/
 
+/* Check that the attributes of a key reported by psa_get_key_attributes()
+ * are consistent with the attributes used when creating the key. */
+static int check_key_attributes(
+    psa_key_handle_t handle,
+    const psa_key_attributes_t *reference_attributes )
+{
+    int ok = 0;
+    psa_key_attributes_t actual_attributes = PSA_KEY_ATTRIBUTES_INIT;
+
+    PSA_ASSERT( psa_get_key_attributes( handle, &actual_attributes ) );
+
+    TEST_EQUAL( psa_get_key_id( &actual_attributes ),
+                psa_get_key_id( reference_attributes ) );
+    TEST_EQUAL( psa_get_key_lifetime( &actual_attributes ),
+                psa_get_key_lifetime( reference_attributes ) );
+    TEST_EQUAL( psa_get_key_type( &actual_attributes ),
+                psa_get_key_type( reference_attributes ) );
+    TEST_EQUAL( psa_get_key_usage_flags( &actual_attributes ),
+                psa_get_key_usage_flags( reference_attributes ) );
+    TEST_EQUAL( psa_get_key_algorithm( &actual_attributes ),
+                psa_get_key_algorithm( reference_attributes ) );
+    TEST_EQUAL( psa_get_key_enrollment_algorithm( &actual_attributes ),
+                psa_get_key_enrollment_algorithm( reference_attributes ) );
+    if( psa_get_key_bits( reference_attributes ) != 0 )
+    {
+        TEST_EQUAL( psa_get_key_bits( &actual_attributes ),
+                    psa_get_key_bits( reference_attributes ) );
+    }
+
+    ok = 1;
+
+exit:
+    return( ok );
+}
+
 /* Check that a function's return status is "smoke-free", i.e. that
  * it's an acceptable error code when calling an API function that operates
  * on a key with potentially bogus parameters. */
@@ -445,6 +485,11 @@ void key_creation_import_export( int min_slot, int restart )
     /* 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. */
+    psa_set_key_bits( &attributes,
+                      PSA_BYTES_TO_BITS( sizeof( key_material ) ) );
+    if( ! check_key_attributes( handle, &attributes ) )
+        goto exit;
     PSA_ASSERT( psa_export_key( handle,
                                 exported, sizeof( exported ),
                                 &exported_length ) );