From 21c1a948131b8d8eb18677172143417ac3736ed3 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:25:00 +0000 Subject: [PATCH 1/4] Copy buffers in psa_aead_encrypt() Signed-off-by: David Horstmann --- library/psa_crypto.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 00e4f2aa98..4221079be9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3796,19 +3796,24 @@ exit: psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *nonce, + const uint8_t *nonce_external, size_t nonce_length, - const uint8_t *additional_data, + const uint8_t *additional_data_external, size_t additional_data_length, - const uint8_t *plaintext, + const uint8_t *plaintext_external, size_t plaintext_length, - uint8_t *ciphertext, + uint8_t *ciphertext_external, size_t ciphertext_size, size_t *ciphertext_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; + LOCAL_INPUT_DECLARE(nonce_external, nonce); + LOCAL_INPUT_DECLARE(additional_data_external, additional_data); + LOCAL_INPUT_DECLARE(plaintext_external, plaintext); + LOCAL_OUTPUT_DECLARE(ciphertext_external, ciphertext); + *ciphertext_length = 0; if (!PSA_ALG_IS_AEAD(alg) || PSA_ALG_IS_WILDCARD(alg)) { @@ -3825,6 +3830,11 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); + LOCAL_INPUT_ALLOC(additional_data_external, additional_data_length, additional_data); + LOCAL_INPUT_ALLOC(plaintext_external, plaintext_length, plaintext); + LOCAL_OUTPUT_ALLOC(ciphertext_external, ciphertext_size, ciphertext); + status = psa_driver_wrapper_aead_encrypt( &attributes, slot->key.data, slot->key.bytes, alg, @@ -3837,6 +3847,15 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, memset(ciphertext, 0, ciphertext_size); } +/* Exit label is only used for buffer copying, prevent unused warnings. */ +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) +exit: +#endif + LOCAL_INPUT_FREE(nonce_external, nonce); + LOCAL_INPUT_FREE(additional_data_external, additional_data); + LOCAL_INPUT_FREE(plaintext_external, plaintext); + LOCAL_OUTPUT_FREE(ciphertext_external, ciphertext); + psa_unlock_key_slot(slot); return status; From 6baf6e9a0632867325d9a22e4f1410488d3ffd9e Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:43:53 +0000 Subject: [PATCH 2/4] Add buffer copying to psa_aead_decrypt() Signed-off-by: David Horstmann --- library/psa_crypto.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4221079be9..3ca3be1cb8 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3863,19 +3863,24 @@ exit: psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *nonce, + const uint8_t *nonce_external, size_t nonce_length, - const uint8_t *additional_data, + const uint8_t *additional_data_external, size_t additional_data_length, - const uint8_t *ciphertext, + const uint8_t *ciphertext_external, size_t ciphertext_length, - uint8_t *plaintext, + uint8_t *plaintext_external, size_t plaintext_size, size_t *plaintext_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; + LOCAL_INPUT_DECLARE(nonce_external, nonce); + LOCAL_INPUT_DECLARE(additional_data_external, additional_data); + LOCAL_INPUT_DECLARE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_DECLARE(plaintext_external, plaintext); + *plaintext_length = 0; if (!PSA_ALG_IS_AEAD(alg) || PSA_ALG_IS_WILDCARD(alg)) { @@ -3892,6 +3897,12 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); + LOCAL_INPUT_ALLOC(additional_data_external, additional_data_length, + additional_data); + LOCAL_INPUT_ALLOC(ciphertext_external, ciphertext_length, ciphertext); + LOCAL_OUTPUT_ALLOC(plaintext_external, plaintext_size, plaintext); + status = psa_driver_wrapper_aead_decrypt( &attributes, slot->key.data, slot->key.bytes, alg, @@ -3904,6 +3915,15 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, memset(plaintext, 0, plaintext_size); } +/* Exit label is only used for buffer copying, prevent unused warnings. */ +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) +exit: +#endif + LOCAL_INPUT_FREE(nonce_external, nonce); + LOCAL_INPUT_FREE(additional_data_external, additional_data); + LOCAL_INPUT_FREE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_FREE(plaintext_external, plaintext); + psa_unlock_key_slot(slot); return status; From 436b2ef6338b1ea511e27640f27e6d68213280ca Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 22 Jan 2024 14:36:01 +0000 Subject: [PATCH 3/4] Generate poisoning wrappers for AEAD Modify wrapper generation script to generate poisoning calls and regenerate wrappers. Signed-off-by: David Horstmann --- tests/scripts/generate_psa_wrappers.py | 3 ++- tests/src/psa_test_wrappers.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index d35598d26a..cace6e3c3b 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -150,7 +150,8 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): _buffer_name: Optional[str]) -> bool: """Whether the specified buffer argument to a PSA function should be copied. """ - # Proof-of-concept: just instrument one function for now + if function_name.startswith('psa_aead'): + return True if function_name == 'psa_cipher_encrypt': return True return False diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index 56bbef03b3..b6815c1f6a 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -66,7 +66,19 @@ psa_status_t mbedtls_test_wrap_psa_aead_decrypt( size_t arg9_plaintext_size, size_t *arg10_plaintext_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_ciphertext, arg7_ciphertext_length); + MBEDTLS_TEST_MEMORY_POISON(arg8_plaintext, arg9_plaintext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_decrypt)(arg0_key, arg1_alg, arg2_nonce, arg3_nonce_length, arg4_additional_data, arg5_additional_data_length, arg6_ciphertext, arg7_ciphertext_length, arg8_plaintext, arg9_plaintext_size, arg10_plaintext_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_ciphertext, arg7_ciphertext_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg8_plaintext, arg9_plaintext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -84,7 +96,19 @@ psa_status_t mbedtls_test_wrap_psa_aead_encrypt( size_t arg9_ciphertext_size, size_t *arg10_ciphertext_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_plaintext, arg7_plaintext_length); + MBEDTLS_TEST_MEMORY_POISON(arg8_ciphertext, arg9_ciphertext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_encrypt)(arg0_key, arg1_alg, arg2_nonce, arg3_nonce_length, arg4_additional_data, arg5_additional_data_length, arg6_plaintext, arg7_plaintext_length, arg8_ciphertext, arg9_ciphertext_size, arg10_ciphertext_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_plaintext, arg7_plaintext_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg8_ciphertext, arg9_ciphertext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 90b94ff85f73381a7ec819387d19de6eb037c264 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 1 Feb 2024 19:32:46 +0000 Subject: [PATCH 4/4] Allow GCM IV to be NULL if zero-length The operation will still return an error, but the assert-based validation checks will pass. This allows GCM to work with buffer copies / local inputs, which may be NULL when they are zero-length. Signed-off-by: David Horstmann --- library/gcm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/gcm.c b/library/gcm.c index 86d5fa2b5f..d3e773278f 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -241,7 +241,7 @@ int mbedtls_gcm_starts(mbedtls_gcm_context *ctx, uint64_t iv_bits; GCM_VALIDATE_RET(ctx != NULL); - GCM_VALIDATE_RET(iv != NULL); + GCM_VALIDATE_RET(iv_len == 0 || iv != NULL); GCM_VALIDATE_RET(add_len == 0 || add != NULL); /* IV and AD are limited to 2^64 bits, so 2^61 bytes */ @@ -433,7 +433,7 @@ int mbedtls_gcm_crypt_and_tag(mbedtls_gcm_context *ctx, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; GCM_VALIDATE_RET(ctx != NULL); - GCM_VALIDATE_RET(iv != NULL); + GCM_VALIDATE_RET(iv_len == 0 || iv != NULL); GCM_VALIDATE_RET(add_len == 0 || add != NULL); GCM_VALIDATE_RET(length == 0 || input != NULL); GCM_VALIDATE_RET(length == 0 || output != NULL); @@ -470,7 +470,7 @@ int mbedtls_gcm_auth_decrypt(mbedtls_gcm_context *ctx, int diff; GCM_VALIDATE_RET(ctx != NULL); - GCM_VALIDATE_RET(iv != NULL); + GCM_VALIDATE_RET(iv_len == 0 || iv != NULL); GCM_VALIDATE_RET(add_len == 0 || add != NULL); GCM_VALIDATE_RET(tag != NULL); GCM_VALIDATE_RET(length == 0 || input != NULL);