From f58993ab2fe9c3379629a697b5b037619dc8f7da Mon Sep 17 00:00:00 2001 From: dkostic Date: Wed, 24 Apr 2024 14:43:36 -0700 Subject: [PATCH 01/10] Start refactoring P-521: point double --- crypto/fipsmodule/ec/p521.c | 101 +++++++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 30 deletions(-) diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index ec720651a7..407db91b23 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -43,6 +43,7 @@ # include "../../../third_party/s2n-bignum/include/s2n-bignum_aws-lc.h" # define P521_USE_S2N_BIGNUM_FIELD_ARITH 1 +# define P521_USE_64BIT_LIMBS_FELEM 1 #else @@ -284,6 +285,36 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { p521_felem_mul(output, acc, t1); } +#if defined(P521_USE_64BIT_LIMBS_FELEM) +#define NISTP_FELEM_MAX_NUM_OF_LIMBS (9) // P-521 +typedef uint64_t felem_limb; +#else +#define NISTP_FELEM_MAX_NUM_OF_LIMBS (19) // P-521 +typedef uint32_t felem_limb; +#endif +typedef felem_limb nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; + +typedef struct { + void (*add)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*sub)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*mul)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*sqr)(felem_limb *c, const felem_limb *a); +} nistp_felem_methods; + +#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH) +static nistp_felem_methods p521_felem_methods = { + bignum_add_p521, + bignum_sub_p521, + bignum_mul_p521, + bignum_sqr_p521 }; +#else +static nistp_felem_methods p521_felem_methods = { + fiat_secp521r1_carry_add, + fiat_secp521r1_carry_sub, + fiat_secp521r1_carry_mul, + fiat_secp521r1_carry_square }; +#endif + // Group operations // ---------------- // @@ -301,52 +332,62 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { // // Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; // while x_out == y_in is not (maybe this works, but it's not tested). -static void p521_point_double(p521_felem x_out, - p521_felem y_out, - p521_felem z_out, - const p521_felem x_in, - const p521_felem y_in, - const p521_felem z_in) { - p521_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; +static void nistp_point_double(nistp_felem_methods *ctx, + felem_limb *x_out, + felem_limb *y_out, + felem_limb *z_out, + const felem_limb *x_in, + const felem_limb *y_in, + const felem_limb *z_in) { + nistp_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; // delta = z^2 - p521_felem_sqr(delta, z_in); + ctx->sqr(delta, z_in); // gamma = y^2 - p521_felem_sqr(gamma, y_in); + ctx->sqr(gamma, y_in); // beta = x*gamma - p521_felem_mul(beta, x_in, gamma); + ctx->mul(beta, x_in, gamma); // alpha = 3*(x-delta)*(x+delta) - p521_felem_sub(ftmp, x_in, delta); - p521_felem_add(ftmp2, x_in, delta); + ctx->sub(ftmp, x_in, delta); + ctx->add(ftmp2, x_in, delta); - p521_felem_add(tmptmp, ftmp2, ftmp2); - p521_felem_add(ftmp2, ftmp2, tmptmp); - p521_felem_mul(alpha, ftmp, ftmp2); + ctx->add(tmptmp, ftmp2, ftmp2); + ctx->add(ftmp2, ftmp2, tmptmp); + ctx->mul(alpha, ftmp, ftmp2); // x' = alpha^2 - 8*beta - p521_felem_sqr(x_out, alpha); - p521_felem_add(fourbeta, beta, beta); - p521_felem_add(fourbeta, fourbeta, fourbeta); - p521_felem_add(tmptmp, fourbeta, fourbeta); - p521_felem_sub(x_out, x_out, tmptmp); + ctx->sqr(x_out, alpha); + ctx->add(fourbeta, beta, beta); + ctx->add(fourbeta, fourbeta, fourbeta); + ctx->add(tmptmp, fourbeta, fourbeta); + ctx->sub(x_out, x_out, tmptmp); // z' = (y + z)^2 - gamma - delta // The following calculation differs from that in p256.c: // an add is replaced with a sub. This saves us 5 cmovznz operations // when Fiat-crypto implementation of felem_add and felem_sub is used, // and also a certain number of intructions when s2n-bignum is used. - p521_felem_add(ftmp, y_in, z_in); - p521_felem_sqr(z_out, ftmp); - p521_felem_sub(z_out, z_out, gamma); - p521_felem_sub(z_out, z_out, delta); + ctx->add(ftmp, y_in, z_in); + ctx->sqr(z_out, ftmp); + ctx->sub(z_out, z_out, gamma); + ctx->sub(z_out, z_out, delta); // y' = alpha*(4*beta - x') - 8*gamma^2 - p521_felem_sub(y_out, fourbeta, x_out); - p521_felem_add(gamma, gamma, gamma); - p521_felem_sqr(gamma, gamma); - p521_felem_mul(y_out, alpha, y_out); - p521_felem_add(gamma, gamma, gamma); - p521_felem_sub(y_out, y_out, gamma); + ctx->sub(y_out, fourbeta, x_out); + ctx->add(gamma, gamma, gamma); + ctx->sqr(gamma, gamma); + ctx->mul(y_out, alpha, y_out); + ctx->add(gamma, gamma, gamma); + ctx->sub(y_out, y_out, gamma); +} + +static void p521_point_double(p521_felem x_out, + p521_felem y_out, + p521_felem z_out, + const p521_felem x_in, + const p521_felem y_in, + const p521_felem z_in) { + nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // p521_point_add calculates (x1, y1, z1) + (x2, y2, z2) From 62866bc9622189368f5af669d1009e07124f8aae Mon Sep 17 00:00:00 2001 From: dkostic Date: Thu, 25 Apr 2024 12:01:18 -0700 Subject: [PATCH 02/10] Created ec_nistp.h/c files --- crypto/fipsmodule/bcm.c | 2 +- crypto/fipsmodule/ec/ec_nistp.c | 110 ++++++++++++++++++++++++ crypto/fipsmodule/ec/ec_nistp.h | 66 ++++++++++++++ crypto/fipsmodule/ec/make_tables.go | 4 +- crypto/fipsmodule/ec/p384.c | 42 ++------- crypto/fipsmodule/ec/p521.c | 128 +++------------------------- crypto/fipsmodule/ec/p521_table.h | 4 +- 7 files changed, 197 insertions(+), 159 deletions(-) create mode 100644 crypto/fipsmodule/ec/ec_nistp.c create mode 100644 crypto/fipsmodule/ec/ec_nistp.h diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c index 517e3fdbfe..3915104447 100644 --- a/crypto/fipsmodule/bcm.c +++ b/crypto/fipsmodule/bcm.c @@ -28,7 +28,6 @@ // to control the order. $b section will place bcm in between the start/end markers // which are in $a and $z. #if defined(BORINGSSL_FIPS) && defined(OPENSSL_WINDOWS) - #pragma code_seg(".fipstx$b") #pragma data_seg(".fipsda$b") #pragma const_seg(".fipsco$b") @@ -93,6 +92,7 @@ #include "ec/ec.c" #include "ec/ec_key.c" #include "ec/ec_montgomery.c" +#include "ec/ec_nistp.c" #include "ec/felem.c" #include "ec/oct.c" #include "ec/p224-64.c" diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c new file mode 100644 index 0000000000..ed2127d038 --- /dev/null +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -0,0 +1,110 @@ +/* +------------------------------------------------------------------------------------ + Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 OR ISC +------------------------------------------------------------------------------------ +*/ + +#if !defined(OPENSSL_SMALL) +// In this file we will implement elliptic curve point operations for +// NIST curves P-256, P-384, and P-521. The idea is to implement the operations +// in a generic way such that the code can be reused instead of having +// a separate implementation for each of the curves. We implement: +// 1. point addition, +// 2. point doubling, +// 3. scalar multiplication of a base point, +// 4. scalar multiplication of an arbitrary point, +// 5. scalar multiplication of a base and an arbitrary point. +// +// Matrix of what has been done so far: +// +// | op | P-521 | P-384 | P-256 | +// |----------------------------| +// | 1. | x | | | +// | 2. | | | | +// | 3. | | | | +// | 4. | | | | +// | 5. | | | | +// + +#include "ec_nistp.h" + +// Some of the functions in below need temporary field element variables. +// To avoid dynamic allocation we define nistp_felem type to have the maximum +// size possible (which is currently P-521 curve). The values are hard-coded +// for the moment, this will be fixed when we migrate the whole P-521 +// implementation to ec_nistp.c. +#if defined(EC_NISTP_USE_64BIT_LIMB) +#define NISTP_FELEM_MAX_NUM_OF_LIMBS (9) +#else +#define NISTP_FELEM_MAX_NUM_OF_LIMBS (19) +#endif +typedef felem_limb nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; + +// Group operations +// ---------------- +// +// Building on top of the field operations we have the operations on the +// elliptic curve group itself. Points on the curve are represented in Jacobian +// coordinates. +// +// p521_point_double calculates 2*(x_in, y_in, z_in) +// +// The method is taken from: +// http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b +// +// Coq transcription and correctness proof: +// +// +// Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; +// while x_out == y_in is not (maybe this works, but it's not tested). +void nistp_point_double(nistp_felem_methods *ctx, + felem_limb *x_out, + felem_limb *y_out, + felem_limb *z_out, + const felem_limb *x_in, + const felem_limb *y_in, + const felem_limb *z_in) { + nistp_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; + // delta = z^2 + ctx->sqr(delta, z_in); + // gamma = y^2 + ctx->sqr(gamma, y_in); + // beta = x*gamma + ctx->mul(beta, x_in, gamma); + + // alpha = 3*(x-delta)*(x+delta) + ctx->sub(ftmp, x_in, delta); + ctx->add(ftmp2, x_in, delta); + + ctx->add(tmptmp, ftmp2, ftmp2); + ctx->add(ftmp2, ftmp2, tmptmp); + ctx->mul(alpha, ftmp, ftmp2); + + // x' = alpha^2 - 8*beta + ctx->sqr(x_out, alpha); + ctx->add(fourbeta, beta, beta); + ctx->add(fourbeta, fourbeta, fourbeta); + ctx->add(tmptmp, fourbeta, fourbeta); + ctx->sub(x_out, x_out, tmptmp); + + // z' = (y + z)^2 - gamma - delta + // The following calculation differs from that in p256.c: + // an add is replaced with a sub. This saves us 5 cmovznz operations + // when Fiat-crypto implementation of felem_add and felem_sub is used, + // and also a certain number of intructions when s2n-bignum is used. + ctx->add(ftmp, y_in, z_in); + ctx->sqr(z_out, ftmp); + ctx->sub(z_out, z_out, gamma); + ctx->sub(z_out, z_out, delta); + + // y' = alpha*(4*beta - x') - 8*gamma^2 + ctx->sub(y_out, fourbeta, x_out); + ctx->add(gamma, gamma, gamma); + ctx->sqr(gamma, gamma); + ctx->mul(y_out, alpha, y_out); + ctx->add(gamma, gamma, gamma); + ctx->sub(y_out, y_out, gamma); +} + +#endif // OPENSSL_SMALL diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h new file mode 100644 index 0000000000..bdd3ecee19 --- /dev/null +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -0,0 +1,66 @@ +/* +------------------------------------------------------------------------------------ + Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + SPDX-License-Identifier: Apache-2.0 OR ISC +------------------------------------------------------------------------------------ +*/ +#ifndef NISTP_H +#define NISTP_H + +#include + +#include + +// We have two implementations of the field arithmetic for NIST curves: +// - Fiat-crypto +// - s2n-bignum +// Both Fiat-crypto and s2n-bignum implementations are formally verified. +// Fiat-crypto implementation is fully portable C code, while s2n-bignum +// implements the operations in assembly for x86_64 and aarch64 platforms. +// If (1) x86_64 or aarch64, (2) linux or apple, and (3) OPENSSL_NO_ASM is not +// set, s2n-bignum path is capable. +#if !defined(OPENSSL_NO_ASM) && \ + (defined(OPENSSL_LINUX) || defined(OPENSSL_APPLE)) && \ + ((defined(OPENSSL_X86_64) && !defined(MY_ASSEMBLER_IS_TOO_OLD_FOR_AVX)) || \ + defined(OPENSSL_AARCH64)) +# define EC_NISTP_USE_S2N_BIGNUM +# define EC_NISTP_USE_64BIT_LIMB +#else +// Fiat-crypto has both 64-bit and 32-bit implementation. +# if defined(BORINGSSL_HAS_UINT128) +# define EC_NISTP_USE_64BIT_LIMB +# endif +#endif + +#if defined(EC_NISTP_USE_64BIT_LIMB) +typedef uint64_t felem_limb; +#else +typedef uint32_t felem_limb; +#endif + +// This is the struct that holds pointers to implementations of field +// arithmetic functions for specific curves. It is meant to be used +// in higher level functions like this: +// void point_double(nistp_felem_methods *ctx, ...) { +// ctx->add(...); +// ctx->mul(...); +// } +// This makes the functions reusable between different curves by simply +// providing an appropriate methods object. +typedef struct { + void (*add)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*sub)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*mul)(felem_limb *c, const felem_limb *a, const felem_limb *b); + void (*sqr)(felem_limb *c, const felem_limb *a); +} nistp_felem_methods; + + +void nistp_point_double(nistp_felem_methods *ctx, + felem_limb *x_out, + felem_limb *y_out, + felem_limb *z_out, + const felem_limb *x_in, + const felem_limb *y_in, + const felem_limb *z_in); +#endif // NISTP_H + diff --git a/crypto/fipsmodule/ec/make_tables.go b/crypto/fipsmodule/ec/make_tables.go index 7ea6fcd4d0..f7c3acd7d9 100644 --- a/crypto/fipsmodule/ec/make_tables.go +++ b/crypto/fipsmodule/ec/make_tables.go @@ -462,7 +462,7 @@ func writeP521Table(path string) error { // is based on the generation method in: // https://gitlab.com/nisec/ecckiila/-/blob/master/main.py#L296 -#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH)` +#if defined(EC_NISTP_USE_S2N_BIGNUM)` table_def_str := fmt.Sprintf("static const p521_felem p521_g_pre_comp[%d][%d][2] = ", num_subtables, pts_per_subtable) @@ -472,7 +472,7 @@ func writeP521Table(path string) error { if err := writeTables(w, curve, tables, writeU64, nil); err != nil { return err } - if _, err := io.WriteString(w, ";\n#else\n#if defined(P521_USE_64BIT_LIMBS_FELEM)\n" + table_def_str); err != nil { + if _, err := io.WriteString(w, ";\n#else\n#if defined(EC_NISTP_USE_64BIT_LIMB)\n" + table_def_str); err != nil { return err } // P-521 Fiat-crypto implementation for 64-bit systems represents a field diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 9e1011a123..73d98fe770 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -17,46 +17,17 @@ #if !defined(OPENSSL_SMALL) -// We have two implementations of the field arithmetic for P-384 curve: -// - Fiat-crypto -// - s2n-bignum -// Both Fiat-crypto and s2n-bignum implementations are formally verified. -// Fiat-crypto implementation is fully portable C code, while s2n-bignum -// implements the operations in assembly for x86_64 and aarch64 platforms. -// All the P-384 field operations supported by Fiat-crypto are supported -// by s2n-bignum as well, so s2n-bignum can be used as a drop-in replacement -// when appropriate. To do that we define macros for the functions. -// For example, field addition macro is either defined as -// #define p384_felem_add(out, in0, in1) fiat_p384_add(out, in0, in1) -// when Fiat-crypto is used, or as: -// #define p384_felem_add(out, in0, in1) bignum_add_p384(out, in0, in1) -// when s2n-bignum is used. -// -// If (1) x86_64 or aarch64, (2) linux or apple, and (3) OPENSSL_NO_ASM is not -// set, s2n-bignum path is capable. -#if !defined(OPENSSL_NO_ASM) && \ - (defined(OPENSSL_LINUX) || defined(OPENSSL_APPLE) || \ - defined(OPENSSL_OPENBSD)) && \ - ((defined(OPENSSL_X86_64) && !defined(MY_ASSEMBLER_IS_TOO_OLD_FOR_AVX)) || \ - defined(OPENSSL_AARCH64)) - +#if defined(EC_NISTP_USE_S2N_BIGNUM) # include "../../../third_party/s2n-bignum/include/s2n-bignum_aws-lc.h" - -# define P384_USE_S2N_BIGNUM_FIELD_ARITH 1 -# define P384_USE_64BIT_LIMBS_FELEM 1 - #else - -# if defined(BORINGSSL_HAS_UINT128) +# if defined(EC_NISTP_USE_64BIT_LIMB) # include "../../../third_party/fiat/p384_64.h" -# define P384_USE_64BIT_LIMBS_FELEM 1 # else # include "../../../third_party/fiat/p384_32.h" # endif - #endif -#if defined(P384_USE_64BIT_LIMBS_FELEM) +#if defined(EC_NISTP_USE_64BIT_LIMB) #define P384_NLIMBS (6) typedef uint64_t p384_limb_t; @@ -74,8 +45,7 @@ static const p384_felem p384_felem_one = { #endif // 64BIT - -#if defined(P384_USE_S2N_BIGNUM_FIELD_ARITH) +#if defined(EC_NISTP_USE_S2N_BIGNUM) #define p384_felem_add(out, in0, in1) bignum_add_p384(out, in0, in1) #define p384_felem_sub(out, in0, in1) bignum_sub_p384(out, in0, in1) @@ -91,7 +61,7 @@ static p384_limb_t p384_felem_nz(const p384_limb_t in1[P384_NLIMBS]) { return bignum_nonzero_6(in1); } -#else // P384_USE_S2N_BIGNUM_FIELD_ARITH +#else // EC_NISTP_USE_S2N_BIGNUM // Fiat-crypto implementation of field arithmetic #define p384_felem_add(out, in0, in1) fiat_p384_add(out, in0, in1) @@ -110,7 +80,7 @@ static p384_limb_t p384_felem_nz(const p384_limb_t in1[P384_NLIMBS]) { return ret; } -#endif // P384_USE_S2N_BIGNUM_FIELD_ARITH +#endif // EC_NISTP_USE_S2N_BIGNUM static void p384_felem_copy(p384_limb_t out[P384_NLIMBS], diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index 407db91b23..16ca196f4f 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -17,47 +17,21 @@ #include "../cpucap/internal.h" #include "../delocate.h" #include "internal.h" +#include "ec_nistp.h" #if !defined(OPENSSL_SMALL) -// We have two implementations of the field arithmetic for P-521 curve: -// - Fiat-crypto -// - s2n-bignum -// Both Fiat-crypto and s2n-bignum implementations are formally verified. -// Fiat-crypto implementation is fully portable C code, while s2n-bignum -// implements the operations in assembly for x86_64 and aarch64 platforms. -// All the P-521 field operations supported by Fiat-crypto are supported -// by s2n-bignum as well, so s2n-bignum can be used as a drop-in replacement -// when appropriate. To do that we define macros for the functions. -// For example, field addition macro is either defined as -// #define p521_felem_add(out, in0, in1) fiat_p521_add(out, in0, in1) -// when Fiat-crypto is used, or as: -// #define p521_felem_add(out, in0, in1) bignum_add_p521(out, in0, in1) -// when s2n-bignum is used. -// If (1) x86_64 or aarch64, (2) linux or apple, and (3) OPENSSL_NO_ASM is not -// set, s2n-bignum path is capable. -#if !defined(OPENSSL_NO_ASM) && \ - (defined(OPENSSL_LINUX) || defined(OPENSSL_APPLE) || \ - defined(OPENSSL_OPENBSD)) && \ - ((defined(OPENSSL_X86_64) && !defined(MY_ASSEMBLER_IS_TOO_OLD_FOR_AVX)) || \ - defined(OPENSSL_AARCH64)) +#if defined(EC_NISTP_USE_S2N_BIGNUM) # include "../../../third_party/s2n-bignum/include/s2n-bignum_aws-lc.h" -# define P521_USE_S2N_BIGNUM_FIELD_ARITH 1 -# define P521_USE_64BIT_LIMBS_FELEM 1 - #else - -// Fiat-crypto has both 64-bit and 32-bit implementation for P-521. -# if defined(BORINGSSL_HAS_UINT128) +# if defined(EC_NISTP_USE_64BIT_LIMB) # include "../../../third_party/fiat/p521_64.h" -# define P521_USE_64BIT_LIMBS_FELEM 1 # else # include "../../../third_party/fiat/p521_32.h" # endif - #endif -#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH) +#if defined(EC_NISTP_USE_S2N_BIGNUM) #define P521_NLIMBS (9) @@ -88,9 +62,9 @@ static const p521_limb_t p521_felem_p[P521_NLIMBS] = { #define p521_felem_mul(out, in0, in1) bignum_mul_p521_selector(out, in0, in1) #define p521_felem_sqr(out, in0) bignum_sqr_p521_selector(out, in0) -#else // P521_USE_S2N_BIGNUM_FIELD_ARITH +#else // EC_NISTP_USE_S2N_BIGNUM -#if defined(P521_USE_64BIT_LIMBS_FELEM) +#if defined(EC_NISTP_USE_64BIT_LIMB) // In the 64-bit case Fiat-crypto represents a field element by 9 58-bit digits. #define P521_NLIMBS (9) @@ -150,7 +124,7 @@ static const p521_limb_t p521_felem_p[P521_NLIMBS] = { #define p521_felem_to_bytes(out, in0) fiat_secp521r1_to_bytes(out, in0) #define p521_felem_from_bytes(out, in0) fiat_secp521r1_from_bytes(out, in0) -#endif // P521_USE_S2N_BIGNUM_FIELD_ARITH +#endif // EC_NISTP_USE_S2N_BIGNUM static p521_limb_t p521_felem_nz(const p521_limb_t in1[P521_NLIMBS]) { p521_limb_t is_not_zero = 0; @@ -158,7 +132,7 @@ static p521_limb_t p521_felem_nz(const p521_limb_t in1[P521_NLIMBS]) { is_not_zero |= in1[i]; } -#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH) +#if defined(EC_NISTP_USE_S2N_BIGNUM) return is_not_zero; #else // Fiat-crypto functions may return p (the field characteristic) @@ -285,23 +259,7 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { p521_felem_mul(output, acc, t1); } -#if defined(P521_USE_64BIT_LIMBS_FELEM) -#define NISTP_FELEM_MAX_NUM_OF_LIMBS (9) // P-521 -typedef uint64_t felem_limb; -#else -#define NISTP_FELEM_MAX_NUM_OF_LIMBS (19) // P-521 -typedef uint32_t felem_limb; -#endif -typedef felem_limb nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; - -typedef struct { - void (*add)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*sub)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*mul)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*sqr)(felem_limb *c, const felem_limb *a); -} nistp_felem_methods; - -#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH) +#if defined(EC_NISTP_USE_S2N_BIGNUM) static nistp_felem_methods p521_felem_methods = { bignum_add_p521, bignum_sub_p521, @@ -315,79 +273,13 @@ static nistp_felem_methods p521_felem_methods = { fiat_secp521r1_carry_square }; #endif -// Group operations -// ---------------- -// -// Building on top of the field operations we have the operations on the -// elliptic curve group itself. Points on the curve are represented in Jacobian -// coordinates. -// -// p521_point_double calculates 2*(x_in, y_in, z_in) -// -// The method is taken from: -// http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b -// -// Coq transcription and correctness proof: -// -// -// Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; -// while x_out == y_in is not (maybe this works, but it's not tested). -static void nistp_point_double(nistp_felem_methods *ctx, - felem_limb *x_out, - felem_limb *y_out, - felem_limb *z_out, - const felem_limb *x_in, - const felem_limb *y_in, - const felem_limb *z_in) { - nistp_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; - // delta = z^2 - ctx->sqr(delta, z_in); - // gamma = y^2 - ctx->sqr(gamma, y_in); - // beta = x*gamma - ctx->mul(beta, x_in, gamma); - - // alpha = 3*(x-delta)*(x+delta) - ctx->sub(ftmp, x_in, delta); - ctx->add(ftmp2, x_in, delta); - - ctx->add(tmptmp, ftmp2, ftmp2); - ctx->add(ftmp2, ftmp2, tmptmp); - ctx->mul(alpha, ftmp, ftmp2); - - // x' = alpha^2 - 8*beta - ctx->sqr(x_out, alpha); - ctx->add(fourbeta, beta, beta); - ctx->add(fourbeta, fourbeta, fourbeta); - ctx->add(tmptmp, fourbeta, fourbeta); - ctx->sub(x_out, x_out, tmptmp); - - // z' = (y + z)^2 - gamma - delta - // The following calculation differs from that in p256.c: - // an add is replaced with a sub. This saves us 5 cmovznz operations - // when Fiat-crypto implementation of felem_add and felem_sub is used, - // and also a certain number of intructions when s2n-bignum is used. - ctx->add(ftmp, y_in, z_in); - ctx->sqr(z_out, ftmp); - ctx->sub(z_out, z_out, gamma); - ctx->sub(z_out, z_out, delta); - - // y' = alpha*(4*beta - x') - 8*gamma^2 - ctx->sub(y_out, fourbeta, x_out); - ctx->add(gamma, gamma, gamma); - ctx->sqr(gamma, gamma); - ctx->mul(y_out, alpha, y_out); - ctx->add(gamma, gamma, gamma); - ctx->sub(y_out, y_out, gamma); -} - static void p521_point_double(p521_felem x_out, p521_felem y_out, p521_felem z_out, const p521_felem x_in, const p521_felem y_in, const p521_felem z_in) { - nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // p521_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p521_table.h b/crypto/fipsmodule/ec/p521_table.h index 0d5e7da1db..029fd82ada 100644 --- a/crypto/fipsmodule/ec/p521_table.h +++ b/crypto/fipsmodule/ec/p521_table.h @@ -25,7 +25,7 @@ // is based on the generation method in: // https://gitlab.com/nisec/ecckiila/-/blob/master/main.py#L296 -#if defined(P521_USE_S2N_BIGNUM_FIELD_ARITH) +#if defined(EC_NISTP_USE_S2N_BIGNUM) static const p521_felem p521_g_pre_comp[27][16][2] = { {{{0xf97e7e31c2e5bd66, 0x3348b3c1856a429b, 0xfe1dc127a2ffa8de, 0xa14b5e77efe75928, 0xf828af606b4d3dba, 0x9c648139053fb521, @@ -2620,7 +2620,7 @@ static const p521_felem p521_g_pre_comp[27][16][2] = { 0xa52d88b032279d4f, 0xcbb5c865dc5e94a4, 0x438dfd2ab800eeb6, 0xca1f3410ea7c4ad8, 0x7753085f3ebf90db, 0x00000000000001d3}}}}; #else -#if defined(P521_USE_64BIT_LIMBS_FELEM) +#if defined(EC_NISTP_USE_64BIT_LIMB) static const p521_felem p521_g_pre_comp[27][16][2] = { {{{0x017e7e31c2e5bd66, 0x022cf0615a90a6fe, 0x00127a2ffa8de334, 0x01dfbf9d64a3f877, 0x006b4d3dbaa14b5e, 0x014fed487e0a2bd8, From 2ff361c8694054fb2efb8f960bc74dd6ebb4943d Mon Sep 17 00:00:00 2001 From: dkostic Date: Thu, 25 Apr 2024 14:07:45 -0700 Subject: [PATCH 03/10] Added P-384 point double --- crypto/fipsmodule/ec/ec_nistp.c | 2 +- crypto/fipsmodule/ec/make_tables.go | 2 +- crypto/fipsmodule/ec/p384.c | 73 +++++++---------------------- crypto/fipsmodule/ec/p384_table.h | 2 +- 4 files changed, 19 insertions(+), 60 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index ed2127d038..278dfd2af2 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -20,7 +20,7 @@ // // | op | P-521 | P-384 | P-256 | // |----------------------------| -// | 1. | x | | | +// | 1. | x | x | | // | 2. | | | | // | 3. | | | | // | 4. | | | | diff --git a/crypto/fipsmodule/ec/make_tables.go b/crypto/fipsmodule/ec/make_tables.go index f7c3acd7d9..08e092f656 100644 --- a/crypto/fipsmodule/ec/make_tables.go +++ b/crypto/fipsmodule/ec/make_tables.go @@ -392,7 +392,7 @@ func writeP384Table(path string) error { // is based on the generation method in: // https://gitlab.com/nisec/ecckiila/-/blob/master/main.py#L296 -#if defined(P384_USE_64BIT_LIMBS_FELEM)` +#if defined(EC_NISTP_USE_64BIT_LIMB)` table_def_str := fmt.Sprintf("static const p384_felem p384_g_pre_comp[%d][%d][2] = ", num_subtables, pts_per_subtable) diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 73d98fe770..22fb42eef9 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -14,6 +14,7 @@ #include "../cpucap/internal.h" #include "../delocate.h" #include "internal.h" +#include "ec_nistp.h" #if !defined(OPENSSL_SMALL) @@ -240,69 +241,27 @@ static void p384_inv_square(p384_felem out, p384_felem_sqr(out, ret); // 2^384 - 2^128 - 2^96 + 2^32 - 2^2 = p - 3 } -// Group operations -// ---------------- -// -// Building on top of the field operations we have the operations on the -// elliptic curve group itself. Points on the curve are represented in Jacobian -// coordinates. -// -// p384_point_double calculates 2*(x_in, y_in, z_in) -// -// The method is taken from: -// http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b -// -// Coq transcription and correctness proof: -// -// -// Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; -// while x_out == y_in is not (maybe this works, but it's not tested). +#if defined(EC_NISTP_USE_S2N_BIGNUM) +static nistp_felem_methods p384_felem_methods = { + bignum_add_p384, + bignum_sub_p384, + bignum_montmul_p384, + bignum_montsqr_p384 }; +#else +static nistp_felem_methods p384_felem_methods = { + fiat_p384_add, + fiat_p384_sub, + fiat_p384_mul, + fiat_p384_square }; +#endif + static void p384_point_double(p384_felem x_out, p384_felem y_out, p384_felem z_out, const p384_felem x_in, const p384_felem y_in, const p384_felem z_in) { - p384_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; - // delta = z^2 - p384_felem_sqr(delta, z_in); - // gamma = y^2 - p384_felem_sqr(gamma, y_in); - // beta = x*gamma - p384_felem_mul(beta, x_in, gamma); - - // alpha = 3*(x-delta)*(x+delta) - p384_felem_sub(ftmp, x_in, delta); - p384_felem_add(ftmp2, x_in, delta); - - p384_felem_add(tmptmp, ftmp2, ftmp2); - p384_felem_add(ftmp2, ftmp2, tmptmp); - p384_felem_mul(alpha, ftmp, ftmp2); - - // x' = alpha^2 - 8*beta - p384_felem_sqr(x_out, alpha); - p384_felem_add(fourbeta, beta, beta); - p384_felem_add(fourbeta, fourbeta, fourbeta); - p384_felem_add(tmptmp, fourbeta, fourbeta); - p384_felem_sub(x_out, x_out, tmptmp); - - // z' = (y + z)^2 - gamma - delta - // The following calculation differs from that in p256.c: - // an add is replaced with a sub. This saves us 5 cmovznz operations - // when Fiat-crypto implementation of felem_add and felem_sub is used, - // and also a certain number of intructions when s2n-bignum is used. - p384_felem_add(ftmp, y_in, z_in); - p384_felem_sqr(z_out, ftmp); - p384_felem_sub(z_out, z_out, gamma); - p384_felem_sub(z_out, z_out, delta); - - // y' = alpha*(4*beta - x') - 8*gamma^2 - p384_felem_sub(y_out, fourbeta, x_out); - p384_felem_add(gamma, gamma, gamma); - p384_felem_sqr(gamma, gamma); - p384_felem_mul(y_out, alpha, y_out); - p384_felem_add(gamma, gamma, gamma); - p384_felem_sub(y_out, y_out, gamma); + nistp_point_double(&p384_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // p384_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p384_table.h b/crypto/fipsmodule/ec/p384_table.h index 511ba6ef48..91a1396059 100644 --- a/crypto/fipsmodule/ec/p384_table.h +++ b/crypto/fipsmodule/ec/p384_table.h @@ -25,7 +25,7 @@ // is based on the generation method in: // https://gitlab.com/nisec/ecckiila/-/blob/master/main.py#L296 -#if defined(P384_USE_64BIT_LIMBS_FELEM) +#if defined(EC_NISTP_USE_64BIT_LIMB) static const p384_felem p384_g_pre_comp[20][16][2] = { {{{0x3dd0756649c0b528, 0x20e378e2a0d6ce38, 0x879c3afc541b4d6e, 0x6454868459a30eff, 0x812ff723614ede2b, 0x4d3aadc2299e1513}, From b1757ca16bc7b2409e6fac37c0d5c403eb2a26f8 Mon Sep 17 00:00:00 2001 From: dkostic Date: Thu, 25 Apr 2024 14:25:29 -0700 Subject: [PATCH 04/10] Added P-256 Fiat-crypto point double --- crypto/fipsmodule/ec/ec_nistp.c | 16 +++++--- crypto/fipsmodule/ec/p256.c | 73 +++++---------------------------- 2 files changed, 20 insertions(+), 69 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index 278dfd2af2..4a66846fc2 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -20,12 +20,12 @@ // // | op | P-521 | P-384 | P-256 | // |----------------------------| -// | 1. | x | x | | +// | 1. | x | x | x* | // | 2. | | | | // | 3. | | | | // | 4. | | | | // | 5. | | | | -// +// * For P-256, only the Fiat-crypto implementation in p256.c is replaced. #include "ec_nistp.h" @@ -89,10 +89,14 @@ void nistp_point_double(nistp_felem_methods *ctx, ctx->sub(x_out, x_out, tmptmp); // z' = (y + z)^2 - gamma - delta - // The following calculation differs from that in p256.c: - // an add is replaced with a sub. This saves us 5 cmovznz operations - // when Fiat-crypto implementation of felem_add and felem_sub is used, - // and also a certain number of intructions when s2n-bignum is used. + // The following calculation differs from the Coq proof cited above. + // The proof is for: + // add(delta, gamma, delta); + // add(ftmp, y_in, z_in); + // square(z_out, ftmp); + // sub(z_out, z_out, delta); + // Our operations sequence is a bit more efficient because it saves us + // a certain number of conditional moves. ctx->add(ftmp, y_in, z_in); ctx->sqr(z_out, ftmp); ctx->sub(z_out, z_out, gamma); diff --git a/crypto/fipsmodule/ec/p256.c b/crypto/fipsmodule/ec/p256.c index 7572b68202..8b622b0cf6 100644 --- a/crypto/fipsmodule/ec/p256.c +++ b/crypto/fipsmodule/ec/p256.c @@ -30,6 +30,7 @@ #include "../../internal.h" #include "../delocate.h" #include "./internal.h" +#include "ec_nistp.h" #if defined(BORINGSSL_HAS_UINT128) #define BORINGSSL_NISTP256_64BIT 1 @@ -166,73 +167,19 @@ static void fiat_p256_inv_square(fiat_p256_felem out, fiat_p256_square(out, ret); // 2^256 - 2^224 + 2^192 + 2^96 - 2^2 } -// Group operations -// ---------------- -// -// Building on top of the field operations we have the operations on the -// elliptic curve group itself. Points on the curve are represented in Jacobian -// coordinates. -// -// Both operations were transcribed to Coq and proven to correspond to naive -// implementations using Affine coordinates, for all suitable fields. In the -// Coq proofs, issues of constant-time execution and memory layout (aliasing) -// conventions were not considered. Specification of affine coordinates: -// -// As a sanity check, a proof that these points form a commutative group: -// - -// fiat_p256_point_double calculates 2*(x_in, y_in, z_in) -// -// The method is taken from: -// http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b -// -// Coq transcription and correctness proof: -// -// -// -// Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed. -// while x_out == y_in is not (maybe this works, but it's not tested). -static void fiat_p256_point_double(fiat_p256_felem x_out, fiat_p256_felem y_out, +static nistp_felem_methods p256_felem_methods = { + fiat_p256_add, + fiat_p256_sub, + fiat_p256_mul, + fiat_p256_square }; + +static void fiat_p256_point_double(fiat_p256_felem x_out, + fiat_p256_felem y_out, fiat_p256_felem z_out, const fiat_p256_felem x_in, const fiat_p256_felem y_in, const fiat_p256_felem z_in) { - fiat_p256_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; - // delta = z^2 - fiat_p256_square(delta, z_in); - // gamma = y^2 - fiat_p256_square(gamma, y_in); - // beta = x*gamma - fiat_p256_mul(beta, x_in, gamma); - - // alpha = 3*(x-delta)*(x+delta) - fiat_p256_sub(ftmp, x_in, delta); - fiat_p256_add(ftmp2, x_in, delta); - - fiat_p256_add(tmptmp, ftmp2, ftmp2); - fiat_p256_add(ftmp2, ftmp2, tmptmp); - fiat_p256_mul(alpha, ftmp, ftmp2); - - // x' = alpha^2 - 8*beta - fiat_p256_square(x_out, alpha); - fiat_p256_add(fourbeta, beta, beta); - fiat_p256_add(fourbeta, fourbeta, fourbeta); - fiat_p256_add(tmptmp, fourbeta, fourbeta); - fiat_p256_sub(x_out, x_out, tmptmp); - - // z' = (y + z)^2 - gamma - delta - fiat_p256_add(delta, gamma, delta); - fiat_p256_add(ftmp, y_in, z_in); - fiat_p256_square(z_out, ftmp); - fiat_p256_sub(z_out, z_out, delta); - - // y' = alpha*(4*beta - x') - 8*gamma^2 - fiat_p256_sub(y_out, fourbeta, x_out); - fiat_p256_add(gamma, gamma, gamma); - fiat_p256_square(gamma, gamma); - fiat_p256_mul(y_out, alpha, y_out); - fiat_p256_add(gamma, gamma, gamma); - fiat_p256_sub(y_out, y_out, gamma); + nistp_point_double(&p256_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // fiat_p256_point_add calculates (x1, y1, z1) + (x2, y2, z2) From b1679a3d2f23c0d73f41976a884a1d48e08d1d68 Mon Sep 17 00:00:00 2001 From: dkostic Date: Thu, 25 Apr 2024 14:33:23 -0700 Subject: [PATCH 05/10] Naming refactor --- crypto/fipsmodule/ec/ec_nistp.c | 20 ++++++++++---------- crypto/fipsmodule/ec/ec_nistp.h | 28 ++++++++++++++-------------- crypto/fipsmodule/ec/p256.c | 4 ++-- crypto/fipsmodule/ec/p384.c | 6 +++--- crypto/fipsmodule/ec/p521.c | 6 +++--- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index 4a66846fc2..003c589676 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -29,7 +29,7 @@ #include "ec_nistp.h" -// Some of the functions in below need temporary field element variables. +// Some of the functions below need temporary field element variables. // To avoid dynamic allocation we define nistp_felem type to have the maximum // size possible (which is currently P-521 curve). The values are hard-coded // for the moment, this will be fixed when we migrate the whole P-521 @@ -39,7 +39,7 @@ #else #define NISTP_FELEM_MAX_NUM_OF_LIMBS (19) #endif -typedef felem_limb nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; +typedef ec_nistp_felem_limb ec_nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; // Group operations // ---------------- @@ -58,14 +58,14 @@ typedef felem_limb nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; // // Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; // while x_out == y_in is not (maybe this works, but it's not tested). -void nistp_point_double(nistp_felem_methods *ctx, - felem_limb *x_out, - felem_limb *y_out, - felem_limb *z_out, - const felem_limb *x_in, - const felem_limb *y_in, - const felem_limb *z_in) { - nistp_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; +void ec_nistp_point_double(ec_nistp_felem_meth *ctx, + ec_nistp_felem_limb *x_out, + ec_nistp_felem_limb *y_out, + ec_nistp_felem_limb *z_out, + const ec_nistp_felem_limb *x_in, + const ec_nistp_felem_limb *y_in, + const ec_nistp_felem_limb *z_in) { + ec_nistp_felem delta, gamma, beta, ftmp, ftmp2, tmptmp, alpha, fourbeta; // delta = z^2 ctx->sqr(delta, z_in); // gamma = y^2 diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h index bdd3ecee19..a8626b2ec6 100644 --- a/crypto/fipsmodule/ec/ec_nistp.h +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -33,9 +33,9 @@ #endif #if defined(EC_NISTP_USE_64BIT_LIMB) -typedef uint64_t felem_limb; +typedef uint64_t ec_nistp_felem_limb; #else -typedef uint32_t felem_limb; +typedef uint32_t ec_nistp_felem_limb; #endif // This is the struct that holds pointers to implementations of field @@ -48,19 +48,19 @@ typedef uint32_t felem_limb; // This makes the functions reusable between different curves by simply // providing an appropriate methods object. typedef struct { - void (*add)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*sub)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*mul)(felem_limb *c, const felem_limb *a, const felem_limb *b); - void (*sqr)(felem_limb *c, const felem_limb *a); -} nistp_felem_methods; + void (*add)(ec_nistp_felem_limb *c, const ec_nistp_felem_limb *a, const ec_nistp_felem_limb *b); + void (*sub)(ec_nistp_felem_limb *c, const ec_nistp_felem_limb *a, const ec_nistp_felem_limb *b); + void (*mul)(ec_nistp_felem_limb *c, const ec_nistp_felem_limb *a, const ec_nistp_felem_limb *b); + void (*sqr)(ec_nistp_felem_limb *c, const ec_nistp_felem_limb *a); +} ec_nistp_felem_meth; -void nistp_point_double(nistp_felem_methods *ctx, - felem_limb *x_out, - felem_limb *y_out, - felem_limb *z_out, - const felem_limb *x_in, - const felem_limb *y_in, - const felem_limb *z_in); +void ec_nistp_point_double(ec_nistp_felem_meth *ctx, + ec_nistp_felem_limb *x_out, + ec_nistp_felem_limb *y_out, + ec_nistp_felem_limb *z_out, + const ec_nistp_felem_limb *x_in, + const ec_nistp_felem_limb *y_in, + const ec_nistp_felem_limb *z_in); #endif // NISTP_H diff --git a/crypto/fipsmodule/ec/p256.c b/crypto/fipsmodule/ec/p256.c index 8b622b0cf6..6fd89cf113 100644 --- a/crypto/fipsmodule/ec/p256.c +++ b/crypto/fipsmodule/ec/p256.c @@ -167,7 +167,7 @@ static void fiat_p256_inv_square(fiat_p256_felem out, fiat_p256_square(out, ret); // 2^256 - 2^224 + 2^192 + 2^96 - 2^2 } -static nistp_felem_methods p256_felem_methods = { +static ec_nistp_felem_meth p256_felem_methods = { fiat_p256_add, fiat_p256_sub, fiat_p256_mul, @@ -179,7 +179,7 @@ static void fiat_p256_point_double(fiat_p256_felem x_out, const fiat_p256_felem x_in, const fiat_p256_felem y_in, const fiat_p256_felem z_in) { - nistp_point_double(&p256_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(&p256_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // fiat_p256_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 22fb42eef9..1284ba0835 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -242,13 +242,13 @@ static void p384_inv_square(p384_felem out, } #if defined(EC_NISTP_USE_S2N_BIGNUM) -static nistp_felem_methods p384_felem_methods = { +static ec_nistp_felem_meth p384_felem_methods = { bignum_add_p384, bignum_sub_p384, bignum_montmul_p384, bignum_montsqr_p384 }; #else -static nistp_felem_methods p384_felem_methods = { +static ec_nistp_felem_meth p384_felem_methods = { fiat_p384_add, fiat_p384_sub, fiat_p384_mul, @@ -261,7 +261,7 @@ static void p384_point_double(p384_felem x_out, const p384_felem x_in, const p384_felem y_in, const p384_felem z_in) { - nistp_point_double(&p384_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(&p384_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // p384_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index 16ca196f4f..8a29cfa82a 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -260,13 +260,13 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { } #if defined(EC_NISTP_USE_S2N_BIGNUM) -static nistp_felem_methods p521_felem_methods = { +static ec_nistp_felem_meth p521_felem_methods = { bignum_add_p521, bignum_sub_p521, bignum_mul_p521, bignum_sqr_p521 }; #else -static nistp_felem_methods p521_felem_methods = { +static ec_nistp_felem_meth p521_felem_methods = { fiat_secp521r1_carry_add, fiat_secp521r1_carry_sub, fiat_secp521r1_carry_mul, @@ -279,7 +279,7 @@ static void p521_point_double(p521_felem x_out, const p521_felem x_in, const p521_felem y_in, const p521_felem z_in) { - nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); } // p521_point_add calculates (x1, y1, z1) + (x2, y2, z2) From ef5e5ff4b3a04db60e276b6a51e83c59d301545b Mon Sep 17 00:00:00 2001 From: dkostic Date: Tue, 30 Apr 2024 16:16:45 -0700 Subject: [PATCH 06/10] Call the new s2n-bignum selector functions --- crypto/fipsmodule/ec/p384.c | 4 ++-- crypto/fipsmodule/ec/p521.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 1284ba0835..8ef838e72c 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -245,8 +245,8 @@ static void p384_inv_square(p384_felem out, static ec_nistp_felem_meth p384_felem_methods = { bignum_add_p384, bignum_sub_p384, - bignum_montmul_p384, - bignum_montsqr_p384 }; + bignum_montmul_p384_selector, + bignum_montsqr_p384_selector }; #else static ec_nistp_felem_meth p384_felem_methods = { fiat_p384_add, diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index 8a29cfa82a..390ae9f4c6 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -263,8 +263,8 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { static ec_nistp_felem_meth p521_felem_methods = { bignum_add_p521, bignum_sub_p521, - bignum_mul_p521, - bignum_sqr_p521 }; + bignum_mul_p521_selector, + bignum_sqr_p521_selector }; #else static ec_nistp_felem_meth p521_felem_methods = { fiat_secp521r1_carry_add, From 4b5b47236b1ff39b7272a8f3081ac9b48de533d5 Mon Sep 17 00:00:00 2001 From: dkostic Date: Fri, 3 May 2024 13:30:38 -0700 Subject: [PATCH 07/10] fix the fips build --- crypto/fipsmodule/ec/ec_nistp.c | 5 +---- crypto/fipsmodule/ec/ec_nistp.h | 8 ++++---- crypto/fipsmodule/ec/p256.c | 13 +++++++------ crypto/fipsmodule/ec/p384.c | 24 +++++++++++++----------- crypto/fipsmodule/ec/p521.c | 24 +++++++++++++----------- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index 003c589676..b9332bc847 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -5,7 +5,6 @@ ------------------------------------------------------------------------------------ */ -#if !defined(OPENSSL_SMALL) // In this file we will implement elliptic curve point operations for // NIST curves P-256, P-384, and P-521. The idea is to implement the operations // in a generic way such that the code can be reused instead of having @@ -58,7 +57,7 @@ typedef ec_nistp_felem_limb ec_nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; // // Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; // while x_out == y_in is not (maybe this works, but it's not tested). -void ec_nistp_point_double(ec_nistp_felem_meth *ctx, +void ec_nistp_point_double(const ec_nistp_felem_meth *ctx, ec_nistp_felem_limb *x_out, ec_nistp_felem_limb *y_out, ec_nistp_felem_limb *z_out, @@ -110,5 +109,3 @@ void ec_nistp_point_double(ec_nistp_felem_meth *ctx, ctx->add(gamma, gamma, gamma); ctx->sub(y_out, y_out, gamma); } - -#endif // OPENSSL_SMALL diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h index a8626b2ec6..a80efaf1e0 100644 --- a/crypto/fipsmodule/ec/ec_nistp.h +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -4,8 +4,8 @@ SPDX-License-Identifier: Apache-2.0 OR ISC ------------------------------------------------------------------------------------ */ -#ifndef NISTP_H -#define NISTP_H +#ifndef EC_NISTP_H +#define EC_NISTP_H #include @@ -55,12 +55,12 @@ typedef struct { } ec_nistp_felem_meth; -void ec_nistp_point_double(ec_nistp_felem_meth *ctx, +void ec_nistp_point_double(const ec_nistp_felem_meth *ctx, ec_nistp_felem_limb *x_out, ec_nistp_felem_limb *y_out, ec_nistp_felem_limb *z_out, const ec_nistp_felem_limb *x_in, const ec_nistp_felem_limb *y_in, const ec_nistp_felem_limb *z_in); -#endif // NISTP_H +#endif // EC_NISTP_H diff --git a/crypto/fipsmodule/ec/p256.c b/crypto/fipsmodule/ec/p256.c index 6fd89cf113..7b07308c08 100644 --- a/crypto/fipsmodule/ec/p256.c +++ b/crypto/fipsmodule/ec/p256.c @@ -167,11 +167,12 @@ static void fiat_p256_inv_square(fiat_p256_felem out, fiat_p256_square(out, ret); // 2^256 - 2^224 + 2^192 + 2^96 - 2^2 } -static ec_nistp_felem_meth p256_felem_methods = { - fiat_p256_add, - fiat_p256_sub, - fiat_p256_mul, - fiat_p256_square }; +DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p256_felem_methods) { + out->add = fiat_p256_add; + out->sub = fiat_p256_sub; + out->mul = fiat_p256_mul; + out->sqr = fiat_p256_square; +} static void fiat_p256_point_double(fiat_p256_felem x_out, fiat_p256_felem y_out, @@ -179,7 +180,7 @@ static void fiat_p256_point_double(fiat_p256_felem x_out, const fiat_p256_felem x_in, const fiat_p256_felem y_in, const fiat_p256_felem z_in) { - ec_nistp_point_double(&p256_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(p256_felem_methods(), x_out, y_out, z_out, x_in, y_in, z_in); } // fiat_p256_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 8ef838e72c..1fe5d0705c 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -242,17 +242,19 @@ static void p384_inv_square(p384_felem out, } #if defined(EC_NISTP_USE_S2N_BIGNUM) -static ec_nistp_felem_meth p384_felem_methods = { - bignum_add_p384, - bignum_sub_p384, - bignum_montmul_p384_selector, - bignum_montsqr_p384_selector }; +DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p384_felem_methods) { + out->add = bignum_add_p384; + out->sub = bignum_sub_p384; + out->mul = bignum_montmul_p384_selector; + out->sqr = bignum_montsqr_p384_selector; +} #else -static ec_nistp_felem_meth p384_felem_methods = { - fiat_p384_add, - fiat_p384_sub, - fiat_p384_mul, - fiat_p384_square }; +DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p384_felem_methods) { + out->add = fiat_p384_add; + out->sub = fiat_p384_sub; + out->mul = fiat_p384_mul; + out->sqr = fiat_p384_square; +} #endif static void p384_point_double(p384_felem x_out, @@ -261,7 +263,7 @@ static void p384_point_double(p384_felem x_out, const p384_felem x_in, const p384_felem y_in, const p384_felem z_in) { - ec_nistp_point_double(&p384_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(p384_felem_methods(), x_out, y_out, z_out, x_in, y_in, z_in); } // p384_point_add calculates (x1, y1, z1) + (x2, y2, z2) diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index 390ae9f4c6..92251b983b 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -260,17 +260,19 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { } #if defined(EC_NISTP_USE_S2N_BIGNUM) -static ec_nistp_felem_meth p521_felem_methods = { - bignum_add_p521, - bignum_sub_p521, - bignum_mul_p521_selector, - bignum_sqr_p521_selector }; +DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p521_felem_methods) { + out->add = bignum_add_p521; + out->sub = bignum_sub_p521; + out->mul = bignum_mul_p521_selector; + out->sqr = bignum_sqr_p521_selector; +} #else -static ec_nistp_felem_meth p521_felem_methods = { - fiat_secp521r1_carry_add, - fiat_secp521r1_carry_sub, - fiat_secp521r1_carry_mul, - fiat_secp521r1_carry_square }; +DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p521_felem_methods) { + out->add = fiat_secp521r1_carry_add; + out->sub = fiat_secp521r1_carry_sub; + out->mul = fiat_secp521r1_carry_mul; + out->sqr = fiat_secp521r1_carry_square; +} #endif static void p521_point_double(p521_felem x_out, @@ -279,7 +281,7 @@ static void p521_point_double(p521_felem x_out, const p521_felem x_in, const p521_felem y_in, const p521_felem z_in) { - ec_nistp_point_double(&p521_felem_methods, x_out, y_out, z_out, x_in, y_in, z_in); + ec_nistp_point_double(p521_felem_methods(), x_out, y_out, z_out, x_in, y_in, z_in); } // p521_point_add calculates (x1, y1, z1) + (x2, y2, z2) From e014623e0a61f842fb541fee835f8fcad6c22c2f Mon Sep 17 00:00:00 2001 From: dkostic Date: Fri, 10 May 2024 15:19:28 -0700 Subject: [PATCH 08/10] Attempt to fix the Windows msvc2017 issue --- crypto/fipsmodule/ec/ec_nistp.h | 3 +++ crypto/fipsmodule/ec/p256.c | 2 +- crypto/fipsmodule/ec/p384.c | 4 ++-- crypto/fipsmodule/ec/p521.c | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h index a80efaf1e0..f88f11514f 100644 --- a/crypto/fipsmodule/ec/ec_nistp.h +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -54,6 +54,9 @@ typedef struct { void (*sqr)(ec_nistp_felem_limb *c, const ec_nistp_felem_limb *a); } ec_nistp_felem_meth; +const ec_nistp_felem_meth *p256_felem_methods(void); +const ec_nistp_felem_meth *p384_felem_methods(void); +const ec_nistp_felem_meth *p521_felem_methods(void); void ec_nistp_point_double(const ec_nistp_felem_meth *ctx, ec_nistp_felem_limb *x_out, diff --git a/crypto/fipsmodule/ec/p256.c b/crypto/fipsmodule/ec/p256.c index 7b07308c08..ba81fcc912 100644 --- a/crypto/fipsmodule/ec/p256.c +++ b/crypto/fipsmodule/ec/p256.c @@ -167,7 +167,7 @@ static void fiat_p256_inv_square(fiat_p256_felem out, fiat_p256_square(out, ret); // 2^256 - 2^224 + 2^192 + 2^96 - 2^2 } -DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p256_felem_methods) { +DEFINE_METHOD_FUNCTION(ec_nistp_felem_meth, p256_felem_methods) { out->add = fiat_p256_add; out->sub = fiat_p256_sub; out->mul = fiat_p256_mul; diff --git a/crypto/fipsmodule/ec/p384.c b/crypto/fipsmodule/ec/p384.c index 1fe5d0705c..777fd20b26 100644 --- a/crypto/fipsmodule/ec/p384.c +++ b/crypto/fipsmodule/ec/p384.c @@ -242,14 +242,14 @@ static void p384_inv_square(p384_felem out, } #if defined(EC_NISTP_USE_S2N_BIGNUM) -DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p384_felem_methods) { +DEFINE_METHOD_FUNCTION(ec_nistp_felem_meth, p384_felem_methods) { out->add = bignum_add_p384; out->sub = bignum_sub_p384; out->mul = bignum_montmul_p384_selector; out->sqr = bignum_montsqr_p384_selector; } #else -DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p384_felem_methods) { +DEFINE_METHOD_FUNCTION(ec_nistp_felem_meth, p384_felem_methods) { out->add = fiat_p384_add; out->sub = fiat_p384_sub; out->mul = fiat_p384_mul; diff --git a/crypto/fipsmodule/ec/p521.c b/crypto/fipsmodule/ec/p521.c index 92251b983b..d5ad43c7ec 100644 --- a/crypto/fipsmodule/ec/p521.c +++ b/crypto/fipsmodule/ec/p521.c @@ -260,14 +260,14 @@ static void p521_felem_inv(p521_felem output, const p521_felem t1) { } #if defined(EC_NISTP_USE_S2N_BIGNUM) -DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p521_felem_methods) { +DEFINE_METHOD_FUNCTION(ec_nistp_felem_meth, p521_felem_methods) { out->add = bignum_add_p521; out->sub = bignum_sub_p521; out->mul = bignum_mul_p521_selector; out->sqr = bignum_sqr_p521_selector; } #else -DEFINE_LOCAL_DATA(ec_nistp_felem_meth, p521_felem_methods) { +DEFINE_METHOD_FUNCTION(ec_nistp_felem_meth, p521_felem_methods) { out->add = fiat_secp521r1_carry_add; out->sub = fiat_secp521r1_carry_sub; out->mul = fiat_secp521r1_carry_mul; From 82eb81b23e0c2924dd441932de7ac727b384d7bf Mon Sep 17 00:00:00 2001 From: dkostic <25055813+dkostic@users.noreply.github.com> Date: Wed, 15 May 2024 15:04:53 -0700 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Nevine Ebeid <66388554+nebeid@users.noreply.github.com> --- crypto/fipsmodule/ec/ec_nistp.c | 2 +- crypto/fipsmodule/ec/ec_nistp.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index b9332bc847..e5972d66d8 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -47,7 +47,7 @@ typedef ec_nistp_felem_limb ec_nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; // elliptic curve group itself. Points on the curve are represented in Jacobian // coordinates. // -// p521_point_double calculates 2*(x_in, y_in, z_in) +// ec_nistp_point_double calculates 2*(x_in, y_in, z_in) // // The method is taken from: // http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h index f88f11514f..eec0c54e14 100644 --- a/crypto/fipsmodule/ec/ec_nistp.h +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -38,7 +38,7 @@ typedef uint64_t ec_nistp_felem_limb; typedef uint32_t ec_nistp_felem_limb; #endif -// This is the struct that holds pointers to implementations of field +// ec_nistp_felem_meth is a struct that holds pointers to implementations of field // arithmetic functions for specific curves. It is meant to be used // in higher level functions like this: // void point_double(nistp_felem_methods *ctx, ...) { From 4b55dad303d4e14c02c8c563a8e2fde962b8986e Mon Sep 17 00:00:00 2001 From: dkostic Date: Wed, 15 May 2024 16:10:30 -0700 Subject: [PATCH 10/10] addressing CR --- crypto/fipsmodule/ec/ec_nistp.c | 25 +++++++++++++------------ crypto/fipsmodule/ec/ec_nistp.h | 8 ++------ 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_nistp.c b/crypto/fipsmodule/ec/ec_nistp.c index e5972d66d8..a4484dc06c 100644 --- a/crypto/fipsmodule/ec/ec_nistp.c +++ b/crypto/fipsmodule/ec/ec_nistp.c @@ -1,9 +1,5 @@ -/* ------------------------------------------------------------------------------------- - Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - SPDX-License-Identifier: Apache-2.0 OR ISC ------------------------------------------------------------------------------------- -*/ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC // In this file we will implement elliptic curve point operations for // NIST curves P-256, P-384, and P-521. The idea is to implement the operations @@ -19,8 +15,8 @@ // // | op | P-521 | P-384 | P-256 | // |----------------------------| -// | 1. | x | x | x* | -// | 2. | | | | +// | 1. | | | | +// | 2. | x | x | x* | // | 3. | | | | // | 4. | | | | // | 5. | | | | @@ -49,12 +45,17 @@ typedef ec_nistp_felem_limb ec_nistp_felem[NISTP_FELEM_MAX_NUM_OF_LIMBS]; // // ec_nistp_point_double calculates 2*(x_in, y_in, z_in) // -// The method is taken from: +// The method is based on: // http://hyperelliptic.org/EFD/g1p/auto-shortw-jacobian-3.html#doubling-dbl-2001-b +// for which there is a Coq transcription and correctness proof: +// +// +// +// However, we slighty changed the computation for efficiency (see the full +// explanation within the function body), which makes the Coq proof above +// not applicable to our implementation. +// TODO(awslc): Write a Coq correctness proof for our version of the algorithm. // -// Coq transcription and correctness proof: -// -// // Outputs can equal corresponding inputs, i.e., x_out == x_in is allowed; // while x_out == y_in is not (maybe this works, but it's not tested). void ec_nistp_point_double(const ec_nistp_felem_meth *ctx, diff --git a/crypto/fipsmodule/ec/ec_nistp.h b/crypto/fipsmodule/ec/ec_nistp.h index eec0c54e14..027380581f 100644 --- a/crypto/fipsmodule/ec/ec_nistp.h +++ b/crypto/fipsmodule/ec/ec_nistp.h @@ -1,9 +1,5 @@ -/* ------------------------------------------------------------------------------------- - Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - SPDX-License-Identifier: Apache-2.0 OR ISC ------------------------------------------------------------------------------------- -*/ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC #ifndef EC_NISTP_H #define EC_NISTP_H