Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 90 additions & 31 deletions library/rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,93 @@ int mbedtls_rsa_public(mbedtls_rsa_context *ctx,
return 0;
}

#if !defined(MBEDTLS_RSA_NO_CRT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're fixing a performance regression in an LTS branch, so this needs a changelog entry.

/*
* Compute T such that T = TP mod P and T = TP mod Q.
* (This is the Chinese Remainder Theorem - CRT.)
*
* WARNING: uses TP as a temporary, so its value is lost!
*/
static int rsa_apply_crt(mbedtls_mpi *T,
mbedtls_mpi *TP,
const mbedtls_mpi *TQ,
const mbedtls_rsa_context *ctx)
{
int ret;

/*
* T = (TP - TQ) * (Q^-1 mod P) mod P
*/
MBEDTLS_MPI_CHK(mbedtls_mpi_sub_mpi(T, TP, TQ));
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(TP, T, &ctx->QP));
MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(T, TP, &ctx->P));

/*
* T = TQ + T * Q
*/
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(TP, T, &ctx->Q));
MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(T, TQ, TP));

cleanup:
return ret;
}
#endif

/* Generate random A and B such that A^-1 = B mod N */
static int rsa_gen_rand_with_inverse(const mbedtls_rsa_context *ctx,
mbedtls_mpi *A,
mbedtls_mpi *B,
int (*f_rng)(void *, unsigned char *, size_t),
void *p_rng)
{
#if defined(MBEDTLS_RSA_NO_CRT)
int ret, count = 0;
mbedtls_mpi G;

mbedtls_mpi_init(&G);

mbedtls_mpi_lset(&G, 0);
do {
if (count++ > 10) {
ret = MBEDTLS_ERR_RSA_RNG_FAILED;
goto cleanup;
}

MBEDTLS_MPI_CHK(mbedtls_mpi_random(A, 1, &ctx->N, f_rng, p_rng));
MBEDTLS_MPI_CHK(mbedtls_mpi_gcd_modinv_odd(&G, B, A, &ctx->N));
} while (mbedtls_mpi_cmp_int(&G, 1) != 0);

cleanup:
mbedtls_mpi_free(&G);

return ret;
#else
int ret;
mbedtls_mpi Ap, Aq, Bp, Bq;

mbedtls_mpi_init(&Ap); mbedtls_mpi_init(&Aq);
mbedtls_mpi_init(&Bp); mbedtls_mpi_init(&Bq);

/* Generate Ap in [1, P) and compute Bp = Ap^-1 mod P */
MBEDTLS_MPI_CHK(mbedtls_mpi_random(&Ap, 1, &ctx->P, f_rng, p_rng));
MBEDTLS_MPI_CHK(mbedtls_mpi_gcd_modinv_odd(NULL, &Bp, &Ap, &ctx->P));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If P is not actually prime, there is a chance that Ap is not invertible mod P, and Bp is “indeterminate” according to the documentation of mbedtls_mpi_gcd_modinv_odd. So subsequent uses of the RSA private key will use indeterminate blinding values.

An RSA key should have probable primes as P and Q, so the probability of finding a factor is very small. However, it bothers me that we aren't checking this. Especially since the old code did check (the condition G=1 catches multiple of P or Q for a perfect RSA key, but also more generally any value that isn't invertible mod N).

In both cases, I'd be ok with erroring immediately out if A turns out not to be invertible, or to try a few iterations. I don't see a reason why the number of retries would be different with a single random value or with two: the case of a multiple of P or Q has negligible probability, so it doesn't really make a difference in terms of expected retries to find a suitable A.


/* Generate Ap in [1, Q) and compute Bq = Aq^-1 mod P */
MBEDTLS_MPI_CHK(mbedtls_mpi_random(&Aq, 1, &ctx->Q, f_rng, p_rng));
MBEDTLS_MPI_CHK(mbedtls_mpi_gcd_modinv_odd(NULL, &Bq, &Aq, &ctx->Q));

/* Reconstruct A and B */
MBEDTLS_MPI_CHK(rsa_apply_crt(A, &Ap, &Aq, ctx));
MBEDTLS_MPI_CHK(rsa_apply_crt(B, &Bp, &Bq, ctx));

cleanup:
mbedtls_mpi_free(&Ap); mbedtls_mpi_free(&Aq);
mbedtls_mpi_free(&Bp); mbedtls_mpi_free(&Bq);

return ret;
#endif
}

/*
* Generate or update blinding values, see section 10 of:
* KOCHER, Paul C. Timing attacks on implementations of Diffie-Hellman, RSA,
Expand All @@ -1277,41 +1364,25 @@ int mbedtls_rsa_public(mbedtls_rsa_context *ctx,
static int rsa_prepare_blinding(mbedtls_rsa_context *ctx,
int (*f_rng)(void *, unsigned char *, size_t), void *p_rng)
{
int ret, count = 0;
mbedtls_mpi R;

mbedtls_mpi_init(&R);
int ret;

if (ctx->Vf.p != NULL) {
/* We already have blinding values, just update them by squaring */
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vi, &ctx->Vi));
MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi, &ctx->Vi, &ctx->N));
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vf, &ctx->Vf, &ctx->Vf));
MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vf, &ctx->Vf, &ctx->N));

goto cleanup;
}

/* Unblinding value: Vf = random number, invertible mod N */
mbedtls_mpi_lset(&R, 0);
do {
if (count++ > 10) {
ret = MBEDTLS_ERR_RSA_RNG_FAILED;
goto cleanup;
}

MBEDTLS_MPI_CHK(mbedtls_mpi_random(&ctx->Vf, 1, &ctx->N, f_rng, p_rng));
MBEDTLS_MPI_CHK(mbedtls_mpi_gcd_modinv_odd(&R, &ctx->Vi, &ctx->Vf, &ctx->N));
} while (mbedtls_mpi_cmp_int(&R, 1) != 0);
MBEDTLS_MPI_CHK(rsa_gen_rand_with_inverse(ctx, &ctx->Vf, &ctx->Vi, f_rng, p_rng));

/* Blinding value: Vi = Vf^(-e) mod N
* (Vi already contains Vf^-1 at this point) */
MBEDTLS_MPI_CHK(mbedtls_mpi_exp_mod(&ctx->Vi, &ctx->Vi, &ctx->E, &ctx->N, &ctx->RN));


cleanup:
mbedtls_mpi_free(&R);

return ret;
}

Expand Down Expand Up @@ -1511,19 +1582,7 @@ int mbedtls_rsa_private(mbedtls_rsa_context *ctx,

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
*/
MBEDTLS_MPI_CHK(mbedtls_mpi_sub_mpi(&T, &TP, &TQ));
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&TP, &T, &ctx->QP));
MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&T, &TP, &ctx->P));

/*
* T = TQ + T * Q
*/
MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&TP, &T, &ctx->Q));
MBEDTLS_MPI_CHK(mbedtls_mpi_add_mpi(&T, &TQ, &TP));
MBEDTLS_MPI_CHK(rsa_apply_crt(&T, &TP, &TQ, ctx));
#endif /* MBEDTLS_RSA_NO_CRT */

/* Verify the result to prevent glitching attacks. */
Expand Down