From 2fccbea3c8a0e9a994cf3c49db9c74174f5338fc Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 27 Jan 2026 09:56:12 +0000 Subject: [PATCH] Squashed 'src/secp256k1/' changes from d543c0d917..14e56970cb 14e56970cb Merge bitcoin-core/secp256k1#1794: ecmult: Use size_t for array indices c7a52400d6 Merge bitcoin-core/secp256k1#1809: release cleanup: bump version after 0.7.1 ae7eb729c0 release cleanup: bump version after 0.7.1 1a53f4961f Merge bitcoin-core/secp256k1#1808: Prepare for 0.7.1 20a209f11c release: prepare for 0.7.1 c4b6a81a60 changelog: update in preparation for the v0.7.1 release ebb35882da Merge bitcoin-core/secp256k1#1796: bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS c09215f7af bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS 471e3a130d Merge bitcoin-core/secp256k1#1800: sage: verify Eisenstein integer connection for GLV constants 29ac4d8491 sage: verify Eisenstein integer connection for GLV constants 4721e077b4 Merge bitcoin-core/secp256k1#1793: doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult bd5ced1fe1 doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult 47eb70959a ecmult: Use size_t for array indices in _odd_multiplies_table bb1d199de5 ecmult: Use size_t for array indices into tables 2d9137ce9d Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules f9a944ff2d Merge bitcoin-core/secp256k1#1790: doc: include arg -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON for cmake 0406cfc4d1 doc: include arg -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1 for cmake 8d445730ec Merge bitcoin-core/secp256k1#1783: Add VERIFY_CHECKs and documentation that flags must be 0 or 1 aa2a39c1a7 Merge bitcoin-core/secp256k1#1778: doc/bench: Added cmake build options to bench error messages 540fec8ae9 Merge bitcoin-core/secp256k1#1788: test: split monolithic ellswift test into independent cases d822b29021 test: split monolithic ellswift test into independent cases ae00c552df Add VERIFY_CHECKs that flags are 0 or 1 5c75183344 Merge bitcoin-core/secp256k1#1784: refactor: remove ret from secp256k1_ec_pubkey_serialize be5e4f02fd Merge bitcoin-core/secp256k1#1779: Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL 3daab83a60 refactor: remove ret from secp256k1_ec_pubkey_serialize 8bcda186d2 test: Add non-NULL checks for "pointer of array" API functions 5a08c1bcdc Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL 3b5b03f301 doc/bench: Added cmake build options to bench error messages e7f7083b53 Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants b6c2a3cd77 Merge bitcoin-core/secp256k1#1761: ecmult_multi: reduce strauss memory usage by 30% f5e815f430 remove secp256k1_eckey_pubkey_serialize function 0d3659c547 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) adb76f82ea use new `_eckey_pubkey_serialize{33,65}` functions in public API fc7458ca3e introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions c8206b1ce6 Merge bitcoin-core/secp256k1#1771: ci: Use Python virtual environment in "x86_64-macos-native" job f252da7e6e ci: Use Python virtual environment in "x86_64-macos-native" job 115b135fe8 Merge bitcoin-core/secp256k1#1763: bench: Use `ALIGNMENT` macro instead of hardcoded value 2f73e5281d group: Avoid using infinity field directly in other modules 153eea20c2 bench: Use `ALIGNMENT` macro instead of hardcoded value 26166c4f5f ecmult_multi: reduce strauss memory usage by 30% 7a2fff85e8 Merge bitcoin-core/secp256k1#1758: ci: Drop workaround for Valgrind older than 3.20.0 43e7b115f7 Merge bitcoin-core/secp256k1#1759: ci: Switch to macOS 15 Sequoia Intel-based image 8bc50b72ff ci: Switch to macOS 15 Sequoia Intel-based image c09519f0e3 ci: Drop workaround for Valgrind older than 3.20.0 git-subtree-dir: src/secp256k1 git-subtree-split: 14e56970cba37ffe4ee992c1e08707a16e22e345 --- .github/workflows/ci.yml | 24 +++---- CHANGELOG.md | 15 ++++- CMakeLists.txt | 4 +- ci/ci.sh | 16 ----- configure.ac | 4 +- include/secp256k1.h | 3 +- sage/gen_split_lambda_constants.sage | 9 +++ src/bench.c | 17 +++-- src/bench.h | 8 ++- src/bench_ecmult.c | 20 ++++-- src/bench_internal.c | 6 +- src/eckey.h | 5 +- src/eckey_impl.h | 29 ++++---- src/ecmult.h | 2 +- src/ecmult_compute_table_impl.h | 2 +- src/ecmult_const_impl.h | 19 +++--- src/ecmult_impl.h | 58 ++++++++++------ src/field.h | 3 +- src/field_10x26_impl.h | 2 + src/field_5x52_impl.h | 2 + src/group.h | 6 +- src/group_impl.h | 2 + src/modules/ellswift/main_impl.h | 9 +-- src/modules/ellswift/tests_impl.h | 98 ++++++++++++++++++---------- src/modules/musig/keyagg_impl.h | 12 ++-- src/modules/musig/session_impl.h | 40 ++++-------- src/modules/musig/tests_impl.h | 21 ++++++ src/precompute_ecmult.c | 2 +- src/scalar.h | 6 +- src/scalar_4x64_impl.h | 3 + src/scalar_8x32_impl.h | 3 + src/scalar_low_impl.h | 3 + src/secp256k1.c | 18 +++-- src/tests.c | 44 ++++++++----- src/tests_exhaustive.c | 14 ++-- src/testutil.h | 12 ++-- src/util.h | 5 +- 37 files changed, 328 insertions(+), 218 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dceab4f8abb..59d2251497c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -429,9 +429,8 @@ jobs: - *PRINT_LOGS x86_64-macos-native: - name: "x86_64: macOS Ventura, Valgrind" - # See: https://github.com/actions/runner-images#available-images. - runs-on: macos-13 + name: "x86_64: macOS Sequoia, Valgrind" + runs-on: macos-15-intel env: CC: 'clang' @@ -470,9 +469,14 @@ jobs: env: ${{ matrix.env_vars }} run: ./ci/ci.sh - - name: Symbol check + - &SYMBOL_CHECK_MACOS + name: Symbol check + env: + VIRTUAL_ENV: '${{ github.workspace }}/venv' run: | python3 --version + python3 -m venv $VIRTUAL_ENV + export PATH="$VIRTUAL_ENV/bin:$PATH" python3 -m pip install lief python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib @@ -513,17 +517,7 @@ jobs: ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc - *CI_SCRIPT_ON_HOST - - - name: Symbol check - env: - VIRTUAL_ENV: '${{ github.workspace }}/venv' - run: | - python3 --version - python3 -m venv $VIRTUAL_ENV - export PATH="$VIRTUAL_ENV/bin:$PATH" - python3 -m pip install lief - python3 ./tools/symbol-check.py .libs/libsecp256k1.dylib - + - *SYMBOL_CHECK_MACOS - *PRINT_LOGS win64-native: diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c787b38f7..d4beb9c801c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.7.1] - 2026-01-26 + +#### Changed + - Tests: Introduced a unit test framework with support for parallel test execution, selective test running, and named command-line arguments. Run `./tests -help` for usage information. + +#### Fixed + - Increased the number of cases where the library attempts to clear secrets from the stack. + - build: Fixed x86_64 assembly feature check that could fail when user-provided `CFLAGS` included `-Werror`. This would cause the build to fall back to the slower C implementation instead of using the optimized x86_64 assembly. + +#### ABI Compatibility +The ABI is backward compatible with version 0.7.0. + ## [0.7.0] - 2025-07-21 #### Added @@ -187,7 +199,8 @@ This version was in fact never released. The number was given by the build system since the introduction of autotools in Jan 2014 (ea0fe5a5bf0c04f9cc955b2966b614f5f378c6f6). Therefore, this version number does not uniquely identify a set of source files. -[unreleased]: https://github.com/bitcoin-core/secp256k1/compare/v0.7.0...HEAD +[Unreleased]: https://github.com/bitcoin-core/secp256k1/compare/v0.7.1...HEAD +[0.7.1]: https://github.com/bitcoin-core/secp256k1/compare/v0.7.0...v0.7.1 [0.7.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.6.0...v0.7.0 [0.6.0]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.1...v0.6.0 [0.5.1]: https://github.com/bitcoin-core/secp256k1/compare/v0.5.0...v0.5.1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 11dc3f6e533..01b14b53758 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ project(libsecp256k1 # The package (a.k.a. release) version is based on semantic versioning 2.0.0 of # the API. All changes in experimental modules are treated as # backwards-compatible and therefore at most increase the minor version. - VERSION 0.7.1 + VERSION 0.7.2 DESCRIPTION "Optimized C library for ECDSA signatures and secret/public key operations on curve secp256k1." HOMEPAGE_URL "https://github.com/bitcoin-core/secp256k1" LANGUAGES C @@ -22,7 +22,7 @@ list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake) # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. set(${PROJECT_NAME}_LIB_VERSION_CURRENT 6) -set(${PROJECT_NAME}_LIB_VERSION_REVISION 1) +set(${PROJECT_NAME}_LIB_VERSION_REVISION 2) set(${PROJECT_NAME}_LIB_VERSION_AGE 0) #============================= diff --git a/ci/ci.sh b/ci/ci.sh index 08e84efd4f7..515c14cd04f 100755 --- a/ci/ci.sh +++ b/ci/ci.sh @@ -52,22 +52,6 @@ if [ -n "$WRAPPER_CMD" ]; then $WRAPPER_CMD --version fi -# Workaround for https://bugs.kde.org/show_bug.cgi?id=452758 (fixed in valgrind 3.20.0). -case "${CC:-undefined}" in - clang*) - if [ "$CTIMETESTS" = "yes" ] && [ "$WITH_VALGRIND" = "yes" ] - then - export CFLAGS="${CFLAGS:+$CFLAGS }-gdwarf-4" - else - case "$WRAPPER_CMD" in - valgrind*) - export CFLAGS="${CFLAGS:+$CFLAGS }-gdwarf-4" - ;; - esac - fi - ;; -esac - ./autogen.sh ./configure \ diff --git a/configure.ac b/configure.ac index 6028ee288d5..d7465612ddd 100644 --- a/configure.ac +++ b/configure.ac @@ -5,7 +5,7 @@ AC_PREREQ([2.60]) # backwards-compatible and therefore at most increase the minor version. define(_PKG_VERSION_MAJOR, 0) define(_PKG_VERSION_MINOR, 7) -define(_PKG_VERSION_PATCH, 1) +define(_PKG_VERSION_PATCH, 2) define(_PKG_VERSION_IS_RELEASE, false) # The library version is based on libtool versioning of the ABI. The set of @@ -14,7 +14,7 @@ define(_PKG_VERSION_IS_RELEASE, false) # All changes in experimental modules are treated as if they don't affect the # interface and therefore only increase the revision. define(_LIB_VERSION_CURRENT, 6) -define(_LIB_VERSION_REVISION, 1) +define(_LIB_VERSION_REVISION, 2) define(_LIB_VERSION_AGE, 0) AC_INIT([libsecp256k1],m4_join([.], _PKG_VERSION_MAJOR, _PKG_VERSION_MINOR, _PKG_VERSION_PATCH)m4_if(_PKG_VERSION_IS_RELEASE, [true], [], [-dev]),[https://github.com/bitcoin-core/secp256k1/issues],[libsecp256k1],[https://github.com/bitcoin-core/secp256k1]) diff --git a/include/secp256k1.h b/include/secp256k1.h index b0a13b144d9..2799d4a1f89 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -349,7 +349,8 @@ SECP256K1_API void secp256k1_context_destroy( * writes the message to stderr and calls abort. This default callback can be * replaced at link time if the preprocessor macro * USE_EXTERNAL_DEFAULT_CALLBACKS is defined, which is the case if the build - * has been configured with --enable-external-default-callbacks. Then the + * has been configured with --enable-external-default-callbacks (GNU Autotools) or + * -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON (CMake). Then the * following two symbols must be provided to link against: * - void secp256k1_default_illegal_callback_fn(const char *message, void *data); * - void secp256k1_default_error_callback_fn(const char *message, void *data); diff --git a/sage/gen_split_lambda_constants.sage b/sage/gen_split_lambda_constants.sage index 7d4359e0f64..7a5761acd29 100644 --- a/sage/gen_split_lambda_constants.sage +++ b/sage/gen_split_lambda_constants.sage @@ -81,6 +81,15 @@ assert (A1 + A2)/2 < sqrt(N) assert B1 < sqrt(N) assert B2 < sqrt(N) +# Verify connection to Eisenstein integers Z[w] where w = (-1 + sqrt(-3))/2. +# The group order N factors as N = pi * conj(pi) in Z[w], where pi = A - B*w +# is an Eisenstein prime with norm A^2 + A*B + B^2. The GLV endomorphism +# eigenvalue LAMBDA equals B/A mod N, which is the image of w^2 under the +# isomorphism Z[w]/(pi) -> Z/NZ (since w -> A/B and (A/B)^2 = B/A in Z/NZ). +A_EIS, B_EIS = -B1, A1 +assert A_EIS**2 + A_EIS*B_EIS + B_EIS**2 == N +assert Z(B_EIS / A_EIS) == LAMBDA + G1 = round((2**384)*B2/N) G2 = round((2**384)*(-B1)/N) diff --git a/src/bench.c b/src/bench.c index 8ba7623a0eb..a5231b71e9e 100644 --- a/src/bench.c +++ b/src/bench.c @@ -177,8 +177,6 @@ int main(int argc, char** argv) { bench_data data; int d = argc == 1; - int default_iters = 20000; - int iters = get_iters(default_iters); /* Check for invalid user arguments */ char* valid_args[] = {"ecdsa", "verify", "ecdsa_verify", "sign", "ecdsa_sign", "ecdh", "recover", @@ -188,6 +186,13 @@ int main(int argc, char** argv) { size_t valid_args_size = sizeof(valid_args)/sizeof(valid_args[0]); int invalid_args = have_invalid_args(argc, argv, valid_args, valid_args_size); + int default_iters = 20000; + int iters = get_iters(default_iters); + if (iters == 0) { + help(default_iters); + return EXIT_FAILURE; + } + if (argc > 1) { if (have_flag(argc, argv, "-h") || have_flag(argc, argv, "--help") @@ -205,7 +210,7 @@ int main(int argc, char** argv) { #ifndef ENABLE_MODULE_ECDH if (have_flag(argc, argv, "ecdh")) { fprintf(stderr, "./bench: ECDH module not enabled.\n"); - fprintf(stderr, "Use ./configure --enable-module-ecdh.\n\n"); + fprintf(stderr, "See README.md for configuration instructions.\n\n"); return EXIT_FAILURE; } #endif @@ -213,7 +218,7 @@ int main(int argc, char** argv) { #ifndef ENABLE_MODULE_RECOVERY if (have_flag(argc, argv, "recover") || have_flag(argc, argv, "ecdsa_recover")) { fprintf(stderr, "./bench: Public key recovery module not enabled.\n"); - fprintf(stderr, "Use ./configure --enable-module-recovery.\n\n"); + fprintf(stderr, "See README.md for configuration instructions.\n\n"); return EXIT_FAILURE; } #endif @@ -221,7 +226,7 @@ int main(int argc, char** argv) { #ifndef ENABLE_MODULE_SCHNORRSIG if (have_flag(argc, argv, "schnorrsig") || have_flag(argc, argv, "schnorrsig_sign") || have_flag(argc, argv, "schnorrsig_verify")) { fprintf(stderr, "./bench: Schnorr signatures module not enabled.\n"); - fprintf(stderr, "Use ./configure --enable-module-schnorrsig.\n\n"); + fprintf(stderr, "See README.md for configuration instructions.\n\n"); return EXIT_FAILURE; } #endif @@ -231,7 +236,7 @@ int main(int argc, char** argv) { have_flag(argc, argv, "encode") || have_flag(argc, argv, "decode") || have_flag(argc, argv, "ellswift_keygen") || have_flag(argc, argv, "ellswift_ecdh")) { fprintf(stderr, "./bench: ElligatorSwift module not enabled.\n"); - fprintf(stderr, "Use ./configure --enable-module-ellswift.\n\n"); + fprintf(stderr, "See README.md for configuration instructions.\n\n"); return EXIT_FAILURE; } #endif diff --git a/src/bench.h b/src/bench.h index 4e8e961b675..72a3f211b60 100644 --- a/src/bench.h +++ b/src/bench.h @@ -150,7 +150,13 @@ static int have_invalid_args(int argc, char** argv, char** valid_args, size_t n) static int get_iters(int default_iters) { char* env = getenv("SECP256K1_BENCH_ITERS"); if (env) { - return strtol(env, NULL, 0); + char* endptr; + long int iters = strtol(env, &endptr, 0); + if (*endptr != '\0' || iters <= 0) { + printf("Error: Value of SECP256K1_BENCH_ITERS is not a positive integer: %s\n\n", env); + return 0; + } + return iters; } else { return default_iters; } diff --git a/src/bench_ecmult.c b/src/bench_ecmult.c index b2bab65d265..bcf8b43153f 100644 --- a/src/bench_ecmult.c +++ b/src/bench_ecmult.c @@ -19,9 +19,12 @@ #define POINTS 32768 -static void help(char **argv) { +static void help(char **argv, int default_iters) { printf("Benchmark EC multiplication algorithms\n"); printf("\n"); + printf("The default number of iterations for each benchmark is %d. This can be\n", default_iters); + printf("customized using the SECP256K1_BENCH_ITERS environment variable.\n"); + printf("\n"); printf("Usage: %s \n", argv[0]); printf("The output shows the number of multiplied and summed points right after the\n"); printf("function name. The letter 'g' indicates that one of the points is the generator.\n"); @@ -308,7 +311,12 @@ int main(int argc, char **argv) { int i, p; size_t scratch_size; - int iters = get_iters(10000); + int default_iters = 10000; + int iters = get_iters(default_iters); + if (iters == 0) { + help(argv, default_iters); + return EXIT_FAILURE; + } data.ecmult_multi = secp256k1_ecmult_multi_var; @@ -316,7 +324,7 @@ int main(int argc, char **argv) { if(have_flag(argc, argv, "-h") || have_flag(argc, argv, "--help") || have_flag(argc, argv, "help")) { - help(argv); + help(argv, default_iters); return EXIT_SUCCESS; } else if(have_flag(argc, argv, "pippenger_wnaf")) { printf("Using pippenger_wnaf:\n"); @@ -328,13 +336,13 @@ int main(int argc, char **argv) { printf("Using simple algorithm:\n"); } else { fprintf(stderr, "%s: unrecognized argument '%s'.\n\n", argv[0], argv[1]); - help(argv); + help(argv, default_iters); return EXIT_FAILURE; } } data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); - scratch_size = secp256k1_strauss_scratch_size(POINTS) + STRAUSS_SCRATCH_OBJECTS*16; + scratch_size = secp256k1_strauss_scratch_size(POINTS) + STRAUSS_SCRATCH_OBJECTS*ALIGNMENT; if (!have_flag(argc, argv, "simple")) { data.scratch = secp256k1_scratch_space_create(data.ctx, scratch_size); } else { @@ -381,6 +389,8 @@ int main(int argc, char **argv) { run_ecmult_multi_bench(&data, i << p, 1, iters); } } + } else { + printf("Skipping some benchmarks due to SECP256K1_BENCH_ITERS <= 2\n"); } if (data.scratch != NULL) { diff --git a/src/bench_internal.c b/src/bench_internal.c index 8688a4dc77c..001bd25e08a 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -385,9 +385,13 @@ static void bench_context(void* arg, int iters) { int main(int argc, char **argv) { bench_inv data; + int d = argc == 1; /* default */ int default_iters = 20000; int iters = get_iters(default_iters); - int d = argc == 1; /* default */ + if (iters == 0) { + help(default_iters); + return EXIT_FAILURE; + } if (argc > 1) { if (have_flag(argc, argv, "-h") diff --git a/src/eckey.h b/src/eckey.h index d54d44c997b..c2bbc4703ec 100644 --- a/src/eckey.h +++ b/src/eckey.h @@ -15,7 +15,10 @@ #include "ecmult_gen.h" static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size); -static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed); +/** Serialize a group element (that is not allowed to be infinity) to a compressed public key (33 bytes). */ +static void secp256k1_eckey_pubkey_serialize33(secp256k1_ge *elem, unsigned char *pub33); +/** Serialize a group element (that is not allowed to be infinity) to an uncompressed public key (65 bytes). */ +static void secp256k1_eckey_pubkey_serialize65(secp256k1_ge *elem, unsigned char *pub65); static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak); static int secp256k1_eckey_pubkey_tweak_add(secp256k1_ge *key, const secp256k1_scalar *tweak); diff --git a/src/eckey_impl.h b/src/eckey_impl.h index a88a5964d81..48745e8fe7b 100644 --- a/src/eckey_impl.h +++ b/src/eckey_impl.h @@ -35,24 +35,23 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char } } -static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed) { - VERIFY_CHECK(compressed == 0 || compressed == 1); +static void secp256k1_eckey_pubkey_serialize33(secp256k1_ge *elem, unsigned char *pub33) { + VERIFY_CHECK(!secp256k1_ge_is_infinity(elem)); - if (secp256k1_ge_is_infinity(elem)) { - return 0; - } secp256k1_fe_normalize_var(&elem->x); secp256k1_fe_normalize_var(&elem->y); - secp256k1_fe_get_b32(&pub[1], &elem->x); - if (compressed) { - *size = 33; - pub[0] = secp256k1_fe_is_odd(&elem->y) ? SECP256K1_TAG_PUBKEY_ODD : SECP256K1_TAG_PUBKEY_EVEN; - } else { - *size = 65; - pub[0] = SECP256K1_TAG_PUBKEY_UNCOMPRESSED; - secp256k1_fe_get_b32(&pub[33], &elem->y); - } - return 1; + pub33[0] = secp256k1_fe_is_odd(&elem->y) ? SECP256K1_TAG_PUBKEY_ODD : SECP256K1_TAG_PUBKEY_EVEN; + secp256k1_fe_get_b32(&pub33[1], &elem->x); +} + +static void secp256k1_eckey_pubkey_serialize65(secp256k1_ge *elem, unsigned char *pub65) { + VERIFY_CHECK(!secp256k1_ge_is_infinity(elem)); + + secp256k1_fe_normalize_var(&elem->x); + secp256k1_fe_normalize_var(&elem->y); + pub65[0] = SECP256K1_TAG_PUBKEY_UNCOMPRESSED; + secp256k1_fe_get_b32(&pub65[1], &elem->x); + secp256k1_fe_get_b32(&pub65[33], &elem->y); } static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak) { diff --git a/src/ecmult.h b/src/ecmult.h index 326a5eeb434..8d0a9f49052 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -38,7 +38,7 @@ #endif /** The number of entries a table with precomputed multiples needs to have. */ -#define ECMULT_TABLE_SIZE(w) (1L << ((w)-2)) +#define ECMULT_TABLE_SIZE(w) ((size_t)1 << ((w)-2)) /** Double multiply: R = na*A + ng*G */ static void secp256k1_ecmult(secp256k1_gej *r, const secp256k1_gej *a, const secp256k1_scalar *na, const secp256k1_scalar *ng); diff --git a/src/ecmult_compute_table_impl.h b/src/ecmult_compute_table_impl.h index 69d59ce5956..09b899b40d6 100644 --- a/src/ecmult_compute_table_impl.h +++ b/src/ecmult_compute_table_impl.h @@ -16,7 +16,7 @@ static void secp256k1_ecmult_compute_table(secp256k1_ge_storage* table, int window_g, const secp256k1_gej* gen) { secp256k1_gej gj; secp256k1_ge ge, dgen; - int j; + size_t j; gj = *gen; secp256k1_ge_set_gej_var(&ge, &gj); diff --git a/src/ecmult_const_impl.h b/src/ecmult_const_impl.h index 4f66209105e..1d24aeaf364 100644 --- a/src/ecmult_const_impl.h +++ b/src/ecmult_const_impl.h @@ -87,17 +87,15 @@ static void secp256k1_ecmult_const_odd_multiples_table_globalz(secp256k1_ge *pre secp256k1_fe neg_y; \ VERIFY_CHECK((n) < (1U << ECMULT_CONST_GROUP_SIZE)); \ VERIFY_CHECK(index < (1U << (ECMULT_CONST_GROUP_SIZE - 1))); \ - /* Unconditionally set r->x = (pre)[m].x. r->y = (pre)[m].y. because it's either the correct one + /* Unconditionally set r->x = (pre)[m].x and r->y = (pre)[m].y because it's either the correct one * or will get replaced in the later iterations, this is needed to make sure `r` is initialized. */ \ - (r)->x = (pre)[m].x; \ - (r)->y = (pre)[m].y; \ + secp256k1_ge_set_xy((r), &(pre)[m].x, &(pre)[m].y); \ for (m = 1; m < ECMULT_CONST_TABLE_SIZE; m++) { \ /* This loop is used to avoid secret data in array indices. See * the comment in ecmult_gen_impl.h for rationale. */ \ secp256k1_fe_cmov(&(r)->x, &(pre)[m].x, m == index); \ secp256k1_fe_cmov(&(r)->y, &(pre)[m].y, m == index); \ } \ - (r)->infinity = 0; \ secp256k1_fe_negate(&neg_y, &(r)->y, 1); \ secp256k1_fe_cmov(&(r)->y, &neg_y, negative); \ } while(0) @@ -375,11 +373,14 @@ static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n, SECP256K1_FE_VERIFY_MAGNITUDE(&g, 2); - /* Compute base point P = (n*g, g^2), the effective affine version of (n*g, g^2, v), which has - * corresponding affine X coordinate n/d. */ - secp256k1_fe_mul(&p.x, &g, n); - secp256k1_fe_sqr(&p.y, &g); - p.infinity = 0; + /* Compute base point P = (n*g, g^2), the effective affine version of + * (n*g, g^2, v), which has corresponding affine X coordinate n/d. */ + { + secp256k1_fe x, y; + secp256k1_fe_mul(&x, &g, n); + secp256k1_fe_sqr(&y, &g); + secp256k1_ge_set_xy(&p, &x, &y); + } /* Perform x-only EC multiplication of P with q. */ VERIFY_CHECK(!secp256k1_scalar_is_zero(q)); diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index 0b53b3fcb98..1a05244c242 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -70,12 +70,12 @@ * Lastly the zr[0] value, which isn't used above, is set so that: * - a.z = z(pre_a[0]) / zr[0] */ -static void secp256k1_ecmult_odd_multiples_table(int n, secp256k1_ge *pre_a, secp256k1_fe *zr, secp256k1_fe *z, const secp256k1_gej *a) { +static void secp256k1_ecmult_odd_multiples_table(size_t n, secp256k1_ge *pre_a, secp256k1_fe *zr, secp256k1_fe *z, const secp256k1_gej *a) { secp256k1_gej d, ai; secp256k1_ge d_ge; - int i; + size_t i; - VERIFY_CHECK(!a->infinity); + VERIFY_CHECK(!secp256k1_gej_is_infinity(a)); secp256k1_gej_double_var(&d, a, NULL); @@ -220,9 +220,24 @@ static int secp256k1_ecmult_wnaf(int *wnaf, int len, const secp256k1_scalar *a, return last_set_bit + 1; } +/* Same as secp256k1_ecmult_wnaf, but stores to int8_t array. Requires w <= 8. */ +static int secp256k1_ecmult_wnaf_small(int8_t *wnaf, int len, const secp256k1_scalar *a, int w) { + int wnaf_tmp[256]; + int ret, i; + + VERIFY_CHECK(2 <= w && w <= 8); + ret = secp256k1_ecmult_wnaf(wnaf_tmp, len, a, w); + + for (i = 0; i < len; i++) { + wnaf[i] = (int8_t)wnaf_tmp[i]; + } + + return ret; +} + struct secp256k1_strauss_point_state { - int wnaf_na_1[129]; - int wnaf_na_lam[129]; + int8_t wnaf_na_1[129]; + int8_t wnaf_na_lam[129]; int bits_na_1; int bits_na_lam; }; @@ -259,8 +274,8 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state * secp256k1_scalar_split_lambda(&na_1, &na_lam, &na[np]); /* build wnaf representation for na_1 and na_lam. */ - state->ps[no].bits_na_1 = secp256k1_ecmult_wnaf(state->ps[no].wnaf_na_1, 129, &na_1, WINDOW_A); - state->ps[no].bits_na_lam = secp256k1_ecmult_wnaf(state->ps[no].wnaf_na_lam, 129, &na_lam, WINDOW_A); + state->ps[no].bits_na_1 = secp256k1_ecmult_wnaf_small(state->ps[no].wnaf_na_1, 129, &na_1, WINDOW_A); + state->ps[no].bits_na_lam = secp256k1_ecmult_wnaf_small(state->ps[no].wnaf_na_lam, 129, &na_lam, WINDOW_A); VERIFY_CHECK(state->ps[no].bits_na_1 <= 129); VERIFY_CHECK(state->ps[no].bits_na_lam <= 129); if (state->ps[no].bits_na_1 > bits) { @@ -296,8 +311,9 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state * } for (np = 0; np < no; ++np) { - for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) { - secp256k1_fe_mul(&state->aux[np * ECMULT_TABLE_SIZE(WINDOW_A) + i], &state->pre_a[np * ECMULT_TABLE_SIZE(WINDOW_A) + i].x, &secp256k1_const_beta); + size_t j; + for (j = 0; j < ECMULT_TABLE_SIZE(WINDOW_A); j++) { + secp256k1_fe_mul(&state->aux[np * ECMULT_TABLE_SIZE(WINDOW_A) + j], &state->pre_a[np * ECMULT_TABLE_SIZE(WINDOW_A) + j].x, &secp256k1_const_beta); } } @@ -341,7 +357,7 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state * } } - if (!r->infinity) { + if (!secp256k1_gej_is_infinity(r)) { secp256k1_fe_mul(&r->z, &r->z, &Z); } } @@ -502,7 +518,6 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi size_t np; size_t no = 0; int i; - int j; for (np = 0; np < num; ++np) { if (secp256k1_scalar_is_zero(&sc[np]) || secp256k1_ge_is_infinity(&pt[np])) { @@ -520,16 +535,17 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi for (i = n_wnaf - 1; i >= 0; i--) { secp256k1_gej running_sum; + int j; + size_t buc; - for(j = 0; j < ECMULT_TABLE_SIZE(bucket_window+2); j++) { - secp256k1_gej_set_infinity(&buckets[j]); + for (buc = 0; buc < ECMULT_TABLE_SIZE(bucket_window+2); buc++) { + secp256k1_gej_set_infinity(&buckets[buc]); } for (np = 0; np < no; ++np) { int n = state->wnaf_na[np*n_wnaf + i]; struct secp256k1_pippenger_point_state point_state = state->ps[np]; secp256k1_ge tmp; - int idx; if (i == 0) { /* correct for wnaf skew */ @@ -540,16 +556,16 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi } } if (n > 0) { - idx = (n - 1)/2; - secp256k1_gej_add_ge_var(&buckets[idx], &buckets[idx], &pt[point_state.input_pos], NULL); + buc = (n - 1)/2; + secp256k1_gej_add_ge_var(&buckets[buc], &buckets[buc], &pt[point_state.input_pos], NULL); } else if (n < 0) { - idx = -(n + 1)/2; + buc = -(n + 1)/2; secp256k1_ge_neg(&tmp, &pt[point_state.input_pos]); - secp256k1_gej_add_ge_var(&buckets[idx], &buckets[idx], &tmp, NULL); + secp256k1_gej_add_ge_var(&buckets[buc], &buckets[buc], &tmp, NULL); } } - for(j = 0; j < bucket_window; j++) { + for (j = 0; j < bucket_window; j++) { secp256k1_gej_double_var(r, r, NULL); } @@ -562,8 +578,8 @@ static int secp256k1_ecmult_pippenger_wnaf(secp256k1_gej *buckets, int bucket_wi * * The doubling is done implicitly by deferring the final window doubling (of 'r'). */ - for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) { - secp256k1_gej_add_var(&running_sum, &running_sum, &buckets[j], NULL); + for (buc = ECMULT_TABLE_SIZE(bucket_window+2) - 1; buc > 0; buc--) { + secp256k1_gej_add_var(&running_sum, &running_sum, &buckets[buc], NULL); secp256k1_gej_add_var(r, r, &running_sum, NULL); } diff --git a/src/field.h b/src/field.h index 1f6ba7460f7..e3f0234f5fe 100644 --- a/src/field.h +++ b/src/field.h @@ -307,7 +307,8 @@ static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe */ static void secp256k1_fe_from_storage(secp256k1_fe *r, const secp256k1_fe_storage *a); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag); /** Conditionally move a field element in constant time. diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index ea14c273187..aa45434d9d2 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -1014,6 +1014,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25 SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint32_t)0); mask1 = ~mask0; @@ -1097,6 +1098,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) { static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint32_t)0); mask1 = ~mask0; diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 46dca6b9814..3a976135ebd 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -349,6 +349,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp25 SECP256K1_INLINE static void secp256k1_fe_impl_cmov(secp256k1_fe *r, const secp256k1_fe *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint64_t)0); mask1 = ~mask0; @@ -416,6 +417,7 @@ static SECP256K1_INLINE void secp256k1_fe_impl_half(secp256k1_fe *r) { static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, const secp256k1_fe_storage *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_CHECKMEM_CHECK_VERIFY(r->n, sizeof(r->n)); mask0 = vflag + ~((uint64_t)0); mask1 = ~mask0; diff --git a/src/group.h b/src/group.h index 05ae0d203cf..ee3ebbbefe7 100644 --- a/src/group.h +++ b/src/group.h @@ -169,10 +169,12 @@ static void secp256k1_ge_to_storage(secp256k1_ge_storage *r, const secp256k1_ge /** Convert a group element back from the storage type. */ static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storage *a); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag); /** Rescale a jacobian point by b which must be non-zero. Constant-time. */ diff --git a/src/group_impl.h b/src/group_impl.h index 81a24b9d5b4..f5169650a12 100644 --- a/src/group_impl.h +++ b/src/group_impl.h @@ -898,6 +898,7 @@ static void secp256k1_ge_from_storage(secp256k1_ge *r, const secp256k1_ge_storag static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k1_gej *a, int flag) { SECP256K1_GEJ_VERIFY(r); SECP256K1_GEJ_VERIFY(a); + VERIFY_CHECK(flag == 0 || flag == 1); secp256k1_fe_cmov(&r->x, &a->x, flag); secp256k1_fe_cmov(&r->y, &a->y, flag); @@ -908,6 +909,7 @@ static SECP256K1_INLINE void secp256k1_gej_cmov(secp256k1_gej *r, const secp256k } static SECP256K1_INLINE void secp256k1_ge_storage_cmov(secp256k1_ge_storage *r, const secp256k1_ge_storage *a, int flag) { + VERIFY_CHECK(flag == 0 || flag == 1); secp256k1_fe_storage_cmov(&r->x, &a->x, flag); secp256k1_fe_storage_cmov(&r->y, &a->y, flag); } diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h index 0d13f734226..096f4a3c712 100644 --- a/src/modules/ellswift/main_impl.h +++ b/src/modules/ellswift/main_impl.h @@ -406,19 +406,12 @@ int secp256k1_ellswift_encode(const secp256k1_context *ctx, unsigned char *ell64 if (secp256k1_pubkey_load(ctx, &p, pubkey)) { secp256k1_fe t; unsigned char p64[64] = {0}; - size_t ser_size; - int ser_ret; secp256k1_sha256 hash; /* Set up hasher state; the used RNG is H(pubkey || "\x00"*31 || rnd32 || cnt++), using * BIP340 tagged hash with tag "secp256k1_ellswift_encode". */ secp256k1_ellswift_sha256_init_encode(&hash); - ser_ret = secp256k1_eckey_pubkey_serialize(&p, p64, &ser_size, 1); -#ifdef VERIFY - VERIFY_CHECK(ser_ret && ser_size == 33); -#else - (void)ser_ret; -#endif + secp256k1_eckey_pubkey_serialize33(&p, p64); secp256k1_sha256_write(&hash, p64, sizeof(p64)); secp256k1_sha256_write(&hash, rnd32, 32); diff --git a/src/modules/ellswift/tests_impl.h b/src/modules/ellswift/tests_impl.h index 63c36e7ff15..4cc7f4b5592 100644 --- a/src/modules/ellswift/tests_impl.h +++ b/src/modules/ellswift/tests_impl.h @@ -177,9 +177,9 @@ static int ellswift_xdh_hash_x32(unsigned char *output, const unsigned char *x32 return 1; } -void run_ellswift_tests(void) { - int i = 0; - /* Test vectors. */ +/* Run the test vectors for ellswift encoding */ +void ellswift_encoding_test_vectors_tests(void) { + int i; for (i = 0; (unsigned)i < sizeof(ellswift_xswiftec_inv_tests) / sizeof(ellswift_xswiftec_inv_tests[0]); ++i) { const struct ellswift_xswiftec_inv_test *testcase = &ellswift_xswiftec_inv_tests[i]; int c; @@ -195,6 +195,11 @@ void run_ellswift_tests(void) { } } } +} + +/* Run the test vectors for ellswift decoding */ +void ellswift_decoding_test_vectors_tests(void) { + int i; for (i = 0; (unsigned)i < sizeof(ellswift_decode_tests) / sizeof(ellswift_decode_tests[0]); ++i) { const struct ellswift_decode_test *testcase = &ellswift_decode_tests[i]; secp256k1_pubkey pubkey; @@ -207,6 +212,11 @@ void run_ellswift_tests(void) { CHECK(fe_equal(&testcase->x, &ge.x)); CHECK(secp256k1_fe_is_odd(&ge.y) == testcase->odd_y); } +} + +/* Run the test vectors for ellswift expected xdh BIP324 shared secrets */ +void ellswift_xdh_test_vectors_tests(void) { + int i; for (i = 0; (unsigned)i < sizeof(ellswift_xdh_tests_bip324) / sizeof(ellswift_xdh_tests_bip324[0]); ++i) { const struct ellswift_xdh_test *test = &ellswift_xdh_tests_bip324[i]; unsigned char shared_secret[32]; @@ -223,7 +233,11 @@ void run_ellswift_tests(void) { CHECK(ret); CHECK(secp256k1_memcmp_var(shared_secret, test->shared_secret, 32) == 0); } - /* Verify that secp256k1_ellswift_encode + decode roundtrips. */ +} + +/* Verify that secp256k1_ellswift_encode + decode roundtrips */ +void ellswift_encode_decode_roundtrip_tests(void) { + int i; for (i = 0; i < 1000 * COUNT; i++) { unsigned char rnd32[32]; unsigned char ell64[64]; @@ -240,7 +254,11 @@ void run_ellswift_tests(void) { /* Compare with original. */ CHECK(secp256k1_ge_eq_var(&g, &g2)); } - /* Verify the behavior of secp256k1_ellswift_create */ +} + +/* Verify the behavior of secp256k1_ellswift_create */ +void ellswift_create_tests(void) { + int i; for (i = 0; i < 400 * COUNT; i++) { unsigned char auxrnd32[32], sec32[32]; secp256k1_scalar sec; @@ -262,7 +280,11 @@ void run_ellswift_tests(void) { secp256k1_ecmult(&res, NULL, &secp256k1_scalar_zero, &sec); CHECK(secp256k1_gej_eq_ge_var(&res, &dec)); } - /* Verify that secp256k1_ellswift_xdh computes the right shared X coordinate. */ +} + +/* Verify that secp256k1_ellswift_xdh computes the right shared X coordinate */ +void ellswift_compute_shared_secret_tests(void) { + int i; for (i = 0; i < 800 * COUNT; i++) { unsigned char ell64[64], sec32[32], share32[32]; secp256k1_scalar sec; @@ -293,6 +315,10 @@ void run_ellswift_tests(void) { /* Compare. */ CHECK(fe_equal(&res.x, &share_x)); } +} + +void ellswift_xdh_correctness_tests(void) { + int i; /* Verify the joint behavior of secp256k1_ellswift_xdh */ for (i = 0; i < 200 * COUNT; i++) { unsigned char auxrnd32a[32], auxrnd32b[32], auxrnd32a_bad[32], auxrnd32b_bad[32]; @@ -403,41 +429,47 @@ void run_ellswift_tests(void) { CHECK(secp256k1_memcmp_var(share32_bad, share32b, 32) != 0); } } +} - /* Test hash initializers. */ - { - secp256k1_sha256 sha_optimized; - /* "secp256k1_ellswift_encode" */ - static const unsigned char encode_tag[] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'e', 'n', 'c', 'o', 'd', 'e'}; - /* "secp256k1_ellswift_create" */ - static const unsigned char create_tag[] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'c', 'r', 'e', 'a', 't', 'e'}; - /* "bip324_ellswift_xonly_ecdh" */ - static const unsigned char bip324_tag[] = {'b', 'i', 'p', '3', '2', '4', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'x', 'o', 'n', 'l', 'y', '_', 'e', 'c', 'd', 'h'}; +/* Test hash initializers */ +void ellswift_hash_init_tests(void) { + secp256k1_sha256 sha_optimized; + /* "secp256k1_ellswift_encode" */ + static const unsigned char encode_tag[] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'e', 'n', 'c', 'o', 'd', 'e'}; + /* "secp256k1_ellswift_create" */ + static const unsigned char create_tag[] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'c', 'r', 'e', 'a', 't', 'e'}; + /* "bip324_ellswift_xonly_ecdh" */ + static const unsigned char bip324_tag[] = {'b', 'i', 'p', '3', '2', '4', '_', 'e', 'l', 'l', 's', 'w', 'i', 'f', 't', '_', 'x', 'o', 'n', 'l', 'y', '_', 'e', 'c', 'd', 'h'}; - /* Check that hash initialized by - * secp256k1_ellswift_sha256_init_encode has the expected - * state. */ - secp256k1_ellswift_sha256_init_encode(&sha_optimized); - test_sha256_tag_midstate(&sha_optimized, encode_tag, sizeof(encode_tag)); + /* Check that hash initialized by + * secp256k1_ellswift_sha256_init_encode has the expected + * state. */ + secp256k1_ellswift_sha256_init_encode(&sha_optimized); + test_sha256_tag_midstate(&sha_optimized, encode_tag, sizeof(encode_tag)); - /* Check that hash initialized by - * secp256k1_ellswift_sha256_init_create has the expected - * state. */ - secp256k1_ellswift_sha256_init_create(&sha_optimized); - test_sha256_tag_midstate(&sha_optimized, create_tag, sizeof(create_tag)); + /* Check that hash initialized by + * secp256k1_ellswift_sha256_init_create has the expected + * state. */ + secp256k1_ellswift_sha256_init_create(&sha_optimized); + test_sha256_tag_midstate(&sha_optimized, create_tag, sizeof(create_tag)); - /* Check that hash initialized by - * secp256k1_ellswift_sha256_init_bip324 has the expected - * state. */ - secp256k1_ellswift_sha256_init_bip324(&sha_optimized); - test_sha256_tag_midstate(&sha_optimized, bip324_tag, sizeof(bip324_tag)); - } + /* Check that hash initialized by + * secp256k1_ellswift_sha256_init_bip324 has the expected + * state. */ + secp256k1_ellswift_sha256_init_bip324(&sha_optimized); + test_sha256_tag_midstate(&sha_optimized, bip324_tag, sizeof(bip324_tag)); } /* --- Test registry --- */ -/* TODO: subdivide test in cases */ static const struct tf_test_entry tests_ellswift[] = { - CASE(ellswift_tests), + CASE1(ellswift_encoding_test_vectors_tests), + CASE1(ellswift_decoding_test_vectors_tests), + CASE1(ellswift_xdh_test_vectors_tests), + CASE1(ellswift_encode_decode_roundtrip_tests), + CASE1(ellswift_create_tests), + CASE1(ellswift_compute_shared_secret_tests), + CASE1(ellswift_xdh_correctness_tests), + CASE1(ellswift_hash_init_tests), }; #endif diff --git a/src/modules/musig/keyagg_impl.h b/src/modules/musig/keyagg_impl.h index 0db4fce8596..e412a27eca6 100644 --- a/src/modules/musig/keyagg_impl.h +++ b/src/modules/musig/keyagg_impl.h @@ -124,18 +124,11 @@ static void secp256k1_musig_keyaggcoef_internal(secp256k1_scalar *r, const unsig } else { secp256k1_sha256 sha; unsigned char buf[33]; - size_t buflen = sizeof(buf); - int ret; secp256k1_musig_keyaggcoef_sha256(&sha); secp256k1_sha256_write(&sha, pks_hash, 32); - ret = secp256k1_eckey_pubkey_serialize(pk, buf, &buflen, 1); -#ifdef VERIFY /* Serialization does not fail since the pk is not the point at infinity * (according to this function's precondition). */ - VERIFY_CHECK(ret && buflen == sizeof(buf)); -#else - (void) ret; -#endif + secp256k1_eckey_pubkey_serialize33(pk, buf); secp256k1_sha256_write(&sha, buf, sizeof(buf)); secp256k1_sha256_finalize(&sha, buf); secp256k1_scalar_set_b32(r, buf, NULL); @@ -184,6 +177,9 @@ int secp256k1_musig_pubkey_agg(const secp256k1_context* ctx, secp256k1_xonly_pub } ARG_CHECK(pubkeys != NULL); ARG_CHECK(n_pubkeys > 0); + for (i = 0; i < n_pubkeys; i++) { + ARG_CHECK(pubkeys[i] != NULL); + } ecmult_data.ctx = ctx; ecmult_data.pks = pubkeys; diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 2c8778b3c0c..05c96310045 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -25,15 +25,8 @@ static void secp256k1_musig_ge_serialize_ext(unsigned char *out33, secp256k1_ge* if (secp256k1_ge_is_infinity(ge)) { memset(out33, 0, 33); } else { - int ret; - size_t size = 33; - ret = secp256k1_eckey_pubkey_serialize(ge, out33, &size, 1); -#ifdef VERIFY /* Serialize must succeed because the point is not at infinity */ - VERIFY_CHECK(ret && size == 33); -#else - (void) ret; -#endif + secp256k1_eckey_pubkey_serialize33(ge, out33); } } @@ -76,7 +69,8 @@ static int secp256k1_musig_secnonce_load(const secp256k1_context* ctx, secp256k1 return 1; } -/* If flag is true, invalidate the secnonce; otherwise leave it. Constant-time. */ +/* If flag is 1, invalidate the secnonce; if flag is 0, leave it. + * Constant-time. Flag must be 0 or 1. */ static void secp256k1_musig_secnonce_invalidate(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, int flag) { secp256k1_memczero(secnonce->data, sizeof(secnonce->data), flag); /* The flag argument is usually classified. So, the line above makes the @@ -224,15 +218,8 @@ int secp256k1_musig_pubnonce_serialize(const secp256k1_context* ctx, unsigned ch return 0; } for (i = 0; i < 2; i++) { - int ret; - size_t size = 33; - ret = secp256k1_eckey_pubkey_serialize(&ges[i], &out66[33*i], &size, 1); -#ifdef VERIFY /* serialize must succeed because the point was just loaded */ - VERIFY_CHECK(ret && size == 33); -#else - (void) ret; -#endif + secp256k1_eckey_pubkey_serialize33(&ges[i], &out66[33*i]); } return 1; } @@ -398,11 +385,9 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp secp256k1_gej nonce_ptj[2]; int i; unsigned char pk_ser[33]; - size_t pk_ser_len = sizeof(pk_ser); unsigned char aggpk_ser[32]; unsigned char *aggpk_ser_ptr = NULL; secp256k1_ge pk; - int pk_serialize_success; int ret = 1; ARG_CHECK(pubnonce != NULL); @@ -429,15 +414,8 @@ static int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp if (!secp256k1_pubkey_load(ctx, &pk, pubkey)) { return 0; } - pk_serialize_success = secp256k1_eckey_pubkey_serialize(&pk, pk_ser, &pk_ser_len, 1); - -#ifdef VERIFY /* A pubkey cannot be the point at infinity */ - VERIFY_CHECK(pk_serialize_success); - VERIFY_CHECK(pk_ser_len == sizeof(pk_ser)); -#else - (void) pk_serialize_success; -#endif + secp256k1_eckey_pubkey_serialize33(&pk, pk_ser); secp256k1_nonce_function_musig(k, input_nonce, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32); VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[0])); @@ -544,10 +522,15 @@ static int secp256k1_musig_sum_pubnonces(const secp256k1_context* ctx, secp256k1 int secp256k1_musig_nonce_agg(const secp256k1_context* ctx, secp256k1_musig_aggnonce *aggnonce, const secp256k1_musig_pubnonce * const* pubnonces, size_t n_pubnonces) { secp256k1_gej aggnonce_ptsj[2]; secp256k1_ge aggnonce_pts[2]; + size_t i; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(aggnonce != NULL); ARG_CHECK(pubnonces != NULL); ARG_CHECK(n_pubnonces > 0); + for (i = 0; i < n_pubnonces; i++) { + ARG_CHECK(pubnonces[i] != NULL); + } if (!secp256k1_musig_sum_pubnonces(ctx, aggnonce_ptsj, pubnonces, n_pubnonces)) { return 0; @@ -805,6 +788,9 @@ int secp256k1_musig_partial_sig_agg(const secp256k1_context* ctx, unsigned char ARG_CHECK(session != NULL); ARG_CHECK(partial_sigs != NULL); ARG_CHECK(n_sigs > 0); + for (i = 0; i < n_sigs; i++) { + ARG_CHECK(partial_sigs[i] != NULL); + } if (!secp256k1_musig_session_load(ctx, &session_i, session)) { return 0; diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h index b4ba185494b..de09c5e2807 100644 --- a/src/modules/musig/tests_impl.h +++ b/src/modules/musig/tests_impl.h @@ -201,6 +201,13 @@ static void musig_api_tests(void) { CHECK(secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, pk_ptr, 2) == 1); CHECK(secp256k1_musig_pubkey_agg(CTX, NULL, &keyagg_cache, pk_ptr, 2) == 1); CHECK(secp256k1_musig_pubkey_agg(CTX, &agg_pk, NULL, pk_ptr, 2) == 1); + /* check that NULL in array of public key pointers is not allowed */ + for (i = 0; i < 2; i++) { + const secp256k1_pubkey *original_ptr = pk_ptr[i]; + pk_ptr[i] = NULL; + CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, NULL, pk_ptr, 2)); + pk_ptr[i] = original_ptr; + } CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, NULL, 2)); CHECK(memcmp_and_randomize(agg_pk.data, zeros132, sizeof(agg_pk.data)) == 0); CHECK_ILLEGAL(CTX, secp256k1_musig_pubkey_agg(CTX, &agg_pk, &keyagg_cache, invalid_pk_ptr2, 2)); @@ -350,6 +357,13 @@ static void musig_api_tests(void) { /** Receive nonces and aggregate **/ CHECK(secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 2) == 1); + /* check that NULL in array of public nonce pointers is not allowed */ + for (i = 0; i < 2; i++) { + const secp256k1_musig_pubnonce *original_ptr = pubnonce_ptr[i]; + pubnonce_ptr[i] = NULL; + CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 2)); + pubnonce_ptr[i] = original_ptr; + } CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, NULL, pubnonce_ptr, 2)); CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, NULL, 2)); CHECK_ILLEGAL(CTX, secp256k1_musig_nonce_agg(CTX, &aggnonce, pubnonce_ptr, 0)); @@ -474,6 +488,13 @@ static void musig_api_tests(void) { /** Signature aggregation and verification */ CHECK(secp256k1_musig_partial_sig_agg(CTX, pre_sig, &session, partial_sig_ptr, 2) == 1); + /* check that NULL in array of partial signature pointers is not allowed */ + for (i = 0; i < 2; i++) { + const secp256k1_musig_partial_sig *original_ptr = partial_sig_ptr[i]; + partial_sig_ptr[i] = NULL; + CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, &session, partial_sig_ptr, 2)); + partial_sig_ptr[i] = original_ptr; + } CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, NULL, &session, partial_sig_ptr, 2)); CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, NULL, partial_sig_ptr, 2)); CHECK_ILLEGAL(CTX, secp256k1_musig_partial_sig_agg(CTX, pre_sig, &invalid_session, partial_sig_ptr, 2)); diff --git a/src/precompute_ecmult.c b/src/precompute_ecmult.c index 021fe3940c5..8579c85f22c 100644 --- a/src/precompute_ecmult.c +++ b/src/precompute_ecmult.c @@ -20,7 +20,7 @@ #include "ecmult_compute_table_impl.h" static void print_table(FILE *fp, const char *name, int window_g, const secp256k1_ge_storage* table) { - int j; + size_t j; int i; fprintf(fp, "const secp256k1_ge_storage %s[ECMULT_TABLE_SIZE(WINDOW_G)] = {\n", name); diff --git a/src/scalar.h b/src/scalar.h index 70f49b1cf20..40d67191acf 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -48,7 +48,7 @@ static void secp256k1_scalar_get_b32(unsigned char *bin, const secp256k1_scalar* /** Add two scalars together (modulo the group order). Returns whether it overflowed. */ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b); -/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. */ +/** Conditionally add a power of two to a scalar. The result is not allowed to overflow. Flag must be 0 or 1. */ static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag); /** Multiply two scalars (modulo the group order). */ @@ -78,7 +78,7 @@ static int secp256k1_scalar_is_even(const secp256k1_scalar *a); /** Check whether a scalar is higher than the group order divided by 2. */ static int secp256k1_scalar_is_high(const secp256k1_scalar *a); -/** Conditionally negate a number, in constant time. +/** Conditionally negate a number, in constant time. Flag must be 0 or 1. * Returns -1 if the number was negated, 1 otherwise */ static int secp256k1_scalar_cond_negate(secp256k1_scalar *a, int flag); @@ -95,7 +95,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT /** Multiply a and b (without taking the modulus!), divide by 2**shift, and round to the nearest integer. Shift must be at least 256. */ static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, const secp256k1_scalar *a, const secp256k1_scalar *b, unsigned int shift); -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. Both *r and *a must be initialized. Flag must be 0 or 1. */ static void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag); /** Check invariants on a scalar (no-op unless VERIFY is enabled). */ diff --git a/src/scalar_4x64_impl.h b/src/scalar_4x64_impl.h index 807b9b70ab1..a5bf18feb93 100644 --- a/src/scalar_4x64_impl.h +++ b/src/scalar_4x64_impl.h @@ -120,6 +120,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { secp256k1_uint128 t; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); VERIFY_CHECK(bit < 256); @@ -259,6 +260,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { uint64_t mask = -vflag; uint64_t nonzero = (secp256k1_scalar_is_zero(r) != 0) - 1; secp256k1_uint128 t; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); secp256k1_u128_from_u64(&t, r->d[0] ^ mask); @@ -911,6 +913,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint64_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d)); diff --git a/src/scalar_8x32_impl.h b/src/scalar_8x32_impl.h index 26104960524..aa87b1d3d15 100644 --- a/src/scalar_8x32_impl.h +++ b/src/scalar_8x32_impl.h @@ -147,6 +147,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { uint64_t t; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); VERIFY_CHECK(bit < 256); @@ -314,6 +315,7 @@ static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { uint32_t mask = -vflag; uint32_t nonzero = 0xFFFFFFFFUL * (secp256k1_scalar_is_zero(r) == 0); uint64_t t = (uint64_t)(r->d[0] ^ mask) + ((SECP256K1_N_0 + 1) & mask); + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(r); r->d[0] = t & nonzero; t >>= 32; @@ -709,6 +711,7 @@ SECP256K1_INLINE static void secp256k1_scalar_mul_shift_var(secp256k1_scalar *r, static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r->d, sizeof(r->d)); diff --git a/src/scalar_low_impl.h b/src/scalar_low_impl.h index 84e1a380a3b..628bfd33e8b 100644 --- a/src/scalar_low_impl.h +++ b/src/scalar_low_impl.h @@ -56,6 +56,7 @@ static int secp256k1_scalar_add(secp256k1_scalar *r, const secp256k1_scalar *a, static void secp256k1_scalar_cadd_bit(secp256k1_scalar *r, unsigned int bit, int flag) { SECP256K1_SCALAR_VERIFY(r); + VERIFY_CHECK(flag == 0 || flag == 1); if (flag && bit < 32) *r += ((uint32_t)1 << bit); @@ -121,6 +122,7 @@ static int secp256k1_scalar_is_high(const secp256k1_scalar *a) { static int secp256k1_scalar_cond_negate(secp256k1_scalar *r, int flag) { SECP256K1_SCALAR_VERIFY(r); + VERIFY_CHECK(flag == 0 || flag == 1); if (flag) secp256k1_scalar_negate(r, r); @@ -157,6 +159,7 @@ SECP256K1_INLINE static int secp256k1_scalar_eq(const secp256k1_scalar *a, const static SECP256K1_INLINE void secp256k1_scalar_cmov(secp256k1_scalar *r, const secp256k1_scalar *a, int flag) { uint32_t mask0, mask1; volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); SECP256K1_SCALAR_VERIFY(a); SECP256K1_CHECKMEM_CHECK_VERIFY(r, sizeof(*r)); diff --git a/src/secp256k1.c b/src/secp256k1.c index 26336a45ccb..ddd9849546d 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -268,7 +268,6 @@ int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pu int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *output, size_t *outputlen, const secp256k1_pubkey* pubkey, unsigned int flags) { secp256k1_ge Q; size_t len; - int ret = 0; VERIFY_CHECK(ctx != NULL); ARG_CHECK(outputlen != NULL); @@ -280,12 +279,16 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o ARG_CHECK(pubkey != NULL); ARG_CHECK((flags & SECP256K1_FLAGS_TYPE_MASK) == SECP256K1_FLAGS_TYPE_COMPRESSION); if (secp256k1_pubkey_load(ctx, &Q, pubkey)) { - ret = secp256k1_eckey_pubkey_serialize(&Q, output, &len, !!(flags & SECP256K1_FLAGS_BIT_COMPRESSION)); - if (ret) { - *outputlen = len; + if (flags & SECP256K1_FLAGS_BIT_COMPRESSION) { + secp256k1_eckey_pubkey_serialize33(&Q, output); + *outputlen = 33; + } else { + secp256k1_eckey_pubkey_serialize65(&Q, output); + *outputlen = 65; } + return 1; } - return ret; + return 0; } int secp256k1_ec_pubkey_cmp(const secp256k1_context* ctx, const secp256k1_pubkey* pubkey0, const secp256k1_pubkey* pubkey1) { @@ -321,8 +324,13 @@ static int secp256k1_ec_pubkey_sort_cmp(const void* pk1, const void* pk2, void * } int secp256k1_ec_pubkey_sort(const secp256k1_context* ctx, const secp256k1_pubkey **pubkeys, size_t n_pubkeys) { + size_t i; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(pubkeys != NULL); + for (i = 0; i < n_pubkeys; i++) { + ARG_CHECK(pubkeys[i] != NULL); + } /* Suppress wrong warning (fixed in MSVC 19.33) */ #if defined(_MSC_VER) && (_MSC_VER < 1933) diff --git a/src/tests.c b/src/tests.c index 0ccdbeecf74..e09f5c7d231 100644 --- a/src/tests.c +++ b/src/tests.c @@ -4152,8 +4152,8 @@ static void test_group_decompress(const secp256k1_fe* x) { secp256k1_fe_normalize_var(&ge_even.y); /* No infinity allowed. */ - CHECK(!ge_even.infinity); - CHECK(!ge_odd.infinity); + CHECK(!secp256k1_ge_is_infinity(&ge_even)); + CHECK(!secp256k1_ge_is_infinity(&ge_odd)); /* Check that the x coordinates check out. */ CHECK(secp256k1_fe_equal(&ge_even.x, x)); @@ -4315,8 +4315,6 @@ static void test_point_times_order(const secp256k1_gej *point) { secp256k1_scalar nx; secp256k1_gej res1, res2; secp256k1_ge res3; - unsigned char pub[65]; - size_t psize = 65; testutil_random_scalar_order_test(&x); secp256k1_scalar_negate(&nx, &x); secp256k1_ecmult(&res1, point, &x, &x); /* calc res1 = x * point + x * G; */ @@ -4326,9 +4324,6 @@ static void test_point_times_order(const secp256k1_gej *point) { secp256k1_ge_set_gej(&res3, &res1); CHECK(secp256k1_ge_is_infinity(&res3)); CHECK(secp256k1_ge_is_valid_var(&res3) == 0); - CHECK(secp256k1_eckey_pubkey_serialize(&res3, pub, &psize, 0) == 0); - psize = 65; - CHECK(secp256k1_eckey_pubkey_serialize(&res3, pub, &psize, 1) == 0); /* check zero/one edge cases */ secp256k1_ecmult(&res1, point, &secp256k1_scalar_zero, &secp256k1_scalar_zero); secp256k1_ge_set_gej(&res3, &res1); @@ -5435,7 +5430,6 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar secp256k1_gej rj1, rj2, rj3, rj4, rj5, rj6, gj, infj; secp256k1_ge r; unsigned char bytes[65]; - size_t size = 65; secp256k1_gej_set_ge(&gj, &secp256k1_ge_const_g); secp256k1_gej_set_infinity(&infj); secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &rj1, x); @@ -5456,9 +5450,8 @@ static void test_ecmult_accumulate(secp256k1_sha256* acc, const secp256k1_scalar secp256k1_sha256_write(acc, zerobyte, 1); } else { /* Store other points using their uncompressed serialization. */ - secp256k1_eckey_pubkey_serialize(&r, bytes, &size, 0); - CHECK(size == 65); - secp256k1_sha256_write(acc, bytes, size); + secp256k1_eckey_pubkey_serialize65(&r, bytes); + secp256k1_sha256_write(acc, bytes, sizeof(bytes)); } } @@ -6059,6 +6052,7 @@ static void run_eckey_edge_case_test(void) { secp256k1_pubkey pubkey_negone; const secp256k1_pubkey *pubkeys[3]; size_t len; + int i; /* Group order is too large, reject. */ CHECK(secp256k1_ec_seckey_verify(CTX, orderc) == 0); SECP256K1_CHECKMEM_UNDEFINE(&pubkey, sizeof(pubkey)); @@ -6252,6 +6246,14 @@ static void run_eckey_edge_case_test(void) { CHECK(secp256k1_ec_pubkey_combine(CTX, &pubkey, pubkeys, 3) == 1); SECP256K1_CHECKMEM_CHECK(&pubkey, sizeof(secp256k1_pubkey)); CHECK(secp256k1_memcmp_var(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0); + /* check that NULL in array of pubkey pointers is not allowed */ + for (i = 0; i < 3; i++) { + const secp256k1_pubkey *original_ptr = pubkeys[i]; + secp256k1_pubkey result; + pubkeys[i] = NULL; + CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_combine(CTX, &result, pubkeys, 3)); + pubkeys[i] = original_ptr; + } len = 33; CHECK(secp256k1_ec_pubkey_serialize(CTX, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1); CHECK(secp256k1_ec_pubkey_serialize(CTX, ctmp2, &len, &pubkey_one, SECP256K1_EC_COMPRESSED) == 1); @@ -6543,16 +6545,18 @@ static void test_random_pubkeys(void) { size_t size = len; firstb = in[0]; /* If the pubkey can be parsed, it should round-trip... */ - CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, len == 33)); - CHECK(size == len); + if (len == 33) { + secp256k1_eckey_pubkey_serialize33(&elem, out); + } else { + secp256k1_eckey_pubkey_serialize65(&elem, out); + } CHECK(secp256k1_memcmp_var(&in[1], &out[1], len-1) == 0); /* ... except for the type of hybrid inputs. */ if ((in[0] != 6) && (in[0] != 7)) { CHECK(in[0] == out[0]); } size = 65; - CHECK(secp256k1_eckey_pubkey_serialize(&elem, in, &size, 0)); - CHECK(size == 65); + secp256k1_eckey_pubkey_serialize65(&elem, in); CHECK(secp256k1_eckey_pubkey_parse(&elem2, in, size)); CHECK(secp256k1_ge_eq_var(&elem2, &elem)); /* Check that the X9.62 hybrid type is checked. */ @@ -6567,7 +6571,7 @@ static void test_random_pubkeys(void) { } if (res) { CHECK(secp256k1_ge_eq_var(&elem, &elem2)); - CHECK(secp256k1_eckey_pubkey_serialize(&elem, out, &size, 0)); + secp256k1_eckey_pubkey_serialize65(&elem, out); CHECK(secp256k1_memcmp_var(&in[1], &out[1], 64) == 0); } } @@ -6645,6 +6649,7 @@ static void permute(size_t *arr, size_t n) { static void test_sort_api(void) { secp256k1_pubkey pks[2]; const secp256k1_pubkey *pks_ptr[2]; + int i; pks_ptr[0] = &pks[0]; pks_ptr[1] = &pks[1]; @@ -6653,6 +6658,13 @@ static void test_sort_api(void) { testutil_random_pubkey_test(&pks[1]); CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2) == 1); + /* check that NULL in array of public key pointers is not allowed */ + for (i = 0; i < 2; i++) { + const secp256k1_pubkey *original_ptr = pks_ptr[i]; + pks_ptr[i] = NULL; + CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2)); + pks_ptr[i] = original_ptr; + } CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_sort(CTX, NULL, 2)); CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 0) == 1); /* Test illegal public keys */ diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index f8bbcaaf5c4..13bda6119d7 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -103,9 +103,11 @@ static void test_exhaustive_addition(const secp256k1_ge *group, const secp256k1_ secp256k1_gej_add_ge_var(&tmp, &groupj[i], &group[j], NULL); CHECK(secp256k1_gej_eq_ge_var(&tmp, &group[(i + j) % EXHAUSTIVE_TEST_ORDER])); /* add_zinv_var */ - zless_gej.infinity = groupj[j].infinity; - zless_gej.x = groupj[j].x; - zless_gej.y = groupj[j].y; + if (secp256k1_gej_is_infinity(&groupj[j])) { + secp256k1_ge_set_infinity(&zless_gej); + } else { + secp256k1_ge_set_xy(&zless_gej, &groupj[j].x, &groupj[j].y); + } secp256k1_gej_add_zinv_var(&tmp, &groupj[i], &zless_gej, &fe_inv); CHECK(secp256k1_gej_eq_ge_var(&tmp, &group[(i + j) % EXHAUSTIVE_TEST_ORDER])); } @@ -422,10 +424,8 @@ int main(int argc, char** argv) { secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &generatedj, &scalar_i); secp256k1_ge_set_gej(&generated, &generatedj); - CHECK(group[i].infinity == 0); - CHECK(generated.infinity == 0); - CHECK(secp256k1_fe_equal(&generated.x, &group[i].x)); - CHECK(secp256k1_fe_equal(&generated.y, &group[i].y)); + CHECK(!secp256k1_ge_is_infinity(&group[i])); + CHECK(secp256k1_ge_eq_var(&group[i], &generated)); } } diff --git a/src/testutil.h b/src/testutil.h index 480f6a1a0cd..93ee3d58b5b 100644 --- a/src/testutil.h +++ b/src/testutil.h @@ -96,17 +96,13 @@ static void testutil_random_ge_test(secp256k1_ge *ge) { break; } } while(1); - ge->infinity = 0; } static void testutil_random_ge_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) { - secp256k1_fe z2, z3; - testutil_random_fe_non_zero_test(&gej->z); - secp256k1_fe_sqr(&z2, &gej->z); - secp256k1_fe_mul(&z3, &z2, &gej->z); - secp256k1_fe_mul(&gej->x, &ge->x, &z2); - secp256k1_fe_mul(&gej->y, &ge->y, &z3); - gej->infinity = ge->infinity; + secp256k1_fe z; + testutil_random_fe_non_zero_test(&z); + secp256k1_gej_set_ge(gej, ge); + secp256k1_gej_rescale(gej, &z); } static void testutil_random_gej_test(secp256k1_gej *gej) { diff --git a/src/util.h b/src/util.h index 94e0837dab5..46ab381077c 100644 --- a/src/util.h +++ b/src/util.h @@ -212,6 +212,7 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) { take only be 0 or 1, which leads to variable time code. */ volatile int vflag = flag; unsigned char mask = -(unsigned char) vflag; + VERIFY_CHECK(flag == 0 || flag == 1); while (len) { *p &= ~mask; p++; @@ -294,7 +295,8 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size return ret; } -/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized and non-negative.*/ +/** If flag is 1, set *r equal to *a; if flag is 0, leave it. Constant-time. + * Both *r and *a must be initialized and non-negative. Flag must be 0 or 1. */ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) { unsigned int mask0, mask1, r_masked, a_masked; /* Access flag with a volatile-qualified lvalue. @@ -302,6 +304,7 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) take only be 0 or 1, which leads to variable time code. */ volatile int vflag = flag; + VERIFY_CHECK(flag == 0 || flag == 1); /* Casting a negative int to unsigned and back to int is implementation defined behavior */ VERIFY_CHECK(*r >= 0 && *a >= 0);