Fix typo in point_add.
Rather than writing the answer into the output, it wrote it into some
awkwardly-named temporaries. Thanks to Daniel Hirche for reporting this
issue!
Bug: chromium:825273
(cherry picked from commit 5fca61391822252baf3dc37529ba02f6d7611acf)
Change-Id: I315087f5e7414118a6f70bf4aed76325aa4f76a0
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc
index 923ce1e..55f4fa2 100644
--- a/crypto/fipsmodule/ec/ec_test.cc
+++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -28,8 +28,9 @@
#include <openssl/nid.h>
#include <openssl/obj.h>
-#include "../bn/internal.h"
#include "../../test/test_util.h"
+#include "../bn/internal.h"
+#include "internal.h"
// kECKeyWithoutPublic is an ECPrivateKey with the optional publicKey field
@@ -359,7 +360,18 @@
EC_KEY_set_public_key(key.get(), EC_GROUP_get0_generator(p256.get())));
}
-class ECCurveTest : public testing::TestWithParam<EC_builtin_curve> {};
+class ECCurveTest : public testing::TestWithParam<EC_builtin_curve> {
+ public:
+ const EC_GROUP *group() const { return group_.get(); }
+
+ void SetUp() override {
+ group_.reset(EC_GROUP_new_by_curve_name(GetParam().nid));
+ ASSERT_TRUE(group_);
+ }
+
+ private:
+ bssl::UniquePtr<EC_GROUP> group_;
+};
TEST_P(ECCurveTest, SetAffine) {
// Generate an EC_KEY.
@@ -367,9 +379,8 @@
ASSERT_TRUE(key);
ASSERT_TRUE(EC_KEY_generate_key(key.get()));
- const EC_GROUP *const group = EC_KEY_get0_group(key.get());
- EXPECT_TRUE(
- EC_POINT_is_on_curve(group, EC_KEY_get0_public_key(key.get()), nullptr));
+ EXPECT_TRUE(EC_POINT_is_on_curve(group(), EC_KEY_get0_public_key(key.get()),
+ nullptr));
// Get the public key's coordinates.
bssl::UniquePtr<BIGNUM> x(BN_new());
@@ -379,30 +390,30 @@
bssl::UniquePtr<BIGNUM> p(BN_new());
ASSERT_TRUE(p);
EXPECT_TRUE(EC_POINT_get_affine_coordinates_GFp(
- group, EC_KEY_get0_public_key(key.get()), x.get(), y.get(), nullptr));
+ group(), EC_KEY_get0_public_key(key.get()), x.get(), y.get(), nullptr));
EXPECT_TRUE(
- EC_GROUP_get_curve_GFp(group, p.get(), nullptr, nullptr, nullptr));
+ EC_GROUP_get_curve_GFp(group(), p.get(), nullptr, nullptr, nullptr));
// Points on the curve should be accepted.
- auto point = bssl::UniquePtr<EC_POINT>(EC_POINT_new(group));
+ auto point = bssl::UniquePtr<EC_POINT>(EC_POINT_new(group()));
ASSERT_TRUE(point);
- EXPECT_TRUE(EC_POINT_set_affine_coordinates_GFp(group, point.get(), x.get(),
+ EXPECT_TRUE(EC_POINT_set_affine_coordinates_GFp(group(), point.get(), x.get(),
y.get(), nullptr));
// Subtract one from |y| to make the point no longer on the curve.
EXPECT_TRUE(BN_sub(y.get(), y.get(), BN_value_one()));
// Points not on the curve should be rejected.
- bssl::UniquePtr<EC_POINT> invalid_point(EC_POINT_new(group));
+ bssl::UniquePtr<EC_POINT> invalid_point(EC_POINT_new(group()));
ASSERT_TRUE(invalid_point);
- EXPECT_FALSE(EC_POINT_set_affine_coordinates_GFp(group, invalid_point.get(),
+ EXPECT_FALSE(EC_POINT_set_affine_coordinates_GFp(group(), invalid_point.get(),
x.get(), y.get(), nullptr));
// Coordinates out of range should be rejected.
EXPECT_TRUE(BN_add(y.get(), y.get(), BN_value_one()));
EXPECT_TRUE(BN_add(y.get(), y.get(), p.get()));
- EXPECT_FALSE(EC_POINT_set_affine_coordinates_GFp(group, invalid_point.get(),
+ EXPECT_FALSE(EC_POINT_set_affine_coordinates_GFp(group(), invalid_point.get(),
x.get(), y.get(), nullptr));
EXPECT_FALSE(
EC_KEY_set_public_key_affine_coordinates(key.get(), x.get(), y.get()));
@@ -420,57 +431,52 @@
ASSERT_TRUE(key);
ASSERT_TRUE(EC_KEY_generate_key(key.get()));
- const EC_GROUP *const group = EC_KEY_get0_group(key.get());
-
- bssl::UniquePtr<EC_POINT> p1(EC_POINT_new(group));
+ bssl::UniquePtr<EC_POINT> p1(EC_POINT_new(group()));
ASSERT_TRUE(p1);
ASSERT_TRUE(EC_POINT_copy(p1.get(), EC_KEY_get0_public_key(key.get())));
- bssl::UniquePtr<EC_POINT> p2(EC_POINT_new(group));
+ bssl::UniquePtr<EC_POINT> p2(EC_POINT_new(group()));
ASSERT_TRUE(p2);
ASSERT_TRUE(EC_POINT_copy(p2.get(), EC_KEY_get0_public_key(key.get())));
- bssl::UniquePtr<EC_POINT> double_p1(EC_POINT_new(group));
+ bssl::UniquePtr<EC_POINT> double_p1(EC_POINT_new(group()));
ASSERT_TRUE(double_p1);
bssl::UniquePtr<BN_CTX> ctx(BN_CTX_new());
ASSERT_TRUE(ctx);
- ASSERT_TRUE(EC_POINT_dbl(group, double_p1.get(), p1.get(), ctx.get()));
+ ASSERT_TRUE(EC_POINT_dbl(group(), double_p1.get(), p1.get(), ctx.get()));
- bssl::UniquePtr<EC_POINT> p1_plus_p2(EC_POINT_new(group));
+ bssl::UniquePtr<EC_POINT> p1_plus_p2(EC_POINT_new(group()));
ASSERT_TRUE(p1_plus_p2);
ASSERT_TRUE(
- EC_POINT_add(group, p1_plus_p2.get(), p1.get(), p2.get(), ctx.get()));
+ EC_POINT_add(group(), p1_plus_p2.get(), p1.get(), p2.get(), ctx.get()));
EXPECT_EQ(0,
- EC_POINT_cmp(group, double_p1.get(), p1_plus_p2.get(), ctx.get()))
+ EC_POINT_cmp(group(), double_p1.get(), p1_plus_p2.get(), ctx.get()))
<< "A+A != 2A";
}
TEST_P(ECCurveTest, MulZero) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
-
- bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group()));
ASSERT_TRUE(point);
bssl::UniquePtr<BIGNUM> zero(BN_new());
ASSERT_TRUE(zero);
BN_zero(zero.get());
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), zero.get(), nullptr,
- nullptr, nullptr));
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), zero.get(), nullptr, nullptr,
+ nullptr));
- EXPECT_TRUE(EC_POINT_is_at_infinity(group.get(), point.get()))
+ EXPECT_TRUE(EC_POINT_is_at_infinity(group(), point.get()))
<< "g * 0 did not return point at infinity.";
// Test that zero times an arbitrary point is also infinity. The generator is
// used as the arbitrary point.
- bssl::UniquePtr<EC_POINT> generator(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> generator(EC_POINT_new(group()));
ASSERT_TRUE(generator);
- ASSERT_TRUE(EC_POINT_mul(group.get(), generator.get(), BN_value_one(),
- nullptr, nullptr, nullptr));
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), nullptr, generator.get(),
+ ASSERT_TRUE(EC_POINT_mul(group(), generator.get(), BN_value_one(), nullptr,
+ nullptr, nullptr));
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), nullptr, generator.get(),
zero.get(), nullptr));
- EXPECT_TRUE(EC_POINT_is_at_infinity(group.get(), point.get()))
+ EXPECT_TRUE(EC_POINT_is_at_infinity(group(), point.get()))
<< "p * 0 did not return point at infinity.";
}
@@ -480,39 +486,32 @@
// 5.6.2.3.2. (Though all our curves have cofactor one, so this check isn't
// useful.)
TEST_P(ECCurveTest, MulOrder) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
-
// Test that g × order = ∞.
- bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group()));
ASSERT_TRUE(point);
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(),
- EC_GROUP_get0_order(group.get()), nullptr, nullptr,
- nullptr));
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), EC_GROUP_get0_order(group()),
+ nullptr, nullptr, nullptr));
- EXPECT_TRUE(EC_POINT_is_at_infinity(group.get(), point.get()))
+ EXPECT_TRUE(EC_POINT_is_at_infinity(group(), point.get()))
<< "g * order did not return point at infinity.";
// Test that p × order = ∞, for some arbitrary p.
bssl::UniquePtr<BIGNUM> forty_two(BN_new());
ASSERT_TRUE(forty_two);
ASSERT_TRUE(BN_set_word(forty_two.get(), 42));
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), forty_two.get(), nullptr,
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), forty_two.get(), nullptr,
nullptr, nullptr));
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), nullptr, point.get(),
- EC_GROUP_get0_order(group.get()), nullptr));
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), nullptr, point.get(),
+ EC_GROUP_get0_order(group()), nullptr));
- EXPECT_TRUE(EC_POINT_is_at_infinity(group.get(), point.get()))
+ EXPECT_TRUE(EC_POINT_is_at_infinity(group(), point.get()))
<< "p * order did not return point at infinity.";
}
// Test that |EC_POINT_mul| works with out-of-range scalars. The operation will
// not be constant-time, but we'll compute the right answer.
TEST_P(ECCurveTest, MulOutOfRange) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
-
- bssl::UniquePtr<BIGNUM> n_minus_one(BN_dup(EC_GROUP_get0_order(group.get())));
+ bssl::UniquePtr<BIGNUM> n_minus_one(BN_dup(EC_GROUP_get0_order(group())));
ASSERT_TRUE(n_minus_one);
ASSERT_TRUE(BN_sub_word(n_minus_one.get(), 1));
@@ -526,81 +525,74 @@
ASSERT_TRUE(BN_set_word(seven.get(), 7));
bssl::UniquePtr<BIGNUM> ten_n_plus_seven(
- BN_dup(EC_GROUP_get0_order(group.get())));
+ BN_dup(EC_GROUP_get0_order(group())));
ASSERT_TRUE(ten_n_plus_seven);
ASSERT_TRUE(BN_mul_word(ten_n_plus_seven.get(), 10));
ASSERT_TRUE(BN_add_word(ten_n_plus_seven.get(), 7));
- bssl::UniquePtr<EC_POINT> point1(EC_POINT_new(group.get())),
- point2(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point1(EC_POINT_new(group())),
+ point2(EC_POINT_new(group()));
ASSERT_TRUE(point1);
ASSERT_TRUE(point2);
- ASSERT_TRUE(EC_POINT_mul(group.get(), point1.get(), n_minus_one.get(),
- nullptr, nullptr, nullptr));
- ASSERT_TRUE(EC_POINT_mul(group.get(), point2.get(), minus_one.get(), nullptr,
+ ASSERT_TRUE(EC_POINT_mul(group(), point1.get(), n_minus_one.get(), nullptr,
nullptr, nullptr));
- EXPECT_EQ(0, EC_POINT_cmp(group.get(), point1.get(), point2.get(), nullptr))
+ ASSERT_TRUE(EC_POINT_mul(group(), point2.get(), minus_one.get(), nullptr,
+ nullptr, nullptr));
+ EXPECT_EQ(0, EC_POINT_cmp(group(), point1.get(), point2.get(), nullptr))
<< "-1 * G and (n-1) * G did not give the same result";
- ASSERT_TRUE(EC_POINT_mul(group.get(), point1.get(), seven.get(), nullptr,
- nullptr, nullptr));
- ASSERT_TRUE(EC_POINT_mul(group.get(), point2.get(), ten_n_plus_seven.get(),
+ ASSERT_TRUE(EC_POINT_mul(group(), point1.get(), seven.get(), nullptr, nullptr,
+ nullptr));
+ ASSERT_TRUE(EC_POINT_mul(group(), point2.get(), ten_n_plus_seven.get(),
nullptr, nullptr, nullptr));
- EXPECT_EQ(0, EC_POINT_cmp(group.get(), point1.get(), point2.get(), nullptr))
+ EXPECT_EQ(0, EC_POINT_cmp(group(), point1.get(), point2.get(), nullptr))
<< "7 * G and (10n + 7) * G did not give the same result";
}
// Test that 10×∞ + G = G.
TEST_P(ECCurveTest, Mul) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
- bssl::UniquePtr<EC_POINT> p(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> p(EC_POINT_new(group()));
ASSERT_TRUE(p);
- bssl::UniquePtr<EC_POINT> result(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> result(EC_POINT_new(group()));
ASSERT_TRUE(result);
bssl::UniquePtr<BIGNUM> n(BN_new());
ASSERT_TRUE(n);
- ASSERT_TRUE(EC_POINT_set_to_infinity(group.get(), p.get()));
+ ASSERT_TRUE(EC_POINT_set_to_infinity(group(), p.get()));
ASSERT_TRUE(BN_set_word(n.get(), 10));
// First check that 10×∞ = ∞.
- ASSERT_TRUE(EC_POINT_mul(group.get(), result.get(), nullptr, p.get(), n.get(),
- nullptr));
- EXPECT_TRUE(EC_POINT_is_at_infinity(group.get(), result.get()));
+ ASSERT_TRUE(
+ EC_POINT_mul(group(), result.get(), nullptr, p.get(), n.get(), nullptr));
+ EXPECT_TRUE(EC_POINT_is_at_infinity(group(), result.get()));
// Now check that 10×∞ + G = G.
- const EC_POINT *generator = EC_GROUP_get0_generator(group.get());
- ASSERT_TRUE(EC_POINT_mul(group.get(), result.get(), BN_value_one(), p.get(),
+ const EC_POINT *generator = EC_GROUP_get0_generator(group());
+ ASSERT_TRUE(EC_POINT_mul(group(), result.get(), BN_value_one(), p.get(),
n.get(), nullptr));
- EXPECT_EQ(0, EC_POINT_cmp(group.get(), result.get(), generator, nullptr));
+ EXPECT_EQ(0, EC_POINT_cmp(group(), result.get(), generator, nullptr));
}
-#if !defined(BORINGSSL_SHARED_LIBRARY)
TEST_P(ECCurveTest, MulNonMinimal) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
-
bssl::UniquePtr<BIGNUM> forty_two(BN_new());
ASSERT_TRUE(forty_two);
ASSERT_TRUE(BN_set_word(forty_two.get(), 42));
// Compute g × 42.
- bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group()));
ASSERT_TRUE(point);
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), forty_two.get(), nullptr,
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), forty_two.get(), nullptr,
nullptr, nullptr));
// Compute it again with a non-minimal 42, much larger than the scalar.
ASSERT_TRUE(bn_resize_words(forty_two.get(), 64));
- bssl::UniquePtr<EC_POINT> point2(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point2(EC_POINT_new(group()));
ASSERT_TRUE(point2);
- ASSERT_TRUE(EC_POINT_mul(group.get(), point2.get(), forty_two.get(), nullptr,
+ ASSERT_TRUE(EC_POINT_mul(group(), point2.get(), forty_two.get(), nullptr,
nullptr, nullptr));
- EXPECT_EQ(0, EC_POINT_cmp(group.get(), point.get(), point2.get(), nullptr));
+ EXPECT_EQ(0, EC_POINT_cmp(group(), point.get(), point2.get(), nullptr));
}
-#endif // BORINGSSL_SHARED_LIBRARY
// Test that EC_KEY_set_private_key rejects invalid values.
TEST_P(ECCurveTest, SetInvalidPrivateKey) {
@@ -622,40 +614,56 @@
}
TEST_P(ECCurveTest, IgnoreOct2PointReturnValue) {
- bssl::UniquePtr<EC_GROUP> group(EC_GROUP_new_by_curve_name(GetParam().nid));
- ASSERT_TRUE(group);
-
bssl::UniquePtr<BIGNUM> forty_two(BN_new());
ASSERT_TRUE(forty_two);
ASSERT_TRUE(BN_set_word(forty_two.get(), 42));
// Compute g × 42.
- bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group.get()));
+ bssl::UniquePtr<EC_POINT> point(EC_POINT_new(group()));
ASSERT_TRUE(point);
- ASSERT_TRUE(EC_POINT_mul(group.get(), point.get(), forty_two.get(), nullptr,
+ ASSERT_TRUE(EC_POINT_mul(group(), point.get(), forty_two.get(), nullptr,
nullptr, nullptr));
// Serialize the point.
- size_t serialized_len =
- EC_POINT_point2oct(group.get(), point.get(),
- POINT_CONVERSION_UNCOMPRESSED, nullptr, 0, nullptr);
+ size_t serialized_len = EC_POINT_point2oct(
+ group(), point.get(), POINT_CONVERSION_UNCOMPRESSED, nullptr, 0, nullptr);
ASSERT_NE(0u, serialized_len);
std::vector<uint8_t> serialized(serialized_len);
- ASSERT_EQ(serialized_len,
- EC_POINT_point2oct(group.get(), point.get(),
- POINT_CONVERSION_UNCOMPRESSED, serialized.data(),
- serialized_len, nullptr));
+ ASSERT_EQ(
+ serialized_len,
+ EC_POINT_point2oct(group(), point.get(), POINT_CONVERSION_UNCOMPRESSED,
+ serialized.data(), serialized_len, nullptr));
// Create a serialized point that is not on the curve.
serialized[serialized_len - 1]++;
- ASSERT_FALSE(EC_POINT_oct2point(group.get(), point.get(), serialized.data(),
+ ASSERT_FALSE(EC_POINT_oct2point(group(), point.get(), serialized.data(),
serialized.size(), nullptr));
// After a failure, |point| should have been set to the generator to defend
// against code that doesn't check the return value.
- ASSERT_EQ(0, EC_POINT_cmp(group.get(), point.get(),
- EC_GROUP_get0_generator(group.get()), nullptr));
+ ASSERT_EQ(0, EC_POINT_cmp(group(), point.get(),
+ EC_GROUP_get0_generator(group()), nullptr));
+}
+
+TEST_P(ECCurveTest, DoubleSpecialCase) {
+ const EC_POINT *g = EC_GROUP_get0_generator(group());
+
+ bssl::UniquePtr<EC_POINT> two_g(EC_POINT_new(group()));
+ ASSERT_TRUE(two_g);
+ ASSERT_TRUE(EC_POINT_dbl(group(), two_g.get(), g, nullptr));
+
+ bssl::UniquePtr<EC_POINT> p(EC_POINT_new(group()));
+ ASSERT_TRUE(p);
+ ASSERT_TRUE(EC_POINT_mul(group(), p.get(), BN_value_one(), g, BN_value_one(),
+ nullptr));
+ EXPECT_EQ(0, EC_POINT_cmp(group(), p.get(), two_g.get(), nullptr));
+
+ EC_SCALAR one;
+ ASSERT_TRUE(ec_bignum_to_scalar(group(), &one, BN_value_one()));
+ ASSERT_TRUE(
+ ec_point_mul_scalar_public(group(), p.get(), &one, g, &one, nullptr));
+ EXPECT_EQ(0, EC_POINT_cmp(group(), p.get(), two_g.get(), nullptr));
}
static std::vector<EC_builtin_curve> AllCurves() {
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index e1a3e71..ee1eb95 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -180,8 +180,8 @@
// 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);
+OPENSSL_EXPORT 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.
@@ -204,9 +204,9 @@
// ec_point_mul_scalar_public performs the same computation as
// ec_point_mul_scalar. It further assumes that the inputs are public so
// there is no concern about leaking their values through timing.
-int ec_point_mul_scalar_public(const EC_GROUP *group, EC_POINT *r,
- const EC_SCALAR *g_scalar, const EC_POINT *p,
- const EC_SCALAR *p_scalar, BN_CTX *ctx);
+OPENSSL_EXPORT int ec_point_mul_scalar_public(
+ const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar,
+ const EC_POINT *p, const EC_SCALAR *p_scalar, BN_CTX *ctx);
// ec_compute_wNAF writes the modified width-(w+1) Non-Adjacent Form (wNAF) of
// |scalar| to |out| and returns one on success or zero on internal error. |out|
diff --git a/third_party/fiat/p256.c b/third_party/fiat/p256.c
index f8ad0bf..82f3608 100644
--- a/third_party/fiat/p256.c
+++ b/third_party/fiat/p256.c
@@ -1120,7 +1120,7 @@
limb_t yneq = fe_nz(r);
if (!xneq && !yneq && z1nz && z2nz) {
- point_double(x_out, y_out, z_out, x1, y1, z1);
+ point_double(x3, y3, z3, x1, y1, z1);
return;
}
