kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix a bug in p224-64.c.

Browse Source 
p224_felem_neg does not produce an output within the tight bounds
suitable for p224_felem_contract. This was found by inspection of the
code.

This only affects the final y-coordinate output of arbitrary-point
multiplication, so it is a no-op for ECDH and ECDSA.

Change-Id: I1d929458d1f21d02cd8e745d2f0f7040a6bb0627
Reviewed-on: https://boringssl-review.googlesource.com/26847
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2018-03-23 16:32:48 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 72bc2328b1
commit 718c88c961
2 changed files with 32 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
crypto/fipsmodule/ec/ec_test.cc
View File

@ -674,6 +674,29 @@ TEST_P(ECCurveTest, DoubleSpecialCase) {
EXPECT_EQ(0, EC_POINT_cmp(group(), p.get(), two_g.get(), nullptr));
}
// This a regression test for a P-224 bug, but we may as well run it for all
// curves.
TEST_P(ECCurveTest, P224Bug) {
// P = -G
const EC_POINT *g = EC_GROUP_get0_generator(group());
bssl::UniquePtr<EC_POINT> p(EC_POINT_dup(g, group()));
ASSERT_TRUE(p);
ASSERT_TRUE(EC_POINT_invert(group(), p.get(), nullptr));
// Compute 31 * P + 32 * G = G
bssl::UniquePtr<EC_POINT> ret(EC_POINT_new(group()));
ASSERT_TRUE(ret);
bssl::UniquePtr<BIGNUM> bn31(BN_new()), bn32(BN_new());
ASSERT_TRUE(bn31);
ASSERT_TRUE(bn32);
ASSERT_TRUE(BN_set_word(bn31.get(), 31));
ASSERT_TRUE(BN_set_word(bn32.get(), 32));
ASSERT_TRUE(EC_POINT_mul(group(), ret.get(), bn32.get(), p.get(), bn31.get(),
nullptr));
EXPECT_EQ(0, EC_POINT_cmp(group(), ret.get(), g, nullptr));
}
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);

26
crypto/fipsmodule/ec/p224-64.c
View File

@ -257,23 +257,6 @@ static void p224_felem_sum(p224_felem out, const p224_felem in) {
out[3] += in[3];
}
// Get negative value: out = -in
// Assumes in[i] < 2^57
static void p224_felem_neg(p224_felem out, const p224_felem in) {
static const p224_limb two58p2 =
(((p224_limb)1) << 58) + (((p224_limb)1) << 2);
static const p224_limb two58m2 =
(((p224_limb)1) << 58) - (((p224_limb)1) << 2);
static const p224_limb two58m42m2 =
(((p224_limb)1) << 58) - (((p224_limb)1) << 42) - (((p224_limb)1) << 2);
// Set to 0 mod 2^224-2^96+1 to ensure out > in
out[0] = two58p2 - in[0];
out[1] = two58m42m2 - in[1];
out[2] = two58m2 - in[2];
out[3] = two58m2 - in[3];
}
// Subtract field elements: out -= in
// Assumes in[i] < 2^57
static void p224_felem_diff(p224_felem out, const p224_felem in) {
@ -513,6 +496,15 @@ static void p224_felem_contract(p224_felem out, const p224_felem in) {
out[3] = tmp[3];
}
// Get negative value: out = -in
// Requires in[i] < 2^63,
// ensures out[0] < 2^56, out[1] < 2^56, out[2] < 2^56, out[3] <= 2^56 + 2^16
static void p224_felem_neg(p224_felem out, const p224_felem in) {
p224_widefelem tmp = {0};
p224_felem_diff_128_64(tmp, in);
p224_felem_reduce(out, tmp);
}
// Zero-check: returns 1 if input is 0, and 0 otherwise. We know that field
// elements are reduced to in < 2^225, so we only need to check three cases: 0,
// 2^224 - 2^96 + 1, and 2^225 - 2^97 + 2