From 55dffe58a010298d81aa23832e2a3e4abb6d00f4 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 d8c722bb1..f67b1ffed 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -255,10 +255,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 1716f3286419bc68071d69db15daa5480c68df70 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 f67b1ffed..5c78c2311 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -268,9 +268,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. @@ -284,7 +283,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; @@ -324,7 +322,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; } @@ -345,7 +342,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; } @@ -400,7 +396,6 @@ static psa_status_t cipher_update( mbedtls_psa_cipher_operation_t *operation, input, input_length, output, - output_size, output_length ); } else From d87d87371f4e3d51be50313a4c2ec26a38ba6d21 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 5c78c2311..2268fc585 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -268,11 +268,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. *