From 1f778bcfd8665eed8ff6f0b723731f93b23f3515 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 21 Mar 2023 16:48:22 +0100 Subject: [PATCH 1/9] EC-JPAKE: remove limitation for user/peer (alow any value) Signed-off-by: Przemek Stekiel --- include/psa/crypto_extra.h | 2 -- library/psa_crypto.c | 35 ----------------------------------- 2 files changed, 37 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 4920508d7..0c097b232 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -1562,7 +1562,6 @@ psa_status_t psa_pake_set_password_key(psa_pake_operation_t *operation, * been set (psa_pake_set_user() hasn't been * called yet). * \param[in] user_id The user ID to authenticate with. - * (temporary limitation: "client" or "server" only) * \param user_id_len Size of the \p user_id buffer in bytes. * * \retval #PSA_SUCCESS @@ -1604,7 +1603,6 @@ psa_status_t psa_pake_set_user(psa_pake_operation_t *operation, * been set (psa_pake_set_peer() hasn't been * called yet). * \param[in] peer_id The peer's ID to authenticate. - * (temporary limitation: "client" or "server" only) * \param peer_id_len Size of the \p peer_id buffer in bytes. * * \retval #PSA_SUCCESS diff --git a/library/psa_crypto.c b/library/psa_crypto.c index bc19ed07c..540ae46af 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -90,10 +90,6 @@ #define BUILTIN_ALG_ANY_HKDF 1 #endif -/* The only two JPAKE user/peer identifiers supported for the time being. */ -static const uint8_t jpake_server_id[] = { 's', 'e', 'r', 'v', 'e', 'r' }; -static const uint8_t jpake_client_id[] = { 'c', 'l', 'i', 'e', 'n', 't' }; - /****************************************************************/ /* Global data, support functions and library management */ /****************************************************************/ @@ -7420,15 +7416,6 @@ psa_status_t psa_pake_set_user( goto exit; } - /* Allow only "client" or "server" values (temporary restriction). */ - if ((user_id_len != sizeof(jpake_server_id) || - memcmp(user_id, jpake_server_id, user_id_len) != 0) && - (user_id_len != sizeof(jpake_client_id) || - memcmp(user_id, jpake_client_id, user_id_len) != 0)) { - status = PSA_ERROR_NOT_SUPPORTED; - goto exit; - } - operation->data.inputs.user = mbedtls_calloc(1, user_id_len); if (operation->data.inputs.user == NULL) { status = PSA_ERROR_INSUFFICIENT_MEMORY; @@ -7466,15 +7453,6 @@ psa_status_t psa_pake_set_peer( goto exit; } - /* Allow only "client" or "server" values (temporary restriction). */ - if ((peer_id_len != sizeof(jpake_server_id) || - memcmp(peer_id, jpake_server_id, peer_id_len) != 0) && - (peer_id_len != sizeof(jpake_client_id) || - memcmp(peer_id, jpake_client_id, peer_id_len) != 0)) { - status = PSA_ERROR_NOT_SUPPORTED; - goto exit; - } - operation->data.inputs.peer = mbedtls_calloc(1, peer_id_len); if (operation->data.inputs.peer == NULL) { status = PSA_ERROR_INSUFFICIENT_MEMORY; @@ -7592,19 +7570,6 @@ static psa_status_t psa_pake_complete_inputs( if (inputs.user_len == 0 || inputs.peer_len == 0) { return PSA_ERROR_BAD_STATE; } - if (memcmp(inputs.user, jpake_client_id, inputs.user_len) == 0 && - memcmp(inputs.peer, jpake_server_id, inputs.peer_len) == 0) { - inputs.role = PSA_PAKE_ROLE_CLIENT; - } else - if (memcmp(inputs.user, jpake_server_id, inputs.user_len) == 0 && - memcmp(inputs.peer, jpake_client_id, inputs.peer_len) == 0) { - inputs.role = PSA_PAKE_ROLE_SERVER; - } - - if (inputs.role != PSA_PAKE_ROLE_CLIENT && - inputs.role != PSA_PAKE_ROLE_SERVER) { - return PSA_ERROR_NOT_SUPPORTED; - } } /* Clear driver context */ From e80ec0a9af680c1d28c310e5031fe7ab66bd13d9 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 21 Mar 2023 16:49:43 +0100 Subject: [PATCH 2/9] Adapt J-PAKE built-in impl to use user/peer Signed-off-by: Przemek Stekiel --- include/psa/crypto_builtin_composites.h | 2 +- library/psa_crypto_pake.c | 77 +++++++++++++++++++++---- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/include/psa/crypto_builtin_composites.h b/include/psa/crypto_builtin_composites.h index 932c50366..acda24274 100644 --- a/include/psa/crypto_builtin_composites.h +++ b/include/psa/crypto_builtin_composites.h @@ -199,7 +199,7 @@ typedef struct { uint8_t *MBEDTLS_PRIVATE(password); size_t MBEDTLS_PRIVATE(password_len); #if defined(MBEDTLS_PSA_BUILTIN_ALG_JPAKE) - uint8_t MBEDTLS_PRIVATE(role); + mbedtls_ecjpake_role MBEDTLS_PRIVATE(role); uint8_t MBEDTLS_PRIVATE(buffer[MBEDTLS_PSA_JPAKE_BUFFER_SIZE]); size_t MBEDTLS_PRIVATE(buffer_length); size_t MBEDTLS_PRIVATE(buffer_offset); diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index a53718496..97aafb457 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -168,13 +168,11 @@ static psa_status_t mbedtls_ecjpake_to_psa_error(int ret) static psa_status_t psa_pake_ecjpake_setup(mbedtls_psa_pake_operation_t *operation) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - mbedtls_ecjpake_role role = (operation->role == PSA_PAKE_ROLE_CLIENT) ? - MBEDTLS_ECJPAKE_CLIENT : MBEDTLS_ECJPAKE_SERVER; mbedtls_ecjpake_init(&operation->ctx.jpake); ret = mbedtls_ecjpake_setup(&operation->ctx.jpake, - role, + operation->role, MBEDTLS_MD_SHA256, MBEDTLS_ECP_DP_SECP256R1, operation->password, @@ -190,21 +188,30 @@ static psa_status_t psa_pake_ecjpake_setup(mbedtls_psa_pake_operation_t *operati } #endif +/* The only two JPAKE user/peer identifiers supported in built-in implementation. */ +static const uint8_t jpake_server_id[] = { 's', 'e', 'r', 'v', 'e', 'r' }; +static const uint8_t jpake_client_id[] = { 'c', 'l', 'i', 'e', 'n', 't' }; + psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, const psa_crypto_driver_pake_inputs_t *inputs) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - size_t password_len = 0; - psa_pake_role_t role = PSA_PAKE_ROLE_NONE; + size_t user_len = 0, peer_len = 0, password_len = 0; + uint8_t *peer = NULL, *user = NULL; + size_t actual_user_len = 0, actual_peer_len = 0, actual_password_len = 0; psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); - size_t actual_password_len = 0; status = psa_crypto_driver_pake_get_password_len(inputs, &password_len); if (status != PSA_SUCCESS) { return status; } - status = psa_crypto_driver_pake_get_role(inputs, &role); + psa_crypto_driver_pake_get_user_len(inputs, &user_len); + if (status != PSA_SUCCESS) { + return status; + } + + psa_crypto_driver_pake_get_peer_len(inputs, &peer_len); if (status != PSA_SUCCESS) { return status; } @@ -216,7 +223,20 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, operation->password = mbedtls_calloc(1, password_len); if (operation->password == NULL) { - return PSA_ERROR_INSUFFICIENT_MEMORY; + status = PSA_ERROR_INSUFFICIENT_MEMORY; + goto error; + } + + user = mbedtls_calloc(1, user_len); + if (user == NULL) { + status = PSA_ERROR_INSUFFICIENT_MEMORY; + goto error; + } + + peer = mbedtls_calloc(1, peer_len); + if (peer == NULL) { + status = PSA_ERROR_INSUFFICIENT_MEMORY; + goto error; } status = psa_crypto_driver_pake_get_password(inputs, operation->password, @@ -225,6 +245,18 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, goto error; } + status = psa_crypto_driver_pake_get_user(inputs, user, + user_len, &actual_user_len); + if (status != PSA_SUCCESS) { + goto error; + } + + status = psa_crypto_driver_pake_get_peer(inputs, peer, + peer_len, &actual_peer_len); + if (status != PSA_SUCCESS) { + goto error; + } + operation->password_len = actual_password_len; operation->alg = cipher_suite.algorithm; @@ -238,7 +270,27 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, goto error; } - operation->role = role; + const size_t user_peer_len = sizeof(jpake_client_id); // client and server have the same length + if (actual_user_len != user_peer_len || + actual_peer_len != user_peer_len) { + status = PSA_ERROR_NOT_SUPPORTED; + goto error; + } + + if (memcmp(user, jpake_client_id, actual_user_len) == 0 && + memcmp(peer, jpake_server_id, actual_peer_len) == 0) { + operation->role = MBEDTLS_ECJPAKE_CLIENT; + } else + if (memcmp(user, jpake_server_id, actual_user_len) == 0 && + memcmp(peer, jpake_client_id, actual_peer_len) == 0) { + operation->role = MBEDTLS_ECJPAKE_SERVER; + } else { + status = PSA_ERROR_NOT_SUPPORTED; + goto error; + } + + /* Role has been set, release user/peer buffers. */ + mbedtls_free(user); mbedtls_free(peer); operation->buffer_length = 0; operation->buffer_offset = 0; @@ -257,6 +309,7 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, { status = PSA_ERROR_NOT_SUPPORTED; } error: + mbedtls_free(user); mbedtls_free(peer); /* In case of failure of the setup of a multipart operation, the PSA driver interface * specifies that the core does not call any other driver entry point thus does not * call mbedtls_psa_pake_abort(). Therefore call it here to do the needed clean @@ -332,7 +385,7 @@ static psa_status_t mbedtls_psa_pake_output_internal( * information is already available. */ if (step == PSA_JPAKE_X2S_STEP_KEY_SHARE && - operation->role == PSA_PAKE_ROLE_SERVER) { + operation->role == MBEDTLS_ECJPAKE_SERVER) { /* Skip ECParameters, with is 3 bytes (RFC 8422) */ operation->buffer_offset += 3; } @@ -423,7 +476,7 @@ static psa_status_t mbedtls_psa_pake_input_internal( * we're a client. */ if (step == PSA_JPAKE_X4S_STEP_KEY_SHARE && - operation->role == PSA_PAKE_ROLE_CLIENT) { + operation->role == MBEDTLS_ECJPAKE_CLIENT) { /* We only support secp256r1. */ /* This is the ECParameters structure defined by RFC 8422. */ unsigned char ecparameters[3] = { @@ -541,7 +594,7 @@ psa_status_t mbedtls_psa_pake_abort(mbedtls_psa_pake_operation_t *operation) #if defined(MBEDTLS_PSA_BUILTIN_ALG_JPAKE) if (operation->alg == PSA_ALG_JPAKE) { - operation->role = PSA_PAKE_ROLE_NONE; + operation->role = 0; mbedtls_platform_zeroize(operation->buffer, sizeof(operation->buffer)); operation->buffer_length = 0; operation->buffer_offset = 0; From 43af7c8a8ad9a1b8ff2a4e905c169a66aa64f909 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 21 Mar 2023 16:50:48 +0100 Subject: [PATCH 3/9] Adapt pake tests Signed-off-by: Przemek Stekiel --- tests/suites/test_suite_psa_crypto_pake.data | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_pake.data b/tests/suites/test_suite_psa_crypto_pake.data index 62157037d..6f704b4e4 100644 --- a/tests/suites/test_suite_psa_crypto_pake.data +++ b/tests/suites/test_suite_psa_crypto_pake.data @@ -48,11 +48,11 @@ ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_ PSA PAKE: set invalid user depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256 -ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:"aaaa":"server":0:ERR_IN_SET_USER:PSA_ERROR_NOT_SUPPORTED +ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:"something":"server":0:ERR_IN_OUTPUT:PSA_ERROR_NOT_SUPPORTED PSA PAKE: set invalid peer depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256 -ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:"client":"aaaa":0:ERR_IN_SET_PEER:PSA_ERROR_NOT_SUPPORTED +ecjpake_setup:PSA_ALG_JPAKE:PSA_KEY_TYPE_PASSWORD:PSA_KEY_USAGE_DERIVE:PSA_PAKE_PRIMITIVE(PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256):PSA_ALG_SHA_256:"client":"something":0:ERR_IN_OUTPUT:PSA_ERROR_NOT_SUPPORTED PSA PAKE: user already set depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256 From b175b146a24a561228dae3eb24c215786ada3343 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 22 Mar 2023 08:37:40 +0100 Subject: [PATCH 4/9] Remove driver_pake_get_role function Signed-off-by: Przemek Stekiel --- docs/proposed/psa-driver-interface.md | 4 --- include/psa/crypto_extra.h | 15 -------- library/psa_crypto.c | 13 ------- tests/suites/test_suite_psa_crypto_pake.data | 3 -- .../test_suite_psa_crypto_pake.function | 35 ------------------- 5 files changed, 70 deletions(-) diff --git a/docs/proposed/psa-driver-interface.md b/docs/proposed/psa-driver-interface.md index 0027ec766..17b4f7787 100644 --- a/docs/proposed/psa-driver-interface.md +++ b/docs/proposed/psa-driver-interface.md @@ -390,10 +390,6 @@ psa_status_t psa_crypto_driver_pake_get_peer( const psa_crypto_driver_pake_inputs_t *inputs, uint8_t *peer_id, size_t peer_id_size, size_t *peer_id_length); -psa_status_t psa_crypto_driver_pake_get_role( -    const psa_crypto_driver_pake_inputs_t *inputs, -    psa_pake_role_t *role); - psa_status_t psa_crypto_driver_pake_get_cipher_suite(     const psa_crypto_driver_pake_inputs_t *inputs,     psa_pake_cipher_suite_t *cipher_suite); diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 0c097b232..cd530cffb 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -1330,20 +1330,6 @@ psa_status_t psa_crypto_driver_pake_get_password( const psa_crypto_driver_pake_inputs_t *inputs, uint8_t *buffer, size_t buffer_size, size_t *buffer_length); -/** Get the role from given inputs. - * - * \param[in] inputs Operation inputs. - * \param[out] role Return buffer for role. - * - * \retval #PSA_SUCCESS - * Success. - * \retval #PSA_ERROR_BAD_STATE - * Role hasn't been set yet. - */ -psa_status_t psa_crypto_driver_pake_get_role( - const psa_crypto_driver_pake_inputs_t *inputs, - psa_pake_role_t *role); - /** Get the length of the user id in bytes from given inputs. * * \param[in] inputs Operation inputs. @@ -2033,7 +2019,6 @@ static inline void psa_pake_cs_set_hash(psa_pake_cipher_suite_t *cipher_suite, struct psa_crypto_driver_pake_inputs_s { uint8_t *MBEDTLS_PRIVATE(password); size_t MBEDTLS_PRIVATE(password_len); - psa_pake_role_t MBEDTLS_PRIVATE(role); uint8_t *MBEDTLS_PRIVATE(user); size_t MBEDTLS_PRIVATE(user_len); uint8_t *MBEDTLS_PRIVATE(peer); diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 540ae46af..2d53bf698 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7211,19 +7211,6 @@ psa_status_t psa_crypto_driver_pake_get_password( return PSA_SUCCESS; } -psa_status_t psa_crypto_driver_pake_get_role( - const psa_crypto_driver_pake_inputs_t *inputs, - psa_pake_role_t *role) -{ - if (inputs->role == PSA_PAKE_ROLE_NONE) { - return PSA_ERROR_BAD_STATE; - } - - *role = inputs->role; - - return PSA_SUCCESS; -} - psa_status_t psa_crypto_driver_pake_get_user_len( const psa_crypto_driver_pake_inputs_t *inputs, size_t *user_len) diff --git a/tests/suites/test_suite_psa_crypto_pake.data b/tests/suites/test_suite_psa_crypto_pake.data index 6f704b4e4..77faebb26 100644 --- a/tests/suites/test_suite_psa_crypto_pake.data +++ b/tests/suites/test_suite_psa_crypto_pake.data @@ -216,9 +216,6 @@ pake_input_getters_password PSA PAKE: input getters: cipher suite pake_input_getters_cipher_suite -PSA PAKE: input getters: role -pake_input_getters_role - PSA PAKE: input getters: user pake_input_getters_user diff --git a/tests/suites/test_suite_psa_crypto_pake.function b/tests/suites/test_suite_psa_crypto_pake.function index 88f24dd55..85e7b7406 100644 --- a/tests/suites/test_suite_psa_crypto_pake.function +++ b/tests/suites/test_suite_psa_crypto_pake.function @@ -1025,41 +1025,6 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:PSA_WANT_ALG_JPAKE */ -void pake_input_getters_role() -{ - psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); - psa_pake_operation_t operation = psa_pake_operation_init(); - psa_pake_role_t role_ret = PSA_PAKE_ROLE_NONE; - - psa_pake_primitive_t primitive = PSA_PAKE_PRIMITIVE( - PSA_PAKE_PRIMITIVE_TYPE_ECC, - PSA_ECC_FAMILY_SECP_R1, 256); - - PSA_INIT(); - - psa_pake_cs_set_algorithm(&cipher_suite, PSA_ALG_JPAKE); - psa_pake_cs_set_primitive(&cipher_suite, primitive); - psa_pake_cs_set_hash(&cipher_suite, PSA_ALG_SHA_256); - - PSA_ASSERT(psa_pake_setup(&operation, &cipher_suite)); - - TEST_EQUAL(psa_crypto_driver_pake_get_role(&operation.data.inputs, &role_ret), - PSA_ERROR_BAD_STATE); - - /* Role can not be set directly using psa_pake_set_role(). It is set by the core - based on given user/peer identifiers. Simulate that Role is already set. */ - operation.data.inputs.role = PSA_PAKE_ROLE_SERVER; - TEST_EQUAL(psa_crypto_driver_pake_get_role(&operation.data.inputs, &role_ret), - PSA_SUCCESS); - - TEST_EQUAL(role_ret, PSA_PAKE_ROLE_SERVER); -exit: - PSA_ASSERT(psa_pake_abort(&operation)); - PSA_DONE(); -} -/* END_CASE */ - /* BEGIN_CASE depends_on:PSA_WANT_ALG_JPAKE */ void pake_input_getters_user() { From 038a3a6b95ec5d2d322e3be3f6186e0c24c6caef Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 22 Mar 2023 09:41:03 +0100 Subject: [PATCH 5/9] Extend j-pake input getters tests for user and peer Signed-off-by: Przemek Stekiel --- .../test_suite_psa_crypto_pake.function | 112 ++++++++++-------- 1 file changed, 62 insertions(+), 50 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_pake.function b/tests/suites/test_suite_psa_crypto_pake.function index 85e7b7406..acb69bead 100644 --- a/tests/suites/test_suite_psa_crypto_pake.function +++ b/tests/suites/test_suite_psa_crypto_pake.function @@ -1030,8 +1030,7 @@ void pake_input_getters_user() { psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); psa_pake_operation_t operation = psa_pake_operation_init(); - const uint8_t user[] = { 's', 'e', 'r', 'v', 'e', 'r' }; - const size_t user_len = sizeof(user); + const char *users[] = { "client", "server", "other" }; uint8_t user_ret[20] = { 0 }; // max user length is 20 bytes size_t user_len_ret = 0; size_t buffer_len_ret = 0; @@ -1046,37 +1045,44 @@ void pake_input_getters_user() psa_pake_cs_set_primitive(&cipher_suite, primitive); psa_pake_cs_set_hash(&cipher_suite, PSA_ALG_SHA_256); - PSA_ASSERT(psa_pake_setup(&operation, &cipher_suite)); + for (size_t i = 0; i < ARRAY_LENGTH(users); i++) { + uint8_t *user = (uint8_t *) users[i]; + uint8_t user_len = strlen(users[i]); - TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, - (uint8_t *) &user_ret, - 10, &buffer_len_ret), - PSA_ERROR_BAD_STATE); + PSA_ASSERT(psa_pake_abort(&operation)); - TEST_EQUAL(psa_crypto_driver_pake_get_user_len(&operation.data.inputs, &user_len_ret), - PSA_ERROR_BAD_STATE); + PSA_ASSERT(psa_pake_setup(&operation, &cipher_suite)); - PSA_ASSERT(psa_pake_set_user(&operation, user, user_len)); + TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, + (uint8_t *) &user_ret, + 10, &buffer_len_ret), + PSA_ERROR_BAD_STATE); - TEST_EQUAL(psa_crypto_driver_pake_get_user_len(&operation.data.inputs, &user_len_ret), - PSA_SUCCESS); + TEST_EQUAL(psa_crypto_driver_pake_get_user_len(&operation.data.inputs, &user_len_ret), + PSA_ERROR_BAD_STATE); - TEST_EQUAL(user_len_ret, user_len); + PSA_ASSERT(psa_pake_set_user(&operation, user, user_len)); - TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, - (uint8_t *) &user_ret, - user_len_ret - 1, - &buffer_len_ret), - PSA_ERROR_BUFFER_TOO_SMALL); + TEST_EQUAL(psa_crypto_driver_pake_get_user_len(&operation.data.inputs, &user_len_ret), + PSA_SUCCESS); - TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, - (uint8_t *) &user_ret, - user_len_ret, - &buffer_len_ret), - PSA_SUCCESS); + TEST_EQUAL(user_len_ret, user_len); - TEST_EQUAL(buffer_len_ret, user_len); - PSA_ASSERT(memcmp(user_ret, user, buffer_len_ret)); + TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, + (uint8_t *) &user_ret, + user_len_ret - 1, + &buffer_len_ret), + PSA_ERROR_BUFFER_TOO_SMALL); + + TEST_EQUAL(psa_crypto_driver_pake_get_user(&operation.data.inputs, + (uint8_t *) &user_ret, + user_len_ret, + &buffer_len_ret), + PSA_SUCCESS); + + TEST_EQUAL(buffer_len_ret, user_len); + PSA_ASSERT(memcmp(user_ret, user, buffer_len_ret)); + } exit: PSA_ASSERT(psa_pake_abort(&operation)); PSA_DONE(); @@ -1088,8 +1094,7 @@ void pake_input_getters_peer() { psa_pake_cipher_suite_t cipher_suite = psa_pake_cipher_suite_init(); psa_pake_operation_t operation = psa_pake_operation_init(); - const uint8_t peer[] = { 's', 'e', 'r', 'v', 'e', 'r' }; - const size_t peer_len = sizeof(peer); + const char *peers[] = { "client", "server", "other" }; uint8_t peer_ret[20] = { 0 }; // max peer length is 20 bytes size_t peer_len_ret = 0; size_t buffer_len_ret = 0; @@ -1104,37 +1109,44 @@ void pake_input_getters_peer() psa_pake_cs_set_primitive(&cipher_suite, primitive); psa_pake_cs_set_hash(&cipher_suite, PSA_ALG_SHA_256); - PSA_ASSERT(psa_pake_setup(&operation, &cipher_suite)); + for (size_t i = 0; i < ARRAY_LENGTH(peers); i++) { + uint8_t *peer = (uint8_t *) peers[i]; + uint8_t peer_len = strlen(peers[i]); - TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, - (uint8_t *) &peer_ret, - 10, &buffer_len_ret), - PSA_ERROR_BAD_STATE); + PSA_ASSERT(psa_pake_abort(&operation)); - TEST_EQUAL(psa_crypto_driver_pake_get_peer_len(&operation.data.inputs, &peer_len_ret), - PSA_ERROR_BAD_STATE); + PSA_ASSERT(psa_pake_setup(&operation, &cipher_suite)); - PSA_ASSERT(psa_pake_set_peer(&operation, peer, peer_len)); + TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, + (uint8_t *) &peer_ret, + 10, &buffer_len_ret), + PSA_ERROR_BAD_STATE); - TEST_EQUAL(psa_crypto_driver_pake_get_peer_len(&operation.data.inputs, &peer_len_ret), - PSA_SUCCESS); + TEST_EQUAL(psa_crypto_driver_pake_get_peer_len(&operation.data.inputs, &peer_len_ret), + PSA_ERROR_BAD_STATE); - TEST_EQUAL(peer_len_ret, peer_len); + PSA_ASSERT(psa_pake_set_peer(&operation, peer, peer_len)); - TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, - (uint8_t *) &peer_ret, - peer_len_ret - 1, - &buffer_len_ret), - PSA_ERROR_BUFFER_TOO_SMALL); + TEST_EQUAL(psa_crypto_driver_pake_get_peer_len(&operation.data.inputs, &peer_len_ret), + PSA_SUCCESS); - TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, - (uint8_t *) &peer_ret, - peer_len_ret, - &buffer_len_ret), - PSA_SUCCESS); + TEST_EQUAL(peer_len_ret, peer_len); - TEST_EQUAL(buffer_len_ret, peer_len); - PSA_ASSERT(memcmp(peer_ret, peer, buffer_len_ret)); + TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, + (uint8_t *) &peer_ret, + peer_len_ret - 1, + &buffer_len_ret), + PSA_ERROR_BUFFER_TOO_SMALL); + + TEST_EQUAL(psa_crypto_driver_pake_get_peer(&operation.data.inputs, + (uint8_t *) &peer_ret, + peer_len_ret, + &buffer_len_ret), + PSA_SUCCESS); + + TEST_EQUAL(buffer_len_ret, peer_len); + PSA_ASSERT(memcmp(peer_ret, peer, buffer_len_ret)); + } exit: PSA_ASSERT(psa_pake_abort(&operation)); PSA_DONE(); From ebfeb172ee411cbabe1ac37946026fea40f586b7 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 22 Mar 2023 10:15:36 +0100 Subject: [PATCH 6/9] Add change log entry (j-pake user/peer accept any values) Signed-off-by: Przemek Stekiel --- ChangeLog.d/ec_jpake_user_peer_2.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/ec_jpake_user_peer_2.txt diff --git a/ChangeLog.d/ec_jpake_user_peer_2.txt b/ChangeLog.d/ec_jpake_user_peer_2.txt new file mode 100644 index 000000000..9572ac7c1 --- /dev/null +++ b/ChangeLog.d/ec_jpake_user_peer_2.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix the J-PAKE driver interface for user and peer to accept any values + (previously accepted values were limited to "client" or "server"). From 6e628a4e7b27cd5fbf93b70179194fedd34abea9 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 25 Apr 2023 13:11:36 +0200 Subject: [PATCH 7/9] Add undfined role for ec j-pake Signed-off-by: Przemek Stekiel --- include/mbedtls/ecjpake.h | 3 ++- library/psa_crypto_pake.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ecjpake.h b/include/mbedtls/ecjpake.h index a63bb32f3..94864faea 100644 --- a/include/mbedtls/ecjpake.h +++ b/include/mbedtls/ecjpake.h @@ -52,7 +52,8 @@ extern "C" { * Roles in the EC J-PAKE exchange */ typedef enum { - MBEDTLS_ECJPAKE_CLIENT = 0, /**< Client */ + MBEDTLS_ECJPAKE_NONE = 0, /**< Undefined */ + MBEDTLS_ECJPAKE_CLIENT, /**< Client */ MBEDTLS_ECJPAKE_SERVER, /**< Server */ } mbedtls_ecjpake_role; diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 97aafb457..d9b2ecd9a 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -594,7 +594,7 @@ psa_status_t mbedtls_psa_pake_abort(mbedtls_psa_pake_operation_t *operation) #if defined(MBEDTLS_PSA_BUILTIN_ALG_JPAKE) if (operation->alg == PSA_ALG_JPAKE) { - operation->role = 0; + operation->role = MBEDTLS_ECJPAKE_NONE; mbedtls_platform_zeroize(operation->buffer, sizeof(operation->buffer)); operation->buffer_length = 0; operation->buffer_offset = 0; From aede2ad554e0355e79411b5999f90755b29017c5 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 25 Apr 2023 14:30:34 +0200 Subject: [PATCH 8/9] Optimize code (pake role type, freeing buffers) Signed-off-by: Przemek Stekiel --- include/mbedtls/ecjpake.h | 4 ++-- library/psa_crypto_pake.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ecjpake.h b/include/mbedtls/ecjpake.h index 94864faea..0008d7312 100644 --- a/include/mbedtls/ecjpake.h +++ b/include/mbedtls/ecjpake.h @@ -52,9 +52,9 @@ extern "C" { * Roles in the EC J-PAKE exchange */ typedef enum { - MBEDTLS_ECJPAKE_NONE = 0, /**< Undefined */ - MBEDTLS_ECJPAKE_CLIENT, /**< Client */ + MBEDTLS_ECJPAKE_CLIENT = 0, /**< Client */ MBEDTLS_ECJPAKE_SERVER, /**< Server */ + MBEDTLS_ECJPAKE_NONE, /**< Undefined */ } mbedtls_ecjpake_role; #if !defined(MBEDTLS_ECJPAKE_ALT) diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index d9b2ecd9a..4136614f3 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -289,9 +289,6 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, goto error; } - /* Role has been set, release user/peer buffers. */ - mbedtls_free(user); mbedtls_free(peer); - operation->buffer_length = 0; operation->buffer_offset = 0; @@ -300,6 +297,9 @@ psa_status_t mbedtls_psa_pake_setup(mbedtls_psa_pake_operation_t *operation, goto error; } + /* Role has been set, release user/peer buffers. */ + mbedtls_free(user); mbedtls_free(peer); + return PSA_SUCCESS; } else #else From d14e04ea7238921a2fd00c47745542c2f92b2b27 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 25 Apr 2023 14:31:28 +0200 Subject: [PATCH 9/9] Use ASSERT_COMPARE for comapring buffers Signed-off-by: Przemek Stekiel --- tests/suites/test_suite_psa_crypto_pake.function | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_pake.function b/tests/suites/test_suite_psa_crypto_pake.function index acb69bead..6ce6c2315 100644 --- a/tests/suites/test_suite_psa_crypto_pake.function +++ b/tests/suites/test_suite_psa_crypto_pake.function @@ -983,8 +983,7 @@ void pake_input_getters_password() &buffer_len_ret), PSA_SUCCESS); - TEST_EQUAL(buffer_len_ret, strlen(password)); - PSA_ASSERT(memcmp(password_ret, password, buffer_len_ret)); + ASSERT_COMPARE(password_ret, buffer_len_ret, password, strlen(password)); exit: PSA_ASSERT(psa_destroy_key(key)); PSA_ASSERT(psa_pake_abort(&operation)); @@ -1017,7 +1016,8 @@ void pake_input_getters_cipher_suite() TEST_EQUAL(psa_crypto_driver_pake_get_cipher_suite(&operation.data.inputs, &cipher_suite_ret), PSA_SUCCESS); - PSA_ASSERT(memcmp(&cipher_suite_ret, &cipher_suite, sizeof(cipher_suite))); + ASSERT_COMPARE(&cipher_suite_ret, sizeof(cipher_suite_ret), + &cipher_suite, sizeof(cipher_suite)); exit: PSA_ASSERT(psa_pake_abort(&operation)); @@ -1080,8 +1080,7 @@ void pake_input_getters_user() &buffer_len_ret), PSA_SUCCESS); - TEST_EQUAL(buffer_len_ret, user_len); - PSA_ASSERT(memcmp(user_ret, user, buffer_len_ret)); + ASSERT_COMPARE(user_ret, buffer_len_ret, user, user_len); } exit: PSA_ASSERT(psa_pake_abort(&operation)); @@ -1144,8 +1143,7 @@ void pake_input_getters_peer() &buffer_len_ret), PSA_SUCCESS); - TEST_EQUAL(buffer_len_ret, peer_len); - PSA_ASSERT(memcmp(peer_ret, peer, buffer_len_ret)); + ASSERT_COMPARE(peer_ret, buffer_len_ret, peer, peer_len); } exit: PSA_ASSERT(psa_pake_abort(&operation));