From ee32cd4af6ad6436caa04b86ceae05806edf862e Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 6 May 2019 18:39:37 +0200
Subject: [PATCH 1/7] Slot management tests: more robust storage purge

Record what key ids have been used in a test case and purge them. The
cleanup code no longer requires the key identifiers used in the tests
to be in a certain small range.
---
 ..._suite_psa_crypto_slot_management.function | 66 ++++++++++++-------
 1 file changed, 41 insertions(+), 25 deletions(-)

diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function
index 03b7197a6..267353e5b 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.function
+++ b/tests/suites/test_suite_psa_crypto_slot_management.function
@@ -23,31 +23,47 @@ typedef enum
 } reopen_policy_t;
 
 /* All test functions that create persistent keys must call
- * `TEST_MAX_KEY_ID( key_id )` before creating a persistent key with this
+ * `TEST_USES_KEY_ID( key_id )` before creating a persistent key with this
  * identifier, and must call psa_purge_key_storage() in their cleanup
  * code. */
 
 #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)
-/* There is no API to purge all keys. For this test suite, require that
- * all key IDs be less than a certain maximum, or a well-known value
- * which corresponds to a file that does not contain a key. */
-#define MAX_KEY_ID_FOR_TEST 32
-#define KEY_ID_IS_WELL_KNOWN( key_id )                  \
-    ( ( key_id ) == PSA_CRYPTO_ITS_RANDOM_SEED_UID )
-#define TEST_MAX_KEY_ID( key_id )                       \
-    TEST_ASSERT( ( key_id ) <= MAX_KEY_ID_FOR_TEST ||   \
-                 KEY_ID_IS_WELL_KNOWN( key_id ) )
-void psa_purge_key_storage( void )
+static psa_key_id_t key_ids_used_in_test[9];
+static size_t num_key_ids_used;
+
+/* Record a key id as potentially used in a test case. */
+static int test_uses_key_id( psa_key_id_t key_id )
 {
-    psa_key_id_t i;
-    /* The tests may have potentially created key ids from 1 to
-     * MAX_KEY_ID_FOR_TEST. In addition, run the destroy function on key id
-     * 0, which file-based storage uses as a temporary file. */
-    for( i = 0; i <= MAX_KEY_ID_FOR_TEST; i++ )
-        psa_destroy_persistent_key( i );
+    size_t i;
+    if( key_id > PSA_MAX_PERSISTENT_KEY_IDENTIFIER )
+    {
+        /* Don't touch key id values that designate non-key files. */
+        return( 1 );
+    }
+    for( i = 0; i < num_key_ids_used ; i++ )
+    {
+        if( key_id == key_ids_used_in_test[i] )
+            return( 1 );
+    }
+    if( num_key_ids_used == ARRAY_LENGTH( key_ids_used_in_test ) )
+        return( 0 );
+    key_ids_used_in_test[num_key_ids_used] = key_id;
+    ++num_key_ids_used;
+    return( 1 );
+}
+#define TEST_USES_KEY_ID( key_id )                       \
+    TEST_ASSERT( test_uses_key_id( key_id ) )
+
+/* Destroy all key ids that may have been created by the current test case. */
+static void psa_purge_key_storage( void )
+{
+    size_t i;
+    for( i = 0; i < num_key_ids_used; i++ )
+        psa_destroy_persistent_key( key_ids_used_in_test[i] );
+    num_key_ids_used = 0;
 }
 #else
-#define TEST_MAX_KEY_ID( key_id ) ( (void) ( key_id ) )
+#define TEST_USES_KEY_ID( key_id ) ( (void) ( key_id ) )
 #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C */
 
 /* END_HEADER */
