From 24493a4ff4909616b6d95ad1e968ff485af0d4c4 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 25 Mar 2016 09:12:48 -1000 Subject: [PATCH] Always cache Montgomery contexts in RSA. Simplify the code by always caching Montgomery contexts in the RSA structure, regardless of the |RSA_FLAG_CACHE_PUBLIC| and |RSA_FLAG_CACHE_PRIVATE| flags. Deprecate those flags. Now that we do this no more than once per key per RSA exponent, the private key exponents better because the initialization of the Montgomery contexts isn't perfectly side-channel protected. Change-Id: I4fbcfec0f2f628930bfeb811285b0ae3d103ac5e Reviewed-on: https://boringssl-review.googlesource.com/7521 Reviewed-by: David Benjamin --- crypto/rsa/rsa_impl.c | 52 +++++++++++-------------------------------- include/openssl/rsa.h | 6 ++--- 2 files changed, 15 insertions(+), 43 deletions(-) diff --git a/crypto/rsa/rsa_impl.c b/crypto/rsa/rsa_impl.c index d98ca0da..ed0b1146 100644 --- a/crypto/rsa/rsa_impl.c +++ b/crypto/rsa/rsa_impl.c @@ -171,13 +171,8 @@ int rsa_default_encrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out, goto err; } - if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) { - if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL) { - goto err; - } - } - - if (!BN_mod_exp_mont(result, f, rsa->e, rsa->n, ctx, rsa->mont_n)) { + if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL || + !BN_mod_exp_mont(result, f, rsa->e, rsa->n, ctx, rsa->mont_n)) { goto err; } @@ -482,13 +477,8 @@ int rsa_default_verify_raw(RSA *rsa, size_t *out_len, uint8_t *out, goto err; } - if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) { - if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL) { - goto err; - } - } - - if (!BN_mod_exp_mont(result, f, rsa->e, rsa->n, ctx, rsa->mont_n)) { + if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL || + !BN_mod_exp_mont(result, f, rsa->e, rsa->n, ctx, rsa->mont_n)) { goto err; } @@ -584,14 +574,8 @@ int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in, d = &local_d; BN_with_flags(d, rsa->d, BN_FLG_CONSTTIME); - if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) { - if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == - NULL) { - goto err; - } - } - - if (!BN_mod_exp_mont_consttime(result, f, d, rsa->n, ctx, rsa->mont_n)) { + if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL || + !BN_mod_exp_mont_consttime(result, f, d, rsa->n, ctx, rsa->mont_n)) { goto err; } } @@ -656,20 +640,14 @@ static int mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) { q = &local_q; BN_with_flags(q, rsa->q, BN_FLG_CONSTTIME); - if (rsa->flags & RSA_FLAG_CACHE_PRIVATE) { - if (BN_MONT_CTX_set_locked(&rsa->mont_p, &rsa->lock, p, ctx) == NULL) { - goto err; - } - if (BN_MONT_CTX_set_locked(&rsa->mont_q, &rsa->lock, q, ctx) == NULL) { - goto err; - } + if (BN_MONT_CTX_set_locked(&rsa->mont_p, &rsa->lock, p, ctx) == NULL || + BN_MONT_CTX_set_locked(&rsa->mont_q, &rsa->lock, q, ctx) == NULL) { + goto err; } } - if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) { - if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL) { - goto err; - } + if (BN_MONT_CTX_set_locked(&rsa->mont_n, &rsa->lock, rsa->n, ctx) == NULL) { + goto err; } /* compute I mod q */ @@ -756,12 +734,8 @@ static int mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) { goto err; } - if ((rsa->flags & RSA_FLAG_CACHE_PRIVATE) && - !BN_MONT_CTX_set_locked(&ap->mont, &rsa->lock, prime, ctx)) { - goto err; - } - - if (!BN_mod_exp_mont_consttime(m1, r1, exp, prime, ctx, ap->mont)) { + if (BN_MONT_CTX_set_locked(&ap->mont, &rsa->lock, prime, ctx) == NULL || + !BN_mod_exp_mont_consttime(m1, r1, exp, prime, ctx, ap->mont)) { goto err; } diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h index 754182db..5dbc77ab 100644 --- a/include/openssl/rsa.h +++ b/include/openssl/rsa.h @@ -399,12 +399,10 @@ OPENSSL_EXPORT void *RSA_get_ex_data(const RSA *r, int idx); * API, like a platform key store. */ #define RSA_FLAG_OPAQUE 1 -/* RSA_FLAG_CACHE_PUBLIC causes a precomputed Montgomery context to be created, - * on demand, for the public key operations. */ +/* Deprecated and ignored. */ #define RSA_FLAG_CACHE_PUBLIC 2 -/* RSA_FLAG_CACHE_PRIVATE causes a precomputed Montgomery context to be - * created, on demand, for the private key operations. */ +/* Deprecated and ignored. */ #define RSA_FLAG_CACHE_PRIVATE 4 /* RSA_FLAG_NO_BLINDING disables blinding of private operations. */