Merge pull request #4713 from gilles-peskine-arm/psa-storage-format-test-lifetimes-3.0

PSA storage format: test lifetimes
Almost straightforward of #4392 thus merging with only one approval.

ChangeLog.d/psa-read-only-keys.txt

@@ -0,0 +1,5 @@
+Bugfix
+   * The PSA API no longer allows the creation or destruction of keys with a
+     read-only lifetime. The persistence level PSA_KEY_PERSISTENCE_READ_ONLY
+     can now only be used as intended, for keys that cannot be modified through
+     normal use of the API.

include/psa/crypto_values.h

@@ -2020,6 +2020,27 @@
     (PSA_KEY_LIFETIME_GET_PERSISTENCE(lifetime) == \
      PSA_KEY_PERSISTENCE_VOLATILE)
 
+/** Whether a key lifetime indicates that the key is read-only.
+ *
+ * Read-only keys cannot be created or destroyed through the PSA Crypto API.
+ * They must be created through platform-specific means that bypass the API.
+ *
+ * Some platforms may offer ways to destroy read-only keys. For example,
+ * consider a platform with multiple levels of privilege, where a
+ * low-privilege application can use a key but is not allowed to destroy
+ * it, and the platform exposes the key to the application with a read-only
+ * lifetime. High-privilege code can destroy the key even though the
+ * application sees the key as read-only.
+ *
+ * \param lifetime      The lifetime value to query (value of type
+ *                      ::psa_key_lifetime_t).
+ *
+ * \return \c 1 if the key is read-only, otherwise \c 0.
+ */
+#define PSA_KEY_LIFETIME_IS_READ_ONLY(lifetime)  \
+    (PSA_KEY_LIFETIME_GET_PERSISTENCE(lifetime) == \
+     PSA_KEY_PERSISTENCE_READ_ONLY)
+
 /** Construct a lifetime from a persistence level and a location.
  *
  * \param persistence   The persistence level
@@ -2140,7 +2161,7 @@ static inline int mbedtls_svc_key_id_equal( mbedtls_svc_key_id_t id1,
  */
 static inline int mbedtls_svc_key_id_is_null( mbedtls_svc_key_id_t key )
 {
-    return( ( key.MBEDTLS_PRIVATE(key_id) == 0 ) && ( key.MBEDTLS_PRIVATE(owner) == 0 ) );
+    return( key.MBEDTLS_PRIVATE(key_id) == 0 );
 }
 
 #endif /* !MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER */

library/psa_crypto.c

@@ -1052,6 +1052,17 @@ psa_status_t psa_destroy_key( mbedtls_svc_key_id_t key )
        return( PSA_ERROR_GENERIC_ERROR );
     }
 
+    if( PSA_KEY_LIFETIME_IS_READ_ONLY( slot->attr.lifetime ) )
+    {
+        /* Refuse the destruction of a read-only key (which may or may not work
+         * if we attempt it, depending on whether the key is merely read-only
+         * by policy or actually physically read-only).
+         * Just do the best we can, which is to wipe the copy in memory
+         * (done in this function's cleanup code). */
+        overall_status = PSA_ERROR_NOT_PERMITTED;
+        goto exit;
+    }
+
 #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
     driver = psa_get_se_driver_entry( slot->attr.lifetime );
     if( driver != NULL )
@@ -1113,12 +1124,10 @@ psa_status_t psa_destroy_key( mbedtls_svc_key_id_t key )
     }
 #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
 
-#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
 exit:
-#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
     status = psa_wipe_key_slot( slot );
     /* Prioritize CORRUPTION_DETECTED from wiping over a storage error */
-    if( overall_status == PSA_SUCCESS )
+    if( status != PSA_SUCCESS )
         overall_status = status;
     return( overall_status );
 }

library/psa_crypto_slot_management.c

