From 7b1c916fe8e12c88deabc36375e7b964b4c8afaf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Sep 2021 09:33:28 +0200 Subject: [PATCH 1/3] Document the internal function psa_cipher_update_ecb Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 2213ad0c2..1500bd6da 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -258,10 +258,30 @@ static psa_status_t cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, iv, iv_length ) ) ); } -/* Process input for which the algorithm is set to ECB mode. This requires - * manual processing, since the PSA API is defined as being able to process - * arbitrary-length calls to psa_cipher_update() with ECB mode, but the - * underlying mbedtls_cipher_update only takes full blocks. */ +/** Process input for which the algorithm is set to ECB mode. + * + * This requires manual processing, since the PSA API is defined as being + * able to process arbitrary-length calls to psa_cipher_update() with ECB mode, + * but the underlying mbedtls_cipher_update only takes full blocks. + * + * \param ctx The mbedtls cipher context to use. It must have been + * set up for ECB. + * \param[in] input The input plaintext or ciphertext to process. + * \param input_length The number of bytes to process from \p input. + * This does not need to be aligned to a block boundary. + * If there is a partial block at the end of the input, + * it is stored in \p ctx for future processing. + * \param output The buffer where the output is written. + * \param output_size The size of \p output in bytes. + * It must be at least `floor((p + input_length) / BS)` + * where `p` is the number of bytes in the unprocessed + * partial block in \p ctx (`0 <= p <= BS - 1`) and + * `BS` is the block size. + * \param output_length On success, the number of bytes written to \p output. + * \c 0 on error. + * + * \return #PSA_SUCCESS or an error from a hardware accelerator + */ static psa_status_t psa_cipher_update_ecb( mbedtls_cipher_context_t *ctx, const uint8_t *input, From a63ba6cd9313457ad4f2890b94abe7a0652e9ec4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Sep 2021 09:36:28 +0200 Subject: [PATCH 2/3] psa_cipher_update_ecb: remove parameter output_size This parameter was set but not used, which was pointless. Clang 14 detects this and legitimately complains. Remove the parameter. This is an internal function, only called once. The caller already has a sufficient check on the output buffer size which applies in more cases, so there is no real gain in robustness in adding the same check inside the internal function. Signed-off-by: Gilles Peskine --- ChangeLog.d/psa_cipher_update_ecp.txt | 2 ++ library/psa_crypto_cipher.c | 9 ++------- 2 files changed, 4 insertions(+), 7 deletions(-) create mode 100644 ChangeLog.d/psa_cipher_update_ecp.txt diff --git a/ChangeLog.d/psa_cipher_update_ecp.txt b/ChangeLog.d/psa_cipher_update_ecp.txt new file mode 100644 index 000000000..1c3fbc6b1 --- /dev/null +++ b/ChangeLog.d/psa_cipher_update_ecp.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix a parameter set but unused in psa_crypto_cipher.c. Fixes #4935. diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 1500bd6da..f0bc43657 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -271,9 +271,8 @@ static psa_status_t cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, * This does not need to be aligned to a block boundary. * If there is a partial block at the end of the input, * it is stored in \p ctx for future processing. - * \param output The buffer where the output is written. - * \param output_size The size of \p output in bytes. - * It must be at least `floor((p + input_length) / BS)` + * \param output The buffer where the output is written. Its size + * must be at least `floor((p + input_length) / BS)` * where `p` is the number of bytes in the unprocessed * partial block in \p ctx (`0 <= p <= BS - 1`) and * `BS` is the block size. @@ -287,7 +286,6 @@ static psa_status_t psa_cipher_update_ecb( const uint8_t *input, size_t input_length, uint8_t *output, - size_t output_size, size_t *output_length ) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; @@ -327,7 +325,6 @@ static psa_status_t psa_cipher_update_ecb( goto exit; output += internal_output_length; - output_size -= internal_output_length; *output_length += internal_output_length; ctx->unprocessed_len = 0; } @@ -348,7 +345,6 @@ static psa_status_t psa_cipher_update_ecb( input += block_size; output += internal_output_length; - output_size -= internal_output_length; *output_length += internal_output_length; } @@ -403,7 +399,6 @@ static psa_status_t cipher_update( mbedtls_psa_cipher_operation_t *operation, input, input_length, output, - output_size, output_length ); } else From 039001609670cc44371321eb84412fdc4ab5a90a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 13 Sep 2021 12:20:51 +0200 Subject: [PATCH 3/3] Fix the size in bytes Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index f0bc43657..6c4150b4c 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -271,11 +271,11 @@ static psa_status_t cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, * This does not need to be aligned to a block boundary. * If there is a partial block at the end of the input, * it is stored in \p ctx for future processing. - * \param output The buffer where the output is written. Its size - * must be at least `floor((p + input_length) / BS)` - * where `p` is the number of bytes in the unprocessed - * partial block in \p ctx (`0 <= p <= BS - 1`) and - * `BS` is the block size. + * \param output The buffer where the output is written. It must be + * at least `BS * floor((p + input_length) / BS)` bytes + * long, where `p` is the number of bytes in the + * unprocessed partial block in \p ctx (with + * `0 <= p <= BS - 1`) and `BS` is the block size. * \param output_length On success, the number of bytes written to \p output. * \c 0 on error. *