From 10f8366499e7a45b67d55c88160ecda1d652e2f9 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:33:54 +0000 Subject: [PATCH 01/10] 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 3c538bf439..c559f110ac 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -40,6 +40,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" @@ -981,6 +982,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, @@ -1172,8 +1207,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 2d8624dae26734f55d1bd9aaad1abefc084b2e57 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:46:43 +0000 Subject: [PATCH 02/10] 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 c559f110ac..9dc0ec505c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1125,8 +1125,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 @@ -1135,6 +1133,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 */ @@ -1203,12 +1203,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)); @@ -1217,6 +1211,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 aa5cc7b930c96f2580562925ce654aad6befa457 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 09:57:27 +0000 Subject: [PATCH 03/10] 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..f729304eef --- /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 8209ff335ef94fbb949f5bd634bcbc2e0ba86a49 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 12:48:52 +0000 Subject: [PATCH 04/10] 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 9dc0ec505c..3095fec883 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -986,7 +986,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 f7f88d6443f369e384731667c47fe28f829bfdda Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 21 Nov 2023 14:20:08 +0000 Subject: [PATCH 05/10] 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 3095fec883..8ddef2dac3 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -986,7 +986,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 dad6d6666141cd9aee7c18f4cda403e738d1f5fd Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:22:59 +0000 Subject: [PATCH 06/10] 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 8ddef2dac3..803759b6b2 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1003,9 +1003,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 c762521e7389dbe2f17d40217890e4307b7a1d18 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:33:00 +0000 Subject: [PATCH 07/10] 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 803759b6b2..1357e1bc02 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1068,18 +1068,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 @@ -1155,8 +1146,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 @@ -1167,8 +1156,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 */ @@ -1177,12 +1164,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 @@ -1191,8 +1176,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 d83dc85a10f52e0c3d0942d61474bbeb49f230f5 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:44:36 +0000 Subject: [PATCH 08/10] 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 1357e1bc02..9e59e9de26 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -986,7 +986,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); @@ -1075,7 +1075,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; @@ -1110,8 +1110,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 */ @@ -1129,7 +1129,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 @@ -1194,9 +1194,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; } @@ -1234,8 +1234,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 8c4cabf6aa65e92b70c2b410021014276586c6d0 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Wed, 27 Dec 2023 10:47:21 +0000 Subject: [PATCH 09/10] 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 f729304eef..017f7b1f80 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 2125443aef349e08885da39f622beb56619091cd Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 29 Dec 2023 11:14:58 +0000 Subject: [PATCH 10/10] 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 69f3981ede..84d5d16e9a 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -696,6 +696,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 @@ -732,6 +736,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 241b7c80d1..f59f7fde12 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -1748,6 +1748,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)