From 8379978bc8f2c8939dc2c72d673f17c0fcfbcc84 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 13 Jun 2017 13:00:25 -0700 Subject: [PATCH] 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 Commit-Queue: Adam Langley Reviewed-by: David Benjamin Reviewed-by: Adam Langley --- crypto/fipsmodule/rsa/rsa_impl.c | 32 ++++++++++++++++---------------- crypto/rsa_extra/rsa_test.cc | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/crypto/fipsmodule/rsa/rsa_impl.c b/crypto/fipsmodule/rsa/rsa_impl.c index e09a37cf..342f1c2c 100644 --- a/crypto/fipsmodule/rsa/rsa_impl.c +++ b/crypto/fipsmodule/rsa/rsa_impl.c @@ -567,20 +567,18 @@ int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in, 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 @@ int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in, * 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 @@ int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in, 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)) { diff --git a/crypto/rsa_extra/rsa_test.cc b/crypto/rsa_extra/rsa_test.cc index a53d04d1..162ac055 100644 --- a/crypto/rsa_extra/rsa_test.cc +++ b/crypto/rsa_extra/rsa_test.cc @@ -679,6 +679,24 @@ TEST(RSATest, RoundKeyLengths) { EXPECT_EQ(1152u, BN_num_bits(rsa->n)); } +TEST(RSATest, BlindingDisabled) { + bssl::UniquePtr 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 sqrt(BN_new()), pow2(BN_new());