@@ -455,6 +455,9 @@ psa_status_t psa_validate_key_persistence( psa_key_lifetime_t lifetime )
     {
         /* Persistent keys require storage support */
 #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)
+        if( PSA_KEY_LIFETIME_IS_READ_ONLY( lifetime ) )
+            return( PSA_ERROR_INVALID_ARGUMENT );
+        else
             return( PSA_SUCCESS );
 #else /* MBEDTLS_PSA_CRYPTO_STORAGE_C */
         return( PSA_ERROR_NOT_SUPPORTED );
@@ -545,16 +548,17 @@ void mbedtls_psa_get_stats( mbedtls_psa_stats_t *stats )
             ++stats->empty_slots;
             continue;
         }
-        if( slot->attr.lifetime == PSA_KEY_LIFETIME_VOLATILE )
+        if( PSA_KEY_LIFETIME_IS_VOLATILE( slot->attr.lifetime ) )
             ++stats->volatile_slots;
-        else if( slot->attr.lifetime == PSA_KEY_LIFETIME_PERSISTENT )
+        else
         {
             psa_key_id_t id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id );
             ++stats->persistent_slots;
             if( id > stats->max_open_internal_key_id )
                 stats->max_open_internal_key_id = id;
         }
-        else
+        if( PSA_KEY_LIFETIME_GET_LOCATION( slot->attr.lifetime ) !=
+            PSA_KEY_LOCATION_LOCAL_STORAGE )
         {
             psa_key_id_t id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id );
             ++stats->external_slots;

scripts/mbedtls_dev/macro_collector.py

@@ -81,11 +81,15 @@ class PSAMacroEnumerator:
     `self.arguments_for` for arguments that are not of a kind that is
     enumerated here.
     """
+    #pylint: disable=too-many-instance-attributes
 
     def __init__(self) -> None:
         """Set up an empty set of known constructor macros.
         """
         self.statuses = set() #type: Set[str]
+        self.lifetimes = set() #type: Set[str]
+        self.locations = set() #type: Set[str]
+        self.persistence_levels = set() #type: Set[str]
         self.algorithms = set() #type: Set[str]
         self.ecc_curves = set() #type: Set[str]
         self.dh_groups = set() #type: Set[str]
@@ -133,6 +137,9 @@ class PSAMacroEnumerator:
         self.arguments_for['aead_alg'] = sorted(self.aead_algorithms)
         self.arguments_for['curve'] = sorted(self.ecc_curves)
         self.arguments_for['group'] = sorted(self.dh_groups)
+        self.arguments_for['persistence'] = sorted(self.persistence_levels)
+        self.arguments_for['location'] = sorted(self.locations)
+        self.arguments_for['lifetime'] = sorted(self.lifetimes)
 
     @staticmethod
     def _format_arguments(name: str, arguments: Iterable[str]) -> str:
@@ -341,6 +348,9 @@ enumerate
             'ALG': self.algorithms,
             'ECC_CURVE': self.ecc_curves,
             'DH_GROUP': self.dh_groups,
+            'KEY_LIFETIME': self.lifetimes,
+            'KEY_LOCATION': self.locations,
+            'KEY_PERSISTENCE': self.persistence_levels,
             'KEY_TYPE': self.key_types,
             'KEY_USAGE': self.key_usage_flags,
         } #type: Dict[str, Set[str]]
@@ -367,6 +377,7 @@ enumerate
             'asymmetric_encryption_algorithm': [],
             'pake_algorithm': [self.pake_algorithms],
             'other_algorithm': [],
+            'lifetime': [self.lifetimes],
         } #type: Dict[str, List[Set[str]]]
         self.arguments_for['mac_length'] += ['1', '63']
         self.arguments_for['min_mac_length'] += ['1', '63']

scripts/mbedtls_dev/psa_storage.py

@@ -164,6 +164,10 @@ class Key:
         """
         return self.bytes().hex()
 
+    def location_value(self) -> int:
+        """The numerical value of the location encoded in the key's lifetime."""
+        return self.lifetime.value() >> 8
+
 
 class TestKey(unittest.TestCase):
     # pylint: disable=line-too-long

tests/scripts/generate_psa_tests.py

@@ -295,6 +295,38 @@ class StorageFormat:
                           *extra_arguments])
         return tc
 
