From 110e5341eb322087c9847a9d4041692b5daecb95 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 22 Jan 2025 14:27:22 +0000 Subject: [PATCH 01/21] Add X.509 formatting validation to SECURITY.md Clarify that strict formatting of X.509 certificates is not checked by Mbed TLS and that it therefore should not be used to construct a CA. Signed-off-by: David Horstmann --- SECURITY.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index 732335b233..b4d564ef44 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -135,3 +135,17 @@ Guide](docs/architecture/alternative-implementations.md) for more information. - Use cryptographic mechanisms that are not based on block ciphers. In particular, for authenticated encryption, use ChaCha20/Poly1305 instead of block cipher modes. For random generation, use HMAC\_DRBG instead of CTR\_DRBG. + +#### Formatting of X.509 certificates and certificate signing requests + +When parsing X.509 certificates and certificate signing requests (CSRs), +Mbed TLS does not check that they are strictly compliant with X.509 and other +relevant standards. In the case of signed certificates, the signing party is +assumed to have performed this validation (and the certificate is trusted to +be correctly formatted as long as the signature is correct). +Similarly, CSRs are implicitly trusted by Mbed TLS to be standards-compliant. + +**Warning!** Mbed TLS must not be used to sign untrusted CSRs unless extra +validation is performed separately to ensure that they are compliant to the +relevant specifications. This makes Mbed TLS on its own unsuitable use in a +Certificate Authority (CA). From 2c400fc1a2211991522cb3e66c18647f53dac6bf Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 22 Jan 2025 14:48:58 +0000 Subject: [PATCH 02/21] Add paragraph on undefined behaviour Add a note that we do aim to protect against undefined behaviour and undefined behaviour in certificate parsing is in scope. Signed-off-by: David Horstmann --- SECURITY.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index b4d564ef44..e6d0bbff08 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -149,3 +149,8 @@ Similarly, CSRs are implicitly trusted by Mbed TLS to be standards-compliant. validation is performed separately to ensure that they are compliant to the relevant specifications. This makes Mbed TLS on its own unsuitable use in a Certificate Authority (CA). + +However, Mbed TLS aims to protect against memory corruption and other +undefined behavior when parsing certificates and CSRs. If a CSR or signed +certificate causes undefined behavior when it is parsed by Mbed TLS, that +is considered a security vulnerability. From 09d0b71d2b81972eea56bf254b669d6dc3887ba6 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Thu, 23 Jan 2025 10:28:06 +0000 Subject: [PATCH 03/21] Fix missing-word typo Signed-off-by: David Horstmann --- SECURITY.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index e6d0bbff08..d6c8f43fb6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -147,8 +147,8 @@ Similarly, CSRs are implicitly trusted by Mbed TLS to be standards-compliant. **Warning!** Mbed TLS must not be used to sign untrusted CSRs unless extra validation is performed separately to ensure that they are compliant to the -relevant specifications. This makes Mbed TLS on its own unsuitable use in a -Certificate Authority (CA). +relevant specifications. This makes Mbed TLS on its own unsuitable for use in +a Certificate Authority (CA). However, Mbed TLS aims to protect against memory corruption and other undefined behavior when parsing certificates and CSRs. If a CSR or signed From c8c89eda5dd1a89267cd2ded7ff6219db5b8dc43 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:35:28 +0000 Subject: [PATCH 04/21] Fix psa_key_derivation_input_integer() not detecting bad state Signed-off-by: Waleed Elmelegy --- ChangeLog.d/fix-key-derive-bad-state-error.txt | 3 +++ library/psa_crypto.c | 6 ++++++ tests/suites/test_suite_psa_crypto.function | 10 ++++++++++ 3 files changed, 19 insertions(+) create mode 100644 ChangeLog.d/fix-key-derive-bad-state-error.txt diff --git a/ChangeLog.d/fix-key-derive-bad-state-error.txt b/ChangeLog.d/fix-key-derive-bad-state-error.txt new file mode 100644 index 0000000000..0bccf77682 --- /dev/null +++ b/ChangeLog.d/fix-key-derive-bad-state-error.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix issue where psa_key_derivation_input_integer() is not detecting + bad state after an operation has been aborted. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ec5934e0e0..69d037b8a1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4751,6 +4751,12 @@ static psa_status_t psa_key_derivation_input_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); + if (kdf_alg == 0) { + /* This is a blank or aborted operation. */ + status = PSA_ERROR_BAD_STATE; + goto exit; + } + status = psa_key_derivation_check_input_type(step, key_type); if (status != PSA_SUCCESS) { goto exit; diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 21b768bd3a..838717e60c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -4566,6 +4566,16 @@ void derive_input(int alg_arg, } TEST_EQUAL(actual_output_status, expected_output_status); + /* Test calling input functions after operation has been aborted + result in PSA_ERROR_BAD_STATE error. + */ + psa_key_derivation_abort(&operation); + + TEST_EQUAL(psa_key_derivation_input_bytes( + &operation, steps[0], + inputs[0]->x, inputs[0]->len), + PSA_ERROR_BAD_STATE); + exit: psa_key_derivation_abort(&operation); for (i = 0; i < ARRAY_LENGTH(keys); i++) { From fd01e44cbe4804d2d23d7466f08eda66870db51b Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:42:55 +0000 Subject: [PATCH 05/21] Simplify testing psa_key_derivation_input_*() bad state Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.data | 4 ++++ tests/suites/test_suite_psa_crypto.function | 14 +++----------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 718b10c5dc..eff07ad35e 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -3454,6 +3454,10 @@ PSA key derivation: ECDH on P256 with HKDF-SHA256, missing info depends_on:PSA_WANT_ALG_ECDH:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:PSA_WANT_ECC_SECP_R1_256 derive_input:PSA_ALG_KEY_AGREEMENT(PSA_ALG_ECDH, PSA_ALG_HKDF(PSA_ALG_SHA_256)):PSA_KEY_DERIVATION_INPUT_SALT:PSA_KEY_TYPE_NONE:"":PSA_SUCCESS:PSA_KEY_DERIVATION_INPUT_SECRET:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"c88f01f510d9ac3f70a292daa2316de544e9aab8afe84049c62a9c57862d1433":PSA_SUCCESS:0:UNUSED:"":UNUSED:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +PSA key derivation: reject calling input functions without calling setup +depends_on:PSA_WANT_ALG_SHA_256 +derive_input:0:PSA_KEY_DERIVATION_INPUT_SEED:PSA_KEY_TYPE_NONE:"":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SECRET:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_LABEL:PSA_KEY_TYPE_DERIVE:"61206c6162656c":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE + PSA key derivation over capacity: HKDF depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 derive_over_capacity:PSA_ALG_HKDF(PSA_ALG_SHA_256) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 838717e60c..39dd89ae7a 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -4520,7 +4520,9 @@ void derive_input(int alg_arg, psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DERIVE); psa_set_key_algorithm(&attributes, alg); - PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); + if (alg != 0) { + PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); + } for (i = 0; i < ARRAY_LENGTH(steps); i++) { mbedtls_test_set_step(i); @@ -4566,16 +4568,6 @@ void derive_input(int alg_arg, } TEST_EQUAL(actual_output_status, expected_output_status); - /* Test calling input functions after operation has been aborted - result in PSA_ERROR_BAD_STATE error. - */ - psa_key_derivation_abort(&operation); - - TEST_EQUAL(psa_key_derivation_input_bytes( - &operation, steps[0], - inputs[0]->x, inputs[0]->len), - PSA_ERROR_BAD_STATE); - exit: psa_key_derivation_abort(&operation); for (i = 0; i < ARRAY_LENGTH(keys); i++) { From 76bafb6a330b5e4487984d5200d3b8f16487ed2d Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 13:12:36 +0000 Subject: [PATCH 06/21] Replace zero by PSA_ALG_NONE in key derivation testing Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index eff07ad35e..3881173886 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -3456,7 +3456,7 @@ derive_input:PSA_ALG_KEY_AGREEMENT(PSA_ALG_ECDH, PSA_ALG_HKDF(PSA_ALG_SHA_256)): PSA key derivation: reject calling input functions without calling setup depends_on:PSA_WANT_ALG_SHA_256 -derive_input:0:PSA_KEY_DERIVATION_INPUT_SEED:PSA_KEY_TYPE_NONE:"":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SECRET:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_LABEL:PSA_KEY_TYPE_DERIVE:"61206c6162656c":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE +derive_input:PSA_ALG_NONE:PSA_KEY_DERIVATION_INPUT_SEED:PSA_KEY_TYPE_NONE:"":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_SECRET:PSA_KEY_TYPE_DERIVE:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ERROR_BAD_STATE:PSA_KEY_DERIVATION_INPUT_LABEL:PSA_KEY_TYPE_DERIVE:"61206c6162656c":PSA_ERROR_BAD_STATE:PSA_KEY_TYPE_NONE:PSA_ERROR_BAD_STATE PSA key derivation over capacity: HKDF depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 From 3dee9a92e4b6b5fd56ef9b94f0ab42629bcd13d2 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 12:48:40 +0000 Subject: [PATCH 07/21] Replace zero by PSA_ALG_NONE in key derivation test function Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_psa_crypto.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 39dd89ae7a..837a137a93 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -4520,7 +4520,7 @@ void derive_input(int alg_arg, psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DERIVE); psa_set_key_algorithm(&attributes, alg); - if (alg != 0) { + if (alg != PSA_ALG_NONE) { PSA_ASSERT(psa_key_derivation_setup(&operation, alg)); } From e014887ea57dbe323279598dbeaf704dd3a382df Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 3 Mar 2025 15:01:38 +0000 Subject: [PATCH 08/21] Fix code style for key derivation input function Signed-off-by: Waleed Elmelegy --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 69d037b8a1..55eadc489a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4752,7 +4752,7 @@ static psa_status_t psa_key_derivation_input_internal( psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); if (kdf_alg == 0) { - /* This is a blank or aborted operation. */ + /* This is a blank or aborted operation. */ status = PSA_ERROR_BAD_STATE; goto exit; } From 27da54de49cf28697b0435c2fa42d79e422bd2ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Fri, 28 Feb 2025 16:22:33 +0100 Subject: [PATCH 09/21] Run test_suite_debug without MBEDTLS_SSL_TLS_C MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the suite's global dependency on MBEDTLS_SSL_TLS_C to the individual test cases. Add an preprocesor guard around string_debug to prevent warning about unused functions. Signed-off-by: Bence Szépkúti --- tests/suites/test_suite_debug.function | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index 9ece280715..8a632fcd91 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -7,7 +7,8 @@ struct buffer_data { char *ptr; }; -void string_debug(void *data, int level, const char *file, int line, const char *str) +#if defined(MBEDTLS_SSL_TLS_C) +static void string_debug(void *data, int level, const char *file, int line, const char *str) { struct buffer_data *buffer = (struct buffer_data *) data; char *p = buffer->ptr; @@ -42,14 +43,15 @@ void string_debug(void *data, int level, const char *file, int line, const char buffer->ptr = p; } +#endif /* MBEDTLS_SSL_TLS_C */ /* END_HEADER */ /* BEGIN_DEPENDENCIES - * depends_on:MBEDTLS_DEBUG_C:MBEDTLS_SSL_TLS_C + * depends_on:MBEDTLS_DEBUG_C * END_DEPENDENCIES */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */ void debug_print_msg_threshold(int threshold, int level, char *file, int line, char *result_str) { @@ -80,7 +82,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */ void mbedtls_debug_print_ret(char *file, int line, char *text, int value, char *result_str) { @@ -109,7 +111,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */ void mbedtls_debug_print_buf(char *file, int line, char *text, data_t *data, char *result_str) { @@ -138,7 +140,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ void mbedtls_debug_print_crt(char *crt_file, char *file, int line, char *prefix, char *result_str) { @@ -172,7 +174,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_BIGNUM_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C:MBEDTLS_BIGNUM_C */ void mbedtls_debug_print_mpi(char *value, char *file, int line, char *prefix, char *result_str) { From 94b0eea23f329794c53cc5b1806eb1f86e63377a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Fri, 28 Feb 2025 22:32:15 +0100 Subject: [PATCH 10/21] Test handling of format macros defined in debug.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bence Szépkúti --- tests/suites/test_suite_debug.data | 7 +++++++ tests/suites/test_suite_debug.function | 28 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/tests/suites/test_suite_debug.data b/tests/suites/test_suite_debug.data index 0b88695623..e844ccc7f1 100644 --- a/tests/suites/test_suite_debug.data +++ b/tests/suites/test_suite_debug.data @@ -1,3 +1,10 @@ +# printf_int_expr expects a smuggled string expression as its first parameter +printf "%" MBEDTLS_PRINTF_SIZET, 0 +printf_int_expr:(uintptr_t) "%" MBEDTLS_PRINTF_SIZET:sizeof(size_t):0:"0" + +printf "%" MBEDTLS_PRINTF_LONGLONG, 0 +printf_int_expr:(uintptr_t) "%" MBEDTLS_PRINTF_LONGLONG:sizeof(long long):0:"0" + Debug print msg (threshold 1, level 0) debug_print_msg_threshold:1:0:"MyFile":999:"MyFile(0999)\: Text message, 2 == 2\n" diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index 8a632fcd91..6c58d56bc8 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -51,6 +51,34 @@ static void string_debug(void *data, int level, const char *file, int line, cons * END_DEPENDENCIES */ +/* BEGIN_CASE */ +void printf_int_expr(intmax_t smuggle_format_expr, /* TODO: teach test framework about string expressions */ + intmax_t sizeof_x, intmax_t x, char *result) +{ + const char *format = (char *) ((uintptr_t) smuggle_format_expr); + char *output = NULL; + const size_t n = strlen(result); + + /* Nominal case: buffer just large enough */ + TEST_CALLOC(output, n + 1); + if ((size_t) sizeof_x <= sizeof(int)) { // Any smaller integers would be promoted to an int due to calling a vararg function + TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (int) x)); + } else if (sizeof_x == sizeof(long)) { + TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (long) x)); + } else if (sizeof_x == sizeof(long long)) { + TEST_EQUAL(n, mbedtls_snprintf(output, n + 1, format, (long long) x)); + } else { + TEST_FAIL( + "sizeof_x <= sizeof(int) || sizeof_x == sizeof(long) || sizeof_x == sizeof(long long)"); + } + TEST_MEMORY_COMPARE(result, n + 1, output, n + 1); + +exit: + mbedtls_free(output); + output = NULL; +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SSL_TLS_C */ void debug_print_msg_threshold(int threshold, int level, char *file, int line, char *result_str) From 5d554667c445e1efab9ba129ce11da1fd4252f57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Sun, 2 Mar 2025 01:17:02 +0100 Subject: [PATCH 11/21] Disable fatal assertions in Windows printf tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows CRT treats any invalid format specifiers passed to the CRT as fatal assertion failures. Disable thie behaviour temporarily while testing if the format specifiers we use are supported. Signed-off-by: Bence Szépkúti --- tests/suites/test_suite_debug.function | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index 6c58d56bc8..ed3505a9fc 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -2,6 +2,11 @@ #include "mbedtls/debug.h" #include "string.h" +#if defined(_WIN32) +# include +# include +#endif + struct buffer_data { char buf[2000]; char *ptr; @@ -44,6 +49,23 @@ static void string_debug(void *data, int level, const char *file, int line, cons buffer->ptr = p; } #endif /* MBEDTLS_SSL_TLS_C */ + +#if defined(_WIN32) +static void noop_invalid_parameter_handler( + const wchar_t *expression, + const wchar_t *function, + const wchar_t *file, + unsigned int line, + uintptr_t pReserved) +{ + (void) expression; + (void) function; + (void) file; + (void) line; + (void) pReserved; +} +#endif /* _WIN32 */ + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -55,6 +77,17 @@ static void string_debug(void *data, int level, const char *file, int line, cons void printf_int_expr(intmax_t smuggle_format_expr, /* TODO: teach test framework about string expressions */ intmax_t sizeof_x, intmax_t x, char *result) { +#if defined(_WIN32) + /* Windows treats any invalid format specifiers passsed to the CRT as fatal assertion failures. + Disable this behaviour temporarily, so the rest of the test cases can complete. */ + _invalid_parameter_handler saved_handler = + _set_invalid_parameter_handler(noop_invalid_parameter_handler); + + // Disable assertion pop-up window in Debug builds + int saved_report_mode = _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_REPORT_MODE); + _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG); +#endif + const char *format = (char *) ((uintptr_t) smuggle_format_expr); char *output = NULL; const size_t n = strlen(result); @@ -76,6 +109,13 @@ void printf_int_expr(intmax_t smuggle_format_expr, /* TODO: teach test framework exit: mbedtls_free(output); output = NULL; + +#if defined(_WIN32) + // Restore default Windows behaviour + _set_invalid_parameter_handler(saved_handler); + _CrtSetReportMode(_CRT_ASSERT, saved_report_mode); + (void) saved_report_mode; +#endif } /* END_CASE */ From e7ee902e09ca90173da30b010d4980ee3eced538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Fri, 28 Feb 2025 22:39:09 +0100 Subject: [PATCH 12/21] Fix MSVC version guard for C99 format size specifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Visual Studio 2013 (_MSC_VER == 1800) doesn't support %zu - only use it on 2015 and above (_MSC_VER >= 1900). %ldd works on Visual Studio 2013, but this patch keeps the two macro definitions together, for simplicity's sake. Signed-off-by: Bence Szépkúti --- ChangeLog.d/fix-msvc-version-guard-format-zu.txt | 5 +++++ include/mbedtls/debug.h | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 ChangeLog.d/fix-msvc-version-guard-format-zu.txt diff --git a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt new file mode 100644 index 0000000000..637388ecaa --- /dev/null +++ b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix definition of MBEDTLS_PRINTF_SIZET to prevent runtime crashes that + occurred whenever SSL debugging was enabled on a copy of Mbed TLS built + with Visual Studio 2013. + Fixes #10017. diff --git a/include/mbedtls/debug.h b/include/mbedtls/debug.h index c29c40eee7..33416bb3c1 100644 --- a/include/mbedtls/debug.h +++ b/include/mbedtls/debug.h @@ -108,16 +108,16 @@ * * This module provides debugging functions. */ -#if (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) +#if (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) #include #define MBEDTLS_PRINTF_SIZET PRIuPTR #define MBEDTLS_PRINTF_LONGLONG "I64d" #else \ - /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) */ + /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) */ #define MBEDTLS_PRINTF_SIZET "zu" #define MBEDTLS_PRINTF_LONGLONG "lld" #endif \ - /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1800) */ + /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) */ #ifdef __cplusplus extern "C" { From 85167e041c85aed93ab89c6b81534c5e0c57aeb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Sat, 1 Mar 2025 23:53:47 +0100 Subject: [PATCH 13/21] Remove Everest VS2010 compatibility headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These headers were necessary for compatibility with Visual Studio 2010, and interfere with the system headers on Visual Studio 2013+, eg. when building Mbed TLS using the .sln file shipped with the project. Move the still-required definition of "inline" to callconv.h, where the definition for GCC also lives. Signed-off-by: Bence Szépkúti --- .../include/everest/kremlin/c_endianness.h | 2 ++ .../everest/kremlin/internal/callconv.h | 4 ++- .../everest/include/everest/vs2010/inttypes.h | 36 ------------------- .../everest/include/everest/vs2010/stdbool.h | 31 ---------------- .../fix-msvc-version-guard-format-zu.txt | 3 ++ 5 files changed, 8 insertions(+), 68 deletions(-) delete mode 100644 3rdparty/everest/include/everest/vs2010/inttypes.h delete mode 100644 3rdparty/everest/include/everest/vs2010/stdbool.h diff --git a/3rdparty/everest/include/everest/kremlin/c_endianness.h b/3rdparty/everest/include/everest/kremlin/c_endianness.h index 5cfde5d9ea..1b0d0eb05b 100644 --- a/3rdparty/everest/include/everest/kremlin/c_endianness.h +++ b/3rdparty/everest/include/everest/kremlin/c_endianness.h @@ -7,6 +7,8 @@ #include #include +#include "kremlin/internal/callconv.h" + /******************************************************************************/ /* Implementing C.fst (part 2: endian-ness macros) */ /******************************************************************************/ diff --git a/3rdparty/everest/include/everest/kremlin/internal/callconv.h b/3rdparty/everest/include/everest/kremlin/internal/callconv.h index bf631ff46f..8ff8ca5ae9 100644 --- a/3rdparty/everest/include/everest/kremlin/internal/callconv.h +++ b/3rdparty/everest/include/everest/kremlin/internal/callconv.h @@ -27,8 +27,10 @@ /* Since KreMLin emits the inline keyword unconditionally, we follow the * guidelines at https://gcc.gnu.org/onlinedocs/gcc/Inline.html and make this * __inline__ to ensure the code compiles with -std=c90 and earlier. */ -#ifdef __GNUC__ +#if defined(__GNUC__) # define inline __inline__ +#elif defined(_MSC_VER) +# define inline __inline #endif /* GCC-specific attribute syntax; everyone else gets the standard C inline diff --git a/3rdparty/everest/include/everest/vs2010/inttypes.h b/3rdparty/everest/include/everest/vs2010/inttypes.h deleted file mode 100644 index 77003be0b0..0000000000 --- a/3rdparty/everest/include/everest/vs2010/inttypes.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Custom inttypes.h for VS2010 KreMLin requires these definitions, - * but VS2010 doesn't provide them. - * - * Copyright 2016-2018 INRIA and Microsoft Corporation - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * This file is part of Mbed TLS (https://tls.mbed.org) - */ - -#ifndef _INTTYPES_H_VS2010 -#define _INTTYPES_H_VS2010 - -#include - -#ifdef _MSC_VER -#define inline __inline -#endif - -/* VS2010 unsigned long == 8 bytes */ - -#define PRIu64 "I64u" - -#endif diff --git a/3rdparty/everest/include/everest/vs2010/stdbool.h b/3rdparty/everest/include/everest/vs2010/stdbool.h deleted file mode 100644 index dcae6d80ad..0000000000 --- a/3rdparty/everest/include/everest/vs2010/stdbool.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Custom stdbool.h for VS2010 KreMLin requires these definitions, - * but VS2010 doesn't provide them. - * - * Copyright 2016-2018 INRIA and Microsoft Corporation - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * This file is part of Mbed TLS (https://tls.mbed.org) - */ - -#ifndef _STDBOOL_H_VS2010 -#define _STDBOOL_H_VS2010 - -typedef int bool; - -static bool true = 1; -static bool false = 0; - -#endif diff --git a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt index 637388ecaa..7e68ddb602 100644 --- a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt +++ b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt @@ -3,3 +3,6 @@ Bugfix occurred whenever SSL debugging was enabled on a copy of Mbed TLS built with Visual Studio 2013. Fixes #10017. + * Remove Everest Visual Studio 2010 compatibility headers, which could + interfere with standard CRT headers in certain situations, eg. when + building Mbed TLS with the .sln file shipped with the project. From b4f25121cc1e742aa2acbd5d22d59d6febc3572b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Fri, 7 Mar 2025 17:22:40 +0100 Subject: [PATCH 14/21] Never use %zu on MinGW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bence Szépkúti --- include/mbedtls/debug.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/debug.h b/include/mbedtls/debug.h index 33416bb3c1..a0f19c0ac4 100644 --- a/include/mbedtls/debug.h +++ b/include/mbedtls/debug.h @@ -108,7 +108,7 @@ * * This module provides debugging functions. */ -#if (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) +#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900) #include #define MBEDTLS_PRINTF_SIZET PRIuPTR #define MBEDTLS_PRINTF_LONGLONG "I64d" From ded35000b059f9dfcffc460c5a82366159fd81d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Sat, 8 Mar 2025 00:40:47 +0100 Subject: [PATCH 15/21] Update changelog to call out MinGW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bence Szépkúti --- ChangeLog.d/fix-msvc-version-guard-format-zu.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt index 7e68ddb602..e61cd7d429 100644 --- a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt +++ b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt @@ -1,7 +1,7 @@ Bugfix * Fix definition of MBEDTLS_PRINTF_SIZET to prevent runtime crashes that occurred whenever SSL debugging was enabled on a copy of Mbed TLS built - with Visual Studio 2013. + with Visual Studio 2013 or MinGW. Fixes #10017. * Remove Everest Visual Studio 2010 compatibility headers, which could interfere with standard CRT headers in certain situations, eg. when From af07ab897c8347f38870b6d52408870b60bfa82a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Sat, 8 Mar 2025 01:02:37 +0100 Subject: [PATCH 16/21] Fix comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bence Szépkúti --- include/mbedtls/debug.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/debug.h b/include/mbedtls/debug.h index a0f19c0ac4..1da0726ff3 100644 --- a/include/mbedtls/debug.h +++ b/include/mbedtls/debug.h @@ -113,11 +113,11 @@ #define MBEDTLS_PRINTF_SIZET PRIuPTR #define MBEDTLS_PRINTF_LONGLONG "I64d" #else \ - /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) */ + /* defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900) */ #define MBEDTLS_PRINTF_SIZET "zu" #define MBEDTLS_PRINTF_LONGLONG "lld" #endif \ - /* (defined(__MINGW32__) && __USE_MINGW_ANSI_STDIO == 0) || (defined(_MSC_VER) && _MSC_VER < 1900) */ + /* defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER < 1900) */ #ifdef __cplusplus extern "C" { From 254cadac7027e7f5d8aca9c19868ba2ab54f504c Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 11 Mar 2025 12:27:34 +0000 Subject: [PATCH 17/21] Replace zero by PSA_ALG_NONE in key derivation internal functions Signed-off-by: Waleed Elmelegy --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 55eadc489a..b01f948fac 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4751,7 +4751,7 @@ static psa_status_t psa_key_derivation_input_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); - if (kdf_alg == 0) { + if (kdf_alg == PSA_ALG_NONE) { /* This is a blank or aborted operation. */ status = PSA_ERROR_BAD_STATE; goto exit; From 012ebb01f9e9377cad726a7d6d64c94d5ba421d5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Mar 2025 15:04:05 +0100 Subject: [PATCH 18/21] Document PSA's need for threading Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 84af7f767e..078d5c52bd 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -2290,6 +2290,10 @@ * That is, the APIs enabled by this option are not covered by the usual * promises of API stability. * + * \warning In multithreaded applications, you must also enable + * #MBEDTLS_THREADING_C, unless only one thread ever calls PSA functions + * (`psa_xxx()`), including indirect calls through SSL/TLS, X.509 or PK. + * * Requires: MBEDTLS_PSA_CRYPTO_C. * * Uncomment this to enable internal use of PSA Crypto and new associated APIs. @@ -3389,6 +3393,14 @@ * * Enable the Platform Security Architecture cryptography API. * + * \note In multithreaded applications, you must enable #MBEDTLS_THREADING_C, + * unless only one thread ever calls `psa_xxx()` functions. + * That includes indirect calls, such as: + * - indirect calls from PK, X.509 or SSL functions when + * #MBEDTLS_USE_PSA_CRYPTO is enabled; + * - any other call to a function that requires calling psa_crypto_init() + * beforehand. + * * Module: library/psa_crypto.c * * Requires: either MBEDTLS_CTR_DRBG_C and MBEDTLS_ENTROPY_C, @@ -3605,11 +3617,29 @@ /** * \def MBEDTLS_THREADING_C * - * Enable the threading abstraction layer. - * By default Mbed TLS assumes it is used in a non-threaded environment or that - * contexts are not shared between threads. If you do intend to use contexts + * Traditionally, Mbed TLS assumes it is used in a non-threaded environment or + * that contexts are not shared between threads. If you do intend to use contexts * between threads, you will need to enable this layer to prevent race - * conditions. See also our Knowledge Base article about threading: + * conditions. + * + * The PSA subsystem has an implicit shared context. Therefore, you must + * enable this option if more than one thread may use any part of + * Mbed TLS that is implemented on top of the PSA subsystem. + * + * You must enable this option in multithreaded applications where more than + * one thread performs any of the following operations: + * + * - Any call to a PSA function (`psa_xxx()`). + * - Any call to a TLS, X.509 or PK function (`mbedtls_ssl_xxx()`, + * `mbedtls_x509_xxx()`, `mbedtls_pkcs7_xxx()`, `mbedtls_pk_xxx()`) + * if `MBEDTLS_USE_PSA_CRYPTO` is enabled (regardless of whether individual + * TLS, X.509 or PK contexts are shared between threads). + * - Any use of a cryptographic context if the same context is used in + * multiple threads. + * - Any call to a function where the documentation specifies that + * psa_crypto_init() must be called prior to that function. + * + * See also our Knowledge Base article about threading: * https://mbed-tls.readthedocs.io/en/latest/kb/development/thread-safety-and-multi-threading * * Module: library/threading.c From cfadd96a9b53d363b5da9c33e5b7d138e0b276bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Tue, 11 Mar 2025 17:47:11 +0100 Subject: [PATCH 19/21] Clarify changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove mention of the shipped .sln files, as those are planned to be removed from Mbed TLS. Clarify the affected CRT headers. Signed-off-by: Bence Szépkúti --- ChangeLog.d/fix-msvc-version-guard-format-zu.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt index e61cd7d429..2713f6c9f4 100644 --- a/ChangeLog.d/fix-msvc-version-guard-format-zu.txt +++ b/ChangeLog.d/fix-msvc-version-guard-format-zu.txt @@ -4,5 +4,6 @@ Bugfix with Visual Studio 2013 or MinGW. Fixes #10017. * Remove Everest Visual Studio 2010 compatibility headers, which could - interfere with standard CRT headers in certain situations, eg. when - building Mbed TLS with the .sln file shipped with the project. + shadow standard CRT headers inttypes.h and stdbool.h with incomplete + implementatios if placed on the include path, eg. when building Mbed TLS + with the .sln file shipped with the project. From cb094f91927e763e4546cbe8654953e1a3fe7c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Sz=C3=A9pk=C3=BAti?= Date: Wed, 12 Mar 2025 17:08:46 +0100 Subject: [PATCH 20/21] Use an array of strings instead of pointer smuggling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bence Szépkúti --- tests/suites/test_suite_debug.data | 5 ++--- tests/suites/test_suite_debug.function | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_debug.data b/tests/suites/test_suite_debug.data index e844ccc7f1..734c8e7a03 100644 --- a/tests/suites/test_suite_debug.data +++ b/tests/suites/test_suite_debug.data @@ -1,9 +1,8 @@ -# printf_int_expr expects a smuggled string expression as its first parameter printf "%" MBEDTLS_PRINTF_SIZET, 0 -printf_int_expr:(uintptr_t) "%" MBEDTLS_PRINTF_SIZET:sizeof(size_t):0:"0" +printf_int_expr:PRINTF_SIZET:sizeof(size_t):0:"0" printf "%" MBEDTLS_PRINTF_LONGLONG, 0 -printf_int_expr:(uintptr_t) "%" MBEDTLS_PRINTF_LONGLONG:sizeof(long long):0:"0" +printf_int_expr:PRINTF_LONGLONG:sizeof(long long):0:"0" Debug print msg (threshold 1, level 0) debug_print_msg_threshold:1:0:"MyFile":999:"MyFile(0999)\: Text message, 2 == 2\n" diff --git a/tests/suites/test_suite_debug.function b/tests/suites/test_suite_debug.function index ed3505a9fc..15605c766d 100644 --- a/tests/suites/test_suite_debug.function +++ b/tests/suites/test_suite_debug.function @@ -7,6 +7,16 @@ # include #endif +typedef enum { + PRINTF_SIZET, + PRINTF_LONGLONG, +} printf_format_indicator_t; + +const char *const printf_formats[] = { + [PRINTF_SIZET] = "%" MBEDTLS_PRINTF_SIZET, + [PRINTF_LONGLONG] = "%" MBEDTLS_PRINTF_LONGLONG, +}; + struct buffer_data { char buf[2000]; char *ptr; @@ -74,8 +84,7 @@ static void noop_invalid_parameter_handler( */ /* BEGIN_CASE */ -void printf_int_expr(intmax_t smuggle_format_expr, /* TODO: teach test framework about string expressions */ - intmax_t sizeof_x, intmax_t x, char *result) +void printf_int_expr(int format_indicator, intmax_t sizeof_x, intmax_t x, char *result) { #if defined(_WIN32) /* Windows treats any invalid format specifiers passsed to the CRT as fatal assertion failures. @@ -88,7 +97,7 @@ void printf_int_expr(intmax_t smuggle_format_expr, /* TODO: teach test framework _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG); #endif - const char *format = (char *) ((uintptr_t) smuggle_format_expr); + const char *format = printf_formats[format_indicator]; char *output = NULL; const size_t n = strlen(result); From 1567199c89c975eaf8a3bacd3472d072f0cb80d3 Mon Sep 17 00:00:00 2001 From: Noah Pendleton Date: Fri, 3 May 2024 11:02:22 -0400 Subject: [PATCH 21/21] mbedtls_net_send API description typo fix Signed-off-by: Noah Pendleton --- include/mbedtls/net_sockets.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/net_sockets.h b/include/mbedtls/net_sockets.h index 1a12c9c803..5f5202fc26 100644 --- a/include/mbedtls/net_sockets.h +++ b/include/mbedtls/net_sockets.h @@ -226,7 +226,7 @@ int mbedtls_net_recv(void *ctx, unsigned char *buf, size_t len); /** * \brief Write at most 'len' characters. If no error occurs, - * the actual amount read is returned. + * the actual amount written is returned. * * \param ctx Socket * \param buf The buffer to read from