From 3235165e070803b0459a37fa7469dd3e918fc77a Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 8 May 2024 17:36:09 +0000 Subject: [PATCH 01/30] Change mpi_core_check_sub to be constant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.c | 5 ++-- tests/suites/test_suite_bignum_core.function | 30 +++++++++++++++++++ tests/suites/test_suite_bignum_core.misc.data | 6 ++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1a3e0b9b6f..ee3d704a13 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -449,9 +449,10 @@ mbedtls_mpi_uint mbedtls_mpi_core_sub(mbedtls_mpi_uint *X, mbedtls_mpi_uint c = 0; for (size_t i = 0; i < limbs; i++) { - mbedtls_mpi_uint z = (A[i] < c); + mbedtls_mpi_uint z = mbedtls_ct_mpi_uint_if(mbedtls_ct_uint_lt(A[i], c), + 1, 0); mbedtls_mpi_uint t = A[i] - c; - c = (t < B[i]) + z; + c = mbedtls_ct_mpi_uint_if(mbedtls_ct_uint_lt(t, B[i]), 1, 0) + z; X[i] = t - B[i]; } diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index db84d6238f..61eeaf5451 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1317,3 +1317,33 @@ exit: mbedtls_free(X); } /* END_CASE */ + +/* BEGIN_CASE */ +void mpi_core_check_sub_ct(char *input_A, char *input_B, int exp_ret) +{ + mbedtls_mpi_uint *A = NULL; + mbedtls_mpi_uint *B = NULL; + mbedtls_mpi_uint *X = NULL; + size_t A_limbs, B_limbs; + int ret; + + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&A, &A_limbs, input_A)); + TEST_EQUAL(0, mbedtls_test_read_mpi_core(&B, &B_limbs, input_B)); + + TEST_EQUAL(A_limbs, B_limbs); + + size_t limbs = A_limbs; + TEST_CALLOC(X, limbs); + + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B, B_limbs * sizeof(mbedtls_mpi_uint)); + + ret = mbedtls_mpi_core_sub(X, A, B, limbs); + TEST_EQUAL(ret, exp_ret); + +exit: + mbedtls_free(A); + mbedtls_free(B); + mbedtls_free(X); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ba86029977..ccf375052e 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -523,3 +523,9 @@ mpi_core_clz:64:0 CLZ: 100000 0: skip overly long input mpi_core_clz:100000:0 + +Constant time Subtraction +mpi_core_check_sub_ct:"1234567890abcdef0":"10000000000000000":0 + +Constant time Subtraction #2 +mpi_core_check_sub_ct:"10000000000000000":"1234567890abcdef0":1 From e27738308cfb45d34e9a91335e65b12d7d6dde2e Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 21 May 2024 16:05:52 +0000 Subject: [PATCH 02/30] Merge mbedtls_mpi_core_sub() constant time testing and functional testing Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_bignum_core.function | 53 ++++++++----------- tests/suites/test_suite_bignum_core.misc.data | 6 --- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 61eeaf5451..1bb1ab06a4 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -660,31 +660,54 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(b, B.p, B.n * sizeof(mbedtls_mpi_uint)); memcpy(x, X.p, X.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(a, bytes); + TEST_CF_SECRET(b, bytes); + /* 1a) r = a - b => we should get the correct carry */ TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, b, limbs)); + TEST_CF_PUBLIC(a, bytes); + TEST_CF_PUBLIC(b, bytes); + TEST_CF_PUBLIC(r, bytes); + /* 1b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 2 and 3 test "r may be aliased to a or b" */ /* 2a) r = a; r -= b => we should get the correct carry (use r to avoid clobbering a) */ memcpy(r, a, bytes); + + TEST_CF_SECRET(r, bytes); + TEST_CF_SECRET(b, bytes); + TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, b, limbs)); + TEST_CF_PUBLIC(r, bytes); + TEST_CF_PUBLIC(b, bytes); + /* 2b) r -= b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 3a) r = b; r = a - r => we should get the correct carry (use r to avoid clobbering b) */ memcpy(r, b, bytes); + + TEST_CF_SECRET(r, bytes); + TEST_CF_SECRET(a, bytes); + TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, r, limbs)); + TEST_CF_PUBLIC(r, bytes); + TEST_CF_PUBLIC(a, bytes); + /* 3b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); /* 4 tests "r may be aliased to [...] both" */ if (A.n == B.n && memcmp(A.p, B.p, bytes) == 0) { memcpy(r, b, bytes); + TEST_CF_SECRET(r, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, r, limbs)); + TEST_CF_PUBLIC(r, bytes); TEST_MEMORY_COMPARE(r, bytes, x, bytes); } @@ -1317,33 +1340,3 @@ exit: mbedtls_free(X); } /* END_CASE */ - -/* BEGIN_CASE */ -void mpi_core_check_sub_ct(char *input_A, char *input_B, int exp_ret) -{ - mbedtls_mpi_uint *A = NULL; - mbedtls_mpi_uint *B = NULL; - mbedtls_mpi_uint *X = NULL; - size_t A_limbs, B_limbs; - int ret; - - TEST_EQUAL(0, mbedtls_test_read_mpi_core(&A, &A_limbs, input_A)); - TEST_EQUAL(0, mbedtls_test_read_mpi_core(&B, &B_limbs, input_B)); - - TEST_EQUAL(A_limbs, B_limbs); - - size_t limbs = A_limbs; - TEST_CALLOC(X, limbs); - - TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); - TEST_CF_SECRET(B, B_limbs * sizeof(mbedtls_mpi_uint)); - - ret = mbedtls_mpi_core_sub(X, A, B, limbs); - TEST_EQUAL(ret, exp_ret); - -exit: - mbedtls_free(A); - mbedtls_free(B); - mbedtls_free(X); -} -/* END_CASE */ diff --git a/tests/suites/test_suite_bignum_core.misc.data b/tests/suites/test_suite_bignum_core.misc.data index ccf375052e..ba86029977 100644 --- a/tests/suites/test_suite_bignum_core.misc.data +++ b/tests/suites/test_suite_bignum_core.misc.data @@ -523,9 +523,3 @@ mpi_core_clz:64:0 CLZ: 100000 0: skip overly long input mpi_core_clz:100000:0 - -Constant time Subtraction -mpi_core_check_sub_ct:"1234567890abcdef0":"10000000000000000":0 - -Constant time Subtraction #2 -mpi_core_check_sub_ct:"10000000000000000":"1234567890abcdef0":1 From a9fe03ea4ed1314d78e0d4833a585cae498931df Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 21 May 2024 16:17:06 +0000 Subject: [PATCH 03/30] Add comment to mbedtls_mpi_core_sub() to indicate it is costant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/bignum_core.h b/library/bignum_core.h index 92c8d47db5..bebd81032b 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -376,6 +376,9 @@ mbedtls_mpi_uint mbedtls_mpi_core_add_if(mbedtls_mpi_uint *X, * \p X may be aliased to \p A or \p B, or even both, but may not overlap * either otherwise. * + * This function operates in constant time with respect to the values + * of \p X and \p A and \p B. + * * \param[out] X The result of the subtraction. * \param[in] A Little-endian presentation of left operand. * \param[in] B Little-endian presentation of right operand. From 77bd47982527ac98b5c1f37dc4dcdddbc06208b1 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 21 May 2024 18:17:22 +0000 Subject: [PATCH 04/30] Change mbedtls_mpi_core_mla() to be constant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.c | 2 +- tests/suites/test_suite_bignum_core.function | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/library/bignum_core.c b/library/bignum_core.c index 1a3e0b9b6f..b43be3032c 100644 --- a/library/bignum_core.c +++ b/library/bignum_core.c @@ -489,7 +489,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_mla(mbedtls_mpi_uint *d, size_t d_len, while (excess_len--) { *d += c; - c = (*d < c); + c = mbedtls_ct_mpi_uint_if(mbedtls_ct_uint_lt(*d, c), 1, 0); d++; } diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index db84d6238f..1b89268f56 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -770,16 +770,36 @@ void mpi_core_mla(char *input_A, char *input_B, char *input_S, memcpy(a, A.p, A.n * sizeof(mbedtls_mpi_uint)); memcpy(x, X->p, X->n * sizeof(mbedtls_mpi_uint)); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(a, bytes); + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(S.p, sizeof(mbedtls_mpi_uint)); +#endif + /* 1a) A += B * s => we should get the correct carry */ TEST_EQUAL(mbedtls_mpi_core_mla(a, limbs, B.p, B.n, *S.p), *cy->p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(a, bytes); + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(S.p, sizeof(mbedtls_mpi_uint)); +#endif + /* 1b) A += B * s => we should get the correct result */ TEST_MEMORY_COMPARE(a, bytes, x, bytes); if (A.n == B.n && memcmp(A.p, B.p, bytes) == 0) { /* Check when A and B are aliased */ memcpy(a, A.p, A.n * sizeof(mbedtls_mpi_uint)); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(a, bytes); + TEST_CF_SECRET(S.p, sizeof(mbedtls_mpi_uint)); +#endif TEST_EQUAL(mbedtls_mpi_core_mla(a, limbs, a, limbs, *S.p), *cy->p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(a, bytes); + TEST_CF_PUBLIC(S.p, sizeof(mbedtls_mpi_uint)); +#endif TEST_MEMORY_COMPARE(a, bytes, x, bytes); } From 11a81cd7dd9f0863128137f036308287e679b713 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Thu, 23 May 2024 08:01:58 +0000 Subject: [PATCH 05/30] Add comment to mbedtls_mpi_core_mla() to indicate it is costant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/bignum_core.h b/library/bignum_core.h index 92c8d47db5..27c0f9df4b 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -397,6 +397,9 @@ mbedtls_mpi_uint mbedtls_mpi_core_sub(mbedtls_mpi_uint *X, * * This function operates modulo `2^(biL*X_limbs)`. * + * This function operates in constant time with respect to the values + * of \p X and \p A and \p b. + * * \param[in,out] X The pointer to the (little-endian) array * representing the bignum to accumulate onto. * \param X_limbs The number of limbs of \p X. This must be From 473dea26a6f2660edb528161e99a0d06103fd4af Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 28 May 2024 11:15:21 +0000 Subject: [PATCH 06/30] Remove unnecessary testing and documentation Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 2 +- tests/suites/test_suite_bignum_core.function | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/library/bignum_core.h b/library/bignum_core.h index bebd81032b..b32d60736c 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -377,7 +377,7 @@ mbedtls_mpi_uint mbedtls_mpi_core_add_if(mbedtls_mpi_uint *X, * either otherwise. * * This function operates in constant time with respect to the values - * of \p X and \p A and \p B. + * of \p A and \p B. * * \param[out] X The result of the subtraction. * \param[in] A Little-endian presentation of left operand. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 1bb1ab06a4..bd1dfa687c 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -666,8 +666,6 @@ void mpi_core_sub(char *input_A, char *input_B, /* 1a) r = a - b => we should get the correct carry */ TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, b, limbs)); - TEST_CF_PUBLIC(a, bytes); - TEST_CF_PUBLIC(b, bytes); TEST_CF_PUBLIC(r, bytes); /* 1b) r = a - b => we should get the correct result */ @@ -678,12 +676,10 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(r, a, bytes); TEST_CF_SECRET(r, bytes); - TEST_CF_SECRET(b, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, r, b, limbs)); TEST_CF_PUBLIC(r, bytes); - TEST_CF_PUBLIC(b, bytes); /* 2b) r -= b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); @@ -692,12 +688,10 @@ void mpi_core_sub(char *input_A, char *input_B, memcpy(r, b, bytes); TEST_CF_SECRET(r, bytes); - TEST_CF_SECRET(a, bytes); TEST_EQUAL(carry, mbedtls_mpi_core_sub(r, a, r, limbs)); TEST_CF_PUBLIC(r, bytes); - TEST_CF_PUBLIC(a, bytes); /* 3b) r = a - b => we should get the correct result */ TEST_MEMORY_COMPARE(r, bytes, x, bytes); From 122ae06ca9f1c84f8b3b27a9472fe478e66e50cb Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 29 May 2024 17:11:28 +0000 Subject: [PATCH 07/30] Add constant time tests to mbedtls_mpi_core_montmul() Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 4 ++ tests/suites/test_suite_bignum_core.function | 46 +++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/library/bignum_core.h b/library/bignum_core.h index eac91f5343..818ca7a208 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -462,6 +462,10 @@ mbedtls_mpi_uint mbedtls_mpi_core_montmul_init(const mbedtls_mpi_uint *N); * \p A and \p B may alias each other, if \p AN_limbs == \p B_limbs. They may * not alias \p N (since they must be in canonical form, they cannot == \p N). * + * This function operates in constant time with respect + * to the values of \p A, \p B and \p N. + * + * * \param[out] X The destination MPI, as a little-endian array of * length \p AN_limbs. * On successful completion, X contains the result of diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 0869a1e0a6..3eec6a4388 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -919,21 +919,36 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, size_t working_limbs = mbedtls_mpi_core_montmul_working_limbs(limbs_AN); TEST_EQUAL(working_limbs, limbs_AN * 2 + 1); TEST_EQUAL(0, mbedtls_mpi_grow(&T, working_limbs)); - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); +#endif /* Calculate the Montgomery constant (this is unit tested separately) */ mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N.p); TEST_EQUAL(0, mbedtls_mpi_grow(&R, limbs_AN)); /* ensure it's got the right number of limbs */ +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_montmul(R.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(R.p, R.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(N.p, N.n * sizeof(mbedtls_mpi_uint)); +#endif size_t bytes = N.n * sizeof(mbedtls_mpi_uint); TEST_MEMORY_COMPARE(R.p, bytes, X->p, bytes); /* The output (R, above) may be aliased to A - use R to save the value of A */ memcpy(R.p, A.p, bytes); - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_montmul(A.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(A.p, A.n * sizeof(mbedtls_mpi_uint)); +#endif TEST_MEMORY_COMPARE(A.p, bytes, X->p, bytes); memcpy(A.p, R.p, bytes); /* restore A */ @@ -941,27 +956,46 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, /* The output may be aliased to N - use R to save the value of N */ memcpy(R.p, N.p, bytes); - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_montmul(N.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(N.p, N.n * sizeof(mbedtls_mpi_uint)); +#endif TEST_MEMORY_COMPARE(N.p, bytes, X->p, bytes); memcpy(N.p, R.p, bytes); - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(A.p, A.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif if (limbs_AN == limbs_B) { /* Test when A aliased to B (requires A == B on input values) */ if (memcmp(A.p, B.p, bytes) == 0) { /* Test with A aliased to B and output, since this is permitted - * don't bother with yet another test with only A and B aliased */ - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_montmul(B.p, B.p, B.p, B.n, N.p, N.n, mm, T.p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif TEST_MEMORY_COMPARE(B.p, bytes, X->p, bytes); memcpy(B.p, A.p, bytes); /* restore B from equal value A */ } /* The output may be aliased to B - last test, so we don't save B */ - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_montmul(B.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); +#endif TEST_MEMORY_COMPARE(B.p, bytes, X->p, bytes); } From 80ab4f38869d374ecd32d3cda57a75d6d15e0d58 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Mon, 24 Jun 2024 13:31:15 +0000 Subject: [PATCH 08/30] change montmul constant time testing to be clearer Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_bignum_core.function | 29 +++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 3eec6a4388..be947578c0 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -919,6 +919,7 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, size_t working_limbs = mbedtls_mpi_core_montmul_working_limbs(limbs_AN); TEST_EQUAL(working_limbs, limbs_AN * 2 + 1); TEST_EQUAL(0, mbedtls_mpi_grow(&T, working_limbs)); + /* Temporary because MEMSAN doesn't support assembly implementation see #1243 */ #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); #endif @@ -928,14 +929,13 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, TEST_EQUAL(0, mbedtls_mpi_grow(&R, limbs_AN)); /* ensure it's got the right number of limbs */ #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_montmul(R.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(R.p, R.n * sizeof(mbedtls_mpi_uint)); - TEST_CF_PUBLIC(N.p, N.n * sizeof(mbedtls_mpi_uint)); -#endif size_t bytes = N.n * sizeof(mbedtls_mpi_uint); TEST_MEMORY_COMPARE(R.p, bytes, X->p, bytes); @@ -944,11 +944,12 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, memcpy(R.p, A.p, bytes); #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_montmul(A.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(A.p, A.n * sizeof(mbedtls_mpi_uint)); -#endif TEST_MEMORY_COMPARE(A.p, bytes, X->p, bytes); memcpy(A.p, R.p, bytes); /* restore A */ @@ -957,32 +958,33 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, memcpy(R.p, N.p, bytes); #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_montmul(N.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(N.p, N.n * sizeof(mbedtls_mpi_uint)); -#endif TEST_MEMORY_COMPARE(N.p, bytes, X->p, bytes); memcpy(N.p, R.p, bytes); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + if (limbs_AN == limbs_B) { /* Test when A aliased to B (requires A == B on input values) */ if (memcmp(A.p, B.p, bytes) == 0) { /* Test with A aliased to B and output, since this is permitted - * don't bother with yet another test with only A and B aliased */ #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_montmul(B.p, B.p, B.p, B.n, N.p, N.n, mm, T.p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif TEST_MEMORY_COMPARE(B.p, bytes, X->p, bytes); memcpy(B.p, A.p, bytes); /* restore B from equal value A */ @@ -990,12 +992,13 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, /* The output may be aliased to B - last test, so we don't save B */ #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_montmul(B.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif TEST_MEMORY_COMPARE(B.p, bytes, X->p, bytes); } From 7b3024e7913d58e6341f42e717e5dd29d6bd5584 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Thu, 30 May 2024 16:41:47 +0000 Subject: [PATCH 09/30] Change mbedtls_mpi_core_exp_mod to constant time Signed-off-by: Waleed Elmelegy --- library/bignum_core.h | 3 +++ tests/suites/test_suite_bignum_core.function | 21 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/library/bignum_core.h b/library/bignum_core.h index 818ca7a208..51ecca5ff7 100644 --- a/library/bignum_core.h +++ b/library/bignum_core.h @@ -621,6 +621,9 @@ size_t mbedtls_mpi_core_exp_mod_working_limbs(size_t AN_limbs, size_t E_limbs); * \p X may be aliased to \p A, but not to \p RR or \p E, even if \p E_limbs == * \p AN_limbs. * + * This function operates in constant time with respect + * to the values of \p A, \p N and \p E. + * * \param[out] X The destination MPI, as a little endian array of length * \p AN_limbs. * \param[in] A The base MPI, as a little endian array of length \p AN_limbs. diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index be947578c0..cd808030a6 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1302,14 +1302,31 @@ void mpi_core_exp_mod(char *input_N, char *input_A, working_limbs); TEST_CALLOC(T, working_limbs); - +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); +#endif mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(N, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_PUBLIC(E, E_limbs * sizeof(mbedtls_mpi_uint)); +#endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); + TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); +#endif /* Check when output aliased to input */ - mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); +#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); +#endif TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); From 7ac7f82053aa2a89084fe91420a5673177113342 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Tue, 25 Jun 2024 09:45:19 +0000 Subject: [PATCH 10/30] Change mpi_core_exp_mod() constant time testing to be clearer Signed-off-by: Waleed Elmelegy --- tests/suites/test_suite_bignum_core.function | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index cd808030a6..89c2282207 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -1302,18 +1302,16 @@ void mpi_core_exp_mod(char *input_N, char *input_A, working_limbs); TEST_CALLOC(T, working_limbs); + + /* Temporary because MEMSAN doesn't support assembly implementation see #1243 */ #if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); #endif mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); - TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); - TEST_CF_PUBLIC(N, N_limbs * sizeof(mbedtls_mpi_uint)); - TEST_CF_PUBLIC(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#endif TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); @@ -1324,10 +1322,8 @@ void mpi_core_exp_mod(char *input_N, char *input_A, #endif /* Check when output aliased to input */ mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) - TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); -#endif + TEST_CF_PUBLIC(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_EQUAL(0, memcmp(X, A, N_limbs * sizeof(mbedtls_mpi_uint))); exit: From 6bba0a8355db632f7287905ec577483979c47905 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2024 23:32:50 +0200 Subject: [PATCH 11/30] 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 db81d7efb0482a661722d9392ec11c1fabea5479 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:46:25 +0200 Subject: [PATCH 12/30] 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 a9e7ac9811054b14c3e754a2735b7ff70e0cf8ad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 27 Jun 2024 10:59:55 +0200 Subject: [PATCH 13/30] 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 66ea31ccd0d2122015fc20b01fadf8ab27ae0d60 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Fri, 28 Jun 2024 14:54:09 +0100 Subject: [PATCH 14/30] Clean up constant-flow memsan testing Disable asm in memsan constant-flow testing and adjust test_suit_bignum_core.function accordingly Signed-off-by: Elena Uziunaite --- tests/scripts/all.sh | 2 + tests/suites/test_suite_bignum_core.function | 43 ++++++++------------ 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index a1203f7726..3eca325003 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2202,6 +2202,7 @@ component_test_memsan_constant_flow () { scripts/config.py set MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN scripts/config.py unset MBEDTLS_USE_PSA_CRYPTO scripts/config.py unset MBEDTLS_AESNI_C # memsan doesn't grok asm + scripts/config.py unset MBEDTLS_HAVE_ASM CC=clang cmake -D CMAKE_BUILD_TYPE:String=MemSan . make @@ -2220,6 +2221,7 @@ component_test_memsan_constant_flow_psa () { scripts/config.py full scripts/config.py set MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN scripts/config.py unset MBEDTLS_AESNI_C # memsan doesn't grok asm + scripts/config.py unset MBEDTLS_HAVE_ASM CC=clang cmake -D CMAKE_BUILD_TYPE:String=MemSan . make diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function index 89c2282207..6c0bd1e0c4 100644 --- a/tests/suites/test_suite_bignum_core.function +++ b/tests/suites/test_suite_bignum_core.function @@ -787,20 +787,16 @@ void mpi_core_mla(char *input_A, char *input_B, char *input_S, memcpy(a, A.p, A.n * sizeof(mbedtls_mpi_uint)); memcpy(x, X->p, X->n * sizeof(mbedtls_mpi_uint)); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(a, bytes); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(S.p, sizeof(mbedtls_mpi_uint)); -#endif /* 1a) A += B * s => we should get the correct carry */ TEST_EQUAL(mbedtls_mpi_core_mla(a, limbs, B.p, B.n, *S.p), *cy->p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_PUBLIC(a, bytes); TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); TEST_CF_PUBLIC(S.p, sizeof(mbedtls_mpi_uint)); -#endif /* 1b) A += B * s => we should get the correct result */ TEST_MEMORY_COMPARE(a, bytes, x, bytes); @@ -808,15 +804,15 @@ void mpi_core_mla(char *input_A, char *input_B, char *input_S, if (A.n == B.n && memcmp(A.p, B.p, bytes) == 0) { /* Check when A and B are aliased */ memcpy(a, A.p, A.n * sizeof(mbedtls_mpi_uint)); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(a, bytes); TEST_CF_SECRET(S.p, sizeof(mbedtls_mpi_uint)); -#endif + TEST_EQUAL(mbedtls_mpi_core_mla(a, limbs, a, limbs, *S.p), *cy->p); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_PUBLIC(a, bytes); TEST_CF_PUBLIC(S.p, sizeof(mbedtls_mpi_uint)); -#endif + TEST_MEMORY_COMPARE(a, bytes, x, bytes); } @@ -919,20 +915,18 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, size_t working_limbs = mbedtls_mpi_core_montmul_working_limbs(limbs_AN); TEST_EQUAL(working_limbs, limbs_AN * 2 + 1); TEST_EQUAL(0, mbedtls_mpi_grow(&T, working_limbs)); - /* Temporary because MEMSAN doesn't support assembly implementation see #1243 */ -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); -#endif + /* Calculate the Montgomery constant (this is unit tested separately) */ mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N.p); TEST_EQUAL(0, mbedtls_mpi_grow(&R, limbs_AN)); /* ensure it's got the right number of limbs */ -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_montmul(R.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); TEST_CF_PUBLIC(R.p, R.n * sizeof(mbedtls_mpi_uint)); @@ -942,11 +936,11 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, /* The output (R, above) may be aliased to A - use R to save the value of A */ memcpy(R.p, A.p, bytes); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_montmul(A.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); TEST_CF_PUBLIC(A.p, A.n * sizeof(mbedtls_mpi_uint)); @@ -957,11 +951,11 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, /* The output may be aliased to N - use R to save the value of N */ memcpy(R.p, N.p, bytes); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_montmul(N.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); TEST_CF_PUBLIC(N.p, N.n * sizeof(mbedtls_mpi_uint)); @@ -977,11 +971,11 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, if (memcmp(A.p, B.p, bytes) == 0) { /* Test with A aliased to B and output, since this is permitted - * don't bother with yet another test with only A and B aliased */ -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_montmul(B.p, B.p, B.p, B.n, N.p, N.n, mm, T.p); TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); @@ -991,11 +985,11 @@ void mpi_core_montmul(int limbs_AN4, int limbs_B4, } /* The output may be aliased to B - last test, so we don't save B */ -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) + TEST_CF_SECRET(N.p, N.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(A.p, A.n * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(B.p, B.n * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_montmul(B.p, A.p, B.p, B.n, N.p, N.n, mm, T.p); TEST_CF_PUBLIC(B.p, B.n * sizeof(mbedtls_mpi_uint)); @@ -1303,23 +1297,20 @@ void mpi_core_exp_mod(char *input_N, char *input_A, TEST_CALLOC(T, working_limbs); - /* Temporary because MEMSAN doesn't support assembly implementation see #1243 */ -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#endif + mbedtls_mpi_core_exp_mod(Y, A, N, N_limbs, E, E_limbs, R2, T); TEST_CF_PUBLIC(Y, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_EQUAL(0, memcmp(X, Y, N_limbs * sizeof(mbedtls_mpi_uint))); -#if !defined(MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) TEST_CF_SECRET(A, A_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(N, N_limbs * sizeof(mbedtls_mpi_uint)); TEST_CF_SECRET(E, E_limbs * sizeof(mbedtls_mpi_uint)); -#endif + /* Check when output aliased to input */ mbedtls_mpi_core_exp_mod(A, A, N, N_limbs, E, E_limbs, R2, T); From 868d2524b8f91f9373f2f96ddd87dab5126555cf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 1 Jul 2024 21:14:45 +0200 Subject: [PATCH 15/30] 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 71b04a737acb708dffc535fe7eb5a3c5ae6a2ca8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 2 Jul 2024 15:47:22 +0200 Subject: [PATCH 16/30] 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 2547cd353565d906175a0b2a34071a81ebf80729 Mon Sep 17 00:00:00 2001 From: Sam Berry Date: Fri, 16 Aug 2024 16:38:34 +0100 Subject: [PATCH 17/30] Free allocated memory where methods were returning without freeing Signed-off-by: Sam Berry --- tf-psa-crypto/core/psa_crypto_rsa.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tf-psa-crypto/core/psa_crypto_rsa.c b/tf-psa-crypto/core/psa_crypto_rsa.c index f8e36d8b12..5fe26ec87c 100644 --- a/tf-psa-crypto/core/psa_crypto_rsa.c +++ b/tf-psa-crypto/core/psa_crypto_rsa.c @@ -197,16 +197,13 @@ 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 +261,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 +328,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 8e70c2bcd997c99be7f4c99f6abfc33b42633fbf 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 18/30] 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 5a4c8f0ba022af47bac736af5b3f5b9ef0e6dfc4 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 19/30] 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 4956e325381b7d87295c25466c12cf249824a87b 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 20/30] 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 92a391e0fe442f0699076622c6c1972d558a5042 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 21/30] 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 fef5c460d9..673c5b7ee2 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 81b125693d..f7a9b1774e 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 19d6d3421c3f591a6062250481f545816b21106d 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 22/30] 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 aeda1fd0a839ea5ee571f41639c4feb25803d3b9 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 23/30] 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 6a04b168b268b548de5595c97d9f0c59a0abe91b Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 15 Aug 2024 15:24:09 +0100 Subject: [PATCH 24/30] 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 f48bfb00bd567eca7f106d38cfe168de281fee37 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Fri, 16 Aug 2024 17:18:28 +0100 Subject: [PATCH 25/30] 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 16f0e18e41c16d64c3113b3934f7d0be2662346b Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Mon, 19 Aug 2024 12:10:22 +0100 Subject: [PATCH 26/30] 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 f72a51059068a97c9a95e8b6cab522fdd549f8d3 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Tue, 20 Aug 2024 12:11:57 +0100 Subject: [PATCH 27/30] 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 98dd0c1f1c2768655f8e33a57a7b03baf7e58538 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 21 Aug 2024 22:03:16 +0200 Subject: [PATCH 28/30] 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 e0c6f804030762aa5ee427b1bae99dc91164110e Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 22 Aug 2024 09:00:57 +0100 Subject: [PATCH 29/30] 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 da27eba669cd13260771c85559e198b6d254e323 Mon Sep 17 00:00:00 2001 From: Elena Uziunaite Date: Thu, 22 Aug 2024 09:23:48 +0100 Subject: [PATCH 30/30] 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