+    def key_for_lifetime(
+            self,
+            lifetime: str,
+    ) -> StorageKey:
+        """Construct a test key for the given lifetime."""
+        short = lifetime
+        short = re.sub(r'PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION',
+                       r'', short)
+        short = re.sub(r'PSA_KEY_[A-Z]+_', r'', short)
+        description = 'lifetime: ' + short
+        key = StorageKey(version=self.version,
+                         id=1, lifetime=lifetime,
+                         type='PSA_KEY_TYPE_RAW_DATA', bits=8,
+                         usage='PSA_KEY_USAGE_EXPORT', alg=0, alg2=0,
+                         material=b'L',
+                         description=description)
+        return key
+
+    def all_keys_for_lifetimes(self) -> Iterator[StorageKey]:
+        """Generate test keys covering lifetimes."""
+        lifetimes = sorted(self.constructors.lifetimes)
+        expressions = self.constructors.generate_expressions(lifetimes)
+        for lifetime in expressions:
+            # Don't attempt to create or load a volatile key in storage
+            if 'VOLATILE' in lifetime:
+                continue
+            # Don't attempt to create a read-only key in storage,
+            # but do attempt to load one.
+            if 'READ_ONLY' in lifetime and self.forward:
+                continue
+            yield self.key_for_lifetime(lifetime)
+
     def key_for_usage_flags(
             self,
             usage_flags: List[str],
@@ -395,12 +427,17 @@ class StorageFormat:
         # one go, which is a significant performance gain as the information
         # includes numerical values obtained by compiling a C program.
         keys = [] #type: List[StorageKey]
+        keys += self.all_keys_for_lifetimes()
         keys += self.all_keys_for_usage_flags()
         keys += self.all_keys_for_types()
         keys += self.all_keys_for_algorithms()
         for key in keys:
+            if key.location_value() != 0:
+                # Skip keys with a non-default location, because they
+                # require a driver and we currently have no mechanism to
+                # determine whether a driver is available.
+                continue
             yield self.make_test_case(key)
-        # To do: vary id, lifetime
 
 
 class TestGenerator:

tests/suites/test_suite_psa_crypto_metadata.data

@@ -344,3 +344,39 @@ ecc_key_family:PSA_ECC_FAMILY_TWISTED_EDWARDS
 
 DH group family: RFC 7919
 dh_key_family:PSA_DH_FAMILY_RFC7919
+
+Lifetime: VOLATILE
+lifetime:PSA_KEY_LIFETIME_VOLATILE:KEY_LIFETIME_IS_VOLATILE:PSA_KEY_PERSISTENCE_VOLATILE:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: PERSISTENT
+lifetime:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_KEY_PERSISTENCE_DEFAULT:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: volatile, local storage
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_VOLATILE, PSA_KEY_LOCATION_LOCAL_STORAGE):KEY_LIFETIME_IS_VOLATILE:PSA_KEY_PERSISTENCE_VOLATILE:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: default, local storage
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_DEFAULT, PSA_KEY_LOCATION_LOCAL_STORAGE):0:PSA_KEY_PERSISTENCE_DEFAULT:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: 2, local storage
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):0:2:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: 254, local storage
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(254, PSA_KEY_LOCATION_LOCAL_STORAGE):0:254:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: read-only, local storage
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_READ_ONLY, PSA_KEY_LOCATION_LOCAL_STORAGE):KEY_LIFETIME_IS_READ_ONLY:PSA_KEY_PERSISTENCE_READ_ONLY:PSA_KEY_LOCATION_LOCAL_STORAGE
+
+Lifetime: volatile, 0x123456
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_VOLATILE, 0x123456):KEY_LIFETIME_IS_VOLATILE:PSA_KEY_PERSISTENCE_VOLATILE:0x123456
+
+Lifetime: default, 0x123456
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_DEFAULT, 0x123456):0:PSA_KEY_PERSISTENCE_DEFAULT:0x123456
+
+Lifetime: 2, 0x123456
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, 0x123456):0:2:0x123456
+
+Lifetime: 254, 0x123456
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(254, 0x123456):0:254:0x123456
+
+Lifetime: read-only, 0x123456
+lifetime:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_READ_ONLY, 0x123456):KEY_LIFETIME_IS_READ_ONLY:PSA_KEY_PERSISTENCE_READ_ONLY:0x123456

