From 0ab853aeaf771f1d1969e20d23b414f38f667025 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 29 Jul 2016 18:51:48 -1000 Subject: [PATCH] Require inputs to |BN_mod_inverse_*| to be reduced. --- crypto/bn/bn_test.cc | 75 ++++++++++++++++++++++++++++++++++++++++++ crypto/bn/bn_tests.txt | 25 +++++++------- crypto/bn/gcd.c | 28 ++++------------ crypto/bn/montgomery.c | 7 ++-- 4 files changed, 99 insertions(+), 36 deletions(-) diff --git a/crypto/bn/bn_test.cc b/crypto/bn/bn_test.cc index 82dbef3b3b..924d097224 100644 --- a/crypto/bn/bn_test.cc +++ b/crypto/bn/bn_test.cc @@ -912,6 +912,80 @@ static bool TestExpModRejectUnreduced(BN_CTX *ctx) { return true; } +static bool TestModInvRejectUnreduced(RAND *rng, BN_CTX *ctx) { + ScopedBIGNUM r(BN_new()); + if (!r) { + return false; + } + + static const BN_ULONG kBases[] = { 2, 4, 6 }; + static const BN_ULONG kModuli[] = { 1, 3 }; + + for (BN_ULONG mod_value : kModuli) { + ScopedBIGNUM mod(BN_new()); + ScopedBN_MONT_CTX mont(BN_MONT_CTX_new()); + if (!mod || + !BN_set_word(mod.get(), mod_value) || + !mont || + !BN_MONT_CTX_set(mont.get(), mod.get(), ctx)) { + return false; + } + for (BN_ULONG base_value : kBases) { + ScopedBIGNUM base(BN_new()); + if (!base || + !BN_set_word(base.get(), base_value)) { + return false; + } + + int no_inverse; + + if (base_value >= mod_value && + BN_mod_inverse(r.get(), base.get(), mod.get(), ctx) != NULL) { + fprintf(stderr, "BN_mod_inverse(%d, %d) succeeded!\n", + (int)base_value, (int)mod_value); + return false; + } + if (base_value >= mod_value && + BN_mod_inverse_no_branch(r.get(), &no_inverse, base.get(), mod.get(), + ctx) != NULL) { + fprintf(stderr, "BN_mod_inverse(%d, %d) succeeded!\n", + (int)base_value, (int)mod_value); + return false; + } + if (base_value >= mod_value && + BN_mod_inverse_blinded(r.get(), &no_inverse, base.get(), mont.get(), + rng, ctx)) { + fprintf(stderr, "BN_mod_inverse(%d, %d) succeeded!\n", + (int)base_value, (int)mod_value); + return false; + } + + BN_set_negative(base.get(), 1); + + if (BN_mod_inverse(r.get(), base.get(), mod.get(), ctx) != NULL) { + fprintf(stderr, "BN_mod_inverse(%d, %d) succeeded!\n", + -(int)base_value, (int)mod_value); + return false; + } + if (BN_mod_inverse_no_branch(r.get(), &no_inverse, base.get(), mod.get(), + ctx) != NULL) { + fprintf(stderr, "BN_mod_inverse_no_branch(%d, %d) succeeded!\n", + -(int)base_value, (int)mod_value); + return false; + } + if (BN_mod_inverse_blinded(r.get(), &no_inverse, base.get(), mont.get(), + rng, ctx)) { + fprintf(stderr, "BN_mod_inverse_blinded(%d, %d) succeeded!\n", + -(int)base_value, (int)mod_value); + return false; + } + + } + } + + return true; +} + static bool TestCmpWord() { static const BN_ULONG kMaxWord = (BN_ULONG)-1; @@ -996,6 +1070,7 @@ extern "C" int bssl_bn_test_main(RAND *rng) { !TestBadModulus(ctx.get()) || !TestExpModZero(rng) || !TestExpModRejectUnreduced(ctx.get()) || + !TestModInvRejectUnreduced(rng, ctx.get()) || !TestCmpWord()) { return 1; } diff --git a/crypto/bn/bn_tests.txt b/crypto/bn/bn_tests.txt index 46783bae0d..f37680f1bb 100644 --- a/crypto/bn/bn_tests.txt +++ b/crypto/bn/bn_tests.txt @@ -10366,14 +10366,17 @@ ModInv = 00 A = 00 M = 01 -ModInv = 00 -A = 01 -M = 01 - -ModInv = 00 -A = 02 -M = 01 - -ModInv = 00 -A = 03 -M = 01 +# Unreduced input is not allowed in *ring*. +# ModInv = 00 +# A = 01 +# M = 01 + +# Unreduced input is not allowed in *ring*. +# ModInv = 00 +# A = 02 +# M = 01 + +# Unreduced input is not allowed in *ring*. +# ModInv = 00 +# A = 03 +# M = 01 diff --git a/crypto/bn/gcd.c b/crypto/bn/gcd.c index 3526b82dcc..ed3d01be86 100644 --- a/crypto/bn/gcd.c +++ b/crypto/bn/gcd.c @@ -151,6 +151,10 @@ static BIGNUM *bn_mod_inverse_ex(BIGNUM *out, int *out_no_inverse, goto err; } A->neg = 0; + if (B->neg || (BN_ucmp(B, A) >= 0)) { + OPENSSL_PUT_ERROR(BN, BN_R_INPUT_NOT_REDUCED); + goto err; + } sign = -1; /* From B = a mod |n|, A = |n| it follows that * @@ -298,23 +302,7 @@ BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, const BIGNUM *n, return BN_mod_inverse_no_branch(out, &no_inverse, a, n, ctx); } - if (!a->neg && BN_ucmp(a, n) < 0) { - return bn_mod_inverse_ex(out, &no_inverse, a, n, ctx); - } - - BIGNUM a_reduced; - BN_init(&a_reduced); - BIGNUM *ret = NULL; - - if (!BN_nnmod(&a_reduced, a, n, ctx)) { - goto err; - } - - ret = bn_mod_inverse_ex(out, &no_inverse, &a_reduced, n, ctx); - -err: - BN_free(&a_reduced); - return ret; + return bn_mod_inverse_ex(out, &no_inverse, a, n, ctx); } int BN_mod_inverse_blinded(BIGNUM *out, int *out_no_inverse, const BIGNUM *a, @@ -384,10 +372,8 @@ BIGNUM *BN_mod_inverse_no_branch(BIGNUM *out, int *out_no_inverse, A->neg = 0; if (B->neg || (BN_ucmp(B, A) >= 0)) { - BN_set_flags(B, BN_FLG_CONSTTIME); - if (!BN_nnmod(B, B, A, ctx)) { - goto err; - } + OPENSSL_PUT_ERROR(BN, BN_R_INPUT_NOT_REDUCED); + goto err; } sign = -1; /* From B = a mod |n|, A = |n| it follows that diff --git a/crypto/bn/montgomery.c b/crypto/bn/montgomery.c index d5296f94fb..6ea457e77f 100644 --- a/crypto/bn/montgomery.c +++ b/crypto/bn/montgomery.c @@ -193,10 +193,9 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) { tmod.top = 2; } - if (BN_mod_inverse(Ri, R, &tmod, ctx) == NULL) { - goto err; - } - if (!BN_lshift(Ri, Ri, BN_MONT_CTX_N0_LIMBS * BN_BITS2)) { + if (!BN_mod(Ri, R, &tmod, ctx) || + BN_mod_inverse(Ri, Ri, &tmod, ctx) == NULL || + !BN_lshift(Ri, Ri, BN_MONT_CTX_N0_LIMBS * BN_BITS2)) { goto err; /* R*Ri */ } const BIGNUM *Ri_dividend;