From 7d2591ce12d8a9b85f210cf9d678e91cee125ee9 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Wed, 17 Apr 2024 17:33:51 +0000 Subject: [PATCH] Add secp256k1_pubkey_sort Co-authored-by: Tim Ruffing Co-authored-by: Russell O'Connor --- CHANGELOG.md | 3 + Makefile.am | 2 + include/secp256k1.h | 14 +++ src/hsort.h | 33 +++++++ src/hsort_impl.h | 125 ++++++++++++++++++++++++ src/secp256k1.c | 29 ++++++ src/tests.c | 231 ++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 437 insertions(+) create mode 100644 src/hsort.h create mode 100644 src/hsort_impl.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 36f53db06a..98252da5a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +#### Added + - New function `secp256k1_ec_pubkey_sort` that sorts public keys using lexicographic (of compressed serialization) order. + #### Changed - The implementation of the point multiplication algorithm used for signing and public key generation was changed, resulting in improved performance for those operations. - The related configure option `--ecmult-gen-precision` was replaced with `--ecmult-gen-kb` (`ECMULT_GEN_KB` for CMake). diff --git a/Makefile.am b/Makefile.am index 5498617915..322e44eaab 100644 --- a/Makefile.am +++ b/Makefile.am @@ -64,6 +64,8 @@ noinst_HEADERS += src/field.h noinst_HEADERS += src/field_impl.h noinst_HEADERS += src/bench.h noinst_HEADERS += src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h +noinst_HEADERS += src/hsort.h +noinst_HEADERS += src/hsort_impl.h noinst_HEADERS += contrib/lax_der_parsing.h noinst_HEADERS += contrib/lax_der_parsing.c noinst_HEADERS += contrib/lax_der_privatekey_parsing.h diff --git a/include/secp256k1.h b/include/secp256k1.h index f4053f2a93..cfbdd528c2 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -474,6 +474,20 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_cmp( const secp256k1_pubkey *pubkey2 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); +/** Sort public keys using lexicographic (of compressed serialization) order + * + * Returns: 0 if the arguments are invalid. 1 otherwise. + * + * Args: ctx: pointer to a context object + * In: pubkeys: array of pointers to pubkeys to sort + * n_pubkeys: number of elements in the pubkeys array + */ +SECP256K1_API int secp256k1_ec_pubkey_sort( + const secp256k1_context *ctx, + const secp256k1_pubkey **pubkeys, + size_t n_pubkeys +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); + /** Parse an ECDSA signature in compact (64 bytes) format. * * Returns: 1 when the signature could be parsed, 0 otherwise. diff --git a/src/hsort.h b/src/hsort.h new file mode 100644 index 0000000000..d54995caad --- /dev/null +++ b/src/hsort.h @@ -0,0 +1,33 @@ +/*********************************************************************** + * Copyright (c) 2021 Russell O'Connor, Jonas Nick * + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#ifndef SECP256K1_HSORT_H +#define SECP256K1_HSORT_H + +#include +#include + +/* In-place, iterative heapsort with an interface matching glibc's qsort_r. This + * is preferred over standard library implementations because they generally + * make no guarantee about being fast for malicious inputs. + * Remember that heapsort is unstable. + * + * In/Out: ptr: pointer to the array to sort. The contents of the array are + * sorted in ascending order according to the comparison function. + * In: count: number of elements in the array. + * size: size in bytes of each element. + * cmp: pointer to a comparison function that is called with two + * arguments that point to the objects being compared. The cmp_data + * argument of secp256k1_hsort is passed as third argument. The + * function must return an integer less than, equal to, or greater + * than zero if the first argument is considered to be respectively + * less than, equal to, or greater than the second. + * cmp_data: pointer passed as third argument to cmp. + */ +static void secp256k1_hsort(void *ptr, size_t count, size_t size, + int (*cmp)(const void *, const void *, void *), + void *cmp_data); +#endif diff --git a/src/hsort_impl.h b/src/hsort_impl.h new file mode 100644 index 0000000000..1c674ff1c4 --- /dev/null +++ b/src/hsort_impl.h @@ -0,0 +1,125 @@ +/*********************************************************************** + * Copyright (c) 2021 Russell O'Connor, Jonas Nick * + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#ifndef SECP256K1_HSORT_IMPL_H +#define SECP256K1_HSORT_IMPL_H + +#include "hsort.h" + +/* An array is a heap when, for all non-zero indexes i, the element at index i + * compares as less than or equal to the element at index parent(i) = (i-1)/2. + */ + +static SECP256K1_INLINE size_t secp256k1_heap_child1(size_t i) { + VERIFY_CHECK(i <= (SIZE_MAX - 1)/2); + return 2*i + 1; +} + +static SECP256K1_INLINE size_t secp256k1_heap_child2(size_t i) { + VERIFY_CHECK(i <= SIZE_MAX/2 - 1); + return secp256k1_heap_child1(i)+1; +} + +static SECP256K1_INLINE void secp256k1_heap_swap64(unsigned char *a, unsigned char *b, size_t len) { + unsigned char tmp[64]; + VERIFY_CHECK(len <= 64); + memcpy(tmp, a, len); + memmove(a, b, len); + memcpy(b, tmp, len); +} + +static SECP256K1_INLINE void secp256k1_heap_swap(unsigned char *arr, size_t i, size_t j, size_t stride) { + unsigned char *a = arr + i*stride; + unsigned char *b = arr + j*stride; + size_t len = stride; + while (64 < len) { + secp256k1_heap_swap64(a + (len - 64), b + (len - 64), 64); + len -= 64; + } + secp256k1_heap_swap64(a, b, len); +} + +/* This function accepts an array arr containing heap_size elements, each of + * size stride. The elements in the array at indices >i satisfy the max-heap + * property, i.e., for any element at index j (where j > i), all of its children + * are smaller than the element itself. The purpose of the function is to update + * the array so that all elements at indices >=i satisfy the max-heap + * property. */ +static SECP256K1_INLINE void secp256k1_heap_down(unsigned char *arr, size_t i, size_t heap_size, size_t stride, + int (*cmp)(const void *, const void *, void *), void *cmp_data) { + while (i < heap_size/2) { + VERIFY_CHECK(i <= SIZE_MAX/2 - 1); + /* Proof: + * i < heap_size/2 + * i + 1 <= heap_size/2 + * 2*i + 2 <= heap_size <= SIZE_MAX + * 2*i <= SIZE_MAX - 2 + */ + + VERIFY_CHECK(secp256k1_heap_child1(i) < heap_size); + /* Proof: + * i < heap_size/2 + * i + 1 <= heap_size/2 + * 2*i + 2 <= heap_size + * 2*i + 1 < heap_size + * child1(i) < heap_size + */ + + /* Let [x] be notation for the contents at arr[x*stride]. + * + * If [child1(i)] > [i] and [child2(i)] > [i], + * swap [i] with the larger child to ensure the new parent is larger + * than both children. When [child1(i)] == [child2(i)], swap [i] with + * [child2(i)]. + * Else if [child1(i)] > [i], swap [i] with [child1(i)]. + * Else if [child2(i)] > [i], swap [i] with [child2(i)]. + */ + if (secp256k1_heap_child2(i) < heap_size + && 0 <= cmp(arr + secp256k1_heap_child2(i)*stride, arr + secp256k1_heap_child1(i)*stride, cmp_data)) { + if (0 < cmp(arr + secp256k1_heap_child2(i)*stride, arr + i*stride, cmp_data)) { + secp256k1_heap_swap(arr, i, secp256k1_heap_child2(i), stride); + i = secp256k1_heap_child2(i); + } else { + /* At this point we have [child2(i)] >= [child1(i)] and we have + * [child2(i)] <= [i], and thus [child1(i)] <= [i] which means + * that the next comparison can be skipped. */ + return; + } + } else if (0 < cmp(arr + secp256k1_heap_child1(i)*stride, arr + i*stride, cmp_data)) { + secp256k1_heap_swap(arr, i, secp256k1_heap_child1(i), stride); + i = secp256k1_heap_child1(i); + } else { + return; + } + } + /* heap_size/2 <= i + * heap_size/2 < i + 1 + * heap_size < 2*i + 2 + * heap_size <= 2*i + 1 + * heap_size <= child1(i) + * Thus child1(i) and child2(i) are now out of bounds and we are at a leaf. + */ +} + +/* In-place heap sort. */ +static void secp256k1_hsort(void *ptr, size_t count, size_t size, + int (*cmp)(const void *, const void *, void *), + void *cmp_data) { + size_t i; + + for (i = count/2; 0 < i; --i) { + secp256k1_heap_down(ptr, i-1, count, size, cmp, cmp_data); + } + for (i = count; 1 < i; --i) { + /* Extract the largest value from the heap */ + secp256k1_heap_swap(ptr, 0, i-1, size); + + /* Repair the heap condition */ + secp256k1_heap_down(ptr, 0, i-1, size, cmp, cmp_data); + } +} + +#endif diff --git a/src/secp256k1.c b/src/secp256k1.c index 15a5eede67..7a32769408 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -36,6 +36,7 @@ #include "int128_impl.h" #include "scratch_impl.h" #include "selftest.h" +#include "hsort_impl.h" #ifdef SECP256K1_NO_BUILD # error "secp256k1.h processed without SECP256K1_BUILD defined while building secp256k1.c" @@ -325,6 +326,34 @@ int secp256k1_ec_pubkey_cmp(const secp256k1_context* ctx, const secp256k1_pubkey return secp256k1_memcmp_var(out[0], out[1], sizeof(out[0])); } +static int secp256k1_ec_pubkey_sort_cmp(const void* pk1, const void* pk2, void *ctx) { + return secp256k1_ec_pubkey_cmp((secp256k1_context *)ctx, + *(secp256k1_pubkey **)pk1, + *(secp256k1_pubkey **)pk2); +} + +int secp256k1_ec_pubkey_sort(const secp256k1_context* ctx, const secp256k1_pubkey **pubkeys, size_t n_pubkeys) { + VERIFY_CHECK(ctx != NULL); + ARG_CHECK(pubkeys != NULL); + + /* Suppress wrong warning (fixed in MSVC 19.33) */ + #if defined(_MSC_VER) && (_MSC_VER < 1933) + #pragma warning(push) + #pragma warning(disable: 4090) + #endif + + /* Casting away const is fine because neither secp256k1_hsort nor + * secp256k1_ec_pubkey_sort_cmp modify the data pointed to by the cmp_data + * argument. */ + secp256k1_hsort(pubkeys, n_pubkeys, sizeof(*pubkeys), secp256k1_ec_pubkey_sort_cmp, (void *)ctx); + + #if defined(_MSC_VER) && (_MSC_VER < 1933) + #pragma warning(pop) + #endif + + return 1; +} + static void secp256k1_ecdsa_signature_load(const secp256k1_context* ctx, secp256k1_scalar* r, secp256k1_scalar* s, const secp256k1_ecdsa_signature* sig) { (void)ctx; if (sizeof(secp256k1_scalar) == 32) { diff --git a/src/tests.c b/src/tests.c index ec9cd0e03d..2eb3fbfdce 100644 --- a/src/tests.c +++ b/src/tests.c @@ -3676,6 +3676,78 @@ static void run_inverse_tests(void) } } +/***** HSORT TESTS *****/ + +static void test_heap_swap(void) { + unsigned char a[600]; + unsigned char e[sizeof(a)]; + memset(a, 21, 200); + memset(a + 200, 99, 200); + memset(a + 400, 42, 200); + memset(e, 42, 200); + memset(e + 200, 99, 200); + memset(e + 400, 21, 200); + secp256k1_heap_swap(a, 0, 2, 200); + CHECK(secp256k1_memcmp_var(a, e, sizeof(a)) == 0); +} + +static void test_hsort_is_sorted(unsigned char *elements, size_t n, size_t len) { + size_t i; + for (i = 1; i < n; i++) { + CHECK(secp256k1_memcmp_var(&elements[(i-1) * len], &elements[i * len], len) <= 0); + } +} + +struct test_hsort_cmp_data { + size_t counter; + size_t element_len; +}; + + +static int test_hsort_cmp(const void *ele1, const void *ele2, void *data) { + struct test_hsort_cmp_data *d = (struct test_hsort_cmp_data *) data; + d->counter += 1; + return secp256k1_memcmp_var((unsigned char *)ele1, (unsigned char *)ele2, d->element_len); +} + +#define NUM 65 +#define MAX_ELEMENT_LEN 65 +static void test_hsort(size_t element_len) { + unsigned char elements[NUM * MAX_ELEMENT_LEN] = { 0 }; + struct test_hsort_cmp_data data; + int i; + + VERIFY_CHECK(element_len <= MAX_ELEMENT_LEN); + data.counter = 0; + data.element_len = element_len; + + secp256k1_hsort(elements, 0, element_len, test_hsort_cmp, &data); + CHECK(data.counter == 0); + secp256k1_hsort(elements, 1, element_len, test_hsort_cmp, &data); + CHECK(data.counter == 0); + secp256k1_hsort(elements, NUM, element_len, test_hsort_cmp, &data); + CHECK(data.counter >= NUM - 1); + test_hsort_is_sorted(elements, NUM, element_len); + + /* Test hsort with array of random length n */ + for (i = 0; i < COUNT; i++) { + int n = secp256k1_testrand_int(NUM); + secp256k1_testrand_bytes_test(elements, n*element_len); + secp256k1_hsort(elements, n, element_len, test_hsort_cmp, &data); + test_hsort_is_sorted(elements, n, element_len); + } +} +#undef NUM +#undef MAX_ELEMENT_LEN + + +static void run_hsort_tests(void) { + test_heap_swap(); + test_hsort(1); + test_hsort(64); + test_hsort(65); +} + /***** GROUP TESTS *****/ /* This compares jacobian points including their Z, not just their geometric meaning. */ @@ -6629,6 +6701,161 @@ static void run_pubkey_comparison(void) { CHECK(secp256k1_ec_pubkey_cmp(CTX, &pk2, &pk1) > 0); } +static void test_sort_helper(secp256k1_pubkey *pk, size_t *pk_order, size_t n_pk) { + size_t i; + const secp256k1_pubkey *pk_test[5]; + + for (i = 0; i < n_pk; i++) { + pk_test[i] = &pk[pk_order[i]]; + } + secp256k1_ec_pubkey_sort(CTX, pk_test, n_pk); + for (i = 0; i < n_pk; i++) { + CHECK(secp256k1_memcmp_var(pk_test[i], &pk[i], sizeof(*pk_test[i])) == 0); + } +} + +static void permute(size_t *arr, size_t n) { + size_t i; + for (i = n - 1; i >= 1; i--) { + size_t tmp, j; + j = secp256k1_testrand_int(i + 1); + tmp = arr[i]; + arr[i] = arr[j]; + arr[j] = tmp; + } +} + +static void rand_pk(secp256k1_pubkey *pk) { + unsigned char seckey[32]; + secp256k1_keypair keypair; + secp256k1_testrand256(seckey); + CHECK(secp256k1_keypair_create(CTX, &keypair, seckey) == 1); + CHECK(secp256k1_keypair_pub(CTX, pk, &keypair) == 1); +} + +static void test_sort_api(void) { + secp256k1_pubkey pks[2]; + const secp256k1_pubkey *pks_ptr[2]; + + pks_ptr[0] = &pks[0]; + pks_ptr[1] = &pks[1]; + + rand_pk(&pks[0]); + rand_pk(&pks[1]); + + CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2) == 1); + CHECK_ILLEGAL(CTX, secp256k1_ec_pubkey_sort(CTX, NULL, 2)); + CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 0) == 1); + /* Test illegal public keys */ + memset(&pks[0], 0, sizeof(pks[0])); + CHECK_ILLEGAL_VOID(CTX, CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2) == 1)); + memset(&pks[1], 0, sizeof(pks[1])); + { + int32_t ecount = 0; + secp256k1_context_set_illegal_callback(CTX, counting_callback_fn, &ecount); + CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, 2) == 1); + CHECK(ecount == 2); + secp256k1_context_set_illegal_callback(CTX, NULL, NULL); + } +} + +static void test_sort(void) { + secp256k1_pubkey pk[5]; + unsigned char pk_ser[5][33] = { + { 0x02, 0x08 }, + { 0x02, 0x0b }, + { 0x02, 0x0c }, + { 0x03, 0x05 }, + { 0x03, 0x0a }, + }; + int i; + size_t pk_order[5] = { 0, 1, 2, 3, 4 }; + + for (i = 0; i < 5; i++) { + CHECK(secp256k1_ec_pubkey_parse(CTX, &pk[i], pk_ser[i], sizeof(pk_ser[i]))); + } + + permute(pk_order, 1); + test_sort_helper(pk, pk_order, 1); + permute(pk_order, 2); + test_sort_helper(pk, pk_order, 2); + permute(pk_order, 3); + test_sort_helper(pk, pk_order, 3); + for (i = 0; i < COUNT; i++) { + permute(pk_order, 4); + test_sort_helper(pk, pk_order, 4); + } + for (i = 0; i < COUNT; i++) { + permute(pk_order, 5); + test_sort_helper(pk, pk_order, 5); + } + /* Check that sorting also works for random pubkeys */ + for (i = 0; i < COUNT; i++) { + int j; + const secp256k1_pubkey *pk_ptr[5]; + for (j = 0; j < 5; j++) { + rand_pk(&pk[j]); + pk_ptr[j] = &pk[j]; + } + secp256k1_ec_pubkey_sort(CTX, pk_ptr, 5); + for (j = 1; j < 5; j++) { + CHECK(secp256k1_ec_pubkey_sort_cmp(&pk_ptr[j - 1], &pk_ptr[j], CTX) <= 0); + } + } +} + +/* Test vectors from BIP-MuSig2 */ +static void test_sort_vectors(void) { + enum { N_PUBKEYS = 6 }; + unsigned char pk_ser[N_PUBKEYS][33] = { + { 0x02, 0xDD, 0x30, 0x8A, 0xFE, 0xC5, 0x77, 0x7E, 0x13, 0x12, 0x1F, + 0xA7, 0x2B, 0x9C, 0xC1, 0xB7, 0xCC, 0x01, 0x39, 0x71, 0x53, 0x09, + 0xB0, 0x86, 0xC9, 0x60, 0xE1, 0x8F, 0xD9, 0x69, 0x77, 0x4E, 0xB8 }, + { 0x02, 0xF9, 0x30, 0x8A, 0x01, 0x92, 0x58, 0xC3, 0x10, 0x49, 0x34, + 0x4F, 0x85, 0xF8, 0x9D, 0x52, 0x29, 0xB5, 0x31, 0xC8, 0x45, 0x83, + 0x6F, 0x99, 0xB0, 0x86, 0x01, 0xF1, 0x13, 0xBC, 0xE0, 0x36, 0xF9 }, + { 0x03, 0xDF, 0xF1, 0xD7, 0x7F, 0x2A, 0x67, 0x1C, 0x5F, 0x36, 0x18, + 0x37, 0x26, 0xDB, 0x23, 0x41, 0xBE, 0x58, 0xFE, 0xAE, 0x1D, 0xA2, + 0xDE, 0xCE, 0xD8, 0x43, 0x24, 0x0F, 0x7B, 0x50, 0x2B, 0xA6, 0x59 }, + { 0x02, 0x35, 0x90, 0xA9, 0x4E, 0x76, 0x8F, 0x8E, 0x18, 0x15, 0xC2, + 0xF2, 0x4B, 0x4D, 0x80, 0xA8, 0xE3, 0x14, 0x93, 0x16, 0xC3, 0x51, + 0x8C, 0xE7, 0xB7, 0xAD, 0x33, 0x83, 0x68, 0xD0, 0x38, 0xCA, 0x66 }, + { 0x02, 0xDD, 0x30, 0x8A, 0xFE, 0xC5, 0x77, 0x7E, 0x13, 0x12, 0x1F, + 0xA7, 0x2B, 0x9C, 0xC1, 0xB7, 0xCC, 0x01, 0x39, 0x71, 0x53, 0x09, + 0xB0, 0x86, 0xC9, 0x60, 0xE1, 0x8F, 0xD9, 0x69, 0x77, 0x4E, 0xFF }, + { 0x02, 0xDD, 0x30, 0x8A, 0xFE, 0xC5, 0x77, 0x7E, 0x13, 0x12, 0x1F, + 0xA7, 0x2B, 0x9C, 0xC1, 0xB7, 0xCC, 0x01, 0x39, 0x71, 0x53, 0x09, + 0xB0, 0x86, 0xC9, 0x60, 0xE1, 0x8F, 0xD9, 0x69, 0x77, 0x4E, 0xB8 } + }; + secp256k1_pubkey pubkeys[N_PUBKEYS]; + secp256k1_pubkey *sorted[N_PUBKEYS]; + const secp256k1_pubkey *pks_ptr[N_PUBKEYS]; + int i; + + sorted[0] = &pubkeys[3]; + sorted[1] = &pubkeys[0]; + sorted[2] = &pubkeys[0]; + sorted[3] = &pubkeys[4]; + sorted[4] = &pubkeys[1]; + sorted[5] = &pubkeys[2]; + + for (i = 0; i < N_PUBKEYS; i++) { + CHECK(secp256k1_ec_pubkey_parse(CTX, &pubkeys[i], pk_ser[i], sizeof(pk_ser[i]))); + pks_ptr[i] = &pubkeys[i]; + } + CHECK(secp256k1_ec_pubkey_sort(CTX, pks_ptr, N_PUBKEYS) == 1); + for (i = 0; i < N_PUBKEYS; i++) { + CHECK(secp256k1_memcmp_var(pks_ptr[i], sorted[i], sizeof(secp256k1_pubkey)) == 0); + } +} + +static void run_pubkey_sort(void) { + test_sort_api(); + test_sort(); + test_sort_vectors(); +} + + static void run_random_pubkeys(void) { int i; for (i = 0; i < 10*COUNT; i++) { @@ -7588,6 +7815,9 @@ int main(int argc, char **argv) { run_modinv_tests(); run_inverse_tests(); + /* sorting tests */ + run_hsort_tests(); + /* hash tests */ run_sha256_known_output_tests(); run_sha256_counter_tests(); @@ -7644,6 +7874,7 @@ int main(int argc, char **argv) { /* ecdsa tests */ run_ec_illegal_argument_tests(); run_pubkey_comparison(); + run_pubkey_sort(); run_random_pubkeys(); run_ecdsa_der_parse(); run_ecdsa_sign_verify();