From b3de69493c8770eff88a7c65e3a38bcd3d219a48 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 6 Nov 2023 14:22:28 +0000 Subject: [PATCH] Remove psa_crypto_alloc_and_copy() API This tied input and output buffers together in awkward pairs, which made the API more difficult to use. Signed-off-by: David Horstmann --- library/psa_crypto.c | 85 ------ library/psa_crypto_core.h | 53 ---- .../suites/test_suite_psa_crypto_memory.data | 60 ---- .../test_suite_psa_crypto_memory.function | 285 ------------------ 4 files changed, 483 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 09180b3c3a..b6533496c6 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -8464,89 +8464,4 @@ psa_status_t psa_crypto_copy_output(const uint8_t *output_copy, size_t output_co return PSA_SUCCESS; } -psa_status_t psa_crypto_alloc_and_copy(const uint8_t *input, size_t input_len, - uint8_t *output, size_t output_len, - psa_crypto_buffer_copy_t *buffers) -{ - psa_status_t ret; - /* Zeroize the buffers struct to ensure we can call free() - * on any pointers safely. */ - memset(buffers, 0, sizeof(*buffers)); - - /* Since calloc() may return NULL if we try to allocate zero-length - * buffers anyway, deal with this corner case explicitly to ensure - * predictable behaviour. Represent zero-length buffers as NULL. */ - if (input_len == 0) { - input = NULL; - } - if (output_len == 0) { - output = NULL; - } - - if (output != NULL) { - buffers->output = mbedtls_calloc(output_len, 1); - if (buffers->output == NULL) { - ret = PSA_ERROR_INSUFFICIENT_MEMORY; - goto error; - } - buffers->output_len = output_len; - - buffers->output_original = output; - } - - if (input != NULL) { - buffers->input = mbedtls_calloc(input_len, 1); - if (buffers->input == NULL) { - ret = PSA_ERROR_INSUFFICIENT_MEMORY; - goto error; - } - buffers->input_len = input_len; - - if (psa_crypto_copy_input(input, input_len, - buffers->input, buffers->input_len) - != PSA_SUCCESS) { - ret = PSA_ERROR_CORRUPTION_DETECTED; - goto error; - } - } - - return PSA_SUCCESS; - -error: - mbedtls_free(buffers->input); - mbedtls_free(buffers->output); - memset(buffers, 0, sizeof(*buffers)); - return ret; -} - -psa_status_t psa_crypto_copy_and_free(psa_crypto_buffer_copy_t *buffers) -{ - if ((buffers->input != NULL) && (buffers->input_len == 0)) { - /* Reject zero-length buffers, these should have been represented by - * NULL in psa_crypto_alloc_and_copy() */ - return PSA_ERROR_INVALID_ARGUMENT; - } - if (buffers->output != NULL) { - if (buffers->output_len == 0) { - /* Reject zero-length buffers, these should have been represented - * by NULL in psa_crypto_alloc_and_copy() */ - return PSA_ERROR_INVALID_ARGUMENT; - } - if (buffers->output_original == NULL) { - /* Output is non-NULL but original output is NULL. The argument - * buffers is invalid. Return an error as we have no original to - * copy back to. */ - return PSA_ERROR_INVALID_ARGUMENT; - } - memcpy(buffers->output_original, buffers->output, buffers->output_len); - } - - mbedtls_free(buffers->input); - buffers->input = NULL; - mbedtls_free(buffers->output); - buffers->output = NULL; - - return PSA_SUCCESS; -} - #endif /* MBEDTLS_PSA_CRYPTO_C */ diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 00d9e9eedd..c9e277c2d3 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -884,57 +884,4 @@ psa_status_t psa_crypto_copy_input(const uint8_t *input, size_t input_len, psa_status_t psa_crypto_copy_output(const uint8_t *output_copy, size_t output_copy_len, uint8_t *output, size_t output_len); -/** - * \brief Structure to store a pair of copied buffers (input, output) with a - * reference to the original output to be used during copy-back. - */ -struct psa_crypto_buffer_copy_s { - uint8_t *input; - size_t input_len; - - uint8_t *output_original; - uint8_t *output; - size_t output_len; -}; -typedef struct psa_crypto_buffer_copy_s psa_crypto_buffer_copy_t; - -/** - * \brief Allocate copies of provided input and output - * buffers and store references to them along with - * the original output buffer in the provided - * psa_crypto_buffer_copy_t struct. - * Either or both buffers may be NULL, in which case - * they are not copied. - * - * \note The input and output buffers may overlap. - * - * \param[in] input Pointer to the input buffer. - * \param[in] input_len Length of the input buffer. - * \param[in] output Pointer to the output buffer. - * \param[in] output_len Length of the output buffer. - * \param[out] buffers Struct containing pointers to the allocated - * copies and the original output buffer. - * \retval #PSA_SUCCESS - * The buffers were successfully allocated and copied. - * \retval #PSA_ERROR_INSUFFICIENT_MEMORY - * Failed to allocate buffers. - */ -psa_status_t psa_crypto_alloc_and_copy(const uint8_t *input, size_t input_len, - uint8_t *output, size_t output_len, - psa_crypto_buffer_copy_t *buffers); - -/** - * \brief Free an allocated pair of buffers after first - * copying the output buffer back to its original. - * - * \param[in] buffers psa_crypto_buffer_copy_t created by a previous - * call to psa_crypto_alloc_and_copy(). - * \retval #PSA_SUCCESS - * The buffers were successfully copied-back and the - * copies freed. - * \retval #PSA_ERROR_INVALID_ARGUMENT - * Could not copy-back as \p buffers is invalid. - */ -psa_status_t psa_crypto_copy_and_free(psa_crypto_buffer_copy_t *buffers); - #endif /* PSA_CRYPTO_CORE_H */ diff --git a/tests/suites/test_suite_psa_crypto_memory.data b/tests/suites/test_suite_psa_crypto_memory.data index 33623afdc1..0d44fb35d5 100644 --- a/tests/suites/test_suite_psa_crypto_memory.data +++ b/tests/suites/test_suite_psa_crypto_memory.data @@ -27,63 +27,3 @@ copy_output:0:10:PSA_SUCCESS PSA output buffer copy: zero-length both buffers copy_output:0:0:PSA_SUCCESS - -PSA buffers alloc and copy -psa_crypto_alloc_and_copy:0:20:0:20:PSA_SUCCESS - -PSA buffers alloc and copy: null input -psa_crypto_alloc_and_copy:1:0:0:20:PSA_SUCCESS - -PSA buffers alloc and copy: null output -psa_crypto_alloc_and_copy:0:20:1:0:PSA_SUCCESS - -PSA buffers alloc and copy: null input and output -psa_crypto_alloc_and_copy:1:0:1:0:PSA_SUCCESS - -PSA buffers alloc and copy zero-length input -psa_crypto_alloc_and_copy_zero_length:1:0 - -PSA buffers alloc and copy zero-length output -psa_crypto_alloc_and_copy_zero_length:0:1 - -PSA buffers alloc and copy both zero length -psa_crypto_alloc_and_copy_zero_length:1:1 - -PSA buffers alloc and copy overlapping input first -psa_crypto_alloc_and_copy_overlapping:20:20:5:PSA_SUCCESS - -PSA buffers alloc and copy overlapping output first -psa_crypto_alloc_and_copy_overlapping:20:20:-5:PSA_SUCCESS - -PSA buffers alloc and copy overlapping output within input -psa_crypto_alloc_and_copy_overlapping:20:10:5:PSA_SUCCESS - -PSA buffers alloc and copy overlapping input within output -psa_crypto_alloc_and_copy_overlapping:10:20:-5:PSA_SUCCESS - -PSA buffers alloc and copy overlapping input equals output -psa_crypto_alloc_and_copy_overlapping:20:20:0:PSA_SUCCESS - -PSA buffers copy and free -psa_crypto_copy_and_free:0:20:0:20:0:PSA_SUCCESS - -PSA buffers copy and free, null input -psa_crypto_copy_and_free:1:0:0:20:0:PSA_SUCCESS - -PSA buffers copy and free, null output -psa_crypto_copy_and_free:0:20:1:0:0:PSA_SUCCESS - -PSA buffers copy and free, null output_original -psa_crypto_copy_and_free:0:20:0:20:1:PSA_ERROR_INVALID_ARGUMENT - -PSA buffers copy and free, null output_original and null output -psa_crypto_copy_and_free:0:20:1:0:1:PSA_SUCCESS - -PSA buffers copy and free, zero-length input -psa_crypto_copy_and_free:0:0:0:20:0:PSA_ERROR_INVALID_ARGUMENT - -PSA buffers copy and free, zero-length output -psa_crypto_copy_and_free:20:0:0:0:0:PSA_ERROR_INVALID_ARGUMENT - -PSA buffers round-trip -psa_crypto_buffer_copy_round_trip diff --git a/tests/suites/test_suite_psa_crypto_memory.function b/tests/suites/test_suite_psa_crypto_memory.function index a27f76b88a..7e59bb96f9 100644 --- a/tests/suites/test_suite_psa_crypto_memory.function +++ b/tests/suites/test_suite_psa_crypto_memory.function @@ -20,63 +20,6 @@ static void fill_buffer_pattern(uint8_t *buffer, size_t len) buffer[i] = data[i % sizeof(data)]; } } - -/* Helper to get 2 buffers that overlap by the specified amount. - * The parameter ptr_diff may be negative, in which case the start of - * buf2 is allocated before the start of buf1. */ -static int setup_overlapping_buffers(size_t buf1_len, size_t buf2_len, - int ptr_diff, - uint8_t **full_buffer, - uint8_t **buf1, uint8_t **buf2) -{ - size_t total_len; - int buf1_offset; - int buf2_offset; - - *full_buffer = NULL; - *buf1 = NULL; - *buf2 = NULL; - - if (ptr_diff >= 0) { - /* - * |---------- buf1 ----------| - * <-- ptr_diff -->|---------- buf2 ----------| - */ - total_len = ptr_diff + buf2_len; - if (buf1_len > total_len) { - total_len = buf1_len; - } - buf1_offset = 0; - buf2_offset = ptr_diff; - } else { - /* - * <-- (-ptr_diff) -->|---------- buf1 ----------| - * |---------- buf2 ----------| - */ - total_len = (-ptr_diff) + buf1_len; - if (buf2_len > total_len) { - total_len = buf2_len; - } - buf1_offset = -ptr_diff; - buf2_offset = 0; - } - - /* Edge case: if the length is zero, allocate a 1-byte buffer to avoid - * calloc returning NULL. */ - if (total_len == 0) { - total_len = 1; - } - - TEST_CALLOC(*full_buffer, total_len); - - *buf1 = *full_buffer + buf1_offset; - *buf2 = *full_buffer + buf2_offset; - - return 0; - -exit: - return -1; -} /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -135,231 +78,3 @@ exit: mbedtls_free(dst_buffer); } /* END_CASE */ - -/* BEGIN_CASE */ -void psa_crypto_alloc_and_copy(int input_null, int input_len, - int output_null, int output_len, - int exp_ret) -{ - uint8_t *input_buffer = NULL; - uint8_t *output_buffer = NULL; - psa_status_t ret; - - psa_crypto_buffer_copy_t buffer_copies; - memset(&buffer_copies, 0, sizeof(buffer_copies)); - - if (!input_null) { - TEST_CALLOC(input_buffer, input_len); - fill_buffer_pattern(input_buffer, input_len); - } - if (!output_null) { - TEST_CALLOC(output_buffer, output_len); - fill_buffer_pattern(output_buffer, output_len); - } - ret = psa_crypto_alloc_and_copy(input_buffer, input_len, - output_buffer, output_len, - &buffer_copies); - TEST_EQUAL(exp_ret, (int) ret); - - if (exp_ret == PSA_SUCCESS) { - TEST_MEMORY_COMPARE(input_buffer, input_len, buffer_copies.input, buffer_copies.input_len); - TEST_EQUAL(output_len, buffer_copies.output_len); - } - -exit: - mbedtls_free(input_buffer); - mbedtls_free(output_buffer); - mbedtls_free(buffer_copies.input); - mbedtls_free(buffer_copies.output); -} -/* END_CASE */ - -/* BEGIN_CASE */ -void psa_crypto_alloc_and_copy_zero_length(int input_zero_length, - int output_zero_length) -{ - uint8_t input_buffer[] = { 0x12 }; - uint8_t output_buffer[] = { 0x34 }; - - size_t input_len = input_zero_length ? 0 : 1; - size_t output_len = output_zero_length ? 0 : 1; - - psa_crypto_buffer_copy_t buffer_copies; - memset(&buffer_copies, 0, sizeof(buffer_copies)); - - psa_status_t ret = psa_crypto_alloc_and_copy(input_buffer, input_len, - output_buffer, output_len, - &buffer_copies); - TEST_EQUAL(ret, PSA_SUCCESS); - - if (input_zero_length) { - TEST_ASSERT(buffer_copies.input == NULL); - } else { - TEST_MEMORY_COMPARE(input_buffer, input_len, buffer_copies.input, buffer_copies.input_len); - } - if (output_zero_length) { - TEST_ASSERT(buffer_copies.output == NULL); - } else { - TEST_EQUAL(output_len, buffer_copies.output_len); - } - -exit: - mbedtls_free(buffer_copies.input); - mbedtls_free(buffer_copies.output); -} -/* END_CASE */ - -/* BEGIN_CASE */ -/* Ensure that overlapping buffers can be copied correctly. */ -void psa_crypto_alloc_and_copy_overlapping(int input_len, int output_len, - int ptr_diff, int exp_ret) -{ - uint8_t *full_buffer = NULL; - uint8_t *input = NULL; - uint8_t *output = NULL; - - psa_status_t ret; - - psa_crypto_buffer_copy_t buffer_copies; - memset(&buffer_copies, 0, sizeof(buffer_copies)); - - TEST_EQUAL(setup_overlapping_buffers(input_len, output_len, ptr_diff, - &full_buffer, &input, &output), 0); - - fill_buffer_pattern(input, input_len); - - ret = psa_crypto_alloc_and_copy(input, input_len, output, output_len, - &buffer_copies); - - TEST_EQUAL((int) ret, exp_ret); - - if (exp_ret == PSA_SUCCESS) { - if (input_len == 0) { - TEST_ASSERT(buffer_copies.input == NULL); - } else { - TEST_MEMORY_COMPARE(input, input_len, buffer_copies.input, buffer_copies.input_len); - } - if (output_len == 0) { - TEST_ASSERT(buffer_copies.output == NULL); - } else { - TEST_EQUAL(output_len, buffer_copies.output_len); - } - } - -exit: - mbedtls_free(full_buffer); - mbedtls_free(buffer_copies.input); - mbedtls_free(buffer_copies.output); -} -/* END_CASE */ - -/* BEGIN_CASE */ -void psa_crypto_copy_and_free(int input_null, int input_len, - int output_null, int output_len, - int orig_output_null, - int exp_ret) -{ - uint8_t *input = NULL; - uint8_t *output = NULL; - uint8_t *output_for_comparison = NULL; - uint8_t *orig_output = NULL; - size_t calloc_len; - psa_status_t ret; - - psa_crypto_buffer_copy_t buffer_copies; - memset(&buffer_copies, 0, sizeof(buffer_copies)); - - if (!input_null) { - /* If zero-length, ensure we actually allocate something - * rather than getting NULL. */ - calloc_len = input_len == 0 ? 1 : input_len; - TEST_CALLOC(input, calloc_len); - } - if (!output_null) { - /* If zero-length, ensure we actually allocate something - * rather than getting NULL. */ - calloc_len = output_len == 0 ? 1 : output_len; - TEST_CALLOC(output, calloc_len); - TEST_CALLOC(output_for_comparison, calloc_len); - - fill_buffer_pattern(output, output_len); - /* We expect the output buffer to be freed, so keep a copy - * for comparison. */ - memcpy(output_for_comparison, output, output_len); - } - if (!orig_output_null) { - /* If zero-length, ensure we actually allocate something - * rather than getting NULL. */ - calloc_len = output_len == 0 ? 1 : output_len; - TEST_CALLOC(orig_output, calloc_len); - } - - buffer_copies.input = input; - buffer_copies.input_len = input_len; - buffer_copies.output = output; - buffer_copies.output_len = output_len; - buffer_copies.output_original = orig_output; - - ret = psa_crypto_copy_and_free(&buffer_copies); - - TEST_EQUAL((int) ret, exp_ret); - - if (exp_ret == PSA_SUCCESS) { - TEST_ASSERT(buffer_copies.input == NULL); - TEST_ASSERT(buffer_copies.output == NULL); - - if (!output_null) { - TEST_MEMORY_COMPARE(output_for_comparison, output_len, - buffer_copies.output_original, buffer_copies.output_len); - } - } - -exit: - mbedtls_free(buffer_copies.input); - mbedtls_free(buffer_copies.output); - mbedtls_free(output_for_comparison); - mbedtls_free(orig_output); -} -/* END_CASE */ - -/* BEGIN_CASE */ -void psa_crypto_buffer_copy_round_trip() -{ - uint8_t input[100]; - uint8_t output[100]; - uint8_t output_for_comparison[100]; - psa_status_t ret; - - psa_crypto_buffer_copy_t buffer_copies; - memset(&buffer_copies, 0, sizeof(buffer_copies)); - - fill_buffer_pattern(input, sizeof(input)); - - ret = psa_crypto_alloc_and_copy(input, sizeof(input), - output, sizeof(output), - &buffer_copies); - - TEST_ASSERT(ret == PSA_SUCCESS); - TEST_MEMORY_COMPARE(input, sizeof(input), - buffer_copies.input, buffer_copies.input_len); - TEST_EQUAL(sizeof(output), buffer_copies.output_len); - - /* Simulate the PSA function filling the (internal) output buffer. */ - fill_buffer_pattern(buffer_copies.output, buffer_copies.output_len); - - /* Make a copy of output to compare the copy-back */ - memcpy(output_for_comparison, buffer_copies.output, - sizeof(output_for_comparison)); - - ret = psa_crypto_copy_and_free(&buffer_copies); - - TEST_EQUAL(ret, PSA_SUCCESS); - /* Check that the output was copied back correctly. */ - TEST_MEMORY_COMPARE(output_for_comparison, sizeof(output_for_comparison), - output, sizeof(output)); - -exit: - mbedtls_free(buffer_copies.input); - mbedtls_free(buffer_copies.output); -} -/* END_CASE */