commit b7ded430e4e32ecbd6716282e3d7278e9fc78fcd
author David Benjamin <davidben@google.com> Tue Apr 11 13:24:31 2017 -0400
committer Adam Langley <agl@google.com> Wed Apr 12 22:27:45 2017 +0000
tree b87445165b0af4cbf2cdec215f4f720e1e9d4802
parent ddd5ba78a949c5458aad9f7c3f19d214a2e8338e

Constrain RSA bit sizes.

The FIPS RSA generation algorithm is unkind to keys with funny bit
sizes. Odd numbers of bits are especially inconvenient, but the sqrt(2)
bound is much simpler if the key size is a multiple of 128 (thus giving
prime sizes a multiple of 64, so the sqrt(2) bound is easier to work
with).

Also impose a minimum RSA key size. 255-bit RSA is far too small as it
is and gives small enough primes that the p-q FIPS bound (2^(n/2-100))
starts risking underflow.

Change-Id: I4583c90b67385e53641ccee9b29044e79e94c920
Reviewed-on: https://boringssl-review.googlesource.com/14947
Reviewed-by: Adam Langley <agl@google.com>

crypto/rsa/rsa_impl.c

diff --git a/crypto/rsa/rsa_impl.c b/crypto/rsa/rsa_impl.c
index 6bb8ab8..325ccc7 100644
--- a/crypto/rsa/rsa_impl.c
+++ b/crypto/rsa/rsa_impl.c
@@ -771,6 +771,16 @@
 }
 
 int rsa_default_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) {
+  /* Always generate RSA keys which are a multiple of 128 bits. Round |bits|
+   * down as needed. */
+  bits &= ~127;
+
+  /* Reject excessively small keys. */
+  if (bits < 256) {
+    OPENSSL_PUT_ERROR(RSA, RSA_R_KEY_SIZE_TOO_SMALL);
+    return 0;
+  }
+
   BIGNUM *r0 = NULL, *r1 = NULL, *r2 = NULL, *r3 = NULL, *tmp;
   int ok = -1, n = 0;
   BN_CTX *ctx = NULL;

crypto/rsa/rsa_test.cc

diff --git a/crypto/rsa/rsa_test.cc b/crypto/rsa/rsa_test.cc
index c44131e..7f4197b 100644
--- a/crypto/rsa/rsa_test.cc
+++ b/crypto/rsa/rsa_test.cc
@@ -805,3 +805,44 @@
   EXPECT_FALSE(rsa);
   ERR_clear_error();
 }
+
+// Attempting to generate an excessively small key should fail.
+TEST(RSATest, GenerateSmallKey) {
+  bssl::UniquePtr<RSA> rsa(RSA_new());
+  ASSERT_TRUE(rsa);
+  bssl::UniquePtr<BIGNUM> e(BN_new());
+  ASSERT_TRUE(e);
+  ASSERT_TRUE(BN_set_word(e.get(), RSA_F4));
+
+  EXPECT_FALSE(RSA_generate_key_ex(rsa.get(), 255, e.get(), nullptr));
+  uint32_t err = ERR_get_error();
+  EXPECT_EQ(ERR_LIB_RSA, ERR_GET_LIB(err));
+  EXPECT_EQ(RSA_R_KEY_SIZE_TOO_SMALL, ERR_GET_REASON(err));
+}
+
+// Attempting to generate an funny RSA key length should round down.
+TEST(RSATest, RoundKeyLengths) {
+  bssl::UniquePtr<BIGNUM> e(BN_new());
+  ASSERT_TRUE(e);
+  ASSERT_TRUE(BN_set_word(e.get(), RSA_F4));
+
+  bssl::UniquePtr<RSA> rsa(RSA_new());
+  ASSERT_TRUE(rsa);
+  EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 1025, e.get(), nullptr));
+  EXPECT_EQ(1024u, BN_num_bits(rsa->n));
+
+  rsa.reset(RSA_new());
+  ASSERT_TRUE(rsa);
+  EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 1027, e.get(), nullptr));
+  EXPECT_EQ(1024u, BN_num_bits(rsa->n));
+
+  rsa.reset(RSA_new());
+  ASSERT_TRUE(rsa);
+  EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 1151, e.get(), nullptr));
+  EXPECT_EQ(1024u, BN_num_bits(rsa->n));
+
+  rsa.reset(RSA_new());
+  ASSERT_TRUE(rsa);
+  EXPECT_TRUE(RSA_generate_key_ex(rsa.get(), 1152, e.get(), nullptr));
+  EXPECT_EQ(1152u, BN_num_bits(rsa->n));
+}