Merge pull request #1132 from davidhorstmann-arm/copying-aead
Copy buffers in AEAD
diff --git a/ChangeLog.d/tls-max-version-reset.txt b/ChangeLog.d/tls-max-version-reset.txt
new file mode 100644
index 0000000..2fa5816
--- /dev/null
+++ b/ChangeLog.d/tls-max-version-reset.txt
@@ -0,0 +1,6 @@
+Security
+ * Restore the maximum TLS version to be negotiated to the configured one
+ when an SSL context is reset with the mbedtls_ssl_session_reset() API.
+ An attacker was able to prevent an Mbed TLS server from establishing any
+ TLS 1.3 connection potentially resulting in a Denial of Service or forced
+ version downgrade from TLS 1.3 to TLS 1.2. Fixes #8654 reported by hey3e.
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 25a375a..5b7a838 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -2366,10 +2366,11 @@
}
psa_status_t psa_hash_update(psa_hash_operation_t *operation,
- const uint8_t *input,
+ const uint8_t *input_external,
size_t input_length)
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ LOCAL_INPUT_DECLARE(input_external, input);
if (operation->id == 0) {
status = PSA_ERROR_BAD_STATE;
@@ -2382,6 +2383,7 @@
return PSA_SUCCESS;
}
+ LOCAL_INPUT_ALLOC(input_external, input_length, input);
status = psa_driver_wrapper_hash_update(operation, input, input_length);
exit:
@@ -2389,32 +2391,57 @@
psa_hash_abort(operation);
}
+ LOCAL_INPUT_FREE(input_external, input);
return status;
}
-psa_status_t psa_hash_finish(psa_hash_operation_t *operation,
- uint8_t *hash,
- size_t hash_size,
- size_t *hash_length)
+static psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation,
+ uint8_t *hash,
+ size_t hash_size,
+ size_t *hash_length)
{
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+
*hash_length = 0;
if (operation->id == 0) {
return PSA_ERROR_BAD_STATE;
}
- psa_status_t status = psa_driver_wrapper_hash_finish(
+ status = psa_driver_wrapper_hash_finish(
operation, hash, hash_size, hash_length);
psa_hash_abort(operation);
+
+ return status;
+}
+
+psa_status_t psa_hash_finish(psa_hash_operation_t *operation,
+ uint8_t *hash_external,
+ size_t hash_size,
+ size_t *hash_length)
+{
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ LOCAL_OUTPUT_DECLARE(hash_external, hash);
+
+ LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash);
+ status = psa_hash_finish_internal(operation, hash, hash_size, hash_length);
+
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+exit:
+#endif
+ LOCAL_OUTPUT_FREE(hash_external, hash);
return status;
}
psa_status_t psa_hash_verify(psa_hash_operation_t *operation,
- const uint8_t *hash,
+ const uint8_t *hash_external,
size_t hash_length)
{
uint8_t actual_hash[PSA_HASH_MAX_SIZE];
size_t actual_hash_length;
- psa_status_t status = psa_hash_finish(
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ LOCAL_INPUT_DECLARE(hash_external, hash);
+
+ status = psa_hash_finish_internal(
operation,
actual_hash, sizeof(actual_hash),
&actual_hash_length);
@@ -2428,6 +2455,7 @@
goto exit;
}
+ LOCAL_INPUT_ALLOC(hash_external, hash_length, hash);
if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) {
status = PSA_ERROR_INVALID_SIGNATURE;
}
@@ -2437,36 +2465,55 @@
if (status != PSA_SUCCESS) {
psa_hash_abort(operation);
}
-
+ LOCAL_INPUT_FREE(hash_external, hash);
return status;
}
psa_status_t psa_hash_compute(psa_algorithm_t alg,
- const uint8_t *input, size_t input_length,
- uint8_t *hash, size_t hash_size,
+ const uint8_t *input_external, size_t input_length,
+ uint8_t *hash_external, size_t hash_size,
size_t *hash_length)
{
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ LOCAL_INPUT_DECLARE(input_external, input);
+ LOCAL_OUTPUT_DECLARE(hash_external, hash);
+
*hash_length = 0;
if (!PSA_ALG_IS_HASH(alg)) {
return PSA_ERROR_INVALID_ARGUMENT;
}
- return psa_driver_wrapper_hash_compute(alg, input, input_length,
- hash, hash_size, hash_length);
+ LOCAL_INPUT_ALLOC(input_external, input_length, input);
+ LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash);
+ status = psa_driver_wrapper_hash_compute(alg, input, input_length,
+ hash, hash_size, hash_length);
+
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+exit:
+#endif
+ LOCAL_INPUT_FREE(input_external, input);
+ LOCAL_OUTPUT_FREE(hash_external, hash);
+ return status;
}
psa_status_t psa_hash_compare(psa_algorithm_t alg,
- const uint8_t *input, size_t input_length,
- const uint8_t *hash, size_t hash_length)
+ const uint8_t *input_external, size_t input_length,
+ const uint8_t *hash_external, size_t hash_length)
{
uint8_t actual_hash[PSA_HASH_MAX_SIZE];
size_t actual_hash_length;
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+
+ LOCAL_INPUT_DECLARE(input_external, input);
+ LOCAL_INPUT_DECLARE(hash_external, hash);
if (!PSA_ALG_IS_HASH(alg)) {
- return PSA_ERROR_INVALID_ARGUMENT;
+ status = PSA_ERROR_INVALID_ARGUMENT;
+ return status;
}
- psa_status_t status = psa_driver_wrapper_hash_compute(
+ LOCAL_INPUT_ALLOC(input_external, input_length, input);
+ status = psa_driver_wrapper_hash_compute(
alg, input, input_length,
actual_hash, sizeof(actual_hash),
&actual_hash_length);
@@ -2477,12 +2524,18 @@
status = PSA_ERROR_INVALID_SIGNATURE;
goto exit;
}
+
+ LOCAL_INPUT_ALLOC(hash_external, hash_length, hash);
if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) {
status = PSA_ERROR_INVALID_SIGNATURE;
}
exit:
mbedtls_platform_zeroize(actual_hash, sizeof(actual_hash));
+
+ LOCAL_INPUT_FREE(input_external, input);
+ LOCAL_INPUT_FREE(hash_external, hash);
+
return status;
}
@@ -5892,10 +5945,12 @@
psa_status_t psa_key_derivation_output_bytes(
psa_key_derivation_operation_t *operation,
- uint8_t *output,
+ uint8_t *output_external,
size_t output_length)
{
psa_status_t status;
+ LOCAL_OUTPUT_DECLARE(output_external, output);
+
psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation);
if (operation->alg == 0) {
@@ -5903,13 +5958,6 @@
return PSA_ERROR_BAD_STATE;
}
- if (output_length > operation->capacity) {
- operation->capacity = 0;
- /* Go through the error path to wipe all confidential data now
- * that the operation object is useless. */
- status = PSA_ERROR_INSUFFICIENT_DATA;
- goto exit;
- }
if (output_length == 0 && operation->capacity == 0) {
/* Edge case: this is a finished operation, and 0 bytes
* were requested. The right error in this case could
@@ -5919,6 +5967,16 @@
* output_length > 0. */
return PSA_ERROR_INSUFFICIENT_DATA;
}
+
+ LOCAL_OUTPUT_ALLOC(output_external, output_length, output);
+ if (output_length > operation->capacity) {
+ operation->capacity = 0;
+ /* Go through the error path to wipe all confidential data now
+ * that the operation object is useless. */
+ status = PSA_ERROR_INSUFFICIENT_DATA;
+ goto exit;
+ }
+
operation->capacity -= output_length;
#if defined(BUILTIN_ALG_ANY_HKDF)
@@ -5952,7 +6010,10 @@
{
(void) kdf_alg;
- return PSA_ERROR_BAD_STATE;
+ status = PSA_ERROR_BAD_STATE;
+ LOCAL_OUTPUT_FREE(output_external, output);
+
+ return status;
}
exit:
@@ -5964,8 +6025,12 @@
psa_algorithm_t alg = operation->alg;
psa_key_derivation_abort(operation);
operation->alg = alg;
- memset(output, '!', output_length);
+ if (output != NULL) {
+ memset(output, '!', output_length);
+ }
}
+
+ LOCAL_OUTPUT_FREE(output_external, output);
return status;
}
@@ -6945,12 +7010,12 @@
{
psa_status_t status = PSA_SUCCESS;
if (input_len > PSA_HASH_BLOCK_LENGTH(hash_alg)) {
- status = psa_hash_compute(hash_alg, input, input_len, output,
- PSA_HMAC_MAX_HASH_BLOCK_SIZE, output_len);
- } else {
+ return psa_hash_compute(hash_alg, input, input_len, output,
+ PSA_HMAC_MAX_HASH_BLOCK_SIZE, output_len);
+ } else if (input_len > 0) {
memcpy(output, input, input_len);
- *output_len = PSA_HASH_BLOCK_LENGTH(hash_alg);
}
+ *output_len = PSA_HASH_BLOCK_LENGTH(hash_alg);
return status;
}
#endif /* MBEDTLS_PSA_BUILTIN_ALG_PBKDF2_HMAC */
@@ -7184,12 +7249,22 @@
psa_status_t psa_key_derivation_input_bytes(
psa_key_derivation_operation_t *operation,
psa_key_derivation_step_t step,
- const uint8_t *data,
+ const uint8_t *data_external,
size_t data_length)
{
- return psa_key_derivation_input_internal(operation, step,
- PSA_KEY_TYPE_NONE,
- data, data_length);
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ LOCAL_INPUT_DECLARE(data_external, data);
+
+ LOCAL_INPUT_ALLOC(data_external, data_length, data);
+
+ status = psa_key_derivation_input_internal(operation, step,
+ PSA_KEY_TYPE_NONE,
+ data, data_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+exit:
+#endif
+ LOCAL_INPUT_FREE(data_external, data);
+ return status;
}
psa_status_t psa_key_derivation_input_integer(
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 0bc18f1..0071b06 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1540,6 +1540,7 @@
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
ssl->state = MBEDTLS_SSL_HELLO_REQUEST;
+ ssl->tls_version = ssl->conf->max_tls_version;
mbedtls_ssl_session_reset_msg_layer(ssl, partial);
diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py
index 8acd4e3..cdc798d 100755
--- a/tests/scripts/generate_psa_wrappers.py
+++ b/tests/scripts/generate_psa_wrappers.py
@@ -146,6 +146,9 @@
return True
if function_name == 'psa_cipher_encrypt':
return True
+ if function_name in ('psa_key_derivation_output_bytes',
+ 'psa_key_derivation_input_bytes'):
+ return True
if function_name in ('psa_import_key',
'psa_export_key',
'psa_export_public_key'):
@@ -155,6 +158,12 @@
'psa_sign_hash',
'psa_verify_hash'):
return True
+ if function_name in ('psa_hash_update',
+ 'psa_hash_finish',
+ 'psa_hash_verify',
+ 'psa_hash_compute',
+ 'psa_hash_compare'):
+ return True
return False
def _write_function_call(self, out: typing_util.Writable,
diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c
index 8c2dcf2..5f0a3dd 100644
--- a/tests/src/psa_test_wrappers.c
+++ b/tests/src/psa_test_wrappers.c
@@ -580,7 +580,15 @@
const uint8_t *arg3_hash,
size_t arg4_hash_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length);
+ MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_hash_compare)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length);
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -593,7 +601,15 @@
size_t arg4_hash_size,
size_t *arg5_hash_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length);
+ MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_size);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_hash_compute)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_size, arg5_hash_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length);
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_size);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -604,7 +620,13 @@
size_t arg2_hash_size,
size_t *arg3_hash_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_size);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_hash_finish)(arg0_operation, arg1_hash, arg2_hash_size, arg3_hash_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_size);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -623,7 +645,13 @@
const uint8_t *arg1_input,
size_t arg2_input_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_hash_update)(arg0_operation, arg1_input, arg2_input_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -633,7 +661,13 @@
const uint8_t *arg1_hash,
size_t arg2_hash_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_hash_verify)(arg0_operation, arg1_hash, arg2_hash_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -678,7 +712,13 @@
const uint8_t *arg2_data,
size_t arg3_data_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg2_data, arg3_data_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_key_derivation_input_bytes)(arg0_operation, arg1_step, arg2_data, arg3_data_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg2_data, arg3_data_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
@@ -720,7 +760,13 @@
uint8_t *arg1_output,
size_t arg2_output_length)
{
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_POISON(arg1_output, arg2_output_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
psa_status_t status = (psa_key_derivation_output_bytes)(arg0_operation, arg1_output, arg2_output_length);
+#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS)
+ MBEDTLS_TEST_MEMORY_UNPOISON(arg1_output, arg2_output_length);
+#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */
return status;
}
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 26c5a79..028a0f4 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -11700,6 +11700,30 @@
-s "ECDH/FFDH group: " \
-s "selected signature algorithm ecdsa_secp256r1_sha256"
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3
+requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED
+requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT
+run_test "Establish TLS 1.2 then TLS 1.3 session" \
+ "$P_SRV" \
+ "( $P_CLI force_version=tls12; \
+ $P_CLI force_version=tls13 )" \
+ 0 \
+ -s "Protocol is TLSv1.2" \
+ -s "Protocol is TLSv1.3" \
+
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3
+requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED
+requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT
+run_test "Establish TLS 1.3 then TLS 1.2 session" \
+ "$P_SRV" \
+ "( $P_CLI force_version=tls13; \
+ $P_CLI force_version=tls12 )" \
+ 0 \
+ -s "Protocol is TLSv1.3" \
+ -s "Protocol is TLSv1.2" \
+
requires_openssl_tls1_3_with_compatible_ephemeral
requires_config_enabled MBEDTLS_DEBUG_C
requires_config_enabled MBEDTLS_SSL_CLI_C
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index 3dd3866..bf40aeb 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -2746,6 +2746,11 @@
depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES
cipher_verify_output_multipart:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":16
+# Encrypt 48 bytes total, initially 16. This forces both calls to update() to output data.
+PSA symmetric encrypt/decrypt multipart: AES-CBC-nopad, 48 bytes, good
+depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES
+cipher_verify_output_multipart:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a6bc1bee22e409f96e93d7e117393172a6bc1bee22e409f96e93d7e117393172a":16
+
PSA symmetric encrypt/decrypt multipart: AES-CBC-PKCS#7, 16 bytes
depends_on:PSA_WANT_ALG_CBC_PKCS7:PSA_WANT_KEY_TYPE_AES
cipher_verify_output_multipart:PSA_ALG_CBC_PKCS7:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":16
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index 05bcf89..f544b41 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -4615,7 +4615,8 @@
PSA_ASSERT(psa_cipher_update(&operation1,
input->x + first_part_size,
input->len - first_part_size,
- output1, output1_buffer_size,
+ output1 + output1_length,
+ output1_buffer_size - output1_length,
&function_output_length));
TEST_LE_U(function_output_length,
PSA_CIPHER_UPDATE_OUTPUT_SIZE(key_type,
@@ -4661,7 +4662,8 @@
PSA_ASSERT(psa_cipher_update(&operation2,
output1 + first_part_size,
output1_length - first_part_size,
- output2, output2_buffer_size,
+ output2 + output2_length,
+ output2_buffer_size - output2_length,
&function_output_length));
TEST_LE_U(function_output_length,
PSA_CIPHER_UPDATE_OUTPUT_SIZE(key_type,