From 041dd68cecd28bdaf5506d148df4634cd0c540af Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 24 Apr 2018 22:53:07 -0400 Subject: [PATCH] Clear mallocs in ec_wNAF_mul. EC_POINT is split into the existing public EC_POINT (where the caller is sanity-checked about group mismatches) and the low-level EC_RAW_POINT (which, like EC_FELEM and EC_SCALAR, assume that is your problem and is a plain old struct). Having both EC_POINT and EC_RAW_POINT is a little silly, but we're going to want different type signatures for functions which return void anyway (my plan is to lift a non-BIGNUM get_affine_coordinates up through the ECDSA and ECDH code), so I think it's fine. This wasn't strictly necessary, but wnaf.c is a lot tidier now. Perf is a wash; once we get up to this layer, it's only 8 entries in the table so not particularly interesting. Bug: 239 Change-Id: I8ace749393d359f42649a5bb0734597bb7c07a2e Reviewed-on: https://boringssl-review.googlesource.com/27706 Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Adam Langley --- crypto/fipsmodule/ec/ec.c | 33 +++--- crypto/fipsmodule/ec/ec_montgomery.c | 4 +- crypto/fipsmodule/ec/internal.h | 58 +++++----- crypto/fipsmodule/ec/p224-64.c | 16 ++- crypto/fipsmodule/ec/p256-x86_64.c | 18 ++-- crypto/fipsmodule/ec/simple.c | 34 +++--- crypto/fipsmodule/ec/wnaf.c | 154 +++++++-------------------- third_party/fiat/p256.c | 30 +++--- 8 files changed, 144 insertions(+), 203 deletions(-) diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c index 7b5031f7..a937e277 100644 --- a/crypto/fipsmodule/ec/ec.c +++ b/crypto/fipsmodule/ec/ec.c @@ -586,7 +586,7 @@ int EC_GROUP_cmp(const EC_GROUP *a, const EC_GROUP *b, BN_CTX *ignored) { BN_cmp(&a->field, &b->field) != 0 || !ec_felem_equal(a, &a->a, &b->a) || !ec_felem_equal(a, &a->b, &b->b) || - ec_GFp_simple_cmp(a, a->generator, b->generator) != 0; + ec_GFp_simple_cmp(a, &a->generator->raw, &b->generator->raw) != 0; } const EC_POINT *EC_GROUP_get0_generator(const EC_GROUP *group) { @@ -635,7 +635,7 @@ EC_POINT *EC_POINT_new(const EC_GROUP *group) { } ret->group = EC_GROUP_dup(group); - ec_GFp_simple_point_init(ret); + ec_GFp_simple_point_init(&ret->raw); return ret; } @@ -663,7 +663,7 @@ int EC_POINT_copy(EC_POINT *dest, const EC_POINT *src) { if (dest == src) { return 1; } - ec_GFp_simple_point_copy(dest, src); + ec_GFp_simple_point_copy(&dest->raw, &src->raw); return 1; } @@ -687,7 +687,7 @@ int EC_POINT_set_to_infinity(const EC_GROUP *group, EC_POINT *point) { OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - ec_GFp_simple_point_set_to_infinity(group, point); + ec_GFp_simple_point_set_to_infinity(group, &point->raw); return 1; } @@ -696,7 +696,7 @@ int EC_POINT_is_at_infinity(const EC_GROUP *group, const EC_POINT *point) { OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - return ec_GFp_simple_is_at_infinity(group, point); + return ec_GFp_simple_is_at_infinity(group, &point->raw); } int EC_POINT_is_on_curve(const EC_GROUP *group, const EC_POINT *point, @@ -705,7 +705,7 @@ int EC_POINT_is_on_curve(const EC_GROUP *group, const EC_POINT *point, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - return ec_GFp_simple_is_on_curve(group, point); + return ec_GFp_simple_is_on_curve(group, &point->raw); } int EC_POINT_cmp(const EC_GROUP *group, const EC_POINT *a, const EC_POINT *b, @@ -715,7 +715,7 @@ int EC_POINT_cmp(const EC_GROUP *group, const EC_POINT *a, const EC_POINT *b, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return -1; } - return ec_GFp_simple_cmp(group, a, b); + return ec_GFp_simple_cmp(group, &a->raw, &b->raw); } int EC_POINT_get_affine_coordinates_GFp(const EC_GROUP *group, @@ -729,7 +729,7 @@ int EC_POINT_get_affine_coordinates_GFp(const EC_GROUP *group, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - return group->meth->point_get_affine_coordinates(group, point, x, y); + return group->meth->point_get_affine_coordinates(group, &point->raw, x, y); } int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group, EC_POINT *point, @@ -739,7 +739,7 @@ int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group, EC_POINT *point, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - if (!ec_GFp_simple_point_set_affine_coordinates(group, point, x, y)) { + if (!ec_GFp_simple_point_set_affine_coordinates(group, &point->raw, x, y)) { return 0; } @@ -751,7 +751,7 @@ int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group, EC_POINT *point, // constructing an arbitrary group. In this, we give up and hope they're // checking the return value. if (generator) { - EC_POINT_copy(point, generator); + ec_GFp_simple_point_copy(&point->raw, &generator->raw); } OPENSSL_PUT_ERROR(EC, EC_R_POINT_IS_NOT_ON_CURVE); return 0; @@ -768,7 +768,7 @@ int EC_POINT_add(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - ec_GFp_simple_add(group, r, a, b); + ec_GFp_simple_add(group, &r->raw, &a->raw, &b->raw); return 1; } @@ -779,7 +779,7 @@ int EC_POINT_dbl(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a, OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - ec_GFp_simple_dbl(group, r, a); + ec_GFp_simple_dbl(group, &r->raw, &a->raw); return 1; } @@ -789,7 +789,7 @@ int EC_POINT_invert(const EC_GROUP *group, EC_POINT *a, BN_CTX *ctx) { OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS); return 0; } - ec_GFp_simple_invert(group, a); + ec_GFp_simple_invert(group, &a->raw); return 1; } @@ -873,7 +873,8 @@ int ec_point_mul_scalar_public(const EC_GROUP *group, EC_POINT *r, return 0; } - return group->meth->mul_public(group, r, g_scalar, p, p_scalar, ctx); + group->meth->mul_public(group, &r->raw, g_scalar, &p->raw, p_scalar); + return 1; } int ec_point_mul_scalar(const EC_GROUP *group, EC_POINT *r, @@ -891,7 +892,9 @@ int ec_point_mul_scalar(const EC_GROUP *group, EC_POINT *r, return 0; } - return group->meth->mul(group, r, g_scalar, p, p_scalar, ctx); + group->meth->mul(group, &r->raw, g_scalar, (p == NULL) ? NULL : &p->raw, + p_scalar); + return 1; } void EC_GROUP_set_asn1_flag(EC_GROUP *group, int flag) {} diff --git a/crypto/fipsmodule/ec/ec_montgomery.c b/crypto/fipsmodule/ec/ec_montgomery.c index c90b9f1c..eb37e501 100644 --- a/crypto/fipsmodule/ec/ec_montgomery.c +++ b/crypto/fipsmodule/ec/ec_montgomery.c @@ -181,9 +181,9 @@ int ec_GFp_mont_felem_to_bignum(const EC_GROUP *group, BIGNUM *out, } static int ec_GFp_mont_point_get_affine_coordinates(const EC_GROUP *group, - const EC_POINT *point, + const EC_RAW_POINT *point, BIGNUM *x, BIGNUM *y) { - if (EC_POINT_is_at_infinity(group, point)) { + if (ec_GFp_simple_is_at_infinity(group, point)) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h index a70c8695..aba7d1dd 100644 --- a/crypto/fipsmodule/ec/internal.h +++ b/crypto/fipsmodule/ec/internal.h @@ -110,12 +110,21 @@ typedef union { BN_ULONG words[EC_MAX_SCALAR_WORDS]; } EC_FELEM; +// An EC_RAW_POINT represents an elliptic curve point. Unlike |EC_POINT|, it is +// a plain struct which can be stack-allocated and needs no cleanup. It is +// specific to an |EC_GROUP| and must not be mixed between groups. +typedef struct { + EC_FELEM X, Y, Z; + // X, Y, and Z are Jacobian projective coordinates. They represent + // (X/Z^2, Y/Z^3) if Z != 0 and the point at infinity otherwise. +} EC_RAW_POINT; + struct ec_method_st { int (*group_init)(EC_GROUP *); void (*group_finish)(EC_GROUP *); int (*group_set_curve)(EC_GROUP *, const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, BN_CTX *); - int (*point_get_affine_coordinates)(const EC_GROUP *, const EC_POINT *, + int (*point_get_affine_coordinates)(const EC_GROUP *, const EC_RAW_POINT *, BIGNUM *x, BIGNUM *y); // Computes |r = g_scalar*generator + p_scalar*p| if |g_scalar| and |p_scalar| @@ -123,14 +132,14 @@ struct ec_method_st { // Computes |r = p_scalar*p| if g_scalar is null. At least one of |g_scalar| // and |p_scalar| must be non-null, and |p| must be non-null if |p_scalar| is // non-null. - int (*mul)(const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar, - const EC_POINT *p, const EC_SCALAR *p_scalar, BN_CTX *ctx); + void (*mul)(const EC_GROUP *group, EC_RAW_POINT *r, const EC_SCALAR *g_scalar, + const EC_RAW_POINT *p, const EC_SCALAR *p_scalar); // mul_public performs the same computation as mul. It further assumes that // the inputs are public so there is no concern about leaking their values // through timing. - int (*mul_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); + void (*mul_public)(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar); // felem_mul and felem_sqr implement multiplication and squaring, // respectively, so that the generic |EC_POINT_add| and |EC_POINT_dbl| @@ -194,10 +203,7 @@ struct ec_point_st { // group is an owning reference to |group|, unless this is // |group->generator|. EC_GROUP *group; - - // X, Y, and Z are Jacobian projective coordinates. They represent - // (X/Z^2, Y/Z^3) if Z != 0 and the point and infinite otherwise. - EC_FELEM X, Y, Z; + EC_RAW_POINT raw; } /* EC_POINT */; EC_GROUP *ec_group_new(const EC_METHOD *meth); @@ -292,8 +298,9 @@ OPENSSL_EXPORT int ec_point_mul_scalar_public( void ec_compute_wNAF(const EC_GROUP *group, int8_t *out, const EC_SCALAR *scalar, size_t bits, int w); -int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar, - const EC_POINT *p, const EC_SCALAR *p_scalar, BN_CTX *ctx); +void ec_wNAF_mul(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar); // method functions in simple.c int ec_GFp_simple_group_init(EC_GROUP *); @@ -303,18 +310,21 @@ int ec_GFp_simple_group_set_curve(EC_GROUP *, const BIGNUM *p, const BIGNUM *a, int ec_GFp_simple_group_get_curve(const EC_GROUP *, BIGNUM *p, BIGNUM *a, BIGNUM *b); unsigned ec_GFp_simple_group_get_degree(const EC_GROUP *); -void ec_GFp_simple_point_init(EC_POINT *); -void ec_GFp_simple_point_copy(EC_POINT *, const EC_POINT *); -void ec_GFp_simple_point_set_to_infinity(const EC_GROUP *, EC_POINT *); -int ec_GFp_simple_point_set_affine_coordinates(const EC_GROUP *, EC_POINT *, - const BIGNUM *x, const BIGNUM *y); -void ec_GFp_simple_add(const EC_GROUP *, EC_POINT *r, const EC_POINT *a, - const EC_POINT *b); -void ec_GFp_simple_dbl(const EC_GROUP *, EC_POINT *r, const EC_POINT *a); -void ec_GFp_simple_invert(const EC_GROUP *, EC_POINT *); -int ec_GFp_simple_is_at_infinity(const EC_GROUP *, const EC_POINT *); -int ec_GFp_simple_is_on_curve(const EC_GROUP *, const EC_POINT *); -int ec_GFp_simple_cmp(const EC_GROUP *, const EC_POINT *a, const EC_POINT *b); +void ec_GFp_simple_point_init(EC_RAW_POINT *); +void ec_GFp_simple_point_copy(EC_RAW_POINT *, const EC_RAW_POINT *); +void ec_GFp_simple_point_set_to_infinity(const EC_GROUP *, EC_RAW_POINT *); +int ec_GFp_simple_point_set_affine_coordinates(const EC_GROUP *, EC_RAW_POINT *, + const BIGNUM *x, + const BIGNUM *y); +void ec_GFp_simple_add(const EC_GROUP *, EC_RAW_POINT *r, const EC_RAW_POINT *a, + const EC_RAW_POINT *b); +void ec_GFp_simple_dbl(const EC_GROUP *, EC_RAW_POINT *r, + const EC_RAW_POINT *a); +void ec_GFp_simple_invert(const EC_GROUP *, EC_RAW_POINT *); +int ec_GFp_simple_is_at_infinity(const EC_GROUP *, const EC_RAW_POINT *); +int ec_GFp_simple_is_on_curve(const EC_GROUP *, const EC_RAW_POINT *); +int ec_GFp_simple_cmp(const EC_GROUP *, const EC_RAW_POINT *a, + const EC_RAW_POINT *b); void ec_simple_scalar_inv_montgomery(const EC_GROUP *group, EC_SCALAR *r, const EC_SCALAR *a); diff --git a/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c index 96ca0411..606108fc 100644 --- a/crypto/fipsmodule/ec/p224-64.c +++ b/crypto/fipsmodule/ec/p224-64.c @@ -970,10 +970,9 @@ static void p224_batch_mul(p224_felem x_out, p224_felem y_out, p224_felem z_out, // Takes the Jacobian coordinates (X, Y, Z) of a point and returns // (X', Y') = (X/Z^2, Y/Z^3) -static int ec_GFp_nistp224_point_get_affine_coordinates(const EC_GROUP *group, - const EC_POINT *point, - BIGNUM *x, BIGNUM *y) { - if (EC_POINT_is_at_infinity(group, point)) { +static int ec_GFp_nistp224_point_get_affine_coordinates( + const EC_GROUP *group, const EC_RAW_POINT *point, BIGNUM *x, BIGNUM *y) { + if (ec_GFp_simple_is_at_infinity(group, point)) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } @@ -1014,10 +1013,10 @@ static int ec_GFp_nistp224_point_get_affine_coordinates(const EC_GROUP *group, return 1; } -static int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r, - const EC_SCALAR *g_scalar, - const EC_POINT *p, - const EC_SCALAR *p_scalar, BN_CTX *ctx) { +static void ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, + const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar) { p224_felem p_pre_comp[17][3]; p224_felem x_in, y_in, z_in, x_out, y_out, z_out; @@ -1060,7 +1059,6 @@ static int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r, p224_felem_to_generic(&r->Y, y_in); p224_felem_contract(z_in, z_out); p224_felem_to_generic(&r->Z, z_in); - return 1; } static void ec_GFp_nistp224_felem_mul(const EC_GROUP *group, EC_FELEM *r, diff --git a/crypto/fipsmodule/ec/p256-x86_64.c b/crypto/fipsmodule/ec/p256-x86_64.c index ea9749f6..a4f65156 100644 --- a/crypto/fipsmodule/ec/p256-x86_64.c +++ b/crypto/fipsmodule/ec/p256-x86_64.c @@ -199,7 +199,7 @@ static void ecp_nistz256_mod_inverse_mont(BN_ULONG r[P256_LIMBS], // r = p * p_scalar static void ecp_nistz256_windowed_mul(const EC_GROUP *group, P256_POINT *r, - const EC_POINT *p, + const EC_RAW_POINT *p, const EC_SCALAR *p_scalar) { assert(p != NULL); assert(p_scalar != NULL); @@ -289,10 +289,10 @@ static void ecp_nistz256_windowed_mul(const EC_GROUP *group, P256_POINT *r, ecp_nistz256_point_add(r, r, &h); } -static int ecp_nistz256_points_mul(const EC_GROUP *group, EC_POINT *r, - const EC_SCALAR *g_scalar, - const EC_POINT *p_, - const EC_SCALAR *p_scalar, BN_CTX *ctx) { +static void ecp_nistz256_points_mul(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, + const EC_RAW_POINT *p_, + const EC_SCALAR *p_scalar) { assert((p_ != NULL) == (p_scalar != NULL)); static const unsigned kWindowSize = 7; @@ -361,12 +361,12 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, EC_POINT *r, OPENSSL_memcpy(r->X.words, p.p.X, P256_LIMBS * sizeof(BN_ULONG)); OPENSSL_memcpy(r->Y.words, p.p.Y, P256_LIMBS * sizeof(BN_ULONG)); OPENSSL_memcpy(r->Z.words, p.p.Z, P256_LIMBS * sizeof(BN_ULONG)); - return 1; } -static int ecp_nistz256_get_affine(const EC_GROUP *group, const EC_POINT *point, - BIGNUM *x, BIGNUM *y) { - if (EC_POINT_is_at_infinity(group, point)) { +static int ecp_nistz256_get_affine(const EC_GROUP *group, + const EC_RAW_POINT *point, BIGNUM *x, + BIGNUM *y) { + if (ec_GFp_simple_is_at_infinity(group, point)) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } diff --git a/crypto/fipsmodule/ec/simple.c b/crypto/fipsmodule/ec/simple.c index 995b421c..8bbb5a96 100644 --- a/crypto/fipsmodule/ec/simple.c +++ b/crypto/fipsmodule/ec/simple.c @@ -175,25 +175,28 @@ unsigned ec_GFp_simple_group_get_degree(const EC_GROUP *group) { return BN_num_bits(&group->field); } -void ec_GFp_simple_point_init(EC_POINT *point) { +void ec_GFp_simple_point_init(EC_RAW_POINT *point) { OPENSSL_memset(&point->X, 0, sizeof(EC_FELEM)); OPENSSL_memset(&point->Y, 0, sizeof(EC_FELEM)); OPENSSL_memset(&point->Z, 0, sizeof(EC_FELEM)); } -void ec_GFp_simple_point_copy(EC_POINT *dest, const EC_POINT *src) { +void ec_GFp_simple_point_copy(EC_RAW_POINT *dest, const EC_RAW_POINT *src) { OPENSSL_memcpy(&dest->X, &src->X, sizeof(EC_FELEM)); OPENSSL_memcpy(&dest->Y, &src->Y, sizeof(EC_FELEM)); OPENSSL_memcpy(&dest->Z, &src->Z, sizeof(EC_FELEM)); } void ec_GFp_simple_point_set_to_infinity(const EC_GROUP *group, - EC_POINT *point) { - OPENSSL_memset(&point->Z, 0, sizeof(EC_FELEM)); + EC_RAW_POINT *point) { + // Although it is strictly only necessary to zero Z, we zero the entire point + // in case |point| was stack-allocated and yet to be initialized. + ec_GFp_simple_point_init(point); } int ec_GFp_simple_point_set_affine_coordinates(const EC_GROUP *group, - EC_POINT *point, const BIGNUM *x, + EC_RAW_POINT *point, + const BIGNUM *x, const BIGNUM *y) { if (x == NULL || y == NULL) { OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER); @@ -209,8 +212,8 @@ int ec_GFp_simple_point_set_affine_coordinates(const EC_GROUP *group, return 1; } -void ec_GFp_simple_add(const EC_GROUP *group, EC_POINT *out, const EC_POINT *a, - const EC_POINT *b) { +void ec_GFp_simple_add(const EC_GROUP *group, EC_RAW_POINT *out, + const EC_RAW_POINT *a, const EC_RAW_POINT *b) { if (a == b) { ec_GFp_simple_dbl(group, out, a); return; @@ -327,7 +330,8 @@ void ec_GFp_simple_add(const EC_GROUP *group, EC_POINT *out, const EC_POINT *a, ec_felem_select(group, &out->Z, z2nz, &z_out, &a->Z); } -void ec_GFp_simple_dbl(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a) { +void ec_GFp_simple_dbl(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_RAW_POINT *a) { void (*const felem_mul)(const EC_GROUP *, EC_FELEM *r, const EC_FELEM *a, const EC_FELEM *b) = group->meth->felem_mul; void (*const felem_sqr)(const EC_GROUP *, EC_FELEM *r, const EC_FELEM *a) = @@ -426,16 +430,18 @@ void ec_GFp_simple_dbl(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a) { } } -void ec_GFp_simple_invert(const EC_GROUP *group, EC_POINT *point) { +void ec_GFp_simple_invert(const EC_GROUP *group, EC_RAW_POINT *point) { ec_felem_neg(group, &point->Y, &point->Y); } -int ec_GFp_simple_is_at_infinity(const EC_GROUP *group, const EC_POINT *point) { +int ec_GFp_simple_is_at_infinity(const EC_GROUP *group, + const EC_RAW_POINT *point) { return ec_felem_non_zero_mask(group, &point->Z) == 0; } -int ec_GFp_simple_is_on_curve(const EC_GROUP *group, const EC_POINT *point) { - if (EC_POINT_is_at_infinity(group, point)) { +int ec_GFp_simple_is_on_curve(const EC_GROUP *group, + const EC_RAW_POINT *point) { + if (ec_GFp_simple_is_at_infinity(group, point)) { return 1; } @@ -491,8 +497,8 @@ int ec_GFp_simple_is_on_curve(const EC_GROUP *group, const EC_POINT *point) { return ec_felem_equal(group, &tmp, &rh); } -int ec_GFp_simple_cmp(const EC_GROUP *group, const EC_POINT *a, - const EC_POINT *b) { +int ec_GFp_simple_cmp(const EC_GROUP *group, const EC_RAW_POINT *a, + const EC_RAW_POINT *b) { // Note this function returns zero if |a| and |b| are equal and 1 if they are // not equal. if (ec_GFp_simple_is_at_infinity(group, a)) { diff --git a/crypto/fipsmodule/ec/wnaf.c b/crypto/fipsmodule/ec/wnaf.c index e6f0cf9a..ca346ce8 100644 --- a/crypto/fipsmodule/ec/wnaf.c +++ b/crypto/fipsmodule/ec/wnaf.c @@ -74,7 +74,6 @@ #include #include #include -#include #include "internal.h" #include "../bn/internal.h" @@ -171,163 +170,92 @@ static size_t window_bits_for_scalar_size(size_t b) { // |window_bits_for_scalar_size|. #define EC_WNAF_MAX_WINDOW_BITS 4 -// compute_precomp sets |out[i]| to a newly-allocated |EC_POINT| containing -// (2*i+1)*p, for i from 0 to |len|. It returns one on success and -// zero on error. -static int compute_precomp(const EC_GROUP *group, EC_POINT **out, - const EC_POINT *p, size_t len, BN_CTX *ctx) { - out[0] = EC_POINT_new(group); - if (out[0] == NULL || - !EC_POINT_copy(out[0], p)) { - return 0; - } - - int ret = 0; - EC_POINT *two_p = EC_POINT_new(group); - if (two_p == NULL || - !EC_POINT_dbl(group, two_p, p, ctx)) { - goto err; - } - +// compute_precomp sets |out[i]| to (2*i+1)*p, for i from 0 to |len|. +static void compute_precomp(const EC_GROUP *group, EC_RAW_POINT *out, + const EC_RAW_POINT *p, size_t len) { + ec_GFp_simple_point_copy(&out[0], p); + EC_RAW_POINT two_p; + ec_GFp_simple_dbl(group, &two_p, p); for (size_t i = 1; i < len; i++) { - out[i] = EC_POINT_new(group); - if (out[i] == NULL || - !EC_POINT_add(group, out[i], out[i - 1], two_p, ctx)) { - goto err; - } + ec_GFp_simple_add(group, &out[i], &out[i - 1], &two_p); } - - ret = 1; - -err: - EC_POINT_free(two_p); - return ret; } -static int lookup_precomp(const EC_GROUP *group, EC_POINT *out, - EC_POINT *const *precomp, int digit, BN_CTX *ctx) { +static void lookup_precomp(const EC_GROUP *group, EC_RAW_POINT *out, + const EC_RAW_POINT *precomp, int digit) { if (digit < 0) { digit = -digit; - return EC_POINT_copy(out, precomp[digit >> 1]) && - EC_POINT_invert(group, out, ctx); + ec_GFp_simple_point_copy(out, &precomp[digit >> 1]); + ec_GFp_simple_invert(group, out); + } else { + ec_GFp_simple_point_copy(out, &precomp[digit >> 1]); } - - return EC_POINT_copy(out, precomp[digit >> 1]); } -int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar, - const EC_POINT *p, const EC_SCALAR *p_scalar, BN_CTX *ctx) { - BN_CTX *new_ctx = NULL; - EC_POINT *precomp_storage[2 * (1 << (EC_WNAF_MAX_WINDOW_BITS - 1))] = {NULL}; - EC_POINT **g_precomp = NULL, **p_precomp = NULL; - int8_t g_wNAF[EC_MAX_SCALAR_BYTES * 8 + 1]; - int8_t p_wNAF[EC_MAX_SCALAR_BYTES * 8 + 1]; - EC_POINT *tmp = NULL; - int ret = 0; - - if (ctx == NULL) { - ctx = new_ctx = BN_CTX_new(); - if (ctx == NULL) { - goto err; - } - } - +void ec_wNAF_mul(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar) { size_t bits = BN_num_bits(&group->order); size_t wsize = window_bits_for_scalar_size(bits); size_t wNAF_len = bits + 1; size_t precomp_len = (size_t)1 << (wsize - 1); - OPENSSL_COMPILE_ASSERT( - OPENSSL_ARRAY_SIZE(g_wNAF) == OPENSSL_ARRAY_SIZE(p_wNAF), - g_wNAF_and_p_wNAF_are_different_sizes); - - if (wNAF_len > OPENSSL_ARRAY_SIZE(g_wNAF) || - 2 * precomp_len > OPENSSL_ARRAY_SIZE(precomp_storage)) { - OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR); - goto err; - } + assert(wsize <= EC_WNAF_MAX_WINDOW_BITS); // TODO(davidben): |mul_public| is for ECDSA verification which can assume // non-NULL inputs, but this code is also used for |mul| which cannot. It's // not constant-time, so replace the generic |mul| and remove the NULL checks. - size_t total_precomp = 0; + int8_t g_wNAF[EC_MAX_SCALAR_BYTES * 8 + 1]; + EC_RAW_POINT g_precomp[1 << (EC_WNAF_MAX_WINDOW_BITS - 1)]; + assert(precomp_len <= OPENSSL_ARRAY_SIZE(g_precomp)); + assert(wNAF_len <= OPENSSL_ARRAY_SIZE(g_wNAF)); if (g_scalar != NULL) { - const EC_POINT *g = EC_GROUP_get0_generator(group); - if (g == NULL) { - OPENSSL_PUT_ERROR(EC, EC_R_UNDEFINED_GENERATOR); - goto err; - } - g_precomp = precomp_storage + total_precomp; - total_precomp += precomp_len; + const EC_RAW_POINT *g = &group->generator->raw; ec_compute_wNAF(group, g_wNAF, g_scalar, bits, wsize); - if (!compute_precomp(group, g_precomp, g, precomp_len, ctx)) { - goto err; - } + compute_precomp(group, g_precomp, g, precomp_len); } + int8_t p_wNAF[EC_MAX_SCALAR_BYTES * 8 + 1]; + EC_RAW_POINT p_precomp[1 << (EC_WNAF_MAX_WINDOW_BITS - 1)]; + assert(precomp_len <= OPENSSL_ARRAY_SIZE(p_precomp)); + assert(wNAF_len <= OPENSSL_ARRAY_SIZE(p_wNAF)); if (p_scalar != NULL) { - p_precomp = precomp_storage + total_precomp; - total_precomp += precomp_len; ec_compute_wNAF(group, p_wNAF, p_scalar, bits, wsize); - if (!compute_precomp(group, p_precomp, p, precomp_len, ctx)) { - goto err; - } - } - - tmp = EC_POINT_new(group); - if (tmp == NULL) { - goto err; + compute_precomp(group, p_precomp, p, precomp_len); } + EC_RAW_POINT tmp; int r_is_at_infinity = 1; for (size_t k = wNAF_len - 1; k < wNAF_len; k--) { - if (!r_is_at_infinity && !EC_POINT_dbl(group, r, r, ctx)) { - goto err; + if (!r_is_at_infinity) { + ec_GFp_simple_dbl(group, r, r); } if (g_scalar != NULL && g_wNAF[k] != 0) { - if (!lookup_precomp(group, tmp, g_precomp, g_wNAF[k], ctx)) { - goto err; - } + lookup_precomp(group, &tmp, g_precomp, g_wNAF[k]); if (r_is_at_infinity) { - if (!EC_POINT_copy(r, tmp)) { - goto err; - } + ec_GFp_simple_point_copy(r, &tmp); r_is_at_infinity = 0; - } else if (!EC_POINT_add(group, r, r, tmp, ctx)) { - goto err; + } else { + ec_GFp_simple_add(group, r, r, &tmp); } } if (p_scalar != NULL && p_wNAF[k] != 0) { - if (!lookup_precomp(group, tmp, p_precomp, p_wNAF[k], ctx)) { - goto err; - } + lookup_precomp(group, &tmp, p_precomp, p_wNAF[k]); if (r_is_at_infinity) { - if (!EC_POINT_copy(r, tmp)) { - goto err; - } + ec_GFp_simple_point_copy(r, &tmp); r_is_at_infinity = 0; - } else if (!EC_POINT_add(group, r, r, tmp, ctx)) { - goto err; + } else { + ec_GFp_simple_add(group, r, r, &tmp); } } } - if (r_is_at_infinity && - !EC_POINT_set_to_infinity(group, r)) { - goto err; + if (r_is_at_infinity) { + ec_GFp_simple_point_set_to_infinity(group, r); } - ret = 1; - -err: - BN_CTX_free(new_ctx); - EC_POINT_free(tmp); OPENSSL_cleanse(&g_wNAF, sizeof(g_wNAF)); OPENSSL_cleanse(&p_wNAF, sizeof(p_wNAF)); - for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(precomp_storage); i++) { - EC_POINT_free(precomp_storage[i]); - } - return ret; } diff --git a/third_party/fiat/p256.c b/third_party/fiat/p256.c index 59209638..f42f8fe2 100644 --- a/third_party/fiat/p256.c +++ b/third_party/fiat/p256.c @@ -1630,11 +1630,10 @@ static void batch_mul(fe x_out, fe y_out, fe z_out, // Takes the Jacobian coordinates (X, Y, Z) of a point and returns (X', Y') = // (X/Z^2, Y/Z^3). -static int ec_GFp_nistp256_point_get_affine_coordinates(const EC_GROUP *group, - const EC_POINT *point, - BIGNUM *x_out, - BIGNUM *y_out) { - if (EC_POINT_is_at_infinity(group, point)) { +static int ec_GFp_nistp256_point_get_affine_coordinates( + const EC_GROUP *group, const EC_RAW_POINT *point, BIGNUM *x_out, + BIGNUM *y_out) { + if (ec_GFp_simple_is_at_infinity(group, point)) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } @@ -1673,11 +1672,10 @@ static int ec_GFp_nistp256_point_get_affine_coordinates(const EC_GROUP *group, return 1; } -static int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r, - const EC_SCALAR *g_scalar, - const EC_POINT *p, - const EC_SCALAR *p_scalar, - BN_CTX *unused_ctx) { +static void ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, + const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar) { fe p_pre_comp[17][3]; fe x_out, y_out, z_out; @@ -1713,14 +1711,13 @@ static int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r, fe_to_generic(&r->X, x_out); fe_to_generic(&r->Y, y_out); fe_to_generic(&r->Z, z_out); - return 1; } -static int ec_GFp_nistp256_point_mul_public(const EC_GROUP *group, EC_POINT *r, - const EC_SCALAR *g_scalar, - const EC_POINT *p, - const EC_SCALAR *p_scalar, - BN_CTX *unused_ctx) { +static void ec_GFp_nistp256_point_mul_public(const EC_GROUP *group, + EC_RAW_POINT *r, + const EC_SCALAR *g_scalar, + const EC_RAW_POINT *p, + const EC_SCALAR *p_scalar) { #define P256_WSIZE_PUBLIC 4 // Precompute multiples of |p|. p_pre_comp[i] is (2*i+1) * |p|. fe p_pre_comp[1 << (P256_WSIZE_PUBLIC-1)][3]; @@ -1795,7 +1792,6 @@ static int ec_GFp_nistp256_point_mul_public(const EC_GROUP *group, EC_POINT *r, fe_to_generic(&r->X, ret[0]); fe_to_generic(&r->Y, ret[1]); fe_to_generic(&r->Z, ret[2]); - return 1; } DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_nistp256_method) {