From 86ba5162950968e8e98571501e202af7b645758e Mon Sep 17 00:00:00 2001 From: Ben Taylor Date: Thu, 20 Aug 2026 15:04:31 +0100 Subject: [PATCH 1/5] Add test for tansform setup resource leak This is a manual backport, not a direct cherry-pick, because 3.6 can build TLS without PSA-backed transforms. Gate the regression on MBEDTLS_USE_PSA_CRYPTO. Signed-off-by: Ben Taylor (cherry picked from commit 95e95268a4c26608612f34f2bb76ff1dfa9ded0a) --- tests/suites/test_suite_ssl.data | 3 + tests/suites/test_suite_ssl.function | 105 +++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 82d2e3c37d..e718f85583 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3003,6 +3003,9 @@ SSL TLS 1.3 Record Encryption RFC 8448 Example #2 depends_on:PSA_WANT_KEY_TYPE_AES:PSA_WANT_ALG_GCM:PSA_WANT_ALG_SHA_256 ssl_tls13_record_protection:MBEDTLS_TLS1_3_AES_128_GCM_SHA256:MBEDTLS_SSL_IS_SERVER:1:1:"9f02283b6c9c07efc26bb9f2ac92e356":"cf782b88dd83549aadf1e984":"17422dda596ed5d9acd890e3c63f5051":"5b78923dee08579033e523d9":"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031":"2e937e11ef4ac740e538ad36005fc4a46932fc3225d05f82aa1b36e30efaf97d90e6dffc602dcb501a59a8fcc49c4bf2e5f0a21c0047c2abf332540dd032e167c2955d" +TLS 1.3 transform frees encryption key if decryption key import fails +ssl_tls13_populate_transform_second_key_import_failure + SSL TLS 1.3 Key schedule: Application secrets derivation helper # Vector from RFC 8448 depends_on:PSA_WANT_ALG_SHA_256 diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 4236c978f9..bdc4b8edf5 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,31 @@ #define TEST_SSL_CONTEXT_LOAD_OOB_OUT_CID 1 #define TEST_SSL_CONTEXT_LOAD_OOB_ALPN 2 +#if defined(MBEDTLS_PSA_KEY_STORE_DYNAMIC) +#if defined(MBEDTLS_TEST_HOOKS) && defined(MBEDTLS_SSL_CLI_C) && \ + defined(MBEDTLS_PSA_CRYPTO_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ + defined(MBEDTLS_USE_PSA_CRYPTO) +#define SSL_TEST_KEY_SLICE_1_LENGTH 4 +#define SSL_TEST_KEY_SLICE_2_LENGTH 10 +static size_t ssl_test_tiny_key_slice_length(size_t slice_idx) +{ + switch (slice_idx) { + case 1: return SSL_TEST_KEY_SLICE_1_LENGTH; + case 2: return SSL_TEST_KEY_SLICE_2_LENGTH; + default: return 1; + } +} +#define SSL_TEST_MAX_VOLATILE_KEYS \ + (SSL_TEST_KEY_SLICE_1_LENGTH + \ + SSL_TEST_KEY_SLICE_2_LENGTH + \ + psa_key_slot_volatile_slice_count() - 2) +#endif /* MBEDTLS_TEST_HOOKS && MBEDTLS_SSL_CLI_C && + MBEDTLS_PSA_CRYPTO_C && MBEDTLS_SSL_PROTO_TLS1_3 && + MBEDTLS_USE_PSA_CRYPTO */ +#else /* MBEDTLS_PSA_KEY_STORE_DYNAMIC */ +#define SSL_TEST_MAX_VOLATILE_KEYS MBEDTLS_PSA_KEY_SLOT_COUNT +#endif /* MBEDTLS_PSA_KEY_STORE_DYNAMIC */ + #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ defined(MBEDTLS_SSL_CLI_C) && defined(MBEDTLS_SSL_SRV_C) && \ @@ -2461,6 +2487,85 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_CLI_C:SSL_TEST_MAX_VOLATILE_KEYS:MBEDTLS_PSA_CRYPTO_C:MBEDTLS_USE_PSA_CRYPTO */ +void ssl_tls13_populate_transform_second_key_import_failure(void) +{ + mbedtls_ssl_key_set traffic_keys; + mbedtls_ssl_transform transform; + mbedtls_svc_key_id_t *keys = NULL; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_psa_stats_t stats_before, stats_after; + psa_status_t status = PSA_SUCCESS; + size_t i = 0; + + memset(&traffic_keys, 0, sizeof(traffic_keys)); + mbedtls_ssl_transform_init(&transform); + +#if defined(MBEDTLS_PSA_KEY_STORE_DYNAMIC) && defined(MBEDTLS_TEST_HOOKS) + mbedtls_test_hook_psa_volatile_key_slice_length = + ssl_test_tiny_key_slice_length; +#endif + + PSA_INIT(); + TEST_CALLOC(keys, SSL_TEST_MAX_VOLATILE_KEYS + 1); + + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_EXPORT); + psa_set_key_type(&attributes, PSA_KEY_TYPE_RAW_DATA); + + /* Allow one more attempt than the maximum number of volatile keys so + * that the loop ends because the key store is full. */ + while (i <= SSL_TEST_MAX_VOLATILE_KEYS) { + status = psa_import_key(&attributes, + (const uint8_t *) &i, sizeof(i), + &keys[i]); + if (status == PSA_ERROR_INSUFFICIENT_MEMORY) { + break; + } + PSA_ASSERT(status); + i++; + } + TEST_EQUAL(status, PSA_ERROR_INSUFFICIENT_MEMORY); + TEST_ASSERT(i > 0); + + /* Leave exactly one slot free: the encryption key import consumes it, + * then the decryption key import fails. */ + PSA_ASSERT(psa_destroy_key(keys[--i])); + keys[i] = MBEDTLS_SVC_KEY_ID_INIT; + mbedtls_psa_get_stats(&stats_before); + + traffic_keys.key_len = 16; + traffic_keys.iv_len = 12; + TEST_ASSERT(mbedtls_ssl_tls13_populate_transform( + &transform, MBEDTLS_SSL_IS_CLIENT, + MBEDTLS_TLS1_3_AES_128_GCM_SHA256, + &traffic_keys, NULL) != 0); + + /* The first import must be rolled back when the second one fails. */ + TEST_ASSERT(mbedtls_svc_key_id_is_null(transform.psa_key_enc)); + + /* Also check the key store itself: the failed operation must neither + * consume a slot nor leave the only free slot unusable. */ + mbedtls_psa_get_stats(&stats_after); + TEST_EQUAL(stats_after.volatile_slots, stats_before.volatile_slots); + TEST_EQUAL(stats_after.empty_slots, stats_before.empty_slots); + PSA_ASSERT(psa_import_key(&attributes, + (const uint8_t *) &i, sizeof(i), + &keys[i])); + i++; + +exit: + mbedtls_ssl_transform_free(&transform); + while (i > 0) { + psa_destroy_key(keys[--i]); + } + mbedtls_free(keys); + PSA_DONE(); +#if defined(MBEDTLS_PSA_KEY_STORE_DYNAMIC) && defined(MBEDTLS_TEST_HOOKS) + mbedtls_test_hook_psa_volatile_key_slice_length = NULL; +#endif +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_3 */ void ssl_tls13_key_evolution(int hash_alg, data_t *secret, From 3626aa2d3482204105bc760b4b80e91ba6efb756 Mon Sep 17 00:00:00 2001 From: Ben Taylor Date: Thu, 20 Aug 2026 15:04:59 +0100 Subject: [PATCH 2/5] Add fix for transform setup resource leak Signed-off-by: Ben Taylor (cherry picked from commit 0ad86e77b2e73100aa65f5148ea865a12caa87f0) --- library/ssl_tls13_keys.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 1967883b61..ef91413bac 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1081,6 +1081,8 @@ int mbedtls_ssl_tls13_populate_transform( &transform->psa_key_dec)) != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_RET( 1, "psa_import_key", PSA_TO_MBEDTLS_ERR(status)); + psa_destroy_key(transform->psa_key_enc); + transform->psa_key_enc = MBEDTLS_SVC_KEY_ID_INIT; return PSA_TO_MBEDTLS_ERR(status); } } From ad282114a1321b22500ba5816d84218315d5c55e Mon Sep 17 00:00:00 2001 From: Ben Taylor Date: Tue, 25 Aug 2026 14:36:02 +0100 Subject: [PATCH 3/5] Add ChangeLog for transform resource leak Signed-off-by: Ben Taylor (cherry picked from commit 5b4be7edaffe86553b4875f80c78c3befe4ae618) --- ChangeLog.d/tls13-transform-resource-leak.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/tls13-transform-resource-leak.txt diff --git a/ChangeLog.d/tls13-transform-resource-leak.txt b/ChangeLog.d/tls13-transform-resource-leak.txt new file mode 100644 index 0000000000..f8008e4723 --- /dev/null +++ b/ChangeLog.d/tls13-transform-resource-leak.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix a resource leak in TLS 1.3 transform setup when importing the + decryption traffic key fails after the encryption traffic key has already + been imported. Fixes #10917. From 23370efdea9590ae9129e8cf011bf466e22c0e8a Mon Sep 17 00:00:00 2001 From: Ben Taylor Date: Mon, 12 Oct 2026 13:41:31 +0100 Subject: [PATCH 4/5] Add comment explaining the derivation of the number of keys Signed-off-by: Ben Taylor (cherry picked from commit e0766f1b4a2133ea043e10cef86cc1c76f69b49b) --- tests/suites/test_suite_ssl.function | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index bdc4b8edf5..418a78bb58 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -49,6 +49,14 @@ static size_t ssl_test_tiny_key_slice_length(size_t slice_idx) default: return 1; } } + +/* In the below definition SSL_TEST_KEY_SLICE_1_LENGTH has 4 slots and + * SSL_TEST_KEY_SLICE_2_LENGTH has 10 all other slices have 1. + * psa_key_slot_volatile_slice_count() returns the total number of slices. + * However we have already countes slices 1 and 2, so we mustr subtract 2 from + * this number to avoid double counting. + */ + #define SSL_TEST_MAX_VOLATILE_KEYS \ (SSL_TEST_KEY_SLICE_1_LENGTH + \ SSL_TEST_KEY_SLICE_2_LENGTH + \ From afa648aa44c698f6f01b1dd653edc3dfbe140355 Mon Sep 17 00:00:00 2001 From: Ben Taylor Date: Thu, 22 Oct 2026 10:53:45 +0100 Subject: [PATCH 5/5] Fix non-PSA TLS 1.3 transform cleanup Signed-off-by: Ben Taylor --- ChangeLog.d/tls13-transform-resource-leak.txt | 6 +++--- library/ssl_tls13_keys.c | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/tls13-transform-resource-leak.txt b/ChangeLog.d/tls13-transform-resource-leak.txt index f8008e4723..98cad09d90 100644 --- a/ChangeLog.d/tls13-transform-resource-leak.txt +++ b/ChangeLog.d/tls13-transform-resource-leak.txt @@ -1,4 +1,4 @@ Bugfix - * Fix a resource leak in TLS 1.3 transform setup when importing the - decryption traffic key fails after the encryption traffic key has already - been imported. Fixes #10917. + * Fix resource leaks in TLS 1.3 transform setup when a setup step fails + after allocating cipher contexts or importing the encryption traffic key. + Fixes #10917. diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index ef91413bac..bd8d8c5508 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -977,6 +977,7 @@ int mbedtls_ssl_tls13_populate_transform( if ((ret = mbedtls_cipher_setup(&transform->cipher_ctx_dec, cipher_info)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_cipher_setup", ret); + mbedtls_ssl_transform_free(transform); return ret; } #endif /* MBEDTLS_USE_PSA_CRYPTO */ @@ -999,6 +1000,9 @@ int mbedtls_ssl_tls13_populate_transform( #endif /* MBEDTLS_SSL_CLI_C */ { /* should not happen */ +#if !defined(MBEDTLS_USE_PSA_CRYPTO) + mbedtls_ssl_transform_free(transform); +#endif return MBEDTLS_ERR_SSL_INTERNAL_ERROR; } @@ -1010,6 +1014,7 @@ int mbedtls_ssl_tls13_populate_transform( key_enc, (int) mbedtls_cipher_info_get_key_bitlen(cipher_info), MBEDTLS_ENCRYPT)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_cipher_setkey", ret); + mbedtls_ssl_transform_free(transform); return ret; } @@ -1017,6 +1022,7 @@ int mbedtls_ssl_tls13_populate_transform( key_dec, (int) mbedtls_cipher_info_get_key_bitlen(cipher_info), MBEDTLS_DECRYPT)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_cipher_setkey", ret); + mbedtls_ssl_transform_free(transform); return ret; } #endif /* MBEDTLS_USE_PSA_CRYPTO */