From 3f21ca7f16744d8e88b8b5fd3316216e3bd46f3d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 1 Jul 2024 21:14:45 +0200 Subject: [PATCH 01/44] Document that MBEDTLS_PSA_HMAC_DRBG_MD_TYPE does not force HMAC MBEDTLS_PSA_HMAC_DRBG_MD_TYPE was documented and announced as causing the PSA DRBG to be HMAC_DRBG. However, that was never actually implemented: CTR_DRBG is prioritized if enabled. Since there is a simple workaround of disabling MBEDTLS_CTR_DRBG_C if you want to use HMAC_DRBG, we have decided to accept the actual behavior and fix the documentation. Signed-off-by: Gilles Peskine --- ChangeLog.d/MBEDTLS_PSA_HMAC_DRBG_MD_TYPE.txt | 4 ++++ include/mbedtls/mbedtls_config.h | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 ChangeLog.d/MBEDTLS_PSA_HMAC_DRBG_MD_TYPE.txt diff --git a/ChangeLog.d/MBEDTLS_PSA_HMAC_DRBG_MD_TYPE.txt b/ChangeLog.d/MBEDTLS_PSA_HMAC_DRBG_MD_TYPE.txt new file mode 100644 index 0000000000..079cd741dc --- /dev/null +++ b/ChangeLog.d/MBEDTLS_PSA_HMAC_DRBG_MD_TYPE.txt @@ -0,0 +1,4 @@ +Security + * Unlike previously documented, enabling MBEDTLS_PSA_HMAC_DRBG_MD_TYPE does + not cause the PSA subsystem to use HMAC_DRBG: it uses HMAC_DRBG only when + MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG and MBEDTLS_CTR_DRBG_C are disabled. diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 35921412c6..871d54e48b 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -4016,11 +4016,18 @@ * Use HMAC_DRBG with the specified hash algorithm for HMAC_DRBG for the * PSA crypto subsystem. * - * If this option is unset: - * - If CTR_DRBG is available, the PSA subsystem uses it rather than HMAC_DRBG. - * - Otherwise, the PSA subsystem uses HMAC_DRBG with either - * #MBEDTLS_MD_SHA512 or #MBEDTLS_MD_SHA256 based on availability and - * on unspecified heuristics. + * If this option is unset, the library chooses a hash (currently between + * #MBEDTLS_MD_SHA512 and #MBEDTLS_MD_SHA256) based on availability and + * unspecified heuristics. + * + * \note The PSA crypto subsystem uses the first available mechanism amongst + * the following: + * - #MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG if enabled; + * - Entropy from #MBEDTLS_ENTROPY_C plus CTR_DRBG with AES + * if #MBEDTLS_CTR_DRBG_C is enabled; + * - Entropy from #MBEDTLS_ENTROPY_C plus HMAC_DRBG. + * + * A future version may reevaluate the prioritization of DRBG mechanisms. */ //#define MBEDTLS_PSA_HMAC_DRBG_MD_TYPE MBEDTLS_MD_SHA256 From 5d660396ecfc1a915d7669e5ec7a553ac68649c6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 2 Jul 2024 15:47:22 +0200 Subject: [PATCH 02/44] Force MBEDTLS_PSA_HMAC_DRBG_MD_TYPE based on CTR_DRBG If MBEDTLS_CTR_DRBG_C is enabled, force MBEDTLS_PSA_HMAC_DRBG_MD_TYPE to be disabled. This resolves the former inconsistency in builds where MBEDTLS_PSA_HMAC_DRBG_MD_TYPE is explicitly defined but MBEDTLS_CTR_DRBG_C remains enabled, where PSA called the CTR_DRBG functions but other parts of the code based assumed that HMAC was in use, in particular error code conversions (leading to a test failure in test_suite_psa_crypto_init). Signed-off-by: Gilles Peskine --- library/psa_crypto_random_impl.h | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/library/psa_crypto_random_impl.h b/library/psa_crypto_random_impl.h index 533fb2e940..5b5163111b 100644 --- a/library/psa_crypto_random_impl.h +++ b/library/psa_crypto_random_impl.h @@ -21,13 +21,10 @@ typedef mbedtls_psa_external_random_context_t mbedtls_psa_random_context_t; #include "mbedtls/entropy.h" /* Choose a DRBG based on configuration and availability */ -#if defined(MBEDTLS_PSA_HMAC_DRBG_MD_TYPE) - -#include "mbedtls/hmac_drbg.h" - -#elif defined(MBEDTLS_CTR_DRBG_C) +#if defined(MBEDTLS_CTR_DRBG_C) #include "mbedtls/ctr_drbg.h" +#undef MBEDTLS_PSA_HMAC_DRBG_MD_TYPE #elif defined(MBEDTLS_HMAC_DRBG_C) @@ -49,17 +46,11 @@ typedef mbedtls_psa_external_random_context_t mbedtls_psa_random_context_t; #error "No hash algorithm available for HMAC_DBRG." #endif -#else /* !MBEDTLS_PSA_HMAC_DRBG_MD_TYPE && !MBEDTLS_CTR_DRBG_C && !MBEDTLS_HMAC_DRBG_C*/ +#else /* !MBEDTLS_CTR_DRBG_C && !MBEDTLS_HMAC_DRBG_C*/ #error "No DRBG module available for the psa_crypto module." -#endif /* !MBEDTLS_PSA_HMAC_DRBG_MD_TYPE && !MBEDTLS_CTR_DRBG_C && !MBEDTLS_HMAC_DRBG_C*/ - -#if defined(MBEDTLS_CTR_DRBG_C) -#include "mbedtls/ctr_drbg.h" -#elif defined(MBEDTLS_HMAC_DRBG_C) -#include "mbedtls/hmac_drbg.h" -#endif /* !MBEDTLS_CTR_DRBG_C && !MBEDTLS_HMAC_DRBG_C */ +#endif /* !MBEDTLS_CTR_DRBG_C && !MBEDTLS_HMAC_DRBG_C*/ /* The maximum number of bytes that mbedtls_psa_get_random() is expected to return. */ #if defined(MBEDTLS_CTR_DRBG_C) From 4269ee6f2dbc38656f4415110bb423fff3f157ff Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2024 23:32:50 +0200 Subject: [PATCH 03/44] Fix stack buffer overflow in ECDSA signature format conversions Signed-off-by: Gilles Peskine --- ChangeLog.d/ecdsa-conversion-overflow.txt | 4 ++++ library/psa_util.c | 6 ++++++ tests/suites/test_suite_psa_crypto_util.data | 6 ++++++ 3 files changed, 16 insertions(+) create mode 100644 ChangeLog.d/ecdsa-conversion-overflow.txt diff --git a/ChangeLog.d/ecdsa-conversion-overflow.txt b/ChangeLog.d/ecdsa-conversion-overflow.txt new file mode 100644 index 0000000000..00cac06513 --- /dev/null +++ b/ChangeLog.d/ecdsa-conversion-overflow.txt @@ -0,0 +1,4 @@ +Security + * Fix a stack buffer overflow in mbedtls_ecdsa_der_to_raw() and + mbedtls_ecdsa_raw_to_der() when curve_bits is larger than the + largest supported curve. diff --git a/library/psa_util.c b/library/psa_util.c index 4ccc5b05d8..679d00ea9b 100644 --- a/library/psa_util.c +++ b/library/psa_util.c @@ -443,6 +443,9 @@ int mbedtls_ecdsa_raw_to_der(size_t bits, const unsigned char *raw, size_t raw_l if (raw_len != (2 * coordinate_len)) { return MBEDTLS_ERR_ASN1_INVALID_DATA; } + if (coordinate_len > sizeof(r)) { + return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } /* Since raw and der buffers might overlap, dump r and s before starting * the conversion. */ @@ -561,6 +564,9 @@ int mbedtls_ecdsa_der_to_raw(size_t bits, const unsigned char *der, size_t der_l if (raw_size < coordinate_size * 2) { return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; } + if (2 * coordinate_size > sizeof(raw_tmp)) { + return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } /* Check that the provided input DER buffer has the right header. */ ret = mbedtls_asn1_get_tag(&p, der + der_len, &data_len, diff --git a/tests/suites/test_suite_psa_crypto_util.data b/tests/suites/test_suite_psa_crypto_util.data index 807007b5e6..34e5065d33 100644 --- a/tests/suites/test_suite_psa_crypto_util.data +++ b/tests/suites/test_suite_psa_crypto_util.data @@ -6,6 +6,9 @@ ECDSA Raw -> DER, 256bit, DER buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":"304402201111111111111111111111111111111111111111111111111111111111111111022022222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +ECDSA Raw -> DER, very large input (544-bit) +ecdsa_raw_to_der:544:"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"deadbeef":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + ECDSA Raw -> DER, 256bit, Null r depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"00000000000000000000000000000000000000000000000000000000000000002222222222222222222222222222222222222222222222222222222222222222":"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_INVALID_DATA @@ -58,6 +61,9 @@ ECDSA DER -> Raw, 256bit, Raw buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +ECDSA DER -> Raw, very large input (544-bit) +ecdsa_der_to_raw:544:"30818c0244111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111102442222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + ECDSA DER -> Raw, 256bit, Wrong sequence tag depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"40440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG From 6907e6ceecede6283cde903d2975c2c6eccb5733 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:46:25 +0200 Subject: [PATCH 04/44] More diversified sizes in tests Test the minimum size that caused an overflow in all configurations, and also a mostly arbitrary larger size. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_psa_crypto_util.data | 22 ++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_util.data b/tests/suites/test_suite_psa_crypto_util.data index 34e5065d33..c84a8368cd 100644 --- a/tests/suites/test_suite_psa_crypto_util.data +++ b/tests/suites/test_suite_psa_crypto_util.data @@ -6,8 +6,15 @@ ECDSA Raw -> DER, 256bit, DER buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_raw_to_der:256:"11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222":"304402201111111111111111111111111111111111111111111111111111111111111111022022222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL -ECDSA Raw -> DER, very large input (544-bit) -ecdsa_raw_to_der:544:"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"deadbeef":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +# Check coordinates one byte larger than the largest supported curve. +# If we add an even larger curve, this test case will fail in the full +# configuration because mbedtls_ecdsa_raw_to_der() will return 0, and we'll +# need to use larger data for this test case. +ECDSA Raw -> DER, very large input (536-bit) +ecdsa_raw_to_der:536:"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"30818a024311111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111024322222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + +ECDSA Raw -> DER, very large input (1016-bit) +ecdsa_raw_to_der:1016:"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"30820102027f11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111027f22222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ECDSA Raw -> DER, 256bit, Null r depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 @@ -61,8 +68,15 @@ ECDSA DER -> Raw, 256bit, Raw buffer too small depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 ecdsa_der_to_raw:256:"30440220111111111111111111111111111111111111111111111111111111111111111102202222222222222222222222222222222222222222222222222222222222222222":"111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL -ECDSA DER -> Raw, very large input (544-bit) -ecdsa_der_to_raw:544:"30818c0244111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111102442222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL +# Check coordinates one byte larger than the largest supported curve. +# If we add an even larger curve, this test case will fail in the full +# configuration because mbedtls_ecdsa_der_to_raw() will return 0, and we'll +# need to use larger data for this test case. +ECDSA DER -> Raw, very large input (536-bit) +ecdsa_der_to_raw:536:"30818a024311111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111024322222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL + +ECDSA DER -> Raw, very large input (1016-bit) +ecdsa_der_to_raw:1016:"30820102027f11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111027f22222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ECDSA DER -> Raw, 256bit, Wrong sequence tag depends_on:PSA_VENDOR_ECC_MAX_CURVE_BITS >= 256 From 79d1cadbcb4d0f44f42fe44e5c962c8dd14889d8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:59:55 +0200 Subject: [PATCH 05/44] Improve description of who is affected Signed-off-by: Gilles Peskine --- ChangeLog.d/ecdsa-conversion-overflow.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/ecdsa-conversion-overflow.txt b/ChangeLog.d/ecdsa-conversion-overflow.txt index 00cac06513..83b7f2f88b 100644 --- a/ChangeLog.d/ecdsa-conversion-overflow.txt +++ b/ChangeLog.d/ecdsa-conversion-overflow.txt @@ -1,4 +1,6 @@ Security * Fix a stack buffer overflow in mbedtls_ecdsa_der_to_raw() and - mbedtls_ecdsa_raw_to_der() when curve_bits is larger than the - largest supported curve. + mbedtls_ecdsa_raw_to_der() when the bits parameter is larger than the + largest supported curve. In some configurations with PSA disabled, + all values of bits are affected. This never happens in internal library + calls, but can affect applications that call these functions directly. From ee1715cb5b7edf166b5e88a7532820885df79705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 5 Aug 2024 12:49:57 +0200 Subject: [PATCH 06/44] Test cert alert KEY_USAGE -> UNSUPPORTED_CERT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In terms of line coverage, this was covered, except we never checked the behaviour was as intended. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0b8f129048..22e6d5ea6e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7721,22 +7721,26 @@ run_test "keyUsage cli: KeyEncipherment, RSA: OK" \ run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ - "$P_CLI debug_level=1 \ + "$P_CLI debug_level=3 \ force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is TLS-" + -C "Ciphersuite is TLS-" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail, soft" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ - "$P_CLI debug_level=1 auth_mode=optional \ + "$P_CLI debug_level=3 auth_mode=optional \ force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ 0 \ -c "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" \ + -C "send alert level=2 message=43" \ -c "! Usage does not match the keyUsage extension" run_test "keyUsage cli: DigitalSignature, DHE-RSA: OK" \ @@ -7752,22 +7756,26 @@ run_test "keyUsage cli: DigitalSignature, DHE-RSA: OK" \ run_test "keyUsage cli: DigitalSignature, RSA: fail" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ - "$P_CLI debug_level=1 \ + "$P_CLI debug_level=3 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is TLS-" + -C "Ciphersuite is TLS-" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT run_test "keyUsage cli: DigitalSignature, RSA: fail, soft" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ - "$P_CLI debug_level=1 auth_mode=optional \ + "$P_CLI debug_level=3 auth_mode=optional \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ 0 \ -c "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" \ + -C "send alert level=2 message=43" \ -c "! Usage does not match the keyUsage extension" requires_openssl_tls1_3_with_compatible_ephemeral From 36d1b4a80fa6ee346cfa81d1984be1c82431c247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Aug 2024 12:14:04 +0200 Subject: [PATCH 07/44] Rationalize ssl-opt tests for keyUsage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - consistent naming with explicit version - in each section, have a positive case with just the needed bit set, and one with an irrelevant bit set in addition (cli 1.3 only had the former, and cli-auth 1.3 only the later) - when auth_mode optional is supported failing cases should come in pairs: soft+hard, this wasn't the case for cli-auth 1.3. (Note: cli 1.3 currently does not support auth_mode optional.) - failing cases should check that the correct flag is printed and the expected alert is sent. The last (two) points have uncovered a bug in 1.3 code: - In fail (hard) cases the correct alert isn't send, but a more generic one instead. - In fail (soft) cases the issue with the certificate is not reported, actually the certificate is reported as valid. Both share the same root cause: the flags are not updated properly when checking the keyUsage extension. This will be addressed in future commits. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 150 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 126 insertions(+), 24 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 22e6d5ea6e..b37747e914 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7640,22 +7640,26 @@ run_test "ALPN: both, no common" \ # Tests for keyUsage in leaf certificates, part 1: # server-side certificate/suite selection +# +# This is only about 1.2 (for 1.3, all key exchanges use signatures). +# In 4.0 this will probably go away as all TLS 1.2 key exchanges will use +# signatures too, following the removal of RSA #8170 and static ECDH #9201. -run_test "keyUsage srv: RSA, digitalSignature -> (EC)DHE-RSA" \ +run_test "keyUsage srv 1.2: RSA, digitalSignature -> (EC)DHE-RSA" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server2.key \ crt_file=$DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI" \ 0 \ -c "Ciphersuite is TLS-[EC]*DHE-RSA-WITH-" -run_test "keyUsage srv: RSA, keyEncipherment -> RSA" \ +run_test "keyUsage srv 1.2: RSA, keyEncipherment -> RSA" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server2.key \ crt_file=$DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI" \ 0 \ -c "Ciphersuite is TLS-RSA-WITH-" -run_test "keyUsage srv: RSA, keyAgreement -> fail" \ +run_test "keyUsage srv 1.2: RSA, keyAgreement -> fail" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server2.key \ crt_file=$DATA_FILES_PATH/server2.ku-ka.crt" \ "$P_CLI" \ @@ -7663,7 +7667,7 @@ run_test "keyUsage srv: RSA, keyAgreement -> fail" \ -C "Ciphersuite is " requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED -run_test "keyUsage srv: ECDSA, digitalSignature -> ECDHE-ECDSA" \ +run_test "keyUsage srv 1.2: ECC, digitalSignature -> ECDHE-ECDSA" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server5.key \ crt_file=$DATA_FILES_PATH/server5.ku-ds.crt" \ "$P_CLI" \ @@ -7671,14 +7675,14 @@ run_test "keyUsage srv: ECDSA, digitalSignature -> ECDHE-ECDSA" \ -c "Ciphersuite is TLS-ECDHE-ECDSA-WITH-" -run_test "keyUsage srv: ECDSA, keyAgreement -> ECDH-" \ +run_test "keyUsage srv 1.2: ECC, keyAgreement -> ECDH-" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server5.key \ crt_file=$DATA_FILES_PATH/server5.ku-ka.crt" \ "$P_CLI" \ 0 \ -c "Ciphersuite is TLS-ECDH-" -run_test "keyUsage srv: ECDSA, keyEncipherment -> fail" \ +run_test "keyUsage srv 1.2: ECC, keyEncipherment -> fail" \ "$P_SRV force_version=tls12 key_file=$DATA_FILES_PATH/server5.key \ crt_file=$DATA_FILES_PATH/server5.ku-ke.crt" \ "$P_CLI" \ @@ -7687,8 +7691,12 @@ run_test "keyUsage srv: ECDSA, keyEncipherment -> fail" \ # Tests for keyUsage in leaf certificates, part 2: # client-side checking of server cert +# +# TLS 1.3 uses only signature, but for 1.2 it depends on the key exchange. +# In 4.0 this will probably change as all TLS 1.2 key exchanges will use +# signatures too, following the removal of RSA #8170 and static ECDH #9201. -run_test "keyUsage cli: DigitalSignature+KeyEncipherment, RSA: OK" \ +run_test "keyUsage cli 1.2: DigitalSignature+KeyEncipherment, RSA: OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds_ke.crt" \ "$P_CLI debug_level=1 \ @@ -7698,7 +7706,7 @@ run_test "keyUsage cli: DigitalSignature+KeyEncipherment, RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli: DigitalSignature+KeyEncipherment, DHE-RSA: OK" \ +run_test "keyUsage cli 1.2: DigitalSignature+KeyEncipherment, DHE-RSA: OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds_ke.crt" \ "$P_CLI debug_level=1 \ @@ -7708,7 +7716,7 @@ run_test "keyUsage cli: DigitalSignature+KeyEncipherment, DHE-RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli: KeyEncipherment, RSA: OK" \ +run_test "keyUsage cli 1.2: KeyEncipherment, RSA: OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI debug_level=1 \ @@ -7718,7 +7726,7 @@ run_test "keyUsage cli: KeyEncipherment, RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail" \ +run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI debug_level=3 \ @@ -7731,7 +7739,7 @@ run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail" \ -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT -run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail, soft" \ +run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail, soft" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI debug_level=3 auth_mode=optional \ @@ -7743,7 +7751,7 @@ run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail, soft" \ -C "send alert level=2 message=43" \ -c "! Usage does not match the keyUsage extension" -run_test "keyUsage cli: DigitalSignature, DHE-RSA: OK" \ +run_test "keyUsage cli 1.2: DigitalSignature, DHE-RSA: OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI debug_level=1 \ @@ -7753,7 +7761,7 @@ run_test "keyUsage cli: DigitalSignature, DHE-RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli: DigitalSignature, RSA: fail" \ +run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI debug_level=3 \ @@ -7766,7 +7774,7 @@ run_test "keyUsage cli: DigitalSignature, RSA: fail" \ -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT -run_test "keyUsage cli: DigitalSignature, RSA: fail, soft" \ +run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail, soft" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI debug_level=3 auth_mode=optional \ @@ -7778,6 +7786,18 @@ run_test "keyUsage cli: DigitalSignature, RSA: fail, soft" \ -C "send alert level=2 message=43" \ -c "! Usage does not match the keyUsage extension" +requires_openssl_tls1_3_with_compatible_ephemeral +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "keyUsage cli 1.3: DigitalSignature, RSA: OK" \ + "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server2.key \ + -cert $DATA_FILES_PATH/server2-sha256.ku-ds.crt" \ + "$P_CLI debug_level=3" \ + 0 \ + -C "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is" + requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED @@ -7801,6 +7821,9 @@ run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" + #-c "send alert level=2 message=43" \ + #-C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -7813,6 +7836,9 @@ run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail" \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" + #-c "send alert level=2 message=43" \ + #-C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -7837,6 +7863,9 @@ run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" + #-c "send alert level=2 message=43" \ + #-C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -7849,12 +7878,17 @@ run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail" \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" + #-c "send alert level=2 message=43" \ + #-C "! Usage does not match the keyUsage extension" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # Tests for keyUsage in leaf certificates, part 3: # server-side checking of client cert +# +# Here, both 1.2 and 1.3 only use signatures. requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli-auth: RSA, DigitalSignature: OK" \ +run_test "keyUsage cli-auth 1.2: RSA, DigitalSignature: OK" \ "$P_SRV debug_level=1 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ @@ -7864,25 +7898,29 @@ run_test "keyUsage cli-auth: RSA, DigitalSignature: OK" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli-auth: RSA, KeyEncipherment: fail (soft)" \ - "$P_SRV debug_level=1 auth_mode=optional" \ +run_test "keyUsage cli-auth 1.2: RSA, KeyEncipherment: fail (soft)" \ + "$P_SRV debug_level=3 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ 0 \ -s "bad certificate (usage extensions)" \ + -S "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli-auth: RSA, KeyEncipherment: fail (hard)" \ - "$P_SRV debug_level=1 force_version=tls12 auth_mode=required" \ +run_test "keyUsage cli-auth 1.2: RSA, KeyEncipherment: fail (hard)" \ + "$P_SRV debug_level=3 force_version=tls12 auth_mode=required" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ 1 \ -s "bad certificate (usage extensions)" \ + -s "send alert level=2 message=43" \ -s "Processing of the Certificate handshake message failed" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli-auth: ECDSA, DigitalSignature: OK" \ +run_test "keyUsage cli-auth 1.2: ECDSA, DigitalSignature: OK" \ "$P_SRV debug_level=1 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ds.crt" \ @@ -7892,14 +7930,27 @@ run_test "keyUsage cli-auth: ECDSA, DigitalSignature: OK" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "keyUsage cli-auth: ECDSA, KeyAgreement: fail (soft)" \ - "$P_SRV debug_level=1 auth_mode=optional" \ +run_test "keyUsage cli-auth 1.2: ECDSA, KeyAgreement: fail (soft)" \ + "$P_SRV debug_level=3 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ 0 \ -s "bad certificate (usage extensions)" \ + -S "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -S "Processing of the Certificate handshake message failed" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.2: ECDSA, KeyAgreement: fail (hard)" \ + "$P_SRV debug_level=3 auth_mode=required" \ + "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ + -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ + 1 \ + -s "bad certificate (usage extensions)" \ + -s "send alert level=2 message=43" \ + -s "Processing of the Certificate handshake message failed" + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT + requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED @@ -7915,13 +7966,45 @@ run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature: OK" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (soft)" \ +run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature+KeyEnciphermen: OK" \ "$P_SRV debug_level=1 force_version=tls13 auth_mode=optional" \ + "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server2.key \ + -cert $DATA_FILES_PATH/server2-sha256.ku-ds_ke.crt" \ + 0 \ + -s "Verifying peer X.509 certificate... ok" \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +requires_openssl_tls1_3_with_compatible_ephemeral +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (soft)" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=optional" \ "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ 0 \ -s "bad certificate (usage extensions)" \ + -S "send alert level=2 message=43" \ -S "Processing of the Certificate handshake message failed" + #-s "! Usage does not match the keyUsage extension" \ + +requires_openssl_tls1_3_with_compatible_ephemeral +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (hard)" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=required" \ + "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server2.key \ + -cert $DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -s "Processing of the Certificate handshake message failed" \ + -s "! mbedtls_ssl_handshake returned" \ + #-s "send alert level=2 message=43" \ + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT + # (not working now, getting alert 46 instead) + # + # OpenSSL client does not seem to mind that the server aborts the + # handshake with a fatal alert and still exits 0... requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -7939,12 +8022,31 @@ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (soft)" \ - "$P_SRV debug_level=1 force_version=tls13 auth_mode=optional" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=optional" \ "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ 0 \ -s "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" + #-s "! Usage does not match the keyUsage extension" \ + +requires_openssl_tls1_3_with_compatible_ephemeral +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (hard)" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=required" \ + "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server5.key \ + -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -s "Processing of the Certificate handshake message failed" \ + -s "! mbedtls_ssl_handshake returned" + #-s "send alert level=2 message=43" \ + # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT + # (not working now, getting alert 46 instead) + # + # OpenSSL client does not seem to mind that the server aborts the + # handshake with a fatal alert and still exits 0... # Tests for extendedKeyUsage, part 1: server-side certificate/suite selection From ef41d8ccbe91c5c59e32f47ceda3365525c47124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 8 Aug 2024 10:28:56 +0200 Subject: [PATCH 08/44] Fix 1.3 failure to update flags for (ext)KeyUsage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 18 +++++++++++---- tests/ssl-opt.sh | 44 ++++++++++++++++++------------------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 8ac6579e05..651a17b5a2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -714,6 +714,18 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) /* * Secondary checks: always done, but change 'ret' only if it was 0 */ + /* keyUsage */ + if ((mbedtls_x509_crt_check_key_usage( + ssl->session_negotiate->peer_cert, + MBEDTLS_X509_KU_DIGITAL_SIGNATURE) != 0)) { + MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); + if (ret == 0) { + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } + verify_result |= MBEDTLS_X509_BADCERT_KEY_USAGE; + } + + /* extKeyUsage */ if (ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT) { ext_oid = MBEDTLS_OID_SERVER_AUTH; ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); @@ -722,16 +734,14 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_CLIENT_AUTH); } - if ((mbedtls_x509_crt_check_key_usage( - ssl->session_negotiate->peer_cert, - MBEDTLS_X509_KU_DIGITAL_SIGNATURE) != 0) || - (mbedtls_x509_crt_check_extended_key_usage( + if ((mbedtls_x509_crt_check_extended_key_usage( ssl->session_negotiate->peer_cert, ext_oid, ext_len) != 0)) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); if (ret == 0) { ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; } + verify_result |= MBEDTLS_X509_BADCERT_EXT_KEY_USAGE; } /* mbedtls_x509_crt_verify_with_profile is supposed to report a diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b37747e914..895d8fcb36 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7816,13 +7816,13 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is" - #-c "send alert level=2 message=43" \ - #-C "! Usage does not match the keyUsage extension" + -C "Ciphersuite is" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7831,13 +7831,13 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ka.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is" - #-c "send alert level=2 message=43" \ - #-C "! Usage does not match the keyUsage extension" + -C "Ciphersuite is" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7858,13 +7858,13 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ke.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is" - #-c "send alert level=2 message=43" \ - #-C "! Usage does not match the keyUsage extension" + -C "Ciphersuite is" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7873,13 +7873,13 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is" - #-c "send alert level=2 message=43" \ - #-C "! Usage does not match the keyUsage extension" + -C "Ciphersuite is" \ + -c "send alert level=2 message=43" \ + -C "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # Tests for keyUsage in leaf certificates, part 3: @@ -7985,8 +7985,8 @@ run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (soft)" \ 0 \ -s "bad certificate (usage extensions)" \ -S "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -S "Processing of the Certificate handshake message failed" - #-s "! Usage does not match the keyUsage extension" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -7998,10 +7998,9 @@ run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (hard)" \ 0 \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ - -s "! mbedtls_ssl_handshake returned" \ - #-s "send alert level=2 message=43" \ + -s "send alert level=2 message=43" \ + -s "! mbedtls_ssl_handshake returned" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT - # (not working now, getting alert 46 instead) # # OpenSSL client does not seem to mind that the server aborts the # handshake with a fatal alert and still exits 0... @@ -8027,8 +8026,8 @@ run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (soft)" \ -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ 0 \ -s "bad certificate (usage extensions)" \ + -s "! Usage does not match the keyUsage extension" \ -S "Processing of the Certificate handshake message failed" - #-s "! Usage does not match the keyUsage extension" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -8040,10 +8039,9 @@ run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (hard)" \ 0 \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ + -s "send alert level=2 message=43" \ -s "! mbedtls_ssl_handshake returned" - #-s "send alert level=2 message=43" \ # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT - # (not working now, getting alert 46 instead) # # OpenSSL client does not seem to mind that the server aborts the # handshake with a fatal alert and still exits 0... From 013d0798c0b5f72661edee4a7adf48bd5b26f671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 8 Aug 2024 10:56:41 +0200 Subject: [PATCH 09/44] Always print detailed cert errors in test programs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the client was only printing them on handshake success, and the server was printing them on success and some but not all failures. This makes ssl-opt.sh more consistent as we can always check for the presence of the expected message in the output, regardless of whether the failure is hard or soft. Signed-off-by: Manuel Pégourié-Gonnard --- programs/ssl/ssl_client2.c | 10 +++++++++- programs/ssl/ssl_server2.c | 3 ++- tests/ssl-opt.sh | 16 ++++++++++------ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 43133d901c..64564ab07c 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -2204,7 +2204,9 @@ usage: ret != MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) { mbedtls_printf(" failed\n ! mbedtls_ssl_handshake returned -0x%x\n", (unsigned int) -ret); - if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) { +#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) + if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || + ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE) { mbedtls_printf( " Unable to verify the server's certificate. " "Either it is invalid,\n" @@ -2215,7 +2217,13 @@ usage: "not using TLS 1.3.\n" " For TLS 1.3 server, try `ca_path=/etc/ssl/certs/`" "or other folder that has root certificates\n"); + + flags = mbedtls_ssl_get_verify_result(&ssl); + char vrfy_buf[512]; + x509_crt_verify_info(vrfy_buf, sizeof(vrfy_buf), " ! ", flags); + mbedtls_printf("%s\n", vrfy_buf); } +#endif mbedtls_printf("\n"); goto exit; } diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index a5d2ed1020..0f871f7123 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -3504,7 +3504,8 @@ handshake: (unsigned int) -ret); #if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) - if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) { + if (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || + ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE) { char vrfy_buf[512]; flags = mbedtls_ssl_get_verify_result(&ssl); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 895d8fcb36..69568058bc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7736,7 +7736,7 @@ run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail, soft" \ @@ -7771,7 +7771,7 @@ run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail, soft" \ @@ -7822,7 +7822,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7837,7 +7837,7 @@ run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7864,7 +7864,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral @@ -7879,7 +7879,7 @@ run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is" \ -c "send alert level=2 message=43" \ - -C "! Usage does not match the keyUsage extension" + -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # Tests for keyUsage in leaf certificates, part 3: @@ -7916,6 +7916,7 @@ run_test "keyUsage cli-auth 1.2: RSA, KeyEncipherment: fail (hard)" \ 1 \ -s "bad certificate (usage extensions)" \ -s "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -s "Processing of the Certificate handshake message failed" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT @@ -7948,6 +7949,7 @@ run_test "keyUsage cli-auth 1.2: ECDSA, KeyAgreement: fail (hard)" \ 1 \ -s "bad certificate (usage extensions)" \ -s "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -s "Processing of the Certificate handshake message failed" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT @@ -7999,6 +8001,7 @@ run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (hard)" \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ -s "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -s "! mbedtls_ssl_handshake returned" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # @@ -8040,6 +8043,7 @@ run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (hard)" \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ -s "send alert level=2 message=43" \ + -s "! Usage does not match the keyUsage extension" \ -s "! mbedtls_ssl_handshake returned" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # From 52c0f5a0fd60688b263b97c39fea232eedff6009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 8 Aug 2024 12:19:46 +0200 Subject: [PATCH 10/44] Rationalize keyUsage testing, round 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cli-auth 1.2 was missing a test with an irrelevant bit set in addition to the relevant bit (which was added for 1.3 previously) - use consistent naming for fail (hard/soft) Note: currently there are no "fail (soft)" cases for 1.3 authentication of server by client, as server auth is mandatory in 1.3 (this will change in 3.6.1). Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 69568058bc..e6e2f99553 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -7726,7 +7726,7 @@ run_test "keyUsage cli 1.2: KeyEncipherment, RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail" \ +run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail (hard)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI debug_level=3 \ @@ -7739,7 +7739,7 @@ run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail" \ -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT -run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail, soft" \ +run_test "keyUsage cli 1.2: KeyEncipherment, DHE-RSA: fail (soft)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ke.crt" \ "$P_CLI debug_level=3 auth_mode=optional \ @@ -7761,7 +7761,7 @@ run_test "keyUsage cli 1.2: DigitalSignature, DHE-RSA: OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" -run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail" \ +run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail (hard)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI debug_level=3 \ @@ -7774,7 +7774,7 @@ run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail" \ -c "! Usage does not match the keyUsage extension" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT -run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail, soft" \ +run_test "keyUsage cli 1.2: DigitalSignature, RSA: fail (soft)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2.ku-ds.crt" \ "$P_CLI debug_level=3 auth_mode=optional \ @@ -7813,7 +7813,7 @@ run_test "keyUsage cli 1.3: DigitalSignature+KeyEncipherment, RSA: OK" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ +run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail (hard)" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ "$P_CLI debug_level=3" \ @@ -7828,7 +7828,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, RSA: fail" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail" \ +run_test "keyUsage cli 1.3: KeyAgreement, RSA: fail (hard)" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ka.crt" \ "$P_CLI debug_level=3" \ @@ -7855,7 +7855,7 @@ run_test "keyUsage cli 1.3: DigitalSignature, ECDSA: OK" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ +run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail (hard)" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ke.crt" \ "$P_CLI debug_level=3" \ @@ -7870,7 +7870,7 @@ run_test "keyUsage cli 1.3: KeyEncipherment, ECDSA: fail" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail" \ +run_test "keyUsage cli 1.3: KeyAgreement, ECDSA: fail (hard)" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ "$P_CLI debug_level=3" \ @@ -7897,6 +7897,16 @@ run_test "keyUsage cli-auth 1.2: RSA, DigitalSignature: OK" \ -S "bad certificate (usage extensions)" \ -S "Processing of the Certificate handshake message failed" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "keyUsage cli-auth 1.2: RSA, DigitalSignature+KeyEncipherment: OK" \ + "$P_SRV debug_level=1 auth_mode=optional" \ + "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server2.key \ + -cert $DATA_FILES_PATH/server2.ku-ds_ke.crt" \ + 0 \ + -s "Verifying peer X.509 certificate... ok" \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "keyUsage cli-auth 1.2: RSA, KeyEncipherment: fail (soft)" \ "$P_SRV debug_level=3 auth_mode=optional" \ @@ -7968,7 +7978,7 @@ run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature: OK" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature+KeyEnciphermen: OK" \ +run_test "keyUsage cli-auth 1.3: RSA, DigitalSignature+KeyEncipherment: OK" \ "$P_SRV debug_level=1 force_version=tls13 auth_mode=optional" \ "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server2.key \ -cert $DATA_FILES_PATH/server2-sha256.ku-ds_ke.crt" \ From cdd5b07eb146f47a8917cba5809d3b4a18037c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 12 Aug 2024 09:50:18 +0200 Subject: [PATCH 11/44] Use P_CLI when O_CLI's status is not reliable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generally speaking, in this group of test we use O_SRV when testing our client's behaviour, and O_CLI when testing our server's behaviour. I don't think that's essential, but why not. Well, for these two tests there's a reason why not: O_CLI often exits 0, seemingly not minding that the server aborted the handshake with a fatal alert, but sometimes it exits 1. (I've observed 0 on my machine, on two runs of OpenCI and Internal CI, and 1 in some test in one run of Internal CI.) So, use our client instead, which exits non-zero consistently. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e6e2f99553..ac6df5a7a4 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8005,18 +8005,15 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "keyUsage cli-auth 1.3: RSA, KeyEncipherment: fail (hard)" \ "$P_SRV debug_level=3 force_version=tls13 auth_mode=required" \ - "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server2.key \ - -cert $DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ - 0 \ + "$P_CLI key_file=$DATA_FILES_PATH/server2.key \ + crt_file=$DATA_FILES_PATH/server2-sha256.ku-ke.crt" \ + 1 \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ -s "send alert level=2 message=43" \ -s "! Usage does not match the keyUsage extension" \ -s "! mbedtls_ssl_handshake returned" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT - # - # OpenSSL client does not seem to mind that the server aborts the - # handshake with a fatal alert and still exits 0... requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -8047,18 +8044,15 @@ requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "keyUsage cli-auth 1.3: ECDSA, KeyAgreement: fail (hard)" \ "$P_SRV debug_level=3 force_version=tls13 auth_mode=required" \ - "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server5.key \ - -cert $DATA_FILES_PATH/server5.ku-ka.crt" \ - 0 \ + "$P_CLI key_file=$DATA_FILES_PATH/server5.key \ + crt_file=$DATA_FILES_PATH/server5.ku-ka.crt" \ + 1 \ -s "bad certificate (usage extensions)" \ -s "Processing of the Certificate handshake message failed" \ -s "send alert level=2 message=43" \ -s "! Usage does not match the keyUsage extension" \ -s "! mbedtls_ssl_handshake returned" # MBEDTLS_X509_BADCERT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT - # - # OpenSSL client does not seem to mind that the server aborts the - # handshake with a fatal alert and still exits 0... # Tests for extendedKeyUsage, part 1: server-side certificate/suite selection From e74c840b5ec578cbe9011d0b28bb0d228b53d4fd Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 15 Aug 2024 15:24:09 +0100 Subject: [PATCH 12/44] Rationalize extKeyUsage tests Signed-off-by: Elena Uziunaite --- tests/ssl-opt.sh | 49 ++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ac6df5a7a4..e1229406e3 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8087,7 +8087,7 @@ run_test "extKeyUsage srv: codeSign -> fail" \ # Tests for extendedKeyUsage, part 2: client-side checking of server cert requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli: serverAuth -> OK" \ +run_test "extKeyUsage cli 1.2: serverAuth -> OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-srv.crt" \ "$P_CLI debug_level=1" \ @@ -8097,7 +8097,7 @@ run_test "extKeyUsage cli: serverAuth -> OK" \ -c "Ciphersuite is TLS-" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli: serverAuth,clientAuth -> OK" \ +run_test "extKeyUsage cli 1.2: serverAuth,clientAuth -> OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-srv_cli.crt" \ "$P_CLI debug_level=1" \ @@ -8107,7 +8107,7 @@ run_test "extKeyUsage cli: serverAuth,clientAuth -> OK" \ -c "Ciphersuite is TLS-" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli: codeSign,anyEKU -> OK" \ +run_test "extKeyUsage cli 1.2: codeSign,anyEKU -> OK" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs_any.crt" \ "$P_CLI debug_level=1" \ @@ -8117,14 +8117,17 @@ run_test "extKeyUsage cli: codeSign,anyEKU -> OK" \ -c "Ciphersuite is TLS-" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli: codeSign -> fail" \ +run_test "extKeyUsage cli 1.2: codeSign -> fail (hard)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is TLS-" + -C "Ciphersuite is TLS-" \ + -c "send alert level=2 message=43" \ + -c "! Usage does not match the extendedKeyUsage extension" + # MBEDTLS_X509_BADCERT_EXT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -8165,19 +8168,22 @@ run_test "extKeyUsage cli 1.3: codeSign,anyEKU -> OK" \ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED -run_test "extKeyUsage cli 1.3: codeSign -> fail" \ +run_test "extKeyUsage cli 1.3: codeSign -> fail (hard)" \ "$O_NEXT_SRV_NO_CERT -tls1_3 -num_tickets=0 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ - "$P_CLI debug_level=1" \ + "$P_CLI debug_level=3" \ 1 \ -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ - -C "Ciphersuite is" + -C "Ciphersuite is" \ + -c "send alert level=2 message=43" \ + -c "! Usage does not match the extendedKeyUsage extension" + # MBEDTLS_X509_BADCERT_EXT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT # Tests for extendedKeyUsage, part 3: server-side checking of client cert requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli-auth: clientAuth -> OK" \ +run_test "extKeyUsage cli-auth 1.2: clientAuth -> OK" \ "$P_SRV debug_level=1 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cli.crt" \ @@ -8186,7 +8192,7 @@ run_test "extKeyUsage cli-auth: clientAuth -> OK" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli-auth: serverAuth,clientAuth -> OK" \ +run_test "extKeyUsage cli-auth 1.2: serverAuth,clientAuth -> OK" \ "$P_SRV debug_level=1 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-srv_cli.crt" \ @@ -8195,7 +8201,7 @@ run_test "extKeyUsage cli-auth: serverAuth,clientAuth -> OK" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli-auth: codeSign,anyEKU -> OK" \ +run_test "extKeyUsage cli-auth 1.2: codeSign,anyEKU -> OK" \ "$P_SRV debug_level=1 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs_any.crt" \ @@ -8204,22 +8210,27 @@ run_test "extKeyUsage cli-auth: codeSign,anyEKU -> OK" \ -S "Processing of the Certificate handshake message failed" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli-auth: codeSign -> fail (soft)" \ - "$P_SRV debug_level=1 auth_mode=optional" \ +run_test "extKeyUsage cli-auth 1.2: codeSign -> fail (soft)" \ + "$P_SRV debug_level=3 auth_mode=optional" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ 0 \ -s "bad certificate (usage extensions)" \ - -S "Processing of the Certificate handshake message failed" + -S "send alert level=2 message=43" \ + -s "! Usage does not match the extendedKeyUsage extension" \ + -S "Processing of the Certificate handshake message failed" \ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -run_test "extKeyUsage cli-auth: codeSign -> fail (hard)" \ - "$P_SRV debug_level=1 auth_mode=required" \ +run_test "extKeyUsage cli-auth 1.2: codeSign -> fail (hard)" \ + "$P_SRV debug_level=3 auth_mode=required" \ "$O_CLI -tls1_2 -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ 1 \ -s "bad certificate (usage extensions)" \ + -s "send alert level=2 message=43" \ + -s "! Usage does not match the extendedKeyUsage extension" \ -s "Processing of the Certificate handshake message failed" + # MBEDTLS_X509_BADCERT_EXT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ @@ -8258,11 +8269,13 @@ requires_openssl_tls1_3_with_compatible_ephemeral requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "extKeyUsage cli-auth 1.3: codeSign -> fail (soft)" \ - "$P_SRV debug_level=1 force_version=tls13 auth_mode=optional" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=optional" \ "$O_NEXT_CLI_NO_CERT -key $DATA_FILES_PATH/server5.key \ -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ 0 \ -s "bad certificate (usage extensions)" \ + -S "send alert level=2 message=43" \ + -s "! Usage does not match the extendedKeyUsage extension" \ -S "Processing of the Certificate handshake message failed" # Tests for DHM parameters loading From 04db1fb481e9b1cbe10f506e6e184f5dab438bce Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Fri, 16 Aug 2024 17:18:28 +0100 Subject: [PATCH 13/44] Add test cases for extKeyUsage Signed-off-by: Elena Uziunaite --- tests/ssl-opt.sh | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e1229406e3..91828ef03a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8116,6 +8116,19 @@ run_test "extKeyUsage cli 1.2: codeSign,anyEKU -> OK" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "extKeyUsage cli 1.2: codeSign -> fail (soft)" \ + "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ + -cert $DATA_FILES_PATH/server5.eku-cs.crt" \ + "$P_CLI debug_level=3 auth_mode=optional" \ + 0 \ + -c "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" \ + -C "send alert level=2 message=43" \ + -c "! Usage does not match the extendedKeyUsage extension" + # MBEDTLS_X509_BADCERT_EXT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT + requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 run_test "extKeyUsage cli 1.2: codeSign -> fail (hard)" \ "$O_SRV -tls1_2 -key $DATA_FILES_PATH/server5.key \ @@ -8278,6 +8291,20 @@ run_test "extKeyUsage cli-auth 1.3: codeSign -> fail (soft)" \ -s "! Usage does not match the extendedKeyUsage extension" \ -S "Processing of the Certificate handshake message failed" +requires_openssl_tls1_3_with_compatible_ephemeral +requires_all_configs_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE \ + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "extKeyUsage cli-auth 1.3: codeSign -> fail (hard)" \ + "$P_SRV debug_level=3 force_version=tls13 auth_mode=required" \ + "$P_CLI key_file=$DATA_FILES_PATH/server5.key \ + crt_file=$DATA_FILES_PATH/server5.eku-cs.crt" \ + 1 \ + -s "bad certificate (usage extensions)" \ + -s "send alert level=2 message=43" \ + -s "! Usage does not match the extendedKeyUsage extension" \ + -s "Processing of the Certificate handshake message failed" + # MBEDTLS_X509_BADCERT_EXT_KEY_USAGE -> MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT + # Tests for DHM parameters loading run_test "DHM parameters: reference" \ From c2ec6fa25b20de48e50d4381bffbead15af3b703 Mon Sep 17 00:00:00 2001 From: Gowtham Suresh Kumar Date: Mon, 19 Aug 2024 11:50:10 +0100 Subject: [PATCH 14/44] Free allocated memory where methods were returning without freeing Signed-off-by: Sam Berry Signed-off-by: Gowtham Suresh Kumar --- library/psa_crypto_rsa.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto_rsa.c b/library/psa_crypto_rsa.c index f8e36d8b12..38dc3b8edc 100644 --- a/library/psa_crypto_rsa.c +++ b/library/psa_crypto_rsa.c @@ -197,16 +197,14 @@ psa_status_t mbedtls_psa_rsa_export_public_key( status = mbedtls_psa_rsa_load_representation( attributes->type, key_buffer, key_buffer_size, &rsa); - if (status != PSA_SUCCESS) { - return status; + if (status == PSA_SUCCESS) { + status = mbedtls_psa_rsa_export_key(PSA_KEY_TYPE_RSA_PUBLIC_KEY, + rsa, + data, + data_size, + data_length); } - status = mbedtls_psa_rsa_export_key(PSA_KEY_TYPE_RSA_PUBLIC_KEY, - rsa, - data, - data_size, - data_length); - mbedtls_rsa_free(rsa); mbedtls_free(rsa); @@ -264,6 +262,7 @@ psa_status_t mbedtls_psa_rsa_generate_key( (unsigned int) attributes->bits, exponent); if (ret != 0) { + mbedtls_rsa_free(&rsa); return mbedtls_to_psa_error(ret); } @@ -330,7 +329,7 @@ psa_status_t mbedtls_psa_rsa_sign_hash( key_buffer_size, &rsa); if (status != PSA_SUCCESS) { - return status; + goto exit; } status = psa_rsa_decode_md_type(alg, hash_length, &md_alg); From 777e3e77c9ff78039101ab881d53206bb7852082 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Mon, 19 Aug 2024 12:10:22 +0100 Subject: [PATCH 15/44] Update ChangeLog Signed-off-by: Elena Uziunaite --- ChangeLog.d/fix_reporting_of_key_usage_issues.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix_reporting_of_key_usage_issues.txt diff --git a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt new file mode 100644 index 0000000000..12f1bb3799 --- /dev/null +++ b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix the failure to correctly update verification flags when + checking the (ext)KeyUsage extension. + Resolves #1260 From b0b71dc5d5748fec0476d0aac876d95c8bc669bd Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Tue, 20 Aug 2024 12:11:57 +0100 Subject: [PATCH 16/44] Edit ChangeLog entry Signed-off-by: Elena Uziunaite --- ChangeLog.d/fix_reporting_of_key_usage_issues.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt index 12f1bb3799..75fbb6cc15 100644 --- a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt +++ b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt @@ -1,4 +1,11 @@ -Bugfix - * Fix the failure to correctly update verification flags when - checking the (ext)KeyUsage extension. - Resolves #1260 +Security + * With TLS 1.3, when a server enables optional authentication of the + client, if the client-provided certificate does not have appropriate values + in if keyUsage or extKeyUsage extensions, then the return value of + mbedtls_ssl_get_verify_result() would incorrectly have the + MBEDTLS_X509_BADCERT_KEY_USAGE and MBEDTLS_X509_BADCERT_KEY_USAGE bits + clear. As a result, an attacker that had a certificate valid for uses other + than TLS client authentication could be able to use it for TLS client + authentication anyway. Only TLS 1.3 servers were affected, and only with + optional authentication (required would abort the handshake with a fatal + alert). From 060e284deea9262d9ecb359ffc4a56ac294e01e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 5 Aug 2024 11:10:47 +0200 Subject: [PATCH 17/44] Add test forcing TLS 1.2 for clearer coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a duplicate from the previous test, except it forces TLS 1.2. The previous test does not force a version, so it picks 1.3 in the default/full config. However we have a build with 1.2 only in all.sh, in which the previous test would pick 1.2. So, there was no test gap and the behaviour was indeed tested with 1.2. However when measuring code coverage with lcov, currently we can only use a single build. So, I'm adding this variant of the test case as a so that the 1.2 code looks covered in the report from basic-build-test.sh. This is for my convenience while I make sure everything is covered before refactoring. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 91828ef03a..7de41da994 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5853,6 +5853,17 @@ run_test "Authentication: server goodcert, client required, no trusted CA" \ -c "! mbedtls_ssl_handshake returned" \ -c "SSL - No CA Chain is set, but required to operate" +requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT +run_test "Authentication: server goodcert, client required, no trusted CA (1.2)" \ + "$P_SRV force_version=tls12" \ + "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ + 1 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -c "! mbedtls_ssl_handshake returned" \ + -c "SSL - No CA Chain is set, but required to operate" + # The purpose of the next two tests is to test the client's behaviour when receiving a server # certificate with an unsupported elliptic curve. This should usually not happen because # the client informs the server about the supported curves - it does, though, in the From a3cf1a53b457f67da4bff579b95b5107f17799d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 5 Aug 2024 11:21:01 +0200 Subject: [PATCH 18/44] Fix ordering of a test case in ssl-opt.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7de41da994..5eb965bb4a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5809,6 +5809,7 @@ run_test "DER format: with 9 trailing random bytes" \ # Tests for auth_mode, there are duplicated tests using ca callback for authentication # When updating these tests, modify the matching authentication tests accordingly +# The next 3 cases test the 3 auth modes with a badly signed server cert. requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Authentication: server badcert, client required" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ @@ -5830,6 +5831,16 @@ run_test "Authentication: server badcert, client optional" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +run_test "Authentication: server badcert, client none" \ + "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ + key_file=$DATA_FILES_PATH/server5.key" \ + "$P_CLI force_version=tls12 debug_level=1 auth_mode=none" \ + 0 \ + -C "x509_verify_cert() returned" \ + -C "! The certificate is not correctly signed by the trusted CA" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT run_test "Authentication: server goodcert, client optional, no trusted CA" \ "$P_SRV" \ @@ -5889,16 +5900,6 @@ run_test "Authentication: server ECDH p256v1, client optional, p256v1 unsuppo -c "! Certificate verification flags"\ -c "bad server certificate (ECDH curve)" # Expect failure only at ECDH params check -run_test "Authentication: server badcert, client none" \ - "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ - key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI force_version=tls12 debug_level=1 auth_mode=none" \ - 0 \ - -C "x509_verify_cert() returned" \ - -C "! The certificate is not correctly signed by the trusted CA" \ - -C "! mbedtls_ssl_handshake returned" \ - -C "X509 - Certificate verification failed" - requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT run_test "Authentication: client SHA256, server required" \ "$P_SRV auth_mode=required" \ From d6e20692dd3eb45d7cb2470eee5e1f1f74cdeb61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 5 Aug 2024 12:41:59 +0200 Subject: [PATCH 19/44] Test cert alert NOT_TRUSTED -> UNKNOWN_CA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In terms of line coverage, this was covered, except we never checked the behaviour was as intended. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 5eb965bb4a..074230f371 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5809,36 +5809,55 @@ run_test "DER format: with 9 trailing random bytes" \ # Tests for auth_mode, there are duplicated tests using ca callback for authentication # When updating these tests, modify the matching authentication tests accordingly -# The next 3 cases test the 3 auth modes with a badly signed server cert. +# The next 4 cases test the 3 auth modes with a badly signed server cert. requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Authentication: server badcert, client required" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI debug_level=1 auth_mode=required" \ + "$P_CLI debug_level=3 auth_mode=required" \ 1 \ -c "x509_verify_cert() returned" \ -c "! The certificate is not correctly signed by the trusted CA" \ -c "! mbedtls_ssl_handshake returned" \ + -c "send alert level=2 message=48" \ -c "X509 - Certificate verification failed" + # MBEDTLS_X509_BADCERT_NOT_TRUSTED -> MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA +# We don't check that the server receives the alert because it might +# detect that its write end of the connection is closed and abort +# before reading the alert message. + +run_test "Authentication: server badcert, client required (1.2)" \ + "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ + key_file=$DATA_FILES_PATH/server5.key" \ + "$P_CLI force_version=tls12 debug_level=3 auth_mode=required" \ + 1 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "send alert level=2 message=48" \ + -c "X509 - Certificate verification failed" + # MBEDTLS_X509_BADCERT_NOT_TRUSTED -> MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA run_test "Authentication: server badcert, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI force_version=tls12 debug_level=1 auth_mode=optional" \ + "$P_CLI force_version=tls12 debug_level=3 auth_mode=optional" \ 0 \ -c "x509_verify_cert() returned" \ -c "! The certificate is not correctly signed by the trusted CA" \ -C "! mbedtls_ssl_handshake returned" \ + -C "send alert level=2 message=48" \ -C "X509 - Certificate verification failed" run_test "Authentication: server badcert, client none" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI force_version=tls12 debug_level=1 auth_mode=none" \ + "$P_CLI force_version=tls12 debug_level=3 auth_mode=none" \ 0 \ -C "x509_verify_cert() returned" \ -C "! The certificate is not correctly signed by the trusted CA" \ -C "! mbedtls_ssl_handshake returned" \ + -C "send alert level=2 message=48" \ -C "X509 - Certificate verification failed" requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT From 4192bba54fd08f44942569791e01e975bdef5a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 5 Aug 2024 12:44:57 +0200 Subject: [PATCH 20/44] Test cert alert REVOKED -> CERT_REVOKED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 074230f371..b9a09e9a5a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6609,7 +6609,9 @@ run_test "SNI: CA override with CRL" \ -S "skip parse certificate verify" \ -s "x509_verify_cert() returned" \ -S "! The certificate is not correctly signed by the trusted CA" \ + -s "send alert level=2 message=44" \ -s "The certificate has been revoked (is on a CRL)" + # MBEDTLS_X509_BADCERT_REVOKED -> MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED # Tests for SNI and DTLS @@ -6757,7 +6759,9 @@ run_test "SNI: DTLS, CA override with CRL" \ -S "skip parse certificate verify" \ -s "x509_verify_cert() returned" \ -S "! The certificate is not correctly signed by the trusted CA" \ + -s "send alert level=2 message=44" \ -s "The certificate has been revoked (is on a CRL)" + # MBEDTLS_X509_BADCERT_REVOKED -> MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED # Tests for non-blocking I/O: exercise a variety of handshake flows From 96a0c5c48e1692fb1ca4f2c6d93731c9d6aee070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 9 Aug 2024 11:26:25 +0200 Subject: [PATCH 21/44] Clean up mbedtls_ssl_check_cert_usage() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_misc.h | 10 +++++----- library/ssl_tls.c | 11 +++++++---- library/ssl_tls12_server.c | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index a8807f67c6..40d7187c34 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1674,18 +1674,18 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert(mbedtls_ssl_context *ssl) } /* - * Check usage of a certificate wrt extensions: - * keyUsage, extendedKeyUsage (later), and nSCertType (later). + * Check usage of a certificate wrt usage extensions: + * keyUsage and extendedKeyUsage. + * (Note: nSCertType is deprecated and not standard, we don't check it.) * - * Warning: cert_endpoint is the endpoint of the cert (ie, of our peer when we - * check a cert we received from them)! + * Note: recv_endpoint is the receiver's endpoint. * * Return 0 if everything is OK, -1 if not. */ MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, - int cert_endpoint, + int recv_endpoint, uint32_t *flags); #endif /* MBEDTLS_X509_CRT_PARSE_C */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d6077a2baa..4376146b5d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6361,7 +6361,7 @@ const char *mbedtls_ssl_get_curve_name_from_tls_id(uint16_t tls_id) #if defined(MBEDTLS_X509_CRT_PARSE_C) int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, - int cert_endpoint, + int recv_endpoint, uint32_t *flags) { int ret = 0; @@ -6369,7 +6369,10 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const char *ext_oid; size_t ext_len; - if (cert_endpoint == MBEDTLS_SSL_IS_SERVER) { + /* Note: don't guard this with MBEDTLS_SSL_CLI_C because the server wants + * to check what a compliant client will think while choosing which cert + * to send to the client. */ + if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { /* Server part of the key exchange */ switch (ciphersuite->key_exchange) { case MBEDTLS_KEY_EXCHANGE_RSA: @@ -6406,7 +6409,7 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, ret = -1; } - if (cert_endpoint == MBEDTLS_SSL_IS_SERVER) { + if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { ext_oid = MBEDTLS_OID_SERVER_AUTH; ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); } else { @@ -8061,7 +8064,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, if (mbedtls_ssl_check_cert_usage(chain, ciphersuite_info, - !ssl->conf->endpoint, + ssl->conf->endpoint, &ssl->session_negotiate->verify_result) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); if (ret == 0) { diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index b5b975ff40..5bfab04adc 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -756,7 +756,7 @@ static int ssl_pick_cert(mbedtls_ssl_context *ssl, * and decrypting with the same RSA key. */ if (mbedtls_ssl_check_cert_usage(cur->cert, ciphersuite_info, - MBEDTLS_SSL_IS_SERVER, &flags) != 0) { + MBEDTLS_SSL_IS_CLIENT, &flags) != 0) { MBEDTLS_SSL_DEBUG_MSG(3, ("certificate mismatch: " "(extended) key usage extension")); continue; From 4938b693f31ec275a39e249ba67650d78f9546ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 9 Aug 2024 11:49:12 +0200 Subject: [PATCH 22/44] Make mbedtls_ssl_check_cert_usage() work for 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_misc.h | 3 +++ library/ssl_tls.c | 26 ++++++++++++++++++++++---- library/ssl_tls12_server.c | 4 +++- library/ssl_tls13_generic.c | 31 +++++-------------------------- 4 files changed, 33 insertions(+), 31 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 40d7187c34..ecb2d03906 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1678,6 +1678,8 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert(mbedtls_ssl_context *ssl) * keyUsage and extendedKeyUsage. * (Note: nSCertType is deprecated and not standard, we don't check it.) * + * Note: if tls_version is 1.3, ciphersuite is ignored and can be NULL. + * * Note: recv_endpoint is the receiver's endpoint. * * Return 0 if everything is OK, -1 if not. @@ -1686,6 +1688,7 @@ MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, int recv_endpoint, + mbedtls_ssl_protocol_version tls_version, uint32_t *flags); #endif /* MBEDTLS_X509_CRT_PARSE_C */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4376146b5d..3f375be92c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6362,6 +6362,7 @@ const char *mbedtls_ssl_get_curve_name_from_tls_id(uint16_t tls_id) int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, int recv_endpoint, + mbedtls_ssl_protocol_version tls_version, uint32_t *flags) { int ret = 0; @@ -6369,11 +6370,17 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, const char *ext_oid; size_t ext_len; + /* + * keyUsage + */ + /* Note: don't guard this with MBEDTLS_SSL_CLI_C because the server wants * to check what a compliant client will think while choosing which cert * to send to the client. */ - if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { - /* Server part of the key exchange */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && + recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { + /* TLS 1.2 server part of the key exchange */ switch (ciphersuite->key_exchange) { case MBEDTLS_KEY_EXCHANGE_RSA: case MBEDTLS_KEY_EXCHANGE_RSA_PSK: @@ -6399,8 +6406,14 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, case MBEDTLS_KEY_EXCHANGE_ECJPAKE: usage = 0; } - } else { - /* Client auth: we only implement rsa_sign and mbedtls_ecdsa_sign for now */ + } else +#endif + { + /* This is either TLS 1.3 autentication, which always uses signatures, + * or 1.2 client auth: rsa_sign and mbedtls_ecdsa_sign are the only + * options we implement, both using signatures. */ + (void) tls_version; + (void) ciphersuite; usage = MBEDTLS_X509_KU_DIGITAL_SIGNATURE; } @@ -6409,6 +6422,10 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, ret = -1; } + /* + * extKeyUsage + */ + if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { ext_oid = MBEDTLS_OID_SERVER_AUTH; ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); @@ -8065,6 +8082,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, if (mbedtls_ssl_check_cert_usage(chain, ciphersuite_info, ssl->conf->endpoint, + MBEDTLS_SSL_VERSION_TLS1_2, &ssl->session_negotiate->verify_result) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); if (ret == 0) { diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 5bfab04adc..0878749464 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -756,7 +756,9 @@ static int ssl_pick_cert(mbedtls_ssl_context *ssl, * and decrypting with the same RSA key. */ if (mbedtls_ssl_check_cert_usage(cur->cert, ciphersuite_info, - MBEDTLS_SSL_IS_CLIENT, &flags) != 0) { + MBEDTLS_SSL_IS_CLIENT, + MBEDTLS_SSL_VERSION_TLS1_2, + &flags) != 0) { MBEDTLS_SSL_DEBUG_MSG(3, ("certificate mismatch: " "(extended) key usage extension")); continue; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 651a17b5a2..8d8af2b19e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -631,8 +631,6 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) int authmode = MBEDTLS_SSL_VERIFY_REQUIRED; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; - const char *ext_oid; - size_t ext_len; uint32_t verify_result = 0; /* If SNI was used, overwrite authentication mode @@ -714,34 +712,15 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) /* * Secondary checks: always done, but change 'ret' only if it was 0 */ - /* keyUsage */ - if ((mbedtls_x509_crt_check_key_usage( - ssl->session_negotiate->peer_cert, - MBEDTLS_X509_KU_DIGITAL_SIGNATURE) != 0)) { + if (mbedtls_ssl_check_cert_usage(ssl->session_negotiate->peer_cert, + NULL, + ssl->conf->endpoint, + MBEDTLS_SSL_VERSION_TLS1_3, + &verify_result) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); if (ret == 0) { ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; } - verify_result |= MBEDTLS_X509_BADCERT_KEY_USAGE; - } - - /* extKeyUsage */ - if (ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT) { - ext_oid = MBEDTLS_OID_SERVER_AUTH; - ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); - } else { - ext_oid = MBEDTLS_OID_CLIENT_AUTH; - ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_CLIENT_AUTH); - } - - if ((mbedtls_x509_crt_check_extended_key_usage( - ssl->session_negotiate->peer_cert, - ext_oid, ext_len) != 0)) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; - } - verify_result |= MBEDTLS_X509_BADCERT_EXT_KEY_USAGE; } /* mbedtls_x509_crt_verify_with_profile is supposed to report a From 8a14aaaca5acc10eb662db3ad5391034463c3ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 9 Aug 2024 12:00:34 +0200 Subject: [PATCH 23/44] Simplify certificate curve check for 1.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comments were about the time we were using mbedtls_pk_ec(), which can return NULL, which we don't want to propagate to other functions. Now we're using mbedtls_pk_get_ec_group_id() with is a safer interface (and works even when EC is provided by drivers). The check for GROUP_NONE was an heritage from the previous NULL check. However it's actually useless: if NONE were returned (which can't happen or parsing of the certificate would have failed and we wouldn't be here), then mbedtls_ssl_check_curve() would work and just say that the curve wasn't valid, which is OK. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3f375be92c..1355feb88b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8050,35 +8050,19 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, * Secondary checks: always done, but change 'ret' only if it was 0 */ + /* Check curve for ECC certs */ #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) - { - const mbedtls_pk_context *pk = &chain->pk; - - /* If certificate uses an EC key, make sure the curve is OK. - * This is a public key, so it can't be opaque, so can_do() is a good - * enough check to ensure pk_ec() is safe to use here. */ - if (mbedtls_pk_can_do(pk, MBEDTLS_PK_ECKEY)) { - /* and in the unlikely case the above assumption no longer holds - * we are making sure that pk_ec() here does not return a NULL - */ - mbedtls_ecp_group_id grp_id = mbedtls_pk_get_ec_group_id(pk); - if (grp_id == MBEDTLS_ECP_DP_NONE) { - MBEDTLS_SSL_DEBUG_MSG(1, ("invalid group ID")); - return MBEDTLS_ERR_SSL_INTERNAL_ERROR; - } - if (mbedtls_ssl_check_curve(ssl, grp_id) != 0) { - ssl->session_negotiate->verify_result |= - MBEDTLS_X509_BADCERT_BAD_KEY; - - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; - } - } + if (mbedtls_pk_can_do(&chain->pk, MBEDTLS_PK_ECKEY) && + mbedtls_ssl_check_curve(ssl, mbedtls_pk_get_ec_group_id(&chain->pk)) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); + ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; + if (ret == 0) { + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; } } #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ + /* Check X.509 usage extensions (keyUsage, extKeyUsage) */ if (mbedtls_ssl_check_cert_usage(chain, ciphersuite_info, ssl->conf->endpoint, From 85b864e1dbdfb66362077e06e43bf1f7317547f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 9 Aug 2024 12:40:48 +0200 Subject: [PATCH 24/44] Rm translation code for unused flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't check the non-standard nsCertType extension, so this flag can't be set, so checking if it's set is useless. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 2 -- library/ssl_tls13_generic.c | 1 - 2 files changed, 3 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1355feb88b..baf8631b92 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8105,8 +8105,6 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE) { alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NS_CERT_TYPE) { - alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_PK) { alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_KEY) { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 8d8af2b19e..5fdc527054 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -752,7 +752,6 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_BAD_CERT, ret); } else if (verify_result & (MBEDTLS_X509_BADCERT_KEY_USAGE | MBEDTLS_X509_BADCERT_EXT_KEY_USAGE | - MBEDTLS_X509_BADCERT_NS_CERT_TYPE | MBEDTLS_X509_BADCERT_BAD_PK | MBEDTLS_X509_BADCERT_BAD_KEY)) { MBEDTLS_SSL_PEND_FATAL_ALERT( From 4d4c0c72daeb71778d564f6b46325f72a9fff191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 12 Aug 2024 10:36:40 +0200 Subject: [PATCH 25/44] Add comments about 1.3 server sending no cert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 5fdc527054..8f8f8c27b4 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -472,6 +472,7 @@ int mbedtls_ssl_tls13_parse_certificate(mbedtls_ssl_context *ssl, mbedtls_free(ssl->session_negotiate->peer_cert); } + /* This is used by ssl_tls13_validate_certificate() */ if (certificate_list_len == 0) { ssl->session_negotiate->peer_cert = NULL; ret = 0; @@ -675,6 +676,11 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) #endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_CLI_C) + /* Regardless of authmode, the server is not allowed to send an empty + * certificate chain. (Last paragraph before 4.4.2.1 in RFC 8446: "The + * server's certificate_list MUST always be non-empty.") With authmode + * optional/none, we continue the handshake if we can't validate the + * server's cert, but we still break it if no certificate was sent. */ if (ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT) { MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_NO_CERT, MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE); From e1cc926717fe78f70535f2544512e13f728ccbda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Aug 2024 09:47:38 +0200 Subject: [PATCH 26/44] Allow optional authentication of the server in 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is for compatibility, for people transitioning from 1.2 to 1.3. See https://github.com/Mbed-TLS/mbedtls/issues/9223 "Mandatory server authentication" and reports linked from there. In the future we're likely to make server authentication mandatory in both 1.2 and 1.3. See https://github.com/Mbed-TLS/mbedtls/issues/7080 Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 24 +----------------------- library/ssl_tls13_generic.c | 19 +++++++------------ tests/ssl-opt.sh | 29 +++++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index baf8631b92..4795e673bd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1354,29 +1354,6 @@ static int ssl_conf_check(const mbedtls_ssl_context *ssl) return ret; } -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - /* RFC 8446 section 4.4.3 - * - * If the verification fails, the receiver MUST terminate the handshake with - * a "decrypt_error" alert. - * - * If the client is configured as TLS 1.3 only with optional verify, return - * bad config. - * - */ - if (mbedtls_ssl_conf_tls13_is_ephemeral_enabled( - (mbedtls_ssl_context *) ssl) && - ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && - ssl->conf->max_tls_version == MBEDTLS_SSL_VERSION_TLS1_3 && - ssl->conf->min_tls_version == MBEDTLS_SSL_VERSION_TLS1_3 && - ssl->conf->authmode == MBEDTLS_SSL_VERIFY_OPTIONAL) { - MBEDTLS_SSL_DEBUG_MSG( - 1, ("Optional verify auth mode " - "is not available for TLS 1.3 client")); - return MBEDTLS_ERR_SSL_BAD_CONFIG; - } -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ - if (ssl->conf->f_rng == NULL) { MBEDTLS_SSL_DEBUG_MSG(1, ("no RNG provided")); return MBEDTLS_ERR_SSL_NO_RNG; @@ -8190,6 +8167,7 @@ int mbedtls_ssl_parse_certificate(mbedtls_ssl_context *ssl) { int ret = 0; int crt_expected; + /* Authmode: precedence order is SNI if used else configuration */ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ? ssl->handshake->sni_authmode diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 8f8f8c27b4..4b027de5aa 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -629,22 +629,17 @@ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) { int ret = 0; - int authmode = MBEDTLS_SSL_VERIFY_REQUIRED; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; uint32_t verify_result = 0; - /* If SNI was used, overwrite authentication mode - * from the configuration. */ -#if defined(MBEDTLS_SSL_SRV_C) - if (ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER) { -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if (ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET) { - authmode = ssl->handshake->sni_authmode; - } else -#endif - authmode = ssl->conf->authmode; - } + /* Authmode: precedence order is SNI if used else configuration */ +#if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET + ? ssl->handshake->sni_authmode + : ssl->conf->authmode; +#else + const int authmode = ssl->conf->authmode; #endif /* diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b9a09e9a5a..e59c407b69 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5839,6 +5839,17 @@ run_test "Authentication: server badcert, client required (1.2)" \ # MBEDTLS_X509_BADCERT_NOT_TRUSTED -> MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA run_test "Authentication: server badcert, client optional" \ + "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ + key_file=$DATA_FILES_PATH/server5.key" \ + "$P_CLI force_version=tls13 debug_level=3 auth_mode=optional" \ + 0 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "send alert level=2 message=48" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: server badcert, client optional (1.2)" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ "$P_CLI force_version=tls12 debug_level=3 auth_mode=optional" \ @@ -5860,8 +5871,22 @@ run_test "Authentication: server badcert, client none" \ -C "send alert level=2 message=48" \ -C "X509 - Certificate verification failed" -requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT +# TODO: server goodcert, client none, no trusted CA + +requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Authentication: server goodcert, client optional, no trusted CA" \ + "$P_SRV" \ + "$P_CLI debug_level=3 auth_mode=optional ca_file=none ca_path=none" \ + 0 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" \ + -C "SSL - No CA Chain is set, but required to operate" + +requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT +run_test "Authentication: server goodcert, client optional, no trusted CA (1.2)" \ "$P_SRV" \ "$P_CLI force_version=tls12 debug_level=3 auth_mode=optional ca_file=none ca_path=none" \ 0 \ @@ -6129,7 +6154,7 @@ requires_full_size_output_buffer run_test "Authentication: server max_int+1 chain, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ - "$P_CLI force_version=tls12 server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt \ + "$P_CLI server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt \ auth_mode=optional" \ 1 \ -c "X509 - A fatal error occurred" From a0a781eaddcf8c9a42de4aa3448a241ed8daf696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Aug 2024 10:34:53 +0200 Subject: [PATCH 27/44] Reorder some tests in ssl-opt.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tests above are required then optional then none. Follow the same pattern here. Just moving things around (see git's --color-moved option). Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e59c407b69..3f64ef683a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5871,7 +5871,27 @@ run_test "Authentication: server badcert, client none" \ -C "send alert level=2 message=48" \ -C "X509 - Certificate verification failed" -# TODO: server goodcert, client none, no trusted CA +requires_key_exchange_with_cert_in_tls12_or_tls13_enabled +run_test "Authentication: server goodcert, client required, no trusted CA" \ + "$P_SRV" \ + "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ + 1 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -c "! mbedtls_ssl_handshake returned" \ + -c "SSL - No CA Chain is set, but required to operate" + +requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT +run_test "Authentication: server goodcert, client required, no trusted CA (1.2)" \ + "$P_SRV force_version=tls12" \ + "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ + 1 \ + -c "x509_verify_cert() returned" \ + -c "! The certificate is not correctly signed by the trusted CA" \ + -c "! Certificate verification flags"\ + -c "! mbedtls_ssl_handshake returned" \ + -c "SSL - No CA Chain is set, but required to operate" requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Authentication: server goodcert, client optional, no trusted CA" \ @@ -5897,27 +5917,7 @@ run_test "Authentication: server goodcert, client optional, no trusted CA (1. -C "X509 - Certificate verification failed" \ -C "SSL - No CA Chain is set, but required to operate" -requires_key_exchange_with_cert_in_tls12_or_tls13_enabled -run_test "Authentication: server goodcert, client required, no trusted CA" \ - "$P_SRV" \ - "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ - 1 \ - -c "x509_verify_cert() returned" \ - -c "! The certificate is not correctly signed by the trusted CA" \ - -c "! Certificate verification flags"\ - -c "! mbedtls_ssl_handshake returned" \ - -c "SSL - No CA Chain is set, but required to operate" - -requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT -run_test "Authentication: server goodcert, client required, no trusted CA (1.2)" \ - "$P_SRV force_version=tls12" \ - "$P_CLI debug_level=3 auth_mode=required ca_file=none ca_path=none" \ - 1 \ - -c "x509_verify_cert() returned" \ - -c "! The certificate is not correctly signed by the trusted CA" \ - -c "! Certificate verification flags"\ - -c "! mbedtls_ssl_handshake returned" \ - -c "SSL - No CA Chain is set, but required to operate" +# TODO: server goodcert, client none, no trusted CA # The purpose of the next two tests is to test the client's behaviour when receiving a server # certificate with an unsupported elliptic curve. This should usually not happen because From 2b98a4ee3b1922b2802f5f4b0781a7de703b5178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Aug 2024 10:44:02 +0200 Subject: [PATCH 28/44] Allow no authentication of the server in 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See notes about optional two commits ago for why we're doing this. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 12 ++++++++++++ tests/ssl-opt.sh | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 4b027de5aa..2104567c85 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -684,6 +684,18 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) #endif /* MBEDTLS_SSL_CLI_C */ } + /* + * NONE means we skip all checks + * + * Note: we still check above that the server did send a certificate, + * because only a non-compliant server would fail to do so. NONE means we + * don't care about the server certificate being valid, but we still care + * about the server otherwise following the TLS standard. + */ + if (authmode == MBEDTLS_SSL_VERIFY_NONE) { + return 0; + } + #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if (ssl->handshake->sni_ca_chain != NULL) { ca_chain = ssl->handshake->sni_ca_chain; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3f64ef683a..84342d588d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5861,6 +5861,17 @@ run_test "Authentication: server badcert, client optional (1.2)" \ -C "X509 - Certificate verification failed" run_test "Authentication: server badcert, client none" \ + "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ + key_file=$DATA_FILES_PATH/server5.key" \ + "$P_CLI debug_level=3 auth_mode=none" \ + 0 \ + -C "x509_verify_cert() returned" \ + -C "! The certificate is not correctly signed by the trusted CA" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "send alert level=2 message=48" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: server badcert, client none (1.2)" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ "$P_CLI force_version=tls12 debug_level=3 auth_mode=none" \ @@ -5917,7 +5928,29 @@ run_test "Authentication: server goodcert, client optional, no trusted CA (1. -C "X509 - Certificate verification failed" \ -C "SSL - No CA Chain is set, but required to operate" -# TODO: server goodcert, client none, no trusted CA +requires_key_exchange_with_cert_in_tls12_or_tls13_enabled +run_test "Authentication: server goodcert, client none, no trusted CA" \ + "$P_SRV" \ + "$P_CLI debug_level=3 auth_mode=none ca_file=none ca_path=none" \ + 0 \ + -C "x509_verify_cert() returned" \ + -C "! The certificate is not correctly signed by the trusted CA" \ + -C "! Certificate verification flags"\ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" \ + -C "SSL - No CA Chain is set, but required to operate" + +requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT +run_test "Authentication: server goodcert, client none, no trusted CA (1.2)" \ + "$P_SRV" \ + "$P_CLI force_version=tls12 debug_level=3 auth_mode=none ca_file=none ca_path=none" \ + 0 \ + -C "x509_verify_cert() returned" \ + -C "! The certificate is not correctly signed by the trusted CA" \ + -C "! Certificate verification flags"\ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" \ + -C "SSL - No CA Chain is set, but required to operate" # The purpose of the next two tests is to test the client's behaviour when receiving a server # certificate with an unsupported elliptic curve. This should usually not happen because From 84442a3bffb6ce0784ce8ae6535111210e1869bd Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 3 Apr 2024 08:57:09 +0200 Subject: [PATCH 29/44] ssl-opt.sh: Fix test case titles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ronald Cron Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 84342d588d..a6b59ae80a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6359,7 +6359,7 @@ run_test "Authentication, CA callback: server ECDH p256v1, client optional, p requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT -run_test "Authentication, CA callback: client SHA256, server required" \ +run_test "Authentication, CA callback: client SHA384, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server6.crt \ key_file=$DATA_FILES_PATH/server6.key \ @@ -6371,7 +6371,7 @@ run_test "Authentication, CA callback: client SHA256, server required" \ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT -run_test "Authentication, CA callback: client SHA384, server required" \ +run_test "Authentication, CA callback: client SHA256, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server6.crt \ key_file=$DATA_FILES_PATH/server6.key \ From cb7f63266fd6169918f6df6c958a7a6ae142f49a Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 3 Apr 2024 09:07:22 +0200 Subject: [PATCH 30/44] tls13: Add support for trusted certificate callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ronald Cron Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 60 +++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2104567c85..c130de0a84 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -629,6 +629,7 @@ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) { int ret = 0; + int have_ca_chain = 0; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; uint32_t verify_result = 0; @@ -696,27 +697,48 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) return 0; } -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if (ssl->handshake->sni_ca_chain != NULL) { - ca_chain = ssl->handshake->sni_ca_chain; - ca_crl = ssl->handshake->sni_ca_crl; - } else -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - { - ca_chain = ssl->conf->ca_chain; - ca_crl = ssl->conf->ca_crl; - } - /* * Main check: verify certificate */ - ret = mbedtls_x509_crt_verify_with_profile( - ssl->session_negotiate->peer_cert, - ca_chain, ca_crl, - ssl->conf->cert_profile, - ssl->hostname, - &verify_result, - ssl->conf->f_vrfy, ssl->conf->p_vrfy); +#if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) + if (ssl->conf->f_ca_cb != NULL) { + have_ca_chain = 1; + + MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); + ret = mbedtls_x509_crt_verify_with_ca_cb( + ssl->session_negotiate->peer_cert, + ssl->conf->f_ca_cb, + ssl->conf->p_ca_cb, + ssl->conf->cert_profile, + ssl->hostname, + &verify_result, + ssl->conf->f_vrfy, ssl->conf->p_vrfy); + } else +#endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ + { +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if (ssl->handshake->sni_ca_chain != NULL) { + ca_chain = ssl->handshake->sni_ca_chain; + ca_crl = ssl->handshake->sni_ca_crl; + } else +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + { + ca_chain = ssl->conf->ca_chain; + ca_crl = ssl->conf->ca_crl; + } + + if (ca_chain != NULL) { + have_ca_chain = 1; + } + + ret = mbedtls_x509_crt_verify_with_profile( + ssl->session_negotiate->peer_cert, + ca_chain, ca_crl, + ssl->conf->cert_profile, + ssl->hostname, + &verify_result, + ssl->conf->f_vrfy, ssl->conf->p_vrfy); + } if (ret != 0) { MBEDTLS_SSL_DEBUG_RET(1, "x509_verify_cert", ret); @@ -749,7 +771,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) ret = 0; } - if (ca_chain == NULL && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { + if (!have_ca_chain && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; } From 8d5da8f4a3afa1948f8fa26e3387f830a027fc24 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 3 Apr 2024 09:10:02 +0200 Subject: [PATCH 31/44] ssl-opt.sh: Test trusted certificate callback in TLS 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ronald Cron Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index a6b59ae80a..33a9d4f6f6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2155,7 +2155,7 @@ run_test "TLS: password protected server key, two certificates" \ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "CA callback on client" \ "$P_SRV debug_level=3" \ - "$P_CLI force_version=tls12 ca_callback=1 debug_level=3 " \ + "$P_CLI ca_callback=1 debug_level=3 " \ 0 \ -c "use CA callback for X.509 CRT verification" \ -S "error" \ @@ -2165,7 +2165,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_256 run_test "CA callback on server" \ - "$P_SRV force_version=tls12 auth_mode=required" \ + "$P_SRV auth_mode=required" \ "$P_CLI ca_callback=1 debug_level=3 crt_file=$DATA_FILES_PATH/server5.crt \ key_file=$DATA_FILES_PATH/server5.key" \ 0 \ @@ -6308,7 +6308,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server badcert, client required" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI force_version=tls12 ca_callback=1 debug_level=3 auth_mode=required" \ + "$P_CLI ca_callback=1 debug_level=3 auth_mode=required" \ 1 \ -c "use CA callback for X.509 CRT verification" \ -c "x509_verify_cert() returned" \ @@ -6320,7 +6320,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server badcert, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ - "$P_CLI force_version=tls12 ca_callback=1 debug_level=3 auth_mode=optional" \ + "$P_CLI ca_callback=1 debug_level=3 auth_mode=optional" \ 0 \ -c "use CA callback for X.509 CRT verification" \ -c "x509_verify_cert() returned" \ @@ -6328,6 +6328,18 @@ run_test "Authentication, CA callback: server badcert, client optional" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK +run_test "Authentication, CA callback: server badcert, client none" \ + "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ + key_file=$DATA_FILES_PATH/server5.key" \ + "$P_CLI ca_callback=1 debug_level=3 auth_mode=none" \ + 0 \ + -C "use CA callback for X.509 CRT verification" \ + -C "x509_verify_cert() returned" \ + -C "! The certificate is not correctly signed by the trusted CA" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + # The purpose of the next two tests is to test the client's behaviour when receiving a server # certificate with an unsupported elliptic curve. This should usually not happen because # the client informs the server about the supported curves - it does, though, in the @@ -6383,7 +6395,7 @@ run_test "Authentication, CA callback: client SHA256, server required" \ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client badcert, server required" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 auth_mode=required" \ + "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ 1 \ @@ -6398,7 +6410,6 @@ run_test "Authentication, CA callback: client badcert, server required" \ -s "! The certificate is not correctly signed by the trusted CA" \ -s "! mbedtls_ssl_handshake returned" \ -s "send alert level=2 message=48" \ - -c "! mbedtls_ssl_handshake returned" \ -s "X509 - Certificate verification failed" # We don't check that the client receives the alert because it might # detect that its write end of the connection is closed and abort @@ -6406,7 +6417,7 @@ run_test "Authentication, CA callback: client badcert, server required" \ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client cert not trusted, server required" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 auth_mode=required" \ + "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-selfsigned.crt \ key_file=$DATA_FILES_PATH/server5.key" \ 1 \ @@ -6420,12 +6431,11 @@ run_test "Authentication, CA callback: client cert not trusted, server requir -s "x509_verify_cert() returned" \ -s "! The certificate is not correctly signed by the trusted CA" \ -s "! mbedtls_ssl_handshake returned" \ - -c "! mbedtls_ssl_handshake returned" \ -s "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client badcert, server optional" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 auth_mode=optional" \ + "$P_SRV ca_callback=1 debug_level=3 auth_mode=optional" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ 0 \ @@ -6448,7 +6458,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int chain, client default" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c09.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/09.key" \ - "$P_CLI force_version=tls12 ca_callback=1 debug_level=3 server_name=CA09 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt" \ + "$P_CLI ca_callback=1 debug_level=3 server_name=CA09 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt" \ 0 \ -c "use CA callback for X.509 CRT verification" \ -C "X509 - A fatal error occurred" @@ -6459,7 +6469,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int+1 chain, client default" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ - "$P_CLI force_version=tls12 debug_level=3 ca_callback=1 server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt" \ + "$P_CLI debug_level=3 ca_callback=1 server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt" \ 1 \ -c "use CA callback for X.509 CRT verification" \ -c "X509 - A fatal error occurred" @@ -6470,7 +6480,7 @@ requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int+1 chain, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ - "$P_CLI force_version=tls12 ca_callback=1 server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt \ + "$P_CLI ca_callback=1 server_name=CA10 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt \ debug_level=3 auth_mode=optional" \ 1 \ -c "use CA callback for X.509 CRT verification" \ @@ -6480,7 +6490,7 @@ requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int+1 chain, server optional" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=optional" \ + "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=optional" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ 1 \ @@ -6491,7 +6501,7 @@ requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int+1 chain, server required" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ + "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ 1 \ @@ -6502,7 +6512,7 @@ requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int chain, server required" \ - "$P_SRV force_version=tls12 ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ + "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c09.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/09.key" \ 0 \ From 523a7e4aaf45ee13aaf4181b23c8f6b4b2a584c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Aug 2024 12:51:00 +0200 Subject: [PATCH 32/44] Restrict the scope of a few variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In particular, make sure pointer variables are initialized right after being declared. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 6 ++---- library/ssl_tls13_generic.c | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4795e673bd..ebd19c366c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7949,13 +7949,12 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, ssl->handshake->ciphersuite_info; int have_ca_chain = 0; - int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); - void *p_vrfy; - if (authmode == MBEDTLS_SSL_VERIFY_NONE) { return 0; } + int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); + void *p_vrfy; if (ssl->f_vrfy != NULL) { MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific verification callback")); f_vrfy = ssl->f_vrfy; @@ -7988,7 +7987,6 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, { mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; - #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if (ssl->handshake->sni_ca_chain != NULL) { ca_chain = ssl->handshake->sni_ca_chain; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index c130de0a84..f883a22f4f 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -630,8 +630,6 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) { int ret = 0; int have_ca_chain = 0; - mbedtls_x509_crt *ca_chain; - mbedtls_x509_crl *ca_crl; uint32_t verify_result = 0; /* Authmode: precedence order is SNI if used else configuration */ @@ -716,6 +714,8 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) } else #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ { + mbedtls_x509_crt *ca_chain; + mbedtls_x509_crl *ca_crl; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if (ssl->handshake->sni_ca_chain != NULL) { ca_chain = ssl->handshake->sni_ca_chain; From e910ac862788c57fe4aff0e8fcccd27a1bd5369c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Aug 2024 12:52:59 +0200 Subject: [PATCH 33/44] Improve a variable's name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 8 ++++---- library/ssl_tls13_generic.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ebd19c366c..c9cca703d6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7947,7 +7947,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, int ret = 0; const mbedtls_ssl_ciphersuite_t *ciphersuite_info = ssl->handshake->ciphersuite_info; - int have_ca_chain = 0; + int have_ca_chain_or_callback = 0; if (authmode == MBEDTLS_SSL_VERIFY_NONE) { return 0; @@ -7971,7 +7971,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) if (ssl->conf->f_ca_cb != NULL) { ((void) rs_ctx); - have_ca_chain = 1; + have_ca_chain_or_callback = 1; MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); ret = mbedtls_x509_crt_verify_with_ca_cb( @@ -7999,7 +7999,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, } if (ca_chain != NULL) { - have_ca_chain = 1; + have_ca_chain_or_callback = 1; } ret = mbedtls_x509_crt_verify_restartable( @@ -8061,7 +8061,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, ret = 0; } - if (have_ca_chain == 0 && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { + if (have_ca_chain_or_callback == 0 && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index f883a22f4f..6ea5e01d47 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -629,7 +629,7 @@ MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) { int ret = 0; - int have_ca_chain = 0; + int have_ca_chain_or_callback = 0; uint32_t verify_result = 0; /* Authmode: precedence order is SNI if used else configuration */ @@ -700,7 +700,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) */ #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) if (ssl->conf->f_ca_cb != NULL) { - have_ca_chain = 1; + have_ca_chain_or_callback = 1; MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); ret = mbedtls_x509_crt_verify_with_ca_cb( @@ -728,7 +728,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) } if (ca_chain != NULL) { - have_ca_chain = 1; + have_ca_chain_or_callback = 1; } ret = mbedtls_x509_crt_verify_with_profile( @@ -771,7 +771,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) ret = 0; } - if (!have_ca_chain && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { + if (!have_ca_chain_or_callback && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; } From dee6ffa961b6bdee43b5c3e767e3cd27fc7d30dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Aug 2024 09:53:41 +0200 Subject: [PATCH 34/44] Add support for context f_vrfy callback in 1.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was only supported in 1.2 for no good reason. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 1 + library/ssl_tls13_generic.c | 17 +++++++++++++++-- tests/ssl-opt.sh | 4 ++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c9cca703d6..a7c6cac6a4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7953,6 +7953,7 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, return 0; } + /* Verify callback: precedence order is SSL context, else conf struct. */ int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); void *p_vrfy; if (ssl->f_vrfy != NULL) { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 6ea5e01d47..fb57aa4a75 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -695,6 +695,19 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) return 0; } + /* Verify callback: precedence order is SSL context, else conf struct. */ + int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); + void *p_vrfy; + if (ssl->f_vrfy != NULL) { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific verification callback")); + f_vrfy = ssl->f_vrfy; + p_vrfy = ssl->p_vrfy; + } else { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use configuration-specific verification callback")); + f_vrfy = ssl->conf->f_vrfy; + p_vrfy = ssl->conf->p_vrfy; + } + /* * Main check: verify certificate */ @@ -710,7 +723,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) ssl->conf->cert_profile, ssl->hostname, &verify_result, - ssl->conf->f_vrfy, ssl->conf->p_vrfy); + f_vrfy, p_vrfy); } else #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ { @@ -737,7 +750,7 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) ssl->conf->cert_profile, ssl->hostname, &verify_result, - ssl->conf->f_vrfy, ssl->conf->p_vrfy); + f_vrfy, p_vrfy); } if (ret != 0) { diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 33a9d4f6f6..2f89ece9e8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2724,7 +2724,7 @@ run_test "Single supported algorithm sending: openssl client" \ # Tests for certificate verification callback run_test "Configuration-specific CRT verification callback" \ "$P_SRV debug_level=3" \ - "$P_CLI force_version=tls12 context_crt_cb=0 debug_level=3" \ + "$P_CLI context_crt_cb=0 debug_level=3" \ 0 \ -S "error" \ -c "Verify requested for " \ @@ -2734,7 +2734,7 @@ run_test "Configuration-specific CRT verification callback" \ run_test "Context-specific CRT verification callback" \ "$P_SRV debug_level=3" \ - "$P_CLI force_version=tls12 context_crt_cb=1 debug_level=3" \ + "$P_CLI context_crt_cb=1 debug_level=3" \ 0 \ -S "error" \ -c "Verify requested for " \ From d37054c82455ce673fe59a1a1b374b9ac1dd7986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Aug 2024 10:01:48 +0200 Subject: [PATCH 35/44] Minor refactoring of generic SSL certificate verif MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename as there was a name collision with a static function in another file: ssl_parse_certificate_verify in ssl_tls12_server.c is the function that parses the CertificateVerify message, which seems appropriate. Here it meant "the 'verify' step after parsing the Certificate message". Use a name that focuses on what it does: verify, not parse. Also, take ciphersuite_info as an argument: when TLS 1.3 calls this function, it can pass NULL as the ciphersuite has no influence there. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a7c6cac6a4..ad8f3f0e35 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7939,14 +7939,13 @@ static int ssl_parse_certificate_coordinate(mbedtls_ssl_context *ssl, } MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl, - int authmode, - mbedtls_x509_crt *chain, - void *rs_ctx) +static int ssl_verify_certificate(mbedtls_ssl_context *ssl, + int authmode, + mbedtls_x509_crt *chain, + const mbedtls_ssl_ciphersuite_t *ciphersuite_info, + void *rs_ctx) { int ret = 0; - const mbedtls_ssl_ciphersuite_t *ciphersuite_info = - ssl->handshake->ciphersuite_info; int have_ca_chain_or_callback = 0; if (authmode == MBEDTLS_SSL_VERIFY_NONE) { @@ -8246,8 +8245,8 @@ crt_verify: } #endif - ret = ssl_parse_certificate_verify(ssl, authmode, - chain, rs_ctx); + ret = ssl_verify_certificate(ssl, authmode, chain, + ssl->handshake->ciphersuite_info, rs_ctx); if (ret != 0) { goto exit; } From ce60330dfb4b6b9cc6d85b0b74020fa7d47bd385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Aug 2024 11:03:42 +0200 Subject: [PATCH 36/44] Merge 1.2 and 1.3 certificate verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_misc.h | 32 ++++++++ library/ssl_tls.c | 45 ++++++----- library/ssl_tls13_generic.c | 153 +----------------------------------- 3 files changed, 62 insertions(+), 168 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index ecb2d03906..e04f4ee446 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1673,6 +1673,38 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert(mbedtls_ssl_context *ssl) return key_cert == NULL ? NULL : key_cert->cert; } +/* + * Verify a certificate. + * + * [in/out] ssl: misc. things read + * ssl->session_negotiate->verify_result updated + * [in] authmode: one of MBEDTLS_SSL_VERIFY_{NONE,OPTIONAL,REQUIRED} + * [in] chain: the certificate chain to verify (ie the peer's chain) + * [in] ciphersuite_info: For TLS 1.2, this session's ciphersuite; + * for TLS 1.3, may be left NULL. + * [in] rs_ctx: restart context if restartable ECC is in use; + * leave NULL for no restartable behaviour. + * + * Return: + * - 0 if the certificate is the handshake should continue. Depending on the + * authmode it means: + * - REQUIRED: the certificate was found to be valid, trusted & acceptable. + * ssl->session_negotiate->verify_result is 0. + * - OPTIONAL: the certificate may or may not be acceptable, but + * ssl->session_negotiate->verify_result was updated with the result. + * - NONE: the certificate wasn't even checked. + * - MBEDTLS_ERR_X509_CERT_VERIFY_FAILED or MBEDTLS_ERR_SSL_BAD_CERTIFICATE if + * the certificate was found to be invalid/untrusted/unacceptable and the + * handshake should be aborted (can only happen with REQUIRED). + * - another error code if another error happened (out-of-memory, etc.) + */ +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, + int authmode, + mbedtls_x509_crt *chain, + const mbedtls_ssl_ciphersuite_t *ciphersuite_info, + void *rs_ctx); + /* * Check usage of a certificate wrt usage extensions: * keyUsage and extendedKeyUsage. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ad8f3f0e35..ad410dc750 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7938,12 +7938,11 @@ static int ssl_parse_certificate_coordinate(mbedtls_ssl_context *ssl, return SSL_CERTIFICATE_EXPECTED; } -MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_verify_certificate(mbedtls_ssl_context *ssl, - int authmode, - mbedtls_x509_crt *chain, - const mbedtls_ssl_ciphersuite_t *ciphersuite_info, - void *rs_ctx) +int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, + int authmode, + mbedtls_x509_crt *chain, + const mbedtls_ssl_ciphersuite_t *ciphersuite_info, + void *rs_ctx) { int ret = 0; int have_ca_chain_or_callback = 0; @@ -8025,23 +8024,32 @@ static int ssl_verify_certificate(mbedtls_ssl_context *ssl, * Secondary checks: always done, but change 'ret' only if it was 0 */ - /* Check curve for ECC certs */ -#if defined(MBEDTLS_PK_HAVE_ECC_KEYS) - if (mbedtls_pk_can_do(&chain->pk, MBEDTLS_PK_ECKEY) && - mbedtls_ssl_check_curve(ssl, mbedtls_pk_get_ec_group_id(&chain->pk)) != 0) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); - ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + /* With TLS 1.2 and ECC certs, check that the curve used by the + * certificate is on our list of acceptable curves. + * + * With TLS 1.3 this is not needed because the curve is part of the + * signature algorithm (eg ecdsa_secp256r1_sha256) which is checked when + * we validate the signature made with the key associated to this cert. + */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_PK_HAVE_ECC_KEYS) + if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && + mbedtls_pk_can_do(&chain->pk, MBEDTLS_PK_ECKEY)) { + if (mbedtls_ssl_check_curve(ssl, mbedtls_pk_get_ec_group_id(&chain->pk)) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); + ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; + if (ret == 0) { + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } } } -#endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_PK_HAVE_ECC_KEYS */ /* Check X.509 usage extensions (keyUsage, extKeyUsage) */ if (mbedtls_ssl_check_cert_usage(chain, ciphersuite_info, ssl->conf->endpoint, - MBEDTLS_SSL_VERSION_TLS1_2, + ssl->tls_version, &ssl->session_negotiate->verify_result) != 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); if (ret == 0) { @@ -8245,8 +8253,9 @@ crt_verify: } #endif - ret = ssl_verify_certificate(ssl, authmode, chain, - ssl->handshake->ciphersuite_info, rs_ctx); + ret = mbedtls_ssl_verify_certificate(ssl, authmode, chain, + ssl->handshake->ciphersuite_info, + rs_ctx); if (ret != 0) { goto exit; } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index fb57aa4a75..3f1f551dd1 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -628,10 +628,6 @@ int mbedtls_ssl_tls13_parse_certificate(mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) { - int ret = 0; - int have_ca_chain_or_callback = 0; - uint32_t verify_result = 0; - /* Authmode: precedence order is SNI if used else configuration */ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) const int authmode = ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET @@ -683,152 +679,9 @@ static int ssl_tls13_validate_certificate(mbedtls_ssl_context *ssl) #endif /* MBEDTLS_SSL_CLI_C */ } - /* - * NONE means we skip all checks - * - * Note: we still check above that the server did send a certificate, - * because only a non-compliant server would fail to do so. NONE means we - * don't care about the server certificate being valid, but we still care - * about the server otherwise following the TLS standard. - */ - if (authmode == MBEDTLS_SSL_VERIFY_NONE) { - return 0; - } - - /* Verify callback: precedence order is SSL context, else conf struct. */ - int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); - void *p_vrfy; - if (ssl->f_vrfy != NULL) { - MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific verification callback")); - f_vrfy = ssl->f_vrfy; - p_vrfy = ssl->p_vrfy; - } else { - MBEDTLS_SSL_DEBUG_MSG(3, ("Use configuration-specific verification callback")); - f_vrfy = ssl->conf->f_vrfy; - p_vrfy = ssl->conf->p_vrfy; - } - - /* - * Main check: verify certificate - */ -#if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) - if (ssl->conf->f_ca_cb != NULL) { - have_ca_chain_or_callback = 1; - - MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); - ret = mbedtls_x509_crt_verify_with_ca_cb( - ssl->session_negotiate->peer_cert, - ssl->conf->f_ca_cb, - ssl->conf->p_ca_cb, - ssl->conf->cert_profile, - ssl->hostname, - &verify_result, - f_vrfy, p_vrfy); - } else -#endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ - { - mbedtls_x509_crt *ca_chain; - mbedtls_x509_crl *ca_crl; -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if (ssl->handshake->sni_ca_chain != NULL) { - ca_chain = ssl->handshake->sni_ca_chain; - ca_crl = ssl->handshake->sni_ca_crl; - } else -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - { - ca_chain = ssl->conf->ca_chain; - ca_crl = ssl->conf->ca_crl; - } - - if (ca_chain != NULL) { - have_ca_chain_or_callback = 1; - } - - ret = mbedtls_x509_crt_verify_with_profile( - ssl->session_negotiate->peer_cert, - ca_chain, ca_crl, - ssl->conf->cert_profile, - ssl->hostname, - &verify_result, - f_vrfy, p_vrfy); - } - - if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "x509_verify_cert", ret); - } - - /* - * Secondary checks: always done, but change 'ret' only if it was 0 - */ - if (mbedtls_ssl_check_cert_usage(ssl->session_negotiate->peer_cert, - NULL, - ssl->conf->endpoint, - MBEDTLS_SSL_VERSION_TLS1_3, - &verify_result) != 0) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; - } - } - - /* mbedtls_x509_crt_verify_with_profile is supposed to report a - * verification failure through MBEDTLS_ERR_X509_CERT_VERIFY_FAILED, - * with details encoded in the verification flags. All other kinds - * of error codes, including those from the user provided f_vrfy - * functions, are treated as fatal and lead to a failure of - * mbedtls_ssl_tls13_parse_certificate even if verification was optional. - */ - if (authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && - (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || - ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE)) { - ret = 0; - } - - if (!have_ca_chain_or_callback && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { - MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); - ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; - } - - if (ret != 0) { - /* The certificate may have been rejected for several reasons. - Pick one and send the corresponding alert. Which alert to send - may be a subject of debate in some cases. */ - if (verify_result & MBEDTLS_X509_BADCERT_OTHER) { - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED, ret); - } else if (verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH) { - MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_BAD_CERT, ret); - } else if (verify_result & (MBEDTLS_X509_BADCERT_KEY_USAGE | - MBEDTLS_X509_BADCERT_EXT_KEY_USAGE | - MBEDTLS_X509_BADCERT_BAD_PK | - MBEDTLS_X509_BADCERT_BAD_KEY)) { - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret); - } else if (verify_result & MBEDTLS_X509_BADCERT_EXPIRED) { - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED, ret); - } else if (verify_result & MBEDTLS_X509_BADCERT_REVOKED) { - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED, ret); - } else if (verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED) { - MBEDTLS_SSL_PEND_FATAL_ALERT(MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA, ret); - } else { - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_CERT_UNKNOWN, ret); - } - } - -#if defined(MBEDTLS_DEBUG_C) - if (verify_result != 0) { - MBEDTLS_SSL_DEBUG_MSG(3, ("! Certificate verification flags %08x", - (unsigned int) verify_result)); - } else { - MBEDTLS_SSL_DEBUG_MSG(3, ("Certificate verification flags clear")); - } -#endif /* MBEDTLS_DEBUG_C */ - - ssl->session_negotiate->verify_result = verify_result; - return ret; + return mbedtls_ssl_verify_certificate(ssl, authmode, + ssl->session_negotiate->peer_cert, + NULL, NULL); } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ MBEDTLS_CHECK_RETURN_CRITICAL From f2aa65fd57d21287cfdd142531b069caf096f83d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Aug 2024 11:19:51 +0200 Subject: [PATCH 37/44] Improve some comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ad410dc750..b2fdcaaee3 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7944,14 +7944,13 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t *ciphersuite_info, void *rs_ctx) { - int ret = 0; - int have_ca_chain_or_callback = 0; - if (authmode == MBEDTLS_SSL_VERIFY_NONE) { return 0; } - /* Verify callback: precedence order is SSL context, else conf struct. */ + /* + * Primary check: use the appropriate X.509 verification function + */ int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); void *p_vrfy; if (ssl->f_vrfy != NULL) { @@ -7964,9 +7963,8 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, p_vrfy = ssl->conf->p_vrfy; } - /* - * Main check: verify certificate - */ + int ret = 0; + int have_ca_chain_or_callback = 0; #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) if (ssl->conf->f_ca_cb != NULL) { ((void) rs_ctx); @@ -8057,18 +8055,22 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, } } - /* mbedtls_x509_crt_verify_with_profile is supposed to report a - * verification failure through MBEDTLS_ERR_X509_CERT_VERIFY_FAILED, - * with details encoded in the verification flags. All other kinds - * of error codes, including those from the user provided f_vrfy - * functions, are treated as fatal and lead to a failure of - * ssl_parse_certificate even if verification was optional. */ + /* With authmode optional, we want to keep going it the certificate was + * unacceptable, but still fail on other error (out of memory etc), + * including fatal errors from the f_vrfy callback. + * + * The only acceptable errors are: + * - MBEDTLS_ERR_X509_CERT_VERIFY_FAILED: cert rejected by primary check; + * - MBEDTLS_ERR_SSL_BAD_CERTIFICATE: cert rejected by secondary checks. + * Anything else is a fatal error. */ if (authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE)) { ret = 0; } + /* Return a specific error as this is a user error: inconsistent + * configuration - can't verify without trust anchors. */ if (have_ca_chain_or_callback == 0 && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; From ff28e4c7f449ca5e1b30cb9755714f1e6bd1ee2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Aug 2024 12:57:34 +0200 Subject: [PATCH 38/44] Fix two dependency declarations in ssl-opt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2f89ece9e8..c3f7c25fdb 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -2722,6 +2722,7 @@ run_test "Single supported algorithm sending: openssl client" \ 0 # Tests for certificate verification callback +requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Configuration-specific CRT verification callback" \ "$P_SRV debug_level=3" \ "$P_CLI context_crt_cb=0 debug_level=3" \ @@ -2732,6 +2733,7 @@ run_test "Configuration-specific CRT verification callback" \ -C "Use context-specific verification callback" \ -C "error" +requires_key_exchange_with_cert_in_tls12_or_tls13_enabled run_test "Context-specific CRT verification callback" \ "$P_SRV debug_level=3" \ "$P_CLI context_crt_cb=1 debug_level=3" \ From 565da768a4e8c4769825a411a22501ef6c5a9829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 20 Aug 2024 10:58:20 +0200 Subject: [PATCH 39/44] Fix typos in comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Horstmann Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_misc.h | 2 +- library/ssl_tls.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index e04f4ee446..762c478328 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1686,7 +1686,7 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert(mbedtls_ssl_context *ssl) * leave NULL for no restartable behaviour. * * Return: - * - 0 if the certificate is the handshake should continue. Depending on the + * - 0 if the handshake should continue. Depending on the * authmode it means: * - REQUIRED: the certificate was found to be valid, trusted & acceptable. * ssl->session_negotiate->verify_result is 0. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b2fdcaaee3..2cff7a9fd3 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6386,7 +6386,7 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, } else #endif { - /* This is either TLS 1.3 autentication, which always uses signatures, + /* This is either TLS 1.3 authentication, which always uses signatures, * or 1.2 client auth: rsa_sign and mbedtls_ecdsa_sign are the only * options we implement, both using signatures. */ (void) tls_version; @@ -8055,8 +8055,8 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, } } - /* With authmode optional, we want to keep going it the certificate was - * unacceptable, but still fail on other error (out of memory etc), + /* With authmode optional, we want to keep going if the certificate was + * unacceptable, but still fail on other errors (out of memory etc), * including fatal errors from the f_vrfy callback. * * The only acceptable errors are: From c32a4a2128fa4a3ac2a6ea3b6d7a197bcdc7d982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 20 Aug 2024 12:14:43 +0200 Subject: [PATCH 40/44] Fix guards around function now used by 1.3 as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Actually moved the function rather than trying to edit guards around it, because the relevant guards are not nearby, the function was part of larger blocks, so it seemed risky. Also, that seems logically correct: the function is no longer part of the "TLS 1.2 handshake functions common to server and client" section, it's part of the "helper functions common to 1.2 and 1.3 server and client" block. Ideally in the future perhaps the file structure should reflect that (`ssl_generic.c` vs `ssl_tls12_generic.c`?) but that's out of scope here. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 536 +++++++++++++++++++++++----------------------- 1 file changed, 270 insertions(+), 266 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2cff7a9fd3..8f5c37ad42 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6335,91 +6335,6 @@ const char *mbedtls_ssl_get_curve_name_from_tls_id(uint16_t tls_id) } #endif -#if defined(MBEDTLS_X509_CRT_PARSE_C) -int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, - const mbedtls_ssl_ciphersuite_t *ciphersuite, - int recv_endpoint, - mbedtls_ssl_protocol_version tls_version, - uint32_t *flags) -{ - int ret = 0; - unsigned int usage = 0; - const char *ext_oid; - size_t ext_len; - - /* - * keyUsage - */ - - /* Note: don't guard this with MBEDTLS_SSL_CLI_C because the server wants - * to check what a compliant client will think while choosing which cert - * to send to the client. */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && - recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { - /* TLS 1.2 server part of the key exchange */ - switch (ciphersuite->key_exchange) { - case MBEDTLS_KEY_EXCHANGE_RSA: - case MBEDTLS_KEY_EXCHANGE_RSA_PSK: - usage = MBEDTLS_X509_KU_KEY_ENCIPHERMENT; - break; - - case MBEDTLS_KEY_EXCHANGE_DHE_RSA: - case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: - case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: - usage = MBEDTLS_X509_KU_DIGITAL_SIGNATURE; - break; - - case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: - case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: - usage = MBEDTLS_X509_KU_KEY_AGREEMENT; - break; - - /* Don't use default: we want warnings when adding new values */ - case MBEDTLS_KEY_EXCHANGE_NONE: - case MBEDTLS_KEY_EXCHANGE_PSK: - case MBEDTLS_KEY_EXCHANGE_DHE_PSK: - case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: - case MBEDTLS_KEY_EXCHANGE_ECJPAKE: - usage = 0; - } - } else -#endif - { - /* This is either TLS 1.3 authentication, which always uses signatures, - * or 1.2 client auth: rsa_sign and mbedtls_ecdsa_sign are the only - * options we implement, both using signatures. */ - (void) tls_version; - (void) ciphersuite; - usage = MBEDTLS_X509_KU_DIGITAL_SIGNATURE; - } - - if (mbedtls_x509_crt_check_key_usage(cert, usage) != 0) { - *flags |= MBEDTLS_X509_BADCERT_KEY_USAGE; - ret = -1; - } - - /* - * extKeyUsage - */ - - if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { - ext_oid = MBEDTLS_OID_SERVER_AUTH; - ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); - } else { - ext_oid = MBEDTLS_OID_CLIENT_AUTH; - ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_CLIENT_AUTH); - } - - if (mbedtls_x509_crt_check_extended_key_usage(cert, ext_oid, ext_len) != 0) { - *flags |= MBEDTLS_X509_BADCERT_EXT_KEY_USAGE; - ret = -1; - } - - return ret; -} -#endif /* MBEDTLS_X509_CRT_PARSE_C */ - #if defined(MBEDTLS_USE_PSA_CRYPTO) int mbedtls_ssl_get_handshake_transcript(mbedtls_ssl_context *ssl, const mbedtls_md_type_t md, @@ -7938,187 +7853,6 @@ static int ssl_parse_certificate_coordinate(mbedtls_ssl_context *ssl, return SSL_CERTIFICATE_EXPECTED; } -int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, - int authmode, - mbedtls_x509_crt *chain, - const mbedtls_ssl_ciphersuite_t *ciphersuite_info, - void *rs_ctx) -{ - if (authmode == MBEDTLS_SSL_VERIFY_NONE) { - return 0; - } - - /* - * Primary check: use the appropriate X.509 verification function - */ - int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); - void *p_vrfy; - if (ssl->f_vrfy != NULL) { - MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific verification callback")); - f_vrfy = ssl->f_vrfy; - p_vrfy = ssl->p_vrfy; - } else { - MBEDTLS_SSL_DEBUG_MSG(3, ("Use configuration-specific verification callback")); - f_vrfy = ssl->conf->f_vrfy; - p_vrfy = ssl->conf->p_vrfy; - } - - int ret = 0; - int have_ca_chain_or_callback = 0; -#if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) - if (ssl->conf->f_ca_cb != NULL) { - ((void) rs_ctx); - have_ca_chain_or_callback = 1; - - MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); - ret = mbedtls_x509_crt_verify_with_ca_cb( - chain, - ssl->conf->f_ca_cb, - ssl->conf->p_ca_cb, - ssl->conf->cert_profile, - ssl->hostname, - &ssl->session_negotiate->verify_result, - f_vrfy, p_vrfy); - } else -#endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ - { - mbedtls_x509_crt *ca_chain; - mbedtls_x509_crl *ca_crl; -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if (ssl->handshake->sni_ca_chain != NULL) { - ca_chain = ssl->handshake->sni_ca_chain; - ca_crl = ssl->handshake->sni_ca_crl; - } else -#endif - { - ca_chain = ssl->conf->ca_chain; - ca_crl = ssl->conf->ca_crl; - } - - if (ca_chain != NULL) { - have_ca_chain_or_callback = 1; - } - - ret = mbedtls_x509_crt_verify_restartable( - chain, - ca_chain, ca_crl, - ssl->conf->cert_profile, - ssl->hostname, - &ssl->session_negotiate->verify_result, - f_vrfy, p_vrfy, rs_ctx); - } - - if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET(1, "x509_verify_cert", ret); - } - -#if defined(MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED) - if (ret == MBEDTLS_ERR_ECP_IN_PROGRESS) { - return MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; - } -#endif - - /* - * Secondary checks: always done, but change 'ret' only if it was 0 - */ - - /* With TLS 1.2 and ECC certs, check that the curve used by the - * certificate is on our list of acceptable curves. - * - * With TLS 1.3 this is not needed because the curve is part of the - * signature algorithm (eg ecdsa_secp256r1_sha256) which is checked when - * we validate the signature made with the key associated to this cert. - */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ - defined(MBEDTLS_PK_HAVE_ECC_KEYS) - if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && - mbedtls_pk_can_do(&chain->pk, MBEDTLS_PK_ECKEY)) { - if (mbedtls_ssl_check_curve(ssl, mbedtls_pk_get_ec_group_id(&chain->pk)) != 0) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); - ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; - } - } - } -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_PK_HAVE_ECC_KEYS */ - - /* Check X.509 usage extensions (keyUsage, extKeyUsage) */ - if (mbedtls_ssl_check_cert_usage(chain, - ciphersuite_info, - ssl->conf->endpoint, - ssl->tls_version, - &ssl->session_negotiate->verify_result) != 0) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); - if (ret == 0) { - ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; - } - } - - /* With authmode optional, we want to keep going if the certificate was - * unacceptable, but still fail on other errors (out of memory etc), - * including fatal errors from the f_vrfy callback. - * - * The only acceptable errors are: - * - MBEDTLS_ERR_X509_CERT_VERIFY_FAILED: cert rejected by primary check; - * - MBEDTLS_ERR_SSL_BAD_CERTIFICATE: cert rejected by secondary checks. - * Anything else is a fatal error. */ - if (authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && - (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || - ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE)) { - ret = 0; - } - - /* Return a specific error as this is a user error: inconsistent - * configuration - can't verify without trust anchors. */ - if (have_ca_chain_or_callback == 0 && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { - MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); - ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; - } - - if (ret != 0) { - uint8_t alert; - - /* The certificate may have been rejected for several reasons. - Pick one and send the corresponding alert. Which alert to send - may be a subject of debate in some cases. */ - if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_OTHER) { - alert = MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH) { - alert = MBEDTLS_SSL_ALERT_MSG_BAD_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE) { - alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE) { - alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_PK) { - alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_KEY) { - alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXPIRED) { - alert = MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_REVOKED) { - alert = MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED; - } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED) { - alert = MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA; - } else { - alert = MBEDTLS_SSL_ALERT_MSG_CERT_UNKNOWN; - } - mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - alert); - } - -#if defined(MBEDTLS_DEBUG_C) - if (ssl->session_negotiate->verify_result != 0) { - MBEDTLS_SSL_DEBUG_MSG(3, ("! Certificate verification flags %08x", - (unsigned int) ssl->session_negotiate->verify_result)); - } else { - MBEDTLS_SSL_DEBUG_MSG(3, ("Certificate verification flags clear")); - } -#endif /* MBEDTLS_DEBUG_C */ - - return ret; -} - #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_remember_peer_crt_digest(mbedtls_ssl_context *ssl, @@ -9923,4 +9657,274 @@ int mbedtls_ssl_session_set_ticket_alpn(mbedtls_ssl_session *session, return 0; } #endif /* MBEDTLS_SSL_SRV_C && MBEDTLS_SSL_EARLY_DATA && MBEDTLS_SSL_ALPN */ + +/* + * The following functions are used by 1.2 and 1.3, client and server. + */ +#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) +int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, + const mbedtls_ssl_ciphersuite_t *ciphersuite, + int recv_endpoint, + mbedtls_ssl_protocol_version tls_version, + uint32_t *flags) +{ + int ret = 0; + unsigned int usage = 0; + const char *ext_oid; + size_t ext_len; + + /* + * keyUsage + */ + + /* Note: don't guard this with MBEDTLS_SSL_CLI_C because the server wants + * to check what a compliant client will think while choosing which cert + * to send to the client. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if (tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && + recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { + /* TLS 1.2 server part of the key exchange */ + switch (ciphersuite->key_exchange) { + case MBEDTLS_KEY_EXCHANGE_RSA: + case MBEDTLS_KEY_EXCHANGE_RSA_PSK: + usage = MBEDTLS_X509_KU_KEY_ENCIPHERMENT; + break; + + case MBEDTLS_KEY_EXCHANGE_DHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA: + usage = MBEDTLS_X509_KU_DIGITAL_SIGNATURE; + break; + + case MBEDTLS_KEY_EXCHANGE_ECDH_RSA: + case MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA: + usage = MBEDTLS_X509_KU_KEY_AGREEMENT; + break; + + /* Don't use default: we want warnings when adding new values */ + case MBEDTLS_KEY_EXCHANGE_NONE: + case MBEDTLS_KEY_EXCHANGE_PSK: + case MBEDTLS_KEY_EXCHANGE_DHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECDHE_PSK: + case MBEDTLS_KEY_EXCHANGE_ECJPAKE: + usage = 0; + } + } else +#endif + { + /* This is either TLS 1.3 authentication, which always uses signatures, + * or 1.2 client auth: rsa_sign and mbedtls_ecdsa_sign are the only + * options we implement, both using signatures. */ + (void) tls_version; + (void) ciphersuite; + usage = MBEDTLS_X509_KU_DIGITAL_SIGNATURE; + } + + if (mbedtls_x509_crt_check_key_usage(cert, usage) != 0) { + *flags |= MBEDTLS_X509_BADCERT_KEY_USAGE; + ret = -1; + } + + /* + * extKeyUsage + */ + + if (recv_endpoint == MBEDTLS_SSL_IS_CLIENT) { + ext_oid = MBEDTLS_OID_SERVER_AUTH; + ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH); + } else { + ext_oid = MBEDTLS_OID_CLIENT_AUTH; + ext_len = MBEDTLS_OID_SIZE(MBEDTLS_OID_CLIENT_AUTH); + } + + if (mbedtls_x509_crt_check_extended_key_usage(cert, ext_oid, ext_len) != 0) { + *flags |= MBEDTLS_X509_BADCERT_EXT_KEY_USAGE; + ret = -1; + } + + return ret; +} + +int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, + int authmode, + mbedtls_x509_crt *chain, + const mbedtls_ssl_ciphersuite_t *ciphersuite_info, + void *rs_ctx) +{ + if (authmode == MBEDTLS_SSL_VERIFY_NONE) { + return 0; + } + + /* + * Primary check: use the appropriate X.509 verification function + */ + int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *); + void *p_vrfy; + if (ssl->f_vrfy != NULL) { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific verification callback")); + f_vrfy = ssl->f_vrfy; + p_vrfy = ssl->p_vrfy; + } else { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use configuration-specific verification callback")); + f_vrfy = ssl->conf->f_vrfy; + p_vrfy = ssl->conf->p_vrfy; + } + + int ret = 0; + int have_ca_chain_or_callback = 0; +#if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) + if (ssl->conf->f_ca_cb != NULL) { + ((void) rs_ctx); + have_ca_chain_or_callback = 1; + + MBEDTLS_SSL_DEBUG_MSG(3, ("use CA callback for X.509 CRT verification")); + ret = mbedtls_x509_crt_verify_with_ca_cb( + chain, + ssl->conf->f_ca_cb, + ssl->conf->p_ca_cb, + ssl->conf->cert_profile, + ssl->hostname, + &ssl->session_negotiate->verify_result, + f_vrfy, p_vrfy); + } else +#endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ + { + mbedtls_x509_crt *ca_chain; + mbedtls_x509_crl *ca_crl; +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if (ssl->handshake->sni_ca_chain != NULL) { + ca_chain = ssl->handshake->sni_ca_chain; + ca_crl = ssl->handshake->sni_ca_crl; + } else +#endif + { + ca_chain = ssl->conf->ca_chain; + ca_crl = ssl->conf->ca_crl; + } + + if (ca_chain != NULL) { + have_ca_chain_or_callback = 1; + } + + ret = mbedtls_x509_crt_verify_restartable( + chain, + ca_chain, ca_crl, + ssl->conf->cert_profile, + ssl->hostname, + &ssl->session_negotiate->verify_result, + f_vrfy, p_vrfy, rs_ctx); + } + + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "x509_verify_cert", ret); + } + +#if defined(MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED) + if (ret == MBEDTLS_ERR_ECP_IN_PROGRESS) { + return MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS; + } +#endif + + /* + * Secondary checks: always done, but change 'ret' only if it was 0 + */ + + /* With TLS 1.2 and ECC certs, check that the curve used by the + * certificate is on our list of acceptable curves. + * + * With TLS 1.3 this is not needed because the curve is part of the + * signature algorithm (eg ecdsa_secp256r1_sha256) which is checked when + * we validate the signature made with the key associated to this cert. + */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ + defined(MBEDTLS_PK_HAVE_ECC_KEYS) + if (ssl->tls_version == MBEDTLS_SSL_VERSION_TLS1_2 && + mbedtls_pk_can_do(&chain->pk, MBEDTLS_PK_ECKEY)) { + if (mbedtls_ssl_check_curve(ssl, mbedtls_pk_get_ec_group_id(&chain->pk)) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (EC key curve)")); + ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; + if (ret == 0) { + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } + } + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_PK_HAVE_ECC_KEYS */ + + /* Check X.509 usage extensions (keyUsage, extKeyUsage) */ + if (mbedtls_ssl_check_cert_usage(chain, + ciphersuite_info, + ssl->conf->endpoint, + ssl->tls_version, + &ssl->session_negotiate->verify_result) != 0) { + MBEDTLS_SSL_DEBUG_MSG(1, ("bad certificate (usage extensions)")); + if (ret == 0) { + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } + } + + /* With authmode optional, we want to keep going if the certificate was + * unacceptable, but still fail on other errors (out of memory etc), + * including fatal errors from the f_vrfy callback. + * + * The only acceptable errors are: + * - MBEDTLS_ERR_X509_CERT_VERIFY_FAILED: cert rejected by primary check; + * - MBEDTLS_ERR_SSL_BAD_CERTIFICATE: cert rejected by secondary checks. + * Anything else is a fatal error. */ + if (authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && + (ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || + ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE)) { + ret = 0; + } + + /* Return a specific error as this is a user error: inconsistent + * configuration - can't verify without trust anchors. */ + if (have_ca_chain_or_callback == 0 && authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { + MBEDTLS_SSL_DEBUG_MSG(1, ("got no CA chain")); + ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; + } + + if (ret != 0) { + uint8_t alert; + + /* The certificate may have been rejected for several reasons. + Pick one and send the corresponding alert. Which alert to send + may be a subject of debate in some cases. */ + if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_OTHER) { + alert = MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH) { + alert = MBEDTLS_SSL_ALERT_MSG_BAD_CERT; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE) { + alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE) { + alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_PK) { + alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_KEY) { + alert = MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXPIRED) { + alert = MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_REVOKED) { + alert = MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED; + } else if (ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED) { + alert = MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA; + } else { + alert = MBEDTLS_SSL_ALERT_MSG_CERT_UNKNOWN; + } + mbedtls_ssl_send_alert_message(ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + alert); + } + +#if defined(MBEDTLS_DEBUG_C) + if (ssl->session_negotiate->verify_result != 0) { + MBEDTLS_SSL_DEBUG_MSG(3, ("! Certificate verification flags %08x", + (unsigned int) ssl->session_negotiate->verify_result)); + } else { + MBEDTLS_SSL_DEBUG_MSG(3, ("Certificate verification flags clear")); + } +#endif /* MBEDTLS_DEBUG_C */ + + return ret; +} +#endif /* MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED */ + #endif /* MBEDTLS_SSL_TLS_C */ From f4f3e92ac9f754e3bf0f606078e626a7ecc698a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 20 Aug 2024 22:00:02 +0200 Subject: [PATCH 41/44] Add a ChangeLog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/tls13-cert-regressions.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 ChangeLog.d/tls13-cert-regressions.txt diff --git a/ChangeLog.d/tls13-cert-regressions.txt b/ChangeLog.d/tls13-cert-regressions.txt new file mode 100644 index 0000000000..8dd8a327d6 --- /dev/null +++ b/ChangeLog.d/tls13-cert-regressions.txt @@ -0,0 +1,18 @@ +Bugfix + * Fixed a regression introduced in 3.6.0 where the CA callback set with + mbedtls_ssl_conf_ca_cb() would stop working when connections were + upgraded to TLS 1.3. Fixed by adding support for the CA callback with TLS + 1.3. + * Fixed a regression introduced in 3.6.0 where clients that relied on + optional/none authentication mode, by calling mbedtls_ssl_conf_authmode() + with MBEDTLS_SSL_VERIFY_OPTIONAL or MBEDTLS_SSL_VERIFY_NONE, would stop + working when connections were upgraded to TLS 1.3. Fixed by adding + support for optional/none with TLS 1.3 as well. Note that the TLS 1.3 + standard makes server authentication mandatory; users are advised not to + use authmode none, and to carefully check the results when using optional + mode. + * Fixed a regression introduced in 3.6.0 where context-specific certificate + verify callbacks, set with mbedtls_ssl_set_verify() as opposed to + mbedtls_ssl_conf_verify(), would stop working when connections were + upgraded to TLS 1.3. Fixed by adding support for context-specific verify + callback in TLS 1.3. From 58da249465bd16ed05ce1c673665ca2350d9cfaf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 21 Aug 2024 22:03:16 +0200 Subject: [PATCH 42/44] Changelog entry for the RSA memory leak Signed-off-by: Gilles Peskine --- .../mbedtls_psa_rsa_load_representation-memory_leak.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/mbedtls_psa_rsa_load_representation-memory_leak.txt diff --git a/ChangeLog.d/mbedtls_psa_rsa_load_representation-memory_leak.txt b/ChangeLog.d/mbedtls_psa_rsa_load_representation-memory_leak.txt new file mode 100644 index 0000000000..dba25af611 --- /dev/null +++ b/ChangeLog.d/mbedtls_psa_rsa_load_representation-memory_leak.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix a memory leak that could occur when failing to process an RSA + key through some PSA functions due to low memory conditions. From c3ed44cc3bae953b94d4cbadd1d5fd91375cb820 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 22 Aug 2024 09:00:57 +0100 Subject: [PATCH 43/44] Tiny fix in ChangeLog Signed-off-by: Elena Uziunaite --- ChangeLog.d/fix_reporting_of_key_usage_issues.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt index 75fbb6cc15..08a0ab2708 100644 --- a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt +++ b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt @@ -1,7 +1,7 @@ Security * With TLS 1.3, when a server enables optional authentication of the client, if the client-provided certificate does not have appropriate values - in if keyUsage or extKeyUsage extensions, then the return value of + in keyUsage or extKeyUsage extensions, then the return value of mbedtls_ssl_get_verify_result() would incorrectly have the MBEDTLS_X509_BADCERT_KEY_USAGE and MBEDTLS_X509_BADCERT_KEY_USAGE bits clear. As a result, an attacker that had a certificate valid for uses other From d2cb074a3a13459767470b5b197e904c454332b6 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 22 Aug 2024 09:23:48 +0100 Subject: [PATCH 44/44] Tiny fix in ChangeLog pt 2 Signed-off-by: Elena Uziunaite --- ChangeLog.d/fix_reporting_of_key_usage_issues.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt index 08a0ab2708..b81fb426a7 100644 --- a/ChangeLog.d/fix_reporting_of_key_usage_issues.txt +++ b/ChangeLog.d/fix_reporting_of_key_usage_issues.txt @@ -3,7 +3,7 @@ Security client, if the client-provided certificate does not have appropriate values in keyUsage or extKeyUsage extensions, then the return value of mbedtls_ssl_get_verify_result() would incorrectly have the - MBEDTLS_X509_BADCERT_KEY_USAGE and MBEDTLS_X509_BADCERT_KEY_USAGE bits + MBEDTLS_X509_BADCERT_KEY_USAGE and MBEDTLS_X509_BADCERT_EXT_KEY_USAGE bits clear. As a result, an attacker that had a certificate valid for uses other than TLS client authentication could be able to use it for TLS client authentication anyway. Only TLS 1.3 servers were affected, and only with