tests/suites/test_suite_psa_crypto_metadata.function

@@ -55,8 +55,21 @@
 #define KEY_TYPE_IS_ECC                 ( 1u << 6 )
 #define KEY_TYPE_IS_DH                  ( 1u << 7 )
 
+/* Flags for lifetime classification macros. There is a flag for every
+ * lifetime classification macro PSA_KEY_LIFETIME_IS_xxx. The name of the
+ * flag is the name of the classification macro without the PSA_ prefix. */
+#define KEY_LIFETIME_IS_VOLATILE        ( 1u << 0 )
+#define KEY_LIFETIME_IS_READ_ONLY       ( 1u << 1 )
+
 #define TEST_CLASSIFICATION_MACRO( flag, alg, flags ) \
-    TEST_ASSERT( PSA_##flag( alg ) == !! ( ( flags ) & flag ) )
+    do                                                \
+    {                                                 \
+        if( ( flags ) & ( flag ) )                    \
+            TEST_ASSERT( PSA_##flag( alg ) );         \
+        else                                          \
+            TEST_ASSERT( ! PSA_##flag( alg ) );       \
+    }                                                 \
+    while( 0 )
 
 /* Check the parity of value.
  *
@@ -665,3 +678,20 @@ void dh_key_family( int group_arg )
     TEST_EQUAL( PSA_KEY_TYPE_DH_GET_FAMILY( pair_type ), group );
 }
 /* END_CASE */
+
+/* BEGIN_CASE */
+void lifetime( int lifetime_arg, int classification_flags,
+               int persistence_arg, int location_arg )
+{
+    psa_key_lifetime_t lifetime = lifetime_arg;
+    psa_key_persistence_t persistence = persistence_arg;
+    psa_key_location_t location = location_arg;
+    unsigned flags = classification_flags;
+
+    TEST_CLASSIFICATION_MACRO( KEY_LIFETIME_IS_VOLATILE, lifetime, flags );
+    TEST_CLASSIFICATION_MACRO( KEY_LIFETIME_IS_READ_ONLY, lifetime, flags );
+
+    TEST_EQUAL( PSA_KEY_LIFETIME_GET_PERSISTENCE( lifetime ), persistence );
+    TEST_EQUAL( PSA_KEY_LIFETIME_GET_LOCATION( lifetime ), location );
+}
+/* END_CASE */

tests/suites/test_suite_psa_crypto_persistent_key.data

@@ -119,3 +119,12 @@ import_export_persistent_key:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c
 import/export-persistent symmetric key with restart: 16 bytes
 depends_on:PSA_WANT_KEY_TYPE_AES:MBEDTLS_PK_C:MBEDTLS_PK_PARSE_C
 import_export_persistent_key:"2b7e151628aed2a6abf7158809cf4f3c":PSA_KEY_TYPE_AES:128:1:0
+
+Destroy invalid id: 0
+destroy_nonexistent:0:PSA_SUCCESS
+
+Destroy non-existent key
+destroy_nonexistent:1:PSA_ERROR_INVALID_HANDLE
+
+Destroy invalid id: 0xffffffff
+destroy_nonexistent:0xffffffff:PSA_ERROR_INVALID_HANDLE

tests/suites/test_suite_psa_crypto_persistent_key.function

@@ -323,3 +323,18 @@ exit:
     psa_destroy_persistent_key( key_id );
 }
 /* END_CASE */
+
+/* BEGIN_CASE */
+void destroy_nonexistent( int id_arg, int expected_status_arg )
+{
+    mbedtls_svc_key_id_t id = mbedtls_svc_key_id_make( 1, id_arg );
+    psa_status_t expected_status = expected_status_arg;
+
+    PSA_INIT( );
+
+    TEST_EQUAL( expected_status, psa_destroy_key( id ) );
+
+exit:
+    PSA_DONE( );
+}
+/* END_CASE */

tests/suites/test_suite_psa_crypto_slot_management.data

@@ -78,6 +78,27 @@ Persistent slot: ECP keypair (ECDH+ECDSA, exportable), restart
 depends_on:PSA_WANT_ALG_ECDH:PSA_WANT_ALG_ECDSA:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256
 persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:137:1:PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH:PSA_ALG_KEY_AGREEMENT(PSA_ALG_ECDH, PSA_ALG_HKDF(PSA_ALG_SHA_256)):PSA_ALG_ECDSA_ANY:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":INVALIDATE_BY_SHUTDOWN
 
+Persistent slot, check after closing, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):124:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_CLOSING
+
+Persistent slot, check after closing and restarting, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):125:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_CLOSING_WITH_SHUTDOWN
+
+Persistent slot, check after destroying, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):126:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_DESTROYING
+
+Persistent slot, check after destroying and restarting, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):127:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_DESTROYING_WITH_SHUTDOWN
+
+Persistent slot, check after purging, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):200:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_PURGING
+
+Persistent slot, check after purging and restarting, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):201:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_PURGING_WITH_SHUTDOWN
+
+Persistent slot, check after restart with live handle, persistence=2
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(2, PSA_KEY_LOCATION_LOCAL_STORAGE):128:PSA_KEY_ID_USER_MIN:0:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":INVALIDATE_BY_SHUTDOWN
+
 Attempt to overwrite: close before
 create_existent:PSA_KEY_LIFETIME_PERSISTENT:0x1736:1:CLOSE_BEFORE
 
