From 348410f7097bfffdd73d9c370d2d1eb7a75b9b2c Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 15 Nov 2022 22:22:07 +0100 Subject: [PATCH 01/10] Make a copy of the key in operation while setting pake password Additionally use psa_get_and_lock_key_slot_with_policy() to obtain key. This requires making this function public. This will have to be solved while adding driver dipatch for EC-JPAKE. Signed-off-by: Przemek Stekiel --- include/psa/crypto_extra.h | 5 ++-- library/psa_crypto.c | 2 +- library/psa_crypto_pake.c | 59 +++++++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 4f65398e24..d527e579b6 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -1829,7 +1829,7 @@ psa_status_t psa_pake_abort( psa_pake_operation_t * operation ); */ #if defined(MBEDTLS_PSA_BUILTIN_PAKE) #define PSA_PAKE_OPERATION_INIT {PSA_ALG_NONE, 0, 0, 0, 0, \ - MBEDTLS_SVC_KEY_ID_INIT, \ + NULL, 0 , \ PSA_PAKE_ROLE_NONE, {0}, 0, 0, \ {.dummy = 0}} #else @@ -1920,7 +1920,8 @@ struct psa_pake_operation_s #if defined(MBEDTLS_PSA_BUILTIN_PAKE) unsigned int MBEDTLS_PRIVATE(input_step); unsigned int MBEDTLS_PRIVATE(output_step); - mbedtls_svc_key_id_t MBEDTLS_PRIVATE(password); + uint8_t* MBEDTLS_PRIVATE(password_data); + size_t MBEDTLS_PRIVATE(password_bytes); psa_pake_role_t MBEDTLS_PRIVATE(role); uint8_t MBEDTLS_PRIVATE(buffer[MBEDTLS_PSA_PAKE_BUFFER_SIZE]); size_t MBEDTLS_PRIVATE(buffer_length); diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 2ce5e4320d..55319c4bdb 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -890,7 +890,7 @@ static psa_status_t psa_restrict_key_policy( * On success, the returned key slot is locked. It is the responsibility of * the caller to unlock the key slot when it does not access it anymore. */ -static psa_status_t psa_get_and_lock_key_slot_with_policy( +psa_status_t psa_get_and_lock_key_slot_with_policy( mbedtls_svc_key_id_t key, psa_key_slot_t **p_slot, psa_key_usage_t usage, diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 870b5b5654..1deb48875f 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -33,6 +33,11 @@ #include #include +extern psa_status_t psa_get_and_lock_key_slot_with_policy( + mbedtls_svc_key_id_t key, + psa_key_slot_t **p_slot, + psa_key_usage_t usage, + psa_algorithm_t alg ); /* * State sequence: * @@ -248,6 +253,7 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, psa_key_attributes_t attributes = psa_key_attributes_init(); psa_key_type_t type; psa_key_usage_t usage; + psa_key_slot_t *slot = NULL; if( operation->alg == PSA_ALG_NONE || operation->state != PSA_PAKE_STATE_SETUP ) @@ -255,6 +261,9 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, return( PSA_ERROR_BAD_STATE ); } + if( psa_is_valid_key_id( password, 1 ) == 0 ) + return( PSA_ERROR_BAD_STATE ); + status = psa_get_key_attributes( password, &attributes ); if( status != PSA_SUCCESS ) return( status ); @@ -273,7 +282,33 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, if( ( usage & PSA_KEY_USAGE_DERIVE ) == 0 ) return( PSA_ERROR_NOT_PERMITTED ); - operation->password = password; + status = psa_get_and_lock_key_slot_with_policy( password, &slot, + PSA_KEY_USAGE_DERIVE, + PSA_ALG_JPAKE ); + if( status != PSA_SUCCESS ) + return( status ); + + if( slot->key.data == NULL || slot->key.bytes == 0 ) + return( PSA_ERROR_INVALID_ARGUMENT ); + + if( operation->password_data != NULL ) + { + mbedtls_free( operation->password_data ); + operation->password_bytes = 0; + } + + operation->password_data = mbedtls_calloc( 1, slot->key.bytes ); + if( operation->password_data == NULL ) + { + status = psa_unlock_key_slot( slot ); + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + } + memcpy( operation->password_data, slot->key.data, slot->key.bytes ); + operation->password_bytes = slot->key.bytes; + + status = psa_unlock_key_slot( slot ); + if( status != PSA_SUCCESS ) + return( status ); return( PSA_SUCCESS ); } @@ -348,9 +383,7 @@ psa_status_t psa_pake_set_role( psa_pake_operation_t *operation, static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; mbedtls_ecjpake_role role; - psa_key_slot_t *slot = NULL; if( operation->role == PSA_PAKE_ROLE_CLIENT ) role = MBEDTLS_ECJPAKE_CLIENT; @@ -359,22 +392,18 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) else return( PSA_ERROR_BAD_STATE ); - if( psa_is_valid_key_id( operation->password, 1 ) == 0 ) + if (operation->password_data == NULL || + operation->password_bytes == 0 ) + { return( PSA_ERROR_BAD_STATE ); - - status = psa_get_and_lock_key_slot( operation->password, &slot ); - if( status != PSA_SUCCESS ) - return( status ); - + } ret = mbedtls_ecjpake_setup( &operation->ctx.ecjpake, role, MBEDTLS_MD_SHA256, MBEDTLS_ECP_DP_SECP256R1, - slot->key.data, slot->key.bytes ); - - psa_unlock_key_slot( slot ); - slot = NULL; + operation->password_data, + operation->password_bytes ); if( ret != 0 ) return( mbedtls_ecjpake_to_psa_error( ret ) ); @@ -840,7 +869,9 @@ psa_status_t psa_pake_abort(psa_pake_operation_t * operation) { operation->input_step = PSA_PAKE_STEP_INVALID; operation->output_step = PSA_PAKE_STEP_INVALID; - operation->password = MBEDTLS_SVC_KEY_ID_INIT; + mbedtls_free( operation->password_data ); + operation->password_data = NULL; + operation->password_bytes = 0; operation->role = PSA_PAKE_ROLE_NONE; mbedtls_platform_zeroize( operation->buffer, MBEDTLS_PSA_PAKE_BUFFER_SIZE ); operation->buffer_length = 0; From 7c7954842b6e287f95168a5dafdc00f0491e1675 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 15 Nov 2022 22:26:12 +0100 Subject: [PATCH 02/10] Adapt ec-jpake_setup test Now when operation holds pointer to dynamically allocated buffer for password key we can't do copy of the operation object in test instead we need to re-initialize operation object after error. Signed-off-by: Przemek Stekiel --- tests/suites/test_suite_psa_crypto.function | 54 ++++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 779f594dca..60befa73f4 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -31,6 +31,29 @@ #define ASSERT_OPERATION_IS_ACTIVE( operation ) TEST_ASSERT( operation.id != 0 ) #define ASSERT_OPERATION_IS_INACTIVE( operation ) TEST_ASSERT( operation.id == 0 ) +#if defined(PSA_WANT_ALG_JPAKE) +void ecjpake_operation_setup( psa_pake_operation_t *operation, + psa_pake_cipher_suite_t *cipher_suite, + psa_pake_role_t role, + mbedtls_svc_key_id_t key, + size_t key_available ) +{ + *operation = psa_pake_operation_init(); + + TEST_EQUAL( psa_pake_setup( operation, cipher_suite ), + PSA_SUCCESS ); + + TEST_EQUAL( psa_pake_set_role( operation, role), + PSA_SUCCESS ); + + if( key_available ) + TEST_EQUAL( psa_pake_set_password_key( operation, key ), + PSA_SUCCESS ); +exit: + return; +} +#endif + /** An invalid export length that will never be set by psa_export_key(). */ static const size_t INVALID_EXPORT_LENGTH = ~0U; @@ -8740,7 +8763,6 @@ void ecjpake_setup( int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, { psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); psa_pake_operation_t operation = psa_pake_operation_init(); - psa_pake_operation_t op_copy = psa_pake_operation_init(); psa_algorithm_t alg = alg_arg; psa_pake_primitive_t primitive = primitive_arg; psa_key_type_t key_type_pw = key_type_pw_arg; @@ -8839,22 +8861,25 @@ void ecjpake_setup( int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, if( input_first ) { /* Invalid parameters (input) */ - op_copy = operation; - TEST_EQUAL( psa_pake_input( &op_copy, PSA_PAKE_STEP_ZK_PROOF, + TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_ZK_PROOF, NULL, 0 ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid parameters (step) */ - op_copy = operation; - TEST_EQUAL( psa_pake_input( &op_copy, PSA_PAKE_STEP_ZK_PROOF + 10, + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); + TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_ZK_PROOF + 10, output_buffer, size_zk_proof ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid first step */ - op_copy = operation; - TEST_EQUAL( psa_pake_input( &op_copy, PSA_PAKE_STEP_ZK_PROOF, + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); + TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_ZK_PROOF, output_buffer, size_zk_proof ), PSA_ERROR_BAD_STATE ); /* Possibly valid */ + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_KEY_SHARE, output_buffer, size_key_share ), expected_status_input_output); @@ -8875,22 +8900,25 @@ void ecjpake_setup( int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, else { /* Invalid parameters (output) */ - op_copy = operation; - TEST_EQUAL( psa_pake_output( &op_copy, PSA_PAKE_STEP_ZK_PROOF, + TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_ZK_PROOF, NULL, 0, NULL ), PSA_ERROR_INVALID_ARGUMENT ); - op_copy = operation; /* Invalid parameters (step) */ - TEST_EQUAL( psa_pake_output( &op_copy, PSA_PAKE_STEP_ZK_PROOF + 10, + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); + TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_ZK_PROOF + 10, output_buffer, buf_size, &output_len ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid first step */ - op_copy = operation; - TEST_EQUAL( psa_pake_output( &op_copy, PSA_PAKE_STEP_ZK_PROOF, + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); + TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_ZK_PROOF, output_buffer, buf_size, &output_len ), PSA_ERROR_BAD_STATE ); /* Possibly valid */ + ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ); TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_KEY_SHARE, output_buffer, buf_size, &output_len ), expected_status_input_output ); From 1def5becc285b9150b334d7626e98d413b29a026 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Wed, 16 Nov 2022 12:00:26 +0100 Subject: [PATCH 03/10] Add psa_get_and_lock_key_slot_with_policy to header file Signed-off-by: Przemyslaw Stekiel --- library/psa_crypto_core.h | 8 ++++++++ library/psa_crypto_pake.c | 5 ----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 98638481c8..37f8162de7 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -183,6 +183,14 @@ static inline psa_key_slot_number_t psa_key_slot_get_slot_number( } #endif +/** Get the description of a key given its identifier and policy constraints + * and lock it. + */ +psa_status_t psa_get_and_lock_key_slot_with_policy( mbedtls_svc_key_id_t key, + psa_key_slot_t **p_slot, + psa_key_usage_t usage, + psa_algorithm_t alg ); + /** Completely wipe a slot in memory, including its policy. * * Persistent storage is not affected. diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 1deb48875f..224f922dbc 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -33,11 +33,6 @@ #include #include -extern psa_status_t psa_get_and_lock_key_slot_with_policy( - mbedtls_svc_key_id_t key, - psa_key_slot_t **p_slot, - psa_key_usage_t usage, - psa_algorithm_t alg ); /* * State sequence: * From 152ae07682a7c7630b03fc2337721b4b0a19df01 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Thu, 17 Nov 2022 13:24:36 +0100 Subject: [PATCH 04/10] Change password ec j-pake operation fields to more suitable Signed-off-by: Przemek Stekiel --- include/psa/crypto_extra.h | 4 ++-- library/psa_crypto_pake.c | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index d527e579b6..33e2e77b99 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -1920,8 +1920,8 @@ struct psa_pake_operation_s #if defined(MBEDTLS_PSA_BUILTIN_PAKE) unsigned int MBEDTLS_PRIVATE(input_step); unsigned int MBEDTLS_PRIVATE(output_step); - uint8_t* MBEDTLS_PRIVATE(password_data); - size_t MBEDTLS_PRIVATE(password_bytes); + uint8_t* MBEDTLS_PRIVATE(password); + size_t MBEDTLS_PRIVATE(password_len); psa_pake_role_t MBEDTLS_PRIVATE(role); uint8_t MBEDTLS_PRIVATE(buffer[MBEDTLS_PSA_PAKE_BUFFER_SIZE]); size_t MBEDTLS_PRIVATE(buffer_length); diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 224f922dbc..b89954830f 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -286,20 +286,20 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, if( slot->key.data == NULL || slot->key.bytes == 0 ) return( PSA_ERROR_INVALID_ARGUMENT ); - if( operation->password_data != NULL ) + if( operation->password != NULL ) { - mbedtls_free( operation->password_data ); - operation->password_bytes = 0; + mbedtls_free( operation->password ); + operation->password_len = 0; } - operation->password_data = mbedtls_calloc( 1, slot->key.bytes ); - if( operation->password_data == NULL ) + operation->password = mbedtls_calloc( 1, slot->key.bytes ); + if( operation->password == NULL ) { status = psa_unlock_key_slot( slot ); return( PSA_ERROR_INSUFFICIENT_MEMORY ); } - memcpy( operation->password_data, slot->key.data, slot->key.bytes ); - operation->password_bytes = slot->key.bytes; + memcpy( operation->password, slot->key.data, slot->key.bytes ); + operation->password_len = slot->key.bytes; status = psa_unlock_key_slot( slot ); if( status != PSA_SUCCESS ) @@ -387,8 +387,8 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) else return( PSA_ERROR_BAD_STATE ); - if (operation->password_data == NULL || - operation->password_bytes == 0 ) + if (operation->password == NULL || + operation->password_len == 0 ) { return( PSA_ERROR_BAD_STATE ); } @@ -397,8 +397,8 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) role, MBEDTLS_MD_SHA256, MBEDTLS_ECP_DP_SECP256R1, - operation->password_data, - operation->password_bytes ); + operation->password, + operation->password_len ); if( ret != 0 ) return( mbedtls_ecjpake_to_psa_error( ret ) ); @@ -864,9 +864,9 @@ psa_status_t psa_pake_abort(psa_pake_operation_t * operation) { operation->input_step = PSA_PAKE_STEP_INVALID; operation->output_step = PSA_PAKE_STEP_INVALID; - mbedtls_free( operation->password_data ); - operation->password_data = NULL; - operation->password_bytes = 0; + mbedtls_free( operation->password ); + operation->password = NULL; + operation->password_len = 0; operation->role = PSA_PAKE_ROLE_NONE; mbedtls_platform_zeroize( operation->buffer, MBEDTLS_PSA_PAKE_BUFFER_SIZE ); operation->buffer_length = 0; From 369ae0afc35079979d32b93ba824898a23e1f733 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Thu, 17 Nov 2022 14:14:31 +0100 Subject: [PATCH 05/10] Zeroize pake password buffer before free Signed-off-by: Przemek Stekiel --- library/psa_crypto_pake.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index b89954830f..ef31af4204 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -288,6 +288,7 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, if( operation->password != NULL ) { + mbedtls_platform_zeroize( operation->password, operation->password_len ); mbedtls_free( operation->password ); operation->password_len = 0; } @@ -864,6 +865,7 @@ psa_status_t psa_pake_abort(psa_pake_operation_t * operation) { operation->input_step = PSA_PAKE_STEP_INVALID; operation->output_step = PSA_PAKE_STEP_INVALID; + mbedtls_platform_zeroize( operation->password, operation->password_len ); mbedtls_free( operation->password ); operation->password = NULL; operation->password_len = 0; From cd356c3cdb312e276473e038d1593f6f92bcd5b3 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Sun, 20 Nov 2022 19:05:20 +0100 Subject: [PATCH 06/10] Add ec-jpake test to verify if key can be destroyed after set_password_key Signed-off-by: Przemek Stekiel --- tests/suites/test_suite_psa_crypto.data | 9 +++++++-- tests/suites/test_suite_psa_crypto.function | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index cce3fd0fe8..659205d529 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -6549,11 +6549,16 @@ ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_ PSA PAKE: ecjpake rounds depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS -ecjpake_rounds:PSA_ALG_JPAKE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"abcdef":0 +ecjpake_rounds:PSA_ALG_JPAKE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"abcdef":0:0 PSA PAKE: ecjpake rounds, client input first depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS -ecjpake_rounds:PSA_ALG_JPAKE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"abcdef":1 +ecjpake_rounds:PSA_ALG_JPAKE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"abcdef":1:0 + +# This test case relies on implementation (it may need to be adjusted in the future) +PSA PAKE: ecjpake rounds - key is destroyed after being passed to set_password_key +depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS +ecjpake_rounds:PSA_ALG_JPAKE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"abcdef":0:1 PSA PAKE: ecjpake no input errors depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256 diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 60befa73f4..f84a0cc3f5 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -9002,7 +9002,7 @@ exit: /* BEGIN_CASE depends_on:PSA_WANT_ALG_JPAKE */ void ecjpake_rounds( int alg_arg, int primitive_arg, int hash_arg, int derive_alg_arg, data_t *pw_data, - int client_input_first ) + int client_input_first, int destroy_key ) { psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); psa_pake_operation_t server = psa_pake_operation_init(); @@ -9053,6 +9053,9 @@ void ecjpake_rounds( int alg_arg, int primitive_arg, int hash_arg, PSA_ASSERT( psa_pake_set_password_key( &server, key ) ); PSA_ASSERT( psa_pake_set_password_key( &client, key ) ); + if( destroy_key == 1 ) + psa_destroy_key( key ); + TEST_EQUAL( psa_pake_get_implicit_key( &server, &server_derive ), PSA_ERROR_BAD_STATE ); TEST_EQUAL( psa_pake_get_implicit_key( &client, &client_derive ), From e2d6b5f45b207efa6745cfdbf73332e7403bb5b8 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 21 Nov 2022 15:03:52 +0100 Subject: [PATCH 07/10] psa_key_slot_get_slot_number: Move documentation to header file Signed-off-by: Przemek Stekiel --- library/psa_crypto.c | 13 ------------- library/psa_crypto_core.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 55319c4bdb..8c9deffadf 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -877,19 +877,6 @@ static psa_status_t psa_restrict_key_policy( return( PSA_SUCCESS ); } -/** Get the description of a key given its identifier and policy constraints - * and lock it. - * - * The key must have allow all the usage flags set in \p usage. If \p alg is - * nonzero, the key must allow operations with this algorithm. If \p alg is - * zero, the algorithm is not checked. - * - * In case of a persistent key, the function loads the description of the key - * into a key slot if not already done. - * - * On success, the returned key slot is locked. It is the responsibility of - * the caller to unlock the key slot when it does not access it anymore. - */ psa_status_t psa_get_and_lock_key_slot_with_policy( mbedtls_svc_key_id_t key, psa_key_slot_t **p_slot, diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 37f8162de7..5cefa273aa 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -185,6 +185,16 @@ static inline psa_key_slot_number_t psa_key_slot_get_slot_number( /** Get the description of a key given its identifier and policy constraints * and lock it. + * + * The key must have allow all the usage flags set in \p usage. If \p alg is + * nonzero, the key must allow operations with this algorithm. If \p alg is + * zero, the algorithm is not checked. + * + * In case of a persistent key, the function loads the description of the key + * into a key slot if not already done. + * + * On success, the returned key slot is locked. It is the responsibility of + * the caller to unlock the key slot when it does not access it anymore. */ psa_status_t psa_get_and_lock_key_slot_with_policy( mbedtls_svc_key_id_t key, psa_key_slot_t **p_slot, From ad0f357178448f9483c572b26b32345d182a99b4 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 21 Nov 2022 15:04:37 +0100 Subject: [PATCH 08/10] Optimize pake code that sets/use password key Signed-off-by: Przemek Stekiel --- library/psa_crypto_pake.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index ef31af4204..9ac4c5f291 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -256,9 +256,6 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, return( PSA_ERROR_BAD_STATE ); } - if( psa_is_valid_key_id( password, 1 ) == 0 ) - return( PSA_ERROR_BAD_STATE ); - status = psa_get_key_attributes( password, &attributes ); if( status != PSA_SUCCESS ) return( status ); @@ -283,15 +280,8 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - if( slot->key.data == NULL || slot->key.bytes == 0 ) - return( PSA_ERROR_INVALID_ARGUMENT ); - if( operation->password != NULL ) - { - mbedtls_platform_zeroize( operation->password, operation->password_len ); - mbedtls_free( operation->password ); - operation->password_len = 0; - } + return( PSA_ERROR_BAD_STATE ); operation->password = mbedtls_calloc( 1, slot->key.bytes ); if( operation->password == NULL ) @@ -388,11 +378,8 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) else return( PSA_ERROR_BAD_STATE ); - if (operation->password == NULL || - operation->password_len == 0 ) - { + if( operation->password_len == 0 ) return( PSA_ERROR_BAD_STATE ); - } ret = mbedtls_ecjpake_setup( &operation->ctx.ecjpake, role, @@ -404,6 +391,11 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) if( ret != 0 ) return( mbedtls_ecjpake_to_psa_error( ret ) ); + mbedtls_platform_zeroize( operation->password, operation->password_len ); + mbedtls_free( operation->password ); + operation->password = NULL; + operation->password_len = 0; + operation->state = PSA_PAKE_STATE_READY; return( PSA_SUCCESS ); @@ -453,7 +445,13 @@ static psa_status_t psa_pake_output_internal( if( operation->state == PSA_PAKE_STATE_SETUP ) { status = psa_pake_ecjpake_setup( operation ); if( status != PSA_SUCCESS ) + { + mbedtls_platform_zeroize( operation->password, operation->password_len ); + mbedtls_free( operation->password ); + operation->password = NULL; + operation->password_len = 0; return( status ); + } } if( operation->state != PSA_PAKE_STATE_READY && @@ -661,7 +659,13 @@ static psa_status_t psa_pake_input_internal( { status = psa_pake_ecjpake_setup( operation ); if( status != PSA_SUCCESS ) + { + mbedtls_platform_zeroize( operation->password, operation->password_len ); + mbedtls_free( operation->password ); + operation->password = NULL; + operation->password_len = 0; return( status ); + } } if( operation->state != PSA_PAKE_STATE_READY && @@ -865,7 +869,8 @@ psa_status_t psa_pake_abort(psa_pake_operation_t * operation) { operation->input_step = PSA_PAKE_STEP_INVALID; operation->output_step = PSA_PAKE_STEP_INVALID; - mbedtls_platform_zeroize( operation->password, operation->password_len ); + if( operation->password_len > 0 ) + mbedtls_platform_zeroize( operation->password, operation->password_len ); mbedtls_free( operation->password ); operation->password = NULL; operation->password_len = 0; From f82effa9826a0e93aaa8c4c7928ad1016a16a8e8 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 21 Nov 2022 15:10:32 +0100 Subject: [PATCH 09/10] Optimize pake test code Signed-off-by: Przemek Stekiel --- tests/suites/test_suite_psa_crypto.function | 40 ++++++++++----------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index f84a0cc3f5..ca1614befa 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -32,25 +32,23 @@ #define ASSERT_OPERATION_IS_INACTIVE( operation ) TEST_ASSERT( operation.id == 0 ) #if defined(PSA_WANT_ALG_JPAKE) -void ecjpake_operation_setup( psa_pake_operation_t *operation, +int ecjpake_operation_setup( psa_pake_operation_t *operation, psa_pake_cipher_suite_t *cipher_suite, psa_pake_role_t role, mbedtls_svc_key_id_t key, size_t key_available ) { - *operation = psa_pake_operation_init(); + PSA_ASSERT( psa_pake_abort( operation ) ); - TEST_EQUAL( psa_pake_setup( operation, cipher_suite ), - PSA_SUCCESS ); + PSA_ASSERT( psa_pake_setup( operation, cipher_suite ) ); - TEST_EQUAL( psa_pake_set_role( operation, role), - PSA_SUCCESS ); + PSA_ASSERT( psa_pake_set_role( operation, role) ); if( key_available ) - TEST_EQUAL( psa_pake_set_password_key( operation, key ), - PSA_SUCCESS ); + PSA_ASSERT( psa_pake_set_password_key( operation, key ) ); + return 0; exit: - return; + return 1; } #endif @@ -8865,21 +8863,21 @@ void ecjpake_setup( int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, NULL, 0 ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid parameters (step) */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ) , 0 ); TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_ZK_PROOF + 10, output_buffer, size_zk_proof ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid first step */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ), 0 ); TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_ZK_PROOF, output_buffer, size_zk_proof ), PSA_ERROR_BAD_STATE ); /* Possibly valid */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ), 0 ); TEST_EQUAL( psa_pake_input( &operation, PSA_PAKE_STEP_KEY_SHARE, output_buffer, size_key_share ), expected_status_input_output); @@ -8904,21 +8902,21 @@ void ecjpake_setup( int alg_arg, int key_type_pw_arg, int key_usage_pw_arg, NULL, 0, NULL ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid parameters (step) */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ), 0 ); TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_ZK_PROOF + 10, output_buffer, buf_size, &output_len ), PSA_ERROR_INVALID_ARGUMENT ); /* Invalid first step */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ), 0 ); TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_ZK_PROOF, output_buffer, buf_size, &output_len ), PSA_ERROR_BAD_STATE ); /* Possibly valid */ - ecjpake_operation_setup( &operation, &cipher_suite, role, - key, pw_data->len ); + TEST_EQUAL( ecjpake_operation_setup( &operation, &cipher_suite, role, + key, pw_data->len ), 0 ); TEST_EQUAL( psa_pake_output( &operation, PSA_PAKE_STEP_KEY_SHARE, output_buffer, buf_size, &output_len ), expected_status_input_output ); From 0bdec19c93a2aacf023c46bb81e3ce0fb8cc6baa Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 22 Nov 2022 09:10:35 +0100 Subject: [PATCH 10/10] Further optimizations of pake set_password implementation Signed-off-by: Przemek Stekiel --- library/psa_crypto_pake.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 9ac4c5f291..659b712a5b 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -274,19 +274,19 @@ psa_status_t psa_pake_set_password_key( psa_pake_operation_t *operation, if( ( usage & PSA_KEY_USAGE_DERIVE ) == 0 ) return( PSA_ERROR_NOT_PERMITTED ); + if( operation->password != NULL ) + return( PSA_ERROR_BAD_STATE ); + status = psa_get_and_lock_key_slot_with_policy( password, &slot, PSA_KEY_USAGE_DERIVE, PSA_ALG_JPAKE ); if( status != PSA_SUCCESS ) return( status ); - if( operation->password != NULL ) - return( PSA_ERROR_BAD_STATE ); - operation->password = mbedtls_calloc( 1, slot->key.bytes ); if( operation->password == NULL ) { - status = psa_unlock_key_slot( slot ); + psa_unlock_key_slot( slot ); return( PSA_ERROR_INSUFFICIENT_MEMORY ); } memcpy( operation->password, slot->key.data, slot->key.bytes ); @@ -388,14 +388,14 @@ static psa_status_t psa_pake_ecjpake_setup( psa_pake_operation_t *operation ) operation->password, operation->password_len ); - if( ret != 0 ) - return( mbedtls_ecjpake_to_psa_error( ret ) ); - mbedtls_platform_zeroize( operation->password, operation->password_len ); mbedtls_free( operation->password ); operation->password = NULL; operation->password_len = 0; + if( ret != 0 ) + return( mbedtls_ecjpake_to_psa_error( ret ) ); + operation->state = PSA_PAKE_STATE_READY; return( PSA_SUCCESS ); @@ -445,13 +445,7 @@ static psa_status_t psa_pake_output_internal( if( operation->state == PSA_PAKE_STATE_SETUP ) { status = psa_pake_ecjpake_setup( operation ); if( status != PSA_SUCCESS ) - { - mbedtls_platform_zeroize( operation->password, operation->password_len ); - mbedtls_free( operation->password ); - operation->password = NULL; - operation->password_len = 0; return( status ); - } } if( operation->state != PSA_PAKE_STATE_READY && @@ -659,13 +653,7 @@ static psa_status_t psa_pake_input_internal( { status = psa_pake_ecjpake_setup( operation ); if( status != PSA_SUCCESS ) - { - mbedtls_platform_zeroize( operation->password, operation->password_len ); - mbedtls_free( operation->password ); - operation->password = NULL; - operation->password_len = 0; return( status ); - } } if( operation->state != PSA_PAKE_STATE_READY &&