From c1bfcdda58e4e38ba12a2b74cd514f26eae9fd84 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Mon, 15 Mar 2021 14:48:14 +0000 Subject: [PATCH 1/5] Fix:4162 Return correct error type for invalid key Return PSA_ERROR_INVALID_HANDLE instead of PSA_ERROR_DOES_NOT_EXIST if invalid key is passed for some key operations. Signed-off-by: Maulik Patel --- library/psa_crypto_slot_management.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index dcbee31aa..b7e3442fb 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -305,13 +305,15 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, status = psa_load_persistent_key_into_slot( *p_slot ); if( status != PSA_SUCCESS ) + { psa_wipe_key_slot( *p_slot ); - + if( status == PSA_ERROR_DOES_NOT_EXIST ) + status = PSA_ERROR_INVALID_HANDLE; + } return( status ); #else - return( PSA_ERROR_DOES_NOT_EXIST ); + return( PSA_ERROR_INVALID_HANDLE ); #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ - } psa_status_t psa_unlock_key_slot( psa_key_slot_t *slot ) @@ -399,6 +401,9 @@ psa_status_t psa_open_key( mbedtls_svc_key_id_t key, psa_key_handle_t *handle ) if( status != PSA_SUCCESS ) { *handle = PSA_KEY_HANDLE_INIT; + if( status == PSA_ERROR_INVALID_HANDLE ) + status = PSA_ERROR_DOES_NOT_EXIST; + return( status ); } @@ -423,8 +428,12 @@ psa_status_t psa_close_key( psa_key_handle_t handle ) status = psa_get_and_lock_key_slot_in_memory( handle, &slot ); if( status != PSA_SUCCESS ) - return( status ); + { + if( status == PSA_ERROR_DOES_NOT_EXIST ) + status = PSA_ERROR_INVALID_HANDLE; + return( status ); + } if( slot->lock_count <= 1 ) return( psa_wipe_key_slot( slot ) ); else From f65ad8ccc1937e8365e43d027dc45f80b3028ab9 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Wed, 17 Mar 2021 14:55:45 +0000 Subject: [PATCH 2/5] Update psa_open_key tests for invalid key. Update expected return values of psa_open_key() to PSA_ERROR_DOES_NOT_EXIST for invalid key handle operations. Signed-off-by: Maulik Patel --- tests/suites/test_suite_psa_crypto_slot_management.data | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data index cfac6b4df..d955125cc 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.data +++ b/tests/suites/test_suite_psa_crypto_slot_management.data @@ -89,15 +89,15 @@ create_existent:PSA_KEY_LIFETIME_PERSISTENT:0x3617:1:KEEP_OPEN Open failure: invalid identifier (0) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C -open_fail:0:PSA_ERROR_INVALID_HANDLE +open_fail:0:PSA_ERROR_DOES_NOT_EXIST 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_HANDLE +open_fail:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_DOES_NOT_EXIST Open failure: invalid identifier (reserved range) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C -open_fail:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_INVALID_HANDLE +open_fail:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_DOES_NOT_EXIST Open failure: invalid identifier (implementation range) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C From 02a175009818da6a60bbf71892f311f6cf6d3d3f Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Wed, 17 Mar 2021 15:05:13 +0000 Subject: [PATCH 3/5] Update tests for psa_close_key for invalid key. Update expected return values of psa_close_key() to PSA_ERROR_INVALID_HANDLE for invalid key handle operations. Signed-off-by: Maulik Patel --- tests/suites/test_suite_psa_crypto_slot_management.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function index bafb7d8bf..9b07c8cfd 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.function +++ b/tests/suites/test_suite_psa_crypto_slot_management.function @@ -179,7 +179,7 @@ void transient_slot_lifecycle( int owner_id_arg, /* Test that the key is now invalid. */ TEST_EQUAL( psa_get_key_attributes( key, &attributes ), PSA_ERROR_DOES_NOT_EXIST ); - TEST_EQUAL( psa_close_key( key ), PSA_ERROR_DOES_NOT_EXIST ); + TEST_EQUAL( psa_close_key( key ), PSA_ERROR_INVALID_HANDLE ); exit: /* @@ -327,7 +327,7 @@ void persistent_slot_lifecycle( int lifetime_arg, int owner_id_arg, int id_arg, */ TEST_EQUAL( psa_get_key_attributes( handle, &read_attributes ), PSA_ERROR_DOES_NOT_EXIST ); - TEST_EQUAL( psa_close_key( handle ), PSA_ERROR_DOES_NOT_EXIST ); + TEST_EQUAL( psa_close_key( handle ), PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_get_key_attributes( id, &read_attributes ), PSA_ERROR_DOES_NOT_EXIST ); break; From 3240c9d2eca3a5de015a430781d1c35aee43d59b Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Wed, 17 Mar 2021 16:11:05 +0000 Subject: [PATCH 4/5] Update tests for other invalid key operations. Update expected return values of psa_get_key_attributes(), psa_export_key() and other key api(s) to PSA_ERROR_INVALID_HANDLE for invalid key. Signed-off-by: Maulik Patel --- tests/suites/test_suite_psa_crypto.function | 6 +++--- .../suites/test_suite_psa_crypto_slot_management.data | 8 ++++---- .../test_suite_psa_crypto_slot_management.function | 11 +++++------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 628380e4b..21985e976 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -205,7 +205,7 @@ static int test_operations_on_invalid_key( mbedtls_svc_key_id_t key ) psa_set_key_algorithm( &attributes, PSA_ALG_CTR ); psa_set_key_type( &attributes, PSA_KEY_TYPE_AES ); TEST_EQUAL( psa_get_key_attributes( key, &attributes ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( &attributes ) ), 0 ); TEST_EQUAL( @@ -217,10 +217,10 @@ static int test_operations_on_invalid_key( mbedtls_svc_key_id_t key ) TEST_EQUAL( psa_get_key_bits( &attributes ), 0 ); TEST_EQUAL( psa_export_key( key, buffer, sizeof( buffer ), &length ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_export_public_key( key, buffer, sizeof( buffer ), &length ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); ok = 1; diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data index d955125cc..fc17ad149 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.data +++ b/tests/suites/test_suite_psa_crypto_slot_management.data @@ -178,16 +178,16 @@ depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C copy_to_occupied:PSA_KEY_LIFETIME_PERSISTENT:1:PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_COPY:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"404142434445464748494a4b4c4d4e4f":PSA_KEY_LIFETIME_PERSISTENT:1:PSA_KEY_USAGE_EXPORT:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"404142434445464748494a4b4c4d4e4f" invalid handle: 0 -invalid_handle:INVALID_HANDLE_0:PSA_SUCCESS:PSA_ERROR_INVALID_HANDLE +invalid_handle:INVALID_HANDLE_0:PSA_SUCCESS invalid handle: never opened -invalid_handle:INVALID_HANDLE_UNOPENED:PSA_ERROR_DOES_NOT_EXIST:PSA_ERROR_DOES_NOT_EXIST +invalid_handle:INVALID_HANDLE_UNOPENED:PSA_ERROR_INVALID_HANDLE invalid handle: already closed -invalid_handle:INVALID_HANDLE_CLOSED:PSA_ERROR_DOES_NOT_EXIST:PSA_ERROR_DOES_NOT_EXIST +invalid_handle:INVALID_HANDLE_CLOSED:PSA_ERROR_INVALID_HANDLE invalid handle: huge -invalid_handle:INVALID_HANDLE_HUGE:PSA_ERROR_INVALID_HANDLE:PSA_ERROR_INVALID_HANDLE +invalid_handle:INVALID_HANDLE_HUGE:PSA_ERROR_INVALID_HANDLE Open many transient keys many_transient_keys:42 diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function index 9b07c8cfd..dac52ab49 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.function +++ b/tests/suites/test_suite_psa_crypto_slot_management.function @@ -178,7 +178,7 @@ void transient_slot_lifecycle( int owner_id_arg, /* Test that the key is now invalid. */ TEST_EQUAL( psa_get_key_attributes( key, &attributes ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_close_key( key ), PSA_ERROR_INVALID_HANDLE ); exit: @@ -326,10 +326,10 @@ void persistent_slot_lifecycle( int lifetime_arg, int owner_id_arg, int id_arg, * existing key. */ TEST_EQUAL( psa_get_key_attributes( handle, &read_attributes ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_close_key( handle ), PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_get_key_attributes( id, &read_attributes ), - PSA_ERROR_DOES_NOT_EXIST ); + PSA_ERROR_INVALID_HANDLE ); break; } @@ -728,13 +728,12 @@ exit: /* BEGIN_CASE */ void invalid_handle( int handle_construction, - int close_status_arg, int usage_status_arg ) + int close_status_arg ) { psa_key_handle_t valid_handle = PSA_KEY_HANDLE_INIT; psa_key_handle_t invalid_handle = PSA_KEY_HANDLE_INIT; psa_key_id_t key_id; psa_status_t close_status = close_status_arg; - psa_status_t usage_status = usage_status_arg; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; uint8_t material[1] = "a"; @@ -793,7 +792,7 @@ void invalid_handle( int handle_construction, /* Attempt to use the invalid handle. */ TEST_EQUAL( psa_get_key_attributes( invalid_handle, &attributes ), - usage_status ); + PSA_ERROR_INVALID_HANDLE ); TEST_EQUAL( psa_close_key( invalid_handle ), close_status ); TEST_EQUAL( psa_destroy_key( invalid_handle ), close_status ); From f41be1426955ddc5be2472081b29be138f49faf9 Mon Sep 17 00:00:00 2001 From: Maulik Patel Date: Thu, 18 Mar 2021 16:39:52 +0000 Subject: [PATCH 5/5] Add Change log entry for bug fix. Signed-off-by: Maulik Patel --- ChangeLog.d/fix_return_type_for_invalid_crypto_key.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix_return_type_for_invalid_crypto_key.txt diff --git a/ChangeLog.d/fix_return_type_for_invalid_crypto_key.txt b/ChangeLog.d/fix_return_type_for_invalid_crypto_key.txt new file mode 100644 index 000000000..dc6996e02 --- /dev/null +++ b/ChangeLog.d/fix_return_type_for_invalid_crypto_key.txt @@ -0,0 +1,4 @@ +Bugfix + * PSA functions other than psa_open_key now return PSA_ERROR_INVALID_HANDLE + rather than PSA_ERROR_DOES_NOT_EXIST for an invalid handle, bringing them + in line with version 1.0.0 of the specification. Fix #4162.