@@ -107,13 +128,17 @@ Open failure: non-existent identifier
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:1:PSA_ERROR_DOES_NOT_EXIST
 
-Create failure: invalid lifetime for a persistent key
+Create failure: read-only key
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-create_fail:0x7fffffff:1:PSA_ERROR_INVALID_ARGUMENT
+create_fail:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_READ_ONLY, PSA_KEY_LOCATION_LOCAL_STORAGE):1:PSA_ERROR_INVALID_ARGUMENT
 
-Create failure: invalid lifetime for a volatile key
+Create failure: invalid location for a persistent key
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-create_fail:0x7fffff00:0:PSA_ERROR_INVALID_ARGUMENT
+create_fail:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_DEFAULT, 0xbad10cU):1:PSA_ERROR_INVALID_ARGUMENT
+
+Create failure: invalid location for a volatile key
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION(PSA_KEY_PERSISTENCE_VOLATILE, 0xbad10cU):0:PSA_ERROR_INVALID_ARGUMENT
 
 Create failure: invalid key id (0) for a persistent key
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C

tests/suites/test_suite_psa_crypto_storage_format.function

@@ -8,6 +8,7 @@
 #include <psa_crypto_its.h>
 
 #define TEST_FLAG_EXERCISE      0x00000001
+#define TEST_FLAG_READ_ONLY     0x00000002
 
 /** Write a key with the given attributes and key material to storage.
  * Test that it has the expected representation.
@@ -115,10 +116,20 @@ static int test_read_key( const psa_key_attributes_t *expected_attributes,
                          psa_get_key_algorithm( expected_attributes ) ) );
     }
 
+
+    if( flags & TEST_FLAG_READ_ONLY )
+    {
+        /* Read-only keys cannot be removed through the API.
+         * The key will be removed through ITS in the cleanup code below. */
+        TEST_EQUAL( PSA_ERROR_NOT_PERMITTED, psa_destroy_key( key_id ) );
+    }
+    else
+    {
         /* Destroy the key. Confirm through direct access to the storage. */
         PSA_ASSERT( psa_destroy_key( key_id ) );
         TEST_EQUAL( PSA_ERROR_DOES_NOT_EXIST,
                     psa_its_get_info( uid, &storage_info ) );
+    }
 
     ok = 1;
 
@@ -219,7 +230,6 @@ void key_storage_read( int lifetime_arg, int type_arg, int bits_arg,
 
 exit:
     psa_reset_key_attributes( &attributes );
-    psa_destroy_key( key_id );
     PSA_DONE( );
 }
 /* END_CASE */