From 1c7629c1c03fcc74781bc448d5b4d5d6ffd7219c Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 15:19:42 +0100 Subject: [PATCH 01/22] Add tests for Issue #8687 Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.data | 6 ++++++ tests/suites/test_suite_x509write.function | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index 0f190286b..f1d4e34a5 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -265,3 +265,9 @@ mbedtls_x509_string_to_names:"C=NL, 2.5.4.10.234.532=#0C084F6666737061726B, OU=P Check max serial length x509_set_serial_check: + +Check max extension length (Max-1) +x509_set_extension_length_check:0xFFFFFFFE + +Check max extension length (Max) +x509_set_extension_length_check:0xFFFFFFFF \ No newline at end of file diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index a7ed26295..7ec655727 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -750,3 +750,24 @@ exit: USE_PSA_DONE(); } /* END_CASE */ + +/* BEGIN_CASE */ +void x509_set_extension_length_check(int val_len) +{ + int ret = 0; + + mbedtls_x509write_csr ctx; + mbedtls_x509write_csr_init(&ctx); + + unsigned char buf[EXT_KEY_USAGE_TMP_BUF_MAX_LENGTH] = { 0 }; + unsigned char *p = buf + sizeof(buf); + + ret = mbedtls_x509_set_extension(&(ctx.MBEDTLS_PRIVATE(extensions)), + MBEDTLS_OID_EXTENDED_KEY_USAGE, + MBEDTLS_OID_SIZE(MBEDTLS_OID_EXTENDED_KEY_USAGE), + 0, + p, + val_len); + TEST_ASSERT(ret == MBEDTLS_ERR_X509_BAD_INPUT_DATA || ret == MBEDTLS_ERR_X509_ALLOC_FAILED); +} +/* END_CASE */ From 63b5e216f8fcaff0f6b87bb05ffd5631158ac3c4 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 15:20:03 +0100 Subject: [PATCH 02/22] Fix Issue #8687 Signed-off-by: Jonathan Winzig --- library/x509_create.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/x509_create.c b/library/x509_create.c index 5e732d67f..2c17cb10c 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -380,6 +380,10 @@ int mbedtls_x509_set_extension(mbedtls_asn1_named_data **head, const char *oid, { mbedtls_asn1_named_data *cur; + if (0xFFFFFFFF == (uint32_t) val_len) { + return MBEDTLS_ERR_X509_BAD_INPUT_DATA; + } + if ((cur = mbedtls_asn1_store_named_data(head, oid, oid_len, NULL, val_len + 1)) == NULL) { return MBEDTLS_ERR_X509_ALLOC_FAILED; From a0c9448beaa6df9d4305c6d85fc659f10eb4ee80 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 16:41:10 +0100 Subject: [PATCH 03/22] Update fix to be more platform-independent Co-authored-by: David Horstmann Signed-off-by: Jonathan Winzig --- library/x509_create.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/x509_create.c b/library/x509_create.c index 2c17cb10c..751ee08ed 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -380,7 +380,7 @@ int mbedtls_x509_set_extension(mbedtls_asn1_named_data **head, const char *oid, { mbedtls_asn1_named_data *cur; - if (0xFFFFFFFF == (uint32_t) val_len) { + if (val_len > (SIZE_MAX - 1)) { return MBEDTLS_ERR_X509_BAD_INPUT_DATA; } From 93f5240ae594a5f88907a57264a1a73ee1886189 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 16:47:12 +0100 Subject: [PATCH 04/22] Add missing newline at the end of test_suite_x509write.data Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index f1d4e34a5..6aa0dadb6 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -270,4 +270,4 @@ Check max extension length (Max-1) x509_set_extension_length_check:0xFFFFFFFE Check max extension length (Max) -x509_set_extension_length_check:0xFFFFFFFF \ No newline at end of file +x509_set_extension_length_check:0xFFFFFFFF From 144bfde1cd10ab6e1647628fe10ead0057395648 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 17:39:42 +0100 Subject: [PATCH 05/22] Update test-data to use SIZE_MAX Co-authored-by: David Horstmann Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index 6aa0dadb6..e41de849b 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -270,4 +270,4 @@ Check max extension length (Max-1) x509_set_extension_length_check:0xFFFFFFFE Check max extension length (Max) -x509_set_extension_length_check:0xFFFFFFFF +x509_set_extension_length_check:SIZE_MAX From acd35a55c849ea0de8ffac164801cc75f286a7bd Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 17:47:10 +0100 Subject: [PATCH 06/22] Remove unneeded testcase Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.data | 7 ++----- tests/suites/test_suite_x509write.function | 6 +++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index e41de849b..f63ae2bea 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -266,8 +266,5 @@ mbedtls_x509_string_to_names:"C=NL, 2.5.4.10.234.532=#0C084F6666737061726B, OU=P Check max serial length x509_set_serial_check: -Check max extension length (Max-1) -x509_set_extension_length_check:0xFFFFFFFE - -Check max extension length (Max) -x509_set_extension_length_check:SIZE_MAX +Check max extension length +x509_set_extension_length_check: diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 7ec655727..11b5f2a02 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -752,7 +752,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void x509_set_extension_length_check(int val_len) +void x509_set_extension_length_check() { int ret = 0; @@ -767,7 +767,7 @@ void x509_set_extension_length_check(int val_len) MBEDTLS_OID_SIZE(MBEDTLS_OID_EXTENDED_KEY_USAGE), 0, p, - val_len); - TEST_ASSERT(ret == MBEDTLS_ERR_X509_BAD_INPUT_DATA || ret == MBEDTLS_ERR_X509_ALLOC_FAILED); + SIZE_MAX); + TEST_ASSERT(MBEDTLS_ERR_X509_BAD_INPUT_DATA == ret); } /* END_CASE */ From af553bf719be37876abe20fbb057fb44b4a6a6e5 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Tue, 9 Jan 2024 18:31:11 +0100 Subject: [PATCH 07/22] Add required dependency to the testcase Co-authored-by: Paul Elliott <62069445+paul-elliott-arm@users.noreply.github.com> Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 11b5f2a02..c557ee00e 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -751,7 +751,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_CSR_WRITE_C */ void x509_set_extension_length_check() { int ret = 0; From 968a92865966b35334655e65547da5f288722769 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 Jan 2024 11:16:56 +0000 Subject: [PATCH 08/22] Add Changelog for #8687 Signed-off-by: Paul Elliott --- ChangeLog.d/fix_int_overflow_x509_extension | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 ChangeLog.d/fix_int_overflow_x509_extension diff --git a/ChangeLog.d/fix_int_overflow_x509_extension b/ChangeLog.d/fix_int_overflow_x509_extension new file mode 100644 index 000000000..2a679284f --- /dev/null +++ b/ChangeLog.d/fix_int_overflow_x509_extension @@ -0,0 +1,8 @@ +Security + * Fix a failure to validate input when writing x509 extensions lengths which + could result in an integer overflow, causing a zero-length buffer to be + allocated to hold the extension. The extension would then be copied into + the buffer, causing a heap buffer overflow. + + + From d6b096532c936390d9a085dedb6444cee069a3ba Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:33:54 +0000 Subject: [PATCH 09/22] Make RSA unblinding constant flow Signed-off-by: Janos Follath --- library/rsa.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index db0b0f74f..32a26500e 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -28,6 +28,7 @@ #if defined(MBEDTLS_RSA_C) #include "mbedtls/rsa.h" +#include "bignum_core.h" #include "rsa_alt_helpers.h" #include "mbedtls/oid.h" #include "mbedtls/platform_util.h" @@ -969,6 +970,40 @@ cleanup: return ret; } +/* + * Unblind + * T = T * Vf mod N + */ +int rsa_unblind(mbedtls_mpi* T, mbedtls_mpi* Vf, mbedtls_mpi* N) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N->p); + const size_t nlimbs = N->n; + const size_t tlimbs = mbedtls_mpi_core_montmul_working_limbs(nlimbs); + mbedtls_mpi RR, M_T; + + mbedtls_mpi_init(&RR); + mbedtls_mpi_init(&M_T); + + MBEDTLS_MPI_CHK(mbedtls_mpi_core_get_mont_r2_unsafe(&RR, N)); + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&M_T, tlimbs)); + + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(T, nlimbs)); + MBEDTLS_MPI_CHK(mbedtls_mpi_grow(Vf, nlimbs)); + + // T = T * R mod N + mbedtls_mpi_core_to_mont_rep(T->p, T->p, N->p, nlimbs, mm, RR.p, M_T.p); + // T = T * Vf mod N + mbedtls_mpi_core_montmul(T->p, T->p, Vf->p, nlimbs, N->p, nlimbs, mm, M_T.p); + +cleanup: + + mbedtls_mpi_free(&RR); + mbedtls_mpi_free(&M_T); + + return ret; +} + /* * Exponent blinding supposed to prevent side-channel attacks using multiple * traces of measurements to recover the RSA key. The more collisions are there, @@ -1160,8 +1195,7 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, * Unblind * T = T * Vf mod N */ - MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&T, &T, &ctx->Vf)); - MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&T, &T, &ctx->N)); + MBEDTLS_MPI_CHK(rsa_unblind(&T, &ctx->Vf, &ctx->N)); /* Verify the result to prevent glitching attacks. */ MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&C, &T, &ctx->E, From 6bcbc925bfe6f56c2d9871e34126cde37181ee14 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:46:43 +0000 Subject: [PATCH 10/22] Extend blinding to RSA result check Signed-off-by: Janos Follath --- library/rsa.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 32a26500e..5b6bf404a 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1113,8 +1113,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, goto cleanup; } - MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&I, &T)); - /* * Blinding * T = T * Vi mod N @@ -1123,6 +1121,8 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&T, &T, &ctx->Vi)); MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&T, &T, &ctx->N)); + MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&I, &T)); + /* * Exponent blinding */ @@ -1191,12 +1191,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&T, &TQ, &TP)); #endif /* MBEDTLS_RSA_NO_CRT */ - /* - * Unblind - * T = T * Vf mod N - */ - MBEDTLS_MPI_CHK(rsa_unblind(&T, &ctx->Vf, &ctx->N)); - /* Verify the result to prevent glitching attacks. */ MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&C, &T, &ctx->E, &ctx->N, &ctx->RN)); @@ -1205,6 +1199,12 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, goto cleanup; } + /* + * Unblind + * T = T * Vf mod N + */ + MBEDTLS_MPI_CHK(rsa_unblind(&T, &ctx->Vf, &ctx->N)); + olen = ctx->len; MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary(&T, output, olen)); From a865fc951ead31a8f85bbee5d7d11bfa1a28de27 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:57:27 +0000 Subject: [PATCH 11/22] Add Changelog for the Marvin attack fix Signed-off-by: Janos Follath --- ChangeLog.d/fix-Marvin-attack.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/fix-Marvin-attack.txt diff --git a/ChangeLog.d/fix-Marvin-attack.txt b/ChangeLog.d/fix-Marvin-attack.txt new file mode 100644 index 000000000..f729304ee --- /dev/null +++ b/ChangeLog.d/fix-Marvin-attack.txt @@ -0,0 +1,6 @@ +Security + * Fix a timing side channel in RSA private operations. This side channel + could be sufficient for a local attacker to recover the plaintext. It + requires the attecker to send a large number of messages for decryption. + For details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. + Reported by Hubert Kario, Red Hat. From 100dcddfca3c10179bc55e3c3dd82b4a468c6809 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 12:48:52 +0000 Subject: [PATCH 12/22] Make local function static Signed-off-by: Janos Follath --- library/rsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/rsa.c b/library/rsa.c index 5b6bf404a..2dc6dae8d 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -974,7 +974,7 @@ cleanup: * Unblind * T = T * Vf mod N */ -int rsa_unblind(mbedtls_mpi* T, mbedtls_mpi* Vf, mbedtls_mpi* N) +static int rsa_unblind(mbedtls_mpi* T, mbedtls_mpi* Vf, mbedtls_mpi* N) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N->p); From a62a554071a0599bb7522d08c4c605588715e508 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 14:20:08 +0000 Subject: [PATCH 13/22] Fix style Signed-off-by: Janos Follath --- library/rsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/rsa.c b/library/rsa.c index 2dc6dae8d..97e7228da 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -974,7 +974,7 @@ cleanup: * Unblind * T = T * Vf mod N */ -static int rsa_unblind(mbedtls_mpi* T, mbedtls_mpi* Vf, mbedtls_mpi* N) +static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, mbedtls_mpi *N) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N->p); From e6750b2a0bf750d35172bdef12c2dcfc28213207 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:22:59 +0000 Subject: [PATCH 14/22] RSA: document Montgomery trick in unblind Signed-off-by: Janos Follath --- library/rsa.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 97e7228da..f57909b71 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -991,9 +991,14 @@ static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, mbedtls_mpi *N) MBEDTLS_MPI_CHK(mbedtls_mpi_grow(T, nlimbs)); MBEDTLS_MPI_CHK(mbedtls_mpi_grow(Vf, nlimbs)); - // T = T * R mod N + /* T = T * Vf mod N + * Reminder: montmul(A, B, N) = A * B * R^-1 mod N + * Usually both operands are multiplied by R mod N beforehand (by calling + * `to_mont_rep()` on them), yielding a result that's also * R mod N (aka + * "in the Montgomery domain"). Here we only multiply one operand by R mod + * N, so the result is directly what we want - no need to call + * `from_mont_rep()` on it. */ mbedtls_mpi_core_to_mont_rep(T->p, T->p, N->p, nlimbs, mm, RR.p, M_T.p); - // T = T * Vf mod N mbedtls_mpi_core_montmul(T->p, T->p, Vf->p, nlimbs, N->p, nlimbs, mm, M_T.p); cleanup: From 47ee7708123347a925aac44709e53a13d1c486e8 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:33:00 +0000 Subject: [PATCH 15/22] RSA: remove unneeded temporaries Signed-off-by: Janos Follath --- library/rsa.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index f57909b71..111af680f 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1056,18 +1056,9 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, /* Temporaries holding the blinded exponents for * the mod p resp. mod q computation (if used). */ mbedtls_mpi DP_blind, DQ_blind; - - /* Pointers to actual exponents to be used - either the unblinded - * or the blinded ones, depending on the presence of a PRNG. */ - mbedtls_mpi *DP = &ctx->DP; - mbedtls_mpi *DQ = &ctx->DQ; #else /* Temporary holding the blinded exponent (if used). */ mbedtls_mpi D_blind; - - /* Pointer to actual exponent to be used - either the unblinded - * or the blinded one, depending on the presence of a PRNG. */ - mbedtls_mpi *D = &ctx->D; #endif /* MBEDTLS_RSA_NO_CRT */ /* Temporaries holding the initial input and the double @@ -1143,8 +1134,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&D_blind, &P1, &Q1)); MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&D_blind, &D_blind, &R)); MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&D_blind, &D_blind, &ctx->D)); - - D = &D_blind; #else /* * DP_blind = ( P - 1 ) * R + DP @@ -1155,8 +1144,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&DP_blind, &DP_blind, &ctx->DP)); - DP = &DP_blind; - /* * DQ_blind = ( Q - 1 ) * R + DQ */ @@ -1165,12 +1152,10 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&DQ_blind, &Q1, &R)); MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&DQ_blind, &DQ_blind, &ctx->DQ)); - - DQ = &DQ_blind; #endif /* MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_RSA_NO_CRT) - MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&T, &T, D, &ctx->N, &ctx->RN)); + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&T, &T, &D_blind, &ctx->N, &ctx->RN)); #else /* * Faster decryption using the CRT @@ -1179,8 +1164,8 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, * TQ = input ^ dQ mod Q */ - MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&TP, &T, DP, &ctx->P, &ctx->RP)); - MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&TQ, &T, DQ, &ctx->Q, &ctx->RQ)); + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&TP, &T, &DP_blind, &ctx->P, &ctx->RP)); + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&TQ, &T, &DQ_blind, &ctx->Q, &ctx->RQ)); /* * T = (TP - TQ) * (Q^-1 mod P) mod P From b4b8f3df3b88fec865d4c2698b94b7f3c08229c1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:44:36 +0000 Subject: [PATCH 16/22] RSA: improve readability Signed-off-by: Janos Follath --- library/rsa.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 111af680f..0ca0bfead 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -974,7 +974,7 @@ cleanup: * Unblind * T = T * Vf mod N */ -static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, mbedtls_mpi *N) +static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, const mbedtls_mpi *N) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const mbedtls_mpi_uint mm = mbedtls_mpi_core_montmul_init(N->p); @@ -1063,7 +1063,7 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, /* Temporaries holding the initial input and the double * checked result; should be the same in the end. */ - mbedtls_mpi I, C; + mbedtls_mpi input_blinded, check_result_blinded; if (f_rng == NULL) { return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; @@ -1098,8 +1098,8 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, mbedtls_mpi_init(&TP); mbedtls_mpi_init(&TQ); #endif - mbedtls_mpi_init(&I); - mbedtls_mpi_init(&C); + mbedtls_mpi_init(&input_blinded); + mbedtls_mpi_init(&check_result_blinded); /* End of MPI initialization */ @@ -1117,7 +1117,7 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&T, &T, &ctx->Vi)); MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&T, &T, &ctx->N)); - MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&I, &T)); + MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&input_blinded, &T)); /* * Exponent blinding @@ -1182,9 +1182,9 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, #endif /* MBEDTLS_RSA_NO_CRT */ /* Verify the result to prevent glitching attacks. */ - MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&C, &T, &ctx->E, + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&check_result_blinded, &T, &ctx->E, &ctx->N, &ctx->RN)); - if (mbedtls_mpi_cmp_mpi(&C, &I) != 0) { + if (mbedtls_mpi_cmp_mpi(&check_result_blinded, &input_blinded) != 0) { ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; goto cleanup; } @@ -1222,8 +1222,8 @@ cleanup: mbedtls_mpi_free(&TP); mbedtls_mpi_free(&TQ); #endif - mbedtls_mpi_free(&C); - mbedtls_mpi_free(&I); + mbedtls_mpi_free(&check_result_blinded); + mbedtls_mpi_free(&input_blinded); if (ret != 0 && ret >= -0x007f) { return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_RSA_PRIVATE_FAILED, ret); From 16ab76bbe774806079e2d5cab0c4209a4f7b0602 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:47:21 +0000 Subject: [PATCH 17/22] Fix typo Signed-off-by: Janos Follath --- ChangeLog.d/fix-Marvin-attack.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix-Marvin-attack.txt b/ChangeLog.d/fix-Marvin-attack.txt index f729304ee..017f7b1f8 100644 --- a/ChangeLog.d/fix-Marvin-attack.txt +++ b/ChangeLog.d/fix-Marvin-attack.txt @@ -1,6 +1,6 @@ Security * Fix a timing side channel in RSA private operations. This side channel could be sufficient for a local attacker to recover the plaintext. It - requires the attecker to send a large number of messages for decryption. + requires the attacker to send a large number of messages for decryption. For details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. Reported by Hubert Kario, Red Hat. From 393df9c99512337b403bbe80a3a3cee30f277fc6 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 29 Dec 2023 11:14:58 +0000 Subject: [PATCH 18/22] Add warning for PKCS 1.5 decryption Any timing variance dependant on the output of this function enables a Bleichenbacher attack. It is extremely difficult to use safely. In the Marvin attack paper (https://people.redhat.com/~hkario/marvin/marvin-attack-paper.pdf) the author suggests that implementations of PKCS 1.5 decryption that don't include a countermeasure should be considered inherently dangerous. They suggest that all libraries implement the same countermeasure, as implementing different countermeasures across libraries enables the Bleichenbacher attack as well. This is extremely fragile and therefore we don't implement it. The use of PKCS 1.5 in Mbed TLS implements the countermeasures recommended in the TLS standard (7.4.7.1 of RFC 5246) and is not vulnerable. Add a warning to PKCS 1.5 decryption to warn users about this. Signed-off-by: Janos Follath --- include/mbedtls/rsa.h | 9 +++++++++ include/psa/crypto_values.h | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index df665240d..be831f19d 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -684,6 +684,10 @@ int mbedtls_rsa_rsaes_oaep_encrypt(mbedtls_rsa_context *ctx, * It is the generic wrapper for performing a PKCS#1 decryption * operation. * + * \warning When \p ctx->padding is set to #MBEDTLS_RSA_PKCS_V15, + * mbedtls_rsa_rsaes_pkcs1_v15_decrypt() is called, which is an + * inherently dangerous function (CWE-242). + * * \note The output buffer length \c output_max_len should be * as large as the size \p ctx->len of \p ctx->N (for example, * 128 Bytes if RSA-1024 is used) to be able to hold an @@ -720,6 +724,11 @@ int mbedtls_rsa_pkcs1_decrypt(mbedtls_rsa_context *ctx, * \brief This function performs a PKCS#1 v1.5 decryption * operation (RSAES-PKCS1-v1_5-DECRYPT). * + * \warning This is an inherently dangerous function (CWE-242). Unless + * it is used in a side channel free and safe way (eg. + * implementing the TLS protocol as per 7.4.7.1 of RFC 5246), + * the calling code is vulnerable. + * * \note The output buffer length \c output_max_len should be * as large as the size \p ctx->len of \p ctx->N, for example, * 128 Bytes if RSA-1024 is used, to be able to hold an diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h index 5e33f6bd5..a17879b94 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -1736,6 +1736,13 @@ 0) /** RSA PKCS#1 v1.5 encryption. + * + * \warning Calling psa_asymmetric_decrypt() with this algorithm as a + * parameter is considered an inherently dangerous function + * (CWE-242). Unless it is used in a side channel free and safe + * way (eg. implementing the TLS protocol as per 7.4.7.1 of + * RFC 5246), the calling code is vulnerable. + * */ #define PSA_ALG_RSA_PKCS1V15_CRYPT ((psa_algorithm_t) 0x07000200) From 0d57f1034e2ebd1b29e1adb8620b1f0b16b6fe80 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 11 Jan 2024 14:24:02 +0000 Subject: [PATCH 19/22] Update Marvin fix Changelog entry Upon further consideration we think that a remote attacker close to the victim might be able to have precise enough timing information to exploit the side channel as well. Update the Changelog to reflect this. Signed-off-by: Janos Follath --- ChangeLog.d/fix-Marvin-attack.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ChangeLog.d/fix-Marvin-attack.txt b/ChangeLog.d/fix-Marvin-attack.txt index 017f7b1f8..763533c25 100644 --- a/ChangeLog.d/fix-Marvin-attack.txt +++ b/ChangeLog.d/fix-Marvin-attack.txt @@ -1,6 +1,8 @@ Security - * Fix a timing side channel in RSA private operations. This side channel - could be sufficient for a local attacker to recover the plaintext. It - requires the attacker to send a large number of messages for decryption. - For details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. - Reported by Hubert Kario, Red Hat. + * Fix a timing side channel in private key RSA operations. This side channel + could be sufficient for an attacker to recover the plaintext. A local + attacker or a remote attacker who is close to the victim on the network + might have precise enough timing measurements to exploit this. It requires + the attacker to send a large number of messages for decryption. For + details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. Reported + by Hubert Kario, Red Hat. From 6ba416968b0c14336141501b90ef9b34ec3a3eff Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 22 Jan 2024 15:40:12 +0000 Subject: [PATCH 20/22] Assemble Changelog Signed-off-by: Dave Rodgman --- ChangeLog | 15 +++++++++++++++ ChangeLog.d/fix-Marvin-attack.txt | 8 -------- ChangeLog.d/fix_int_overflow_x509_extension | 8 -------- 3 files changed, 15 insertions(+), 16 deletions(-) delete mode 100644 ChangeLog.d/fix-Marvin-attack.txt delete mode 100644 ChangeLog.d/fix_int_overflow_x509_extension diff --git a/ChangeLog b/ChangeLog index 28c45f718..28f2654b4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,20 @@ Mbed TLS ChangeLog (Sorted per branch, date) += Mbed TLS 3.5.2 branch released 2024-01-26 + +Security + * Fix a timing side channel in private key RSA operations. This side channel + could be sufficient for an attacker to recover the plaintext. A local + attacker or a remote attacker who is close to the victim on the network + might have precise enough timing measurements to exploit this. It requires + the attacker to send a large number of messages for decryption. For + details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. Reported + by Hubert Kario, Red Hat. + * Fix a failure to validate input when writing x509 extensions lengths which + could result in an integer overflow, causing a zero-length buffer to be + allocated to hold the extension. The extension would then be copied into + the buffer, causing a heap buffer overflow. + = Mbed TLS 3.5.1 branch released 2023-11-06 Changes diff --git a/ChangeLog.d/fix-Marvin-attack.txt b/ChangeLog.d/fix-Marvin-attack.txt deleted file mode 100644 index 763533c25..000000000 --- a/ChangeLog.d/fix-Marvin-attack.txt +++ /dev/null @@ -1,8 +0,0 @@ -Security - * Fix a timing side channel in private key RSA operations. This side channel - could be sufficient for an attacker to recover the plaintext. A local - attacker or a remote attacker who is close to the victim on the network - might have precise enough timing measurements to exploit this. It requires - the attacker to send a large number of messages for decryption. For - details, see "Everlasting ROBOT: the Marvin Attack", Hubert Kario. Reported - by Hubert Kario, Red Hat. diff --git a/ChangeLog.d/fix_int_overflow_x509_extension b/ChangeLog.d/fix_int_overflow_x509_extension deleted file mode 100644 index 2a679284f..000000000 --- a/ChangeLog.d/fix_int_overflow_x509_extension +++ /dev/null @@ -1,8 +0,0 @@ -Security - * Fix a failure to validate input when writing x509 extensions lengths which - could result in an integer overflow, causing a zero-length buffer to be - allocated to hold the extension. The extension would then be copied into - the buffer, causing a heap buffer overflow. - - - From e23d6479cc5925fa6221d3ca010334ad18302f4e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 22 Jan 2024 15:45:49 +0000 Subject: [PATCH 21/22] Bump version ./scripts/bump_version.sh --version 3.5.1 Signed-off-by: Dave Rodgman --- CMakeLists.txt | 2 +- doxygen/input/doc_mainpage.h | 2 +- doxygen/mbedtls.doxyfile | 2 +- include/mbedtls/build_info.h | 8 ++++---- library/CMakeLists.txt | 6 +++--- tests/suites/test_suite_version.data | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 87a41d75c..4321db8c5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -377,7 +377,7 @@ if(NOT DISABLE_PACKAGE_CONFIG_AND_INSTALL) write_basic_package_version_file( "cmake/MbedTLSConfigVersion.cmake" COMPATIBILITY SameMajorVersion - VERSION 3.5.1) + VERSION 3.5.2) install( FILES "${CMAKE_CURRENT_BINARY_DIR}/cmake/MbedTLSConfig.cmake" diff --git a/doxygen/input/doc_mainpage.h b/doxygen/input/doc_mainpage.h index c391c59ce..17762d726 100644 --- a/doxygen/input/doc_mainpage.h +++ b/doxygen/input/doc_mainpage.h @@ -10,7 +10,7 @@ */ /** - * @mainpage Mbed TLS v3.5.1 API Documentation + * @mainpage Mbed TLS v3.5.2 API Documentation * * This documentation describes the internal structure of Mbed TLS. It was * automatically generated from specially formatted comment blocks in diff --git a/doxygen/mbedtls.doxyfile b/doxygen/mbedtls.doxyfile index 89048f221..cbbb7597f 100644 --- a/doxygen/mbedtls.doxyfile +++ b/doxygen/mbedtls.doxyfile @@ -1,4 +1,4 @@ -PROJECT_NAME = "Mbed TLS v3.5.1" +PROJECT_NAME = "Mbed TLS v3.5.2" OUTPUT_DIRECTORY = ../apidoc/ FULL_PATH_NAMES = NO OPTIMIZE_OUTPUT_FOR_C = YES diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index c4fab1205..87e3c2ea1 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -26,16 +26,16 @@ */ #define MBEDTLS_VERSION_MAJOR 3 #define MBEDTLS_VERSION_MINOR 5 -#define MBEDTLS_VERSION_PATCH 1 +#define MBEDTLS_VERSION_PATCH 2 /** * The single version number has the following structure: * MMNNPP00 * Major version | Minor version | Patch version */ -#define MBEDTLS_VERSION_NUMBER 0x03050100 -#define MBEDTLS_VERSION_STRING "3.5.1" -#define MBEDTLS_VERSION_STRING_FULL "Mbed TLS 3.5.1" +#define MBEDTLS_VERSION_NUMBER 0x03050200 +#define MBEDTLS_VERSION_STRING "3.5.2" +#define MBEDTLS_VERSION_STRING_FULL "Mbed TLS 3.5.2" /* Macros for build-time platform detection */ diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index eeda06aee..fcd00a0ab 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -296,7 +296,7 @@ endif(USE_STATIC_MBEDTLS_LIBRARY) if(USE_SHARED_MBEDTLS_LIBRARY) set(CMAKE_LIBRARY_PATH ${CMAKE_CURRENT_BINARY_DIR}) add_library(${mbedcrypto_target} SHARED ${src_crypto}) - set_target_properties(${mbedcrypto_target} PROPERTIES VERSION 3.5.1 SOVERSION 15) + set_target_properties(${mbedcrypto_target} PROPERTIES VERSION 3.5.2 SOVERSION 15) target_link_libraries(${mbedcrypto_target} PUBLIC ${libs}) if(TARGET ${everest_target}) @@ -308,11 +308,11 @@ if(USE_SHARED_MBEDTLS_LIBRARY) endif() add_library(${mbedx509_target} SHARED ${src_x509}) - set_target_properties(${mbedx509_target} PROPERTIES VERSION 3.5.1 SOVERSION 6) + set_target_properties(${mbedx509_target} PROPERTIES VERSION 3.5.2 SOVERSION 6) target_link_libraries(${mbedx509_target} PUBLIC ${libs} ${mbedcrypto_target}) add_library(${mbedtls_target} SHARED ${src_tls}) - set_target_properties(${mbedtls_target} PROPERTIES VERSION 3.5.1 SOVERSION 20) + set_target_properties(${mbedtls_target} PROPERTIES VERSION 3.5.2 SOVERSION 20) target_link_libraries(${mbedtls_target} PUBLIC ${libs} ${mbedx509_target}) endif(USE_SHARED_MBEDTLS_LIBRARY) diff --git a/tests/suites/test_suite_version.data b/tests/suites/test_suite_version.data index faa31662a..6290331c1 100644 --- a/tests/suites/test_suite_version.data +++ b/tests/suites/test_suite_version.data @@ -1,8 +1,8 @@ Check compile time library version -check_compiletime_version:"3.5.1" +check_compiletime_version:"3.5.2" Check runtime library version -check_runtime_version:"3.5.1" +check_runtime_version:"3.5.2" Check for MBEDTLS_VERSION_C check_feature:"MBEDTLS_VERSION_C":0 From daca7a3979c22da155ec9dce49ab1abf3b65d3a9 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 24 Jan 2024 09:49:11 +0000 Subject: [PATCH 22/22] Update BRANCHES.md Signed-off-by: Dave Rodgman --- BRANCHES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BRANCHES.md b/BRANCHES.md index c085b1616..b71247f3e 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -106,6 +106,6 @@ The following branches are currently maintained: - [`development`](https://github.com/Mbed-TLS/mbedtls/) - [`mbedtls-2.28`](https://github.com/Mbed-TLS/mbedtls/tree/mbedtls-2.28) maintained until at least the end of 2024, see - . + . Users are urged to always use the latest version of a maintained branch.