From b7ded430e4e32ecbd6716282e3d7278e9fc78fcd Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Tue, 11 Apr 2017 13:24:31 -0400
Subject: [PATCH] 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  | 10 ++++++++++
 crypto/rsa/rsa_test.cc | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/crypto/rsa/rsa_impl.c b/crypto/rsa/rsa_impl.c
index 6bb8ab8ac..325ccc746 100644
--- a/crypto/rsa/rsa_impl.c
+++ b/crypto/rsa/rsa_impl.c
@@ -771,6 +771,16 @@ static int ensure_bignum(BIGNUM **out) {
 }
 
 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;
diff --git a/crypto/rsa/rsa_test.cc b/crypto/rsa/rsa_test.cc
index c44131ec6..7f4197b23 100644
--- a/crypto/rsa/rsa_test.cc
+++ b/crypto/rsa/rsa_test.cc
@@ -805,3 +805,44 @@ TEST(RSATest, BadExponent) {
   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));
+}