@@ -122,7 +138,7 @@ void persistent_slot_lifecycle( int lifetime_arg, int id_arg,
     psa_key_handle_t handle = 0;
     psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
 
-    TEST_MAX_KEY_ID( id );
+    TEST_USES_KEY_ID( id );
 
     PSA_ASSERT( psa_crypto_init( ) );
 
@@ -200,7 +216,7 @@ void create_existent( int lifetime_arg, int id_arg,
     size_t reexported_length;
     reopen_policy_t reopen_policy = reopen_policy_arg;
 
-    TEST_MAX_KEY_ID( id );
+    TEST_USES_KEY_ID( id );
 
     PSA_ASSERT( psa_crypto_init( ) );
 
@@ -279,7 +295,7 @@ void create_fail( int lifetime_arg, int id_arg,
     psa_key_handle_t handle = 0xdead;
     uint8_t material[1] = {'k'};
 
-    TEST_MAX_KEY_ID( id );
+    TEST_USES_KEY_ID( id );
 
     PSA_ASSERT( psa_crypto_init( ) );
 
@@ -323,8 +339,8 @@ void copy_across_lifetimes( int source_lifetime_arg, int source_id_arg,
     psa_algorithm_t expected_alg = expected_alg_arg;
     uint8_t *export_buffer = NULL;
 
-    TEST_MAX_KEY_ID( source_id );
-    TEST_MAX_KEY_ID( target_id );
+    TEST_USES_KEY_ID( source_id );
+    TEST_USES_KEY_ID( target_id );
 
     PSA_ASSERT( psa_crypto_init( ) );
 
@@ -427,8 +443,8 @@ void copy_to_occupied( int source_lifetime_arg, int source_id_arg,
     psa_key_attributes_t attributes1 = PSA_KEY_ATTRIBUTES_INIT;
     psa_key_attributes_t attributes2 = PSA_KEY_ATTRIBUTES_INIT;
 
-    TEST_MAX_KEY_ID( source_id );
-    TEST_MAX_KEY_ID( target_id );
+    TEST_USES_KEY_ID( source_id );
+    TEST_USES_KEY_ID( target_id );
 
     PSA_ASSERT( psa_crypto_init( ) );
 

From 225010fdf77debcdcd8697c330d62038dac20a4e Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 6 May 2019 18:44:55 +0200
Subject: [PATCH 2/7] Remove lifetime parameter from psa_open_key

Change the scope of key identifiers to be global, rather than
per lifetime. As a result, you now need to specify the lifetime of a
key only when creating it.
---
 include/psa/crypto.h                             | 14 ++++----------
 include/psa/crypto_types.h                       | 13 +++++++++++++
 library/psa_crypto_slot_management.c             |  7 +++----
 tests/suites/test_suite_psa_crypto.function      |  5 ++---
 ...test_suite_psa_crypto_persistent_key.function | 12 ++++--------
 .../test_suite_psa_crypto_slot_management.data   | 14 ++++----------
 ...est_suite_psa_crypto_slot_management.function | 16 +++++++---------
 7 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/include/psa/crypto.h b/include/psa/crypto.h
index 2e680b101..424c16e31 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -512,9 +512,6 @@ void psa_reset_key_attributes(psa_key_attributes_t *attributes);
  *
  * Open a handle to a key which was previously created with psa_create_key().
  *
- * \param lifetime      The lifetime of the key. This designates a storage
- *                      area where the key material is stored. This must not
- *                      be #PSA_KEY_LIFETIME_VOLATILE.
  * \param id            The persistent identifier of the key.
  * \param[out] handle   On success, a handle to a key slot which contains
  *                      the data and metadata loaded from the specified
@@ -526,19 +523,16 @@ void psa_reset_key_attributes(psa_key_attributes_t *attributes);
  * \retval #PSA_ERROR_INSUFFICIENT_MEMORY
  * \retval #PSA_ERROR_DOES_NOT_EXIST
  * \retval #PSA_ERROR_INVALID_ARGUMENT
- *         \p lifetime is invalid, for example #PSA_KEY_LIFETIME_VOLATILE.
- * \retval #PSA_ERROR_INVALID_ARGUMENT
- *         \p id is invalid for the specified lifetime.
- * \retval #PSA_ERROR_NOT_SUPPORTED
- *         \p lifetime is not supported.
+ *         \p id is invalid.
  * \retval #PSA_ERROR_NOT_PERMITTED
  *         The specified key exists, but the application does not have the
  *         permission to access it. Note that this specification does not
  *         define any way to create such a key, but it may be possible
  *         through implementation-specific means.
+ * \retval #PSA_ERROR_COMMUNICATION_FAILURE
+ * \retval #PSA_ERROR_STORAGE_FAILURE
  */
-psa_status_t psa_open_key(psa_key_lifetime_t lifetime,
-                          psa_key_id_t id,
+psa_status_t psa_open_key(psa_key_id_t id,
                           psa_key_handle_t *handle);
 
 /** Close a key handle.
diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h
index 7054de72e..da6e6b9c5 100644
--- a/include/psa/crypto_types.h
+++ b/include/psa/crypto_types.h
@@ -85,6 +85,19 @@ typedef uint32_t psa_algorithm_t;
  */
 
 /** Encoding of key lifetimes.
+ *
+ * The lifetime of a key indicates where it is stored and what system actions
+ * may create and destroy it.
+ *
+ * Keys with the lifetime #PSA_KEY_LIFETIME_VOLATILE are automatically
+ * destroyed when the application terminates or on a power reset.
+ *
+ * Keys with a lifetime other than #PSA_KEY_LIFETIME_VOLATILE are said
+ * to be _persistent_.
+ * Persistent keys are preserved if the application or the system restarts.
+ * Persistent keys have a key identifier of type #psa_key_id_t.
+ * The application can call psa_open_key() to open a persistent key that
+ * it created previously.
  */
 typedef uint32_t psa_key_lifetime_t;
 
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 8ee561512..30cc05bd3 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -278,11 +278,10 @@ static psa_status_t persistent_key_setup( psa_key_lifetime_t lifetime,
 #endif /* !defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */
 }
 
-psa_status_t psa_open_key( psa_key_lifetime_t lifetime,
-                           psa_key_file_id_t id,
-                           psa_key_handle_t *handle )
+psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle )
 {
-    return( persistent_key_setup( lifetime, id, handle, PSA_SUCCESS ) );
+    return( persistent_key_setup( PSA_KEY_LIFETIME_PERSISTENT,
+                                  id, handle, PSA_SUCCESS ) );
 }
 
 psa_status_t psa_create_key( psa_key_lifetime_t lifetime,
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index 67c2c77f9..85ac4ebba 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -4911,8 +4911,7 @@ void persistent_key_load_key_from_storage( data_t *data,
     PSA_ASSERT( psa_crypto_init() );
 
     /* Check key slot still contains key data */
-    PSA_ASSERT( psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id,
-                              &handle ) );
+    PSA_ASSERT( psa_open_key( key_id, &handle ) );
     PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) );
     TEST_EQUAL( psa_get_key_id( &attributes ), key_id );
     TEST_EQUAL( psa_get_key_lifetime( &attributes ),
@@ -4947,7 +4946,7 @@ exit:
         /* In case there was a test failure after creating the persistent key
          * but while it was not open, try to re-open the persistent key
          * to delete it. */
-        psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id, &handle );
+        psa_open_key( key_id, &handle );
     }
     psa_destroy_key( handle );
     mbedtls_psa_crypto_free();
diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function
index a2f4f779b..827a7d8e4 100644
--- a/tests/suites/test_suite_psa_crypto_persistent_key.function
+++ b/tests/suites/test_suite_psa_crypto_persistent_key.function
@@ -134,8 +134,7 @@ void persistent_key_destroy( int key_id_arg, int restart,
         psa_close_key( handle );
         mbedtls_psa_crypto_free();
         PSA_ASSERT( psa_crypto_init() );
-        PSA_ASSERT( psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id,
-                                  &handle ) );
+        PSA_ASSERT( psa_open_key( key_id, &handle ) );
     }
     TEST_EQUAL( psa_is_key_present_in_storage( key_id ), 1 );
 
@@ -144,8 +143,7 @@ void persistent_key_destroy( int key_id_arg, int restart,
 
     /* Check key slot storage is removed */
     TEST_EQUAL( psa_is_key_present_in_storage( key_id ), 0 );
-    TEST_EQUAL( psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id, &handle ),
-                PSA_ERROR_DOES_NOT_EXIST );
+    TEST_EQUAL( psa_open_key( key_id, &handle ), PSA_ERROR_DOES_NOT_EXIST );
     TEST_EQUAL( handle, 0 );
 
     /* Shutdown and restart */
@@ -191,8 +189,7 @@ void persistent_key_import( int key_id_arg, int type_arg, data_t *data,
         psa_close_key( handle );
         mbedtls_psa_crypto_free();
         PSA_ASSERT( psa_crypto_init() );
-        PSA_ASSERT( psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id,
-                                  &handle ) );
+        PSA_ASSERT( psa_open_key( key_id, &handle ) );
     }
 
     psa_reset_key_attributes( &attributes );
@@ -242,8 +239,7 @@ void import_export_persistent_key( data_t *data, int type_arg,
         psa_close_key( handle );
         mbedtls_psa_crypto_free();
         PSA_ASSERT( psa_crypto_init() );
-        PSA_ASSERT( psa_open_key( PSA_KEY_LIFETIME_PERSISTENT, key_id,
-                                  &handle ) );
+        PSA_ASSERT( psa_open_key( key_id, &handle ) );
     }
 
     /* Test the key information */
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data
index 5dc2b6787..c5afdfa95 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.data
+++ b/tests/suites/test_suite_psa_crypto_slot_management.data
@@ -27,21 +27,15 @@ create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:KEEP_OPEN
 
 Open failure: invalid identifier (0)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-open_fail:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_ERROR_INVALID_ARGUMENT
+open_fail:0:PSA_ERROR_INVALID_ARGUMENT
 
 Open failure: invalid identifier (random seed UID)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-open_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT
+open_fail:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT
 
 Open failure: non-existent identifier
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-open_fail:PSA_KEY_LIFETIME_PERSISTENT:1:PSA_ERROR_DOES_NOT_EXIST
-
-Open failure: volatile lifetime
-open_fail:PSA_KEY_LIFETIME_VOLATILE:1:PSA_ERROR_INVALID_ARGUMENT
-
-Open failure: invalid lifetime
-open_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT
+open_fail:1:PSA_ERROR_DOES_NOT_EXIST
 
 Create failure: invalid lifetime
 create_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT
@@ -56,7 +50,7 @@ create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR
 
 Open not supported
 depends_on:!MBEDTLS_PSA_CRYPTO_STORAGE_C
-open_fail:PSA_KEY_LIFETIME_PERSISTENT:1:PSA_ERROR_NOT_SUPPORTED
+open_fail:1:PSA_ERROR_NOT_SUPPORTED
 
 Create not supported
 depends_on:!MBEDTLS_PSA_CRYPTO_STORAGE_C
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function
index 267353e5b..d06d3d749 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.function
+++ b/tests/suites/test_suite_psa_crypto_slot_management.function
@@ -155,7 +155,7 @@ void persistent_slot_lifecycle( int lifetime_arg, int id_arg,
 
     /* Close the key and reopen it. */
     PSA_ASSERT( psa_close_key( handle ) );
-    PSA_ASSERT( psa_open_key( lifetime, id, &handle ) );
+    PSA_ASSERT( psa_open_key( id, &handle ) );
     PSA_ASSERT( psa_get_key_information( handle, &read_type, NULL ) );
     TEST_EQUAL( read_type, type );
 
@@ -184,12 +184,12 @@ void persistent_slot_lifecycle( int lifetime_arg, int id_arg,
     {
         case CLOSE_BY_CLOSE:
         case CLOSE_BY_SHUTDOWN:
-            PSA_ASSERT( psa_open_key( lifetime, id, &handle ) );
+            PSA_ASSERT( psa_open_key( id, &handle ) );
             PSA_ASSERT( psa_get_key_information( handle, &read_type, NULL ) );
             TEST_EQUAL( read_type, type );
             break;
         case CLOSE_BY_DESTROY:
-            TEST_EQUAL( psa_open_key( lifetime, id, &handle ),
+            TEST_EQUAL( psa_open_key( id, &handle ),
                         PSA_ERROR_DOES_NOT_EXIST );
             break;
     }
@@ -241,7 +241,7 @@ void create_existent( int lifetime_arg, int id_arg,
     if( reopen_policy == CLOSE_AFTER )
         PSA_ASSERT( psa_close_key( handle1 ) );
     if( reopen_policy == CLOSE_BEFORE || reopen_policy == CLOSE_AFTER )
-        PSA_ASSERT( psa_open_key( lifetime, id, &handle1 ) );
+        PSA_ASSERT( psa_open_key( id, &handle1 ) );
 
     /* Check that the original key hasn't changed. */
     psa_reset_key_attributes( &attributes );
@@ -266,17 +266,16 @@ exit:
 /* END_CASE */
 
 /* BEGIN_CASE */
-void open_fail( int lifetime_arg, int id_arg,
+void open_fail( int id_arg,
                 int expected_status_arg )
 {
-    psa_key_lifetime_t lifetime = lifetime_arg;
     psa_key_id_t id = id_arg;
     psa_status_t expected_status = expected_status_arg;
     psa_key_handle_t handle = 0xdead;
 
     PSA_ASSERT( psa_crypto_init( ) );
 
-    TEST_EQUAL( psa_open_key( lifetime, id, &handle ), expected_status );
+    TEST_EQUAL( psa_open_key( id, &handle ), expected_status );
     TEST_EQUAL( handle, 0 );
 
 exit:
@@ -376,8 +375,7 @@ void copy_across_lifetimes( int source_lifetime_arg, int source_id_arg,
     {
         mbedtls_psa_crypto_free( );
         PSA_ASSERT( psa_crypto_init( ) );
-        PSA_ASSERT( psa_open_key( target_lifetime, target_id,
-                                  &target_handle ) );
+        PSA_ASSERT( psa_open_key( target_id, &target_handle ) );
     }
 
     /* Test that the target slot has the expected content. */

From 4a231b8d3b195abad1e8ef28d564a1f69c733a8d Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 6 May 2019 18:56:14 +0200
Subject: [PATCH 3/7] Break up key identifiers into user, vendor and reserved
 ranges

Define a range of key identifiers for use by the application
(0..2^30-1), a range for use by implementations (2^30..2^31), and a
range that is reserved for future use (2^31..2^32-1).
---
 include/psa/crypto.h        |  4 ++++
 include/psa/crypto_types.h  |  7 +++++++
 include/psa/crypto_values.h | 13 +++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/include/psa/crypto.h b/include/psa/crypto.h
index 424c16e31..a62dd8bff 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -512,6 +512,10 @@ void psa_reset_key_attributes(psa_key_attributes_t *attributes);
  *
  * Open a handle to a key which was previously created with psa_create_key().
  *
+ * Implementations may provide additional keys that can be opened with
+ * psa_open_key(). Such keys have a key identifier in the vendor range,
+ * as documented in the description of #psa_key_id_t.
+ *
  * \param id            The persistent identifier of the key.
  * \param[out] handle   On success, a handle to a key slot which contains
  *                      the data and metadata loaded from the specified
diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h
index da6e6b9c5..44c7c66e0 100644
--- a/include/psa/crypto_types.h
+++ b/include/psa/crypto_types.h
@@ -102,6 +102,13 @@ typedef uint32_t psa_algorithm_t;
 typedef uint32_t psa_key_lifetime_t;
 
 /** Encoding of identifiers of persistent keys.
+ *
+ * - Applications may freely choose key identifiers in the range
+ *   #PSA_KEY_ID_USER_MIN to #PSA_KEY_ID_USER_MAX.
+ * - Implementations may define additional key identifiers in the range
+ *   #PSA_KEY_ID_VENDOR_MIN to #PSA_KEY_ID_VENDOR_MAX.
+ * - Key identifiers outside these ranges are reserved for future use
+ *   in future versions of this specification.
  */
 /* Implementation-specific quirk: The Mbed Crypto library can be built as
  * part of a multi-client service that exposes the PSA Crypto API in each
diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h
index eddf63262..40172b32d 100644
--- a/include/psa/crypto_values.h
+++ b/include/psa/crypto_values.h
@@ -1440,6 +1440,19 @@
  */
 #define PSA_KEY_LIFETIME_PERSISTENT             ((psa_key_lifetime_t)0x00000001)
 
+/** The minimum value for a key identifier chosen by the application.
+ */
+#define PSA_KEY_ID_USER_MIN                     ((psa_key_id_t)0x00000000)
+/** The minimum value for a key identifier chosen by the application.
+ */
+#define PSA_KEY_ID_USER_MAX                     ((psa_key_id_t)0x3fffffff)
+/** The minimum value for a key identifier chosen by the application.
+ */
+#define PSA_KEY_ID_VENDOR_MIN                   ((psa_key_id_t)0x40000000)
+/** The minimum value for a key identifier chosen by the application.
+ */
+#define PSA_KEY_ID_VENDOR_MAX                   ((psa_key_id_t)0x7fffffff)
+
 /**@}*/
 
 /** \defgroup policy Key policies

From f9666595e147cdb9efcc680cc39d9653f3a61f4c Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 6 May 2019 18:56:30 +0200
Subject: [PATCH 4/7] Implement and test the new key identifier range

Only allow creating keys in the application (user) range. Allow
opening keys in the implementation (vendor) range as well.

Compared with what the implementation allowed, which was undocumented:
0 is now allowed; values from 0x40000000 to 0xfffeffff are now
forbidden.
---
 library/psa_crypto.c                          |  2 +-
 library/psa_crypto_slot_management.c          | 29 +++++++-----
 library/psa_crypto_slot_management.h          |  5 ++-
 library/psa_crypto_storage.h                  |  2 +-
 ...test_suite_psa_crypto_slot_management.data | 45 +++++++++++++------
 5 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 9cf90ddaf..fa459a176 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1401,7 +1401,7 @@ static psa_status_t psa_start_key_creation(
     if( attributes->lifetime != PSA_KEY_LIFETIME_VOLATILE )
     {
         status = psa_validate_persistent_key_parameters( attributes->lifetime,
-                                                         attributes->id );
+                                                         attributes->id, 1 );
         if( status != PSA_SUCCESS )
             return( status );
         slot->persistent_storage_id = attributes->id;
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 30cc05bd3..2ef70db59 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -176,20 +176,23 @@ exit:
  * is provided.
  *
  * \param file_id       The key identifier to check.
+ * \param vendor_ok     Nonzero to allow key ids in the vendor range.
+ *                      0 to allow only key ids in the application range.
  *
  * \return              1 if \p file_id is acceptable, otherwise 0.
  */
-static int psa_is_key_id_valid( psa_key_file_id_t file_id )
+static int psa_is_key_id_valid( psa_key_file_id_t file_id,
+                                int vendor_ok )
 {
     psa_app_key_id_t key_id = PSA_KEY_FILE_GET_KEY_ID( file_id );
-    /* Reject id=0 because by general library conventions, 0 is an invalid
-     * value wherever possible. */
-    if( key_id == 0 )
-        return( 0 );
     /* Reject high values because the file names are reserved for the
      * library's internal use. */
     if( key_id > PSA_MAX_PERSISTENT_KEY_IDENTIFIER )
         return( 0 );
+    /* Applications may only create keys in the range
+     * 0..PSA_KEY_ID_USER_MAX. */
+    if( ! vendor_ok && key_id > PSA_KEY_ID_USER_MAX )
+        return( 0 );
     return( 1 );
 }
 
@@ -231,13 +234,14 @@ static psa_status_t psa_internal_make_key_persistent( psa_key_handle_t handle,
 
 psa_status_t psa_validate_persistent_key_parameters(
     psa_key_lifetime_t lifetime,
-    psa_key_file_id_t id )
+    psa_key_file_id_t id,
+    int creating )
 {
     if( lifetime != PSA_KEY_LIFETIME_PERSISTENT )
         return( PSA_ERROR_INVALID_ARGUMENT );
 
 #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)
-    if( ! psa_is_key_id_valid( id ) )
+    if( ! psa_is_key_id_valid( id, ! creating ) )
         return( PSA_ERROR_INVALID_ARGUMENT );
     return( PSA_SUCCESS );
 
@@ -250,13 +254,15 @@ psa_status_t psa_validate_persistent_key_parameters(
 static psa_status_t persistent_key_setup( psa_key_lifetime_t lifetime,
                                           psa_key_file_id_t id,
                                           psa_key_handle_t *handle,
-                                          psa_status_t wanted_load_status )
+                                          int creating )
 {
     psa_status_t status;
+    psa_status_t wanted_load_status =
+        ( creating ? PSA_ERROR_DOES_NOT_EXIST : PSA_SUCCESS );
 
     *handle = 0;
 
-    status = psa_validate_persistent_key_parameters( lifetime, id );
+    status = psa_validate_persistent_key_parameters( lifetime, id, creating );
     if( status != PSA_SUCCESS )
         return( status );
 
@@ -281,7 +287,7 @@ static psa_status_t persistent_key_setup( psa_key_lifetime_t lifetime,
 psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle )
 {
     return( persistent_key_setup( PSA_KEY_LIFETIME_PERSISTENT,
-                                  id, handle, PSA_SUCCESS ) );
+                                  id, handle, 0 ) );
 }
 
 psa_status_t psa_create_key( psa_key_lifetime_t lifetime,
@@ -290,8 +296,7 @@ psa_status_t psa_create_key( psa_key_lifetime_t lifetime,
 {
     psa_status_t status;
 
-    status = persistent_key_setup( lifetime, id, handle,
-                                   PSA_ERROR_DOES_NOT_EXIST );
+    status = persistent_key_setup( lifetime, id, handle, 1 );
     switch( status )
     {
         case PSA_SUCCESS: return( PSA_ERROR_ALREADY_EXISTS );
diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h
index 914e2d507..2e459d1a7 100644
--- a/library/psa_crypto_slot_management.h
+++ b/library/psa_crypto_slot_management.h
@@ -64,6 +64,8 @@ void psa_wipe_all_key_slots( void );
  *
  * \param lifetime      The lifetime to test.
  * \param id            The key id to test.
+ * \param creating      0 if attempting to open an existing key.
+ *                      Nonzero if attempting to create a key.
  *
  * \retval PSA_SUCCESS
  *         The given parameters are valid.
@@ -74,7 +76,8 @@ void psa_wipe_all_key_slots( void );
  */
 psa_status_t psa_validate_persistent_key_parameters(
     psa_key_lifetime_t lifetime,
-    psa_key_file_id_t id );
+    psa_key_file_id_t id,
+    int creating );
 
 
 #endif /* PSA_CRYPTO_SLOT_MANAGEMENT_H */
diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h
index 5434d0529..2af624a0c 100644
--- a/library/psa_crypto_storage.h
+++ b/library/psa_crypto_storage.h
@@ -59,7 +59,7 @@ extern "C" {
  * This limitation will probably become moot when we implement client
  * separation for key storage.
  */
-#define PSA_MAX_PERSISTENT_KEY_IDENTIFIER 0xfffeffff
+#define PSA_MAX_PERSISTENT_KEY_IDENTIFIER PSA_KEY_ID_VENDOR_MAX
 
 /**
  * \brief Checks if persistent data is stored for the given key slot number
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data
index c5afdfa95..519e81ec7 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.data
+++ b/tests/suites/test_suite_psa_crypto_slot_management.data
@@ -7,14 +7,23 @@ transient_slot_lifecycle:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789ab
 Transient slot, check after restart
 transient_slot_lifecycle:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN
 
-Persistent slot, check after closing
-persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE
+Persistent slot, check after closing, id=min
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE
 
-Persistent slot, check after destroying
-persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY
+Persistent slot, check after destroying, id=min
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY
 
-Persistent slot, check after restart
-persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN
+Persistent slot, check after restart, id=min
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN
+
+Persistent slot, check after closing, id=max
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE
+
+Persistent slot, check after destroying, id=max
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY
+
+Persistent slot, check after restart, id=max
+persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN
 
 Attempt to overwrite: close before
 create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:CLOSE_BEFORE
@@ -25,14 +34,18 @@ create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:CLOSE_AFTER
 Attempt to overwrite: keep open
 create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:KEEP_OPEN
 
-Open failure: invalid identifier (0)
-depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-open_fail:0:PSA_ERROR_INVALID_ARGUMENT
-
 Open failure: invalid identifier (random seed UID)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT
 
+Open failure: invalid identifier (reserved range)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+open_fail:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_INVALID_ARGUMENT
+
+Open failure: invalid identifier (implementation range)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+open_fail:PSA_KEY_ID_USER_MAX + 1:PSA_ERROR_DOES_NOT_EXIST
+
 Open failure: non-existent identifier
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:1:PSA_ERROR_DOES_NOT_EXIST
@@ -40,14 +53,18 @@ open_fail:1:PSA_ERROR_DOES_NOT_EXIST
 Create failure: invalid lifetime
 create_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT
 
-Create failure: invalid key id (0)
-depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
-create_fail:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_ERROR_INVALID_ARGUMENT
-
 Create failure: invalid key id (random seed UID)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT
 
+Create failure: invalid key id (reserved range)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_INVALID_ARGUMENT
+
+Create failure: invalid key id (implementation range)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX + 1:PSA_ERROR_INVALID_ARGUMENT
+
 Open not supported
 depends_on:!MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:1:PSA_ERROR_NOT_SUPPORTED

From f9fbc38e66fbd6ee0a375e3ff43df2370c3502f1 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Wed, 15 May 2019 18:42:09 +0200
Subject: [PATCH 5/7] Declare key id 0 as invalid

In keeping with other integral types, declare 0 to be an invalid key
identifier.

Documented, implemented and tested.
---
 include/psa/crypto_types.h                        |  4 ++--
 include/psa/crypto_values.h                       |  2 +-
 library/psa_crypto_slot_management.c              | 15 +++++++--------
 .../test_suite_psa_crypto_slot_management.data    |  8 ++++++++
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h
index 44c7c66e0..ced42de1a 100644
--- a/include/psa/crypto_types.h
+++ b/include/psa/crypto_types.h
@@ -107,8 +107,8 @@ typedef uint32_t psa_key_lifetime_t;
  *   #PSA_KEY_ID_USER_MIN to #PSA_KEY_ID_USER_MAX.
  * - Implementations may define additional key identifiers in the range
  *   #PSA_KEY_ID_VENDOR_MIN to #PSA_KEY_ID_VENDOR_MAX.
- * - Key identifiers outside these ranges are reserved for future use
- *   in future versions of this specification.
+ * - 0 is reserved as an invalid key identifier.
+ * - Key identifiers outside these ranges are reserved for future use.
  */
 /* Implementation-specific quirk: The Mbed Crypto library can be built as
  * part of a multi-client service that exposes the PSA Crypto API in each
diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h
index 40172b32d..2ee8839c6 100644
--- a/include/psa/crypto_values.h
+++ b/include/psa/crypto_values.h
@@ -1442,7 +1442,7 @@
 
 /** The minimum value for a key identifier chosen by the application.
  */
-#define PSA_KEY_ID_USER_MIN                     ((psa_key_id_t)0x00000000)
+#define PSA_KEY_ID_USER_MIN                     ((psa_key_id_t)0x00000001)
 /** The minimum value for a key identifier chosen by the application.
  */
 #define PSA_KEY_ID_USER_MAX                     ((psa_key_id_t)0x3fffffff)
diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 2ef70db59..22cac619d 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -185,15 +185,14 @@ static int psa_is_key_id_valid( psa_key_file_id_t file_id,
                                 int vendor_ok )
 {
     psa_app_key_id_t key_id = PSA_KEY_FILE_GET_KEY_ID( file_id );
-    /* Reject high values because the file names are reserved for the
-     * library's internal use. */
-    if( key_id > PSA_MAX_PERSISTENT_KEY_IDENTIFIER )
+    if( PSA_KEY_ID_USER_MIN <= key_id && key_id <= PSA_KEY_ID_USER_MAX )
+        return( 1 );
+    else if( vendor_ok &&
+             PSA_KEY_ID_VENDOR_MIN <= key_id &&
+             key_id <= PSA_KEY_ID_VENDOR_MAX )
+        return( 1 );
+    else
         return( 0 );
-    /* Applications may only create keys in the range
-     * 0..PSA_KEY_ID_USER_MAX. */
-    if( ! vendor_ok && key_id > PSA_KEY_ID_USER_MAX )
-        return( 0 );
-    return( 1 );
 }
 
 /** Declare a slot as persistent and load it from storage.
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data
index 519e81ec7..ecfb37a0c 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.data
+++ b/tests/suites/test_suite_psa_crypto_slot_management.data
@@ -34,6 +34,10 @@ create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:CLOSE_AFTER
 Attempt to overwrite: keep open
 create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:KEEP_OPEN
 
+Open failure: invalid identifier (0)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+open_fail:0:PSA_ERROR_INVALID_ARGUMENT
+
 Open failure: invalid identifier (random seed UID)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 open_fail:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT
@@ -53,6 +57,10 @@ open_fail:1:PSA_ERROR_DOES_NOT_EXIST
 Create failure: invalid lifetime
 create_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT
 
+Create failure: invalid key id (0)
+depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
+create_fail:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_ERROR_INVALID_ARGUMENT
+
 Create failure: invalid key id (random seed UID)
 depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C
 create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT

From e2f62ba9ec89685abe53ca6733df6f4eb246bcbd Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Thu, 16 May 2019 00:31:48 +0200
Subject: [PATCH 6/7] Fix unused variable in builds without storage

---
 library/psa_crypto_slot_management.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index 22cac619d..4f0245c62 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -246,6 +246,7 @@ psa_status_t psa_validate_persistent_key_parameters(
 
 #else /* MBEDTLS_PSA_CRYPTO_STORAGE_C */
     (void) id;
+    (void) creating;
     return( PSA_ERROR_NOT_SUPPORTED );
 #endif /* !MBEDTLS_PSA_CRYPTO_STORAGE_C */
 }

From 280948a32be8e2a145851bc6f368cb47e9e78e92 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Thu, 16 May 2019 15:27:14 +0200
Subject: [PATCH 7/7] Fix copypasta in the documentation of
 PSA_KEY_ID_xxx_{MIN,MAX}

---
 include/psa/crypto_values.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h
index 2ee8839c6..89d85e09e 100644
--- a/include/psa/crypto_values.h
+++ b/include/psa/crypto_values.h
@@ -1443,13 +1443,13 @@
 /** The minimum value for a key identifier chosen by the application.
  */
 #define PSA_KEY_ID_USER_MIN                     ((psa_key_id_t)0x00000001)
-/** The minimum value for a key identifier chosen by the application.
+/** The maximum value for a key identifier chosen by the application.
  */
 #define PSA_KEY_ID_USER_MAX                     ((psa_key_id_t)0x3fffffff)
-/** The minimum value for a key identifier chosen by the application.
+/** The minimum value for a key identifier chosen by the implementation.
  */
 #define PSA_KEY_ID_VENDOR_MIN                   ((psa_key_id_t)0x40000000)
-/** The minimum value for a key identifier chosen by the application.
+/** The maximum value for a key identifier chosen by the implementation.
  */
 #define PSA_KEY_ID_VENDOR_MAX                   ((psa_key_id_t)0x7fffffff)