commit 8379978bc8f2c8939dc2c72d673f17c0fcfbcc84
author Adam Langley <alangley@gmail.com> Tue Jun 13 13:00:25 2017 -0700
committer Adam Langley <agl@google.com> Tue Jun 13 20:27:25 2017 +0000
tree 59f161a8cdc444ad0d28f4eaf097e8ffce0a1e28
parent 0a3663a64f00b6337ec80d78c8945f2c77c63dba

Allow |RSA_FLAG_NO_BLINDING| to be set with |e| set.

This change allows blinding to be disabled without also having to remove
|e|, which would disable the CRT and the glitch checks. This is to
support disabling blinding in the FIPS power-on tests.

(Note: the case where |e| isn't set is tested by RSATest.OnlyDGiven.)

Change-Id: I28f18beda33b1687bf145f4cbdfd37ce262dd70f
Reviewed-on: https://boringssl-review.googlesource.com/17146
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/fipsmodule/rsa/rsa_impl.c

diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c
index e09a37c..342f1c2 100644
--- a/crypto/fipsmodule/rsa/rsa_impl.c
+++ b/crypto/fipsmodule/rsa/rsa_impl.c
@@ -567,20 +567,18 @@
     goto err;
   }
 
-  /* We cannot do blinding or verification without |e|, and continuing without
-   * those countermeasures is dangerous. However, the Java/Android RSA API
-   * requires support for keys where only |d| and |n| (and not |e|) are known.
-   * The callers that require that bad behavior set |RSA_FLAG_NO_BLINDING|. */
-  int disable_security = (rsa->flags & RSA_FLAG_NO_BLINDING) && rsa->e == NULL;
+  const int do_blinding = (rsa->flags & RSA_FLAG_NO_BLINDING) == 0;
 
-  if (!disable_security) {
-    /* Keys without public exponents must have blinding explicitly disabled to
-     * be used. */
-    if (rsa->e == NULL) {
-      OPENSSL_PUT_ERROR(RSA, RSA_R_NO_PUBLIC_EXPONENT);
-      goto err;
-    }
+  if (rsa->e == NULL && do_blinding) {
+    /* We cannot do blinding or verification without |e|, and continuing without
+     * those countermeasures is dangerous. However, the Java/Android RSA API
+     * requires support for keys where only |d| and |n| (and not |e|) are known.
+     * The callers that require that bad behavior set |RSA_FLAG_NO_BLINDING|. */
+    OPENSSL_PUT_ERROR(RSA, RSA_R_NO_PUBLIC_EXPONENT);
+    goto err;
+  }
 
+  if (do_blinding) {
     blinding = rsa_blinding_get(rsa, &blinding_index, ctx);
     if (blinding == NULL) {
       OPENSSL_PUT_ERROR(RSA, ERR_R_INTERNAL_ERROR);
@@ -610,7 +608,7 @@
    * than the CRT attack, but there have likely been improvements since 1997.
    *
    * This check is cheap assuming |e| is small; it almost always is. */
-  if (!disable_security) {
+  if (rsa->e != NULL) {
     BIGNUM *vrfy = BN_CTX_get(ctx);
     if (vrfy == NULL ||
         !BN_mod_exp_mont(vrfy, result, rsa->e, rsa->n, ctx, rsa->mont_n) ||
@@ -619,9 +617,11 @@
       goto err;
     }
 
-    if (!BN_BLINDING_invert(result, blinding, rsa->mont_n, ctx)) {
-      goto err;
-    }
+  }
+
+  if (do_blinding &&
+      !BN_BLINDING_invert(result, blinding, rsa->mont_n, ctx)) {
+    goto err;
   }
 
   if (!BN_bn2bin_padded(out, len, result)) {

crypto/rsa_extra/rsa_test.cc

diff --git a/crypto/rsa_extra/rsa_test.cc b/crypto/rsa_extra/rsa_test.cc
index a53d04d..162ac05 100644
--- a/crypto/rsa_extra/rsa_test.cc
+++ b/crypto/rsa_extra/rsa_test.cc
@@ -679,6 +679,24 @@
   EXPECT_EQ(1152u, BN_num_bits(rsa->n));
 }
 
+TEST(RSATest, BlindingDisabled) {
+  bssl::UniquePtr<RSA> rsa(
+      RSA_private_key_from_bytes(kTwoPrimeKey, sizeof(kTwoPrimeKey) - 1));
+  ASSERT_TRUE(rsa);
+
+  rsa->flags |= RSA_FLAG_NO_BLINDING;
+
+  uint8_t sig[256];
+  ASSERT_GE(sizeof(sig), RSA_size(rsa.get()));
+
+  static const uint8_t kZeros[32] = {0};
+  unsigned sig_len;
+  ASSERT_TRUE(
+      RSA_sign(NID_sha256, kZeros, sizeof(kZeros), sig, &sig_len, rsa.get()));
+  EXPECT_TRUE(
+      RSA_verify(NID_sha256, kZeros, sizeof(kZeros), sig, sig_len, rsa.get()));
+}
+
 #if !defined(BORINGSSL_SHARED_LIBRARY)
 TEST(RSATest, SqrtTwo) {
   bssl::UniquePtr<BIGNUM> sqrt(BN_new()), pow2(BN_new());