From 4edaf06fb02a9ac9cd115e0c967bb0ef35cae01d Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 12 Jul 2019 09:56:56 +0000 Subject: [PATCH 01/22] Add check preventing integer multiplication wrapping around in scratch_max_allocation --- src/scratch_impl.h | 4 ++++ src/tests.c | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/scratch_impl.h b/src/scratch_impl.h index 4cee70000..937e29a0d 100644 --- a/src/scratch_impl.h +++ b/src/scratch_impl.h @@ -60,6 +60,10 @@ static size_t secp256k1_scratch_max_allocation(const secp256k1_callback* error_c secp256k1_callback_call(error_callback, "invalid scratch space"); return 0; } + /* Ensure that multiplication will not wrap around */ + if (ALIGNMENT > 1 && objects > SIZE_MAX/(ALIGNMENT - 1)) { + return 0; + } if (scratch->max_size - scratch->alloc_size <= objects * (ALIGNMENT - 1)) { return 0; } diff --git a/src/tests.c b/src/tests.c index 132df9ba9..990f7d65e 100644 --- a/src/tests.c +++ b/src/tests.c @@ -400,6 +400,14 @@ void run_scratch_tests(void) { secp256k1_scratch_space_destroy(none, scratch); CHECK(ecount == 5); + /* Test that large integers do not wrap around in a bad way */ + scratch = secp256k1_scratch_space_create(none, 1000); + /* Try max allocation with a large number of objects. Only makes sense if + * ALIGNMENT is greater than 1 because otherwise the objects take no extra + * space. */ + CHECK(ALIGNMENT <= 1 || !secp256k1_scratch_max_allocation(&none->error_callback, scratch, (SIZE_MAX / (ALIGNMENT - 1)) + 1)); + secp256k1_scratch_space_destroy(none, scratch); + /* cleanup */ secp256k1_scratch_space_destroy(none, NULL); /* no-op */ secp256k1_context_destroy(none); From 8ecc6ce50ead28a0b8bab2f1fb18a58ee5204a13 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 12 Jul 2019 10:00:39 +0000 Subject: [PATCH 02/22] Add check preventing rounding to alignment from wrapping around in scratch_alloc --- src/scratch_impl.h | 9 ++++++++- src/tests.c | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/scratch_impl.h b/src/scratch_impl.h index 937e29a0d..f53de48be 100644 --- a/src/scratch_impl.h +++ b/src/scratch_impl.h @@ -72,7 +72,14 @@ static size_t secp256k1_scratch_max_allocation(const secp256k1_callback* error_c static void *secp256k1_scratch_alloc(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t size) { void *ret; - size = ROUND_TO_ALIGN(size); + size_t rounded_size; + + rounded_size = ROUND_TO_ALIGN(size); + /* Check that rounding did not wrap around */ + if (rounded_size < size) { + return NULL; + } + size = rounded_size; if (memcmp(scratch->magic, "scratch", 8) != 0) { secp256k1_callback_call(error_callback, "invalid scratch space"); diff --git a/src/tests.c b/src/tests.c index 990f7d65e..94deb4c1d 100644 --- a/src/tests.c +++ b/src/tests.c @@ -406,6 +406,10 @@ void run_scratch_tests(void) { * ALIGNMENT is greater than 1 because otherwise the objects take no extra * space. */ CHECK(ALIGNMENT <= 1 || !secp256k1_scratch_max_allocation(&none->error_callback, scratch, (SIZE_MAX / (ALIGNMENT - 1)) + 1)); + /* Try allocating SIZE_MAX to test wrap around which only happens if + * ALIGNMENT > 1, otherwise it returns NULL anyway because the scratch + * space is too small. */ + CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, SIZE_MAX) == NULL); secp256k1_scratch_space_destroy(none, scratch); /* cleanup */ From ada6361dece4265823478e0019a8c196e9285a26 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 29 Jul 2019 15:50:53 +0000 Subject: [PATCH 03/22] Use ROUND_TO_ALIGN in scratch_create --- src/scratch_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scratch_impl.h b/src/scratch_impl.h index f53de48be..b20562022 100644 --- a/src/scratch_impl.h +++ b/src/scratch_impl.h @@ -11,7 +11,7 @@ #include "scratch.h" static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* error_callback, size_t size) { - const size_t base_alloc = ((sizeof(secp256k1_scratch) + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT; + const size_t base_alloc = ROUND_TO_ALIGN(sizeof(secp256k1_scratch)); void *alloc = checked_malloc(error_callback, base_alloc + size); secp256k1_scratch* ret = (secp256k1_scratch *)alloc; if (ret != NULL) { From 60f7f2de5de917c2bee32a4cd79cc3818b6a94a0 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 30 Jul 2019 14:42:17 +0200 Subject: [PATCH 04/22] Don't assume that ALIGNMENT > 1 in tests --- src/tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests.c b/src/tests.c index 94deb4c1d..0f46cf54a 100644 --- a/src/tests.c +++ b/src/tests.c @@ -366,8 +366,8 @@ void run_scratch_tests(void) { CHECK(scratch->alloc_size != 0); CHECK(scratch->alloc_size % ALIGNMENT == 0); - /* Allocating another 500 bytes fails */ - CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL); + /* Allocating another 501 bytes fails */ + CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 501) == NULL); CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000 - adj_alloc); CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) == 1000 - adj_alloc - (ALIGNMENT - 1)); CHECK(scratch->alloc_size != 0); From 61d1ecb02847be9d65ffe9df2d2408d85f3a0711 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Tue, 10 Dec 2019 18:08:25 +0200 Subject: [PATCH 05/22] Added test with additions resulting in infinity --- src/tests.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/tests.c b/src/tests.c index d1cfcb5d0..15012e831 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2258,6 +2258,39 @@ void test_ge(void) { free(zinv); } + +void test_intialized_inf(void) { + secp256k1_ge p; + secp256k1_gej pj, npj, infj1, infj2, infj3; + secp256k1_fe zinv; + + /* Test that adding P+(-P) results in a fully initalized infinity*/ + random_group_element_test(&p); + secp256k1_gej_set_ge(&pj, &p); + secp256k1_gej_neg(&npj, &pj); + + secp256k1_gej_add_var(&infj1, &pj, &npj, NULL); + CHECK(secp256k1_gej_is_infinity(&infj1)); + CHECK(secp256k1_fe_is_zero(&infj1.x)); + CHECK(secp256k1_fe_is_zero(&infj1.y)); + CHECK(secp256k1_fe_is_zero(&infj1.z)); + + secp256k1_gej_add_ge_var(&infj2, &npj, &p, NULL); + CHECK(secp256k1_gej_is_infinity(&infj2)); + CHECK(secp256k1_fe_is_zero(&infj2.x)); + CHECK(secp256k1_fe_is_zero(&infj2.y)); + CHECK(secp256k1_fe_is_zero(&infj2.z)); + + secp256k1_fe_set_int(&zinv, 1); + secp256k1_gej_add_zinv_var(&infj3, &npj, &p, &zinv); + CHECK(secp256k1_gej_is_infinity(&infj3)); + CHECK(secp256k1_fe_is_zero(&infj3.x)); + CHECK(secp256k1_fe_is_zero(&infj3.y)); + CHECK(secp256k1_fe_is_zero(&infj3.z)); + + +} + void test_add_neg_y_diff_x(void) { /* The point of this test is to check that we can add two points * whose y-coordinates are negatives of each other but whose x @@ -2331,6 +2364,7 @@ void run_ge(void) { test_ge(); } test_add_neg_y_diff_x(); + test_intialized_inf(); } void test_ec_combine(void) { From 47a7b8382fd6f1458d859b315cf3bcd3b9790b68 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Tue, 10 Dec 2019 18:08:53 +0200 Subject: [PATCH 06/22] Clear field elements when writing infinity --- src/group_impl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/group_impl.h b/src/group_impl.h index 9b93c39e9..ade7a5930 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -397,7 +397,7 @@ static void secp256k1_gej_add_var(secp256k1_gej *r, const secp256k1_gej *a, cons if (rzr != NULL) { secp256k1_fe_set_int(rzr, 0); } - r->infinity = 1; + secp256k1_gej_set_infinity(r); } return; } @@ -447,7 +447,7 @@ static void secp256k1_gej_add_ge_var(secp256k1_gej *r, const secp256k1_gej *a, c if (rzr != NULL) { secp256k1_fe_set_int(rzr, 0); } - r->infinity = 1; + secp256k1_gej_set_infinity(r); } return; } @@ -506,7 +506,7 @@ static void secp256k1_gej_add_zinv_var(secp256k1_gej *r, const secp256k1_gej *a, if (secp256k1_fe_normalizes_to_zero_var(&i)) { secp256k1_gej_double_var(r, a, NULL); } else { - r->infinity = 1; + secp256k1_gej_set_infinity(r); } return; } From 39295362cfc856aae1c37cc1194c2f6d53fd6f25 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 28 Jul 2020 17:41:07 -0700 Subject: [PATCH 07/22] Test travis s390x (big endian) --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index a6ad6fb27..491da6c37 100644 --- a/.travis.yml +++ b/.travis.yml @@ -83,6 +83,10 @@ matrix: - valgrind - libtool-bin - libc6-dbg:i386 + # S390x build (big endian system) + - compiler: gcc + env: HOST=s390x-unknown-linux-gnu ECDH=yes RECOVERY=yes EXPERIMENTAL=yes CTIMETEST= + arch: s390x # We use this to install macOS dependencies instead of the built in `homebrew` plugin, # because in xcode earlier than 11 they have a bug requiring updating the system which overall takes ~8 minutes. From 0d7727f95e52d99c13f55c64e9d1f799ba7d7967 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 10 Aug 2020 14:32:28 -0700 Subject: [PATCH 08/22] Add SECP256K1_FE_STORAGE_CONST_GET to 5x52 field So far this has not been needed, as it's only used by the static precomputation which always builds with 32-bit fields. This prepares for the ability to have __int128 detected on the C side, breaking that restriction. --- src/field_5x52.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/field_5x52.h b/src/field_5x52.h index fc5bfe357..6a068484c 100644 --- a/src/field_5x52.h +++ b/src/field_5x52.h @@ -46,4 +46,10 @@ typedef struct { (d6) | (((uint64_t)(d7)) << 32) \ }} +#define SECP256K1_FE_STORAGE_CONST_GET(d) \ + (uint32_t)(d.n[3] >> 32), (uint32_t)d.n[3], \ + (uint32_t)(d.n[2] >> 32), (uint32_t)d.n[2], \ + (uint32_t)(d.n[1] >> 32), (uint32_t)d.n[1], \ + (uint32_t)(d.n[0] >> 32), (uint32_t)d.n[0] + #endif /* SECP256K1_FIELD_REPR_H */ From 79f1f7a4f123765cf07be92ae894d882c5845191 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 9 Aug 2020 10:58:40 -0700 Subject: [PATCH 09/22] Autodetect __int128 availability on the C side Instead of supporting configuration of the field and scalar size independently, both are now controlled by the availability of a 64x64->128 bit multiplication (currently only through __int128). This is autodetected from the C code through __SIZEOF_INT128__, but can be overridden using configure's --with-test-override-wide-multiply, or by defining USE_FORCE_WIDEMUL_{INT64,INT128} manually. --- .travis.yml | 20 +++---- build-aux/m4/bitcoin_secp.m4 | 5 -- configure.ac | 102 ++++++----------------------------- contrib/travis.sh | 2 +- src/basic-config.h | 10 ++-- src/field.h | 12 ++--- src/field_impl.h | 8 +-- src/scalar.h | 7 +-- src/scalar_impl.h | 6 +-- src/util.h | 27 +++++++--- 10 files changed, 67 insertions(+), 132 deletions(-) diff --git a/.travis.yml b/.travis.yml index 491da6c37..81d1ba894 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,17 +17,17 @@ compiler: - gcc env: global: - - FIELD=auto BIGNUM=auto SCALAR=auto ENDOMORPHISM=no STATICPRECOMPUTATION=yes ECMULTGENPRECISION=auto ASM=no BUILD=check EXTRAFLAGS= HOST= ECDH=no RECOVERY=no EXPERIMENTAL=no CTIMETEST=yes BENCH=yes ITERS=2 + - WIDEMUL=auto BIGNUM=auto ENDOMORPHISM=no STATICPRECOMPUTATION=yes ECMULTGENPRECISION=auto ASM=no BUILD=check EXTRAFLAGS= HOST= ECDH=no RECOVERY=no EXPERIMENTAL=no CTIMETEST=yes BENCH=yes ITERS=2 matrix: - - SCALAR=32bit RECOVERY=yes - - SCALAR=32bit FIELD=32bit ECDH=yes EXPERIMENTAL=yes - - SCALAR=64bit - - FIELD=64bit RECOVERY=yes - - FIELD=64bit ENDOMORPHISM=yes - - FIELD=64bit ENDOMORPHISM=yes ECDH=yes EXPERIMENTAL=yes - - FIELD=64bit ASM=x86_64 - - FIELD=64bit ENDOMORPHISM=yes ASM=x86_64 - - FIELD=32bit ENDOMORPHISM=yes + - WIDEMUL=int64 RECOVERY=yes + - WIDEMUL=int64 ECDH=yes EXPERIMENTAL=yes + - WIDEMUL=int64 ENDOMORPHISM=yes + - WIDEMUL=int128 + - WIDEMUL=int128 RECOVERY=yes + - WIDEMUL=int128 ENDOMORPHISM=yes + - WIDEMUL=int128 ENDOMORPHISM=yes ECDH=yes EXPERIMENTAL=yes + - WIDEMUL=int128 ASM=x86_64 + - WIDEMUL=int128 ENDOMORPHISM=yes ASM=x86_64 - BIGNUM=no - BIGNUM=no ENDOMORPHISM=yes RECOVERY=yes EXPERIMENTAL=yes - BIGNUM=no STATICPRECOMPUTATION=no diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index 1b2b71e6a..57595f449 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/build-aux/m4/bitcoin_secp.m4 @@ -1,8 +1,3 @@ -dnl libsecp25k1 helper checks -AC_DEFUN([SECP_INT128_CHECK],[ -has_int128=$ac_cv_type___int128 -]) - dnl escape "$0x" below using the m4 quadrigaph @S|@, and escape it again with a \ for the shell. AC_DEFUN([SECP_64BIT_ASM_CHECK],[ AC_MSG_CHECKING(for x86_64 assembly availability) diff --git a/configure.ac b/configure.ac index 6021b760b..1c01c4f98 100644 --- a/configure.ac +++ b/configure.ac @@ -141,15 +141,13 @@ AC_ARG_ENABLE(external_default_callbacks, [use_external_default_callbacks=$enableval], [use_external_default_callbacks=no]) -AC_ARG_WITH([field], [AS_HELP_STRING([--with-field=64bit|32bit|auto], -[finite field implementation to use [default=auto]])],[req_field=$withval], [req_field=auto]) +dnl Test-only override of the (autodetected by the C code) "widemul" setting. +dnl Legal values are int64 (for [u]int64_t), int128 (for [unsigned] __int128), and auto (the default). +AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto]) AC_ARG_WITH([bignum], [AS_HELP_STRING([--with-bignum=gmp|no|auto], [bignum implementation to use [default=auto]])],[req_bignum=$withval], [req_bignum=auto]) -AC_ARG_WITH([scalar], [AS_HELP_STRING([--with-scalar=64bit|32bit|auto], -[scalar implementation to use [default=auto]])],[req_scalar=$withval], [req_scalar=auto]) - AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto], [assembly optimizations to useĀ (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto]) @@ -170,8 +168,6 @@ AC_ARG_WITH([ecmult-gen-precision], [AS_HELP_STRING([--with-ecmult-gen-precision )], [req_ecmult_gen_precision=$withval], [req_ecmult_gen_precision=auto]) -AC_CHECK_TYPES([__int128]) - AC_CHECK_HEADER([valgrind/memcheck.h], [enable_valgrind=yes], [enable_valgrind=no], []) AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"]) @@ -265,63 +261,6 @@ else esac fi -if test x"$req_field" = x"auto"; then - if test x"set_asm" = x"x86_64"; then - set_field=64bit - fi - if test x"$set_field" = x; then - SECP_INT128_CHECK - if test x"$has_int128" = x"yes"; then - set_field=64bit - fi - fi - if test x"$set_field" = x; then - set_field=32bit - fi -else - set_field=$req_field - case $set_field in - 64bit) - if test x"$set_asm" != x"x86_64"; then - SECP_INT128_CHECK - if test x"$has_int128" != x"yes"; then - AC_MSG_ERROR([64bit field explicitly requested but neither __int128 support or x86_64 assembly available]) - fi - fi - ;; - 32bit) - ;; - *) - AC_MSG_ERROR([invalid field implementation selection]) - ;; - esac -fi - -if test x"$req_scalar" = x"auto"; then - SECP_INT128_CHECK - if test x"$has_int128" = x"yes"; then - set_scalar=64bit - fi - if test x"$set_scalar" = x; then - set_scalar=32bit - fi -else - set_scalar=$req_scalar - case $set_scalar in - 64bit) - SECP_INT128_CHECK - if test x"$has_int128" != x"yes"; then - AC_MSG_ERROR([64bit scalar explicitly requested but __int128 support not available]) - fi - ;; - 32bit) - ;; - *) - AC_MSG_ERROR([invalid scalar implementation selected]) - ;; - esac -fi - if test x"$req_bignum" = x"auto"; then SECP_GMP_CHECK if test x"$has_gmp" = x"yes"; then @@ -365,16 +304,18 @@ no) ;; esac -# select field implementation -case $set_field in -64bit) - AC_DEFINE(USE_FIELD_5X52, 1, [Define this symbol to use the FIELD_5X52 implementation]) +# select wide multiplication implementation +case $set_widemul in +int128) + AC_DEFINE(USE_FORCE_WIDEMUL_INT128, 1, [Define this symbol to force the use of the (unsigned) __int128 based wide multiplication implementation]) ;; -32bit) - AC_DEFINE(USE_FIELD_10X26, 1, [Define this symbol to use the FIELD_10X26 implementation]) +int64) + AC_DEFINE(USE_FORCE_WIDEMUL_INT64, 1, [Define this symbol to force the use of the (u)int64_t based wide multiplication implementation]) + ;; +auto) ;; *) - AC_MSG_ERROR([invalid field implementation]) + AC_MSG_ERROR([invalid wide multiplication implementation]) ;; esac @@ -396,19 +337,6 @@ no) ;; esac -#select scalar implementation -case $set_scalar in -64bit) - AC_DEFINE(USE_SCALAR_4X64, 1, [Define this symbol to use the 4x64 scalar implementation]) - ;; -32bit) - AC_DEFINE(USE_SCALAR_8X32, 1, [Define this symbol to use the 8x32 scalar implementation]) - ;; -*) - AC_MSG_ERROR([invalid scalar implementation]) - ;; -esac - #set ecmult window size if test x"$req_ecmult_window" = x"auto"; then set_ecmult_window=15 @@ -553,10 +481,12 @@ echo " module recovery = $enable_module_recovery" echo echo " asm = $set_asm" echo " bignum = $set_bignum" -echo " field = $set_field" -echo " scalar = $set_scalar" echo " ecmult window size = $set_ecmult_window" echo " ecmult gen prec. bits = $set_ecmult_gen_precision" +dnl Hide test-only options unless they're used. +if test x"$set_widemul" != xauto; then +echo " wide multiplication = $set_widemul" +fi echo echo " valgrind = $enable_valgrind" echo " CC = $CC" diff --git a/contrib/travis.sh b/contrib/travis.sh index 2c0cbcd6b..2023d0b5a 100755 --- a/contrib/travis.sh +++ b/contrib/travis.sh @@ -14,7 +14,7 @@ fi ./configure \ --enable-experimental="$EXPERIMENTAL" --enable-endomorphism="$ENDOMORPHISM" \ - --with-field="$FIELD" --with-bignum="$BIGNUM" --with-asm="$ASM" --with-scalar="$SCALAR" \ + --with-test-override-wide-multiply="$WIDEMUL" --with-bignum="$BIGNUM" --with-asm="$ASM" \ --enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \ --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \ --host="$HOST" $EXTRAFLAGS diff --git a/src/basic-config.h b/src/basic-config.h index b5ccd8398..83dbe6f25 100644 --- a/src/basic-config.h +++ b/src/basic-config.h @@ -14,24 +14,20 @@ #undef USE_ENDOMORPHISM #undef USE_EXTERNAL_ASM #undef USE_EXTERNAL_DEFAULT_CALLBACKS -#undef USE_FIELD_10X26 -#undef USE_FIELD_5X52 #undef USE_FIELD_INV_BUILTIN #undef USE_FIELD_INV_NUM #undef USE_NUM_GMP #undef USE_NUM_NONE -#undef USE_SCALAR_4X64 -#undef USE_SCALAR_8X32 #undef USE_SCALAR_INV_BUILTIN #undef USE_SCALAR_INV_NUM +#undef USE_FORCE_WIDEMUL_INT64 +#undef USE_FORCE_WIDEMUL_INT128 #undef ECMULT_WINDOW_SIZE -#undef HAVE___INT128 /* used in util.h */ #define USE_NUM_NONE 1 #define USE_FIELD_INV_BUILTIN 1 #define USE_SCALAR_INV_BUILTIN 1 -#define USE_FIELD_10X26 1 -#define USE_SCALAR_8X32 1 +#define USE_WIDEMUL_64 1 #define ECMULT_WINDOW_SIZE 15 #endif /* USE_BASIC_CONFIG */ diff --git a/src/field.h b/src/field.h index 7993a1f11..aca1fb72c 100644 --- a/src/field.h +++ b/src/field.h @@ -22,16 +22,16 @@ #include "libsecp256k1-config.h" #endif -#if defined(USE_FIELD_10X26) -#include "field_10x26.h" -#elif defined(USE_FIELD_5X52) +#include "util.h" + +#if defined(SECP256K1_WIDEMUL_INT128) #include "field_5x52.h" +#elif defined(SECP256K1_WIDEMUL_INT64) +#include "field_10x26.h" #else -#error "Please select field implementation" +#error "Please select wide multiplication implementation" #endif -#include "util.h" - /** Normalize a field element. This brings the field element to a canonical representation, reduces * its magnitude to 1, and reduces it modulo field size `p`. */ diff --git a/src/field_impl.h b/src/field_impl.h index 485921a60..18e4d2f30 100644 --- a/src/field_impl.h +++ b/src/field_impl.h @@ -14,12 +14,12 @@ #include "util.h" #include "num.h" -#if defined(USE_FIELD_10X26) -#include "field_10x26_impl.h" -#elif defined(USE_FIELD_5X52) +#if defined(SECP256K1_WIDEMUL_INT128) #include "field_5x52_impl.h" +#elif defined(SECP256K1_WIDEMUL_INT64) +#include "field_10x26_impl.h" #else -#error "Please select field implementation" +#error "Please select wide multiplication implementation" #endif SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { diff --git a/src/scalar.h b/src/scalar.h index 2a7470352..95d3e326c 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -8,6 +8,7 @@ #define SECP256K1_SCALAR_H #include "num.h" +#include "util.h" #if defined HAVE_CONFIG_H #include "libsecp256k1-config.h" @@ -15,12 +16,12 @@ #if defined(EXHAUSTIVE_TEST_ORDER) #include "scalar_low.h" -#elif defined(USE_SCALAR_4X64) +#elif defined(SECP256K1_WIDEMUL_INT128) #include "scalar_4x64.h" -#elif defined(USE_SCALAR_8X32) +#elif defined(SECP256K1_WIDEMUL_INT64) #include "scalar_8x32.h" #else -#error "Please select scalar implementation" +#error "Please select wide multiplication implementation" #endif /** Clear a scalar to prevent the leak of sensitive data. */ diff --git a/src/scalar_impl.h b/src/scalar_impl.h index 70cd73db0..2ec04b1ae 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -16,12 +16,12 @@ #if defined(EXHAUSTIVE_TEST_ORDER) #include "scalar_low_impl.h" -#elif defined(USE_SCALAR_4X64) +#elif defined(SECP256K1_WIDEMUL_INT128) #include "scalar_4x64_impl.h" -#elif defined(USE_SCALAR_8X32) +#elif defined(SECP256K1_WIDEMUL_INT64) #include "scalar_8x32_impl.h" #else -#error "Please select scalar implementation" +#error "Please select wide multiplication implementation" #endif static const secp256k1_scalar secp256k1_scalar_one = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 1); diff --git a/src/util.h b/src/util.h index 17f6b1851..c14bf0e5e 100644 --- a/src/util.h +++ b/src/util.h @@ -170,13 +170,10 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz # define I64uFORMAT "llu" #endif -#if defined(HAVE___INT128) -# if defined(__GNUC__) -# define SECP256K1_GNUC_EXT __extension__ -# else -# define SECP256K1_GNUC_EXT -# endif -SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; +#if defined(__GNUC__) +# define SECP256K1_GNUC_EXT __extension__ +#else +# define SECP256K1_GNUC_EXT #endif /* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */ @@ -213,4 +210,20 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) *r = (int)(r_masked | a_masked); } +/* If USE_FORCE_WIDEMUL_{INT128,INT64} is set, use that wide multiplication implementation. + * Otherwise use the presence of __SIZEOF_INT128__ to decide. + */ +#if defined(USE_FORCE_WIDEMUL_INT128) +# define SECP256K1_WIDEMUL_INT128 1 +#elif defined(USE_FORCE_WIDEMUL_INT64) +# define SECP256K1_WIDEMUL_INT64 1 +#elif defined(__SIZEOF_INT128__) +# define SECP256K1_WIDEMUL_INT128 1 +#else +# define SECP256K1_WIDEMUL_INT64 1 +#endif +#if defined(SECP256K1_WIDEMUL_INT128) +SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; +#endif + #endif /* SECP256K1_UTIL_H */ From 57d3a3c64cf3d435d5d45e323cf9cbe21da8c6cf Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 10 Aug 2020 22:13:43 +0000 Subject: [PATCH 10/22] Avoid linking libcrypto in the valgrind ct test. Libcrypto isn't useful here and on some systems UB in OpenSSL's init causes failures. Fixes #775. --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index d8c1c79e8..a30a84cd5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -99,7 +99,7 @@ if VALGRIND_ENABLED tests_CPPFLAGS += -DVALGRIND noinst_PROGRAMS += valgrind_ctime_test valgrind_ctime_test_SOURCES = src/valgrind_ctime_test.c -valgrind_ctime_test_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB) +valgrind_ctime_test_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_LIBS) $(COMMON_LIB) endif if !ENABLE_COVERAGE tests_CPPFLAGS += -DVERIFY From 0dccf98a21beb245f6cd9ed76fb7368529df09c7 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Tue, 21 Jul 2020 14:05:56 +0200 Subject: [PATCH 11/22] Use preprocessor macros instead of autoconf to detect endianness This does not fix any particular issue but it's preferable to not rely on autoconf. This avoids endianness mess for users on BE hosts if they use their build without autoconf. The macros are carefully written to err on the side of the caution, e.g., we #error if the user manually configures a different endianness than what we detect. --- configure.ac | 2 -- src/hash_impl.h | 5 +++-- src/util.h | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 6021b760b..2fd340a6a 100644 --- a/configure.ac +++ b/configure.ac @@ -493,8 +493,6 @@ if test x"$enable_module_recovery" = x"yes"; then AC_DEFINE(ENABLE_MODULE_RECOVERY, 1, [Define this symbol to enable the ECDSA pubkey recovery module]) fi -AC_C_BIGENDIAN() - if test x"$use_external_asm" = x"yes"; then AC_DEFINE(USE_EXTERNAL_ASM, 1, [Define this symbol if an external (non-inline) assembly implementation is used]) fi diff --git a/src/hash_impl.h b/src/hash_impl.h index 782f97216..1985a0783 100644 --- a/src/hash_impl.h +++ b/src/hash_impl.h @@ -8,6 +8,7 @@ #define SECP256K1_HASH_IMPL_H #include "hash.h" +#include "util.h" #include #include @@ -27,9 +28,9 @@ (h) = t1 + t2; \ } while(0) -#ifdef WORDS_BIGENDIAN +#if defined(SECP256K1_BIG_ENDIAN) #define BE32(x) (x) -#else +#elif defined(SECP256K1_LITTLE_ENDIAN) #define BE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24)) #endif diff --git a/src/util.h b/src/util.h index 8289e23e0..befb9dccd 100644 --- a/src/util.h +++ b/src/util.h @@ -179,6 +179,20 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; #endif +#if defined(__BYTE_ORDER__) +# if defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && !defined(SECP256K1_LITTLE_ENDIAN) +# define SECP256K1_LITTLE_ENDIAN +# elif defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ && !defined(SECP256K1_BIG_ENDIAN) +# define SECP256K1_BIG_ENDIAN +# endif +#endif +#if defined(_MSC_VER) && defined(_WIN32) && !defined(SECP256K1_LITTLE_ENDIAN) +# define SECP256K1_LITTLE_ENDIAN +#endif +#if defined(SECP256K1_LITTLE_ENDIAN) == defined(SECP256K1_BIG_ENDIAN) +# error Please make sure that either SECP256K1_LITTLE_ENDIAN or SECP256K1_BIG_ENDIAN is set, see src/util.h. +#endif + /* Zero memory if flag == 1. Flag must be 0 or 1. Constant time. */ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { unsigned char *p = (unsigned char *)s; From 02b6c87b52dbac1557b689ab2ebc8b91d67fd0f3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 12 Aug 2020 17:41:08 -0700 Subject: [PATCH 12/22] Add support for (signed) __int128 --- src/util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util.h b/src/util.h index eb4db4faf..1070e0aca 100644 --- a/src/util.h +++ b/src/util.h @@ -238,6 +238,7 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) #endif #if defined(SECP256K1_WIDEMUL_INT128) SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t; +SECP256K1_GNUC_EXT typedef __int128 int128_t; #endif #endif /* SECP256K1_UTIL_H */ From 7c068998bac3e4a254d8542458b2068e38fca435 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 12 Aug 2020 15:52:20 -0700 Subject: [PATCH 13/22] Compile-time check assumptions on integer types --- Makefile.am | 1 + src/assumptions.h | 74 +++++++++++++++++++++++++++++++++++++++ src/bench_internal.c | 1 + src/gen_context.c | 1 + src/secp256k1.c | 1 + src/tests_exhaustive.c | 1 + src/valgrind_ctime_test.c | 1 + 7 files changed, 80 insertions(+) create mode 100644 src/assumptions.h diff --git a/Makefile.am b/Makefile.am index a30a84cd5..1c19e14de 100644 --- a/Makefile.am +++ b/Makefile.am @@ -34,6 +34,7 @@ noinst_HEADERS += src/field_5x52.h noinst_HEADERS += src/field_5x52_impl.h noinst_HEADERS += src/field_5x52_int128_impl.h noinst_HEADERS += src/field_5x52_asm_impl.h +noinst_HEADERS += src/assumptions.h noinst_HEADERS += src/util.h noinst_HEADERS += src/scratch.h noinst_HEADERS += src/scratch_impl.h diff --git a/src/assumptions.h b/src/assumptions.h new file mode 100644 index 000000000..f9d4e8e79 --- /dev/null +++ b/src/assumptions.h @@ -0,0 +1,74 @@ +/********************************************************************** + * Copyright (c) 2020 Pieter Wuille * + * Distributed under the MIT software license, see the accompanying * + * file COPYING or http://www.opensource.org/licenses/mit-license.php.* + **********************************************************************/ + +#ifndef SECP256K1_ASSUMPTIONS_H +#define SECP256K1_ASSUMPTIONS_H + +#include "util.h" + +/* This library, like most software, relies on a number of compiler implementation defined (but not undefined) + behaviours. Although the behaviours we require are essentially universal we test them specifically here to + reduce the odds of experiencing an unwelcome surprise. +*/ + +struct secp256k1_assumption_checker { + /* This uses a trick to implement a static assertion in C89: a type with an array of negative size is not + allowed. */ + int dummy_array[( + /* Bytes are 8 bits. */ + CHAR_BIT == 8 && + + /* Conversions from unsigned to signed outside of the bounds of the signed type are + implementation-defined. Verify that they function as reinterpreting the lower + bits of the input in two's complement notation. Do this for conversions: + - from uint(N)_t to int(N)_t with negative result + - from uint(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with positive result */ + + /* To int8_t. */ + ((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55) && + ((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33) && + ((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF) && + ((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34) && + + /* To int16_t. */ + ((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322) && + ((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C) && + ((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4) && + ((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678) && + + /* To int32_t. */ + ((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B) && + ((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE) && + ((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8) && + ((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789) && + + /* To int64_t. */ + ((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) && +#if defined(SECP256K1_WIDEMUL_INT128) + ((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) && + (((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) && + (((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) && + + /* To int128_t. */ + ((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)) && +#endif + + /* Right shift on negative signed values is implementation defined. Verify that it + acts as a right shift in two's complement with sign extension (i.e duplicating + the top bit into newly added bits). */ + ((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA) && + ((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) && + ((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) && + ((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) && +#if defined(SECP256K1_WIDEMUL_INT128) + ((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) && +#endif + 1) * 2 - 1]; +}; + +#endif /* SECP256K1_ASSUMPTIONS_H */ diff --git a/src/bench_internal.c b/src/bench_internal.c index 20759127d..cb8368ddf 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -7,6 +7,7 @@ #include "include/secp256k1.h" +#include "assumptions.h" #include "util.h" #include "hash_impl.h" #include "num_impl.h" diff --git a/src/gen_context.c b/src/gen_context.c index 539f574bf..8b7729aee 100644 --- a/src/gen_context.c +++ b/src/gen_context.c @@ -13,6 +13,7 @@ #include "basic-config.h" #include "include/secp256k1.h" +#include "assumptions.h" #include "util.h" #include "field_impl.h" #include "scalar_impl.h" diff --git a/src/secp256k1.c b/src/secp256k1.c index 3e7926503..b43a9592e 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -7,6 +7,7 @@ #include "include/secp256k1.h" #include "include/secp256k1_preallocated.h" +#include "assumptions.h" #include "util.h" #include "num_impl.h" #include "field_impl.h" diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index 8f346701a..681ed80bd 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -22,6 +22,7 @@ #endif #include "include/secp256k1.h" +#include "assumptions.h" #include "group.h" #include "secp256k1.c" #include "testrand_impl.h" diff --git a/src/valgrind_ctime_test.c b/src/valgrind_ctime_test.c index 60a82d599..090fde2b6 100644 --- a/src/valgrind_ctime_test.c +++ b/src/valgrind_ctime_test.c @@ -6,6 +6,7 @@ #include #include "include/secp256k1.h" +#include "assumptions.h" #include "util.h" #if ENABLE_MODULE_ECDH From 5e5fb28b4a45d7e35e55b5f5feead2be07bccc28 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 14 Aug 2020 11:49:34 -0700 Subject: [PATCH 14/22] Use additional system macros to figure out endianness Also permit it being overridden by explicitly passing SECP256K1_{BIG,LITTLE}_ENDIAN --- src/util.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/util.h b/src/util.h index eb4db4faf..e8d7015c5 100644 --- a/src/util.h +++ b/src/util.h @@ -176,16 +176,27 @@ static SECP256K1_INLINE void *manual_alloc(void** prealloc_ptr, size_t alloc_siz # define SECP256K1_GNUC_EXT #endif -#if defined(__BYTE_ORDER__) -# if defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && !defined(SECP256K1_LITTLE_ENDIAN) +/* If SECP256K1_{LITTLE,BIG}_ENDIAN is not explicitly provided, infer from various other system macros. */ +#if !defined(SECP256K1_LITTLE_ENDIAN) && !defined(SECP256K1_BIG_ENDIAN) +/* Inspired by https://github.com/rofl0r/endianness.h/blob/9853923246b065a3b52d2c43835f3819a62c7199/endianness.h#L52L73 */ +# if (defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) && __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) || \ + defined(_X86_) || defined(__x86_64__) || defined(__i386__) || \ + defined(__i486__) || defined(__i586__) || defined(__i686__) || \ + defined(__MIPSEL) || defined(_MIPSEL) || defined(MIPSEL) || \ + defined(__ARMEL__) || defined(__AARCH64EL__) || \ + (defined(__LITTLE_ENDIAN__) && __LITTLE_ENDIAN__ == 1) || \ + (defined(_LITTLE_ENDIAN) && _LITTLE_ENDIAN == 1) || \ + defined(_M_IX86) || defined(_M_AMD64) || defined(_M_ARM) /* MSVC */ # define SECP256K1_LITTLE_ENDIAN -# elif defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ && !defined(SECP256K1_BIG_ENDIAN) +# endif +# if (defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) || \ + defined(__MIPSEB) || defined(_MIPSEB) || defined(MIPSEB) || \ + defined(__MICROBLAZEEB__) || defined(__ARMEB__) || defined(__AARCH64EB__) || \ + (defined(__BIG_ENDIAN__) && __BIG_ENDIAN__ == 1) || \ + (defined(_BIG_ENDIAN) && _BIG_ENDIAN == 1) # define SECP256K1_BIG_ENDIAN # endif #endif -#if defined(_MSC_VER) && defined(_WIN32) && !defined(SECP256K1_LITTLE_ENDIAN) -# define SECP256K1_LITTLE_ENDIAN -#endif #if defined(SECP256K1_LITTLE_ENDIAN) == defined(SECP256K1_BIG_ENDIAN) # error Please make sure that either SECP256K1_LITTLE_ENDIAN or SECP256K1_BIG_ENDIAN is set, see src/util.h. #endif From 8bc6aeffa9a191e677cb9e3a22fff130f16990f3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 17 Aug 2020 13:48:22 -0700 Subject: [PATCH 15/22] Add SHA256 selftest --- Makefile.am | 1 + src/secp256k1.c | 4 ++++ src/selftest.h | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 src/selftest.h diff --git a/Makefile.am b/Makefile.am index a30a84cd5..1d9bc7a2c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -37,6 +37,7 @@ noinst_HEADERS += src/field_5x52_asm_impl.h noinst_HEADERS += src/util.h noinst_HEADERS += src/scratch.h noinst_HEADERS += src/scratch_impl.h +noinst_HEADERS += src/selftest.h noinst_HEADERS += src/testrand.h noinst_HEADERS += src/testrand_impl.h noinst_HEADERS += src/hash.h diff --git a/src/secp256k1.c b/src/secp256k1.c index 3e7926503..4b7e82d26 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -19,6 +19,7 @@ #include "eckey_impl.h" #include "hash_impl.h" #include "scratch_impl.h" +#include "selftest.h" #if defined(VALGRIND) # include @@ -117,6 +118,9 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne size_t prealloc_size; secp256k1_context* ret; + if (!secp256k1_selftest()) { + secp256k1_callback_call(&default_error_callback, "self test failed"); + } VERIFY_CHECK(prealloc != NULL); prealloc_size = secp256k1_context_preallocated_size(flags); ret = (secp256k1_context*)manual_alloc(&prealloc, sizeof(secp256k1_context), base, prealloc_size); diff --git a/src/selftest.h b/src/selftest.h new file mode 100644 index 000000000..885983aa2 --- /dev/null +++ b/src/selftest.h @@ -0,0 +1,32 @@ +/********************************************************************** + * Copyright (c) 2020 Pieter Wuille * + * Distributed under the MIT software license, see the accompanying * + * file COPYING or http://www.opensource.org/licenses/mit-license.php.* + **********************************************************************/ + +#ifndef SECP256K1_SELFTEST_H +#define SECP256K1_SELFTEST_H + +#include "hash.h" + +#include + +static int secp256k1_selftest_sha256(void) { + static const char *input63 = "For this sample, this 63-byte string will be used as input data"; + static const unsigned char output32[32] = { + 0xf0, 0x8a, 0x78, 0xcb, 0xba, 0xee, 0x08, 0x2b, 0x05, 0x2a, 0xe0, 0x70, 0x8f, 0x32, 0xfa, 0x1e, + 0x50, 0xc5, 0xc4, 0x21, 0xaa, 0x77, 0x2b, 0xa5, 0xdb, 0xb4, 0x06, 0xa2, 0xea, 0x6b, 0xe3, 0x42, + }; + unsigned char out[32]; + secp256k1_sha256 hasher; + secp256k1_sha256_initialize(&hasher); + secp256k1_sha256_write(&hasher, (const unsigned char*)input63, 63); + secp256k1_sha256_finalize(&hasher, out); + return memcmp(out, output32, 32) == 0; +} + +static int secp256k1_selftest(void) { + return secp256k1_selftest_sha256(); +} + +#endif /* SECP256K1_SELFTEST_H */ From 1c325199d590e018cdfb5ea2ab541774009bf7f7 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 31 Aug 2020 23:11:41 +0000 Subject: [PATCH 16/22] Remove the extremely outdated TODO file. This had two things in it-- tests for the scalar/field code and constant time signing and keygen. The signing and keygen have been thoroughly constant time for years and there are now powerful tests to verify it... no further work on constant-time is needed at least on ordinary platforms (other sidechannels-- sure). The scalar and field code have extensive tests. They could use better static test vectors but they're well tested. TODOs for the project are currently better documented on github right now. This file could return in the future with current info, if needed. --- TODO | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 TODO diff --git a/TODO b/TODO deleted file mode 100644 index a300e1c5e..000000000 --- a/TODO +++ /dev/null @@ -1,3 +0,0 @@ -* Unit tests for fieldelem/groupelem, including ones intended to - trigger fieldelem's boundary cases. -* Complete constant-time operations for signing/keygen From bceefd6547635132ba17f022a52db18f17e00df6 Mon Sep 17 00:00:00 2001 From: Jake Rawsthorne Date: Tue, 1 Sep 2020 00:35:47 +0100 Subject: [PATCH 17/22] Add test logs to gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index cb4331aa9..c061705f4 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,8 @@ libtool *.lo *.o *~ +*.log +*.trs src/libsecp256k1-config.h src/libsecp256k1-config.h.in src/ecmult_static_context.h From c7a3424c5f45a538ef141402a653b038e050a1ac Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 11 Aug 2020 10:50:01 -0700 Subject: [PATCH 18/22] Rename bench_internal variables The _x and _y suffices are confusing; they don't actually correspond to X and Y coordinates. Instead replace them with arrays. --- src/bench_internal.c | 117 ++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/src/bench_internal.c b/src/bench_internal.c index cb8368ddf..011e7161f 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -20,10 +20,10 @@ #include "secp256k1.c" typedef struct { - secp256k1_scalar scalar_x, scalar_y; - secp256k1_fe fe_x, fe_y; - secp256k1_ge ge_x, ge_y; - secp256k1_gej gej_x, gej_y; + secp256k1_scalar scalar[2]; + secp256k1_fe fe[2]; + secp256k1_ge ge[2]; + secp256k1_gej gej[2]; unsigned char data[64]; int wnaf[256]; } bench_inv; @@ -31,30 +31,31 @@ typedef struct { void bench_setup(void* arg) { bench_inv *data = (bench_inv*)arg; - static const unsigned char init_x[32] = { - 0x02, 0x03, 0x05, 0x07, 0x0b, 0x0d, 0x11, 0x13, - 0x17, 0x1d, 0x1f, 0x25, 0x29, 0x2b, 0x2f, 0x35, - 0x3b, 0x3d, 0x43, 0x47, 0x49, 0x4f, 0x53, 0x59, - 0x61, 0x65, 0x67, 0x6b, 0x6d, 0x71, 0x7f, 0x83 + static const unsigned char init[2][32] = { + { + 0x02, 0x03, 0x05, 0x07, 0x0b, 0x0d, 0x11, 0x13, + 0x17, 0x1d, 0x1f, 0x25, 0x29, 0x2b, 0x2f, 0x35, + 0x3b, 0x3d, 0x43, 0x47, 0x49, 0x4f, 0x53, 0x59, + 0x61, 0x65, 0x67, 0x6b, 0x6d, 0x71, 0x7f, 0x83 + }, + { + 0x82, 0x83, 0x85, 0x87, 0x8b, 0x8d, 0x81, 0x83, + 0x97, 0xad, 0xaf, 0xb5, 0xb9, 0xbb, 0xbf, 0xc5, + 0xdb, 0xdd, 0xe3, 0xe7, 0xe9, 0xef, 0xf3, 0xf9, + 0x11, 0x15, 0x17, 0x1b, 0x1d, 0xb1, 0xbf, 0xd3 + } }; - static const unsigned char init_y[32] = { - 0x82, 0x83, 0x85, 0x87, 0x8b, 0x8d, 0x81, 0x83, - 0x97, 0xad, 0xaf, 0xb5, 0xb9, 0xbb, 0xbf, 0xc5, - 0xdb, 0xdd, 0xe3, 0xe7, 0xe9, 0xef, 0xf3, 0xf9, - 0x11, 0x15, 0x17, 0x1b, 0x1d, 0xb1, 0xbf, 0xd3 - }; - - secp256k1_scalar_set_b32(&data->scalar_x, init_x, NULL); - secp256k1_scalar_set_b32(&data->scalar_y, init_y, NULL); - secp256k1_fe_set_b32(&data->fe_x, init_x); - secp256k1_fe_set_b32(&data->fe_y, init_y); - CHECK(secp256k1_ge_set_xo_var(&data->ge_x, &data->fe_x, 0)); - CHECK(secp256k1_ge_set_xo_var(&data->ge_y, &data->fe_y, 1)); - secp256k1_gej_set_ge(&data->gej_x, &data->ge_x); - secp256k1_gej_set_ge(&data->gej_y, &data->ge_y); - memcpy(data->data, init_x, 32); - memcpy(data->data + 32, init_y, 32); + secp256k1_scalar_set_b32(&data->scalar[0], init[0], NULL); + secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL); + secp256k1_fe_set_b32(&data->fe[0], init[0]); + secp256k1_fe_set_b32(&data->fe[1], init[1]); + CHECK(secp256k1_ge_set_xo_var(&data->ge[0], &data->fe[0], 0)); + CHECK(secp256k1_ge_set_xo_var(&data->ge[1], &data->fe[1], 1)); + secp256k1_gej_set_ge(&data->gej[0], &data->ge[0]); + secp256k1_gej_set_ge(&data->gej[1], &data->ge[1]); + memcpy(data->data, init[0], 32); + memcpy(data->data + 32, init[1], 32); } void bench_scalar_add(void* arg, int iters) { @@ -62,7 +63,7 @@ void bench_scalar_add(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - j += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + j += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(j <= iters); } @@ -72,7 +73,7 @@ void bench_scalar_negate(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_negate(&data->scalar_x, &data->scalar_x); + secp256k1_scalar_negate(&data->scalar[0], &data->scalar[0]); } } @@ -81,7 +82,7 @@ void bench_scalar_sqr(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_sqr(&data->scalar_x, &data->scalar_x); + secp256k1_scalar_sqr(&data->scalar[0], &data->scalar[0]); } } @@ -90,7 +91,7 @@ void bench_scalar_mul(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_mul(&data->scalar_x, &data->scalar_x, &data->scalar_y); + secp256k1_scalar_mul(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } } @@ -100,8 +101,8 @@ void bench_scalar_split(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_split_lambda(&data->scalar_x, &data->scalar_y, &data->scalar_x); - j += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + secp256k1_scalar_split_lambda(&data->scalar[0], &data->scalar[1], &data->scalar[0]); + j += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(j <= iters); } @@ -112,8 +113,8 @@ void bench_scalar_inverse(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_inverse(&data->scalar_x, &data->scalar_x); - j += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + secp256k1_scalar_inverse(&data->scalar[0], &data->scalar[0]); + j += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(j <= iters); } @@ -123,8 +124,8 @@ void bench_scalar_inverse_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_scalar_inverse_var(&data->scalar_x, &data->scalar_x); - j += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + secp256k1_scalar_inverse_var(&data->scalar[0], &data->scalar[0]); + j += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(j <= iters); } @@ -134,7 +135,7 @@ void bench_field_normalize(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_normalize(&data->fe_x); + secp256k1_fe_normalize(&data->fe[0]); } } @@ -143,7 +144,7 @@ void bench_field_normalize_weak(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_normalize_weak(&data->fe_x); + secp256k1_fe_normalize_weak(&data->fe[0]); } } @@ -152,7 +153,7 @@ void bench_field_mul(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_mul(&data->fe_x, &data->fe_x, &data->fe_y); + secp256k1_fe_mul(&data->fe[0], &data->fe[0], &data->fe[1]); } } @@ -161,7 +162,7 @@ void bench_field_sqr(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_sqr(&data->fe_x, &data->fe_x); + secp256k1_fe_sqr(&data->fe[0], &data->fe[0]); } } @@ -170,8 +171,8 @@ void bench_field_inverse(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_inv(&data->fe_x, &data->fe_x); - secp256k1_fe_add(&data->fe_x, &data->fe_y); + secp256k1_fe_inv(&data->fe[0], &data->fe[0]); + secp256k1_fe_add(&data->fe[0], &data->fe[1]); } } @@ -180,8 +181,8 @@ void bench_field_inverse_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_fe_inv_var(&data->fe_x, &data->fe_x); - secp256k1_fe_add(&data->fe_x, &data->fe_y); + secp256k1_fe_inv_var(&data->fe[0], &data->fe[0]); + secp256k1_fe_add(&data->fe[0], &data->fe[1]); } } @@ -191,9 +192,9 @@ void bench_field_sqrt(void* arg, int iters) { secp256k1_fe t; for (i = 0; i < iters; i++) { - t = data->fe_x; - j += secp256k1_fe_sqrt(&data->fe_x, &t); - secp256k1_fe_add(&data->fe_x, &data->fe_y); + t = data->fe[0]; + j += secp256k1_fe_sqrt(&data->fe[0], &t); + secp256k1_fe_add(&data->fe[0], &data->fe[1]); } CHECK(j <= iters); } @@ -203,7 +204,7 @@ void bench_group_double_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_gej_double_var(&data->gej_x, &data->gej_x, NULL); + secp256k1_gej_double_var(&data->gej[0], &data->gej[0], NULL); } } @@ -212,7 +213,7 @@ void bench_group_add_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_gej_add_var(&data->gej_x, &data->gej_x, &data->gej_y, NULL); + secp256k1_gej_add_var(&data->gej[0], &data->gej[0], &data->gej[1], NULL); } } @@ -221,7 +222,7 @@ void bench_group_add_affine(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_gej_add_ge(&data->gej_x, &data->gej_x, &data->ge_y); + secp256k1_gej_add_ge(&data->gej[0], &data->gej[0], &data->ge[1]); } } @@ -230,7 +231,7 @@ void bench_group_add_affine_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - secp256k1_gej_add_ge_var(&data->gej_x, &data->gej_x, &data->ge_y, NULL); + secp256k1_gej_add_ge_var(&data->gej[0], &data->gej[0], &data->ge[1], NULL); } } @@ -239,7 +240,7 @@ void bench_group_jacobi_var(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - j += secp256k1_gej_has_quad_y_var(&data->gej_x); + j += secp256k1_gej_has_quad_y_var(&data->gej[0]); } CHECK(j == iters); } @@ -249,8 +250,8 @@ void bench_ecmult_wnaf(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - bits += secp256k1_ecmult_wnaf(data->wnaf, 256, &data->scalar_x, WINDOW_A); - overflow += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + bits += secp256k1_ecmult_wnaf(data->wnaf, 256, &data->scalar[0], WINDOW_A); + overflow += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(overflow >= 0); CHECK(bits <= 256*iters); @@ -261,8 +262,8 @@ void bench_wnaf_const(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; for (i = 0; i < iters; i++) { - bits += secp256k1_wnaf_const(data->wnaf, &data->scalar_x, WINDOW_A, 256); - overflow += secp256k1_scalar_add(&data->scalar_x, &data->scalar_x, &data->scalar_y); + bits += secp256k1_wnaf_const(data->wnaf, &data->scalar[0], WINDOW_A, 256); + overflow += secp256k1_scalar_add(&data->scalar[0], &data->scalar[0], &data->scalar[1]); } CHECK(overflow >= 0); CHECK(bits <= 256*iters); @@ -326,9 +327,9 @@ void bench_num_jacobi(void* arg, int iters) { bench_inv *data = (bench_inv*)arg; secp256k1_num nx, norder; - secp256k1_scalar_get_num(&nx, &data->scalar_x); + secp256k1_scalar_get_num(&nx, &data->scalar[0]); secp256k1_scalar_order_get_num(&norder); - secp256k1_scalar_get_num(&norder, &data->scalar_y); + secp256k1_scalar_get_num(&norder, &data->scalar[1]); for (i = 0; i < iters; i++) { j += secp256k1_num_jacobi(&nx, &norder); From d0fdd5f00969861ebe3e48d39be6d5f706b9b17c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 11 Aug 2020 11:02:16 -0700 Subject: [PATCH 19/22] Randomize the Z coordinates in bench_internal Also increase the number of fe inputs. --- src/bench_internal.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/bench_internal.c b/src/bench_internal.c index 011e7161f..1053212f9 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -21,7 +21,7 @@ typedef struct { secp256k1_scalar scalar[2]; - secp256k1_fe fe[2]; + secp256k1_fe fe[4]; secp256k1_ge ge[2]; secp256k1_gej gej[2]; unsigned char data[64]; @@ -31,18 +31,36 @@ typedef struct { void bench_setup(void* arg) { bench_inv *data = (bench_inv*)arg; - static const unsigned char init[2][32] = { + static const unsigned char init[4][32] = { + /* Initializer for scalar[0], fe[0], first half of data, the X coordinate of ge[0], + and the (implied affine) X coordinate of gej[0]. */ { 0x02, 0x03, 0x05, 0x07, 0x0b, 0x0d, 0x11, 0x13, 0x17, 0x1d, 0x1f, 0x25, 0x29, 0x2b, 0x2f, 0x35, 0x3b, 0x3d, 0x43, 0x47, 0x49, 0x4f, 0x53, 0x59, 0x61, 0x65, 0x67, 0x6b, 0x6d, 0x71, 0x7f, 0x83 }, + /* Initializer for scalar[1], fe[1], first half of data, the X coordinate of ge[1], + and the (implied affine) X coordinate of gej[1]. */ { 0x82, 0x83, 0x85, 0x87, 0x8b, 0x8d, 0x81, 0x83, 0x97, 0xad, 0xaf, 0xb5, 0xb9, 0xbb, 0xbf, 0xc5, 0xdb, 0xdd, 0xe3, 0xe7, 0xe9, 0xef, 0xf3, 0xf9, 0x11, 0x15, 0x17, 0x1b, 0x1d, 0xb1, 0xbf, 0xd3 + }, + /* Initializer for fe[2] and the Z coordinate of gej[0]. */ + { + 0x3d, 0x2d, 0xef, 0xf4, 0x25, 0x98, 0x4f, 0x5d, + 0xe2, 0xca, 0x5f, 0x41, 0x3f, 0x3f, 0xce, 0x44, + 0xaa, 0x2c, 0x53, 0x8a, 0xc6, 0x59, 0x1f, 0x38, + 0x38, 0x23, 0xe4, 0x11, 0x27, 0xc6, 0xa0, 0xe7 + }, + /* Initializer for fe[3] and the Z coordinate of gej[1]. */ + { + 0xbd, 0x21, 0xa5, 0xe1, 0x13, 0x50, 0x73, 0x2e, + 0x52, 0x98, 0xc8, 0x9e, 0xab, 0x00, 0xa2, 0x68, + 0x43, 0xf5, 0xd7, 0x49, 0x80, 0x72, 0xa7, 0xf3, + 0xd7, 0x60, 0xe6, 0xab, 0x90, 0x92, 0xdf, 0xc5 } }; @@ -50,10 +68,14 @@ void bench_setup(void* arg) { secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL); secp256k1_fe_set_b32(&data->fe[0], init[0]); secp256k1_fe_set_b32(&data->fe[1], init[1]); + secp256k1_fe_set_b32(&data->fe[2], init[2]); + secp256k1_fe_set_b32(&data->fe[3], init[3]); CHECK(secp256k1_ge_set_xo_var(&data->ge[0], &data->fe[0], 0)); CHECK(secp256k1_ge_set_xo_var(&data->ge[1], &data->fe[1], 1)); secp256k1_gej_set_ge(&data->gej[0], &data->ge[0]); + secp256k1_gej_rescale(&data->gej[0], &data->fe[2]); secp256k1_gej_set_ge(&data->gej[1], &data->ge[1]); + secp256k1_gej_rescale(&data->gej[1], &data->fe[3]); memcpy(data->data, init[0], 32); memcpy(data->data + 32, init[1], 32); } From 5c6af60ec5f1f4bc7883737ba34dd1789f1e9bd8 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 11 Aug 2020 11:25:50 -0700 Subject: [PATCH 20/22] Make jacobi benchmarks vary inputs Also make the num_jacobi benchmark use the scalar order as modulus, instead of a random number. --- src/bench_internal.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/bench_internal.c b/src/bench_internal.c index 1053212f9..a7c1bc02b 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -263,8 +263,18 @@ void bench_group_jacobi_var(void* arg, int iters) { for (i = 0; i < iters; i++) { j += secp256k1_gej_has_quad_y_var(&data->gej[0]); + /* Vary the Y and Z coordinates of the input (the X coordinate doesn't matter to + secp256k1_gej_has_quad_y_var). Note that the resulting coordinates will + generally not correspond to a point on the curve, but this is not a problem + for the code being benchmarked here. Adding and normalizing have less + overhead than EC operations (which could guarantee the point remains on the + curve). */ + secp256k1_fe_add(&data->gej[0].y, &data->fe[1]); + secp256k1_fe_add(&data->gej[0].z, &data->fe[2]); + secp256k1_fe_normalize_var(&data->gej[0].y); + secp256k1_fe_normalize_var(&data->gej[0].z); } - CHECK(j == iters); + CHECK(j <= iters); } void bench_ecmult_wnaf(void* arg, int iters) { @@ -347,14 +357,15 @@ void bench_context_sign(void* arg, int iters) { void bench_num_jacobi(void* arg, int iters) { int i, j = 0; bench_inv *data = (bench_inv*)arg; - secp256k1_num nx, norder; + secp256k1_num nx, na, norder; secp256k1_scalar_get_num(&nx, &data->scalar[0]); secp256k1_scalar_order_get_num(&norder); - secp256k1_scalar_get_num(&norder, &data->scalar[1]); + secp256k1_scalar_get_num(&na, &data->scalar[1]); for (i = 0; i < iters; i++) { j += secp256k1_num_jacobi(&nx, &norder); + secp256k1_num_add(&nx, &nx, &na); } CHECK(j <= iters); } From cb5524adc589d3ae5066a1aa2f818bbfb91d0b1d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 11 Aug 2020 11:30:16 -0700 Subject: [PATCH 21/22] Add benchmark for secp256k1_ge_set_gej_var --- src/bench_internal.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/bench_internal.c b/src/bench_internal.c index a7c1bc02b..9687fe448 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -277,6 +277,24 @@ void bench_group_jacobi_var(void* arg, int iters) { CHECK(j <= iters); } +void bench_group_to_affine_var(void* arg, int iters) { + int i; + bench_inv *data = (bench_inv*)arg; + + for (i = 0; i < iters; ++i) { + secp256k1_ge_set_gej_var(&data->ge[1], &data->gej[0]); + /* Use the output affine X/Y coordinates to vary the input X/Y/Z coordinates. + Similar to bench_group_jacobi_var, this approach does not result in + coordinates of points on the curve. */ + secp256k1_fe_add(&data->gej[0].x, &data->ge[1].y); + secp256k1_fe_add(&data->gej[0].y, &data->fe[2]); + secp256k1_fe_add(&data->gej[0].z, &data->ge[1].x); + secp256k1_fe_normalize_var(&data->gej[0].x); + secp256k1_fe_normalize_var(&data->gej[0].y); + secp256k1_fe_normalize_var(&data->gej[0].z); + } +} + void bench_ecmult_wnaf(void* arg, int iters) { int i, bits = 0, overflow = 0; bench_inv *data = (bench_inv*)arg; @@ -398,6 +416,7 @@ int main(int argc, char **argv) { if (have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine", bench_group_add_affine, bench_setup, NULL, &data, 10, iters*10); if (have_flag(argc, argv, "group") || have_flag(argc, argv, "add")) run_benchmark("group_add_affine_var", bench_group_add_affine_var, bench_setup, NULL, &data, 10, iters*10); if (have_flag(argc, argv, "group") || have_flag(argc, argv, "jacobi")) run_benchmark("group_jacobi_var", bench_group_jacobi_var, bench_setup, NULL, &data, 10, iters); + if (have_flag(argc, argv, "group") || have_flag(argc, argv, "to_affine")) run_benchmark("group_to_affine_var", bench_group_to_affine_var, bench_setup, NULL, &data, 10, iters); if (have_flag(argc, argv, "ecmult") || have_flag(argc, argv, "wnaf")) run_benchmark("wnaf_const", bench_wnaf_const, bench_setup, NULL, &data, 10, iters); if (have_flag(argc, argv, "ecmult") || have_flag(argc, argv, "wnaf")) run_benchmark("ecmult_wnaf", bench_ecmult_wnaf, bench_setup, NULL, &data, 10, iters); From 8b70795b5e6e1e702a42a0836aeede932877ea6d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 9 Oct 2020 14:16:07 +0000 Subject: [PATCH 22/22] Fix BE platforms by updating endianness macros to match upstream --- src/modules/rangeproof/borromean_impl.h | 4 ++-- src/scalar_4x64_impl.h | 4 ++-- src/scalar_8x32_impl.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/modules/rangeproof/borromean_impl.h b/src/modules/rangeproof/borromean_impl.h index 3a82f096a..e11db181a 100644 --- a/src/modules/rangeproof/borromean_impl.h +++ b/src/modules/rangeproof/borromean_impl.h @@ -20,9 +20,9 @@ #include #include -#ifdef WORDS_BIGENDIAN +#if defined(SECP256K1_BIG_ENDIAN) #define BE32(x) (x) -#else +#elif defined(SECP256K1_LITTLE_ENDIAN) #define BE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24)) #endif diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 294413bd8..2cd0f9bc4 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -974,9 +974,9 @@ static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const se a += b; d = ROTL32(d ^ a, 8); \ c += d; b = ROTL32(b ^ c, 7); -#ifdef WORDS_BIGENDIAN +#if defined(SECP256K1_BIG_ENDIAN) #define LE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24)) -#else +#elif defined(SECP256K1_LITTLE_ENDIAN) #define LE32(p) (p) #endif diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h index c12b06e63..9afb9e951 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -753,9 +753,9 @@ static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const se a += b; d = ROTL32(d ^ a, 8); \ c += d; b = ROTL32(b ^ c, 7); -#ifdef WORDS_BIGENDIAN +#if defined(SECP256K1_BIG_ENDIAN) #define LE32(p) ((((p) & 0xFF) << 24) | (((p) & 0xFF00) << 8) | (((p) & 0xFF0000) >> 8) | (((p) & 0xFF000000) >> 24)) -#else +#elif defined(SECP256K1_LITTLE_ENDIAN) #define LE32(p) (p) #endif