diff --git a/crypto/fipsmodule/bn/bn.c b/crypto/fipsmodule/bn/bn.c index aa0c619a..520ca27d 100644 --- a/crypto/fipsmodule/bn/bn.c +++ b/crypto/fipsmodule/bn/bn.c @@ -297,7 +297,7 @@ int bn_set_words(BIGNUM *bn, const BN_ULONG *words, size_t num) { return 1; } -static int bn_fits_in_words(const BIGNUM *bn, size_t num) { +int bn_fits_in_words(const BIGNUM *bn, size_t num) { // All words beyond |num| must be zero. BN_ULONG mask = 0; for (size_t i = num; i < (size_t)bn->top; i++) { diff --git a/crypto/fipsmodule/bn/bn_test.cc b/crypto/fipsmodule/bn/bn_test.cc index 707d5f58..89db4ad1 100644 --- a/crypto/fipsmodule/bn/bn_test.cc +++ b/crypto/fipsmodule/bn/bn_test.cc @@ -1957,6 +1957,28 @@ TEST_F(BNTest, NonMinimal) { EXPECT_TRUE(bn_resize_words(eight.get(), 4)); EXPECT_EQ(4, eight->top); EXPECT_TRUE(BN_is_pow2(eight.get())); + + // |BN_MONT_CTX| is always stored minimally and uses the same R independent of + // input width. + static const uint8_t kP[] = { + 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }; + bssl::UniquePtr p(BN_bin2bn(kP, sizeof(kP), nullptr)); + ASSERT_TRUE(p); + + bssl::UniquePtr mont(BN_MONT_CTX_new()); + ASSERT_TRUE(mont); + ASSERT_TRUE(BN_MONT_CTX_set(mont.get(), p.get(), ctx())); + + ASSERT_TRUE(bn_resize_words(p.get(), 32)); + bssl::UniquePtr mont2(BN_MONT_CTX_new()); + ASSERT_TRUE(mont2); + ASSERT_TRUE(BN_MONT_CTX_set(mont2.get(), p.get(), ctx())); + + EXPECT_EQ(mont->N.top, mont2->N.top); + EXPECT_EQ(0, BN_cmp(&mont->RR, &mont2->RR)); } #endif // !BORINGSSL_SHARED_LIBRARY diff --git a/crypto/fipsmodule/bn/internal.h b/crypto/fipsmodule/bn/internal.h index 83102d8d..f3b8d8ad 100644 --- a/crypto/fipsmodule/bn/internal.h +++ b/crypto/fipsmodule/bn/internal.h @@ -226,6 +226,10 @@ int bn_resize_words(BIGNUM *bn, size_t words); // least significant word first. int bn_set_words(BIGNUM *bn, const BN_ULONG *words, size_t num); +// bn_fits_in_words returns one if |bn| may be represented in |num| words, plus +// a sign bit, and zero otherwise. +int bn_fits_in_words(const BIGNUM *bn, size_t num); + // bn_copy_words copies the value of |bn| to |out| and returns one if the value // is representable in |num| words. Otherwise, it returns zero. int bn_copy_words(BN_ULONG *out, size_t num, const BIGNUM *bn); @@ -347,6 +351,10 @@ int bn_is_bit_set_words(const BN_ULONG *a, size_t num, unsigned bit); // as public. int bn_one_to_montgomery(BIGNUM *r, const BN_MONT_CTX *mont, BN_CTX *ctx); +// bn_less_than_montgomery_R returns one if |bn| is less than the Montgomery R +// value for |mont| and zero otherwise. +int bn_less_than_montgomery_R(const BIGNUM *bn, const BN_MONT_CTX *mont); + // Low-level operations for small numbers. // diff --git a/crypto/fipsmodule/bn/montgomery.c b/crypto/fipsmodule/bn/montgomery.c index 49979fdb..624ab5fc 100644 --- a/crypto/fipsmodule/bn/montgomery.c +++ b/crypto/fipsmodule/bn/montgomery.c @@ -189,6 +189,10 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) { OPENSSL_PUT_ERROR(BN, ERR_R_INTERNAL_ERROR); return 0; } + // |mont->N| is always stored minimally. Computing RR efficiently leaks the + // size of the modulus. While the modulus may be private in RSA (one of the + // primes), their sizes are public, so this is fine. + bn_correct_top(&mont->N); // Find n0 such that n0 * N == -1 (mod r). // @@ -196,7 +200,7 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) { // others, we could use a shorter R value and use faster |BN_ULONG|-based // math instead of |uint64_t|-based math, which would be double-precision. // However, currently only the assembler files know which is which. - uint64_t n0 = bn_mont_n0(mod); + uint64_t n0 = bn_mont_n0(&mont->N); mont->n0[0] = (BN_ULONG)n0; #if BN_MONT_CTX_N0_LIMBS == 2 mont->n0[1] = (BN_ULONG)(n0 >> BN_BITS2); @@ -211,7 +215,7 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) { // as |BN_MONT_CTX_N0_LIMBS| is either one or two. // // XXX: This is not constant time with respect to |mont->N|, but it should be. - unsigned lgBigR = (BN_num_bits(mod) + (BN_BITS2 - 1)) / BN_BITS2 * BN_BITS2; + unsigned lgBigR = mont->N.top * BN_BITS2; if (!bn_mod_exp_base_2_vartime(&mont->RR, lgBigR * 2, &mont->N)) { return 0; } @@ -443,6 +447,11 @@ int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, return bn_mod_mul_montgomery_fallback(r, a, b, mont, ctx); } +int bn_less_than_montgomery_R(const BIGNUM *bn, const BN_MONT_CTX *mont) { + return !BN_is_negative(bn) && + bn_fits_in_words(bn, mont->N.top); +} + int bn_to_montgomery_small(BN_ULONG *r, size_t num_r, const BN_ULONG *a, size_t num_a, const BN_MONT_CTX *mont) { return bn_mod_mul_montgomery_small(r, num_r, a, num_a, mont->RR.d, diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index 1c7bcefa..626bbe85 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -646,12 +646,11 @@ err: static int mod_montgomery(BIGNUM *r, const BIGNUM *I, const BIGNUM *p, const BN_MONT_CTX *mont_p, const BIGNUM *q, BN_CTX *ctx) { - // Reduce in constant time with Montgomery reduction, which requires I <= p * - // R. If p and q are the same size, which is true for any RSA keys we or - // anyone sane generates, we have q < R and I < p * q, so this holds. - // - // If q is too big, fall back to |BN_mod|. - if (q->top > p->top) { + // Reducing in constant-time with Montgomery reduction requires I <= p * R. We + // have I < p * q, so this follows if q < R. In particular, this always holds + // if p and q are the same size, which is true for any RSA keys we or anyone + // sane generates. For other keys, we fall back to |BN_mod|. + if (!bn_less_than_montgomery_R(q, mont_p)) { return BN_mod(r, I, p, ctx); } diff --git a/include/openssl/bn.h b/include/openssl/bn.h index becc9553..d9a7f082 100644 --- a/include/openssl/bn.h +++ b/include/openssl/bn.h @@ -909,8 +909,11 @@ struct bignum_st { }; struct bn_mont_ctx_st { - BIGNUM RR; // used to convert to montgomery form - BIGNUM N; // The modulus + // RR is R^2, reduced modulo |N|. It is used to convert to Montgomery form. + BIGNUM RR; + // N is the modulus. It is always stored in minimal form, so |N.top| + // determines R. + BIGNUM N; BN_ULONG n0[2]; // least significant words of (R*Ri-1)/N };