From cb4e300f176429d091e07cbdb420748b7900f271 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 22 Jan 2018 09:28:13 -0500 Subject: [PATCH] Store EC field and orders in minimal form. The order (and later the field) are used to size stack-allocated fixed width word arrays. They're also entirely public, so this is fine. Bug: 232 Change-Id: Ie98869cdbbdfea92dcad64a300f7e0b47bef6bf2 Reviewed-on: https://boringssl-review.googlesource.com/25256 Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Adam Langley --- crypto/fipsmodule/ec/ec.c | 2 ++ crypto/fipsmodule/ec/ec_test.cc | 23 +++++++++++++++++++++++ crypto/fipsmodule/ec/simple.c | 6 ++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c index 616df162a..cc1e633ca 100644 --- a/crypto/fipsmodule/ec/ec.c +++ b/crypto/fipsmodule/ec/ec.c @@ -387,6 +387,8 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator, EC_POINT_free(copy); return 0; } + // Store the order in minimal form, so it can be used with |BN_ULONG| arrays. + bn_correct_top(&group->order); BN_MONT_CTX_free(group->order_mont); group->order_mont = BN_MONT_CTX_new_for_modulus(&group->order, NULL); diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index 8a215e913..260b9725f 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc @@ -304,6 +304,29 @@ TEST(ECTest, ArbitraryCurve) { order.get(), BN_value_one())); EXPECT_NE(0, EC_GROUP_cmp(group.get(), group3.get(), NULL)); + +#if !defined(BORINGSSL_SHARED_LIBRARY) + // group4 has non-minimal components that do not fit in |EC_SCALAR| and the + // future |EC_FELEM|. + ASSERT_TRUE(bn_resize_words(p.get(), 32)); + ASSERT_TRUE(bn_resize_words(a.get(), 32)); + ASSERT_TRUE(bn_resize_words(b.get(), 32)); + ASSERT_TRUE(bn_resize_words(gx.get(), 32)); + ASSERT_TRUE(bn_resize_words(gy.get(), 32)); + ASSERT_TRUE(bn_resize_words(order.get(), 32)); + + bssl::UniquePtr group4( + EC_GROUP_new_curve_GFp(p.get(), a.get(), b.get(), ctx.get())); + ASSERT_TRUE(group4); + bssl::UniquePtr generator4(EC_POINT_new(group4.get())); + ASSERT_TRUE(generator4); + ASSERT_TRUE(EC_POINT_set_affine_coordinates_GFp( + group4.get(), generator4.get(), gx.get(), gy.get(), ctx.get())); + ASSERT_TRUE(EC_GROUP_set_generator(group4.get(), generator4.get(), + order.get(), BN_value_one())); + + EXPECT_EQ(0, EC_GROUP_cmp(group.get(), group4.get(), NULL)); +#endif } TEST(ECTest, SetKeyWithoutGroup) { diff --git a/crypto/fipsmodule/ec/simple.c b/crypto/fipsmodule/ec/simple.c index ab011ca3f..8bc20a416 100644 --- a/crypto/fipsmodule/ec/simple.c +++ b/crypto/fipsmodule/ec/simple.c @@ -135,9 +135,11 @@ int ec_GFp_simple_group_set_curve(EC_GROUP *group, const BIGNUM *p, goto err; } BN_set_negative(&group->field, 0); + // Store the field in minimal form, so it can be used with |BN_ULONG| arrays. + bn_correct_top(&group->field); // group->a - if (!BN_nnmod(tmp_a, a, p, ctx)) { + if (!BN_nnmod(tmp_a, a, &group->field, ctx)) { goto err; } if (group->meth->field_encode) { @@ -149,7 +151,7 @@ int ec_GFp_simple_group_set_curve(EC_GROUP *group, const BIGNUM *p, } // group->b - if (!BN_nnmod(&group->b, b, p, ctx)) { + if (!BN_nnmod(&group->b, b, &group->field, ctx)) { goto err; } if (group->meth->field_encode &&