diff --git a/cmake/module/AddBoostIfNeeded.cmake b/cmake/module/AddBoostIfNeeded.cmake index e2eaf556..b3f24800 100644 --- a/cmake/module/AddBoostIfNeeded.cmake +++ b/cmake/module/AddBoostIfNeeded.cmake @@ -29,7 +29,7 @@ function(add_boost_if_needed) endif() endif() - find_package(Boost 1.73.0 REQUIRED CONFIG) + find_package(Boost 1.74.0 REQUIRED CONFIG) mark_as_advanced(Boost_INCLUDE_DIR boost_headers_DIR) # Workaround for a bug in NetBSD pkgsrc. # See: https://github.com/NetBSD/pkgsrc/issues/167. diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index 312f19e2..02f59abe 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -1,9 +1,8 @@ package=boost -$(package)_version = 1.88.0 +$(package)_version = 1.90.0 $(package)_download_path = https://github.com/boostorg/boost/releases/download/boost-$($(package)_version) $(package)_file_name = boost-$($(package)_version)-cmake.tar.gz -$(package)_sha256_hash = dcea50f40ba1ecfc448fdf886c0165cf3e525fef2c9e3e080b9804e8117b9694 -$(package)_patches = skip_compiled_targets.patch +$(package)_sha256_hash = 913ca43d49e93d1b158c9862009add1518a4c665e7853b349a6492d158b036d4 $(package)_build_subdir = build define $(package)_set_vars @@ -14,10 +13,8 @@ define $(package)_set_vars $(package)_config_opts += -DBOOST_INSTALL_LAYOUT=system $(package)_config_opts += -DBUILD_TESTING=OFF $(package)_config_opts += -DCMAKE_DISABLE_FIND_PACKAGE_ICU=ON -endef - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/skip_compiled_targets.patch + # Install to a unique path to prevent accidental inclusion via other dependencies' -I flags. + $(package)_config_opts += -DCMAKE_INSTALL_INCLUDEDIR=$(package)/include endef define $(package)_config_cmds diff --git a/depends/packages/native_capnp.mk b/depends/packages/native_capnp.mk index a3a089d4..0cd0dcdf 100644 --- a/depends/packages/native_capnp.mk +++ b/depends/packages/native_capnp.mk @@ -1,9 +1,9 @@ package=native_capnp -$(package)_version=1.2.0 +$(package)_version=1.3.0 $(package)_download_path=https://capnproto.org/ $(package)_download_file=capnproto-c++-$($(package)_version).tar.gz $(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz -$(package)_sha256_hash=ed00e44ecbbda5186bc78a41ba64a8dc4a861b5f8d4e822959b0144ae6fd42ef +$(package)_sha256_hash=098f824a495a1a837d56ae17e07b3f721ac86f8dbaf58896a389923458522108 define $(package)_set_vars $(package)_config_opts := -DBUILD_TESTING=OFF diff --git a/depends/patches/boost/skip_compiled_targets.patch b/depends/patches/boost/skip_compiled_targets.patch deleted file mode 100644 index 32b69ce6..00000000 --- a/depends/patches/boost/skip_compiled_targets.patch +++ /dev/null @@ -1,136 +0,0 @@ -cmake: Add `BOOST_TEST_HEADERS_ONLY` configuration variable - -This change allows the build to be configured to install only the -Boost.Test headers required for using the headers-only variant of the -Unit Test Framework. - -Upstream commit: 097e97820e654ead9c477b47443a545cef5d3b12 - - ---- a/libs/test/CMakeLists.txt -+++ b/libs/test/CMakeLists.txt -@@ -30,60 +30,70 @@ set(_boost_test_dependencies - Boost::utility - ) - --# Compiled targets -+option(BOOST_TEST_HEADERS_ONLY "Boost.Test: Only install headers" OFF) - --function(boost_test_add_library name) -+set(_boost_test_libraries "") - -- add_library(boost_${name} ${ARGN}) -- add_library(Boost::${name} ALIAS boost_${name}) -+if (NOT BOOST_TEST_HEADERS_ONLY) - -- target_include_directories(boost_${name} PUBLIC include) -- target_link_libraries(boost_${name} PUBLIC ${_boost_test_dependencies}) -+ # Compiled targets - -- target_compile_definitions(boost_${name} -- PUBLIC BOOST_TEST_NO_LIB -- # Source files already define BOOST_TEST_SOURCE -- # PRIVATE BOOST_TEST_SOURCE -- ) -+ function(boost_test_add_library name) - -- if(BUILD_SHARED_LIBS) -- target_compile_definitions(boost_${name} PUBLIC BOOST_TEST_DYN_LINK) -- else() -- target_compile_definitions(boost_${name} PUBLIC BOOST_TEST_STATIC_LINK) -- endif() -+ add_library(boost_${name} ${ARGN}) -+ add_library(Boost::${name} ALIAS boost_${name}) - --endfunction() -+ target_include_directories(boost_${name} PUBLIC include) -+ target_link_libraries(boost_${name} PUBLIC ${_boost_test_dependencies}) - --boost_test_add_library(prg_exec_monitor -- src/cpp_main.cpp -- src/debug.cpp -- src/execution_monitor.cpp --) -+ target_compile_definitions(boost_${name} -+ PUBLIC BOOST_TEST_NO_LIB -+ # Source files already define BOOST_TEST_SOURCE -+ # PRIVATE BOOST_TEST_SOURCE -+ ) - --set(SOURCES -- src/compiler_log_formatter.cpp -- src/debug.cpp -- src/decorator.cpp -- src/execution_monitor.cpp -- src/framework.cpp -- src/junit_log_formatter.cpp -- src/plain_report_formatter.cpp -- src/progress_monitor.cpp -- src/results_collector.cpp -- src/results_reporter.cpp -- src/test_framework_init_observer.cpp -- src/test_tools.cpp -- src/test_tree.cpp -- src/unit_test_log.cpp -- src/unit_test_main.cpp -- src/unit_test_monitor.cpp -- src/unit_test_parameters.cpp -- src/xml_log_formatter.cpp -- src/xml_report_formatter.cpp --) -+ if(BUILD_SHARED_LIBS) -+ target_compile_definitions(boost_${name} PUBLIC BOOST_TEST_DYN_LINK) -+ else() -+ target_compile_definitions(boost_${name} PUBLIC BOOST_TEST_STATIC_LINK) -+ endif() -+ -+ endfunction() - --boost_test_add_library(test_exec_monitor STATIC ${SOURCES} src/test_main.cpp) --boost_test_add_library(unit_test_framework ${SOURCES}) -+ boost_test_add_library(prg_exec_monitor -+ src/cpp_main.cpp -+ src/debug.cpp -+ src/execution_monitor.cpp -+ ) -+ -+ set(SOURCES -+ src/compiler_log_formatter.cpp -+ src/debug.cpp -+ src/decorator.cpp -+ src/execution_monitor.cpp -+ src/framework.cpp -+ src/junit_log_formatter.cpp -+ src/plain_report_formatter.cpp -+ src/progress_monitor.cpp -+ src/results_collector.cpp -+ src/results_reporter.cpp -+ src/test_framework_init_observer.cpp -+ src/test_tools.cpp -+ src/test_tree.cpp -+ src/unit_test_log.cpp -+ src/unit_test_main.cpp -+ src/unit_test_monitor.cpp -+ src/unit_test_parameters.cpp -+ src/xml_log_formatter.cpp -+ src/xml_report_formatter.cpp -+ ) -+ -+ boost_test_add_library(test_exec_monitor STATIC ${SOURCES} src/test_main.cpp) -+ boost_test_add_library(unit_test_framework ${SOURCES}) -+ -+ set(_boost_test_libraries boost_prg_exec_monitor boost_test_exec_monitor boost_unit_test_framework) -+ -+endif() - - # Header-only targets - -@@ -107,7 +117,7 @@ if(BOOST_SUPERPROJECT_VERSION AND NOT CMAKE_VERSION VERSION_LESS 3.13) - - boost_install( - TARGETS -- boost_prg_exec_monitor boost_test_exec_monitor boost_unit_test_framework -+ ${_boost_test_libraries} - boost_included_prg_exec_monitor boost_included_test_exec_monitor boost_included_unit_test_framework - VERSION ${BOOST_SUPERPROJECT_VERSION} - HEADER_DIRECTORY include diff --git a/doc/dependencies.md b/doc/dependencies.md index 2987b868..bdd74992 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -19,7 +19,7 @@ Bitcoin Core requires one of the following compilers. | Dependency | Releases | Minimum required | | --- | --- | --- | -| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | +| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | [1.74.0](https://github.com/bitcoin/bitcoin/pull/34107) | | CMake | [link](https://cmake.org/) | [3.22](https://github.com/bitcoin/bitcoin/pull/30454) | ### Runtime diff --git a/src/ipc/libmultiprocess/.github/workflows/ci.yml b/src/ipc/libmultiprocess/.github/workflows/ci.yml index 87b17065..f4a66234 100644 --- a/src/ipc/libmultiprocess/.github/workflows/ci.yml +++ b/src/ipc/libmultiprocess/.github/workflows/ci.yml @@ -5,6 +5,66 @@ on: pull_request: jobs: + build-netbsd: + runs-on: ubuntu-latest + name: build • netbsd ${{ matrix.release }} + defaults: + run: + shell: netbsd {0} + strategy: + fail-fast: false + matrix: + # Test all supported releases. + # See https://www.netbsd.org/releases/. + include: + - release: 9.4 + capnproto-cppflags: 'CPPFLAGS="-DKJ_NO_EXCEPTIONS=0 -DKJ_USE_KQUEUE=0"' + - release: 10.1 + capnproto-cppflags: 'CPPFLAGS="-DKJ_NO_EXCEPTIONS=0"' + steps: + - uses: actions/checkout@v6 + + - name: Start NetBSD VM + uses: vmactions/netbsd-vm@v1 + with: + release: ${{ matrix.release }} + # The installed compiler version must match the CXX variable + # defined in `ci/configs/netbsd.bash`. + prepare: | + pkg_add cmake ninja-build gcc14 + # capnproto prerequisites. + # See the following "Install capnproto" step. + run: | + set -e + pkg_add digest libtool-base mktools pkgconf cwrappers + pkg_admin -K /usr/pkg/pkgdb fetch-pkg-vulnerabilities + cd /usr + cvs -danoncvs@anoncvs.NetBSD.org:/cvsroot checkout -P \ + pkgsrc/devel/capnproto \ + pkgsrc/devel/libtool-base \ + pkgsrc/devel/pkgconf \ + pkgsrc/devel/zlib \ + `# gcc15 is referenced here because the pkgsrc framework requires lang/gcc15/version.mk to exist` \ + `# during the "make install" step below, even though we compile our project with gcc14.` \ + pkgsrc/lang/gcc15 \ + pkgsrc/mk \ + pkgsrc/pkgtools \ + pkgsrc/security/openssl \ + pkgsrc/sysutils/install-sh/files + sync: 'rsync' + copyback: false + + - name: Install capnproto + run: | + cd /usr/pkgsrc/devel/capnproto/ + unset PKG_PATH + make ${{ matrix.capnproto-cppflags }} install + + - name: Run CI script + run: | + cd ${{ github.workspace }} + CI_CONFIG="ci/configs/netbsd.bash" bash ci/scripts/ci.sh + build-openbsd: runs-on: ubuntu-latest name: build • openbsd @@ -18,17 +78,10 @@ jobs: uses: vmactions/openbsd-vm@v1 with: prepare: | - pkg_add -v cmake ninja git bash - run: | - git clone --depth=1 https://codeberg.org/OpenBSD/ports.git /usr/ports + pkg_add -v cmake ninja bash capnproto sync: 'rsync' copyback: false - - name: Install capnproto - run: | - cd /usr/ports/devel/capnproto/ - make install - - name: Run CI script run: | cd ${{ github.workspace }} @@ -76,6 +129,11 @@ jobs: build: runs-on: ubuntu-latest + env: + NIX_EXTRA_CONFIG_ACT: | + sandbox = false + filter-syscalls = false + strategy: fail-fast: false matrix: @@ -90,6 +148,10 @@ jobs: uses: cachix/install-nix-action@v31 # 2025-05-27, from https://github.com/cachix/install-nix-action/tags with: nix_path: nixpkgs=channel:nixos-25.05 # latest release + # Act executes inside an unprivileged container (Docker or Podman), + # so KVM support isn't available. + enable_kvm: "${{ github.actor != 'nektos/act' }}" + extra_nix_config: ${{ github.actor == 'nektos/act' && env.NIX_EXTRA_CONFIG_ACT || '' }} - name: Run CI script env: diff --git a/src/ipc/libmultiprocess/ci/README.md b/src/ipc/libmultiprocess/ci/README.md index 2ddaa9e1..fef1c022 100644 --- a/src/ipc/libmultiprocess/ci/README.md +++ b/src/ipc/libmultiprocess/ci/README.md @@ -24,3 +24,29 @@ CI_CONFIG=ci/configs/olddeps.bash ci/scripts/run.sh ``` By default CI jobs will reuse their build directories. `CI_CLEAN=1` can be specified to delete them before running instead. + +### Running workflows with `act` + +You can run either the entire workflow or a single matrix entry locally. On +macOS or Linux: + +1. Install [`act`](https://github.com/nektos/act) and either Docker or + Podman. +2. Inside the Podman VM, create a named volume for the Nix store (ext4, + case-sensitive) so builds persist across runs. Recreate it any time you want + a clean cache: + ```bash + podman volume create libmultiprocess-nix + ``` +3. From the repo root, launch the workflow. The example below targets the + sanitize matrix entry; drop the `--matrix` flag to run every configuration. + ```bash + act \ + --reuse \ + -P ubuntu-latest=ghcr.io/catthehacker/ubuntu:act-24.04 \ + --container-options "-v libmultiprocess-nix:/nix" \ + -j build \ + --matrix config:sanitize + ``` + + diff --git a/src/ipc/libmultiprocess/ci/configs/netbsd.bash b/src/ipc/libmultiprocess/ci/configs/netbsd.bash new file mode 100644 index 00000000..c3e7d258 --- /dev/null +++ b/src/ipc/libmultiprocess/ci/configs/netbsd.bash @@ -0,0 +1,11 @@ +CI_DESC="CI config for NetBSD" +CI_DIR=build-netbsd +export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter" +# Hardcode GCC 14, since default GCC versions installed by NetBSD are older +# and may not be compatible with libmultiprocess. GCC 14 was chosen because +# it's the latest compiler available on all versions of NetBSD that we test. +# Note that the GCC version specified here must match the version specified +# in pkg_add in ci.yml. +export CXX="/usr/pkg/gcc14/bin/g++" +CMAKE_ARGS=(-G Ninja) +BUILD_ARGS=(-k 0) diff --git a/src/ipc/libmultiprocess/ci/scripts/run.sh b/src/ipc/libmultiprocess/ci/scripts/run.sh index 11b91845..4b780831 100755 --- a/src/ipc/libmultiprocess/ci/scripts/run.sh +++ b/src/ipc/libmultiprocess/ci/scripts/run.sh @@ -10,4 +10,4 @@ set -o errexit -o nounset -o pipefail -o xtrace [ "${CI_CONFIG+x}" ] && source "$CI_CONFIG" -nix-shell --pure --keep CI_CONFIG --keep CI_CLEAN "${NIX_ARGS[@]+"${NIX_ARGS[@]}"}" --run ci/scripts/ci.sh shell.nix +nix develop --ignore-environment --keep CI_CONFIG --keep CI_CLEAN "${NIX_ARGS[@]+"${NIX_ARGS[@]}"}" -f shell.nix --command ci/scripts/ci.sh diff --git a/src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake b/src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake index 347ef4a0..abcca70f 100644 --- a/src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake +++ b/src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake @@ -55,7 +55,7 @@ Example: function(target_capnp_sources target include_prefix) cmake_parse_arguments(PARSE_ARGV 2 "TCS" # prefix - "" # options + "ONLY_CAPNP" # options "" # one_value_keywords "IMPORT_PATHS" # multi_value_keywords ) @@ -85,11 +85,14 @@ function(target_capnp_sources target include_prefix) set_source_files_properties(${capnp_file}.c++ PROPERTIES SKIP_LINTING TRUE) # Ignored before cmake 3.27 target_sources(${target} PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.c++ - ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-client.c++ - ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-server.c++ - ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-types.c++ ) - + if(NOT TCS_ONLY_CAPNP) + target_sources(${target} PRIVATE + ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-client.c++ + ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-server.c++ + ${CMAKE_CURRENT_BINARY_DIR}/${capnp_file}.proxy-types.c++ + ) + endif() list(APPEND generated_headers ${capnp_file}.h) endforeach() @@ -111,5 +114,7 @@ function(target_capnp_sources target include_prefix) # dependencies explicitly because while cmake detect dependencies of non # generated files on generated headers, it does not reliably detect # dependencies of generated headers on other generated headers. - add_custom_target("${target}_headers" DEPENDS ${generated_headers}) + if(NOT TARGET "${target}_headers") + add_custom_target("${target}_headers" DEPENDS ${generated_headers}) + endif() endfunction() diff --git a/src/ipc/libmultiprocess/doc/design.md b/src/ipc/libmultiprocess/doc/design.md index 4422953d..113cafc4 100644 --- a/src/ipc/libmultiprocess/doc/design.md +++ b/src/ipc/libmultiprocess/doc/design.md @@ -13,7 +13,7 @@ There is also optional support for thread mapping, so each thread making interpr Libmultiprocess acts as a pure wrapper or layer over the underlying protocol. Clients and servers written in other languages, but using a shared capnproto schema can communicate with interprocess counterparties using libmultiprocess without having to use libmultiprocess themselves or having to know about the implementation details of libmultiprocess. -### Internals +## Core Architecture The `ProxyClient` and `ProxyServer` generated classes are not directly exposed to the user, as described in [usage.md](usage.md). Instead, they wrap C++ interfaces and appear to the user as pointers to an interface. They are first instantiated when calling `ConnectStream` and `ServeStream` respectively for creating the `InitInterface`. These methods establish connections through sockets, internally creating `Connection` objects wrapping a `capnp::RpcSystem` configured for client and server mode respectively. @@ -25,7 +25,190 @@ When a generated method on the `ProxyClient` is called, it calls `clientInvoke` On the server side, the `capnp::RpcSystem` receives the capnp request and invokes the corresponding C++ method through the corresponding `ProxyServer` and the heavily templated `serverInvoke` triggering a `ServerCall`. The return values from the actual C++ methods are copied into capnp responses by `ServerRet` and exceptions are caught and copied by `ServerExcept`. The two are connected through `ServerField`. The main method driving execution of a request is `PassField`, which is invoked through `ServerField`. Instantiated interfaces, or capabilities in capnp speak, are tracked and owned by the server's `capnp::RpcSystem`. -## Interface descriptions +## Request and Response Flow + +Method parameters and return values are serialized using Cap'n Proto's Builder objects (for sending) and Reader objects (for receiving). Input parameters flow from the client to the server, while output parameters (return values) flow back from the server to the client. + +```mermaid +sequenceDiagram + participant clientInvoke + participant BuildField as BuildField
(Client) + participant ReadField_C as ReadField
(Client) + participant Request as Request
message + participant serverInvoke + participant ReadField as ReadField
(Server) + participant BuildField_S as BuildField
(Server) + participant Response as Response
message + + Note over clientInvoke,ReadField: Input Parameter Flow + clientInvoke->>BuildField: BuildField(input_arg) + BuildField->>Request: Serialize input + Request->>serverInvoke: Cap'n Proto message + serverInvoke->>ReadField: Deserialize input + + Note over clientInvoke,Response: Output Parameter Flow + serverInvoke-->>BuildField_S: BuildField(output) + BuildField_S-->Response: Serialize output + Response-->>ReadField_C: Cap'n Proto message + ReadField_C-->>clientInvoke: Deserialize output +``` + +### Detailed Serialization Mechanism + +Parameters are represented as Fields that must be set on Cap'n Proto Builder objects (for sending) and read from Reader objects (for receiving). + +#### Building Fields + +`BuildField` uses a generated parameter `Accessor` to set the appropriate field in the Cap'n Proto Builder object. + +```mermaid +sequenceDiagram + participant clientInvoke as clientInvoke or
serverInvoke + participant BuildField + participant Accessor + participant Builder as Params::Builder + + Note over clientInvoke,Builder: Serializing Parameters + clientInvoke->>BuildField: BuildField(param1) + BuildField->>Accessor: Use generated field accessor + Accessor->>Builder: builder.setField1(param1) + + clientInvoke->>BuildField: BuildField(param2) + BuildField->>Accessor: Use generated field Accessor + Accessor->>Builder: builder.setField2(param2) +``` + +#### Reading Fields + +`ReadField` uses a generated parameter `Accessor` to read the appropriate field from the Cap'n Proto Reader object and reconstruct C++ parameters. + +```mermaid +sequenceDiagram + participant serverInvoke as clientInvoke or
serverInvoke + participant ReadField + participant Accessor + participant Reader as Params::Reader + participant ServerCall + + Note over serverInvoke,ServerCall: Deserializing Parameters + serverInvoke->>ReadField: Read param1 + ReadField->>Accessor: Use generated field accessor + Accessor->>Reader: reader.getField1() + Reader-->>ServerCall: call function with param1 +``` + +## Server-Side Request Processing + +The generated server code uses a Russian nesting doll structure to process method fields. Each `ServerField` wraps another `ServerField` (for the next parameter), or wraps `ServerRet` (for the return value), which finally wraps `ServerCall` (which invokes the actual C++ method). + +Each `ServerField` invokes `PassField`, which: +1. Calls `ReadField` to deserialize the parameter from the `Params::Reader` +2. Calls the next nested layer's `invoke()` with the accumulated parameters +3. Calls `BuildField` to serialize the parameter back if it's an output parameter + +`ServerRet` invokes the next layer (typically `ServerCall`), stores the result, and calls `BuildField` to serialize it into the `Results::Builder`. + +`ServerCall` uses the generated `ProxyMethod::impl` pointer-to-member to invoke the actual C++ method on the wrapped implementation object. + +```mermaid +sequenceDiagram + participant serverInvoke + participant SF1 as ServerField
(param 1) + participant SF2 as ServerField
(param 2) + participant SR as ServerRet
(return value) + participant SC as ServerCall + participant PMT as ProxyMethodTraits + participant Impl as Actual C++ Method + + serverInvoke->>SF1: SF1::invoke + SF1->>SF2: SF2::invoke + SF2->>SR: SR::invoke + SR->>SC: SC::invoke + SC->>PMT: PMT::invoke + PMT->>Impl: Call impl method + Impl->>PMT: return + PMT->>SC: return + SC->>SR: return + SR->>SF2: return + SF2->>SF1: return + SF1->>serverInvoke: return +``` + +## Advanced Features + +### Callbacks + +Callbacks (passed as `std::function` arguments) are intercepted by `CustomBuildField` and converted into Cap'n Proto capabilities that can be invoked across process boundaries. On the receiving end, `CustomReadField` intercepts the capability and constructs a `ProxyCallFn` object with an `operator()` that sends function calls back over the socket to invoke the original callback. + +```mermaid +sequenceDiagram + participant CT as Client Thread + participant C as clientInvoke + participant CBF1 as CustomBuildField (Client) + participant S as Socket + participant CRF1 as CustomReadField (Server) + participant Srv as Server Code + participant PCF as ProxyCallFn + + C->>CBF1: send function parameter + CBF1->>S: creates a Server for the function and sends a capability + S->>CRF1: receives a capability and creates ProxyCallFn + CRF1->>Srv: + Srv->>PCF: call the callback + PCF-->>CT: sends request to Client +``` + +### Thread Mapping + +Thread mapping enables each client thread to have a dedicated server thread processing its requests, preserving thread-local state and allowing recursive mutex usage across process boundaries. + +Thread mapping is initialized by defining an interface method with a `ThreadMap` parameter and/or response. The example below adds `ThreadMap` to the `construct` method because libmultiprocess calls the `construct` method automatically. + +```capnp +interface InitInterface $Proxy.wrap("Init") { + construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap); +} +``` + +- **ThreadMap in parameter**: The client's `CustomBuildField` creates a `ThreadMap::Server` capability and sends it to the server, where `CustomReadField` stores the `ThreadMap::Client` in `connection.m_thread_map` +- **ThreadMap in response**: The server's `CustomBuildField` creates a `ThreadMap::Server` capability and sends it to the client, where `CustomReadField` stores the `ThreadMap::Client` in `connection.m_thread_map` + +You can specify ThreadMap in the parameter only, response only, or both: +- **Parameter only**: Server can create threads on the client +- **Response only**: Client can create threads on the server +- **Both (as shown)**: Bidirectional thread creation + +When both parameter and response include ThreadMap, both processes end up with `ThreadMap::Client` capabilities pointing to each other's `ThreadMap::Server`, allowing both sides to create threads on the other process. + +### Async Processing with Context + +By adding a `Context` parameter to a method in the capnp interface file, you enable async processing where the client tells the server to execute the request in a separate worker thread. For example: + +```capnp +processData @5 (context :Proxy.Context, data :Data) -> (result :Result); +``` + +If a method does not have a `Context` parameter, then libmultiprocess will execute IPC requests invoking that method on the I/O event loop thread. This is fine for fast and non-blocking methods, but should be avoided for any methods that are slow or blocking or make any IPC calls(including callbacks to the client), since as long as the method is executing, the Cap'n Proto event loop will not be able to perform any I/O. + +When a method has a `Context` parameter: + +**Client side** (`CustomBuildField`): +If this is the first asynchronous request made from the current client thread, `CustomBuildField` will: +1. Call `connection.m_thread_map.makeThreadRequest()` to request a dedicated worker thread on the server (stored in `request_threads` map) +2. Set the remote thread capability in `Context.thread` +3. Create a local `Thread::Server` object for the current thread (stored in `callback_threads` map) +4. Set the local thread capability in `Context.callbackThread` + +Subsequent requests will reuse the existing thread capabilities held in `callback_threads` and `request_threads`. + +**Server side** (`PassField`): +1. Looks up the local `Thread::Server` object specified by `context.thread` +2. The worker thread: + - Stores `context.callbackThread` in its `request_threads` map (so callbacks go to the right client thread) + - Posts the work lambda to that thread's queue via `waiter->post(invoke)` + - Cleans up the `request_threads` entry + +## Interface Definitions As explained in the [usage](usage.md) document, interface descriptions need to be consumed both by the _libmultiprocess_ code generator, and by C++ code that calls and implements the interfaces. The C++ code only needs to know about C++ arguments and return types, while the code generator only needs to know about capnp arguments and return types, but both need to know class and method names, so the corresponding `.h` and `.capnp` source files contain some of the same information, and have to be kept in sync manually when methods or parameters change. Despite the redundancy, reconciling the interface definitions is designed to be _straightforward_ and _safe_. _Straightforward_ because there is no need to write manual serialization code or use awkward intermediate types like [`UniValue`](https://github.com/bitcoin/bitcoin/blob/master/src/univalue/include/univalue.h) instead of native types. _Safe_ because if there are any inconsistencies between API and data definitions (even minor ones like using a narrow int data type for a wider int API input), there are errors at build time instead of errors or bugs at runtime. diff --git a/src/ipc/libmultiprocess/doc/versions.md b/src/ipc/libmultiprocess/doc/versions.md new file mode 100644 index 00000000..9a7b1223 --- /dev/null +++ b/src/ipc/libmultiprocess/doc/versions.md @@ -0,0 +1,61 @@ +# libmultiprocess Versions + +Library versions are tracked with simple +[tags](https://github.com/bitcoin-core/libmultiprocess/tags) and +[branches](https://github.com/bitcoin-core/libmultiprocess/branches). + +Versioning policy is described in the [version.h](../include/mp/version.h) +include. + +## v8 +- Current unstable version in master branch. + +## v7.0 +- Adds SpawnProcess race fix, cmake `target_capnp_sources` option, ci and documentation improvements. +- Used in Bitcoin Core master branch, pulled in by [#34363](https://github.com/bitcoin/bitcoin/pull/34363). + +## v7.0-pre1 + +- Adds support for log levels to reduce logging and "thread busy" error to avoid a crash on misuse. Fixes intermittent mptest hang and makes other minor improvements. +- Used in Bitcoin Core 30.1 and 30.2 releases and 30.x branch. Pulled in by [#33412](https://github.com/bitcoin/bitcoin/pull/33412), [#33518](https://github.com/bitcoin/bitcoin/pull/33518), and [#33519](https://github.com/bitcoin/bitcoin/pull/33519). + +## v6.0 + +- Adds fixes for unclean shutdowns and thread sanitizer issues and adds CI scripts. +- Drops `EventLoop::addClient` and `EventLoop::removeClient` methods, + requiring clients to use new `EventLoopRef` class instead. +- Used in Bitcoin Core 30.0 release. Pulled in by [#31741](https://github.com/bitcoin/bitcoin/pull/31741), [#32641](https://github.com/bitcoin/bitcoin/pull/32641), [#32345](https://github.com/bitcoin/bitcoin/pull/32345), [#33241](https://github.com/bitcoin/bitcoin/pull/33241), and [#33322](https://github.com/bitcoin/bitcoin/pull/33322). + +## v5.0 +- Adds build improvements and tidy/warning fixes. +- Used in Bitcoin Core 29 releases, pulled in by [#31945](https://github.com/bitcoin/bitcoin/pull/31945). + +## v5.0-pre1 +- Adds many improvements to Bitcoin Core mining interface: splitting up type headers, fixing shutdown bugs, adding subtree build support. +- Broke up `proxy-types.h` into `type-*.h` files requiring clients to explicitly + include overloads needed for C++ ↔️ Cap'n Proto type conversions. +- Now requires C++ 20 support. +- Minimum required version for Bitcoin Core 29 releases, pulled in by [#30509](https://github.com/bitcoin/bitcoin/pull/30509), [#30510](https://github.com/bitcoin/bitcoin/pull/30510), [#31105](https://github.com/bitcoin/bitcoin/pull/31105), [#31740](https://github.com/bitcoin/bitcoin/pull/31740). + +## v4.0 +- Added better cmake support, installing cmake package files so clients do not + need to use pkgconfig. +- Used in Bitcoin Core 28 releases, pulled in by [#30490](https://github.com/bitcoin/bitcoin/pull/30490) and [#30513](https://github.com/bitcoin/bitcoin/pull/30513). + +## v3.0 +- Dropped compatibility with Cap'n Proto versions before 0.7. +- Used in Bitcoin Core 27 releases, pulled in by [#28735](https://github.com/bitcoin/bitcoin/pull/28735), [#28907](https://github.com/bitcoin/bitcoin/pull/28907), and [#29276](https://github.com/bitcoin/bitcoin/pull/29276). + +## v2.0 +- Changed `PassField` function signature. +- Now requires C++17 support. +- Used in Bitcoin Core 25 and 26 releases, pulled in by [#26672](https://github.com/bitcoin/bitcoin/pull/26672). + +## v1.0 +- Dropped hardcoded includes in generated files, now requiring `include` and + `includeTypes` annotations. +- Used in Bitcoin Core 22, 23, and 24 releases, pulled in by [#19160](https://github.com/bitcoin/bitcoin/pull/19160). + +## v0.0 +- Initial version used in a downstream release. +- Used in Bitcoin Core 21 releases, pulled in by [#16367](https://github.com/bitcoin/bitcoin/pull/16367), [#18588](https://github.com/bitcoin/bitcoin/pull/18588), and [#18677](https://github.com/bitcoin/bitcoin/pull/18677). diff --git a/src/ipc/libmultiprocess/include/mp/proxy-io.h b/src/ipc/libmultiprocess/include/mp/proxy-io.h index f7367468..b3c67a32 100644 --- a/src/ipc/libmultiprocess/include/mp/proxy-io.h +++ b/src/ipc/libmultiprocess/include/mp/proxy-io.h @@ -317,12 +317,12 @@ class EventLoop void* m_context; }; -//! Single element task queue used to handle recursive capnp calls. (If server -//! makes an callback into the client in the middle of a request, while client +//! Single element task queue used to handle recursive capnp calls. (If the +//! server makes a callback into the client in the middle of a request, while the client //! thread is blocked waiting for server response, this is what allows the -//! client to run the request in the same thread, the same way code would run in -//! single process, with the callback sharing same thread stack as the original -//! call. +//! client to run the request in the same thread, the same way code would run in a +//! single process, with the callback sharing the same thread stack as the original +//! call.) struct Waiter { Waiter() = default; diff --git a/src/ipc/libmultiprocess/include/mp/proxy-types.h b/src/ipc/libmultiprocess/include/mp/proxy-types.h index 22468b15..7301aa59 100644 --- a/src/ipc/libmultiprocess/include/mp/proxy-types.h +++ b/src/ipc/libmultiprocess/include/mp/proxy-types.h @@ -87,7 +87,7 @@ struct StructField // actually construct the read destination object. For example, if a std::string // is being read, the ReadField call will call the custom emplace_fn with char* // and size_t arguments, and the emplace function can decide whether to call the -// constructor via the operator or make_shared or emplace or just return a +// constructor via the operator, make_shared, emplace or just return a // temporary string that is moved from. template struct ReadDestEmplace @@ -205,11 +205,11 @@ void BuildField(TypeList, Context& context, Output&& output, Valu } } -// Adapter to let BuildField overloads methods work set & init list elements as -// if they were fields of a struct. If BuildField is changed to use some kind of -// accessor class instead of calling method pointers, then then maybe this could -// go away or be simplified, because would no longer be a need to return -// ListOutput method pointers emulating capnp struct method pointers.. +// Adapter that allows BuildField overloads to work with, set, and initialize list +// elements as if they were fields of a struct. If BuildField is changed to use some +// kind of accessor class instead of calling method pointers, then maybe this could +// go away or be simplified, because there would no longer be a need to return +// ListOutput method pointers emulating capnp struct method pointers. template struct ListOutput; diff --git a/src/ipc/libmultiprocess/include/mp/util.h b/src/ipc/libmultiprocess/include/mp/util.h index e5b4dd19..6cd11fda 100644 --- a/src/ipc/libmultiprocess/include/mp/util.h +++ b/src/ipc/libmultiprocess/include/mp/util.h @@ -221,13 +221,16 @@ using FdToArgsFn = std::function(int fd)>; //! Spawn a new process that communicates with the current process over a socket //! pair. Returns pid through an output argument, and file descriptor for the -//! local side of the socket. Invokes fd_to_args callback with the remote file -//! descriptor number which returns the command line arguments that should be -//! used to execute the process, and which should have the remote file -//! descriptor embedded in whatever format the child process expects. +//! local side of the socket. +//! The fd_to_args callback is invoked in the parent process before fork(). +//! It must not rely on child pid/state, and must return the command line +//! arguments that should be used to execute the process. Embed the remote file +//! descriptor number in whatever format the child process expects. int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args); //! Call execvp with vector args. +//! Not safe to call in a post-fork child of a multi-threaded process. +//! Currently only used by mpgen at build time. void ExecProcess(const std::vector& args); //! Wait for a process to exit and return its exit code. diff --git a/src/ipc/libmultiprocess/include/mp/version.h b/src/ipc/libmultiprocess/include/mp/version.h new file mode 100644 index 00000000..a6b0096f --- /dev/null +++ b/src/ipc/libmultiprocess/include/mp/version.h @@ -0,0 +1,34 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef MP_VERSION_H +#define MP_VERSION_H + +//! @file +//! @brief Major and minor version numbers +//! +//! Versioning uses a cruder form of SemVer where the major number is +//! incremented with all significant changes, regardless of whether they are +//! backward compatible, and the minor number is treated like a patch level and +//! only incremented when a fix or backport is applied to an old branch. + +//! Major version number. Should be incremented after any release or external +//! usage of the library (like a subtree update) so the previous number +//! identifies that release. Should also be incremented before any change that +//! breaks backward compatibility or introduces nontrivial features, so +//! downstream code can use it to detect compatibility. +//! +//! Each time this is incremented, a new stable branch should be created. E.g. +//! when this is incremented to 8, a "v7" stable branch should be created +//! pointing at the prior merge commit. The /doc/versions.md file should also be +//! updated, noting any significant or incompatible changes made since the +//! previous version. +#define MP_MAJOR_VERSION 8 + +//! Minor version number. Should be incremented in stable branches after +//! backporting changes. The /doc/versions.md file should also be updated to +//! list the new minor version. +#define MP_MINOR_VERSION 0 + +#endif // MP_VERSION_H diff --git a/src/ipc/libmultiprocess/src/mp/gen.cpp b/src/ipc/libmultiprocess/src/mp/gen.cpp index 18400710..cedb4dc4 100644 --- a/src/ipc/libmultiprocess/src/mp/gen.cpp +++ b/src/ipc/libmultiprocess/src/mp/gen.cpp @@ -136,7 +136,7 @@ static bool BoxedType(const ::capnp::Type& type) // include_prefix can be used to control relative include paths used in // generated files. For example if src_file is "/a/b/c/d/file.canp" and // include_prefix is "/a/b/c" include lines like -// "#include " "#include "i +// "#include ", "#include " // will be generated. static void Generate(kj::StringPtr src_prefix, kj::StringPtr include_prefix, diff --git a/src/ipc/libmultiprocess/src/mp/proxy.cpp b/src/ipc/libmultiprocess/src/mp/proxy.cpp index 57545d37..9d81284e 100644 --- a/src/ipc/libmultiprocess/src/mp/proxy.cpp +++ b/src/ipc/libmultiprocess/src/mp/proxy.cpp @@ -102,12 +102,12 @@ Connection::~Connection() // The ProxyClient cleanup handlers are synchronous because they are fast // and don't do anything besides release capnp resources and reset state so // future calls to client methods immediately throw exceptions instead of - // trying to communicating across the socket. The synchronous callbacks set + // trying to communicate across the socket. The synchronous callbacks set // ProxyClient capability pointers to null, so new method calls on client // objects fail without triggering i/o or relying on event loop which may go // out of scope or trigger obscure capnp i/o errors. // - // The ProxySever cleanup handlers call user defined destructors on server + // The ProxyServer cleanup handlers call user defined destructors on the server // object, which can run arbitrary blocking bitcoin code so they have to run // asynchronously in a different thread. The asynchronous cleanup functions // intentionally aren't started until after the synchronous cleanup @@ -136,7 +136,7 @@ Connection::~Connection() // // Either way disconnect code runs in the event loop thread and called both // on clean and unclean shutdowns. In unclean shutdown case when the - // connection is broken, sync and async cleanup lists will filled with + // connection is broken, sync and async cleanup lists will be filled with // callbacks. In the clean shutdown case both lists will be empty. Lock lock{m_loop->m_mutex}; while (!m_sync_cleanup_fns.empty()) { diff --git a/src/ipc/libmultiprocess/src/mp/util.cpp b/src/ipc/libmultiprocess/src/mp/util.cpp index 509913b8..7b486082 100644 --- a/src/ipc/libmultiprocess/src/mp/util.cpp +++ b/src/ipc/libmultiprocess/src/mp/util.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,17 @@ namespace fs = std::filesystem; namespace mp { namespace { +std::vector MakeArgv(const std::vector& args) +{ + std::vector argv; + argv.reserve(args.size() + 1); + for (const auto& arg : args) { + argv.push_back(const_cast(arg.c_str())); + } + argv.push_back(nullptr); + return argv; +} + //! Return highest possible file descriptor. size_t MaxFd() { @@ -111,35 +123,57 @@ int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args) throw std::system_error(errno, std::system_category(), "socketpair"); } + // Evaluate the callback and build the argv array before forking. + // + // The parent process may be multi-threaded and holding internal library + // locks at fork time. In that case, running code that allocates memory or + // takes locks in the child between fork() and exec() can deadlock + // indefinitely. Precomputing arguments in the parent avoids this. + const std::vector args{fd_to_args(fds[0])}; + const std::vector argv{MakeArgv(args)}; + pid = fork(); if (pid == -1) { throw std::system_error(errno, std::system_category(), "fork"); } - // Parent process closes the descriptor for socket 0, child closes the descriptor for socket 1. + // Parent process closes the descriptor for socket 0, child closes the + // descriptor for socket 1. On failure, the parent throws, but the child + // must _exit(126) (post-fork child must not throw). if (close(fds[pid ? 0 : 1]) != 0) { - throw std::system_error(errno, std::system_category(), "close"); + if (pid) { + (void)close(fds[1]); + throw std::system_error(errno, std::system_category(), "close"); + } + static constexpr char msg[] = "SpawnProcess(child): close(fds[1]) failed\n"; + const ssize_t writeResult = ::write(STDERR_FILENO, msg, sizeof(msg) - 1); + (void)writeResult; + _exit(126); } + if (!pid) { - // Child process must close all potentially open descriptors, except socket 0. + // Child process must close all potentially open descriptors, except + // socket 0. Do not throw, allocate, or do non-fork-safe work here. const int maxFd = MaxFd(); for (int fd = 3; fd < maxFd; ++fd) { if (fd != fds[0]) { close(fd); } } - ExecProcess(fd_to_args(fds[0])); + + execvp(argv[0], argv.data()); + // NOTE: perror() is not async-signal-safe; calling it here in a + // post-fork child may deadlock in multithreaded parents. + // TODO: Report errors to the parent via a pipe (e.g. write errno) + // so callers can get diagnostics without relying on perror(). + perror("execvp failed"); + _exit(127); } return fds[1]; } void ExecProcess(const std::vector& args) { - std::vector argv; - argv.reserve(args.size()); - for (const auto& arg : args) { - argv.push_back(const_cast(arg.c_str())); - } - argv.push_back(nullptr); + const std::vector argv{MakeArgv(args)}; if (execvp(argv[0], argv.data()) != 0) { perror("execvp failed"); if (errno == ENOENT && !args.empty()) { @@ -152,7 +186,7 @@ void ExecProcess(const std::vector& args) int WaitProcess(int pid) { int status; - if (::waitpid(pid, &status, 0 /* options */) != pid) { + if (::waitpid(pid, &status, /*options=*/0) != pid) { throw std::system_error(errno, std::system_category(), "waitpid"); } return status; diff --git a/src/ipc/libmultiprocess/test/CMakeLists.txt b/src/ipc/libmultiprocess/test/CMakeLists.txt index 2a1a7e9e..1f21ba44 100644 --- a/src/ipc/libmultiprocess/test/CMakeLists.txt +++ b/src/ipc/libmultiprocess/test/CMakeLists.txt @@ -26,6 +26,7 @@ if(BUILD_TESTING AND TARGET CapnProto::kj-test) ${MP_PROXY_HDRS} mp/test/foo-types.h mp/test/foo.h + mp/test/spawn_tests.cpp mp/test/test.cpp ) include(${PROJECT_SOURCE_DIR}/cmake/TargetCapnpSources.cmake) diff --git a/src/ipc/libmultiprocess/test/mp/test/spawn_tests.cpp b/src/ipc/libmultiprocess/test/mp/test/spawn_tests.cpp new file mode 100644 index 00000000..4c7edba4 --- /dev/null +++ b/src/ipc/libmultiprocess/test/mp/test/spawn_tests.cpp @@ -0,0 +1,110 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +// Poll for child process exit using waitpid(..., WNOHANG) until the child exits +// or timeout expires. Returns true if the child exited and status_out was set. +// Returns false on timeout or error. +static bool WaitPidWithTimeout(int pid, std::chrono::milliseconds timeout, int& status_out) +{ + const auto deadline = std::chrono::steady_clock::now() + timeout; + while (std::chrono::steady_clock::now() < deadline) { + const int r = ::waitpid(pid, &status_out, WNOHANG); + if (r == pid) return true; + if (r == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds{1}); + continue; + } + // waitpid error + return false; + } + return false; +} + +} // namespace + +KJ_TEST("SpawnProcess does not run callback in child") +{ + // This test is designed to fail deterministically if fd_to_args is invoked + // in the post-fork child: a mutex held by another parent thread at fork + // time appears locked forever in the child. + std::mutex target_mutex; + std::mutex control_mutex; + std::condition_variable control_cv; + bool locked{false}; + bool release{false}; + + // Holds target_mutex until the releaser thread updates release + std::thread locker([&] { + std::unique_lock target_lock(target_mutex); + { + std::lock_guard g(control_mutex); + locked = true; + } + control_cv.notify_one(); + + std::unique_lock control_lock(control_mutex); + control_cv.wait(control_lock, [&] { return release; }); + }); + + // Wait for target_mutex to be held by the locker thread. + { + std::unique_lock l(control_mutex); + control_cv.wait(l, [&] { return locked; }); + } + + // Release the lock shortly after SpawnProcess starts. + std::thread releaser([&] { + // In the unlikely event a CI machine overshoots this delay, a + // regression could be missed. This is preferable to spurious + // test failures. + std::this_thread::sleep_for(std::chrono::milliseconds{50}); + { + std::lock_guard g(control_mutex); + release = true; + } + control_cv.notify_one(); + }); + + int pid{-1}; + const int fd{mp::SpawnProcess(pid, [&](int child_fd) -> std::vector { + // If this callback runs in the post-fork child, target_mutex appears + // locked forever (the owning thread does not exist), so this deadlocks. + std::lock_guard g(target_mutex); + return {"true", std::to_string(child_fd)}; + })}; + ::close(fd); + + int status{0}; + // Give the child up to 1 second to exit. If it does not, terminate it and + // reap it to avoid leaving a zombie behind. + const bool exited{WaitPidWithTimeout(pid, std::chrono::milliseconds{1000}, status)}; + if (!exited) { + ::kill(pid, SIGKILL); + ::waitpid(pid, &status, /*options=*/0); + } + + releaser.join(); + locker.join(); + + KJ_EXPECT(exited, "Timeout waiting for child process to exit"); + KJ_EXPECT(WIFEXITED(status) && WEXITSTATUS(status) == 0); +} diff --git a/src/ipc/libmultiprocess/test/mp/test/test.cpp b/src/ipc/libmultiprocess/test/mp/test/test.cpp index e32365fa..aa155685 100644 --- a/src/ipc/libmultiprocess/test/mp/test/test.cpp +++ b/src/ipc/libmultiprocess/test/mp/test/test.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,12 +33,19 @@ #include #include #include +#include #include #include namespace mp { namespace test { +/** Check version.h header values */ +constexpr auto kMP_MAJOR_VERSION{MP_MAJOR_VERSION}; +constexpr auto kMP_MINOR_VERSION{MP_MINOR_VERSION}; +static_assert(std::is_integral_v, "MP_MAJOR_VERSION must be an integral constant"); +static_assert(std::is_integral_v, "MP_MINOR_VERSION must be an integral constant"); + /** * Test setup class creating a two way connection between a * ProxyServer object and a ProxyClient. diff --git a/src/secp256k1/.github/actions/run-in-docker-action/action.yml b/src/secp256k1/.github/actions/run-in-docker-action/action.yml index 74933686..5d46ca1b 100644 --- a/src/secp256k1/.github/actions/run-in-docker-action/action.yml +++ b/src/secp256k1/.github/actions/run-in-docker-action/action.yml @@ -4,37 +4,35 @@ inputs: dockerfile: description: 'A Dockerfile that defines an image' required: true - tag: - description: 'A tag of an image' - required: true + scope: + description: 'A cached image scope' + required: false + default: ${{ runner.arch }} command: description: 'A command to run in a container' - required: false - default: ./ci/ci.sh + required: true runs: using: "composite" steps: - uses: docker/setup-buildx-action@v3 - - uses: docker/build-push-action@v5 + - uses: docker/build-push-action@v6 id: main_builder continue-on-error: true with: context: . file: ${{ inputs.dockerfile }} - tags: ${{ inputs.tag }} load: true - cache-from: type=gha + cache-from: type=gha,scope=${{ inputs.scope }} - - uses: docker/build-push-action@v5 + - uses: docker/build-push-action@v6 id: retry_builder if: steps.main_builder.outcome == 'failure' with: context: . file: ${{ inputs.dockerfile }} - tags: ${{ inputs.tag }} load: true - cache-from: type=gha + cache-from: type=gha,scope=${{ inputs.scope }} - # Workaround for https://github.com/google/sanitizers/issues/1614 . # The underlying issue has been fixed in clang 18.1.3. @@ -47,7 +45,8 @@ runs: $(echo '${{ toJSON(env) }}' | jq -r 'keys[] | "--env \(.) "') \ --volume ${{ github.workspace }}:${{ github.workspace }} \ --workdir ${{ github.workspace }} \ - ${{ inputs.tag }} bash -c " + $(docker images -q | head -n1) \ + bash -c " git config --global --add safe.directory ${{ github.workspace }} ${{ inputs.command }} " diff --git a/src/secp256k1/.github/workflows/ci.yml b/src/secp256k1/.github/workflows/ci.yml index d5196c53..59d22514 100644 --- a/src/secp256k1/.github/workflows/ci.yml +++ b/src/secp256k1/.github/workflows/ci.yml @@ -67,12 +67,11 @@ jobs: network=host - name: Build container - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v6 with: file: ./ci/linux-debian.Dockerfile - tags: ${{ matrix.arch }}-debian-image - cache-from: type=gha - cache-to: type=gha,mode=min + cache-from: type=gha,scope=${{ runner.arch }} + cache-to: type=gha,scope=${{ runner.arch }},mode=min x86_64-debian: name: "x86_64: Linux (Debian stable)" @@ -108,17 +107,20 @@ jobs: CC: ${{ matrix.cc }} steps: - - name: Checkout - uses: actions/checkout@v4 + - &CHECKOUT + name: Checkout + uses: actions/checkout@v5 - - name: CI script + - &CI_SCRIPT_IN_DOCKER + name: CI script env: ${{ matrix.configuration.env_vars }} uses: ./.github/actions/run-in-docker-action with: dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image + command: ./ci/ci.sh - - name: Print logs + - &PRINT_LOGS + name: Print logs uses: ./.github/actions/print-logs if: ${{ !cancelled() }} @@ -130,6 +132,8 @@ jobs: strategy: fail-fast: false matrix: + configuration: + - env_vars: {} cc: - 'i686-linux-gnu-gcc' - 'clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include' @@ -145,24 +149,20 @@ jobs: CC: ${{ matrix.cc }} steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS s390x_debian: name: "s390x (big-endian): Linux (Debian stable, QEMU)" runs-on: ubuntu-latest needs: docker_cache + strategy: + matrix: + configuration: + - env_vars: {} + env: WRAPPER_CMD: 'qemu-s390x' SECP256K1_TEST_ITERS: 16 @@ -177,19 +177,9 @@ jobs: CTIMETESTS: 'no' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} - + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS arm32_debian: name: "ARM32: Linux (Debian stable, QEMU)" @@ -217,19 +207,9 @@ jobs: CTIMETESTS: 'no' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - env: ${{ matrix.configuration.env_vars }} - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS arm64-debian: name: "arm64: Linux (Debian stable)" @@ -251,6 +231,8 @@ jobs: strategy: fail-fast: false matrix: + configuration: + - env_vars: {} cc: - 'gcc' - 'clang' @@ -258,24 +240,20 @@ jobs: - 'clang-snapshot' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: arm64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS ppc64le_debian: name: "ppc64le: Linux (Debian stable, QEMU)" runs-on: ubuntu-latest needs: docker_cache + strategy: + matrix: + configuration: + - env_vars: {} + env: WRAPPER_CMD: 'qemu-ppc64le' SECP256K1_TEST_ITERS: 16 @@ -290,51 +268,35 @@ jobs: CTIMETESTS: 'no' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} - + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS valgrind_debian: - name: "Valgrind ${{ matrix.binary_arch }} (memcheck)" - runs-on: ${{ matrix.runner }} + name: "Valgrind ${{ matrix.configuration.binary_arch }} (memcheck)" + runs-on: ${{ matrix.configuration.runner }} needs: docker_cache strategy: fail-fast: false matrix: - include: - - docker_arch: x64 - runner: ubuntu-latest + configuration: + - runner: ubuntu-latest binary_arch: x64 env_vars: { CC: 'clang', ASM: 'auto' } - - docker_arch: x64 - runner: ubuntu-latest + - runner: ubuntu-latest binary_arch: i686 env_vars: { CC: 'i686-linux-gnu-gcc', HOST: 'i686-linux-gnu', ASM: 'auto' } - - docker_arch: arm64 - runner: ubuntu-24.04-arm + - runner: ubuntu-24.04-arm binary_arch: arm64 env_vars: { CC: 'clang', ASM: 'auto' } - - docker_arch: x64 - runner: ubuntu-latest + - runner: ubuntu-latest binary_arch: x64 env_vars: { CC: 'clang', ASM: 'no', ECMULTGENKB: 2, ECMULTWINDOW: 2 } - - docker_arch: x64 - runner: ubuntu-latest + - runner: ubuntu-latest binary_arch: i686 env_vars: { CC: 'i686-linux-gnu-gcc', HOST: 'i686-linux-gnu', ASM: 'no', ECMULTGENKB: 2, ECMULTWINDOW: 2 } - - docker_arch: arm64 - runner: ubuntu-24.04-arm + - runner: ubuntu-24.04-arm binary_arch: arm64 env_vars: { CC: 'clang', ASM: 'no', ECMULTGENKB: 2, ECMULTWINDOW: 2 } @@ -352,19 +314,9 @@ jobs: SECP256K1_TEST_ITERS: 2 steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - env: ${{ matrix.env_vars }} - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: ${{ matrix.docker_arch }}-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS sanitizers_debian: name: "UBSan, ASan, LSan" @@ -396,19 +348,9 @@ jobs: SYMBOL_CHECK: 'no' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - env: ${{ matrix.configuration.env_vars }} - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS msan_debian: name: "MSan" @@ -432,6 +374,9 @@ jobs: # when ctime_tests when enabled. CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g' CTIMETESTS: 'no' + cc: + - 'clang' + - 'clang-snapshot' env: ECDH: 'yes' @@ -440,27 +385,16 @@ jobs: SCHNORRSIG: 'yes' MUSIG: 'yes' ELLSWIFT: 'yes' - CC: 'clang' + CC: ${{ matrix.cc }} SECP256K1_TEST_ITERS: 32 ASM: 'no' WITH_VALGRIND: 'no' SYMBOL_CHECK: 'no' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - env: ${{ matrix.configuration.env_vars }} - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} - + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS mingw_debian: name: ${{ matrix.configuration.job_name }} @@ -490,24 +424,13 @@ jobs: HOST: 'i686-w64-mingw32' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - env: ${{ matrix.configuration.env_vars }} - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *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' @@ -531,8 +454,7 @@ jobs: - BUILD: 'distcheck' steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: Install Homebrew packages run: | @@ -542,19 +464,23 @@ jobs: - name: Install and cache Valgrind uses: ./.github/actions/install-homebrew-valgrind - - name: CI script + - &CI_SCRIPT_ON_HOST + name: CI script 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 - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *PRINT_LOGS arm64-macos-native: name: "ARM64: macOS Sonoma" @@ -583,32 +509,16 @@ jobs: - BUILD: 'distcheck' steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: Install Homebrew packages run: | brew install --quiet automake libtool gcc ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc - - name: CI script - env: ${{ matrix.env_vars }} - run: ./ci/ci.sh - - - 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 - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} - + - *CI_SCRIPT_ON_HOST + - *SYMBOL_CHECK_MACOS + - *PRINT_LOGS win64-native: name: ${{ matrix.configuration.job_name }} @@ -631,12 +541,19 @@ jobs: cpp_flags: '/DSECP256K1_MSVC_MULH_TEST_OVERRIDE' - job_name: 'x86 (MSVC): Windows (VS 2022)' cmake_options: '-A Win32' - - job_name: 'x64 (MSVC): Windows (clang-cl)' - cmake_options: '-T ClangCL' + - job_name: 'x64 (clang-cl): Windows (VS 2022, shared)' + cmake_options: '-T ClangCL -DBUILD_SHARED_LIBS=ON' + symbol_check: 'true' + - job_name: 'x64 (clang-cl): Windows (VS 2022, static)' + cmake_options: '-T ClangCL -DBUILD_SHARED_LIBS=OFF' + - job_name: 'x64 (clang-cl): Windows (VS 2022, int128_struct)' + cmake_options: '-T ClangCL -DSECP256K1_TEST_OVERRIDE_WIDE_MULTIPLY=int128_struct' + - job_name: 'x64 (clang-cl): Windows (VS 2022, int128_struct with __(u)mulh)' + cmake_options: '-T ClangCL -DSECP256K1_TEST_OVERRIDE_WIDE_MULTIPLY=int128_struct' + cpp_flags: '/DSECP256K1_MSVC_MULH_TEST_OVERRIDE' steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: Generate buildsystem run: cmake -E env CFLAGS="/WX ${{ matrix.configuration.cpp_flags }}" cmake -B build -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON ${{ matrix.configuration.cmake_options }} @@ -671,8 +588,7 @@ jobs: runs-on: windows-2022 steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: Add cl.exe to PATH uses: ilammy/msvc-dev-cmd@v1 @@ -686,6 +602,11 @@ jobs: runs-on: ubuntu-latest needs: docker_cache + strategy: + matrix: + configuration: + - env_vars: {} + env: CC: 'g++' CFLAGS: '-fpermissive -g' @@ -699,18 +620,9 @@ jobs: ELLSWIFT: 'yes' steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: CI script - uses: ./.github/actions/run-in-docker-action - with: - dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image - - - name: Print logs - uses: ./.github/actions/print-logs - if: ${{ !cancelled() }} + - *CHECKOUT + - *CI_SCRIPT_IN_DOCKER + - *PRINT_LOGS cxx_headers_debian: name: "C++ (public headers)" @@ -718,14 +630,12 @@ jobs: needs: docker_cache steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: CI script uses: ./.github/actions/run-in-docker-action with: dockerfile: ./ci/linux-debian.Dockerfile - tag: x64-debian-image command: | g++ -Werror include/*.h clang -Werror -x c++-header include/*.h @@ -738,8 +648,7 @@ jobs: options: --user root steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - name: CI script run: | @@ -750,8 +659,7 @@ jobs: runs-on: ubuntu-latest steps: - - name: Checkout - uses: actions/checkout@v4 + - *CHECKOUT - run: ./autogen.sh && ./configure --enable-dev-mode && make distcheck diff --git a/src/secp256k1/CHANGELOG.md b/src/secp256k1/CHANGELOG.md index 53c787b3..29d23a96 100644 --- a/src/secp256k1/CHANGELOG.md +++ b/src/secp256k1/CHANGELOG.md @@ -5,7 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [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 @@ -187,7 +197,7 @@ 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 +[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/src/secp256k1/CMakeLists.txt b/src/secp256k1/CMakeLists.txt index 25919fd5..11dc3f6e 100644 --- a/src/secp256k1/CMakeLists.txt +++ b/src/secp256k1/CMakeLists.txt @@ -34,10 +34,8 @@ set(CMAKE_C_EXTENSIONS OFF) #============================= # Configurable options #============================= -option(BUILD_SHARED_LIBS "Build shared libraries." ON) -option(SECP256K1_DISABLE_SHARED "Disable shared library. Overrides BUILD_SHARED_LIBS." OFF) -if(SECP256K1_DISABLE_SHARED) - set(BUILD_SHARED_LIBS OFF) +if(libsecp256k1_IS_TOP_LEVEL) + option(BUILD_SHARED_LIBS "Build shared libraries." ON) endif() option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL}) diff --git a/src/secp256k1/CONTRIBUTING.md b/src/secp256k1/CONTRIBUTING.md index 80890fb7..f0011086 100644 --- a/src/secp256k1/CONTRIBUTING.md +++ b/src/secp256k1/CONTRIBUTING.md @@ -92,12 +92,14 @@ Run the tests: To create a report, `gcovr` is recommended, as it includes branch coverage reporting: - $ gcovr --exclude 'src/bench*' --print-summary + $ gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --print-summary To create a HTML report with coloured and annotated source code: $ mkdir -p coverage - $ gcovr --exclude 'src/bench*' --html --html-details -o coverage/coverage.html + $ gcovr --gcov-ignore-parse-errors=all --merge-mode-functions=separate --exclude 'src/bench*' --exclude 'src/modules/.*/bench_impl.h' --html --html-details -o coverage/coverage.html + +On `gcovr` >=8.3, `--gcov-ignore-parse-errors=all` can be replaced with `--gcov-suspicious-hits-threshold=140737488355330`. #### Exhaustive tests diff --git a/src/secp256k1/Makefile.am b/src/secp256k1/Makefile.am index d511853b..dc798575 100644 --- a/src/secp256k1/Makefile.am +++ b/src/secp256k1/Makefile.am @@ -45,7 +45,10 @@ noinst_HEADERS += src/precomputed_ecmult.h noinst_HEADERS += src/precomputed_ecmult_gen.h noinst_HEADERS += src/assumptions.h noinst_HEADERS += src/checkmem.h +noinst_HEADERS += src/tests_common.h noinst_HEADERS += src/testutil.h +noinst_HEADERS += src/unit_test.h +noinst_HEADERS += src/unit_test.c noinst_HEADERS += src/util.h noinst_HEADERS += src/util_local_visibility.h noinst_HEADERS += src/int128.h @@ -120,7 +123,7 @@ if USE_TESTS TESTS += noverify_tests noinst_PROGRAMS += noverify_tests noverify_tests_SOURCES = src/tests.c -noverify_tests_CPPFLAGS = $(SECP_CONFIG_DEFINES) +noverify_tests_CPPFLAGS = $(SECP_CONFIG_DEFINES) $(TEST_DEFINES) noverify_tests_LDADD = $(COMMON_LIB) $(PRECOMPUTED_LIB) noverify_tests_LDFLAGS = -static if !ENABLE_COVERAGE diff --git a/src/secp256k1/README.md b/src/secp256k1/README.md index 3d3118ad..90edae1a 100644 --- a/src/secp256k1/README.md +++ b/src/secp256k1/README.md @@ -135,13 +135,11 @@ To cross compile for Android with [NDK](https://developer.android.com/ndk/guides ### Building on Windows -To build on Windows with Visual Studio, a proper [generator](https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html#visual-studio-generators) must be specified for a new build tree. - -The following example assumes using of Visual Studio 2022 and CMake v3.21+. +The following example assumes Visual Studio 2022. Using clang-cl is recommended. In "Developer Command Prompt for VS 2022": - >cmake -G "Visual Studio 17 2022" -A x64 -B build + >cmake -B build -T ClangCL >cmake --build build --config RelWithDebInfo Usage examples diff --git a/src/secp256k1/build-aux/m4/bitcoin_secp.m4 b/src/secp256k1/build-aux/m4/bitcoin_secp.m4 index 048267fa..1428d4d9 100644 --- a/src/secp256k1/build-aux/m4/bitcoin_secp.m4 +++ b/src/secp256k1/build-aux/m4/bitcoin_secp.m4 @@ -3,7 +3,7 @@ AC_DEFUN([SECP_X86_64_ASM_CHECK],[ AC_MSG_CHECKING(for x86_64 assembly availability) AC_LINK_IFELSE([AC_LANG_PROGRAM([[ #include ]],[[ - uint64_t a = 11, tmp; + uint64_t a = 11, tmp = 0; __asm__ __volatile__("movq \@S|@0x100000000,%1; mulq %%rsi" : "+a"(a) : "S"(tmp) : "cc", "%rdx"); ]])], [has_x86_64_asm=yes], [has_x86_64_asm=no]) AC_MSG_RESULT([$has_x86_64_asm]) diff --git a/src/secp256k1/ci/ci.sh b/src/secp256k1/ci/ci.sh index 08e84efd..515c14cd 100755 --- a/src/secp256k1/ci/ci.sh +++ b/src/secp256k1/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/src/secp256k1/ci/linux-debian.Dockerfile b/src/secp256k1/ci/linux-debian.Dockerfile index 92ec80d2..a575d9b1 100644 --- a/src/secp256k1/ci/linux-debian.Dockerfile +++ b/src/secp256k1/ci/linux-debian.Dockerfile @@ -19,9 +19,9 @@ RUN dpkg --add-architecture i386 && \ dpkg --add-architecture arm64 && \ dpkg --add-architecture ppc64el -# dkpg-dev: to make pkg-config work in cross-builds +# dpkg-dev: to make pkg-config work in cross-builds # llvm: for llvm-symbolizer, which is used by clang's UBSan for symbolized stack traces -RUN apt-get update && apt-get install --no-install-recommends -y \ +RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ git ca-certificates \ make automake libtool pkg-config dpkg-dev valgrind qemu-user \ gcc clang llvm libclang-rt-dev libc6-dbg \ @@ -34,14 +34,15 @@ RUN apt-get update && apt-get install --no-install-recommends -y \ gcc-mingw-w64-i686-win32 wine32 \ python3-full && \ if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \ - apt-get install --no-install-recommends -y \ + DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\ fi && \ apt-get clean && rm -rf /var/lib/apt/lists/* # Build and install gcc snapshot ARG GCC_SNAPSHOT_MAJOR=16 -RUN apt-get update && apt-get install --no-install-recommends -y wget libgmp-dev libmpfr-dev libmpc-dev flex && \ +RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ + wget libgmp-dev libmpfr-dev libmpc-dev flex && \ mkdir gcc && cd gcc && \ wget --progress=dot:giga --https-only --recursive --accept '*.tar.xz' --level 1 --no-directories "https://gcc.gnu.org/pub/gcc/snapshots/LATEST-${GCC_SNAPSHOT_MAJOR}" && \ wget "https://gcc.gnu.org/pub/gcc/snapshots/LATEST-${GCC_SNAPSHOT_MAJOR}/sha512.sum" && \ @@ -62,7 +63,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y wget libgmp-dev # Install clang snapshot, see https://apt.llvm.org/ RUN \ # Setup GPG keys of LLVM repository - apt-get update && apt-get install --no-install-recommends -y wget && \ + apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y wget && \ wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc && \ # Add repository for this Debian release . /etc/os-release && echo "deb http://apt.llvm.org/${VERSION_CODENAME} llvm-toolchain-${VERSION_CODENAME} main" >> /etc/apt/sources.list && \ @@ -70,7 +71,7 @@ RUN \ # Determine the version number of the LLVM development branch LLVM_VERSION=$(apt-cache search --names-only '^clang-[0-9]+$' | sort -V | tail -1 | cut -f1 -d" " | cut -f2 -d"-" ) && \ # Install - apt-get install --no-install-recommends -y "clang-${LLVM_VERSION}" && \ + DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y "clang-${LLVM_VERSION}" "libclang-rt-${LLVM_VERSION}-dev" && \ # Create symlink ln -s "/usr/bin/clang-${LLVM_VERSION}" /usr/bin/clang-snapshot && \ # Clean up diff --git a/src/secp256k1/cmake/CheckX86_64Assembly.cmake b/src/secp256k1/cmake/CheckX86_64Assembly.cmake index ae82cd47..ca18919e 100644 --- a/src/secp256k1/cmake/CheckX86_64Assembly.cmake +++ b/src/secp256k1/cmake/CheckX86_64Assembly.cmake @@ -4,10 +4,11 @@ function(check_x86_64_assembly) check_c_source_compiles(" #include - int main() + int main(void) { - uint64_t a = 11, tmp; + uint64_t a = 11, tmp = 0; __asm__ __volatile__(\"movq $0x100000000,%1; mulq %%rsi\" : \"+a\"(a) : \"S\"(tmp) : \"cc\", \"%rdx\"); + return 0; } " HAVE_X86_64_ASM) set(HAVE_X86_64_ASM ${HAVE_X86_64_ASM} PARENT_SCOPE) diff --git a/src/secp256k1/configure.ac b/src/secp256k1/configure.ac index ce24248f..97ea2259 100644 --- a/src/secp256k1/configure.ac +++ b/src/secp256k1/configure.ac @@ -6,7 +6,7 @@ AC_PREREQ([2.60]) define(_PKG_VERSION_MAJOR, 0) define(_PKG_VERSION_MINOR, 7) define(_PKG_VERSION_PATCH, 1) -define(_PKG_VERSION_IS_RELEASE, false) +define(_PKG_VERSION_IS_RELEASE, true) # The library version is based on libtool versioning of the ABI. The set of # rules for updating the version can be found here: @@ -144,7 +144,7 @@ AC_ARG_ENABLE(benchmark, [SECP_SET_DEFAULT([enable_benchmark], [yes], [yes])]) AC_ARG_ENABLE(coverage, - AS_HELP_STRING([--enable-coverage],[enable compiler flags to support kcov coverage analysis [default=no]]), [], + AS_HELP_STRING([--enable-coverage],[enable coverage analysis support [default=no]]), [], [SECP_SET_DEFAULT([enable_coverage], [no], [no])]) AC_ARG_ENABLE(tests, @@ -443,6 +443,14 @@ if test x"$enable_experimental" = x"no"; then fi fi +# Check for concurrency support (tests only) +if test "x$enable_tests" != x"no"; then + AC_CHECK_HEADERS([sys/types.h sys/wait.h unistd.h]) + AS_IF([test "x$ac_cv_header_sys_types_h" = xyes && test "x$ac_cv_header_sys_wait_h" = xyes && + test "x$ac_cv_header_unistd_h" = xyes], [TEST_DEFINES="-DSUPPORTS_CONCURRENCY=1"], TEST_DEFINES="") + AC_SUBST(TEST_DEFINES) +fi + ### ### Generate output ### diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h index f55a6ab9..2799d4a1 100644 --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -261,7 +261,7 @@ SECP256K1_DEPRECATED("Use secp256k1_context_static instead"); * secp256k1_context_create (or secp256k1_context_preallocated_create), which will * take care of performing the self tests. * - * If the tests fail, this function will call the default error handler to abort the + * If the tests fail, this function will call the default error callback to abort the * program (see secp256k1_context_set_error_callback). */ SECP256K1_API void secp256k1_selftest(void); @@ -334,36 +334,38 @@ SECP256K1_API void secp256k1_context_destroy( * an API call. It will only trigger for violations that are mentioned * explicitly in the header. * - * The philosophy is that these shouldn't be dealt with through a - * specific return value, as calling code should not have branches to deal with - * the case that this code itself is broken. + * The philosophy is that these shouldn't be dealt with through a specific + * return value, as calling code should not have branches to deal with the case + * that this code itself is broken. * * On the other hand, during debug stage, one would want to be informed about - * such mistakes, and the default (crashing) may be inadvisable. - * When this callback is triggered, the API function called is guaranteed not - * to cause a crash, though its return value and output arguments are - * undefined. - * - * When this function has not been called (or called with fn==NULL), then the - * default handler will be used. The library provides a default handler which - * writes the message to stderr and calls abort. This default handler can be + * such mistakes, and the default (crashing) may be inadvisable. Should this + * callback return instead of crashing, the return value and output arguments + * of the API function call are undefined. Moreover, the same API call may + * trigger the callback again in this case. + * + * When this function has not been called (or called with fun==NULL), then the + * default callback will be used. The library provides a default callback which + * 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); - * The library can call these default handlers even before a proper callback data + * The library may call a default callback even before a proper callback data * pointer could have been set using secp256k1_context_set_illegal_callback or * secp256k1_context_set_error_callback, e.g., when the creation of a context - * fails. In this case, the corresponding default handler will be called with + * fails. In this case, the corresponding default callback will be called with * the data pointer argument set to NULL. * * Args: ctx: pointer to a context object. * In: fun: pointer to a function to call when an illegal argument is * passed to the API, taking a message and an opaque pointer. - * (NULL restores the default handler.) - * data: the opaque pointer to pass to fun above, must be NULL for the default handler. + * (NULL restores the default callback.) + * data: the opaque pointer to pass to fun above, must be NULL for the + * default callback. * * See also secp256k1_context_set_error_callback. */ @@ -380,8 +382,8 @@ SECP256K1_API void secp256k1_context_set_illegal_callback( * to abort the program. * * This can only trigger in case of a hardware failure, miscompilation, - * memory corruption, serious bug in the library, or other error would can - * otherwise result in undefined behaviour. It will not trigger due to mere + * memory corruption, serious bug in the library, or other error that would + * result in undefined behaviour. It will not trigger due to mere * incorrect usage of the API (see secp256k1_context_set_illegal_callback * for that). After this callback returns, anything may happen, including * crashing. @@ -389,9 +391,10 @@ SECP256K1_API void secp256k1_context_set_illegal_callback( * Args: ctx: pointer to a context object. * In: fun: pointer to a function to call when an internal error occurs, * taking a message and an opaque pointer (NULL restores the - * default handler, see secp256k1_context_set_illegal_callback + * default callback, see secp256k1_context_set_illegal_callback * for details). - * data: the opaque pointer to pass to fun above, must be NULL for the default handler. + * data: the opaque pointer to pass to fun above, must be NULL for the + * default callback. * * See also secp256k1_context_set_illegal_callback. */ diff --git a/src/secp256k1/include/secp256k1_ellswift.h b/src/secp256k1/include/secp256k1_ellswift.h index 0d1293e9..4cda5d5c 100644 --- a/src/secp256k1/include/secp256k1_ellswift.h +++ b/src/secp256k1/include/secp256k1_ellswift.h @@ -130,7 +130,7 @@ SECP256K1_API int secp256k1_ellswift_decode( * * Returns: 1: secret was valid, public key was stored. * 0: secret was invalid, try again. - * Args: ctx: pointer to a context object + * Args: ctx: pointer to a context object (not secp256k1_context_static) * Out: ell64: pointer to a 64-byte array to receive the ElligatorSwift * public key * In: seckey32: pointer to a 32-byte secret key diff --git a/src/secp256k1/include/secp256k1_recovery.h b/src/secp256k1/include/secp256k1_recovery.h index 93a2e4cc..2430f993 100644 --- a/src/secp256k1/include/secp256k1_recovery.h +++ b/src/secp256k1/include/secp256k1_recovery.h @@ -92,7 +92,17 @@ SECP256K1_API int secp256k1_ecdsa_sign_recoverable( /** Recover an ECDSA public key from a signature. * - * Returns: 1: public key successfully recovered (which guarantees a correct signature). + * Successful public key recovery guarantees that the signature, after normalization, + * passes `secp256k1_ecdsa_verify`. Thus, explicit verification is not necessary. + * + * However, a recoverable signature that successfully passes `secp256k1_ecdsa_recover`, + * when converted to a non-recoverable signature (using + * `secp256k1_ecdsa_recoverable_signature_convert`), is not guaranteed to be + * normalized and thus not guaranteed to pass `secp256k1_ecdsa_verify`. If a + * normalized signature is required, call `secp256k1_ecdsa_signature_normalize` + * after `secp256k1_ecdsa_recoverable_signature_convert`. + * + * Returns: 1: public key successfully recovered * 0: otherwise. * Args: ctx: pointer to a context object. * Out: pubkey: pointer to the recovered public key. diff --git a/src/secp256k1/sage/gen_split_lambda_constants.sage b/src/secp256k1/sage/gen_split_lambda_constants.sage index 7d4359e0..7a5761ac 100644 --- a/src/secp256k1/sage/gen_split_lambda_constants.sage +++ b/src/secp256k1/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/secp256k1/src/CMakeLists.txt b/src/secp256k1/src/CMakeLists.txt index fa3b2903..ecbbbbe8 100644 --- a/src/secp256k1/src/CMakeLists.txt +++ b/src/secp256k1/src/CMakeLists.txt @@ -134,15 +134,27 @@ if(SECP256K1_BUILD_BENCHMARK) endif() if(SECP256K1_BUILD_TESTS) + include(CheckIncludeFile) + check_include_file(sys/types.h HAVE_SYS_TYPES_H) + check_include_file(sys/wait.h HAVE_SYS_WAIT_H) + check_include_file(unistd.h HAVE_UNISTD_H) + + set(TEST_DEFINITIONS "") + if(HAVE_SYS_TYPES_H AND HAVE_SYS_WAIT_H AND HAVE_UNISTD_H) + list(APPEND TEST_DEFINITIONS SUPPORTS_CONCURRENCY=1) + endif() + add_executable(noverify_tests tests.c) target_link_libraries(noverify_tests secp256k1_precomputed secp256k1_asm) + target_compile_definitions(noverify_tests PRIVATE ${TEST_DEFINITIONS}) add_test(NAME secp256k1_noverify_tests COMMAND noverify_tests) if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage") add_executable(tests tests.c) - target_compile_definitions(tests PRIVATE VERIFY) + target_compile_definitions(tests PRIVATE VERIFY ${TEST_DEFINITIONS}) target_link_libraries(tests secp256k1_precomputed secp256k1_asm) add_test(NAME secp256k1_tests COMMAND tests) endif() + unset(TEST_DEFINITIONS) endif() if(SECP256K1_BUILD_EXHAUSTIVE_TESTS) diff --git a/src/secp256k1/src/bench.c b/src/secp256k1/src/bench.c index 8ba7623a..a5231b71 100644 --- a/src/secp256k1/src/bench.c +++ b/src/secp256k1/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/secp256k1/src/bench.h b/src/secp256k1/src/bench.h index 232fb35f..72a3f211 100644 --- a/src/secp256k1/src/bench.h +++ b/src/secp256k1/src/bench.h @@ -12,27 +12,7 @@ #include #include -#if (defined(_MSC_VER) && _MSC_VER >= 1900) -# include -#else -# include -#endif - -static int64_t gettime_i64(void) { -#if (defined(_MSC_VER) && _MSC_VER >= 1900) - /* C11 way to get wallclock time */ - struct timespec tv; - if (!timespec_get(&tv, TIME_UTC)) { - fputs("timespec_get failed!", stderr); - exit(EXIT_FAILURE); - } - return (int64_t)tv.tv_nsec / 1000 + (int64_t)tv.tv_sec * 1000000LL; -#else - struct timeval tv; - gettimeofday(&tv, NULL); - return (int64_t)tv.tv_usec + (int64_t)tv.tv_sec * 1000000LL; -#endif -} +#include "tests_common.h" #define FP_EXP (6) #define FP_MULT (1000000LL) @@ -170,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/secp256k1/src/bench_ecmult.c b/src/secp256k1/src/bench_ecmult.c index b2bab65d..bcf8b431 100644 --- a/src/secp256k1/src/bench_ecmult.c +++ b/src/secp256k1/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/secp256k1/src/bench_internal.c b/src/secp256k1/src/bench_internal.c index 8688a4dc..001bd25e 100644 --- a/src/secp256k1/src/bench_internal.c +++ b/src/secp256k1/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/secp256k1/src/checkmem.h b/src/secp256k1/src/checkmem.h index 7e333ce5..08eae47d 100644 --- a/src/secp256k1/src/checkmem.h +++ b/src/secp256k1/src/checkmem.h @@ -48,7 +48,17 @@ # if __has_feature(memory_sanitizer) # include # define SECP256K1_CHECKMEM_ENABLED 1 -# define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len)) +# if defined(__clang__) && ((__clang_major__ == 21 && __clang_minor__ >= 1) || __clang_major__ >= 22) +# define SECP256K1_CHECKMEM_UNDEFINE(p, len) do { \ + /* Work around https://github.com/llvm/llvm-project/issues/160094 */ \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wuninitialized-const-pointer\"") \ + __msan_allocated_memory((p), (len)); \ + _Pragma("clang diagnostic pop") \ + } while(0) +# else +# define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len)) +# endif # define SECP256K1_CHECKMEM_DEFINE(p, len) __msan_unpoison((p), (len)) # define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) __msan_unpoison((p), (len)) # define SECP256K1_CHECKMEM_CHECK(p, len) __msan_check_mem_is_initialized((p), (len)) diff --git a/src/secp256k1/src/eckey.h b/src/secp256k1/src/eckey.h index d54d44c9..c2bbc470 100644 --- a/src/secp256k1/src/eckey.h +++ b/src/secp256k1/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/secp256k1/src/eckey_impl.h b/src/secp256k1/src/eckey_impl.h index a88a5964..48745e8f 100644 --- a/src/secp256k1/src/eckey_impl.h +++ b/src/secp256k1/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/secp256k1/src/ecmult_const_impl.h b/src/secp256k1/src/ecmult_const_impl.h index 4f662091..1d24aeaf 100644 --- a/src/secp256k1/src/ecmult_const_impl.h +++ b/src/secp256k1/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/secp256k1/src/ecmult_gen_impl.h b/src/secp256k1/src/ecmult_gen_impl.h index 070a1213..2159eed5 100644 --- a/src/secp256k1/src/ecmult_gen_impl.h +++ b/src/secp256k1/src/ecmult_gen_impl.h @@ -213,7 +213,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 * but this would simply discard the bits that fall off at the bottom, * and thus, for example, bitdata could still have only two values if we * happen to shift by exactly 31 positions. We use a rotation instead, - * which ensures that bitdata doesn't loose entropy. This relies on the + * which ensures that bitdata doesn't lose entropy. This relies on the * rotation being atomic, i.e., the compiler emitting an actual rot * instruction. */ uint32_t bitdata = secp256k1_rotr32(recoded[bit_pos >> 5], bit_pos & 0x1f); @@ -242,7 +242,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 * (https://cryptojedi.org/peter/data/chesrump-20130822.pdf) and * "Cache Attacks and Countermeasures: the Case of AES", RSA 2006, * by Dag Arne Osvik, Adi Shamir, and Eran Tromer - * (https://www.tau.ac.il/~tromer/papers/cache.pdf) + * (https://eprint.iacr.org/2005/271.pdf) */ for (index = 0; index < COMB_POINTS; ++index) { secp256k1_ge_storage_cmov(&adds, &secp256k1_ecmult_gen_prec_table[block][index], index == abs); @@ -277,8 +277,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25 /* Cleanup. */ secp256k1_fe_clear(&neg); secp256k1_ge_clear(&add); - secp256k1_memclear(&adds, sizeof(adds)); - secp256k1_memclear(&recoded, sizeof(recoded)); + secp256k1_memclear_explicit(&adds, sizeof(adds)); + secp256k1_memclear_explicit(&recoded, sizeof(recoded)); } /* Setup blinding values for secp256k1_ecmult_gen. */ @@ -310,7 +310,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const VERIFY_CHECK(seed32 != NULL); memcpy(keydata + 32, seed32, 32); secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64); - secp256k1_memclear(keydata, sizeof(keydata)); + secp256k1_memclear_explicit(keydata, sizeof(keydata)); /* Compute projective blinding factor (cannot be 0). */ secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); @@ -331,7 +331,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const secp256k1_ge_set_gej(&ctx->ge_offset, &gb); /* Clean up. */ - secp256k1_memclear(nonce32, sizeof(nonce32)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&b); secp256k1_gej_clear(&gb); secp256k1_fe_clear(&f); diff --git a/src/secp256k1/src/ecmult_impl.h b/src/secp256k1/src/ecmult_impl.h index 0b53b3fc..c421750d 100644 --- a/src/secp256k1/src/ecmult_impl.h +++ b/src/secp256k1/src/ecmult_impl.h @@ -75,7 +75,7 @@ static void secp256k1_ecmult_odd_multiples_table(int n, secp256k1_ge *pre_a, sec secp256k1_ge d_ge; int 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) { @@ -341,7 +356,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); } } diff --git a/src/secp256k1/src/field.h b/src/secp256k1/src/field.h index 1f6ba746..e3f0234f 100644 --- a/src/secp256k1/src/field.h +++ b/src/secp256k1/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/secp256k1/src/field_10x26_impl.h b/src/secp256k1/src/field_10x26_impl.h index ea14c273..aa45434d 100644 --- a/src/secp256k1/src/field_10x26_impl.h +++ b/src/secp256k1/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/secp256k1/src/field_5x52_impl.h b/src/secp256k1/src/field_5x52_impl.h index 46dca6b9..3a976135 100644 --- a/src/secp256k1/src/field_5x52_impl.h +++ b/src/secp256k1/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/secp256k1/src/field_impl.h b/src/secp256k1/src/field_impl.h index 896507a3..7aa7de43 100644 --- a/src/secp256k1/src/field_impl.h +++ b/src/secp256k1/src/field_impl.h @@ -19,7 +19,7 @@ #endif SECP256K1_INLINE static void secp256k1_fe_clear(secp256k1_fe *a) { - secp256k1_memclear(a, sizeof(secp256k1_fe)); + secp256k1_memclear_explicit(a, sizeof(secp256k1_fe)); } SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { diff --git a/src/secp256k1/src/group.h b/src/secp256k1/src/group.h index 05ae0d20..ee3ebbbe 100644 --- a/src/secp256k1/src/group.h +++ b/src/secp256k1/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/secp256k1/src/group_impl.h b/src/secp256k1/src/group_impl.h index b8f2395d..f5169650 100644 --- a/src/secp256k1/src/group_impl.h +++ b/src/secp256k1/src/group_impl.h @@ -337,11 +337,11 @@ static void secp256k1_ge_set_infinity(secp256k1_ge *r) { } static void secp256k1_gej_clear(secp256k1_gej *r) { - secp256k1_memclear(r, sizeof(secp256k1_gej)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_gej)); } static void secp256k1_ge_clear(secp256k1_ge *r) { - secp256k1_memclear(r, sizeof(secp256k1_ge)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_ge)); } static int secp256k1_ge_set_xo_var(secp256k1_ge *r, const secp256k1_fe *x, int odd) { @@ -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/secp256k1/src/hash_impl.h b/src/secp256k1/src/hash_impl.h index 956e0ea4..43419177 100644 --- a/src/secp256k1/src/hash_impl.h +++ b/src/secp256k1/src/hash_impl.h @@ -172,7 +172,7 @@ static void secp256k1_sha256_initialize_tagged(secp256k1_sha256 *hash, const uns } static void secp256k1_sha256_clear(secp256k1_sha256 *hash) { - secp256k1_memclear(hash, sizeof(*hash)); + secp256k1_memclear_explicit(hash, sizeof(*hash)); } static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t keylen) { @@ -200,7 +200,7 @@ static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const rkey[n] ^= 0x5c ^ 0x36; } secp256k1_sha256_write(&hash->inner, rkey, sizeof(rkey)); - secp256k1_memclear(rkey, sizeof(rkey)); + secp256k1_memclear_explicit(rkey, sizeof(rkey)); } static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size) { @@ -211,12 +211,12 @@ static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned unsigned char temp[32]; secp256k1_sha256_finalize(&hash->inner, temp); secp256k1_sha256_write(&hash->outer, temp, 32); - secp256k1_memclear(temp, sizeof(temp)); + secp256k1_memclear_explicit(temp, sizeof(temp)); secp256k1_sha256_finalize(&hash->outer, out32); } static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash) { - secp256k1_memclear(hash, sizeof(*hash)); + secp256k1_memclear_explicit(hash, sizeof(*hash)); } static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen) { @@ -265,7 +265,7 @@ static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 while (outlen > 0) { secp256k1_hmac_sha256 hmac; - int now = outlen; + size_t now = outlen; secp256k1_hmac_sha256_initialize(&hmac, rng->k, 32); secp256k1_hmac_sha256_write(&hmac, rng->v, 32); secp256k1_hmac_sha256_finalize(&hmac, rng->v); @@ -285,7 +285,7 @@ static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 } static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng) { - secp256k1_memclear(rng, sizeof(*rng)); + secp256k1_memclear_explicit(rng, sizeof(*rng)); } #undef Round diff --git a/src/secp256k1/src/modules/ecdh/bench_impl.h b/src/secp256k1/src/modules/ecdh/bench_impl.h index c23aaa94..8924e1fa 100644 --- a/src/secp256k1/src/modules/ecdh/bench_impl.h +++ b/src/secp256k1/src/modules/ecdh/bench_impl.h @@ -10,7 +10,7 @@ #include "../../../include/secp256k1_ecdh.h" typedef struct { - secp256k1_context *ctx; + const secp256k1_context *ctx; secp256k1_pubkey point; unsigned char scalar[32]; } bench_ecdh_data; @@ -46,12 +46,9 @@ static void run_ecdh_bench(int iters, int argc, char** argv) { bench_ecdh_data data; int d = argc == 1; - /* create a context with no capabilities */ - data.ctx = secp256k1_context_create(SECP256K1_FLAGS_TYPE_CONTEXT); + data.ctx = secp256k1_context_static; if (d || have_flag(argc, argv, "ecdh")) run_benchmark("ecdh", bench_ecdh, bench_ecdh_setup, NULL, &data, 10, iters); - - secp256k1_context_destroy(data.ctx); } #endif /* SECP256K1_MODULE_ECDH_BENCH_H */ diff --git a/src/secp256k1/src/modules/ecdh/main_impl.h b/src/secp256k1/src/modules/ecdh/main_impl.h index 842b5359..9f2dfdd5 100644 --- a/src/secp256k1/src/modules/ecdh/main_impl.h +++ b/src/secp256k1/src/modules/ecdh/main_impl.h @@ -62,8 +62,8 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se ret = hashfp(output, x, y, data); - secp256k1_memclear(x, sizeof(x)); - secp256k1_memclear(y, sizeof(y)); + secp256k1_memclear_explicit(x, sizeof(x)); + secp256k1_memclear_explicit(y, sizeof(y)); secp256k1_scalar_clear(&s); secp256k1_ge_clear(&pt); secp256k1_gej_clear(&res); diff --git a/src/secp256k1/src/modules/ecdh/tests_impl.h b/src/secp256k1/src/modules/ecdh/tests_impl.h index 6888f18c..cb1d953d 100644 --- a/src/secp256k1/src/modules/ecdh/tests_impl.h +++ b/src/secp256k1/src/modules/ecdh/tests_impl.h @@ -7,6 +7,8 @@ #ifndef SECP256K1_MODULE_ECDH_TESTS_H #define SECP256K1_MODULE_ECDH_TESTS_H +#include "../../unit_test.h" + static int ecdh_hash_function_test_xpassthru(unsigned char *output, const unsigned char *x, const unsigned char *y, void *data) { (void)y; (void)data; @@ -90,12 +92,7 @@ static void test_ecdh_generator_basepoint(void) { static void test_bad_scalar(void) { unsigned char s_zero[32] = { 0 }; - unsigned char s_overflow[32] = { - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, - 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, - 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41 - }; + unsigned char s_overflow[32] = { 0 }; unsigned char s_rand[32] = { 0 }; unsigned char output[32]; secp256k1_scalar rand; @@ -107,6 +104,7 @@ static void test_bad_scalar(void) { CHECK(secp256k1_ec_pubkey_create(CTX, &point, s_rand) == 1); /* Try to multiply it by bad values */ + memcpy(s_overflow, secp256k1_group_order_bytes, 32); CHECK(secp256k1_ecdh(CTX, output, &point, s_zero, NULL, NULL) == 0); CHECK(secp256k1_ecdh(CTX, output, &point, s_overflow, NULL, NULL) == 0); /* ...and a good one */ @@ -182,12 +180,13 @@ static void test_ecdh_wycheproof(void) { } } -static void run_ecdh_tests(void) { - test_ecdh_api(); - test_ecdh_generator_basepoint(); - test_bad_scalar(); - test_result_basepoint(); - test_ecdh_wycheproof(); -} +/* --- Test registry --- */ +static const struct tf_test_entry tests_ecdh[] = { + CASE1(test_ecdh_api), + CASE1(test_ecdh_generator_basepoint), + CASE1(test_bad_scalar), + CASE1(test_result_basepoint), + CASE1(test_ecdh_wycheproof), +}; #endif /* SECP256K1_MODULE_ECDH_TESTS_H */ diff --git a/src/secp256k1/src/modules/ellswift/main_impl.h b/src/secp256k1/src/modules/ellswift/main_impl.h index 745a9691..096f4a3c 100644 --- a/src/secp256k1/src/modules/ellswift/main_impl.h +++ b/src/secp256k1/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); @@ -582,7 +575,7 @@ int secp256k1_ellswift_xdh(const secp256k1_context *ctx, unsigned char *output, /* Invoke hasher */ ret = hashfp(output, sx, ell_a64, ell_b64, data); - secp256k1_memclear(sx, sizeof(sx)); + secp256k1_memclear_explicit(sx, sizeof(sx)); secp256k1_fe_clear(&px); secp256k1_scalar_clear(&s); diff --git a/src/secp256k1/src/modules/ellswift/tests_impl.h b/src/secp256k1/src/modules/ellswift/tests_impl.h index 3c314c9b..4cc7f4b5 100644 --- a/src/secp256k1/src/modules/ellswift/tests_impl.h +++ b/src/secp256k1/src/modules/ellswift/tests_impl.h @@ -7,6 +7,7 @@ #define SECP256K1_MODULE_ELLSWIFT_TESTS_H #include "../../../include/secp256k1_ellswift.h" +#include "../../unit_test.h" struct ellswift_xswiftec_inv_test { int enc_bitmap; @@ -176,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; @@ -194,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; @@ -206,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]; @@ -222,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]; @@ -239,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; @@ -261,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; @@ -292,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]; @@ -402,35 +429,47 @@ void run_ellswift_tests(void) { CHECK(secp256k1_memcmp_var(share32_bad, share32b, 32) != 0); } } +} - /* Test hash initializers. */ - { - secp256k1_sha256 sha, sha_optimized; - 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'}; - 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'}; - 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_sha256_initialize_tagged(&sha, encode_tag, sizeof(encode_tag)); - secp256k1_ellswift_sha256_init_encode(&sha_optimized); - test_sha256_eq(&sha, &sha_optimized); + /* 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_sha256_initialize_tagged(&sha, create_tag, sizeof(create_tag)); - secp256k1_ellswift_sha256_init_create(&sha_optimized); - test_sha256_eq(&sha, &sha_optimized); + /* 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_sha256_initialize_tagged(&sha, bip324_tag, sizeof(bip324_tag)); - secp256k1_ellswift_sha256_init_bip324(&sha_optimized); - test_sha256_eq(&sha, &sha_optimized); - } + /* 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 --- */ +static const struct tf_test_entry tests_ellswift[] = { + 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/secp256k1/src/modules/extrakeys/tests_impl.h b/src/secp256k1/src/modules/extrakeys/tests_impl.h index ab4ef4a7..abebd110 100644 --- a/src/secp256k1/src/modules/extrakeys/tests_impl.h +++ b/src/secp256k1/src/modules/extrakeys/tests_impl.h @@ -8,6 +8,7 @@ #define SECP256K1_MODULE_EXTRAKEYS_TESTS_H #include "../../../include/secp256k1_extrakeys.h" +#include "../../unit_test.h" static void test_xonly_pubkey(void) { secp256k1_pubkey pk; @@ -467,17 +468,17 @@ static void test_keypair_add(void) { } } -static void run_extrakeys_tests(void) { +/* --- Test registry --- */ +static const struct tf_test_entry tests_extrakeys[] = { /* xonly key test cases */ - test_xonly_pubkey(); - test_xonly_pubkey_tweak(); - test_xonly_pubkey_tweak_check(); - test_xonly_pubkey_tweak_recursive(); - test_xonly_pubkey_comparison(); - + CASE1(test_xonly_pubkey), + CASE1(test_xonly_pubkey_tweak), + CASE1(test_xonly_pubkey_tweak_check), + CASE1(test_xonly_pubkey_tweak_recursive), + CASE1(test_xonly_pubkey_comparison), /* keypair tests */ - test_keypair(); - test_keypair_add(); -} + CASE1(test_keypair), + CASE1(test_keypair_add), +}; #endif diff --git a/src/secp256k1/src/modules/musig/keyagg_impl.h b/src/secp256k1/src/modules/musig/keyagg_impl.h index 0db4fce8..e412a27e 100644 --- a/src/secp256k1/src/modules/musig/keyagg_impl.h +++ b/src/secp256k1/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/secp256k1/src/modules/musig/session_impl.h b/src/secp256k1/src/modules/musig/session_impl.h index d8dcd00c..05c96310 100644 --- a/src/secp256k1/src/modules/musig/session_impl.h +++ b/src/secp256k1/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; } @@ -385,10 +372,10 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c secp256k1_scalar_set_b32(&k[i], buf, NULL); /* Attempt to erase secret data */ - secp256k1_memclear(buf, sizeof(buf)); + secp256k1_memclear_explicit(buf, sizeof(buf)); secp256k1_sha256_clear(&sha_tmp); } - secp256k1_memclear(rand, sizeof(rand)); + secp256k1_memclear_explicit(rand, sizeof(rand)); secp256k1_sha256_clear(&sha); } @@ -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])); @@ -518,7 +496,7 @@ int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_mu if (!secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, &pubkey, msg32, keyagg_cache, extra_input32)) { return 0; } - secp256k1_memclear(seckey, sizeof(seckey)); + secp256k1_memclear_explicit(seckey, sizeof(seckey)); return 1; } @@ -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; @@ -679,7 +662,7 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, secp256k1_musig_p ret = secp256k1_musig_secnonce_load(ctx, k, &pk, secnonce); /* Set nonce to zero to avoid nonce reuse. This will cause subsequent calls * of this function to fail */ - memset(secnonce, 0, sizeof(*secnonce)); + secp256k1_memzero_explicit(secnonce, sizeof(*secnonce)); if (!ret) { secp256k1_musig_partial_sign_clear(&sk, k); 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/secp256k1/src/modules/musig/tests_impl.h b/src/secp256k1/src/modules/musig/tests_impl.h index 4694c31a..de09c5e2 100644 --- a/src/secp256k1/src/modules/musig/tests_impl.h +++ b/src/secp256k1/src/modules/musig/tests_impl.h @@ -20,6 +20,7 @@ #include "../../group.h" #include "../../hash.h" #include "../../util.h" +#include "../../unit_test.h" #include "vectors.h" @@ -36,7 +37,7 @@ static int create_keypair_and_pk(secp256k1_keypair *keypair, secp256k1_pubkey *p /* Just a simple (non-tweaked) 2-of-2 MuSig aggregate, sign, verify * test. */ -static void musig_simple_test(void) { +static void musig_simple_test_internal(void) { unsigned char sk[2][32]; secp256k1_keypair keypair[2]; secp256k1_musig_pubnonce pubnonce[2]; @@ -200,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)); @@ -349,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)); @@ -473,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)); @@ -548,40 +570,39 @@ static void musig_nonce_test(void) { } } -static void sha256_tag_test_internal(secp256k1_sha256 *sha_tagged, unsigned char *tag, size_t taglen) { - secp256k1_sha256 sha; - secp256k1_sha256_initialize_tagged(&sha, tag, taglen); - test_sha256_eq(&sha, sha_tagged); -} - /* Checks that the initialized tagged hashes have the expected * state. */ static void sha256_tag_test(void) { secp256k1_sha256 sha; { - char tag[] = "KeyAgg list"; + /* "KeyAgg list" */ + static const unsigned char tag[] = {'K', 'e', 'y', 'A', 'g', 'g', ' ', 'l', 'i', 's', 't'}; secp256k1_musig_keyagglist_sha256(&sha); - sha256_tag_test_internal(&sha, (unsigned char*)tag, sizeof(tag) - 1); + test_sha256_tag_midstate(&sha, tag, sizeof(tag)); } { - char tag[] = "KeyAgg coefficient"; + /* "KeyAgg coefficient" */ + static const unsigned char tag[] = {'K', 'e', 'y', 'A', 'g', 'g', ' ', 'c', 'o', 'e', 'f', 'f', 'i', 'c', 'i', 'e', 'n', 't'}; secp256k1_musig_keyaggcoef_sha256(&sha); - sha256_tag_test_internal(&sha, (unsigned char*)tag, sizeof(tag) - 1); + test_sha256_tag_midstate(&sha, tag, sizeof(tag)); } { - unsigned char tag[] = "MuSig/aux"; + /* "MuSig/aux" */ + static const unsigned char tag[] = { 'M', 'u', 'S', 'i', 'g', '/', 'a', 'u', 'x' }; secp256k1_nonce_function_musig_sha256_tagged_aux(&sha); - sha256_tag_test_internal(&sha, (unsigned char*)tag, sizeof(tag) - 1); + test_sha256_tag_midstate(&sha, tag, sizeof(tag)); } { - unsigned char tag[] = "MuSig/nonce"; + /* "MuSig/nonce" */ + static const unsigned char tag[] = { 'M', 'u', 'S', 'i', 'g', '/', 'n', 'o', 'n', 'c', 'e' }; secp256k1_nonce_function_musig_sha256_tagged(&sha); - sha256_tag_test_internal(&sha, (unsigned char*)tag, sizeof(tag) - 1); + test_sha256_tag_midstate(&sha, tag, sizeof(tag)); } { - unsigned char tag[] = "MuSig/noncecoef"; + /* "MuSig/noncecoef" */ + static const unsigned char tag[] = { 'M', 'u', 'S', 'i', 'g', '/', 'n', 'o', 'n', 'c', 'e', 'c', 'o', 'e', 'f' }; secp256k1_musig_compute_noncehash_sha256_tagged(&sha); - sha256_tag_test_internal(&sha, (unsigned char*)tag, sizeof(tag) - 1); + test_sha256_tag_midstate(&sha, tag, sizeof(tag)); } } @@ -630,7 +651,7 @@ static void musig_tweak_test_helper(const secp256k1_xonly_pubkey* agg_pk, const /* Create aggregate public key P[0], tweak multiple times (using xonly and * plain tweaking) and test signing. */ -static void musig_tweak_test(void) { +static void musig_tweak_test_internal(void) { unsigned char sk[2][32]; secp256k1_pubkey pk[2]; const secp256k1_pubkey *pk_ptr[2]; @@ -1115,28 +1136,24 @@ static void musig_test_static_nonce_gen_counter(void) { CHECK(secp256k1_memcmp_var(pubnonce66, expected_pubnonce, sizeof(pubnonce66)) == 0); } -static void run_musig_tests(void) { - int i; - - for (i = 0; i < COUNT; i++) { - musig_simple_test(); - } - musig_api_tests(); - musig_nonce_test(); - for (i = 0; i < COUNT; i++) { - /* Run multiple times to ensure that pk and nonce have different y - * parities */ - musig_tweak_test(); - } - sha256_tag_test(); - musig_test_vectors_keyagg(); - musig_test_vectors_noncegen(); - musig_test_vectors_nonceagg(); - musig_test_vectors_signverify(); - musig_test_vectors_tweak(); - musig_test_vectors_sigagg(); - - musig_test_static_nonce_gen_counter(); -} +/* --- Test registry --- */ +REPEAT_TEST(musig_simple_test) +/* Run multiple times to ensure that pk and nonce have different y parities */ +REPEAT_TEST(musig_tweak_test) + +static const struct tf_test_entry tests_musig[] = { + CASE1(musig_simple_test), + CASE1(musig_api_tests), + CASE1(musig_nonce_test), + CASE1(musig_tweak_test), + CASE1(sha256_tag_test), + CASE1(musig_test_vectors_keyagg), + CASE1(musig_test_vectors_noncegen), + CASE1(musig_test_vectors_nonceagg), + CASE1(musig_test_vectors_signverify), + CASE1(musig_test_vectors_tweak), + CASE1(musig_test_vectors_sigagg), + CASE1(musig_test_static_nonce_gen_counter), +}; #endif diff --git a/src/secp256k1/src/modules/recovery/tests_impl.h b/src/secp256k1/src/modules/recovery/tests_impl.h index 7a28a3ce..09554a24 100644 --- a/src/secp256k1/src/modules/recovery/tests_impl.h +++ b/src/secp256k1/src/modules/recovery/tests_impl.h @@ -7,6 +7,8 @@ #ifndef SECP256K1_MODULE_RECOVERY_TESTS_H #define SECP256K1_MODULE_RECOVERY_TESTS_H +#include "../../unit_test.h" + static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *algo16, void *data, unsigned int counter) { (void) msg32; (void) key32; @@ -28,7 +30,7 @@ static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned c return testrand_bits(1); } -static void test_ecdsa_recovery_api(void) { +static void test_ecdsa_recovery_api_internal(void) { /* Setup contexts that just count errors */ secp256k1_pubkey pubkey; secp256k1_pubkey recpubkey; @@ -92,7 +94,7 @@ static void test_ecdsa_recovery_api(void) { CHECK(secp256k1_ecdsa_recoverable_signature_parse_compact(CTX, &recsig, sig, recid) == 0); } -static void test_ecdsa_recovery_end_to_end(void) { +static void test_ecdsa_recovery_end_to_end_internal(void) { unsigned char extra[32] = {0x00}; unsigned char privkey[32]; unsigned char message[32]; @@ -324,15 +326,14 @@ static void test_ecdsa_recovery_edge_cases(void) { } } -static void run_recovery_tests(void) { - int i; - for (i = 0; i < COUNT; i++) { - test_ecdsa_recovery_api(); - } - for (i = 0; i < 64*COUNT; i++) { - test_ecdsa_recovery_end_to_end(); - } - test_ecdsa_recovery_edge_cases(); -} +/* --- Test registry --- */ +REPEAT_TEST(test_ecdsa_recovery_api) +REPEAT_TEST_MULT(test_ecdsa_recovery_end_to_end, 64) + +static const struct tf_test_entry tests_recovery[] = { + CASE1(test_ecdsa_recovery_api), + CASE1(test_ecdsa_recovery_end_to_end), + CASE1(test_ecdsa_recovery_edge_cases) +}; #endif /* SECP256K1_MODULE_RECOVERY_TESTS_H */ diff --git a/src/secp256k1/src/modules/schnorrsig/main_impl.h b/src/secp256k1/src/modules/schnorrsig/main_impl.h index 2ed7be67..b410b19e 100644 --- a/src/secp256k1/src/modules/schnorrsig/main_impl.h +++ b/src/secp256k1/src/modules/schnorrsig/main_impl.h @@ -94,7 +94,7 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms secp256k1_sha256_write(&sha, msg, msglen); secp256k1_sha256_finalize(&sha, nonce32); secp256k1_sha256_clear(&sha); - secp256k1_memclear(masked_key, sizeof(masked_key)); + secp256k1_memclear_explicit(masked_key, sizeof(masked_key)); return 1; } @@ -139,7 +139,7 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi secp256k1_gej rj; secp256k1_ge pk; secp256k1_ge r; - unsigned char buf[32] = { 0 }; + unsigned char nonce32[32] = { 0 }; unsigned char pk_buf[32]; unsigned char seckey[32]; int ret = 1; @@ -164,8 +164,8 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi secp256k1_scalar_get_b32(seckey, &sk); secp256k1_fe_get_b32(pk_buf, &pk.x); - ret &= !!noncefp(buf, msg, msglen, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata); - secp256k1_scalar_set_b32(&k, buf, NULL); + ret &= !!noncefp(nonce32, msg, msglen, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata); + secp256k1_scalar_set_b32(&k, nonce32, NULL); ret &= !secp256k1_scalar_is_zero(&k); secp256k1_scalar_cmov(&k, &secp256k1_scalar_one, !ret); @@ -190,7 +190,8 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi secp256k1_memczero(sig64, 64, !ret); secp256k1_scalar_clear(&k); secp256k1_scalar_clear(&sk); - secp256k1_memclear(seckey, sizeof(seckey)); + secp256k1_memclear_explicit(seckey, sizeof(seckey)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_gej_clear(&rj); return ret; diff --git a/src/secp256k1/src/modules/schnorrsig/tests_impl.h b/src/secp256k1/src/modules/schnorrsig/tests_impl.h index 2d716a01..9a1b15f0 100644 --- a/src/secp256k1/src/modules/schnorrsig/tests_impl.h +++ b/src/secp256k1/src/modules/schnorrsig/tests_impl.h @@ -8,6 +8,7 @@ #define SECP256K1_MODULE_SCHNORRSIG_TESTS_H #include "../../../include/secp256k1_schnorrsig.h" +#include "../../unit_test.h" /* Checks that a bit flip in the n_flip-th argument (that has n_bytes many * bytes) changes the hash function @@ -21,11 +22,12 @@ static void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, s } static void run_nonce_function_bip340_tests(void) { - unsigned char tag[] = {'B', 'I', 'P', '0', '3', '4', '0', '/', 'n', 'o', 'n', 'c', 'e'}; - unsigned char aux_tag[] = {'B', 'I', 'P', '0', '3', '4', '0', '/', 'a', 'u', 'x'}; + /* "BIP0340/nonce" */ + static const unsigned char tag[] = {'B', 'I', 'P', '0', '3', '4', '0', '/', 'n', 'o', 'n', 'c', 'e'}; + /* "BIP0340/aux" */ + static const unsigned char aux_tag[] = {'B', 'I', 'P', '0', '3', '4', '0', '/', 'a', 'u', 'x'}; unsigned char algo[] = {'B', 'I', 'P', '0', '3', '4', '0', '/', 'n', 'o', 'n', 'c', 'e'}; size_t algolen = sizeof(algo); - secp256k1_sha256 sha; secp256k1_sha256 sha_optimized; unsigned char nonce[32], nonce_z[32]; unsigned char msg[32]; @@ -39,16 +41,15 @@ static void run_nonce_function_bip340_tests(void) { /* Check that hash initialized by * secp256k1_nonce_function_bip340_sha256_tagged has the expected * state. */ - secp256k1_sha256_initialize_tagged(&sha, tag, sizeof(tag)); secp256k1_nonce_function_bip340_sha256_tagged(&sha_optimized); - test_sha256_eq(&sha, &sha_optimized); + test_sha256_tag_midstate(&sha_optimized, tag, sizeof(tag)); + /* Check that hash initialized by * secp256k1_nonce_function_bip340_sha256_tagged_aux has the expected * state. */ - secp256k1_sha256_initialize_tagged(&sha, aux_tag, sizeof(aux_tag)); secp256k1_nonce_function_bip340_sha256_tagged_aux(&sha_optimized); - test_sha256_eq(&sha, &sha_optimized); + test_sha256_tag_midstate(&sha_optimized, aux_tag, sizeof(aux_tag)); testrand256(msg); testrand256(key); @@ -802,7 +803,7 @@ static int nonce_function_overflowing(unsigned char *nonce32, const unsigned cha return 1; } -static void test_schnorrsig_sign(void) { +static void test_schnorrsig_sign_internal(void) { unsigned char sk[32]; secp256k1_xonly_pubkey pk; secp256k1_keypair keypair; @@ -852,7 +853,7 @@ static void test_schnorrsig_sign(void) { /* Creates N_SIGS valid signatures and verifies them with verify and * verify_batch (TODO). Then flips some bits and checks that verification now * fails. */ -static void test_schnorrsig_sign_verify(void) { +static void test_schnorrsig_sign_verify_internal(void) { unsigned char sk[32]; unsigned char msg[N_SIGS][32]; unsigned char sig[N_SIGS][64]; @@ -965,18 +966,18 @@ static void test_schnorrsig_taproot(void) { CHECK(secp256k1_xonly_pubkey_tweak_add_check(CTX, output_pk_bytes, pk_parity, &internal_pk, tweak) == 1); } -static void run_schnorrsig_tests(void) { - int i; - run_nonce_function_bip340_tests(); - - test_schnorrsig_api(); - test_schnorrsig_sha256_tagged(); - test_schnorrsig_bip_vectors(); - for (i = 0; i < COUNT; i++) { - test_schnorrsig_sign(); - test_schnorrsig_sign_verify(); - } - test_schnorrsig_taproot(); -} +/* --- Test registry --- */ +REPEAT_TEST(test_schnorrsig_sign) +REPEAT_TEST(test_schnorrsig_sign_verify) + +static const struct tf_test_entry tests_schnorrsig[] = { + CASE(nonce_function_bip340_tests), + CASE1(test_schnorrsig_api), + CASE1(test_schnorrsig_sha256_tagged), + CASE1(test_schnorrsig_bip_vectors), + CASE1(test_schnorrsig_sign), + CASE1(test_schnorrsig_sign_verify), + CASE1(test_schnorrsig_taproot), +}; #endif diff --git a/src/secp256k1/src/scalar.h b/src/secp256k1/src/scalar.h index 70f49b1c..40d67191 100644 --- a/src/secp256k1/src/scalar.h +++ b/src/secp256k1/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/secp256k1/src/scalar_4x64_impl.h b/src/secp256k1/src/scalar_4x64_impl.h index 807b9b70..a5bf18fe 100644 --- a/src/secp256k1/src/scalar_4x64_impl.h +++ b/src/secp256k1/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/secp256k1/src/scalar_8x32_impl.h b/src/secp256k1/src/scalar_8x32_impl.h index 26104960..aa87b1d3 100644 --- a/src/secp256k1/src/scalar_8x32_impl.h +++ b/src/secp256k1/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/secp256k1/src/scalar_impl.h b/src/secp256k1/src/scalar_impl.h index 0232a8c2..9965c2ba 100644 --- a/src/secp256k1/src/scalar_impl.h +++ b/src/secp256k1/src/scalar_impl.h @@ -28,7 +28,7 @@ static const secp256k1_scalar secp256k1_scalar_one = SECP256K1_SCALAR_CONST(0, 0 static const secp256k1_scalar secp256k1_scalar_zero = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 0); SECP256K1_INLINE static void secp256k1_scalar_clear(secp256k1_scalar *r) { - secp256k1_memclear(r, sizeof(secp256k1_scalar)); + secp256k1_memclear_explicit(r, sizeof(secp256k1_scalar)); } static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin) { diff --git a/src/secp256k1/src/scalar_low_impl.h b/src/secp256k1/src/scalar_low_impl.h index 84e1a380..628bfd33 100644 --- a/src/secp256k1/src/scalar_low_impl.h +++ b/src/secp256k1/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/src/secp256k1.c b/src/secp256k1/src/secp256k1.c index 0915af77..ddd98495 100644 --- a/src/secp256k1/src/secp256k1.c +++ b/src/secp256k1/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) @@ -499,7 +507,7 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m } secp256k1_rfc6979_hmac_sha256_finalize(&rng); - secp256k1_memclear(keydata, sizeof(keydata)); + secp256k1_memclear_explicit(keydata, sizeof(keydata)); secp256k1_rfc6979_hmac_sha256_clear(&rng); return 1; } @@ -550,7 +558,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc * seckey. As a result is_sec_valid is included in ret only after ret was * used as a branching variable. */ ret &= is_sec_valid; - secp256k1_memclear(nonce32, sizeof(nonce32)); + secp256k1_memclear_explicit(nonce32, sizeof(nonce32)); secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); diff --git a/src/secp256k1/src/tests.c b/src/secp256k1/src/tests.c index 52614401..e09f5c7d 100644 --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -25,6 +25,8 @@ #include "checkmem.h" #include "testutil.h" #include "util.h" +#include "unit_test.h" +#include "unit_test.c" #include "../contrib/lax_der_parsing.c" #include "../contrib/lax_der_privatekey_parsing.c" @@ -37,7 +39,6 @@ #define CONDITIONAL_TEST(cnt, nam) if (COUNT < (cnt)) { printf("Skipping %s (iteration count too low)\n", nam); } else -static int COUNT = 16; static secp256k1_context *CTX = NULL; static secp256k1_context *STATIC_CTX = NULL; @@ -227,6 +228,12 @@ static void run_static_context_tests(int use_prealloc) { } } +static void run_all_static_context_tests(void) +{ + run_static_context_tests(0); + run_static_context_tests(1); +} + static void run_proper_context_tests(int use_prealloc) { int32_t dummy = 0; secp256k1_context *my_ctx, *my_ctx_fresh; @@ -349,6 +356,12 @@ static void run_proper_context_tests(int use_prealloc) { secp256k1_context_preallocated_destroy(NULL); } +static void run_all_proper_context_tests(void) +{ + run_proper_context_tests(0); + run_proper_context_tests(1); +} + static void run_scratch_tests(void) { const size_t adj_alloc = ((500 + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT; @@ -609,6 +622,13 @@ static void test_sha256_eq(const secp256k1_sha256 *sha1, const secp256k1_sha256 CHECK(sha1->bytes == sha2->bytes); CHECK(secp256k1_memcmp_var(sha1->s, sha2->s, sizeof(sha1->s)) == 0); } +/* Convenience function for using test_sha256_eq to verify the correctness of a + * tagged hash midstate. This function is used by some module tests. */ +static void test_sha256_tag_midstate(secp256k1_sha256 *sha_tagged, const unsigned char *tag, size_t taglen) { + secp256k1_sha256 sha; + secp256k1_sha256_initialize_tagged(&sha, tag, taglen); + test_sha256_eq(&sha, sha_tagged); +} static void run_hmac_sha256_tests(void) { static const char *keys[6] = { @@ -3904,7 +3924,7 @@ static void test_ge(void) { free(gej); } -static void test_intialized_inf(void) { +static void test_initialized_inf(void) { secp256k1_ge p; secp256k1_gej pj, npj, infj1, infj2, infj3; secp256k1_fe zinv; @@ -4030,7 +4050,7 @@ static void run_ge(void) { test_ge(); } test_add_neg_y_diff_x(); - test_intialized_inf(); + test_initialized_inf(); test_ge_bytes(); } @@ -4132,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)); @@ -4295,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; */ @@ -4306,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); @@ -5415,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); @@ -5436,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)); } } @@ -6029,12 +6042,7 @@ static void run_ec_pubkey_parse_test(void) { } static void run_eckey_edge_case_test(void) { - const unsigned char orderc[32] = { - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, - 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, - 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41 - }; + const unsigned char *orderc = secp256k1_group_order_bytes; const unsigned char zeros[sizeof(secp256k1_pubkey)] = {0x00}; unsigned char ctmp[33]; unsigned char ctmp2[33]; @@ -6044,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)); @@ -6237,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); @@ -6348,13 +6365,7 @@ static int nonce_function_test_retry(unsigned char *nonce32, const unsigned char return 1; } if (counter < 5) { - static const unsigned char order[] = { - 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, - 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFE, - 0xBA,0xAE,0xDC,0xE6,0xAF,0x48,0xA0,0x3B, - 0xBF,0xD2,0x5E,0x8C,0xD0,0x36,0x41,0x41 - }; - memcpy(nonce32, order, 32); + memcpy(nonce32, secp256k1_group_order_bytes, 32); if (counter == 4) { nonce32[31]++; } @@ -6534,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. */ @@ -6558,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); } } @@ -6636,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]; @@ -6644,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 */ @@ -7372,12 +7393,7 @@ static void test_ecdsa_edge_cases(void) { /* Privkey export where pubkey is the point at infinity. */ { unsigned char privkey[300]; - unsigned char seckey[32] = { - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, - 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, - 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41, - }; + const unsigned char *seckey = secp256k1_group_order_bytes; size_t outlen = 300; CHECK(!ec_privkey_export_der(CTX, privkey, &outlen, seckey, 0)); outlen = 300; @@ -7659,179 +7675,177 @@ static void run_cmov_tests(void) { ge_storage_cmov_test(); } -int main(int argc, char **argv) { - /* Disable buffering for stdout to improve reliability of getting - * diagnostic information. Happens right at the start of main because - * setbuf must be used before any other operation on the stream. */ - setbuf(stdout, NULL); - /* Also disable buffering for stderr because it's not guaranteed that it's - * unbuffered on all systems. */ - setbuf(stderr, NULL); - - /* find iteration count */ - if (argc > 1) { - COUNT = strtol(argv[1], NULL, 0); - } else { - const char* env = getenv("SECP256K1_TEST_ITERS"); - if (env && strlen(env) > 0) { - COUNT = strtol(env, NULL, 0); - } - } - if (COUNT <= 0) { - fputs("An iteration count of 0 or less is not allowed.\n", stderr); - return EXIT_FAILURE; - } - printf("test count = %i\n", COUNT); +/* --------------------------------------------------------- */ +/* Test Registry */ +/* --------------------------------------------------------- */ - /* run test RNG tests (must run before we really initialize the test RNG) */ - run_xoshiro256pp_tests(); +/* --- Special test cases that must run before RNG initialization --- */ +static const struct tf_test_entry tests_no_rng[] = { + CASE(xoshiro256pp_tests), +}; +static const struct tf_test_module registry_modules_no_rng = MAKE_TEST_MODULE(no_rng); + +/* --- Standard test cases start here --- */ +static const struct tf_test_entry tests_general[] = { + CASE(selftest_tests), + CASE(all_proper_context_tests), + CASE(all_static_context_tests), + CASE(deprecated_context_flags_test), + CASE(scratch_tests), +}; - /* find random seed */ - testrand_init(argc > 2 ? argv[2] : NULL); +static const struct tf_test_entry tests_integer[] = { +#ifdef SECP256K1_WIDEMUL_INT128 + CASE(int128_tests), +#endif + CASE(ctz_tests), + CASE(modinv_tests), + CASE(inverse_tests), +}; - /*** Setup test environment ***/ +static const struct tf_test_entry tests_hash[] = { + CASE(sha256_known_output_tests), + CASE(sha256_counter_tests), + CASE(hmac_sha256_tests), + CASE(rfc6979_hmac_sha256_tests), + CASE(tagged_sha256_tests), +}; - /* Create a global context available to all tests */ - CTX = secp256k1_context_create(SECP256K1_CONTEXT_NONE); - /* Randomize the context only with probability 15/16 - to make sure we test without context randomization from time to time. - TODO Reconsider this when recalibrating the tests. */ - if (testrand_bits(4)) { - unsigned char rand32[32]; - testrand256(rand32); - CHECK(secp256k1_context_randomize(CTX, rand32)); - } - /* Make a writable copy of secp256k1_context_static in order to test the effect of API functions - that write to the context. The API does not support cloning the static context, so we use - memcpy instead. The user is not supposed to copy a context but we should still ensure that - the API functions handle copies of the static context gracefully. */ - STATIC_CTX = malloc(sizeof(*secp256k1_context_static)); - CHECK(STATIC_CTX != NULL); - memcpy(STATIC_CTX, secp256k1_context_static, sizeof(secp256k1_context)); - CHECK(!secp256k1_context_is_proper(STATIC_CTX)); +static const struct tf_test_entry tests_scalar[] = { + CASE(scalar_tests), +}; - /*** Run actual tests ***/ +static const struct tf_test_entry tests_field[] = { + CASE(field_half), + CASE(field_misc), + CASE(field_convert), + CASE(field_be32_overflow), + CASE(fe_mul), + CASE(sqr), + CASE(sqrt), +}; - /* selftest tests */ - run_selftest_tests(); +static const struct tf_test_entry tests_group[] = { + CASE(ge), + CASE(gej), + CASE(group_decompress), +}; - /* context tests */ - run_proper_context_tests(0); run_proper_context_tests(1); - run_static_context_tests(0); run_static_context_tests(1); - run_deprecated_context_flags_test(); +static const struct tf_test_entry tests_ecmult[] = { + CASE(ecmult_pre_g), + CASE(wnaf), + CASE(point_times_order), + CASE(ecmult_near_split_bound), + CASE(ecmult_chain), + CASE(ecmult_constants), + CASE(ecmult_gen_blind), + CASE(ecmult_const_tests), + CASE(ecmult_multi_tests), + CASE(ec_combine), +}; - /* scratch tests */ - run_scratch_tests(); +static const struct tf_test_entry tests_ec[] = { + CASE(endomorphism_tests), + CASE(ec_pubkey_parse_test), + CASE(eckey_edge_case_test), + CASE(eckey_negate_test), +}; - /* integer arithmetic tests */ -#ifdef SECP256K1_WIDEMUL_INT128 - run_int128_tests(); -#endif - run_ctz_tests(); - run_modinv_tests(); - run_inverse_tests(); - - /* sorting tests */ - run_hsort_tests(); - - /* hash tests */ - run_sha256_known_output_tests(); - run_sha256_counter_tests(); - run_hmac_sha256_tests(); - run_rfc6979_hmac_sha256_tests(); - run_tagged_sha256_tests(); - - /* scalar tests */ - run_scalar_tests(); - - /* field tests */ - run_field_half(); - run_field_misc(); - run_field_convert(); - run_field_be32_overflow(); - run_fe_mul(); - run_sqr(); - run_sqrt(); - - /* group tests */ - run_ge(); - run_gej(); - run_group_decompress(); - - /* ecmult tests */ - run_ecmult_pre_g(); - run_wnaf(); - run_point_times_order(); - run_ecmult_near_split_bound(); - run_ecmult_chain(); - run_ecmult_constants(); - run_ecmult_gen_blind(); - run_ecmult_const_tests(); - run_ecmult_multi_tests(); - run_ec_combine(); - - /* endomorphism tests */ - run_endomorphism_tests(); - - /* EC point parser test */ - run_ec_pubkey_parse_test(); - - /* EC key edge cases */ - run_eckey_edge_case_test(); - - /* EC key arithmetic test */ - run_eckey_negate_test(); +static const struct tf_test_entry tests_ecdsa[] = { + CASE(ec_illegal_argument_tests), + CASE(pubkey_comparison), + CASE(pubkey_sort), + CASE(random_pubkeys), + CASE(ecdsa_der_parse), + CASE(ecdsa_sign_verify), + CASE(ecdsa_end_to_end), + CASE(ecdsa_edge_cases), + CASE(ecdsa_wycheproof), +}; + +static const struct tf_test_entry tests_utils[] = { + CASE(hsort_tests), + CASE(secp256k1_memczero_test), + CASE(secp256k1_is_zero_array_test), + CASE(secp256k1_byteorder_tests), + CASE(cmov_tests), +}; +/* Register test modules */ +static const struct tf_test_module registry_modules[] = { + MAKE_TEST_MODULE(general), + MAKE_TEST_MODULE(integer), + MAKE_TEST_MODULE(hash), + MAKE_TEST_MODULE(scalar), + MAKE_TEST_MODULE(field), + MAKE_TEST_MODULE(group), + MAKE_TEST_MODULE(ecmult), + MAKE_TEST_MODULE(ec), #ifdef ENABLE_MODULE_ECDH - /* ecdh tests */ - run_ecdh_tests(); + MAKE_TEST_MODULE(ecdh), #endif - - /* ecdsa tests */ - run_ec_illegal_argument_tests(); - run_pubkey_comparison(); - run_pubkey_sort(); - run_random_pubkeys(); - run_ecdsa_der_parse(); - run_ecdsa_sign_verify(); - run_ecdsa_end_to_end(); - run_ecdsa_edge_cases(); - run_ecdsa_wycheproof(); - + MAKE_TEST_MODULE(ecdsa), #ifdef ENABLE_MODULE_RECOVERY /* ECDSA pubkey recovery tests */ - run_recovery_tests(); + MAKE_TEST_MODULE(recovery), #endif - #ifdef ENABLE_MODULE_EXTRAKEYS - run_extrakeys_tests(); + MAKE_TEST_MODULE(extrakeys), #endif - #ifdef ENABLE_MODULE_SCHNORRSIG - run_schnorrsig_tests(); + MAKE_TEST_MODULE(schnorrsig), #endif - #ifdef ENABLE_MODULE_MUSIG - run_musig_tests(); + MAKE_TEST_MODULE(musig), #endif - #ifdef ENABLE_MODULE_ELLSWIFT - run_ellswift_tests(); + MAKE_TEST_MODULE(ellswift), #endif + MAKE_TEST_MODULE(utils), +}; - /* util tests */ - run_secp256k1_memczero_test(); - run_secp256k1_is_zero_array_test(); - run_secp256k1_byteorder_tests(); - - run_cmov_tests(); +/* Setup test environment */ +static int setup(void) { + /* Create a global context available to all tests */ + CTX = secp256k1_context_create(SECP256K1_CONTEXT_NONE); + /* Randomize the context only with probability 15/16 + to make sure we test without context randomization from time to time. + TODO Reconsider this when recalibrating the tests. */ + if (testrand_bits(4)) { + unsigned char rand32[32]; + testrand256(rand32); + CHECK(secp256k1_context_randomize(CTX, rand32)); + } + /* Make a writable copy of secp256k1_context_static in order to test the effect of API functions + that write to the context. The API does not support cloning the static context, so we use + memcpy instead. The user is not supposed to copy a context but we should still ensure that + the API functions handle copies of the static context gracefully. */ + STATIC_CTX = malloc(sizeof(*secp256k1_context_static)); + CHECK(STATIC_CTX != NULL); + memcpy(STATIC_CTX, secp256k1_context_static, sizeof(secp256k1_context)); + CHECK(!secp256k1_context_is_proper(STATIC_CTX)); + return 0; +} - /*** Tear down test environment ***/ +/* Shutdown test environment */ +static int teardown(void) { free(STATIC_CTX); secp256k1_context_destroy(CTX); + return 0; +} - testrand_finish(); +int main(int argc, char **argv) { + struct tf_framework tf = {0}; + tf.registry_modules = registry_modules; + tf.num_modules = sizeof(registry_modules) / sizeof(registry_modules[0]); + tf.registry_no_rng = ®istry_modules_no_rng; - printf("no problems found\n"); - return EXIT_SUCCESS; + /* Add context creation/destruction functions */ + tf.fn_setup = setup; + tf.fn_teardown = teardown; + + /* Init and run framework */ + if (tf_init(&tf, argc, argv) != 0) return EXIT_FAILURE; + return tf_run(&tf); } + diff --git a/src/secp256k1/src/tests_common.h b/src/secp256k1/src/tests_common.h new file mode 100644 index 00000000..a341633b --- /dev/null +++ b/src/secp256k1/src/tests_common.h @@ -0,0 +1,42 @@ +/*********************************************************************** + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#ifndef SECP256K1_TESTS_COMMON_H +#define SECP256K1_TESTS_COMMON_H + +/*********************************************************************** + * Test Support Utilities + * + * This file provides general-purpose functions for tests and benchmark + * programs. Unlike testutil.h, this file is not linked to the library, + * allowing each program to choose whether to run against the production + * API or access library internals directly. + ***********************************************************************/ + +#include + +#if (defined(_MSC_VER) && _MSC_VER >= 1900) +# include +#else +# include +#endif + +static int64_t gettime_i64(void) { +#if (defined(_MSC_VER) && _MSC_VER >= 1900) + /* C11 way to get wallclock time */ + struct timespec tv; + if (!timespec_get(&tv, TIME_UTC)) { + fputs("timespec_get failed!", stderr); + exit(EXIT_FAILURE); + } + return (int64_t)tv.tv_nsec / 1000 + (int64_t)tv.tv_sec * 1000000LL; +#else + struct timeval tv; + gettimeofday(&tv, NULL); + return (int64_t)tv.tv_usec + (int64_t)tv.tv_sec * 1000000LL; +#endif +} + +#endif /* SECP256K1_TESTS_COMMON_H */ diff --git a/src/secp256k1/src/tests_exhaustive.c b/src/secp256k1/src/tests_exhaustive.c index f8bbcaaf..13bda611 100644 --- a/src/secp256k1/src/tests_exhaustive.c +++ b/src/secp256k1/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/secp256k1/src/testutil.h b/src/secp256k1/src/testutil.h index 64b3bb41..93ee3d58 100644 --- a/src/secp256k1/src/testutil.h +++ b/src/secp256k1/src/testutil.h @@ -11,6 +11,14 @@ #include "testrand.h" #include "util.h" +/* group order of the secp256k1 curve in 32-byte big endian representation */ +static const unsigned char secp256k1_group_order_bytes[32] = { + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, + 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, + 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41 +}; + static void testutil_random_fe(secp256k1_fe *x) { unsigned char bin[32]; do { @@ -88,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/secp256k1/src/unit_test.c b/src/secp256k1/src/unit_test.c new file mode 100644 index 00000000..a1858a11 --- /dev/null +++ b/src/secp256k1/src/unit_test.c @@ -0,0 +1,479 @@ +/*********************************************************************** + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#include +#include +#include + +#if defined(SUPPORTS_CONCURRENCY) +#include +#include +#include +#endif + +#include "unit_test.h" +#include "testrand.h" +#include "tests_common.h" + +#define UNUSED(x) (void)(x) + +/* Number of times certain tests will run */ +int COUNT = 16; + +static int parse_jobs_count(const char* key, const char* value, struct tf_framework* tf); +static int parse_iterations(const char* key, const char* value, struct tf_framework* tf); +static int parse_seed(const char* key, const char* value, struct tf_framework* tf); +static int parse_target(const char* key, const char* value, struct tf_framework* tf); +static int parse_logging(const char* key, const char* value, struct tf_framework* tf); + +/* Mapping table: key -> handler */ +typedef int (*ArgHandler)(const char* key, const char* value, struct tf_framework* tf); +struct ArgMap { + const char* key; + ArgHandler handler; +}; + +/* + * Main entry point for handling command-line arguments. + * + * Developers should extend this map whenever new command-line + * options are introduced. Each new argument should be validated, + * converted to the appropriate type, and stored in 'tf->args' struct. + */ +static struct ArgMap arg_map[] = { + { "t", parse_target }, { "target", parse_target }, + { "j", parse_jobs_count }, { "jobs", parse_jobs_count }, + { "i", parse_iterations }, { "iterations", parse_iterations }, + { "seed", parse_seed }, + { "log", parse_logging }, + { NULL, NULL } /* sentinel */ +}; + +/* Display options that are not printed elsewhere */ +static void print_args(const struct tf_args* args) { + printf("iterations = %d\n", COUNT); + printf("jobs = %d. %s execution.\n", args->num_processes, args->num_processes > 1 ? "Parallel" : "Sequential"); +} + +/* Main entry point for reading environment variables */ +static int read_env(struct tf_framework* tf) { + const char* env_iter = getenv("SECP256K1_TEST_ITERS"); + if (env_iter && strlen(env_iter) > 0) { + return parse_iterations("i", env_iter, tf); + } + return 0; +} + +static int parse_arg(const char* key, const char* value, struct tf_framework* tf) { + int i; + for (i = 0; arg_map[i].key != NULL; i++) { + if (strcmp(key, arg_map[i].key) == 0) { + return arg_map[i].handler(key, value, tf); + } + } + /* Unknown key: report just so typos don't silently pass. */ + fprintf(stderr, "Unknown argument '-%s=%s'\n", key, value); + return -1; +} + +static void help(void) { + printf("Usage: ./tests [options]\n\n"); + printf("Run the test suite for the project with optional configuration.\n\n"); + printf("Options:\n"); + printf(" --help, -h Show this help message\n"); + printf(" --list_tests, -l Display list of all available tests and modules\n"); + printf(" --jobs=, -j= Number of parallel worker processes (default: 0 = sequential)\n"); + printf(" --iterations=, -i= Number of iterations for each test (default: 16)\n"); + printf(" --seed= Set a specific RNG seed (default: random)\n"); + printf(" --target=, -t= Run a specific test (can be provided multiple times)\n"); + printf(" --target=, -t= Run all tests within a specific module (can be provided multiple times)\n"); + printf(" --log=<0|1> Enable or disable test execution logging (default: 0 = disabled)\n"); + printf("\n"); + printf("Notes:\n"); + printf(" - All arguments must be provided in the form '--key=value', '-key=value' or '-k=value'.\n"); + printf(" - Single or double dashes are allowed for multi character options.\n"); + printf(" - Unknown arguments are reported but ignored.\n"); + printf(" - Sequential execution occurs if -jobs=0 or unspecified.\n"); + printf(" - Iterations and seed can also be passed as positional arguments before any other argument for backward compatibility.\n"); +} + +/* Print all tests in registry */ +static void print_test_list(struct tf_framework* tf) { + int m, t, total = 0; + printf("\nAvailable tests (%d modules):\n", tf->num_modules); + printf("========================================\n"); + for (m = 0; m < tf->num_modules; m++) { + const struct tf_test_module* mod = &tf->registry_modules[m]; + printf("Module: %s (%d tests)\n", mod->name, mod->size); + for (t = 0; t < mod->size; t++) { + printf("\t[%3d] %s\n", total + 1, mod->data[t].name); + total++; + } + printf("----------------------------------------\n"); + } + printf("\nRun specific module: ./tests -t=\n"); + printf("Run specific test: ./tests -t=\n\n"); +} + +static int parse_jobs_count(const char* key, const char* value, struct tf_framework* tf) { + char* ptr_val; + long val = strtol(value, &ptr_val, 10); /* base 10 */ + if (*ptr_val != '\0') { + fprintf(stderr, "Invalid number for -%s=%s\n", key, value); + return -1; + } + if (val < 0 || val > MAX_SUBPROCESSES) { + fprintf(stderr, "Arg '-%s' out of range: '%ld'. Range: 0..%d\n", key, val, MAX_SUBPROCESSES); + return -1; + } + tf->args.num_processes = (int) val; + return 0; +} + +static int parse_iterations(const char* key, const char* value, struct tf_framework* tf) { + UNUSED(key); UNUSED(tf); + if (!value) return 0; + COUNT = (int) strtol(value, NULL, 0); + if (COUNT <= 0) { + fputs("An iteration count of 0 or less is not allowed.\n", stderr); + return -1; + } + return 0; +} + +static int parse_seed(const char* key, const char* value, struct tf_framework* tf) { + UNUSED(key); + tf->args.custom_seed = (!value || strcmp(value, "NULL") == 0) ? NULL : value; + return 0; +} + +static int parse_logging(const char* key, const char* value, struct tf_framework* tf) { + UNUSED(key); + tf->args.logging = value && strcmp(value, "1") == 0; + return 0; +} + +/* Strip up to two leading dashes */ +static const char* normalize_key(const char* arg, const char** err_msg) { + const char* key; + if (!arg || arg[0] != '-') { + *err_msg = "missing initial dash"; + return NULL; + } + /* single-dash short option */ + if (arg[1] != '-') return arg + 1; + + /* double-dash checks now */ + if (arg[2] == '\0') { + *err_msg = "missing option name after double dash"; + return NULL; + } + + if (arg[2] == '-') { + *err_msg = "too many leading dashes"; + return NULL; + } + + key = arg + 2; + if (key[1] == '\0') { + *err_msg = "short option cannot use double dash"; + return NULL; + } + return key; +} + +static int parse_target(const char* key, const char* value, struct tf_framework* tf) { + int group, idx; + const struct tf_test_entry* entry; + UNUSED(key); + /* Find test index in the registry */ + for (group = 0; group < tf->num_modules; group++) { + const struct tf_test_module* module = &tf->registry_modules[group]; + int add_all = strcmp(value, module->name) == 0; /* select all from module */ + for (idx = 0; idx < module->size; idx++) { + entry = &module->data[idx]; + if (add_all || strcmp(value, entry->name) == 0) { + if (tf->args.targets.size >= MAX_ARGS) { + fprintf(stderr, "Too many -target args (max: %d)\n", MAX_ARGS); + return -1; + } + tf->args.targets.slots[tf->args.targets.size++] = entry; + /* Matched a single test, we're done */ + if (!add_all) return 0; + } + } + /* If add_all was true, we added all tests in the module, so return */ + if (add_all) return 0; + } + fprintf(stderr, "Error: target '%s' not found (missing or module disabled).\n" + "Run program with -list_tests option to display available tests and modules.\n", value); + return -1; +} + +/* Read args: all must be in the form -key=value, --key=value or -key=value */ +static int read_args(int argc, char** argv, int start, struct tf_framework* tf) { + int i; + const char* key; + const char* value; + char* eq; + const char* err_msg = "unknown error"; + for (i = start; i < argc; i++) { + char* raw_arg = argv[i]; + if (!raw_arg || raw_arg[0] != '-') { + fprintf(stderr, "Invalid arg '%s': must start with '-'\n", raw_arg ? raw_arg : "(null)"); + return -1; + } + + key = normalize_key(raw_arg, &err_msg); + if (!key || *key == '\0') { + fprintf(stderr, "Invalid arg '%s': %s. Must be -k=value or --key=value\n", raw_arg, err_msg); + return -1; + } + + eq = strchr(raw_arg, '='); + if (!eq || eq == raw_arg + 1) { + /* Allowed options without value */ + if (strcmp(key, "h") == 0 || strcmp(key, "help") == 0) { + tf->args.help = 1; + return 0; + } + if (strcmp(key, "l") == 0 || strcmp(key, "list_tests") == 0) { + tf->args.list_tests = 1; + return 0; + } + fprintf(stderr, "Invalid arg '%s': must be -k=value or --key=value\n", raw_arg); + return -1; + } + + *eq = '\0'; /* split key and value */ + value = eq + 1; + if (!value || *value == '\0') { /* value is empty */ + fprintf(stderr, "Invalid arg '%s': value cannot be empty\n", raw_arg); + return -1; + } + + if (parse_arg(key, value, tf) != 0) return -1; + } + return 0; +} + +static void run_test_log(const struct tf_test_entry* t) { + int64_t start_time = gettime_i64(); + printf("Running %s..\n", t->name); + t->func(); + printf("Test %s PASSED (%.3f sec)\n", t->name, (double)(gettime_i64() - start_time) / 1000000); +} + +static void run_test(const struct tf_test_entry* t) { t->func(); } + +/* Process tests in sequential order */ +static int run_sequential(struct tf_framework* tf) { + int it; + for (it = 0; it < tf->args.targets.size; it++) { + tf->fn_run_test(tf->args.targets.slots[it]); + } + return EXIT_SUCCESS; +} + +#if defined(SUPPORTS_CONCURRENCY) +static const int MAX_TARGETS = 255; + +/* Process tests in parallel */ +static int run_concurrent(struct tf_framework* tf) { + /* Sub-processes info */ + pid_t workers[MAX_SUBPROCESSES]; + int pipefd[2]; + int status = EXIT_SUCCESS; + int it; /* loop iterator */ + unsigned char idx; /* test index */ + + if (tf->args.targets.size > MAX_TARGETS) { + fprintf(stderr, "Internal Error: the number of targets (%d) exceeds the maximum supported (%d). " + "If you need more, extend 'run_concurrent()' to handle additional targets.\n", + tf->args.targets.size, MAX_TARGETS); + exit(EXIT_FAILURE); + } + + + if (pipe(pipefd) != 0) { + perror("Error during pipe setup"); + return EXIT_FAILURE; + } + + /* Launch worker processes */ + for (it = 0; it < tf->args.num_processes; it++) { + pid_t pid = fork(); + if (pid < 0) { + perror("Error during process fork"); + return EXIT_FAILURE; + } + if (pid == 0) { + /* Child worker: read jobs from the shared pipe */ + close(pipefd[1]); /* children never write */ + while (read(pipefd[0], &idx, sizeof(idx)) == sizeof(idx)) { + tf->fn_run_test(tf->args.targets.slots[(int)idx]); + } + _exit(EXIT_SUCCESS); /* finish child process */ + } else { + /* Parent: save worker pid */ + workers[it] = pid; + } + } + + /* Parent: write all tasks into the pipe */ + close(pipefd[0]); /* close read end */ + for (it = 0; it < tf->args.targets.size; it++) { + idx = (unsigned char)it; + if (write(pipefd[1], &idx, sizeof(idx)) == -1) { + perror("Error during workload distribution"); + close(pipefd[1]); + return EXIT_FAILURE; + } + } + /* Close write end to signal EOF */ + close(pipefd[1]); + /* Wait for all workers */ + for (it = 0; it < tf->args.num_processes; it++) { + int ret = 0; + if (waitpid(workers[it], &ret, 0) == -1 || ret != 0) { + status = EXIT_FAILURE; + } + } + + return status; +} +#endif + +static int tf_init(struct tf_framework* tf, int argc, char** argv) +{ + /* Caller must set the registry and its size before calling tf_init */ + if (tf->registry_modules == NULL || tf->num_modules <= 0) { + fprintf(stderr, "Error: tests registry not provided or empty\n"); + return EXIT_FAILURE; + } + + /* Initialize command-line options */ + tf->args.num_processes = 0; + tf->args.custom_seed = NULL; + tf->args.help = 0; + tf->args.targets.size = 0; + tf->args.list_tests = 0; + tf->args.logging = 0; + + /* Disable buffering for stdout to improve reliability of getting + * diagnostic information. Happens right at the start of main because + * setbuf must be used before any other operation on the stream. */ + setbuf(stdout, NULL); + /* Also disable buffering for stderr because it's not guaranteed that it's + * unbuffered on all systems. */ + setbuf(stderr, NULL); + + /* Parse env args */ + if (read_env(tf) != 0) return EXIT_FAILURE; + + /* Parse command-line args */ + if (argc > 1) { + int named_arg_start = 1; /* index to begin processing named arguments */ + if (argc - 1 > MAX_ARGS) { /* first arg is always the binary path */ + fprintf(stderr, "Too many command-line arguments (max: %d)\n", MAX_ARGS); + return EXIT_FAILURE; + } + + /* Compatibility Note: The first two args were the number of iterations and the seed. */ + /* If provided, parse them and adjust the starting index for named arguments accordingly. */ + if (argv[1][0] != '-') { + int has_seed = argc > 2 && argv[2][0] != '-'; + if (parse_iterations("i", argv[1], tf) != 0) return EXIT_FAILURE; + if (has_seed) parse_seed("seed", argv[2], tf); + named_arg_start = has_seed ? 3 : 2; + } + if (read_args(argc, argv, named_arg_start, tf) != 0) { + return EXIT_FAILURE; + } + + if (tf->args.help) { + help(); + exit(EXIT_SUCCESS); + } + + if (tf->args.list_tests) { + print_test_list(tf); + exit(EXIT_SUCCESS); + } + } + + tf->fn_run_test = tf->args.logging ? run_test_log : run_test; + return EXIT_SUCCESS; +} + +static int tf_run(struct tf_framework* tf) { + /* Process exit status */ + int status; + /* Whether to run all tests */ + int run_all; + /* Loop iterator */ + int it; + /* Initial test time */ + int64_t start_time = gettime_i64(); + /* Verify 'tf_init' has been called */ + if (!tf->fn_run_test) { + fprintf(stderr, "Error: No test runner set. You must call 'tf_init' first to initialize the framework " + "or manually assign 'fn_run_test' before calling 'tf_run'.\n"); + return EXIT_FAILURE; + } + + /* Populate targets with all tests if none were explicitly specified */ + run_all = tf->args.targets.size == 0; + if (run_all) { + int group, idx; + for (group = 0; group < tf->num_modules; group++) { + const struct tf_test_module* module = &tf->registry_modules[group]; + for (idx = 0; idx < module->size; idx++) { + if (tf->args.targets.size >= MAX_ARGS) { + fprintf(stderr, "Internal Error: Number of tests (%d) exceeds MAX_ARGS (%d). " + "Increase MAX_ARGS to accommodate all tests.\n", tf->args.targets.size, MAX_ARGS); + return EXIT_FAILURE; + } + tf->args.targets.slots[tf->args.targets.size++] = &module->data[idx]; + } + } + } + + if (!tf->args.logging) printf("Tests running silently. Use '-log=1' to enable detailed logging\n"); + + /* Log configuration */ + print_args(&tf->args); + + /* Run test RNG tests (must run before we really initialize the test RNG) */ + /* Note: currently, these tests are executed sequentially because there */ + /* is really only one test. */ + for (it = 0; tf->registry_no_rng && it < tf->registry_no_rng->size; it++) { + if (run_all) { /* future: support filtering */ + tf->fn_run_test(&tf->registry_no_rng->data[it]); + } + } + + /* Initialize test RNG and library contexts */ + testrand_init(tf->args.custom_seed); + if (tf->fn_setup && tf->fn_setup() != 0) return EXIT_FAILURE; + + /* Check whether to process tests sequentially or concurrently */ + if (tf->args.num_processes <= 1) { + status = run_sequential(tf); + } else { +#if defined(SUPPORTS_CONCURRENCY) + status = run_concurrent(tf); +#else + fputs("Parallel execution not supported on your system. Running sequentially...\n", stderr); + status = run_sequential(tf); +#endif + } + + /* Print accumulated time */ + printf("Total execution time: %.3f seconds\n", (double)(gettime_i64() - start_time) / 1000000); + if (tf->fn_teardown && tf->fn_teardown() != 0) return EXIT_FAILURE; + + return status; +} diff --git a/src/secp256k1/src/unit_test.h b/src/secp256k1/src/unit_test.h new file mode 100644 index 00000000..bf301e53 --- /dev/null +++ b/src/secp256k1/src/unit_test.h @@ -0,0 +1,145 @@ +/*********************************************************************** + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#ifndef SECP256K1_UNIT_TEST_H +#define SECP256K1_UNIT_TEST_H + +/* --------------------------------------------------------- */ +/* Configurable constants */ +/* --------------------------------------------------------- */ + +/* Maximum number of command-line arguments. + * Must be at least as large as the total number of tests + * to allow specifying all tests individually. */ +#define MAX_ARGS 150 +/* Maximum number of parallel jobs */ +#define MAX_SUBPROCESSES 16 + +/* --------------------------------------------------------- */ +/* Test Framework Registry Macros */ +/* --------------------------------------------------------- */ + +#define CASE(name) { #name, run_##name } +#define CASE1(name) { #name, name } + +#define MAKE_TEST_MODULE(name) { \ + #name, \ + tests_##name, \ + sizeof(tests_##name) / sizeof(tests_##name[0]) \ +} + +/* Macro to wrap a test internal function with a COUNT loop (iterations number) */ +#define REPEAT_TEST(fn) REPEAT_TEST_MULT(fn, 1) +#define REPEAT_TEST_MULT(fn, multiplier) \ + static void fn(void) { \ + int i; \ + int repeat = COUNT * (multiplier); \ + for (i = 0; i < repeat; i++) \ + fn##_internal(); \ + } + + + +/* --------------------------------------------------------- */ +/* Test Framework API */ +/* --------------------------------------------------------- */ + +typedef void (*test_fn)(void); + +struct tf_test_entry { + const char* name; + test_fn func; +}; + +struct tf_test_module { + const char* name; + const struct tf_test_entry* data; + int size; +}; + +typedef int (*setup_ctx_fn)(void); +typedef int (*teardown_fn)(void); +typedef void (*run_test_fn)(const struct tf_test_entry*); + +struct tf_targets { + /* Target tests indexes */ + const struct tf_test_entry* slots[MAX_ARGS]; + /* Next available slot */ + int size; +}; + +/* --- Command-line args --- */ +struct tf_args { + /* 0 => sequential; 1..MAX_SUBPROCESSES => parallel workers */ + int num_processes; + /* Specific RNG seed */ + const char* custom_seed; + /* Whether to print the help msg */ + int help; + /* Whether to print the tests list msg */ + int list_tests; + /* Target tests indexes */ + struct tf_targets targets; + /* Enable test execution logging */ + int logging; +}; + +/* --------------------------------------------------------- */ +/* Public API */ +/* --------------------------------------------------------- */ + +struct tf_framework { + /* Command-line args */ + struct tf_args args; + /* Test modules registry */ + const struct tf_test_module* registry_modules; + /* Num of modules */ + int num_modules; + /* Registry for tests that require no RNG init */ + const struct tf_test_module* registry_no_rng; + /* Specific context setup and teardown functions */ + setup_ctx_fn fn_setup; + teardown_fn fn_teardown; + /* Test runner function (can be customized) */ + run_test_fn fn_run_test; +}; + +/* + * Initialize the test framework. + * + * Must be called before tf_run() and before any output is performed to + * stdout or stderr, because this function disables buffering on both + * streams to ensure reliable diagnostic output. + * + * Parses command-line arguments and configures the framework context. + * The caller must initialize the following members of 'tf' before calling: + * - tf->registry_modules + * - tf->num_modules + * + * Side effects: + * - stdout and stderr are set to unbuffered mode via setbuf(). + * This allows immediate flushing of diagnostic messages but may + * affect performance for other output operations. + * + * Returns: + * EXIT_SUCCESS (0) on success, + * EXIT_FAILURE (non-zero) on error. + */ +static int tf_init(struct tf_framework* tf, int argc, char** argv); + +/* + * Run tests based on the provided test framework context. + * + * This function uses the configuration stored in the tf_framework + * (targets, number of processes, iteration count, etc.) to determine + * which tests to execute and how to execute them. + * + * Returns: + * EXIT_SUCCESS (0) if all tests passed, + * EXIT_FAILURE (non-zero) otherwise. + */ +static int tf_run(struct tf_framework* tf); + +#endif /* SECP256K1_UNIT_TEST_H */ diff --git a/src/secp256k1/src/util.h b/src/secp256k1/src/util.h index 5f29f407..46ab3810 100644 --- a/src/secp256k1/src/util.h +++ b/src/secp256k1/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++; @@ -219,8 +220,8 @@ static SECP256K1_INLINE void secp256k1_memczero(void *s, size_t len, int flag) { } } -/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. */ -static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) { +/* Zeroes memory to prevent leaking sensitive info. Won't be optimized out. */ +static SECP256K1_INLINE void secp256k1_memzero_explicit(void *ptr, size_t len) { #if defined(_MSC_VER) /* SecureZeroMemory is guaranteed not to be optimized out by MSVC. */ SecureZeroMemory(ptr, len); @@ -242,6 +243,19 @@ static SECP256K1_INLINE void secp256k1_memclear(void *ptr, size_t len) { void *(*volatile const volatile_memset)(void *, int, size_t) = memset; volatile_memset(ptr, 0, len); #endif +} + +/* Cleanses memory to prevent leaking sensitive info. Won't be optimized out. + * The state of the memory after this call is unspecified so callers must not + * make any assumptions about its contents. + * + * In VERIFY builds, it has the side effect of marking the memory as undefined. + * This helps to detect use-after-clear bugs where code incorrectly reads from + * cleansed memory during testing. + */ +static SECP256K1_INLINE void secp256k1_memclear_explicit(void *ptr, size_t len) { + /* The current implementation zeroes, but callers must not rely on this */ + secp256k1_memzero_explicit(ptr, len); #ifdef VERIFY SECP256K1_CHECKMEM_UNDEFINE(ptr, len); #endif @@ -277,11 +291,12 @@ static SECP256K1_INLINE int secp256k1_is_zero_array(const unsigned char *s, size } ret = (acc == 0); /* acc may contain secret values. Try to explicitly clear it. */ - secp256k1_memclear(&acc, sizeof(acc)); + secp256k1_memclear_explicit(&acc, sizeof(acc)); 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. @@ -289,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); diff --git a/src/secp256k1/src/wycheproof/WYCHEPROOF_COPYING b/src/secp256k1/src/wycheproof/WYCHEPROOF_COPYING index 26957043..c9a4ef81 100644 --- a/src/secp256k1/src/wycheproof/WYCHEPROOF_COPYING +++ b/src/secp256k1/src/wycheproof/WYCHEPROOF_COPYING @@ -1,7 +1,7 @@ * The file `ecdsa_secp256k1_sha256_bitcoin_test.json` in this directory comes from project Wycheproof with git commit - `df4e933efef449fc88af0c06e028d425d84a9495`, see - https://github.com/C2SP/wycheproof/blob/df4e933efef449fc88af0c06e028d425d84a9495/testvectors_v1/ecdsa_secp256k1_sha256_bitcoin_test.json + `7ae4532f417575ced2b1cbbabed81a7fecfaef5d`, see + https://github.com/C2SP/wycheproof/blob/7ae4532f417575ced2b1cbbabed81a7fecfaef5d/testvectors_v1/ecdsa_secp256k1_sha256_bitcoin_test.json * The file `ecdh_secp256k1_test.json` in this directory comes from project Wycheproof with git commit diff --git a/src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json b/src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json index aa0cc8a4..add468f5 100644 --- a/src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json +++ b/src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json @@ -5,7 +5,7 @@ "numberOfTests" : 463, "header" : [ "Test vectors of type EcdsaBitcoinVerify are meant for the verification", - "of a ECDSA variant used for bitcoin, that add signature non-malleability." + "of a ECDSA variant used for Bitcoin, that add signature non-malleability." ], "notes" : { "ArithmeticError" : { @@ -47,7 +47,7 @@ "InvalidSignature" : { "bugType" : "AUTH_BYPASS", "description" : "The signature contains special case values such as r=0 and s=0. Buggy implementations may accept such values, if the implementation does not check boundaries and computes s^(-1) == 0.", - "effect" : "Accepting such signatures can have the effect that an adversary can forge signatures without even knowning the message to sign.", + "effect" : "Accepting such signatures can have the effect that an adversary can forge signatures without even knowing the message to sign.", "cves" : [ "CVE-2022-21449", "CVE-2021-43572", @@ -57,7 +57,7 @@ "InvalidTypesInSignature" : { "bugType" : "AUTH_BYPASS", "description" : "The signature contains invalid types. Dynamic typed languages sometime coerce such values of different types into integers. If an implementation is careless and has additional bugs, such as not checking integer boundaries then it may be possible that such signatures are accepted.", - "effect" : "Accepting such signatures can have the effect that an adversary can forge signatures without even knowning the message to sign.", + "effect" : "Accepting such signatures can have the effect that an adversary can forge signatures without even knowing the message to sign.", "cves" : [ "CVE-2022-21449" ] @@ -95,8 +95,8 @@ }, "SignatureMalleabilityBitcoin" : { "bugType" : "SIGNATURE_MALLEABILITY", - "description" : "\"BitCoins\"-curves are curves where signature malleability can be a serious issue. An implementation should only accept a signature s where s < n/2. If an implementation is not meant for uses cases that require signature malleability then this implementation should be tested with another set of test vectors.", - "effect" : "In bitcoin exchanges, it may be used to make a double deposits or double withdrawals", + "description" : "Signature malleability can be a serious issue in Bitcoin. An implementation should only accept a signature s where s < n/2. If an implementation is meant for use cases that tolerate signature malleability then this implementation should not be tested with this set of test vectors.", + "effect" : "In Bitcoin exchanges, it may be used to make a double deposits or double withdrawals", "links" : [ "https://en.bitcoin.it/wiki/Transaction_malleability", "https://en.bitcoinwiki.org/wiki/Transaction_Malleability" diff --git a/src/secp256k1/tools/check-abi.sh b/src/secp256k1/tools/check-abi.sh index 601a64ba..a3ca67a6 100755 --- a/src/secp256k1/tools/check-abi.sh +++ b/src/secp256k1/tools/check-abi.sh @@ -49,14 +49,7 @@ checkout_and_build() { -DSECP256K1_BUILD_CTIME_TESTS=OFF \ -DSECP256K1_BUILD_EXAMPLES=OFF cmake --build . -j "$(nproc)" - # FIXME: Just set LIBPATH to lib/libsecp256k1.so once version 0.6.0 is - # released. - if [ -f "src/libsecp256k1.so" ]; then - LIBPATH="src/libsecp256k1.so" - else - LIBPATH="lib/libsecp256k1.so" - fi - abi-dumper $LIBPATH -o ABI.dump -lver "$2" -public-headers ../include/ + abi-dumper lib/libsecp256k1.so -o ABI.dump -lver "$2" -public-headers ../include/ cd "$_orig_dir" }