From 42175031ca48e2fba62b97fc802e5df33d5221ff Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 8 Jan 2024 13:45:49 +0000 Subject: [PATCH 01/16] Move calculating RR into a separate function So far we needed it only locally here, but we will need calculating RR for safe unblinding in RSA as well. Signed-off-by: Janos Follath --- library/bignum.c | 18 +++++++++++++++--- library/bignum_internal.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 library/bignum_internal.h diff --git a/library/bignum.c b/library/bignum.c index 3e1c48c320..551fd81dd9 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -2039,6 +2039,20 @@ cleanup: return ret; } +int mbedtls_mpi_get_mont_r2_unsafe(mbedtls_mpi *X, + const mbedtls_mpi *N) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + MBEDTLS_MPI_CHK(mbedtls_mpi_lset(X, 1)); + MBEDTLS_MPI_CHK(mbedtls_mpi_shift_l(X, N->n * 2 * biL)); + MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(X, X, N)); + MBEDTLS_MPI_CHK(mbedtls_mpi_shrink(X, N->n)); + +cleanup: + return ret; +} + /* * Sliding-window exponentiation: X = A^E mod N (HAC 14.85) */ @@ -2153,9 +2167,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, * If 1st call, pre-compute R^2 mod N */ if (prec_RR == NULL || prec_RR->p == NULL) { - MBEDTLS_MPI_CHK(mbedtls_mpi_lset(&RR, 1)); - MBEDTLS_MPI_CHK(mbedtls_mpi_shift_l(&RR, N->n * 2 * biL)); - MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&RR, &RR, N)); + mbedtls_mpi_get_mont_r2_unsafe(&RR, N); if (prec_RR != NULL) { memcpy(prec_RR, &RR, sizeof(mbedtls_mpi)); diff --git a/library/bignum_internal.h b/library/bignum_internal.h new file mode 100644 index 0000000000..39909f3dd8 --- /dev/null +++ b/library/bignum_internal.h @@ -0,0 +1,31 @@ +/** + * Low level bignum functions + * + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#ifndef MBEDTLS_BIGNUM_INTERNAL_H +#define MBEDTLS_BIGNUM_INTERNAL_H + +#include "mbedtls/bignum.h" + +/** + * \brief Calculate the square of the Montgomery constant. (Needed + * for conversion and operations in Montgomery form.) + * + * \param[out] X A pointer to the result of the calculation of + * the square of the Montgomery constant: + * 2^{2*n*biL} mod N. + * \param[in] N Little-endian presentation of the modulus, which must be odd. + * + * \return 0 if successful. + * \return #MBEDTLS_ERR_MPI_ALLOC_FAILED if there is not enough space + * to store the value of Montgomery constant squared. + * \return #MBEDTLS_ERR_MPI_DIVISION_BY_ZERO if \p N modulus is zero. + * \return #MBEDTLS_ERR_MPI_NEGATIVE_VALUE if \p N modulus is negative. + */ +int mbedtls_mpi_get_mont_r2_unsafe(mbedtls_mpi *X, + const mbedtls_mpi *N); + +#endif /* MBEDTLS_BIGNUM_INTERNAL_H */ From 4fe396f1e1aa84346e23b89435a251624c205035 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 8 Jan 2024 14:08:17 +0000 Subject: [PATCH 02/16] Move some bignum functions to internal header We will need a couple of low level functions to implement safe unblinding in RSA. Signed-off-by: Janos Follath --- library/bignum.c | 67 ++++++++++++++------------------------- library/bignum_internal.h | 40 +++++++++++++++++++++++ 2 files changed, 63 insertions(+), 44 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 551fd81dd9..50da6b33b5 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1907,7 +1907,7 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_s /* * Fast Montgomery initialization (thanks to Tom St Denis) */ -static void mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N) +void mbedtls_mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N) { mbedtls_mpi_uint x, m0 = N->p[0]; unsigned int i; @@ -1922,33 +1922,11 @@ static void mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N) *mm = ~x + 1; } -/** Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36) - * - * \param[in,out] A One of the numbers to multiply. - * It must have at least as many limbs as N - * (A->n >= N->n), and any limbs beyond n are ignored. - * On successful completion, A contains the result of - * the multiplication A * B * R^-1 mod N where - * R = (2^ciL)^n. - * \param[in] B One of the numbers to multiply. - * It must be nonzero and must not have more limbs than N - * (B->n <= N->n). - * \param[in] N The modulo. N must be odd. - * \param mm The value calculated by `mpi_montg_init(&mm, N)`. - * This is -N^-1 mod 2^ciL. - * \param[in,out] T A bignum for temporary storage. - * It must be at least twice the limb size of N plus 2 - * (T->n >= 2 * (N->n + 1)). - * Its initial content is unused and - * its final content is indeterminate. - * Note that unlike the usual convention in the library - * for `const mbedtls_mpi*`, the content of T can change. - */ -static void mpi_montmul(mbedtls_mpi *A, - const mbedtls_mpi *B, - const mbedtls_mpi *N, - mbedtls_mpi_uint mm, - const mbedtls_mpi *T) +void mbedtls_mpi_montmul(mbedtls_mpi *A, + const mbedtls_mpi *B, + const mbedtls_mpi *N, + mbedtls_mpi_uint mm, + const mbedtls_mpi *T) { size_t i, n, m; mbedtls_mpi_uint u0, u1, *d; @@ -1996,7 +1974,8 @@ static void mpi_montmul(mbedtls_mpi *A, /* * Montgomery reduction: A = A * R^-1 mod N * - * See mpi_montmul() regarding constraints and guarantees on the parameters. + * See mbedtls_mpi_montmul() regarding constraints and guarantees on the + * parameters. */ static void mpi_montred(mbedtls_mpi *A, const mbedtls_mpi *N, mbedtls_mpi_uint mm, const mbedtls_mpi *T) @@ -2007,7 +1986,7 @@ static void mpi_montred(mbedtls_mpi *A, const mbedtls_mpi *N, U.n = U.s = (int) z; U.p = &z; - mpi_montmul(A, &U, N, mm, T); + mbedtls_mpi_montmul(A, &U, N, mm, T); } /** @@ -2090,7 +2069,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, /* * Init temps and window size */ - mpi_montg_init(&mm, N); + mbedtls_mpi_montg_init(&mm, N); mbedtls_mpi_init(&RR); mbedtls_mpi_init(&T); mbedtls_mpi_init(&Apos); mbedtls_mpi_init(&WW); @@ -2144,10 +2123,10 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, j = N->n + 1; /* All W[i] including the accumulator must have at least N->n limbs for - * the mpi_montmul() and mpi_montred() calls later. Here we ensure that - * W[1] and the accumulator W[x_index] are large enough. later we'll grow - * other W[i] to the same length. They must not be shrunk midway through - * this function! + * the mbedtls_mpi_montmul() and mpi_montred() calls later. Here we ensure + * that W[1] and the accumulator W[x_index] are large enough. later we'll + * grow other W[i] to the same length. They must not be shrunk midway + * through this function! */ MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&W[x_index], j)); MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&W[1], j)); @@ -2183,7 +2162,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&W[1], A, N)); /* This should be a no-op because W[1] is already that large before * mbedtls_mpi_mod_mpi(), but it's necessary to avoid an overflow - * in mpi_montmul() below, so let's make sure. */ + * in mbedtls_mpi_montmul() below, so let's make sure. */ MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&W[1], N->n + 1)); } else { MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&W[1], A)); @@ -2191,7 +2170,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, /* Note that this is safe because W[1] always has at least N->n limbs * (it grew above and was preserved by mbedtls_mpi_copy()). */ - mpi_montmul(&W[1], &RR, N, mm, &T); + mbedtls_mpi_montmul(&W[1], &RR, N, mm, &T); /* * W[x_index] = R^2 * R^-1 mod N = R mod N @@ -2217,7 +2196,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&W[j], &W[1])); for (i = 0; i < window_bitsize - 1; i++) { - mpi_montmul(&W[j], &W[j], N, mm, &T); + mbedtls_mpi_montmul(&W[j], &W[j], N, mm, &T); } /* @@ -2227,7 +2206,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, MBEDTLS_MPI_CHK(mbedtls_mpi_grow(&W[i], N->n + 1)); MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&W[i], &W[i - 1])); - mpi_montmul(&W[i], &W[1], N, mm, &T); + mbedtls_mpi_montmul(&W[i], &W[1], N, mm, &T); } } @@ -2263,7 +2242,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, * out of window, square W[x_index] */ MBEDTLS_MPI_CHK(mpi_select(&WW, W, w_table_used_size, x_index)); - mpi_montmul(&W[x_index], &WW, N, mm, &T); + mbedtls_mpi_montmul(&W[x_index], &WW, N, mm, &T); continue; } @@ -2282,7 +2261,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, for (i = 0; i < window_bitsize; i++) { MBEDTLS_MPI_CHK(mpi_select(&WW, W, w_table_used_size, x_index)); - mpi_montmul(&W[x_index], &WW, N, mm, &T); + mbedtls_mpi_montmul(&W[x_index], &WW, N, mm, &T); } /* @@ -2290,7 +2269,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, */ MBEDTLS_MPI_CHK(mpi_select(&WW, W, w_table_used_size, exponent_bits_in_window)); - mpi_montmul(&W[x_index], &WW, N, mm, &T); + mbedtls_mpi_montmul(&W[x_index], &WW, N, mm, &T); state--; nbits = 0; @@ -2303,13 +2282,13 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, */ for (i = 0; i < nbits; i++) { MBEDTLS_MPI_CHK(mpi_select(&WW, W, w_table_used_size, x_index)); - mpi_montmul(&W[x_index], &WW, N, mm, &T); + mbedtls_mpi_montmul(&W[x_index], &WW, N, mm, &T); exponent_bits_in_window <<= 1; if ((exponent_bits_in_window & ((size_t) 1 << window_bitsize)) != 0) { MBEDTLS_MPI_CHK(mpi_select(&WW, W, w_table_used_size, 1)); - mpi_montmul(&W[x_index], &WW, N, mm, &T); + mbedtls_mpi_montmul(&W[x_index], &WW, N, mm, &T); } } diff --git a/library/bignum_internal.h b/library/bignum_internal.h index 39909f3dd8..f14c294a5a 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -28,4 +28,44 @@ int mbedtls_mpi_get_mont_r2_unsafe(mbedtls_mpi *X, const mbedtls_mpi *N); +/** + * \brief Calculate initialisation value for fast Montgomery modular + * multiplication. + * + * \param[out] mm The initialisation value for fast Montgomery modular + * multiplication. + * \param[in] N Little-endian presentation of the modulus. This must have + * at least one limb. + */ +void mbedtls_mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N); + +/** Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36) + * + * \param[in,out] A One of the numbers to multiply. + * It must have at least as many limbs as N + * (A->n >= N->n), and any limbs beyond n are ignored. + * On successful completion, A contains the result of + * the multiplication A * B * R^-1 mod N where + * R = (2^ciL)^n. + * \param[in] B One of the numbers to multiply. + * It must be nonzero and must not have more limbs than N + * (B->n <= N->n). + * \param[in] N The modulo. N must be odd. + * \param mm The value calculated by + * `mbedtls_mpi_montg_init(&mm, N)`. + * This is -N^-1 mod 2^ciL. + * \param[in,out] T A bignum for temporary storage. + * It must be at least twice the limb size of N plus 2 + * (T->n >= 2 * (N->n + 1)). + * Its initial content is unused and + * its final content is indeterminate. + * Note that unlike the usual convention in the library + * for `const mbedtls_mpi*`, the content of T can change. + */ +void mbedtls_mpi_montmul(mbedtls_mpi *A, + const mbedtls_mpi *B, + const mbedtls_mpi *N, + mbedtls_mpi_uint mm, + const mbedtls_mpi *T); + #endif /* MBEDTLS_BIGNUM_INTERNAL_H */ From aa6760d7b5d9a218eaf072f4155974f58b00986b Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 8 Jan 2024 15:09:34 +0000 Subject: [PATCH 03/16] Make RSA unblinding constant flow Signed-off-by: Janos Follath --- library/rsa.c | 45 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 84403c4579..937d4aacdb 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -34,6 +34,7 @@ #include "mbedtls/error.h" #include "constant_time_internal.h" #include "mbedtls/constant_time.h" +#include "bignum_internal.h" #include @@ -804,6 +805,47 @@ cleanup: return ret; } +/* + * Unblind + * T = T * Vf mod N + */ +static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, const mbedtls_mpi *N) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const size_t nlimbs = N->n; + const size_t tlimbs = 2 * (nlimbs + 1); + + mbedtls_mpi_uint mm; + mbedtls_mpi_montg_init(&mm, N); + + mbedtls_mpi RR, M_T; + + mbedtls_mpi_init(&RR); + mbedtls_mpi_init(&M_T); + + MBEDTLS_MPI_CHK(mbedtls_mpi_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 * Vf mod N + * Reminder: montmul(A, B, N) = A * B * R^-1 mod N + * Usually both operands are multiplied by R mod N beforehand, 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 `mpi_montred()` on it. */ + mbedtls_mpi_montmul(T, &RR, N, mm, &M_T); + mbedtls_mpi_montmul(T, Vf, N, mm, &M_T); + +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, @@ -1000,8 +1042,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. */ From 601bffc4cec7c78cfc6b64048379172578fce13c Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 8 Jan 2024 15:19:11 +0000 Subject: [PATCH 04/16] Extend blinding to RSA result check Signed-off-by: Janos Follath --- library/rsa.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 937d4aacdb..23fe84310b 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -909,7 +909,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; RSA_VALIDATE_RET(ctx != NULL); RSA_VALIDATE_RET(input != NULL); @@ -946,8 +946,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 */ @@ -957,8 +957,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, goto cleanup; } - MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&I, &T)); - if (f_rng != NULL) { /* * Blinding @@ -1010,6 +1008,9 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, #endif /* MBEDTLS_RSA_NO_CRT */ } + /* Make a copy of the input (after blinding if there was any) */ + MBEDTLS_MPI_CHK(mbedtls_mpi_copy(&input_blinded, &T)); + #if defined(MBEDTLS_RSA_NO_CRT) MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&T, &T, D, &ctx->N, &ctx->RN)); #else @@ -1037,6 +1038,14 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&T, &TQ, &TP)); #endif /* MBEDTLS_RSA_NO_CRT */ + /* Verify the result to prevent glitching attacks. */ + MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&check_result_blinded, &T, &ctx->E, + &ctx->N, &ctx->RN)); + if (mbedtls_mpi_cmp_mpi(&check_result_blinded, &input_blinded) != 0) { + ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; + goto cleanup; + } + if (f_rng != NULL) { /* * Unblind @@ -1045,14 +1054,6 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx, 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)); - if (mbedtls_mpi_cmp_mpi(&C, &I) != 0) { - ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; - goto cleanup; - } - olen = ctx->len; MBEDTLS_MPI_CHK(mbedtls_mpi_write_binary(&T, output, olen)); @@ -1082,8 +1083,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 eaeff5b75acb15fb376793d28018c8450367ee5f Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 29 Dec 2023 11:14:58 +0000 Subject: [PATCH 05/16] 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 667e6257e6..1779775155 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -712,6 +712,10 @@ int mbedtls_rsa_rsaes_oaep_encrypt(mbedtls_rsa_context *ctx, * It is the generic wrapper for performing a PKCS#1 decryption * operation using the \p mode from the context. * + * \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 @@ -761,6 +765,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 a398249904..c25fda63ec 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -1659,6 +1659,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 f10bfbbe744ecffeb88ac8518390e432b2d1bb2a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:57:27 +0000 Subject: [PATCH 06/16] 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 0000000000..017f7b1f80 --- /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 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 8cdb6064de78582a3bed86524bb342a393e19150 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 9 Jan 2024 09:28:48 +0000 Subject: [PATCH 07/16] Align Montgomery init with development The signature and naming of the Montgomrey initialisation function in development and in the LTS was different. Align them for easier readability and maintenance. Signed-off-by: Janos Follath --- library/bignum.c | 16 +++++++--------- library/bignum_internal.h | 12 ++++++------ library/rsa.c | 3 +-- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 50da6b33b5..74f10af8d5 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1907,19 +1907,17 @@ int mbedtls_mpi_mod_int(mbedtls_mpi_uint *r, const mbedtls_mpi *A, mbedtls_mpi_s /* * Fast Montgomery initialization (thanks to Tom St Denis) */ -void mbedtls_mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N) +mbedtls_mpi_uint mbedtls_mpi_montmul_init(const mbedtls_mpi_uint *N) { - mbedtls_mpi_uint x, m0 = N->p[0]; - unsigned int i; + mbedtls_mpi_uint x = N[0]; - x = m0; - x += ((m0 + 2) & 4) << 1; + x += ((N[0] + 2) & 4) << 1; - for (i = biL; i >= 8; i /= 2) { - x *= (2 - (m0 * x)); + for (unsigned int i = biL; i >= 8; i /= 2) { + x *= (2 - (N[0] * x)); } - *mm = ~x + 1; + return ~x + 1; } void mbedtls_mpi_montmul(mbedtls_mpi *A, @@ -2069,7 +2067,7 @@ int mbedtls_mpi_exp_mod(mbedtls_mpi *X, const mbedtls_mpi *A, /* * Init temps and window size */ - mbedtls_mpi_montg_init(&mm, N); + mm = mbedtls_mpi_montmul_init(N->p); mbedtls_mpi_init(&RR); mbedtls_mpi_init(&T); mbedtls_mpi_init(&Apos); mbedtls_mpi_init(&WW); diff --git a/library/bignum_internal.h b/library/bignum_internal.h index f14c294a5a..5435ebb464 100644 --- a/library/bignum_internal.h +++ b/library/bignum_internal.h @@ -30,14 +30,14 @@ int mbedtls_mpi_get_mont_r2_unsafe(mbedtls_mpi *X, /** * \brief Calculate initialisation value for fast Montgomery modular - * multiplication. + * multiplication * - * \param[out] mm The initialisation value for fast Montgomery modular - * multiplication. - * \param[in] N Little-endian presentation of the modulus. This must have - * at least one limb. + * \param[in] N Little-endian presentation of the modulus. This must have + * at least one limb. + * + * \return The initialisation value for fast Montgomery modular multiplication */ -void mbedtls_mpi_montg_init(mbedtls_mpi_uint *mm, const mbedtls_mpi *N); +mbedtls_mpi_uint mbedtls_mpi_montmul_init(const mbedtls_mpi_uint *N); /** Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36) * diff --git a/library/rsa.c b/library/rsa.c index 23fe84310b..0a0c2e3880 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -815,8 +815,7 @@ static int rsa_unblind(mbedtls_mpi *T, mbedtls_mpi *Vf, const mbedtls_mpi *N) const size_t nlimbs = N->n; const size_t tlimbs = 2 * (nlimbs + 1); - mbedtls_mpi_uint mm; - mbedtls_mpi_montg_init(&mm, N); + mbedtls_mpi_uint mm = mbedtls_mpi_montmul_init(N->p); mbedtls_mpi RR, M_T; From 1a9a69778e2bdd4280c71ddcbd2a8a0731828bc3 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 9 Jan 2024 09:37:06 +0000 Subject: [PATCH 08/16] Fix 'missing prototype' warnings Signed-off-by: Janos Follath --- library/bignum.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/bignum.c b/library/bignum.c index 74f10af8d5..fadd9e9cc2 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -30,6 +30,7 @@ #include "mbedtls/platform_util.h" #include "mbedtls/error.h" #include "constant_time_internal.h" +#include "bignum_internal.h" #include #include From 80a12f86f9586e6a8e7867e51f228c4f89a5145d Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 10 Jan 2024 08:54:17 +0000 Subject: [PATCH 09/16] Add new internal header to visualc project Signed-off-by: Janos Follath --- visualc/VS2010/mbedTLS.vcxproj | 1 + 1 file changed, 1 insertion(+) diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj index c75b353548..e36a7b5706 100644 --- a/visualc/VS2010/mbedTLS.vcxproj +++ b/visualc/VS2010/mbedTLS.vcxproj @@ -258,6 +258,7 @@ + From 4a606d6f3fb0febf62517b410b63ced5f095e050 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 11 Jan 2024 14:24:02 +0000 Subject: [PATCH 10/16] 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 017f7b1f80..763533c25c 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 d78496cccf5bca8edbbfee1fcd04683d09d48b4d Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Wed, 10 Jan 2024 13:26:12 +0100 Subject: [PATCH 11/16] Add tests for Issue #8687 Signed-off-by: Jonathan Winzig --- tests/suites/test_suite_x509write.data | 3 +++ tests/suites/test_suite_x509write.function | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index 999c05f1cf..6bb81c7e78 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -138,3 +138,6 @@ mbedtls_x509_string_to_names:"ABC123":"":MBEDTLS_ERR_X509_INVALID_NAME Check max serial length x509_set_serial_check: + +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 b4509e235c..039690be1c 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -498,3 +498,24 @@ exit: USE_PSA_DONE(); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CSR_WRITE_C */ +void x509_set_extension_length_check() +{ + 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.extensions), + MBEDTLS_OID_EXTENDED_KEY_USAGE, + MBEDTLS_OID_SIZE(MBEDTLS_OID_EXTENDED_KEY_USAGE), + 0, + p, + SIZE_MAX); + TEST_ASSERT(MBEDTLS_ERR_X509_BAD_INPUT_DATA == ret); +} +/* END_CASE */ From e90cbc3d1260901efc879178a15f2d90a5e17612 Mon Sep 17 00:00:00 2001 From: Jonathan Winzig Date: Wed, 10 Jan 2024 13:26:36 +0100 Subject: [PATCH 12/16] 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 73789dad58..4ffd3b6a80 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -195,6 +195,10 @@ int mbedtls_x509_set_extension(mbedtls_asn1_named_data **head, const char *oid, { mbedtls_asn1_named_data *cur; + if (val_len > (SIZE_MAX - 1)) { + 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 634748da23df4f47409a3ab4eb41c43b0d6b35b0 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 16 Jan 2024 11:16:56 +0000 Subject: [PATCH 13/16] 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 0000000000..2a679284f8 --- /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 e557764cf3ab530d7ab93ea9dd4db94edf535f95 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 22 Jan 2024 16:46:41 +0000 Subject: [PATCH 14/16] 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 a856275c73..5434e5509b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,20 @@ Mbed TLS ChangeLog (Sorted per branch, date) += Mbed TLS 2.28.7 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 2.28.6 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 763533c25c..0000000000 --- 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 2a679284f8..0000000000 --- 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 f15483106779bf8ec31499f5fb65982c17e06b72 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 22 Jan 2024 16:47:12 +0000 Subject: [PATCH 15/16] bump version Signed-off-by: Dave Rodgman --- doxygen/input/doc_mainpage.h | 2 +- doxygen/mbedtls.doxyfile | 2 +- include/mbedtls/version.h | 8 ++++---- library/CMakeLists.txt | 6 +++--- tests/suites/test_suite_version.data | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/doxygen/input/doc_mainpage.h b/doxygen/input/doc_mainpage.h index c2343a68b2..f22ca106d3 100644 --- a/doxygen/input/doc_mainpage.h +++ b/doxygen/input/doc_mainpage.h @@ -10,7 +10,7 @@ */ /** - * @mainpage Mbed TLS v2.28.6 API Documentation + * @mainpage Mbed TLS v2.28.7 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 1939ac9e80..2c710406be 100644 --- a/doxygen/mbedtls.doxyfile +++ b/doxygen/mbedtls.doxyfile @@ -1,4 +1,4 @@ -PROJECT_NAME = "Mbed TLS v2.28.6" +PROJECT_NAME = "Mbed TLS v2.28.7" OUTPUT_DIRECTORY = ../apidoc/ FULL_PATH_NAMES = NO OPTIMIZE_OUTPUT_FOR_C = YES diff --git a/include/mbedtls/version.h b/include/mbedtls/version.h index ae91a09cb0..0533bca681 100644 --- a/include/mbedtls/version.h +++ b/include/mbedtls/version.h @@ -26,16 +26,16 @@ */ #define MBEDTLS_VERSION_MAJOR 2 #define MBEDTLS_VERSION_MINOR 28 -#define MBEDTLS_VERSION_PATCH 6 +#define MBEDTLS_VERSION_PATCH 7 /** * The single version number has the following structure: * MMNNPP00 * Major version | Minor version | Patch version */ -#define MBEDTLS_VERSION_NUMBER 0x021C0600 -#define MBEDTLS_VERSION_STRING "2.28.6" -#define MBEDTLS_VERSION_STRING_FULL "Mbed TLS 2.28.6" +#define MBEDTLS_VERSION_NUMBER 0x021C0700 +#define MBEDTLS_VERSION_STRING "2.28.7" +#define MBEDTLS_VERSION_STRING_FULL "Mbed TLS 2.28.7" #if defined(MBEDTLS_VERSION_C) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index c7105a1fdf..1b2980dbc6 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -204,15 +204,15 @@ 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 2.28.6 SOVERSION 7) + set_target_properties(${mbedcrypto_target} PROPERTIES VERSION 2.28.7 SOVERSION 7) target_link_libraries(${mbedcrypto_target} PUBLIC ${libs}) add_library(${mbedx509_target} SHARED ${src_x509}) - set_target_properties(${mbedx509_target} PROPERTIES VERSION 2.28.6 SOVERSION 1) + set_target_properties(${mbedx509_target} PROPERTIES VERSION 2.28.7 SOVERSION 1) target_link_libraries(${mbedx509_target} PUBLIC ${libs} ${mbedcrypto_target}) add_library(${mbedtls_target} SHARED ${src_tls}) - set_target_properties(${mbedtls_target} PROPERTIES VERSION 2.28.6 SOVERSION 14) + set_target_properties(${mbedtls_target} PROPERTIES VERSION 2.28.7 SOVERSION 14) 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 c1b81a425b..7d8f252cce 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:"2.28.6" +check_compiletime_version:"2.28.7" Check runtime library version -check_runtime_version:"2.28.6" +check_runtime_version:"2.28.7" Check for MBEDTLS_VERSION_C check_feature:"MBEDTLS_VERSION_C":0 From 555f84735aecdbd76a566cf087ec8425dfb0c8ab Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 22 Jan 2024 16:47:55 +0000 Subject: [PATCH 16/16] 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 6711cb95c9..28a2c6a5ca 100644 --- a/BRANCHES.md +++ b/BRANCHES.md @@ -76,6 +76,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.