commit 549b9024d42fa91dd2f4aa35cff41d03a0007710
author Adam Langley <agl@google.com> Wed Nov 14 13:56:37 2018 -0800
committer Adam Langley <agl@google.com> Wed Nov 14 22:38:12 2018 +0000
tree 93562b86970c2d0cd9dab5bdcba365465401beb2
parent e6eef1ca16a022e476bbaedffef044597cfc8f4b

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>

crypto/fipsmodule/ec/ec_test.cc

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);

crypto/fipsmodule/ec/p224-64.c

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);
 }