Contract P-224 elements before returning them. cfd50c63 switched to using the add/dbl of p224_64.c, but the outputs weren't contracted before being returned and could be out of range, giving invalid results. Change-Id: I3cc295c7ddbff43375770dbafe73b37a668e4e6b Reviewed-on: https://boringssl-review.googlesource.com/c/33184 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index 87146ad..97c6d45 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -754,6 +754,17 @@ EXPECT_EQ(0, EC_POINT_cmp(group(), ret.get(), g, nullptr)); } +TEST_P(ECCurveTest, GPlusMinusG) { + 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)); + bssl::UniquePtr<EC_POINT> sum(EC_POINT_new(group())); + + ASSERT_TRUE(EC_POINT_add(group(), sum.get(), g, p.get(), nullptr)); + EXPECT_TRUE(EC_POINT_is_at_infinity(group(), sum.get())); +} + 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/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c index 49d5328..96f7bb7 100644 --- a/crypto/fipsmodule/ec/p224-64.c +++ b/crypto/fipsmodule/ec/p224-64.c
@@ -1011,8 +1011,12 @@ p224_generic_to_felem(y2, &b->Y); p224_generic_to_felem(z2, &b->Z); p224_point_add(x1, y1, z1, x1, y1, z1, 0 /* both Jacobian */, x2, y2, z2); + // The outputs are already reduced, but still need to be contracted. + p224_felem_contract(x1, x1); p224_felem_to_generic(&r->X, x1); + p224_felem_contract(y1, y1); p224_felem_to_generic(&r->Y, y1); + p224_felem_contract(z1, z1); p224_felem_to_generic(&r->Z, z1); } @@ -1023,8 +1027,12 @@ p224_generic_to_felem(y, &a->Y); p224_generic_to_felem(z, &a->Z); p224_point_double(x, y, z, x, y, z); + // The outputs are already reduced, but still need to be contracted. + p224_felem_contract(x, x); p224_felem_to_generic(&r->X, x); + p224_felem_contract(y, y); p224_felem_to_generic(&r->Y, y); + p224_felem_contract(z, z); p224_felem_to_generic(&r->Z, z); }