diff options
Diffstat (limited to 'src/crypto/fipsmodule')
-rwxr-xr-x | src/crypto/fipsmodule/bn/asm/rsaz-avx2.pl | 15 | ||||
-rw-r--r-- | src/crypto/fipsmodule/bn/bn_tests.txt | 6 | ||||
-rw-r--r-- | src/crypto/fipsmodule/ec/ec.c | 38 | ||||
-rw-r--r-- | src/crypto/fipsmodule/ec/ec_key.c | 9 | ||||
-rw-r--r-- | src/crypto/fipsmodule/ec/ec_test.cc | 19 | ||||
-rw-r--r-- | src/crypto/fipsmodule/ec/internal.h | 16 | ||||
-rw-r--r-- | src/crypto/fipsmodule/ecdsa/ecdsa.c | 109 |
7 files changed, 139 insertions, 73 deletions
diff --git a/src/crypto/fipsmodule/bn/asm/rsaz-avx2.pl b/src/crypto/fipsmodule/bn/asm/rsaz-avx2.pl index 0bb50cdb..32c21673 100755 --- a/src/crypto/fipsmodule/bn/asm/rsaz-avx2.pl +++ b/src/crypto/fipsmodule/bn/asm/rsaz-avx2.pl @@ -232,7 +232,7 @@ $code.=<<___; vmovdqu 32*8-128($ap), $ACC8 lea 192(%rsp), $tp0 # 64+128=192 - vpbroadcastq .Land_mask(%rip), $AND_MASK + vmovdqu .Land_mask(%rip), $AND_MASK jmp .LOOP_GRANDE_SQR_1024 .align 32 @@ -1082,10 +1082,10 @@ $code.=<<___; vpmuludq 32*6-128($np),$Yi,$TEMP1 vpaddq $TEMP1,$ACC6,$ACC6 vpmuludq 32*7-128($np),$Yi,$TEMP2 - vpblendd \$3, $ZERO, $ACC9, $ACC9 # correct $ACC3 + vpblendd \$3, $ZERO, $ACC9, $TEMP1 # correct $ACC3 vpaddq $TEMP2,$ACC7,$ACC7 vpmuludq 32*8-128($np),$Yi,$TEMP0 - vpaddq $ACC9, $ACC3, $ACC3 # correct $ACC3 + vpaddq $TEMP1, $ACC3, $ACC3 # correct $ACC3 vpaddq $TEMP0,$ACC8,$ACC8 mov %rbx, %rax @@ -1098,7 +1098,9 @@ $code.=<<___; vmovdqu -8+32*2-128($ap),$TEMP2 mov $r1, %rax + vpblendd \$0xfc, $ZERO, $ACC9, $ACC9 # correct $ACC3 imull $n0, %eax + vpaddq $ACC9,$ACC4,$ACC4 # correct $ACC3 and \$0x1fffffff, %eax imulq 16-128($ap),%rbx @@ -1334,15 +1336,12 @@ ___ # But as we underutilize resources, it's possible to correct in # each iteration with marginal performance loss. But then, as # we do it in each iteration, we can correct less digits, and -# avoid performance penalties completely. Also note that we -# correct only three digits out of four. This works because -# most significant digit is subjected to less additions. +# avoid performance penalties completely. $TEMP0 = $ACC9; $TEMP3 = $Bi; $TEMP4 = $Yi; $code.=<<___; - vpermq \$0, $AND_MASK, $AND_MASK vpaddq (%rsp), $TEMP1, $ACC0 vpsrlq \$29, $ACC0, $TEMP1 @@ -1790,7 +1789,7 @@ $code.=<<___; .align 64 .Land_mask: - .quad 0x1fffffff,0x1fffffff,0x1fffffff,-1 + .quad 0x1fffffff,0x1fffffff,0x1fffffff,0x1fffffff .Lscatter_permd: .long 0,2,4,6,7,7,7,7 .Lgather_permd: diff --git a/src/crypto/fipsmodule/bn/bn_tests.txt b/src/crypto/fipsmodule/bn/bn_tests.txt index eb447b53..87e64e2b 100644 --- a/src/crypto/fipsmodule/bn/bn_tests.txt +++ b/src/crypto/fipsmodule/bn/bn_tests.txt @@ -10507,6 +10507,12 @@ A = -80000000000000000000000000000000000000000000000000000000000000000000000000 E = 61803d4973ae68cfb2ba6770dbed70d36760fa42c01a16d1482eacf0d01adf7a917bc86ece58a73b920295c1291b90f49167ef856ecad149330e1fd49ec71392fb62d47270b53e6d4f3c8f044b80a5736753364896932abc6d872c4c5e135d1edb200597a93ceb262ff6c99079177cd10808b9ed20c8cd7352d80ac7f6963103 M = b5d257b2c50b050d42f0852eff5cfa2571157c500cd0bd9aa0b2ccdd89c531c9609d520eb81d928fb52b06da25dc713561aa0bd365ee56db9e62ac6787a85936990f44438363560f7af9e0c16f378e5b83f658252390d849401817624da97ec613a1b855fd901847352f434a777e4e32af0cb4033c7547fb6437d067fcd3d965 +# Regression test for CVE-2017-3738. +ModExp = d360792bd8210786607817c3dda64cc38c8d0f25569597cb1f363c7919a0c3587baff01a2283edaeb04fc288ac0ab3f279b2a89ffcb452d8bdf72422a9f9780f4aa702dc964cf033149d3a339883062cab8564aebdbfac0bf68985e522c6fe545b346044690c525ca85d3f4eb3e3c25cdf541545afc84a309e9b1d7807003461 +A = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff2020202020df +E = 2020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020FF2020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020 +M = ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff2020202020ff + # Exp tests. # diff --git a/src/crypto/fipsmodule/ec/ec.c b/src/crypto/fipsmodule/ec/ec.c index 266baa24..41c2540b 100644 --- a/src/crypto/fipsmodule/ec/ec.c +++ b/src/crypto/fipsmodule/ec/ec.c @@ -819,20 +819,22 @@ int EC_POINT_invert(const EC_GROUP *group, EC_POINT *a, BN_CTX *ctx) { static int arbitrary_bignum_to_scalar(const EC_GROUP *group, EC_SCALAR *out, const BIGNUM *in, BN_CTX *ctx) { - const BIGNUM *order = EC_GROUP_get0_order(group); - if (BN_is_negative(in) || BN_num_bits(in) > BN_num_bits(order)) { - // This is an unusual input, so we do not guarantee constant-time - // processing, even ignoring |bn_correct_top|. - BN_CTX_start(ctx); - BIGNUM *tmp = BN_CTX_get(ctx); - int ok = tmp != NULL && - BN_nnmod(tmp, in, order, ctx) && - ec_bignum_to_scalar(group, out, tmp); - BN_CTX_end(ctx); - return ok; + if (ec_bignum_to_scalar(group, out, in)) { + return 1; } - return ec_bignum_to_scalar(group, out, in); + ERR_clear_error(); + + // This is an unusual input, so we do not guarantee constant-time + // processing, even ignoring |bn_correct_top|. + const BIGNUM *order = &group->order; + BN_CTX_start(ctx); + BIGNUM *tmp = BN_CTX_get(ctx); + int ok = tmp != NULL && + BN_nnmod(tmp, in, order, ctx) && + ec_bignum_to_scalar_unchecked(group, out, tmp); + BN_CTX_end(ctx); + return ok; } int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar, @@ -943,6 +945,18 @@ size_t EC_get_builtin_curves(EC_builtin_curve *out_curves, int ec_bignum_to_scalar(const EC_GROUP *group, EC_SCALAR *out, const BIGNUM *in) { + if (!ec_bignum_to_scalar_unchecked(group, out, in)) { + return 0; + } + if (!bn_less_than_words(out->words, group->order.d, group->order.top)) { + OPENSSL_PUT_ERROR(EC, EC_R_INVALID_SCALAR); + return 0; + } + return 1; +} + +int ec_bignum_to_scalar_unchecked(const EC_GROUP *group, EC_SCALAR *out, + const BIGNUM *in) { if (BN_is_negative(in) || in->top > group->order.top) { OPENSSL_PUT_ERROR(EC, EC_R_INVALID_SCALAR); return 0; diff --git a/src/crypto/fipsmodule/ec/ec_key.c b/src/crypto/fipsmodule/ec/ec_key.c index bba4402b..f64cb21e 100644 --- a/src/crypto/fipsmodule/ec/ec_key.c +++ b/src/crypto/fipsmodule/ec/ec_key.c @@ -242,7 +242,8 @@ int EC_KEY_set_group(EC_KEY *key, const EC_GROUP *group) { } // XXX: |BN_cmp| is not constant time. if (key->priv_key != NULL && - BN_cmp(key->priv_key, EC_GROUP_get0_order(group)) >= 0) { + (BN_is_negative(key->priv_key) || + BN_cmp(key->priv_key, EC_GROUP_get0_order(group)) >= 0)) { return 0; } return 1; @@ -255,7 +256,8 @@ const BIGNUM *EC_KEY_get0_private_key(const EC_KEY *key) { int EC_KEY_set_private_key(EC_KEY *key, const BIGNUM *priv_key) { // XXX: |BN_cmp| is not constant time. if (key->group != NULL && - BN_cmp(priv_key, EC_GROUP_get0_order(key->group)) >= 0) { + (BN_is_negative(priv_key) || + BN_cmp(priv_key, EC_GROUP_get0_order(key->group)) >= 0)) { OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); return 0; } @@ -318,7 +320,8 @@ int EC_KEY_check_key(const EC_KEY *eckey) { // check if generator * priv_key == pub_key if (eckey->priv_key) { // XXX: |BN_cmp| is not constant time. - if (BN_cmp(eckey->priv_key, EC_GROUP_get0_order(eckey->group)) >= 0) { + if (BN_is_negative(eckey->priv_key) || + BN_cmp(eckey->priv_key, EC_GROUP_get0_order(eckey->group)) >= 0) { OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); goto err; } diff --git a/src/crypto/fipsmodule/ec/ec_test.cc b/src/crypto/fipsmodule/ec/ec_test.cc index 139840e5..8e7a81d9 100644 --- a/src/crypto/fipsmodule/ec/ec_test.cc +++ b/src/crypto/fipsmodule/ec/ec_test.cc @@ -510,6 +510,25 @@ TEST_P(ECCurveTest, Mul) { EXPECT_EQ(0, EC_POINT_cmp(group.get(), result.get(), generator, nullptr)); } +// Test that EC_KEY_set_private_key rejects invalid values. +TEST_P(ECCurveTest, SetInvalidPrivateKey) { + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + ASSERT_TRUE(key); + + bssl::UniquePtr<BIGNUM> bn(BN_new()); + ASSERT_TRUE(BN_one(bn.get())); + BN_set_negative(bn.get(), 1); + EXPECT_FALSE(EC_KEY_set_private_key(key.get(), bn.get())) + << "Unexpectedly set a key of -1"; + ERR_clear_error(); + + ASSERT_TRUE( + BN_copy(bn.get(), EC_GROUP_get0_order(EC_KEY_get0_group(key.get())))); + EXPECT_FALSE(EC_KEY_set_private_key(key.get(), bn.get())) + << "Unexpectedly set a key of the group order."; + ERR_clear_error(); +} + static std::vector<EC_builtin_curve> AllCurves() { const size_t num_curves = EC_get_builtin_curves(nullptr, 0); std::vector<EC_builtin_curve> curves(num_curves); diff --git a/src/crypto/fipsmodule/ec/internal.h b/src/crypto/fipsmodule/ec/internal.h index 7374e8b5..1b860c6b 100644 --- a/src/crypto/fipsmodule/ec/internal.h +++ b/src/crypto/fipsmodule/ec/internal.h @@ -91,10 +91,9 @@ extern "C" { OPENSSL_COMPILE_ASSERT(EC_MAX_SCALAR_WORDS <= BN_SMALL_MAX_WORDS, bn_small_functions_applicable); -// An EC_SCALAR is a |BN_num_bits(order)|-bit integer. Only the first +// An EC_SCALAR is an integer fully reduced modulo the order. Only the first // |order->top| words are used. An |EC_SCALAR| is specific to an |EC_GROUP| and -// must not be mixed between groups. Unless otherwise specified, it is fully -// reduced modulo the |order|. +// must not be mixed between groups. typedef union { // bytes is the representation of the scalar in little-endian order. uint8_t bytes[EC_MAX_SCALAR_BYTES]; @@ -173,13 +172,16 @@ struct ec_point_st { EC_GROUP *ec_group_new(const EC_METHOD *meth); -// ec_bignum_to_scalar converts |in| to an |EC_SCALAR| and writes it to |*out|. -// |in| must be non-negative and have at most |BN_num_bits(&group->order)| bits. -// It returns one on success and zero on error. It does not ensure |in| is fully -// reduced. +// ec_bignum_to_scalar converts |in| to an |EC_SCALAR| and writes it to +// |*out|. It returns one on success and zero if |in| is out of range. int ec_bignum_to_scalar(const EC_GROUP *group, EC_SCALAR *out, const BIGNUM *in); +// ec_bignum_to_scalar_unchecked behaves like |ec_bignum_to_scalar| but does not +// check |in| is fully reduced. +int ec_bignum_to_scalar_unchecked(const EC_GROUP *group, EC_SCALAR *out, + const BIGNUM *in); + // ec_random_nonzero_scalar sets |out| to a uniformly selected random value from // 1 to |group->order| - 1. It returns one on success and zero on error. int ec_random_nonzero_scalar(const EC_GROUP *group, EC_SCALAR *out, diff --git a/src/crypto/fipsmodule/ecdsa/ecdsa.c b/src/crypto/fipsmodule/ecdsa/ecdsa.c index 319a934e..6571c941 100644 --- a/src/crypto/fipsmodule/ecdsa/ecdsa.c +++ b/src/crypto/fipsmodule/ecdsa/ecdsa.c @@ -66,10 +66,53 @@ #include "../../internal.h" +// EC_LOOSE_SCALAR is like |EC_SCALAR| but is bounded by 2^|BN_num_bits(order)| +// rather than |order|. +typedef union { + // bytes is the representation of the scalar in little-endian order. + uint8_t bytes[EC_MAX_SCALAR_BYTES]; + BN_ULONG words[EC_MAX_SCALAR_WORDS]; +} EC_LOOSE_SCALAR; + +static void scalar_add_loose(const EC_GROUP *group, EC_LOOSE_SCALAR *r, + const EC_LOOSE_SCALAR *a, const EC_SCALAR *b) { + // Add and subtract one copy of |order| if necessary. We have: + // |a| + |b| < 2^BN_num_bits(order) + order + // so this leaves |r| < 2^BN_num_bits(order). + const BIGNUM *order = &group->order; + BN_ULONG carry = bn_add_words(r->words, a->words, b->words, order->top); + EC_LOOSE_SCALAR tmp; + BN_ULONG v = bn_sub_words(tmp.words, r->words, order->d, order->top) - carry; + v = 0u - v; + for (int i = 0; i < order->top; i++) { + OPENSSL_COMPILE_ASSERT(sizeof(BN_ULONG) <= sizeof(crypto_word_t), + crypto_word_t_too_small); + r->words[i] = constant_time_select_w(v, r->words[i], tmp.words[i]); + } +} + +static int scalar_mod_mul_montgomery(const EC_GROUP *group, EC_SCALAR *r, + const EC_SCALAR *a, const EC_SCALAR *b) { + const BIGNUM *order = &group->order; + return bn_mod_mul_montgomery_small(r->words, order->top, a->words, order->top, + b->words, order->top, group->order_mont); +} + +static int scalar_mod_mul_montgomery_loose(const EC_GROUP *group, EC_SCALAR *r, + const EC_LOOSE_SCALAR *a, + const EC_SCALAR *b) { + // Although |a| is loose, |bn_mod_mul_montgomery_small| only requires the + // product not exceed R * |order|. |b| is fully reduced and |a| < + // 2^BN_num_bits(order) <= R, so this holds. + const BIGNUM *order = &group->order; + return bn_mod_mul_montgomery_small(r->words, order->top, a->words, order->top, + b->words, order->top, group->order_mont); +} + // digest_to_scalar interprets |digest_len| bytes from |digest| as a scalar for // ECDSA. Note this value is not fully reduced modulo the order, only the // correct number of bits. -static void digest_to_scalar(const EC_GROUP *group, EC_SCALAR *out, +static void digest_to_scalar(const EC_GROUP *group, EC_LOOSE_SCALAR *out, const uint8_t *digest, size_t digest_len) { const BIGNUM *order = &group->order; size_t num_bits = BN_num_bits(order); @@ -195,15 +238,12 @@ int ECDSA_do_verify(const uint8_t *digest, size_t digest_len, goto err; } - EC_SCALAR r, s, m, u1, u2, s_inv_mont; + EC_SCALAR r, s, u1, u2, s_inv_mont; + EC_LOOSE_SCALAR m; const BIGNUM *order = EC_GROUP_get0_order(group); if (BN_is_zero(sig->r) || - BN_is_negative(sig->r) || - BN_ucmp(sig->r, order) >= 0 || !ec_bignum_to_scalar(group, &r, sig->r) || BN_is_zero(sig->s) || - BN_is_negative(sig->s) || - BN_ucmp(sig->s, order) >= 0 || !ec_bignum_to_scalar(group, &s, sig->s)) { OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_BAD_SIGNATURE); goto err; @@ -212,26 +252,21 @@ int ECDSA_do_verify(const uint8_t *digest, size_t digest_len, // the products below. int no_inverse; if (!BN_mod_inverse_odd(X, &no_inverse, sig->s, order, ctx) || - !ec_bignum_to_scalar(group, &s_inv_mont, X) || + // TODO(davidben): Add a words version of |BN_mod_inverse_odd| and write + // into |s_inv_mont| directly. + !ec_bignum_to_scalar_unchecked(group, &s_inv_mont, X) || !bn_to_montgomery_small(s_inv_mont.words, order->top, s_inv_mont.words, order->top, group->order_mont)) { goto err; } - // u1 = m * s_inv_mont mod order - // u2 = r * s_inv_mont mod order + // u1 = m * s^-1 mod order + // u2 = r * s^-1 mod order // // |s_inv_mont| is in Montgomery form while |m| and |r| are not, so |u1| and - // |u2| will be taken out of Montgomery form, as desired. Note that, although - // |m| is not fully reduced, |bn_mod_mul_montgomery_small| only requires the - // product not exceed R * |order|. |s_inv_mont| is fully reduced and |m| < - // 2^BN_num_bits(order) <= R, so this holds. + // |u2| will be taken out of Montgomery form, as desired. digest_to_scalar(group, &m, digest, digest_len); - if (!bn_mod_mul_montgomery_small(u1.words, order->top, m.words, order->top, - s_inv_mont.words, order->top, - group->order_mont) || - !bn_mod_mul_montgomery_small(u2.words, order->top, r.words, order->top, - s_inv_mont.words, order->top, - group->order_mont)) { + if (!scalar_mod_mul_montgomery_loose(group, &u1, &m, &s_inv_mont) || + !scalar_mod_mul_montgomery(group, &u2, &r, &s_inv_mont)) { goto err; } @@ -368,14 +403,17 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len, int ok = 0; ECDSA_SIG *ret = ECDSA_SIG_new(); BN_CTX *ctx = BN_CTX_new(); - EC_SCALAR kinv_mont, priv_key, r_mont, s, tmp, m; + EC_SCALAR kinv_mont, priv_key, r_mont, s; + EC_LOOSE_SCALAR m, tmp; if (ret == NULL || ctx == NULL) { OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE); return NULL; } digest_to_scalar(group, &m, digest, digest_len); - if (!ec_bignum_to_scalar(group, &priv_key, priv_key_bn)) { + // TODO(davidben): Store the private key as an |EC_SCALAR| so this is obvious + // via the type system. + if (!ec_bignum_to_scalar_unchecked(group, &priv_key, priv_key_bn)) { goto err; } for (;;) { @@ -385,36 +423,21 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len, } // Compute priv_key * r (mod order). Note if only one parameter is in the - // Montgomery domain, |bn_mod_mul_montgomery_small| will compute the answer - // in the normal domain. + // Montgomery domain, |scalar_mod_mul_montgomery| will compute the answer in + // the normal domain. if (!ec_bignum_to_scalar(group, &r_mont, ret->r) || !bn_to_montgomery_small(r_mont.words, order->top, r_mont.words, order->top, group->order_mont) || - !bn_mod_mul_montgomery_small(s.words, order->top, priv_key.words, - order->top, r_mont.words, order->top, - group->order_mont)) { + !scalar_mod_mul_montgomery(group, &s, &priv_key, &r_mont)) { goto err; } - // Compute s += m in constant time. Reduce one copy of |order| if necessary. - // Note this does not leave |s| fully reduced. We have - // |m| < 2^BN_num_bits(order), so subtracting |order| leaves - // 0 <= |s| < 2^BN_num_bits(order). - BN_ULONG carry = bn_add_words(s.words, s.words, m.words, order->top); - BN_ULONG v = bn_sub_words(tmp.words, s.words, order->d, order->top) - carry; - v = 0u - v; - for (int i = 0; i < order->top; i++) { - s.words[i] = constant_time_select_w(v, s.words[i], tmp.words[i]); - } + // Compute tmp = m + priv_key * r. + scalar_add_loose(group, &tmp, &m, &s); // Finally, multiply s by k^-1. That was retained in Montgomery form, so the - // same technique as the previous multiplication works. Although the - // previous step did not fully reduce |s|, |bn_mod_mul_montgomery_small| - // only requires the product not exceed R * |order|. |kinv_mont| is fully - // reduced and |s| < 2^BN_num_bits(order) <= R, so this holds. - if (!bn_mod_mul_montgomery_small(s.words, order->top, s.words, order->top, - kinv_mont.words, order->top, - group->order_mont) || + // same technique as the previous multiplication works. + if (!scalar_mod_mul_montgomery_loose(group, &s, &tmp, &kinv_mont) || !bn_set_words(ret->s, s.words, order->top)) { goto err; } |