diff --git a/crypto/fipsmodule/bn/bn_test.cc b/crypto/fipsmodule/bn/bn_test.cc index a9442cdab..707d5f582 100644 --- a/crypto/fipsmodule/bn/bn_test.cc +++ b/crypto/fipsmodule/bn/bn_test.cc @@ -1890,44 +1890,61 @@ TEST_F(BNTest, NonMinimal) { bssl::UniquePtr ten(BN_new()); ASSERT_TRUE(ten); ASSERT_TRUE(BN_set_word(ten.get(), 10)); + bssl::UniquePtr ten_copy(BN_dup(ten.get())); ASSERT_TRUE(ten_copy); + bssl::UniquePtr eight(BN_new()); ASSERT_TRUE(eight); ASSERT_TRUE(BN_set_word(eight.get(), 8)); - // Check some comparison functions on |ten|. - EXPECT_TRUE(BN_abs_is_word(ten.get(), 10)); - EXPECT_TRUE(BN_is_word(ten.get(), 10)); - EXPECT_EQ(10u, BN_get_word(ten.get())); - uint64_t v; - ASSERT_TRUE(BN_get_u64(ten.get(), &v)); - EXPECT_EQ(10u, v); - EXPECT_TRUE(BN_equal_consttime(ten.get(), ten_copy.get())); - EXPECT_EQ(BN_cmp(ten.get(), ten_copy.get()), 0); - EXPECT_FALSE(BN_equal_consttime(ten.get(), eight.get())); - EXPECT_LT(BN_cmp(eight.get(), ten.get()), 0); - EXPECT_EQ(4u, BN_num_bits(ten.get())); - EXPECT_EQ(1u, BN_num_bytes(ten.get())); - EXPECT_FALSE(BN_is_pow2(ten.get())); + bssl::UniquePtr forty_two(BN_new()); + ASSERT_TRUE(forty_two); + ASSERT_TRUE(BN_set_word(forty_two.get(), 42)); - // Make a wider version of |ten|. - EXPECT_TRUE(bn_resize_words(ten.get(), 4)); - EXPECT_EQ(4, ten->top); + bssl::UniquePtr two_exp_256(BN_new()); + ASSERT_TRUE(two_exp_256); + ASSERT_TRUE(BN_lshift(two_exp_256.get(), BN_value_one(), 256)); - // The same properties hold. - EXPECT_TRUE(BN_abs_is_word(ten.get(), 10)); - EXPECT_TRUE(BN_is_word(ten.get(), 10)); - EXPECT_EQ(10u, BN_get_word(ten.get())); - ASSERT_TRUE(BN_get_u64(ten.get(), &v)); - EXPECT_EQ(10u, v); - EXPECT_TRUE(BN_equal_consttime(ten.get(), ten_copy.get())); - EXPECT_EQ(BN_cmp(ten.get(), ten_copy.get()), 0); - EXPECT_FALSE(BN_equal_consttime(ten.get(), eight.get())); - EXPECT_LT(BN_cmp(eight.get(), ten.get()), 0); - EXPECT_EQ(4u, BN_num_bits(ten.get())); - EXPECT_EQ(1u, BN_num_bytes(ten.get())); - EXPECT_FALSE(BN_is_pow2(ten.get())); + // Check some comparison functions on |ten| before and after expanding. + for (size_t width = 1; width < 10; width++) { + SCOPED_TRACE(width); + // Make a wider version of |ten|. + EXPECT_TRUE(bn_resize_words(ten.get(), width)); + EXPECT_EQ(static_cast(width), ten->top); + + EXPECT_TRUE(BN_abs_is_word(ten.get(), 10)); + EXPECT_TRUE(BN_is_word(ten.get(), 10)); + EXPECT_EQ(10u, BN_get_word(ten.get())); + uint64_t v; + ASSERT_TRUE(BN_get_u64(ten.get(), &v)); + EXPECT_EQ(10u, v); + + EXPECT_TRUE(BN_equal_consttime(ten.get(), ten_copy.get())); + EXPECT_TRUE(BN_equal_consttime(ten_copy.get(), ten.get())); + EXPECT_FALSE(BN_less_than_consttime(ten.get(), ten_copy.get())); + EXPECT_FALSE(BN_less_than_consttime(ten_copy.get(), ten.get())); + EXPECT_EQ(BN_cmp(ten.get(), ten_copy.get()), 0); + + EXPECT_FALSE(BN_equal_consttime(ten.get(), eight.get())); + EXPECT_FALSE(BN_less_than_consttime(ten.get(), eight.get())); + EXPECT_TRUE(BN_less_than_consttime(eight.get(), ten.get())); + EXPECT_LT(BN_cmp(eight.get(), ten.get()), 0); + + EXPECT_FALSE(BN_equal_consttime(ten.get(), forty_two.get())); + EXPECT_TRUE(BN_less_than_consttime(ten.get(), forty_two.get())); + EXPECT_FALSE(BN_less_than_consttime(forty_two.get(), ten.get())); + EXPECT_GT(BN_cmp(forty_two.get(), ten.get()), 0); + + EXPECT_FALSE(BN_equal_consttime(ten.get(), two_exp_256.get())); + EXPECT_TRUE(BN_less_than_consttime(ten.get(), two_exp_256.get())); + EXPECT_FALSE(BN_less_than_consttime(two_exp_256.get(), ten.get())); + EXPECT_GT(BN_cmp(two_exp_256.get(), ten.get()), 0); + + EXPECT_EQ(4u, BN_num_bits(ten.get())); + EXPECT_EQ(1u, BN_num_bytes(ten.get())); + EXPECT_FALSE(BN_is_pow2(ten.get())); + } // |ten| may be resized back down to one word. EXPECT_TRUE(bn_resize_words(ten.get(), 1)); diff --git a/crypto/fipsmodule/bn/cmp.c b/crypto/fipsmodule/bn/cmp.c index 3a5bbb252..265c85260 100644 --- a/crypto/fipsmodule/bn/cmp.c +++ b/crypto/fipsmodule/bn/cmp.c @@ -177,19 +177,41 @@ int bn_cmp_part_words(const BN_ULONG *a, const BN_ULONG *b, int cl, int dl) { return bn_cmp_words(a, b, cl); } -int bn_less_than_words(const BN_ULONG *a, const BN_ULONG *b, size_t len) { +static int bn_less_than_words_impl(const BN_ULONG *a, size_t a_len, + const BN_ULONG *b, size_t b_len) { OPENSSL_COMPILE_ASSERT(sizeof(BN_ULONG) <= sizeof(crypto_word_t), crypto_word_t_too_small); int ret = 0; - // Process the words in little-endian order. - for (size_t i = 0; i < len; i++) { + // Process the common words in little-endian order. + size_t min = a_len < b_len ? a_len : b_len; + for (size_t i = 0; i < min; i++) { crypto_word_t eq = constant_time_eq_w(a[i], b[i]); crypto_word_t lt = constant_time_lt_w(a[i], b[i]); ret = constant_time_select_int(eq, ret, constant_time_select_int(lt, 1, 0)); } + + // If |a| or |b| has non-zero words beyond |min|, they take precedence. + if (a_len < b_len) { + crypto_word_t mask = 0; + for (size_t i = a_len; i < b_len; i++) { + mask |= b[i]; + } + ret = constant_time_select_int(constant_time_is_zero_w(mask), ret, 1); + } else if (b_len < a_len) { + crypto_word_t mask = 0; + for (size_t i = b_len; i < a_len; i++) { + mask |= a[i]; + } + ret = constant_time_select_int(constant_time_is_zero_w(mask), ret, 0); + } + return ret; } +int bn_less_than_words(const BN_ULONG *a, const BN_ULONG *b, size_t len) { + return bn_less_than_words_impl(a, len, b, len); +} + int BN_abs_is_word(const BIGNUM *bn, BN_ULONG w) { switch (bn_minimal_width(bn)) { case 1: @@ -261,3 +283,20 @@ int BN_equal_consttime(const BIGNUM *a, const BIGNUM *b) { mask |= (a->neg ^ b->neg); return mask == 0; } + +int BN_less_than_consttime(const BIGNUM *a, const BIGNUM *b) { + // We do not attempt to process the sign bit in constant time. Negative + // |BIGNUM|s should never occur in crypto, only calculators. + if (a->neg && !b->neg) { + return 1; + } + if (b->neg && !a->neg) { + return 0; + } + if (a->neg && b->neg) { + const BIGNUM *tmp = a; + a = b; + b = tmp; + } + return bn_less_than_words_impl(a->d, a->top, b->d, b->top); +} diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index b5a4e515b..1c7bcefa7 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -838,7 +838,8 @@ int rsa_greater_than_pow2(const BIGNUM *b, int n) { // relatively prime to |e|. If |p| is non-NULL, |out| will also not be close to // |p|. static int generate_prime(BIGNUM *out, int bits, const BIGNUM *e, - const BIGNUM *p, BN_CTX *ctx, BN_GENCB *cb) { + const BIGNUM *p, const BIGNUM *sqrt2, BN_CTX *ctx, + BN_GENCB *cb) { if (bits < 128 || (bits % BN_BITS2) != 0) { OPENSSL_PUT_ERROR(RSA, ERR_R_INTERNAL_ERROR); return 0; @@ -882,30 +883,14 @@ static int generate_prime(BIGNUM *out, int bits, const BIGNUM *e, } } - // If out < 2^(bits-1)×√2, try again (steps 4.4 and 5.5). - // - // We check the most significant words, so we retry if ⌊out/2^k⌋ <= ⌊b/2^k⌋, - // where b = 2^(bits-1)×√2 and k = max(0, bits - 1536). For key sizes up to - // 3072 (bits = 1536), k = 0, so we are testing that ⌊out⌋ <= ⌊b⌋. out is an - // integer and b is not, so this is equivalent to out < b. That is, the - // comparison is exact for FIPS key sizes. + // If out < 2^(bits-1)×√2, try again (steps 4.4 and 5.5). This is equivalent + // to out <= ⌊2^(bits-1)×√2⌋, or out <= sqrt2 for FIPS key sizes. // // For larger keys, the comparison is approximate, leaning towards // retrying. That is, we reject a negligible fraction of primes that are // within the FIPS bound, but we will never accept a prime outside the - // bound, ensuring the resulting RSA key is the right size. Specifically, if - // the FIPS bound holds, we have ⌊out/2^k⌋ < out/2^k < b/2^k. This implies - // ⌊out/2^k⌋ <= ⌊b/2^k⌋. That is, the FIPS bound implies our bound and so we - // are slightly tighter. - size_t out_len = (size_t)out->top; - assert(out_len == (size_t)bits / BN_BITS2); - size_t to_check = kBoringSSLRSASqrtTwoLen; - if (to_check > out_len) { - to_check = out_len; - } - if (!bn_less_than_words( - kBoringSSLRSASqrtTwo + kBoringSSLRSASqrtTwoLen - to_check, - out->d + out_len - to_check, to_check)) { + // bound, ensuring the resulting RSA key is the right size. + if (!BN_less_than_consttime(sqrt2, out)) { continue; } @@ -969,7 +954,9 @@ int RSA_generate_key_ex(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) { BIGNUM *pm1 = BN_CTX_get(ctx); BIGNUM *qm1 = BN_CTX_get(ctx); BIGNUM *gcd = BN_CTX_get(ctx); - if (totient == NULL || pm1 == NULL || qm1 == NULL || gcd == NULL) { + BIGNUM *sqrt2 = BN_CTX_get(ctx); + if (totient == NULL || pm1 == NULL || qm1 == NULL || gcd == NULL || + sqrt2 == NULL) { goto bn_err; } @@ -990,12 +977,35 @@ int RSA_generate_key_ex(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) { } int prime_bits = bits / 2; + + // Compute sqrt2 >= ⌊2^(prime_bits-1)×√2⌋. + if (!bn_set_words(sqrt2, kBoringSSLRSASqrtTwo, kBoringSSLRSASqrtTwoLen)) { + goto bn_err; + } + int sqrt2_bits = kBoringSSLRSASqrtTwoLen * BN_BITS2; + assert(sqrt2_bits == (int)BN_num_bits(sqrt2)); + if (sqrt2_bits > prime_bits) { + // For key sizes up to 3072 (prime_bits = 1536), this is exactly + // ⌊2^(prime_bits-1)×√2⌋. + if (!BN_rshift(sqrt2, sqrt2, sqrt2_bits - prime_bits)) { + goto bn_err; + } + } else if (prime_bits > sqrt2_bits) { + // For key sizes beyond 3072, this is approximate. We err towards retrying + // to ensure our key is the right size and round up. + if (!BN_add_word(sqrt2, 1) || + !BN_lshift(sqrt2, sqrt2, prime_bits - sqrt2_bits)) { + goto bn_err; + } + } + assert(prime_bits == (int)BN_num_bits(sqrt2)); + do { // Generate p and q, each of size |prime_bits|, using the steps outlined in // appendix FIPS 186-4 appendix B.3.3. - if (!generate_prime(rsa->p, prime_bits, rsa->e, NULL, ctx, cb) || + if (!generate_prime(rsa->p, prime_bits, rsa->e, NULL, sqrt2, ctx, cb) || !BN_GENCB_call(cb, 3, 0) || - !generate_prime(rsa->q, prime_bits, rsa->e, rsa->p, ctx, cb) || + !generate_prime(rsa->q, prime_bits, rsa->e, rsa->p, sqrt2, ctx, cb) || !BN_GENCB_call(cb, 3, 1)) { goto bn_err; } diff --git a/include/openssl/bn.h b/include/openssl/bn.h index bb32c2f56..becc95530 100644 --- a/include/openssl/bn.h +++ b/include/openssl/bn.h @@ -440,6 +440,11 @@ OPENSSL_EXPORT int BN_ucmp(const BIGNUM *a, const BIGNUM *b); // independent of the contents (including the signs) of |a| and |b|. OPENSSL_EXPORT int BN_equal_consttime(const BIGNUM *a, const BIGNUM *b); +// BN_less_than_consttime returns one if |a| is less than |b|, and zero +// otherwise. It takes an amount of time dependent on the sizes and signs of |a| +// and |b|, but independent of the contents of |a| and |b|. +OPENSSL_EXPORT int BN_less_than_consttime(const BIGNUM *a, const BIGNUM *b); + // BN_abs_is_word returns one if the absolute value of |bn| equals |w| and zero // otherwise. OPENSSL_EXPORT int BN_abs_is_word(const BIGNUM *bn, BN_ULONG w);