From 691e91adac10c9466f0b84f930b72e277440279b Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 7 Mar 2023 16:26:37 +0100 Subject: [PATCH] Further pake code optimizations Signed-off-by: Przemek Stekiel --- docs/proposed/psa-driver-interface.md | 4 +-- library/psa_crypto.c | 44 ++++++++------------------- library/psa_crypto_pake.c | 10 +++--- library/psa_crypto_pake.h | 6 +--- 4 files changed, 20 insertions(+), 44 deletions(-) diff --git a/docs/proposed/psa-driver-interface.md b/docs/proposed/psa-driver-interface.md index c00796a49..f681ea60e 100644 --- a/docs/proposed/psa-driver-interface.md +++ b/docs/proposed/psa-driver-interface.md @@ -458,9 +458,7 @@ For `PSA_ALG_JPAKE` the following steps are available for input operation: * `PSA_JPAKE_X4S_STEP_ZK_PUBLIC`    Round 2: input Schnorr NIZKP public key for the X4S key * `PSA_JPAKE_X4S_STEP_ZK_PROOF`     Round 2: input Schnorr NIZKP proof for the X4S key -The core has checked that input_length is smaller than PSA_PAKE_INPUT_SIZE(PSA_ALG_JPAKE, primitive, step) -where primitive is the JPAKE algorithm primitive and step the PSA API level input step. -Thus no risk of integer overflow while checking operation buffer overflow. +The core checks that input_length is smaller than PSA_PAKE_INPUT_MAX_SIZE. ### PAKE driver get implicit key diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 115e994bb..917a9fae6 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7609,6 +7609,7 @@ psa_status_t psa_pake_output( size_t *output_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_crypto_driver_pake_step_t driver_step = PSA_JPAKE_STEP_INVALID; *output_length = 0; if (operation->stage == PSA_PAKE_OPERATION_STAGE_COLLECT_INPUTS) { @@ -7635,6 +7636,8 @@ psa_status_t psa_pake_output( if (status != PSA_SUCCESS) { goto exit; } + driver_step = convert_jpake_computation_stage_to_driver_step( + &operation->computation_stage.jpake); break; #endif /* PSA_WANT_ALG_JPAKE */ default: @@ -7643,17 +7646,8 @@ psa_status_t psa_pake_output( goto exit; } -#if defined(PSA_WANT_ALG_JPAKE) - status = psa_driver_wrapper_pake_output(operation, - convert_jpake_computation_stage_to_driver_step( - &operation->computation_stage.jpake), - output, - output_size, - output_length); -#else - (void) output; - status = PSA_ERROR_NOT_SUPPORTED; -#endif /* PSA_WANT_ALG_JPAKE */ + status = psa_driver_wrapper_pake_output(operation, driver_step, + output, output_size, output_length); if (status != PSA_SUCCESS) { goto exit; @@ -7682,8 +7676,7 @@ exit: #if defined(PSA_WANT_ALG_JPAKE) static psa_status_t psa_jpake_input_prologue( psa_pake_operation_t *operation, - psa_pake_step_t step, - size_t input_length) + psa_pake_step_t step) { if (step != PSA_PAKE_STEP_KEY_SHARE && step != PSA_PAKE_STEP_ZK_PUBLIC && @@ -7698,12 +7691,6 @@ static psa_status_t psa_jpake_input_prologue( return PSA_ERROR_BAD_STATE; } - const psa_pake_primitive_t prim = PSA_PAKE_PRIMITIVE( - PSA_PAKE_PRIMITIVE_TYPE_ECC, PSA_ECC_FAMILY_SECP_R1, 256); - if (input_length > (size_t) PSA_PAKE_INPUT_SIZE(PSA_ALG_JPAKE, prim, step)) { - return PSA_ERROR_INVALID_ARGUMENT; - } - if (computation_stage->state != PSA_PAKE_STATE_READY && computation_stage->state != PSA_PAKE_INPUT_X1_X2 && computation_stage->state != PSA_PAKE_INPUT_X4S) { @@ -7787,6 +7774,7 @@ psa_status_t psa_pake_input( size_t input_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_crypto_driver_pake_step_t driver_step = PSA_JPAKE_STEP_INVALID; if (operation->stage == PSA_PAKE_OPERATION_STAGE_COLLECT_INPUTS) { status = psa_pake_complete_inputs(operation); @@ -7800,7 +7788,7 @@ psa_status_t psa_pake_input( goto exit; } - if (input_length == 0) { + if (input_length == 0 || input_length > PSA_PAKE_INPUT_MAX_SIZE) { status = PSA_ERROR_INVALID_ARGUMENT; goto exit; } @@ -7808,10 +7796,12 @@ psa_status_t psa_pake_input( switch (operation->alg) { #if defined(PSA_WANT_ALG_JPAKE) case PSA_ALG_JPAKE: - status = psa_jpake_input_prologue(operation, step, input_length); + status = psa_jpake_input_prologue(operation, step); if (status != PSA_SUCCESS) { goto exit; } + driver_step = convert_jpake_computation_stage_to_driver_step( + &operation->computation_stage.jpake); break; #endif /* PSA_WANT_ALG_JPAKE */ default: @@ -7820,16 +7810,8 @@ psa_status_t psa_pake_input( goto exit; } -#if defined(PSA_WANT_ALG_JPAKE) - status = psa_driver_wrapper_pake_input(operation, - convert_jpake_computation_stage_to_driver_step( - &operation->computation_stage.jpake), - input, - input_length); -#else - (void) input; - status = PSA_ERROR_NOT_SUPPORTED; -#endif /* PSA_WANT_ALG_JPAKE */ + status = psa_driver_wrapper_pake_input(operation, driver_step, + input, input_length); if (status != PSA_SUCCESS) { goto exit; diff --git a/library/psa_crypto_pake.c b/library/psa_crypto_pake.c index 538df8744..a53718496 100644 --- a/library/psa_crypto_pake.c +++ b/library/psa_crypto_pake.c @@ -431,7 +431,8 @@ static psa_status_t mbedtls_psa_pake_input_internal( 0, 23 /* secp256r1 */ }; - if (operation->buffer_length + sizeof(ecparameters) > sizeof(operation->buffer)) { + if (operation->buffer_length + sizeof(ecparameters) > + sizeof(operation->buffer)) { return PSA_ERROR_BUFFER_TOO_SMALL; } @@ -441,10 +442,9 @@ static psa_status_t mbedtls_psa_pake_input_internal( } /* - * The core has checked that input_length is smaller than - * PSA_PAKE_INPUT_SIZE(PSA_ALG_JPAKE, primitive, step) - * where primitive is the JPAKE algorithm primitive and step - * the PSA API level input step. Thus no risk of integer overflow here. + * The core checks that input_length is smaller than + * PSA_PAKE_INPUT_MAX_SIZE. + * Thus no risk of integer overflow here. */ if (operation->buffer_length + input_length + 1 > sizeof(operation->buffer)) { return PSA_ERROR_BUFFER_TOO_SMALL; diff --git a/library/psa_crypto_pake.h b/library/psa_crypto_pake.h index eb308813e..001c987a4 100644 --- a/library/psa_crypto_pake.h +++ b/library/psa_crypto_pake.h @@ -96,11 +96,7 @@ psa_status_t mbedtls_psa_pake_output(mbedtls_psa_pake_operation_t *operation, * entry point as defined in the PSA driver interface specification for * transparent drivers. * - * \note The core has checked that input_length is smaller than - PSA_PAKE_INPUT_SIZE(PSA_ALG_JPAKE, primitive, step) - where primitive is the JPAKE algorithm primitive and step - the PSA API level input step. Thus no risk of integer overflow while - checking operation buffer overflow. + * \note The core checks that input_length is smaller than PSA_PAKE_INPUT_MAX_SIZE. * * \param[in,out] operation Active PAKE operation. * \param step The driver step for which the input is provided.