From 9570f674cc729cafcba65f4cce03552d9a6108f4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 23 Jan 2021 21:54:46 -0800 Subject: [PATCH 01/26] Avoid passing out-of-bound pointers to 0-size memcpy Doing so could be considered UB in a strict reading of the standard. Avoid it. --- contrib/lax_der_parsing.c | 4 ++-- contrib/lax_der_privatekey_parsing.c | 2 +- src/ecdsa_impl.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/lax_der_parsing.c b/contrib/lax_der_parsing.c index c1627e37e..90aed3407 100644 --- a/contrib/lax_der_parsing.c +++ b/contrib/lax_der_parsing.c @@ -121,7 +121,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_ /* Copy R value */ if (rlen > 32) { overflow = 1; - } else { + } else if (rlen) { memcpy(tmpsig + 32 - rlen, input + rpos, rlen); } @@ -133,7 +133,7 @@ int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_ /* Copy S value */ if (slen > 32) { overflow = 1; - } else { + } else if (slen) { memcpy(tmpsig + 64 - slen, input + spos, slen); } diff --git a/contrib/lax_der_privatekey_parsing.c b/contrib/lax_der_privatekey_parsing.c index 429760fbb..0653f8e4f 100644 --- a/contrib/lax_der_privatekey_parsing.c +++ b/contrib/lax_der_privatekey_parsing.c @@ -45,7 +45,7 @@ int ec_privkey_import_der(const secp256k1_context* ctx, unsigned char *out32, co if (end < privkey+2 || privkey[0] != 0x04 || privkey[1] > 0x20 || end < privkey+2+privkey[1]) { return 0; } - memcpy(out32 + 32 - privkey[1], privkey + 2, privkey[1]); + if (privkey[1]) memcpy(out32 + 32 - privkey[1], privkey + 2, privkey[1]); if (!secp256k1_ec_seckey_verify(ctx, out32)) { memset(out32, 0, 32); return 0; diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 156a33d11..c32141e88 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -140,7 +140,7 @@ static int secp256k1_der_parse_integer(secp256k1_scalar *r, const unsigned char overflow = 1; } if (!overflow) { - memcpy(ra + 32 - rlen, *sig, rlen); + if (rlen) memcpy(ra + 32 - rlen, *sig, rlen); secp256k1_scalar_set_b32(r, ra, &overflow); } if (overflow) { From 99e8614812bf23798a48c53649957e26e5b12f4a Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 30 Oct 2020 20:36:18 +0000 Subject: [PATCH 02/26] README: mention schnorrsig module --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a7eb2b0e8..182c29d9c 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ Features: * Suitable for embedded systems. * Optional module for public key recovery. * Optional module for ECDH key exchange. +* Optional module for Schnorr signatures according to [BIP-340](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki) (experimental). Experimental features have not received enough scrutiny to satisfy the standard of quality of this library but are made available for testing and review by the community. The APIs of these features should not be considered stable. From df3bfa12c3b728241d3e61d13f8c976719a3de41 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 30 Oct 2020 20:39:48 +0000 Subject: [PATCH 03/26] schnorrsig: clarify result of calling nonce_function_bip340 without data --- include/secp256k1_schnorrsig.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index 0150cd339..076e3b555 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -50,11 +50,11 @@ typedef int (*secp256k1_nonce_function_hardened)( * * If a data pointer is passed, it is assumed to be a pointer to 32 bytes of * auxiliary random data as defined in BIP-340. If the data pointer is NULL, - * schnorrsig_sign does not produce BIP-340 compliant signatures. The algo16 - * argument must be non-NULL, otherwise the function will fail and return 0. - * The hash will be tagged with algo16 after removing all terminating null - * bytes. Therefore, to create BIP-340 compliant signatures, algo16 must be set - * to "BIP0340/nonce\0\0\0" + * the nonce derivation procedure follows BIP-340 by setting the auxiliary + * random data to zero. The algo16 argument must be non-NULL, otherwise the + * function will fail and return 0. The hash will be tagged with algo16 after + * removing all terminating null bytes. Therefore, to create BIP-340 compliant + * signatures, algo16 must be set to "BIP0340/nonce\0\0\0" */ SECP256K1_API extern const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; From 442cee5bafbd7419acadf203ca11569e371f1f85 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 15 Jan 2021 21:43:23 +0000 Subject: [PATCH 04/26] schnorrsig: add algolen argument to nonce_function_hardened This avoids having to remove trailing NUL bytes in the nonce function --- include/secp256k1_schnorrsig.h | 28 ++++---- src/modules/schnorrsig/main_impl.h | 22 +++--- .../schnorrsig/tests_exhaustive_impl.h | 6 +- src/modules/schnorrsig/tests_impl.h | 70 +++++++++++-------- 4 files changed, 67 insertions(+), 59 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index 076e3b555..f5a3d1fd1 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -23,14 +23,15 @@ extern "C" { * * Returns: 1 if a nonce was successfully generated. 0 will cause signing to * return an error. - * Out: nonce32: pointer to a 32-byte array to be filled by the function. - * In: msg32: the 32-byte message hash being verified (will not be NULL) - * key32: pointer to a 32-byte secret key (will not be NULL) - * xonly_pk32: the 32-byte serialized xonly pubkey corresponding to key32 - * (will not be NULL) - * algo16: pointer to a 16-byte array describing the signature - * algorithm (will not be NULL). - * data: Arbitrary data pointer that is passed through. + * Out: nonce32: pointer to a 32-byte array to be filled by the function + * In: msg32: the 32-byte message hash being verified (will not be NULL) + * key32: pointer to a 32-byte secret key (will not be NULL) + * xonly_pk32: the 32-byte serialized xonly pubkey corresponding to key32 + * (will not be NULL) + * algo: pointer to an array describing the signature + * algorithm (will not be NULL) + * algolen: the length of the algo array + * data: arbitrary data pointer that is passed through * * Except for test cases, this function should compute some cryptographic hash of * the message, the key, the pubkey, the algorithm description, and data. @@ -40,7 +41,8 @@ typedef int (*secp256k1_nonce_function_hardened)( const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, - const unsigned char *algo16, + const unsigned char *algo, + size_t algolen, void *data ); @@ -51,10 +53,10 @@ typedef int (*secp256k1_nonce_function_hardened)( * If a data pointer is passed, it is assumed to be a pointer to 32 bytes of * auxiliary random data as defined in BIP-340. If the data pointer is NULL, * the nonce derivation procedure follows BIP-340 by setting the auxiliary - * random data to zero. The algo16 argument must be non-NULL, otherwise the - * function will fail and return 0. The hash will be tagged with algo16 after - * removing all terminating null bytes. Therefore, to create BIP-340 compliant - * signatures, algo16 must be set to "BIP0340/nonce\0\0\0" + * random data to zero. The algo argument must be non-NULL, otherwise the + * function will fail and return 0. The hash will be tagged with algo. + * Therefore, to create BIP-340 compliant signatures, algo must be set to + * "BIP0340/nonce" and algolen to 13. */ SECP256K1_API extern const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index af503bf5e..27f719b7e 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -43,16 +43,16 @@ static void secp256k1_nonce_function_bip340_sha256_tagged_aux(secp256k1_sha256 * sha->bytes = 64; } -/* algo16 argument for nonce_function_bip340 to derive the nonce exactly as stated in BIP-340 +/* algo argument for nonce_function_bip340 to derive the nonce exactly as stated in BIP-340 * by using the correct tagged hash function. */ -static const unsigned char bip340_algo16[16] = "BIP0340/nonce\0\0\0"; +static const unsigned char bip340_algo[13] = "BIP0340/nonce"; -static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo16, void *data) { +static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { secp256k1_sha256 sha; unsigned char masked_key[32]; int i; - if (algo16 == NULL) { + if (algo == NULL) { return 0; } @@ -65,18 +65,14 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms } } - /* Tag the hash with algo16 which is important to avoid nonce reuse across + /* Tag the hash with algo which is important to avoid nonce reuse across * algorithms. If this nonce function is used in BIP-340 signing as defined * in the spec, an optimized tagging implementation is used. */ - if (secp256k1_memcmp_var(algo16, bip340_algo16, 16) == 0) { + if (algolen == sizeof(bip340_algo) + && secp256k1_memcmp_var(algo, bip340_algo, algolen) == 0) { secp256k1_nonce_function_bip340_sha256_tagged(&sha); } else { - int algo16_len = 16; - /* Remove terminating null bytes */ - while (algo16_len > 0 && !algo16[algo16_len - 1]) { - algo16_len--; - } - secp256k1_sha256_initialize_tagged(&sha, algo16, algo16_len); + secp256k1_sha256_initialize_tagged(&sha, algo, algolen); } /* Hash (masked-)key||pk||msg using the tagged hash as per the spec */ @@ -156,7 +152,7 @@ int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64 secp256k1_scalar_get_b32(seckey, &sk); secp256k1_fe_get_b32(pk_buf, &pk.x); - ret &= !!noncefp(buf, msg32, seckey, pk_buf, bip340_algo16, ndata); + ret &= !!noncefp(buf, msg32, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata); secp256k1_scalar_set_b32(&k, buf, NULL); ret &= !secp256k1_scalar_is_zero(&k); secp256k1_scalar_cmov(&k, &secp256k1_scalar_one, !ret); diff --git a/src/modules/schnorrsig/tests_exhaustive_impl.h b/src/modules/schnorrsig/tests_exhaustive_impl.h index affabd23a..3359bf180 100644 --- a/src/modules/schnorrsig/tests_exhaustive_impl.h +++ b/src/modules/schnorrsig/tests_exhaustive_impl.h @@ -60,13 +60,15 @@ static const unsigned char invalid_pubkey_bytes[][32] = { static int secp256k1_hardened_nonce_function_smallint(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, - const unsigned char *algo16, void* data) { + const unsigned char *algo, size_t algolen, + void* data) { secp256k1_scalar s; int *idata = data; (void)msg32; (void)key32; (void)xonly_pk32; - (void)algo16; + (void)algo; + (void)algolen; secp256k1_scalar_set_int(&s, *idata); secp256k1_scalar_get_b32(nonce32, &s); return 1; diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index f4fa5b4d8..a308ccf4d 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -12,11 +12,11 @@ /* Checks that a bit flip in the n_flip-th argument (that has n_bytes many * bytes) changes the hash function */ -void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, size_t n_bytes) { +void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, size_t n_bytes, size_t algolen) { unsigned char nonces[2][32]; - CHECK(nonce_function_bip340(nonces[0], args[0], args[1], args[2], args[3], args[4]) == 1); + CHECK(nonce_function_bip340(nonces[0], args[0], args[1], args[2], args[3], algolen, args[4]) == 1); secp256k1_testrand_flip(args[n_flip], n_bytes); - CHECK(nonce_function_bip340(nonces[1], args[0], args[1], args[2], args[3], args[4]) == 1); + CHECK(nonce_function_bip340(nonces[1], args[0], args[1], args[2], args[3], algolen, args[4]) == 1); CHECK(secp256k1_memcmp_var(nonces[0], nonces[1], 32) != 0); } @@ -34,7 +34,8 @@ void test_sha256_eq(const secp256k1_sha256 *sha1, const secp256k1_sha256 *sha2) void run_nonce_function_bip340_tests(void) { unsigned char tag[13] = "BIP0340/nonce"; unsigned char aux_tag[11] = "BIP0340/aux"; - unsigned char algo16[16] = "BIP0340/nonce\0\0\0"; + unsigned char algo[13] = "BIP0340/nonce"; + size_t algolen = sizeof(algo); secp256k1_sha256 sha; secp256k1_sha256 sha_optimized; unsigned char nonce[32]; @@ -68,33 +69,37 @@ void run_nonce_function_bip340_tests(void) { args[0] = msg; args[1] = key; args[2] = pk; - args[3] = algo16; + args[3] = algo; args[4] = aux_rand; for (i = 0; i < count; i++) { - nonce_function_bip340_bitflip(args, 0, 32); - nonce_function_bip340_bitflip(args, 1, 32); - nonce_function_bip340_bitflip(args, 2, 32); - /* Flip algo16 special case "BIP0340/nonce" */ - nonce_function_bip340_bitflip(args, 3, 16); - /* Flip algo16 again */ - nonce_function_bip340_bitflip(args, 3, 16); - nonce_function_bip340_bitflip(args, 4, 32); + nonce_function_bip340_bitflip(args, 0, 32, algolen); + nonce_function_bip340_bitflip(args, 1, 32, algolen); + nonce_function_bip340_bitflip(args, 2, 32, algolen); + /* Flip algo special case "BIP0340/nonce" */ + nonce_function_bip340_bitflip(args, 3, algolen, algolen); + /* Flip algo again */ + nonce_function_bip340_bitflip(args, 3, algolen, algolen); + nonce_function_bip340_bitflip(args, 4, 32, algolen); } - /* NULL algo16 is disallowed */ - CHECK(nonce_function_bip340(nonce, msg, key, pk, NULL, NULL) == 0); - /* Empty algo16 is fine */ - memset(algo16, 0x00, 16); - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo16, NULL) == 1); - /* algo16 with terminating null bytes is fine */ - algo16[1] = 65; - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo16, NULL) == 1); - /* Other algo16 is fine */ - memset(algo16, 0xFF, 16); - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo16, NULL) == 1); + /* NULL algo is disallowed */ + CHECK(nonce_function_bip340(nonce, msg, key, pk, NULL, 0, NULL) == 0); + CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); + /* Other algo is fine */ + secp256k1_rfc6979_hmac_sha256_generate(&secp256k1_test_rng, algo, algolen); + CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); + + for (i = 0; i < count; i++) { + unsigned char nonce2[32]; + /* Different algolen gives different nonce */ + uint32_t offset = secp256k1_testrand_int(algolen - 1); + size_t algolen_tmp = (algolen + offset) % algolen; + CHECK(nonce_function_bip340(nonce2, msg, key, pk, algo, algolen_tmp, NULL) == 1); + CHECK(secp256k1_memcmp_var(nonce, nonce2, 32) != 0); + } /* NULL aux_rand argument is allowed. */ - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo16, NULL) == 1); + CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); } void test_schnorrsig_api(void) { @@ -634,22 +639,24 @@ void test_schnorrsig_bip_vectors(void) { } /* Nonce function that returns constant 0 */ -static int nonce_function_failing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo16, void *data) { +static int nonce_function_failing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { (void) msg32; (void) key32; (void) xonly_pk32; - (void) algo16; + (void) algo; + (void) algolen; (void) data; (void) nonce32; return 0; } /* Nonce function that sets nonce to 0 */ -static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo16, void *data) { +static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { (void) msg32; (void) key32; (void) xonly_pk32; - (void) algo16; + (void) algo; + (void) algolen; (void) data; memset(nonce32, 0, 32); @@ -657,11 +664,12 @@ static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg32, } /* Nonce function that sets nonce to 0xFF...0xFF */ -static int nonce_function_overflowing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo16, void *data) { +static int nonce_function_overflowing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { (void) msg32; (void) key32; (void) xonly_pk32; - (void) algo16; + (void) algo; + (void) algolen; (void) data; memset(nonce32, 0xFF, 32); From a4642fa15ee731b0a620a3f089826d556e5405f0 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 15 Jun 2021 19:33:57 +0300 Subject: [PATCH 05/26] configure: replace AC_PATH_PROG to AC_CHECK_PROG Bitcoin Core's `configure` script uses `AC_CHECK_PROG` to find brew in the `PATH` [1]. If found, this macro will set `BREW=brew`. When building with dependencies however the `BREW` variable is set to `no` on macOS via `depends//share/config.site` [2] and this overrides `AC_CHECK_PROG` results [3]. Ideally, secp256k1's `configure` script should follow the same logic but this is not what happens because secp256k1's `configure` uses `AC_PATH_PROG` instead which respects preset variable values (in this case for variable `BREW`) only if they are a valid path (i.e., they match `[\\/*] | ?:[\\/]*` [4]), and `no` is not a path. This commit changes `AC_PATH_PROG` to `AC_CHECK_PROG` to be consistent with Core's `AC_CHECK_PROG`. Both of these macros are supposed to find executables in the `PATH` but the difference is that former is supposed to return the full path whereas the latter is supposed to find only the program. As a result, the latter will accept even non-paths `no` as an override. Not knowing the full path is not an issue for the `configure` script because it will only execute `BREW` immediately afterwards, which works fine without the full path. (In particular, `PATH` cannot have changed in between [5].) [1] https://github.com/bitcoin/bitcoin/blob/master/configure.ac#L684 [2] https://github.com/bitcoin/bitcoin/blob/master/depends/config.site.in#L73-L76 [3] https://github.com/autotools-mirror/autoconf/blob/6d38e9fa2b39b3c3a8e4d6d7da38c59909d3f39d/lib/autoconf/programs.m4#L47 [4] https://github.com/autotools-mirror/autoconf/blob/6d38e9fa2b39b3c3a8e4d6d7da38c59909d3f39d/lib/autoconf/programs.m4#L127 [5] [3ab1178](https://github.com/bitcoin-core/secp256k1/commit/3ab1178d54029745219d67e6c305df4d7564e278) --- configure.ac | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 1ed991afa..ae26e3cda 100644 --- a/configure.ac +++ b/configure.ac @@ -42,8 +42,8 @@ AM_PROG_AS case $host_os in *darwin*) if test x$cross_compiling != xyes; then - AC_PATH_PROG([BREW],brew,) - if test x$BREW != x; then + AC_CHECK_PROG([BREW], brew, brew) + if test x$BREW = xbrew; then # These Homebrew packages may be keg-only, meaning that they won't be found # in expected paths because they may conflict with system files. Ask # Homebrew where each one is located, then adjust paths accordingly. @@ -58,10 +58,10 @@ case $host_os in VALGRIND_CPPFLAGS="-I$valgrind_prefix/include" fi else - AC_PATH_PROG([PORT],port,) + AC_CHECK_PROG([PORT], port, port) # If homebrew isn't installed and macports is, add the macports default paths # as a last resort. - if test x$PORT != x; then + if test x$PORT = xport; then CPPFLAGS="$CPPFLAGS -isystem /opt/local/include" LDFLAGS="$LDFLAGS -L/opt/local/lib" fi From bdf19f105c64a48ae607304ea6483b3286093f24 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 21 Jun 2021 16:19:00 -0700 Subject: [PATCH 06/26] Add random field multiply/square tests --- src/tests.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/tests.c b/src/tests.c index 6ceaba5e3..819fa9b79 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2508,6 +2508,70 @@ void run_field_misc(void) { } } +void test_fe_mul(const secp256k1_fe* a, const secp256k1_fe* b, int use_sqr) +{ + secp256k1_fe c, an, bn; + /* Variables in BE 32-byte format. */ + unsigned char a32[32], b32[32], c32[32]; + /* Variables in LE 16x uint16_t format. */ + uint16_t a16[16], b16[16], c16[16]; + /* Field modulus in LE 16x uint16_t format. */ + static const uint16_t m16[16] = { + 0xfc2f, 0xffff, 0xfffe, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, + }; + uint16_t t16[32]; + int i; + + /* Compute C = A * B in fe format. */ + c = *a; + if (use_sqr) { + secp256k1_fe_sqr(&c, &c); + } else { + secp256k1_fe_mul(&c, &c, b); + } + + /* Convert A, B, C into LE 16x uint16_t format. */ + an = *a; + bn = *b; + secp256k1_fe_normalize_var(&c); + secp256k1_fe_normalize_var(&an); + secp256k1_fe_normalize_var(&bn); + secp256k1_fe_get_b32(a32, &an); + secp256k1_fe_get_b32(b32, &bn); + secp256k1_fe_get_b32(c32, &c); + for (i = 0; i < 16; ++i) { + a16[i] = a32[31 - 2*i] + ((uint16_t)a32[30 - 2*i] << 8); + b16[i] = b32[31 - 2*i] + ((uint16_t)b32[30 - 2*i] << 8); + c16[i] = c32[31 - 2*i] + ((uint16_t)c32[30 - 2*i] << 8); + } + /* Compute T = A * B in LE 16x uint16_t format. */ + mulmod256(t16, a16, b16, m16); + /* Compare */ + CHECK(secp256k1_memcmp_var(t16, c16, 32) == 0); +} + +void run_fe_mul(void) { + int i; + for (i = 0; i < 100 * count; ++i) { + secp256k1_fe a, b, c, d; + random_fe(&a); + random_field_element_magnitude(&a); + random_fe(&b); + random_field_element_magnitude(&b); + random_fe_test(&c); + random_field_element_magnitude(&c); + random_fe_test(&d); + random_field_element_magnitude(&d); + test_fe_mul(&a, &a, 1); + test_fe_mul(&c, &c, 1); + test_fe_mul(&a, &b, 0); + test_fe_mul(&a, &c, 0); + test_fe_mul(&c, &b, 0); + test_fe_mul(&c, &d, 0); + } +} + void run_sqr(void) { secp256k1_fe x, s; @@ -6512,6 +6576,7 @@ int main(int argc, char **argv) { /* field tests */ run_field_misc(); run_field_convert(); + run_fe_mul(); run_sqr(); run_sqrt(); From b6c0b72fb06e3c31121f1ef4403d2a229a31ec1c Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 30 Oct 2020 17:48:16 +0000 Subject: [PATCH 07/26] schnorrsig: remove noncefp args from sign; add sign_custom function This makes the default sign function easier to use while allowing more granular control through sign_custom. Tests for sign_custom follow in a later commit. --- include/secp256k1_schnorrsig.h | 34 ++++++++++++++----- src/bench_schnorrsig.c | 4 +-- src/modules/schnorrsig/main_impl.h | 7 +++- .../schnorrsig/tests_exhaustive_impl.h | 2 +- src/modules/schnorrsig/tests_impl.h | 34 +++++++++---------- src/valgrind_ctime_test.c | 2 +- 6 files changed, 52 insertions(+), 31 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index f5a3d1fd1..92bc8dbfe 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -66,22 +66,38 @@ SECP256K1_API extern const secp256k1_nonce_function_hardened secp256k1_nonce_fun * signature. Instead, you can manually use secp256k1_schnorrsig_verify and * abort if it fails. * - * Otherwise BIP-340 compliant if the noncefp argument is NULL or - * secp256k1_nonce_function_bip340 and the ndata argument is 32-byte auxiliary - * randomness. - * * Returns 1 on success, 0 on failure. * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) * Out: sig64: pointer to a 64-byte array to store the serialized signature (cannot be NULL) * In: msg32: the 32-byte message being signed (cannot be NULL) * keypair: pointer to an initialized keypair (cannot be NULL) - * noncefp: pointer to a nonce generation function. If NULL, secp256k1_nonce_function_bip340 is used - * ndata: pointer to arbitrary data used by the nonce generation - * function (can be NULL). If it is non-NULL and - * secp256k1_nonce_function_bip340 is used, then ndata must be a - * pointer to 32-byte auxiliary randomness as per BIP-340. + * aux_rand32: 32 bytes of fresh randomness. While recommended to provide + * this, it is only supplemental to security and can be NULL. See + * BIP-340 "Default Signing" for a full explanation of this + * argument and for guidance if randomness is expensive. */ SECP256K1_API int secp256k1_schnorrsig_sign( + const secp256k1_context* ctx, + unsigned char *sig64, + const unsigned char *msg32, + const secp256k1_keypair *keypair, + unsigned char *aux_rand32 +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); + +/** Create a Schnorr signature with a more flexible API. + * + * Same arguments as secp256k1_schnorrsig_sign except that it misses aux_rand32 + * and instead allows allows providing a different nonce derivation function + * with its own data argument. + * + * In: noncefp: pointer to a nonce generation function. If NULL, + * secp256k1_nonce_function_bip340 is used + * ndata: pointer to arbitrary data used by the nonce generation function + * (can be NULL). If it is non-NULL and + * secp256k1_nonce_function_bip340 is used, then ndata must be a + * pointer to 32-byte auxiliary randomness as per BIP-340. + */ +SECP256K1_API int secp256k1_schnorrsig_sign_custom( const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, diff --git a/src/bench_schnorrsig.c b/src/bench_schnorrsig.c index dfea14414..52b5ca0e1 100644 --- a/src/bench_schnorrsig.c +++ b/src/bench_schnorrsig.c @@ -32,7 +32,7 @@ void bench_schnorrsig_sign(void* arg, int iters) { for (i = 0; i < iters; i++) { msg[0] = i; msg[1] = i >> 8; - CHECK(secp256k1_schnorrsig_sign(data->ctx, sig, msg, data->keypairs[i], NULL, NULL)); + CHECK(secp256k1_schnorrsig_sign(data->ctx, sig, msg, data->keypairs[i], NULL)); } } @@ -78,7 +78,7 @@ int main(void) { data.sigs[i] = sig; CHECK(secp256k1_keypair_create(data.ctx, keypair, sk)); - CHECK(secp256k1_schnorrsig_sign(data.ctx, sig, msg, keypair, NULL, NULL)); + CHECK(secp256k1_schnorrsig_sign(data.ctx, sig, msg, keypair, NULL)); CHECK(secp256k1_keypair_xonly_pub(data.ctx, &pk, NULL, keypair)); CHECK(secp256k1_xonly_pubkey_serialize(data.ctx, pk_char, &pk) == 1); } diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 27f719b7e..050e7f348 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -120,7 +120,12 @@ static void secp256k1_schnorrsig_challenge(secp256k1_scalar* e, const unsigned c secp256k1_scalar_set_b32(e, buf, NULL); } -int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { + +int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, unsigned char *aux_rand32) { + return secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, keypair, NULL, aux_rand32); +} + +int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { secp256k1_scalar sk; secp256k1_scalar e; secp256k1_scalar k; diff --git a/src/modules/schnorrsig/tests_exhaustive_impl.h b/src/modules/schnorrsig/tests_exhaustive_impl.h index 3359bf180..0df22f8d6 100644 --- a/src/modules/schnorrsig/tests_exhaustive_impl.h +++ b/src/modules/schnorrsig/tests_exhaustive_impl.h @@ -163,7 +163,7 @@ static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsign unsigned char expected_s_bytes[32]; secp256k1_scalar_get_b32(expected_s_bytes, &expected_s); /* Invoke the real function to construct a signature. */ - CHECK(secp256k1_schnorrsig_sign(ctx, sig64, msg32, &keypairs[d - 1], secp256k1_hardened_nonce_function_smallint, &k)); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, &keypairs[d - 1], secp256k1_hardened_nonce_function_smallint, &k)); /* The first 32 bytes must match the xonly pubkey for the specified k. */ CHECK(secp256k1_memcmp_var(sig64, xonly_pubkey_bytes[k - 1], 32) == 0); /* The last 32 bytes must match the expected s value. */ diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index a308ccf4d..2e1b3e743 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -143,23 +143,23 @@ void test_schnorrsig_api(void) { /** main test body **/ ecount = 0; - CHECK(secp256k1_schnorrsig_sign(none, sig, msg, &keypairs[0], NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(none, sig, msg, &keypairs[0], NULL) == 0); CHECK(ecount == 1); - CHECK(secp256k1_schnorrsig_sign(vrfy, sig, msg, &keypairs[0], NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(vrfy, sig, msg, &keypairs[0], NULL) == 0); CHECK(ecount == 2); - CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL) == 1); CHECK(ecount == 2); - CHECK(secp256k1_schnorrsig_sign(sign, NULL, msg, &keypairs[0], NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(sign, NULL, msg, &keypairs[0], NULL) == 0); CHECK(ecount == 3); - CHECK(secp256k1_schnorrsig_sign(sign, sig, NULL, &keypairs[0], NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(sign, sig, NULL, &keypairs[0], NULL) == 0); CHECK(ecount == 4); - CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, NULL, NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, NULL, NULL) == 0); CHECK(ecount == 5); - CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &invalid_keypair, NULL, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &invalid_keypair, NULL) == 0); CHECK(ecount == 6); ecount = 0; - CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL) == 1); CHECK(secp256k1_schnorrsig_verify(none, sig, msg, &pk[0]) == 0); CHECK(ecount == 1); CHECK(secp256k1_schnorrsig_verify(sign, sig, msg, &pk[0]) == 0); @@ -201,7 +201,7 @@ void test_schnorrsig_bip_vectors_check_signing(const unsigned char *sk, const un secp256k1_xonly_pubkey pk, pk_expected; CHECK(secp256k1_keypair_create(ctx, &keypair, sk)); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL, aux_rand)); + CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, aux_rand)); CHECK(secp256k1_memcmp_var(sig, expected_sig, 64) == 0); CHECK(secp256k1_xonly_pubkey_parse(ctx, &pk_expected, pk_serialized)); @@ -685,16 +685,16 @@ void test_schnorrsig_sign(void) { secp256k1_testrand256(sk); CHECK(secp256k1_keypair_create(ctx, &keypair, sk)); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL) == 1); /* Test different nonce functions */ memset(sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, nonce_function_failing, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_failing, NULL) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); memset(&sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, nonce_function_0, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_0, NULL) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, nonce_function_overflowing, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_overflowing, NULL) == 1); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) != 0); } @@ -717,7 +717,7 @@ void test_schnorrsig_sign_verify(void) { for (i = 0; i < N_SIGS; i++) { secp256k1_testrand256(msg[i]); - CHECK(secp256k1_schnorrsig_sign(ctx, sig[i], msg[i], &keypair, NULL, NULL)); + CHECK(secp256k1_schnorrsig_sign(ctx, sig[i], msg[i], &keypair, NULL)); CHECK(secp256k1_schnorrsig_verify(ctx, sig[i], msg[i], &pk)); } @@ -746,13 +746,13 @@ void test_schnorrsig_sign_verify(void) { } /* Test overflowing s */ - CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL, NULL)); + CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL)); CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); memset(&sig[0][32], 0xFF, 32); CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); /* Test negative s */ - CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL, NULL)); + CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL)); CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); secp256k1_scalar_set_b32(&s, &sig[0][32], NULL); secp256k1_scalar_negate(&s, &s); @@ -785,7 +785,7 @@ void test_schnorrsig_taproot(void) { /* Key spend */ secp256k1_testrand256(msg); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL) == 1); /* Verify key spend */ CHECK(secp256k1_xonly_pubkey_parse(ctx, &output_pk, output_pk_bytes) == 1); CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, &output_pk) == 1); diff --git a/src/valgrind_ctime_test.c b/src/valgrind_ctime_test.c index 4ac0f011b..ea6d4b3de 100644 --- a/src/valgrind_ctime_test.c +++ b/src/valgrind_ctime_test.c @@ -166,7 +166,7 @@ void run_tests(secp256k1_context *ctx, unsigned char *key) { ret = secp256k1_keypair_create(ctx, &keypair, key); VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); CHECK(ret == 1); - ret = secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL, NULL); + ret = secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL); VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); CHECK(ret == 1); #endif From 5a8e4991ad443cc0cc613d80380a2db802a4cbce Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 15 Jan 2021 20:58:01 +0000 Subject: [PATCH 08/26] Add secp256k1_tagged_sha256 as defined in BIP-340 Gives users the ability to hash messages to 32 byte before they are signed while allowing efficient domain separation through the tag. --- include/secp256k1.h | 25 +++++++++++++++++++++++++ src/secp256k1.c | 13 +++++++++++++ src/tests.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/include/secp256k1.h b/include/secp256k1.h index c05a06d29..7be7fd572 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -793,6 +793,31 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_combine( size_t n ) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); +/** Compute a tagged hash as defined in BIP-340. + * + * This is useful for creating a message hash and achieving domain separation + * through an application-specific tag. This function returns + * SHA256(SHA256(tag)||SHA256(tag)||msg). Therefore, tagged hash + * implementations optimized for a specific tag can precompute the SHA256 state + * after hashing the tag hashes. + * + * Returns 0 if the arguments are invalid and 1 otherwise. + * Args: ctx: pointer to a context object + * Out: hash32: pointer to a 32-byte array to store the resulting hash + * In: tag: pointer to an array containing the tag + * taglen: length of the tag array + * msg: pointer to an array containing the message + * msglen: length of the message array + */ +SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_tagged_sha256( + const secp256k1_context* ctx, + unsigned char *hash32, + const unsigned char *tag, + size_t taglen, + const unsigned char *msg, + size_t msglen +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5); + #ifdef __cplusplus } #endif diff --git a/src/secp256k1.c b/src/secp256k1.c index 81b1d6eb2..9908cab86 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -790,6 +790,19 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey * return 1; } +int secp256k1_tagged_sha256(const secp256k1_context* ctx, unsigned char *hash32, const unsigned char *tag, size_t taglen, const unsigned char *msg, size_t msglen) { + secp256k1_sha256 sha; + VERIFY_CHECK(ctx != NULL); + ARG_CHECK(hash32 != NULL); + ARG_CHECK(tag != NULL); + ARG_CHECK(msg != NULL); + + secp256k1_sha256_initialize_tagged(&sha, tag, taglen); + secp256k1_sha256_write(&sha, msg, msglen); + secp256k1_sha256_finalize(&sha, hash32); + return 1; +} + #ifdef ENABLE_MODULE_ECDH # include "modules/ecdh/main_impl.h" #endif diff --git a/src/tests.c b/src/tests.c index 6ceaba5e3..b72e5f8eb 100644 --- a/src/tests.c +++ b/src/tests.c @@ -564,6 +564,38 @@ void run_rfc6979_hmac_sha256_tests(void) { secp256k1_rfc6979_hmac_sha256_finalize(&rng); } +void run_tagged_sha256_tests(void) { + int ecount = 0; + secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); + unsigned char tag[32] = { 0 }; + unsigned char msg[32] = { 0 }; + unsigned char hash32[32]; + unsigned char hash_expected[32] = { + 0x04, 0x7A, 0x5E, 0x17, 0xB5, 0x86, 0x47, 0xC1, + 0x3C, 0xC6, 0xEB, 0xC0, 0xAA, 0x58, 0x3B, 0x62, + 0xFB, 0x16, 0x43, 0x32, 0x68, 0x77, 0x40, 0x6C, + 0xE2, 0x76, 0x55, 0x9A, 0x3B, 0xDE, 0x55, 0xB3 + }; + + secp256k1_context_set_illegal_callback(none, counting_illegal_callback_fn, &ecount); + + /* API test */ + CHECK(secp256k1_tagged_sha256(none, hash32, tag, sizeof(tag), msg, sizeof(msg)) == 1); + CHECK(secp256k1_tagged_sha256(none, NULL, tag, sizeof(tag), msg, sizeof(msg)) == 0); + CHECK(ecount == 1); + CHECK(secp256k1_tagged_sha256(none, hash32, NULL, 0, msg, sizeof(msg)) == 0); + CHECK(ecount == 2); + CHECK(secp256k1_tagged_sha256(none, hash32, tag, sizeof(tag), NULL, 0) == 0); + CHECK(ecount == 3); + + /* Static test vector */ + memcpy(tag, "tag", 3); + memcpy(msg, "msg", 3); + CHECK(secp256k1_tagged_sha256(none, hash32, tag, 3, msg, 3) == 1); + CHECK(secp256k1_memcmp_var(hash32, hash_expected, sizeof(hash32)) == 0); + secp256k1_context_destroy(none); +} + /***** RANDOM TESTS *****/ void test_rand_bits(int rand32, int bits) { @@ -6505,6 +6537,7 @@ int main(int argc, char **argv) { run_sha256_tests(); run_hmac_sha256_tests(); run_rfc6979_hmac_sha256_tests(); + run_tagged_sha256_tests(); /* scalar tests */ run_scalar_tests(); From a0c3fc177f7f435e593962504182c3861c47d1be Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 15 Jan 2021 21:19:34 +0000 Subject: [PATCH 09/26] schnorrsig: allow signing and verification of variable length msgs Varlen message support for the default sign function comes from recommending tagged_sha256. sign_custom on the other hand gets the ability to directly sign message of any length. This also implies signing and verification support for the empty message (NULL) with msglen 0. Tests for variable lengths follow in a later commit. --- include/secp256k1_schnorrsig.h | 39 +++++-- src/bench_schnorrsig.c | 8 +- src/modules/schnorrsig/main_impl.h | 27 +++-- .../schnorrsig/tests_exhaustive_impl.h | 14 ++- src/modules/schnorrsig/tests_impl.h | 104 ++++++++++-------- 5 files changed, 111 insertions(+), 81 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index 92bc8dbfe..bf02b69e8 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -24,7 +24,9 @@ extern "C" { * Returns: 1 if a nonce was successfully generated. 0 will cause signing to * return an error. * Out: nonce32: pointer to a 32-byte array to be filled by the function - * In: msg32: the 32-byte message hash being verified (will not be NULL) + * In: msg: the message being verified. Is NULL if and only if msglen + * is 0. + * msglen: the length of the message * key32: pointer to a 32-byte secret key (will not be NULL) * xonly_pk32: the 32-byte serialized xonly pubkey corresponding to key32 * (will not be NULL) @@ -38,7 +40,8 @@ extern "C" { */ typedef int (*secp256k1_nonce_function_hardened)( unsigned char *nonce32, - const unsigned char *msg32, + const unsigned char *msg, + size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, @@ -66,6 +69,13 @@ SECP256K1_API extern const secp256k1_nonce_function_hardened secp256k1_nonce_fun * signature. Instead, you can manually use secp256k1_schnorrsig_verify and * abort if it fails. * + * This function only signs 32-byte messages. If you have messages of a + * different size (or the same size but without a context-specific tag + * prefix), it is recommended to create a 32-byte message hash with + * secp256k1_tagged_sha256 and then sign the hash. Tagged hashing allows + * providing an context-specific tag for domain separation. This prevents + * signatures from being valid in multiple contexts by accident. + * * Returns 1 on success, 0 on failure. * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) * Out: sig64: pointer to a 64-byte array to store the serialized signature (cannot be NULL) @@ -86,12 +96,14 @@ SECP256K1_API int secp256k1_schnorrsig_sign( /** Create a Schnorr signature with a more flexible API. * - * Same arguments as secp256k1_schnorrsig_sign except that it misses aux_rand32 - * and instead allows allows providing a different nonce derivation function - * with its own data argument. + * Same arguments as secp256k1_schnorrsig_sign except that it allows signing + * variable length messages and allows providing a different nonce derivation + * function with its own data argument. * - * In: noncefp: pointer to a nonce generation function. If NULL, - * secp256k1_nonce_function_bip340 is used + * In: msg: the message being signed. Can only be NULL if msglen is 0. + * msglen: length of the message + * noncefp: pointer to a nonce generation function. If NULL, + * secp256k1_nonce_function_bip340 is used. * ndata: pointer to arbitrary data used by the nonce generation function * (can be NULL). If it is non-NULL and * secp256k1_nonce_function_bip340 is used, then ndata must be a @@ -100,11 +112,12 @@ SECP256K1_API int secp256k1_schnorrsig_sign( SECP256K1_API int secp256k1_schnorrsig_sign_custom( const secp256k1_context* ctx, unsigned char *sig64, - const unsigned char *msg32, + const unsigned char *msg, + size_t msglen, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata -) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(5); /** Verify a Schnorr signature. * @@ -112,15 +125,17 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom( * 0: incorrect signature * Args: ctx: a secp256k1 context object, initialized for verification. * In: sig64: pointer to the 64-byte signature to verify (cannot be NULL) - * msg32: the 32-byte message being verified (cannot be NULL) + * msg: the message being verified. Can only be NULL if msglen is 0. + * msglen: length of the message * pubkey: pointer to an x-only public key to verify with (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify( const secp256k1_context* ctx, const unsigned char *sig64, - const unsigned char *msg32, + const unsigned char *msg, + size_t msglen, const secp256k1_xonly_pubkey *pubkey -) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(5); #ifdef __cplusplus } diff --git a/src/bench_schnorrsig.c b/src/bench_schnorrsig.c index 52b5ca0e1..7b4b4b44a 100644 --- a/src/bench_schnorrsig.c +++ b/src/bench_schnorrsig.c @@ -13,6 +13,8 @@ #include "util.h" #include "bench.h" +#define MSGLEN 32 + typedef struct { secp256k1_context *ctx; int n; @@ -26,7 +28,7 @@ typedef struct { void bench_schnorrsig_sign(void* arg, int iters) { bench_schnorrsig_data *data = (bench_schnorrsig_data *)arg; int i; - unsigned char msg[32] = "benchmarkexamplemessagetemplate"; + unsigned char msg[MSGLEN] = "benchmarkexamplemessagetemplate"; unsigned char sig[64]; for (i = 0; i < iters; i++) { @@ -43,7 +45,7 @@ void bench_schnorrsig_verify(void* arg, int iters) { for (i = 0; i < iters; i++) { secp256k1_xonly_pubkey pk; CHECK(secp256k1_xonly_pubkey_parse(data->ctx, &pk, data->pk[i]) == 1); - CHECK(secp256k1_schnorrsig_verify(data->ctx, data->sigs[i], data->msgs[i], &pk)); + CHECK(secp256k1_schnorrsig_verify(data->ctx, data->sigs[i], data->msgs[i], MSGLEN, &pk)); } } @@ -60,7 +62,7 @@ int main(void) { for (i = 0; i < iters; i++) { unsigned char sk[32]; - unsigned char *msg = (unsigned char *)malloc(32); + unsigned char *msg = (unsigned char *)malloc(MSGLEN); unsigned char *sig = (unsigned char *)malloc(64); secp256k1_keypair *keypair = (secp256k1_keypair *)malloc(sizeof(*keypair)); unsigned char *pk_char = (unsigned char *)malloc(32); diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index 050e7f348..f4de6ad3b 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -47,7 +47,7 @@ static void secp256k1_nonce_function_bip340_sha256_tagged_aux(secp256k1_sha256 * * by using the correct tagged hash function. */ static const unsigned char bip340_algo[13] = "BIP0340/nonce"; -static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { +static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { secp256k1_sha256 sha; unsigned char masked_key[32]; int i; @@ -82,7 +82,7 @@ static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *ms secp256k1_sha256_write(&sha, key32, 32); } secp256k1_sha256_write(&sha, xonly_pk32, 32); - secp256k1_sha256_write(&sha, msg32, 32); + secp256k1_sha256_write(&sha, msg, msglen); secp256k1_sha256_finalize(&sha, nonce32); return 1; } @@ -104,28 +104,27 @@ static void secp256k1_schnorrsig_sha256_tagged(secp256k1_sha256 *sha) { sha->bytes = 64; } -static void secp256k1_schnorrsig_challenge(secp256k1_scalar* e, const unsigned char *r32, const unsigned char *msg32, const unsigned char *pubkey32) +static void secp256k1_schnorrsig_challenge(secp256k1_scalar* e, const unsigned char *r32, const unsigned char *msg, size_t msglen, const unsigned char *pubkey32) { unsigned char buf[32]; secp256k1_sha256 sha; - /* tagged hash(r.x, pk.x, msg32) */ + /* tagged hash(r.x, pk.x, msg) */ secp256k1_schnorrsig_sha256_tagged(&sha); secp256k1_sha256_write(&sha, r32, 32); secp256k1_sha256_write(&sha, pubkey32, 32); - secp256k1_sha256_write(&sha, msg32, 32); + secp256k1_sha256_write(&sha, msg, msglen); secp256k1_sha256_finalize(&sha, buf); /* Set scalar e to the challenge hash modulo the curve order as per * BIP340. */ secp256k1_scalar_set_b32(e, buf, NULL); } - int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, unsigned char *aux_rand32) { - return secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, keypair, NULL, aux_rand32); + return secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, 32, keypair, NULL, aux_rand32); } -int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { +int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { secp256k1_scalar sk; secp256k1_scalar e; secp256k1_scalar k; @@ -140,7 +139,7 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char VERIFY_CHECK(ctx != NULL); ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); ARG_CHECK(sig64 != NULL); - ARG_CHECK(msg32 != NULL); + ARG_CHECK(msg != NULL || msglen == 0); ARG_CHECK(keypair != NULL); if (noncefp == NULL) { @@ -157,7 +156,7 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char secp256k1_scalar_get_b32(seckey, &sk); secp256k1_fe_get_b32(pk_buf, &pk.x); - ret &= !!noncefp(buf, msg32, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata); + ret &= !!noncefp(buf, msg, msglen, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata); secp256k1_scalar_set_b32(&k, buf, NULL); ret &= !secp256k1_scalar_is_zero(&k); secp256k1_scalar_cmov(&k, &secp256k1_scalar_one, !ret); @@ -175,7 +174,7 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char secp256k1_fe_normalize_var(&r.x); secp256k1_fe_get_b32(&sig64[0], &r.x); - secp256k1_schnorrsig_challenge(&e, &sig64[0], msg32, pk_buf); + secp256k1_schnorrsig_challenge(&e, &sig64[0], msg, msglen, pk_buf); secp256k1_scalar_mul(&e, &e, &sk); secp256k1_scalar_add(&e, &e, &k); secp256k1_scalar_get_b32(&sig64[32], &e); @@ -188,7 +187,7 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char return ret; } -int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned char *sig64, const unsigned char *msg32, const secp256k1_xonly_pubkey *pubkey) { +int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_xonly_pubkey *pubkey) { secp256k1_scalar s; secp256k1_scalar e; secp256k1_gej rj; @@ -202,7 +201,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha VERIFY_CHECK(ctx != NULL); ARG_CHECK(secp256k1_ecmult_context_is_built(&ctx->ecmult_ctx)); ARG_CHECK(sig64 != NULL); - ARG_CHECK(msg32 != NULL); + ARG_CHECK(msg != NULL || msglen == 0); ARG_CHECK(pubkey != NULL); if (!secp256k1_fe_set_b32(&rx, &sig64[0])) { @@ -220,7 +219,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha /* Compute e. */ secp256k1_fe_get_b32(buf, &pk.x); - secp256k1_schnorrsig_challenge(&e, &sig64[0], msg32, buf); + secp256k1_schnorrsig_challenge(&e, &sig64[0], msg, msglen, buf); /* Compute rj = s*G + (-e)*pkj */ secp256k1_scalar_negate(&e, &e); diff --git a/src/modules/schnorrsig/tests_exhaustive_impl.h b/src/modules/schnorrsig/tests_exhaustive_impl.h index 0df22f8d6..4ffdde691 100644 --- a/src/modules/schnorrsig/tests_exhaustive_impl.h +++ b/src/modules/schnorrsig/tests_exhaustive_impl.h @@ -58,13 +58,15 @@ static const unsigned char invalid_pubkey_bytes[][32] = { #define NUM_INVALID_KEYS (sizeof(invalid_pubkey_bytes) / sizeof(invalid_pubkey_bytes[0])) -static int secp256k1_hardened_nonce_function_smallint(unsigned char *nonce32, const unsigned char *msg32, +static int secp256k1_hardened_nonce_function_smallint(unsigned char *nonce32, const unsigned char *msg, + size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void* data) { secp256k1_scalar s; int *idata = data; - (void)msg32; + (void)msg; + (void)msglen; (void)key32; (void)xonly_pk32; (void)algo; @@ -103,7 +105,7 @@ static void test_exhaustive_schnorrsig_verify(const secp256k1_context *ctx, cons secp256k1_scalar e; unsigned char msg32[32]; secp256k1_testrand256(msg32); - secp256k1_schnorrsig_challenge(&e, sig64, msg32, pk32); + secp256k1_schnorrsig_challenge(&e, sig64, msg32, sizeof(msg32), pk32); /* Only do work if we hit a challenge we haven't tried before. */ if (!e_done[e]) { /* Iterate over the possible valid last 32 bytes in the signature. @@ -121,7 +123,7 @@ static void test_exhaustive_schnorrsig_verify(const secp256k1_context *ctx, cons secp256k1_testrand256(sig64 + 32); expect_valid = 0; } - valid = secp256k1_schnorrsig_verify(ctx, sig64, msg32, &pubkeys[d - 1]); + valid = secp256k1_schnorrsig_verify(ctx, sig64, msg32, sizeof(msg32), &pubkeys[d - 1]); CHECK(valid == expect_valid); count_valid += valid; } @@ -156,14 +158,14 @@ static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsign while (e_count_done < EXHAUSTIVE_TEST_ORDER) { secp256k1_scalar e; secp256k1_testrand256(msg32); - secp256k1_schnorrsig_challenge(&e, xonly_pubkey_bytes[k - 1], msg32, xonly_pubkey_bytes[d - 1]); + secp256k1_schnorrsig_challenge(&e, xonly_pubkey_bytes[k - 1], msg32, sizeof(msg32), xonly_pubkey_bytes[d - 1]); /* Only do work if we hit a challenge we haven't tried before. */ if (!e_done[e]) { secp256k1_scalar expected_s = (actual_k + e * actual_d) % EXHAUSTIVE_TEST_ORDER; unsigned char expected_s_bytes[32]; secp256k1_scalar_get_b32(expected_s_bytes, &expected_s); /* Invoke the real function to construct a signature. */ - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, &keypairs[d - 1], secp256k1_hardened_nonce_function_smallint, &k)); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, sizeof(msg32), &keypairs[d - 1], secp256k1_hardened_nonce_function_smallint, &k)); /* The first 32 bytes must match the xonly pubkey for the specified k. */ CHECK(secp256k1_memcmp_var(sig64, xonly_pubkey_bytes[k - 1], 32) == 0); /* The last 32 bytes must match the expected s value. */ diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index 2e1b3e743..e35b4edc7 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -12,11 +12,11 @@ /* Checks that a bit flip in the n_flip-th argument (that has n_bytes many * bytes) changes the hash function */ -void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, size_t n_bytes, size_t algolen) { +void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, size_t n_bytes, size_t msglen, size_t algolen) { unsigned char nonces[2][32]; - CHECK(nonce_function_bip340(nonces[0], args[0], args[1], args[2], args[3], algolen, args[4]) == 1); + CHECK(nonce_function_bip340(nonces[0], args[0], msglen, args[1], args[2], args[3], algolen, args[4]) == 1); secp256k1_testrand_flip(args[n_flip], n_bytes); - CHECK(nonce_function_bip340(nonces[1], args[0], args[1], args[2], args[3], algolen, args[4]) == 1); + CHECK(nonce_function_bip340(nonces[1], args[0], msglen, args[1], args[2], args[3], algolen, args[4]) == 1); CHECK(secp256k1_memcmp_var(nonces[0], nonces[1], 32) != 0); } @@ -40,6 +40,7 @@ void run_nonce_function_bip340_tests(void) { secp256k1_sha256 sha_optimized; unsigned char nonce[32]; unsigned char msg[32]; + size_t msglen = sizeof(msg); unsigned char key[32]; unsigned char pk[32]; unsigned char aux_rand[32]; @@ -72,34 +73,42 @@ void run_nonce_function_bip340_tests(void) { args[3] = algo; args[4] = aux_rand; for (i = 0; i < count; i++) { - nonce_function_bip340_bitflip(args, 0, 32, algolen); - nonce_function_bip340_bitflip(args, 1, 32, algolen); - nonce_function_bip340_bitflip(args, 2, 32, algolen); + nonce_function_bip340_bitflip(args, 0, 32, msglen, algolen); + nonce_function_bip340_bitflip(args, 1, 32, msglen, algolen); + nonce_function_bip340_bitflip(args, 2, 32, msglen, algolen); /* Flip algo special case "BIP0340/nonce" */ - nonce_function_bip340_bitflip(args, 3, algolen, algolen); + nonce_function_bip340_bitflip(args, 3, algolen, msglen, algolen); /* Flip algo again */ - nonce_function_bip340_bitflip(args, 3, algolen, algolen); - nonce_function_bip340_bitflip(args, 4, 32, algolen); + nonce_function_bip340_bitflip(args, 3, algolen, msglen, algolen); + nonce_function_bip340_bitflip(args, 4, 32, msglen, algolen); } /* NULL algo is disallowed */ - CHECK(nonce_function_bip340(nonce, msg, key, pk, NULL, 0, NULL) == 0); - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); + CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, NULL, 0, NULL) == 0); + CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, algo, algolen, NULL) == 1); /* Other algo is fine */ secp256k1_rfc6979_hmac_sha256_generate(&secp256k1_test_rng, algo, algolen); - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); + CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, algo, algolen, NULL) == 1); for (i = 0; i < count; i++) { unsigned char nonce2[32]; + uint32_t offset = secp256k1_testrand_int(msglen - 1); + size_t msglen_tmp = (msglen + offset) % msglen; + size_t algolen_tmp; + + /* Different msglen gives different nonce */ + CHECK(nonce_function_bip340(nonce2, msg, msglen_tmp, key, pk, algo, algolen, NULL) == 1); + CHECK(secp256k1_memcmp_var(nonce, nonce2, 32) != 0); + /* Different algolen gives different nonce */ - uint32_t offset = secp256k1_testrand_int(algolen - 1); - size_t algolen_tmp = (algolen + offset) % algolen; - CHECK(nonce_function_bip340(nonce2, msg, key, pk, algo, algolen_tmp, NULL) == 1); + offset = secp256k1_testrand_int(algolen - 1); + algolen_tmp = (algolen + offset) % algolen; + CHECK(nonce_function_bip340(nonce2, msg, msglen, key, pk, algo, algolen_tmp, NULL) == 1); CHECK(secp256k1_memcmp_var(nonce, nonce2, 32) != 0); } /* NULL aux_rand argument is allowed. */ - CHECK(nonce_function_bip340(nonce, msg, key, pk, algo, algolen, NULL) == 1); + CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, algo, algolen, NULL) == 1); } void test_schnorrsig_api(void) { @@ -160,19 +169,19 @@ void test_schnorrsig_api(void) { ecount = 0; CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL) == 1); - CHECK(secp256k1_schnorrsig_verify(none, sig, msg, &pk[0]) == 0); + CHECK(secp256k1_schnorrsig_verify(none, sig, msg, sizeof(msg), &pk[0]) == 0); CHECK(ecount == 1); - CHECK(secp256k1_schnorrsig_verify(sign, sig, msg, &pk[0]) == 0); + CHECK(secp256k1_schnorrsig_verify(sign, sig, msg, sizeof(msg), &pk[0]) == 0); CHECK(ecount == 2); - CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, &pk[0]) == 1); + CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, sizeof(msg), &pk[0]) == 1); CHECK(ecount == 2); - CHECK(secp256k1_schnorrsig_verify(vrfy, NULL, msg, &pk[0]) == 0); + CHECK(secp256k1_schnorrsig_verify(vrfy, NULL, msg, sizeof(msg), &pk[0]) == 0); CHECK(ecount == 3); - CHECK(secp256k1_schnorrsig_verify(vrfy, sig, NULL, &pk[0]) == 0); + CHECK(secp256k1_schnorrsig_verify(vrfy, sig, NULL, sizeof(msg), &pk[0]) == 0); CHECK(ecount == 4); - CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, NULL) == 0); + CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, sizeof(msg), NULL) == 0); CHECK(ecount == 5); - CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, &zero_pk) == 0); + CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, sizeof(msg), &zero_pk) == 0); CHECK(ecount == 6); secp256k1_context_destroy(none); @@ -195,19 +204,19 @@ void test_schnorrsig_sha256_tagged(void) { /* Helper function for schnorrsig_bip_vectors * Signs the message and checks that it's the same as expected_sig. */ -void test_schnorrsig_bip_vectors_check_signing(const unsigned char *sk, const unsigned char *pk_serialized, unsigned char *aux_rand, const unsigned char *msg, const unsigned char *expected_sig) { +void test_schnorrsig_bip_vectors_check_signing(const unsigned char *sk, const unsigned char *pk_serialized, unsigned char *aux_rand, const unsigned char *msg32, const unsigned char *expected_sig) { unsigned char sig[64]; secp256k1_keypair keypair; secp256k1_xonly_pubkey pk, pk_expected; CHECK(secp256k1_keypair_create(ctx, &keypair, sk)); - CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, aux_rand)); + CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg32, &keypair, aux_rand)); CHECK(secp256k1_memcmp_var(sig, expected_sig, 64) == 0); CHECK(secp256k1_xonly_pubkey_parse(ctx, &pk_expected, pk_serialized)); CHECK(secp256k1_keypair_xonly_pub(ctx, &pk, NULL, &keypair)); CHECK(secp256k1_memcmp_var(&pk, &pk_expected, sizeof(pk)) == 0); - CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, &pk)); + CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg32, 32, &pk)); } /* Helper function for schnorrsig_bip_vectors @@ -216,7 +225,7 @@ void test_schnorrsig_bip_vectors_check_verify(const unsigned char *pk_serialized secp256k1_xonly_pubkey pk; CHECK(secp256k1_xonly_pubkey_parse(ctx, &pk, pk_serialized)); - CHECK(expected == secp256k1_schnorrsig_verify(ctx, sig, msg32, &pk)); + CHECK(expected == secp256k1_schnorrsig_verify(ctx, sig, msg32, 32, &pk)); } /* Test vectors according to BIP-340 ("Schnorr Signatures for secp256k1"). See @@ -639,8 +648,9 @@ void test_schnorrsig_bip_vectors(void) { } /* Nonce function that returns constant 0 */ -static int nonce_function_failing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { - (void) msg32; +static int nonce_function_failing(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { + (void) msg; + (void) msglen; (void) key32; (void) xonly_pk32; (void) algo; @@ -651,8 +661,9 @@ static int nonce_function_failing(unsigned char *nonce32, const unsigned char *m } /* Nonce function that sets nonce to 0 */ -static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { - (void) msg32; +static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { + (void) msg; + (void) msglen; (void) key32; (void) xonly_pk32; (void) algo; @@ -664,8 +675,9 @@ static int nonce_function_0(unsigned char *nonce32, const unsigned char *msg32, } /* Nonce function that sets nonce to 0xFF...0xFF */ -static int nonce_function_overflowing(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { - (void) msg32; +static int nonce_function_overflowing(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { + (void) msg; + (void) msglen; (void) key32; (void) xonly_pk32; (void) algo; @@ -689,12 +701,12 @@ void test_schnorrsig_sign(void) { /* Test different nonce functions */ memset(sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_failing, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_failing, NULL) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); memset(&sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_0, NULL) == 0); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_0, NULL) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, &keypair, nonce_function_overflowing, NULL) == 1); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_overflowing, NULL) == 1); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) != 0); } @@ -718,7 +730,7 @@ void test_schnorrsig_sign_verify(void) { for (i = 0; i < N_SIGS; i++) { secp256k1_testrand256(msg[i]); CHECK(secp256k1_schnorrsig_sign(ctx, sig[i], msg[i], &keypair, NULL)); - CHECK(secp256k1_schnorrsig_verify(ctx, sig[i], msg[i], &pk)); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[i], msg[i], sizeof(msg[i]), &pk)); } { @@ -728,36 +740,36 @@ void test_schnorrsig_sign_verify(void) { size_t byte_idx = secp256k1_testrand_int(32); unsigned char xorbyte = secp256k1_testrand_int(254)+1; sig[sig_idx][byte_idx] ^= xorbyte; - CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], &pk)); + CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); sig[sig_idx][byte_idx] ^= xorbyte; byte_idx = secp256k1_testrand_int(32); sig[sig_idx][32+byte_idx] ^= xorbyte; - CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], &pk)); + CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); sig[sig_idx][32+byte_idx] ^= xorbyte; byte_idx = secp256k1_testrand_int(32); msg[sig_idx][byte_idx] ^= xorbyte; - CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], &pk)); + CHECK(!secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); msg[sig_idx][byte_idx] ^= xorbyte; /* Check that above bitflips have been reversed correctly */ - CHECK(secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], &pk)); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); } /* Test overflowing s */ CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL)); - CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], sizeof(msg[0]), &pk)); memset(&sig[0][32], 0xFF, 32); - CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); + CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], sizeof(msg[0]), &pk)); /* Test negative s */ CHECK(secp256k1_schnorrsig_sign(ctx, sig[0], msg[0], &keypair, NULL)); - CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], sizeof(msg[0]), &pk)); secp256k1_scalar_set_b32(&s, &sig[0][32], NULL); secp256k1_scalar_negate(&s, &s); secp256k1_scalar_get_b32(&sig[0][32], &s); - CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], &pk)); + CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], sizeof(msg[0]), &pk)); } #undef N_SIGS @@ -788,7 +800,7 @@ void test_schnorrsig_taproot(void) { CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL) == 1); /* Verify key spend */ CHECK(secp256k1_xonly_pubkey_parse(ctx, &output_pk, output_pk_bytes) == 1); - CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, &output_pk) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &output_pk) == 1); /* Script spend */ CHECK(secp256k1_xonly_pubkey_serialize(ctx, internal_pk_bytes, &internal_pk) == 1); From d8d806aaf386c7ead9431649f899ff82b0185aae Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Mon, 2 Nov 2020 14:41:25 +0000 Subject: [PATCH 10/26] schnorrsig: add extra parameter struct for sign_custom This simplifies the interface of sign_custom and allows adding more parameters later in a backward compatible way. --- include/secp256k1_schnorrsig.h | 46 +++++++++++++++---- src/modules/schnorrsig/main_impl.h | 25 ++++++++-- .../schnorrsig/tests_exhaustive_impl.h | 6 ++- src/modules/schnorrsig/tests_impl.h | 18 ++++++-- 4 files changed, 75 insertions(+), 20 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index bf02b69e8..d68bba62c 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -63,6 +63,35 @@ typedef int (*secp256k1_nonce_function_hardened)( */ SECP256K1_API extern const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; +/** Data structure that contains additional arguments for schnorrsig_sign_custom. + * + * A schnorrsig_extraparams structure object can be initialized correctly by + * setting it to SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT. + * + * Members: + * magic: set to SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC at initialization + * and has no other function than making sure the object is + * initialized. + * noncefp: pointer to a nonce generation function. If NULL, + * secp256k1_nonce_function_bip340 is used + * ndata: pointer to arbitrary data used by the nonce generation function + * (can be NULL). If it is non-NULL and + * secp256k1_nonce_function_bip340 is used, then ndata must be a + * pointer to 32-byte auxiliary randomness as per BIP-340. + */ +typedef struct { + unsigned char magic[4]; + secp256k1_nonce_function_hardened noncefp; + void* ndata; +} secp256k1_schnorrsig_extraparams; + +#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC "\xda\x6f\xb3\x8c" +#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT {\ + SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC,\ + NULL,\ + NULL\ +} + /** Create a Schnorr signature. * * Does _not_ strictly follow BIP-340 because it does not verify the resulting @@ -97,17 +126,15 @@ SECP256K1_API int secp256k1_schnorrsig_sign( /** Create a Schnorr signature with a more flexible API. * * Same arguments as secp256k1_schnorrsig_sign except that it allows signing - * variable length messages and allows providing a different nonce derivation - * function with its own data argument. + * variable length messages and accepts a pointer to an extraparams object that + * allows customizing signing by passing additional arguments. + * + * Creates the same signatures as schnorrsig_sign if msglen is 32 and the + * extraparams.ndata is the same as aux_rand32. * * In: msg: the message being signed. Can only be NULL if msglen is 0. * msglen: length of the message - * noncefp: pointer to a nonce generation function. If NULL, - * secp256k1_nonce_function_bip340 is used. - * ndata: pointer to arbitrary data used by the nonce generation function - * (can be NULL). If it is non-NULL and - * secp256k1_nonce_function_bip340 is used, then ndata must be a - * pointer to 32-byte auxiliary randomness as per BIP-340. + * extraparams: pointer to a extraparams object (can be NULL) */ SECP256K1_API int secp256k1_schnorrsig_sign_custom( const secp256k1_context* ctx, @@ -115,8 +142,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom( const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, - secp256k1_nonce_function_hardened noncefp, - void *ndata + secp256k1_schnorrsig_extraparams *extraparams ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(5); /** Verify a Schnorr signature. diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index f4de6ad3b..e6de73b8a 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -120,11 +120,7 @@ static void secp256k1_schnorrsig_challenge(secp256k1_scalar* e, const unsigned c secp256k1_scalar_set_b32(e, buf, NULL); } -int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, unsigned char *aux_rand32) { - return secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, 32, keypair, NULL, aux_rand32); -} - -int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { +int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, secp256k1_nonce_function_hardened noncefp, void *ndata) { secp256k1_scalar sk; secp256k1_scalar e; secp256k1_scalar k; @@ -187,6 +183,25 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char return ret; } +int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, unsigned char *aux_rand32) { + return secp256k1_schnorrsig_sign_internal(ctx, sig64, msg32, 32, keypair, secp256k1_nonce_function_bip340, aux_rand32); +} + +int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, secp256k1_schnorrsig_extraparams *extraparams) { + secp256k1_nonce_function_hardened noncefp = NULL; + void *ndata = NULL; + VERIFY_CHECK(ctx != NULL); + + if (extraparams != NULL) { + ARG_CHECK(secp256k1_memcmp_var(extraparams->magic, + SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC, + sizeof(extraparams->magic)) == 0); + noncefp = extraparams->noncefp; + ndata = extraparams->ndata; + } + return secp256k1_schnorrsig_sign_internal(ctx, sig64, msg, msglen, keypair, noncefp, ndata); +} + int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_xonly_pubkey *pubkey) { secp256k1_scalar s; secp256k1_scalar e; diff --git a/src/modules/schnorrsig/tests_exhaustive_impl.h b/src/modules/schnorrsig/tests_exhaustive_impl.h index 4ffdde691..d8df9dd2d 100644 --- a/src/modules/schnorrsig/tests_exhaustive_impl.h +++ b/src/modules/schnorrsig/tests_exhaustive_impl.h @@ -141,6 +141,8 @@ static void test_exhaustive_schnorrsig_verify(const secp256k1_context *ctx, cons static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsigned char (*xonly_pubkey_bytes)[32], const secp256k1_keypair* keypairs, const int* parities) { int d, k; uint64_t iter = 0; + secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; + /* Loop over keys. */ for (d = 1; d < EXHAUSTIVE_TEST_ORDER; ++d) { int actual_d = d; @@ -153,6 +155,8 @@ static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsign unsigned char sig64[64]; int actual_k = k; if (skip_section(&iter)) continue; + extraparams.noncefp = secp256k1_hardened_nonce_function_smallint; + extraparams.ndata = &k; if (parities[k - 1]) actual_k = EXHAUSTIVE_TEST_ORDER - k; /* Generate random messages until all challenges have been tried. */ while (e_count_done < EXHAUSTIVE_TEST_ORDER) { @@ -165,7 +169,7 @@ static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsign unsigned char expected_s_bytes[32]; secp256k1_scalar_get_b32(expected_s_bytes, &expected_s); /* Invoke the real function to construct a signature. */ - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, sizeof(msg32), &keypairs[d - 1], secp256k1_hardened_nonce_function_smallint, &k)); + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig64, msg32, sizeof(msg32), &keypairs[d - 1], &extraparams)); /* The first 32 bytes must match the xonly pubkey for the specified k. */ CHECK(secp256k1_memcmp_var(sig64, xonly_pubkey_bytes[k - 1], 32) == 0); /* The last 32 bytes must match the expected s value. */ diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index e35b4edc7..4942315cf 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -690,24 +690,34 @@ static int nonce_function_overflowing(unsigned char *nonce32, const unsigned cha void test_schnorrsig_sign(void) { unsigned char sk[32]; + secp256k1_xonly_pubkey pk; secp256k1_keypair keypair; const unsigned char msg[32] = "this is a msg for a schnorrsig.."; unsigned char sig[64]; unsigned char zeros64[64] = { 0 }; + secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; secp256k1_testrand256(sk); CHECK(secp256k1_keypair_create(ctx, &keypair, sk)); + CHECK(secp256k1_keypair_xonly_pub(ctx, &pk, NULL, &keypair)); CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &pk)); /* Test different nonce functions */ + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &pk)); memset(sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_failing, NULL) == 0); + extraparams.noncefp = nonce_function_failing; + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); memset(&sig, 1, sizeof(sig)); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_0, NULL) == 0); + extraparams.noncefp = nonce_function_0; + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 0); CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) == 0); - CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, nonce_function_overflowing, NULL) == 1); - CHECK(secp256k1_memcmp_var(sig, zeros64, sizeof(sig)) != 0); + memset(&sig, 1, sizeof(sig)); + extraparams.noncefp = nonce_function_overflowing; + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &pk)); } #define N_SIGS 3 From fdd06b7967196a3b34f73a5b19632637b4bde90a Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Thu, 21 Jan 2021 22:23:04 +0000 Subject: [PATCH 11/26] schnorrsig: add tests for sign_custom and varlen msg verification --- src/modules/schnorrsig/tests_impl.h | 55 +++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index 4942315cf..0405d46f4 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -121,6 +121,8 @@ void test_schnorrsig_api(void) { secp256k1_xonly_pubkey pk[3]; secp256k1_xonly_pubkey zero_pk; unsigned char sig[64]; + secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; + secp256k1_schnorrsig_extraparams invalid_extraparams = { 0 }; /** setup **/ secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); @@ -167,6 +169,28 @@ void test_schnorrsig_api(void) { CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &invalid_keypair, NULL) == 0); CHECK(ecount == 6); + ecount = 0; + CHECK(secp256k1_schnorrsig_sign_custom(none, sig, msg, sizeof(msg), &keypairs[0], &extraparams) == 0); + CHECK(ecount == 1); + CHECK(secp256k1_schnorrsig_sign_custom(vrfy, sig, msg, sizeof(msg), &keypairs[0], &extraparams) == 0); + CHECK(ecount == 2); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &keypairs[0], &extraparams) == 1); + CHECK(ecount == 2); + CHECK(secp256k1_schnorrsig_sign_custom(sign, NULL, msg, sizeof(msg), &keypairs[0], &extraparams) == 0); + CHECK(ecount == 3); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, NULL, sizeof(msg), &keypairs[0], &extraparams) == 0); + CHECK(ecount == 4); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, NULL, 0, &keypairs[0], &extraparams) == 1); + CHECK(ecount == 4); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), NULL, &extraparams) == 0); + CHECK(ecount == 5); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &invalid_keypair, &extraparams) == 0); + CHECK(ecount == 6); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &keypairs[0], NULL) == 1); + CHECK(ecount == 6); + CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &keypairs[0], &invalid_extraparams) == 0); + CHECK(ecount == 7); + ecount = 0; CHECK(secp256k1_schnorrsig_sign(sign, sig, msg, &keypairs[0], NULL) == 1); CHECK(secp256k1_schnorrsig_verify(none, sig, msg, sizeof(msg), &pk[0]) == 0); @@ -179,6 +203,8 @@ void test_schnorrsig_api(void) { CHECK(ecount == 3); CHECK(secp256k1_schnorrsig_verify(vrfy, sig, NULL, sizeof(msg), &pk[0]) == 0); CHECK(ecount == 4); + CHECK(secp256k1_schnorrsig_verify(vrfy, sig, NULL, 0, &pk[0]) == 0); + CHECK(ecount == 4); CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, sizeof(msg), NULL) == 0); CHECK(ecount == 5); CHECK(secp256k1_schnorrsig_verify(vrfy, sig, msg, sizeof(msg), &zero_pk) == 0); @@ -694,10 +720,13 @@ void test_schnorrsig_sign(void) { secp256k1_keypair keypair; const unsigned char msg[32] = "this is a msg for a schnorrsig.."; unsigned char sig[64]; + unsigned char sig2[64]; unsigned char zeros64[64] = { 0 }; secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; + unsigned char aux_rand[32]; secp256k1_testrand256(sk); + secp256k1_testrand256(aux_rand); CHECK(secp256k1_keypair_create(ctx, &keypair, sk)); CHECK(secp256k1_keypair_xonly_pub(ctx, &pk, NULL, &keypair)); CHECK(secp256k1_schnorrsig_sign(ctx, sig, msg, &keypair, NULL) == 1); @@ -718,6 +747,14 @@ void test_schnorrsig_sign(void) { extraparams.noncefp = nonce_function_overflowing; CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 1); CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &pk)); + + /* When using the default nonce function, schnorrsig_sign_custom produces + * the same result as schnorrsig_sign with aux_rand = extraparams.ndata */ + extraparams.noncefp = NULL; + extraparams.ndata = aux_rand; + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig, msg, sizeof(msg), &keypair, &extraparams) == 1); + CHECK(secp256k1_schnorrsig_sign(ctx, sig2, msg, &keypair, extraparams.ndata) == 1); + CHECK(secp256k1_memcmp_var(sig, sig2, sizeof(sig)) == 0); } #define N_SIGS 3 @@ -780,6 +817,24 @@ void test_schnorrsig_sign_verify(void) { secp256k1_scalar_negate(&s, &s); secp256k1_scalar_get_b32(&sig[0][32], &s); CHECK(!secp256k1_schnorrsig_verify(ctx, sig[0], msg[0], sizeof(msg[0]), &pk)); + + /* The empty message can be signed & verified */ + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig[0], NULL, 0, &keypair, NULL) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], NULL, 0, &pk) == 1); + + { + /* Test varying message lengths */ + unsigned char msg_large[32 * 8]; + uint32_t msglen = secp256k1_testrand_int(sizeof(msg_large)); + for (i = 0; i < sizeof(msg_large); i += 32) { + secp256k1_testrand256(&msg_large[i]); + } + CHECK(secp256k1_schnorrsig_sign_custom(ctx, sig[0], msg_large, msglen, &keypair, NULL) == 1); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg_large, msglen, &pk) == 1); + /* Verification for a random wrong message length fails */ + msglen = (msglen + (sizeof(msg_large) - 1)) % sizeof(msg_large); + CHECK(secp256k1_schnorrsig_verify(ctx, sig[0], msg_large, msglen, &pk) == 0); + } } #undef N_SIGS From 5f6ceafcfa46a69e901bed87e2c5f323b03b1e8c Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Thu, 18 Mar 2021 22:43:54 +0000 Subject: [PATCH 12/26] schnorrsig: allow setting MSGLEN != 32 in benchmark --- src/bench_schnorrsig.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bench_schnorrsig.c b/src/bench_schnorrsig.c index 7b4b4b44a..d95bc00f4 100644 --- a/src/bench_schnorrsig.c +++ b/src/bench_schnorrsig.c @@ -28,13 +28,13 @@ typedef struct { void bench_schnorrsig_sign(void* arg, int iters) { bench_schnorrsig_data *data = (bench_schnorrsig_data *)arg; int i; - unsigned char msg[MSGLEN] = "benchmarkexamplemessagetemplate"; + unsigned char msg[MSGLEN] = {0}; unsigned char sig[64]; for (i = 0; i < iters; i++) { msg[0] = i; msg[1] = i >> 8; - CHECK(secp256k1_schnorrsig_sign(data->ctx, sig, msg, data->keypairs[i], NULL)); + CHECK(secp256k1_schnorrsig_sign_custom(data->ctx, sig, msg, MSGLEN, data->keypairs[i], NULL)); } } @@ -60,6 +60,7 @@ int main(void) { data.msgs = (const unsigned char **)malloc(iters * sizeof(unsigned char *)); data.sigs = (const unsigned char **)malloc(iters * sizeof(unsigned char *)); + CHECK(MSGLEN >= 4); for (i = 0; i < iters; i++) { unsigned char sk[32]; unsigned char *msg = (unsigned char *)malloc(MSGLEN); @@ -71,7 +72,7 @@ int main(void) { msg[1] = sk[1] = i >> 8; msg[2] = sk[2] = i >> 16; msg[3] = sk[3] = i >> 24; - memset(&msg[4], 'm', 28); + memset(&msg[4], 'm', MSGLEN - 4); memset(&sk[4], 's', 28); data.keypairs[i] = keypair; @@ -80,7 +81,7 @@ int main(void) { data.sigs[i] = sig; CHECK(secp256k1_keypair_create(data.ctx, keypair, sk)); - CHECK(secp256k1_schnorrsig_sign(data.ctx, sig, msg, keypair, NULL)); + CHECK(secp256k1_schnorrsig_sign_custom(data.ctx, sig, msg, MSGLEN, keypair, NULL)); CHECK(secp256k1_keypair_xonly_pub(data.ctx, &pk, NULL, keypair)); CHECK(secp256k1_xonly_pubkey_serialize(data.ctx, pk_char, &pk) == 1); } From 41ed13942bd722ff0ee4d2eb8bef55155549afd6 Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Mon, 28 Jun 2021 15:21:00 +0200 Subject: [PATCH 13/26] tests: really test the non-var scalar inverse Function `test_inverse_scalar` contains: (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, x); /* l = 1/x */ The two sides of the condition are the same function. This seems to be an error, as there also exists a non-var function, named `secp256k1_scalar_inverse`. Make `test_inverse_scalar` use this other function when `var` is false. This issue was found using clang's static analyzer, which reported a "Logic error: Identical expressions in conditional expression" (with checker `alpha.core.IdenticalExpr`). --- src/tests.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests.c b/src/tests.c index 6ceaba5e3..9c28eb2a8 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2595,7 +2595,7 @@ void test_inverse_scalar(secp256k1_scalar* out, const secp256k1_scalar* x, int v { secp256k1_scalar l, r, t; - (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, x); /* l = 1/x */ + (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse)(&l, x); /* l = 1/x */ if (out) *out = l; if (secp256k1_scalar_is_zero(x)) { CHECK(secp256k1_scalar_is_zero(&l)); @@ -2605,9 +2605,9 @@ void test_inverse_scalar(secp256k1_scalar* out, const secp256k1_scalar* x, int v CHECK(secp256k1_scalar_is_one(&t)); /* x*(1/x) == 1 */ secp256k1_scalar_add(&r, x, &scalar_minus_one); /* r = x-1 */ if (secp256k1_scalar_is_zero(&r)) return; - (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&r, &r); /* r = 1/(x-1) */ + (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse)(&r, &r); /* r = 1/(x-1) */ secp256k1_scalar_add(&l, &scalar_minus_one, &l); /* l = 1/x-1 */ - (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, &l); /* l = 1/(1/x-1) */ + (var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse)(&l, &l); /* l = 1/(1/x-1) */ secp256k1_scalar_add(&l, &l, &secp256k1_scalar_one); /* l = 1/(1/x-1)+1 */ secp256k1_scalar_add(&l, &r, &l); /* l = 1/(1/x-1)+1 + 1/(x-1) */ CHECK(secp256k1_scalar_is_zero(&l)); /* l == 0 */ From 07256267ffa9fb37609ec46260e9990bccd35dc5 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 13 May 2021 17:06:16 +0200 Subject: [PATCH 14/26] build: Use own variable SECP_CFLAGS instead of touching user CFLAGS Fixes one of the items in #923, namely the warnings of the form '_putenv' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] This also cleans up the way we add CFLAGS, in particular flags enabling warnings. Now we perform some more fine-grained checking for flag support, which is not strictly necessary but the changes also help to document autoconf.ac. --- Makefile.am | 8 +++- build-aux/m4/bitcoin_secp.m4 | 16 +++++++ configure.ac | 83 ++++++++++++++++++------------------ 3 files changed, 64 insertions(+), 43 deletions(-) diff --git a/Makefile.am b/Makefile.am index 23b29281d..1e0356088 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,9 @@ ACLOCAL_AMFLAGS = -I build-aux/m4 +# AM_CFLAGS will be automatically prepended to CFLAGS by Automake when compiling some foo +# which does not have an explicit foo_CFLAGS variable set. +AM_CFLAGS = $(SECP_CFLAGS) + lib_LTLIBRARIES = libsecp256k1.la include_HEADERS = include/secp256k1.h include_HEADERS += include/secp256k1_preallocated.h @@ -129,10 +133,10 @@ CPPFLAGS_FOR_BUILD +=-I$(top_srcdir) -I$(builddir)/src gen_context_OBJECTS = gen_context.o gen_context_BIN = gen_context$(BUILD_EXEEXT) gen_%.o: src/gen_%.c src/libsecp256k1-config.h - $(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@ + $(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@ $(gen_context_BIN): $(gen_context_OBJECTS) - $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@ + $(CC_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@ $(libsecp256k1_la_OBJECTS): src/ecmult_static_context.h $(tests_OBJECTS): src/ecmult_static_context.h diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index e57888ca1..8245b2b86 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/build-aux/m4/bitcoin_secp.m4 @@ -82,3 +82,19 @@ if test x"$has_valgrind" != x"yes"; then AC_CHECK_HEADER([valgrind/memcheck.h], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed])]) fi ]) + +dnl SECP_TRY_APPEND_CFLAGS(flags, VAR) +dnl Append flags to VAR if CC accepts them. +AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [ + AC_MSG_CHECKING([if ${CC} supports $1]) + SECP_TRY_APPEND_CFLAGS_saved_CFLAGS="$CFLAGS" + CFLAGS="$1 $CFLAGS" + AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], [flag_works=yes], [flag_works=no]) + AC_MSG_RESULT($flag_works) + CFLAGS="$SECP_TRY_APPEND_CFLAGS_saved_CFLAGS" + if test x"$flag_works" = x"yes"; then + $2="$$2 $1" + fi + unset flag_works + AC_SUBST($2) +]) diff --git a/configure.ac b/configure.ac index 1ed991afa..aaca53430 100644 --- a/configure.ac +++ b/configure.ac @@ -70,35 +70,41 @@ case $host_os in ;; esac -CFLAGS="-W $CFLAGS" - -warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-unused-function -Wno-long-long -Wno-overlength-strings" -saved_CFLAGS="$CFLAGS" -CFLAGS="$warn_CFLAGS $CFLAGS" -AC_MSG_CHECKING([if ${CC} supports ${warn_CFLAGS}]) -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], - [ AC_MSG_RESULT([yes]) ], - [ AC_MSG_RESULT([no]) - CFLAGS="$saved_CFLAGS" - ]) - -saved_CFLAGS="$CFLAGS" -CFLAGS="-Wconditional-uninitialized $CFLAGS" -AC_MSG_CHECKING([if ${CC} supports -Wconditional-uninitialized]) -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], - [ AC_MSG_RESULT([yes]) ], - [ AC_MSG_RESULT([no]) - CFLAGS="$saved_CFLAGS" - ]) - -saved_CFLAGS="$CFLAGS" -CFLAGS="-fvisibility=hidden $CFLAGS" -AC_MSG_CHECKING([if ${CC} supports -fvisibility=hidden]) -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], - [ AC_MSG_RESULT([yes]) ], - [ AC_MSG_RESULT([no]) - CFLAGS="$saved_CFLAGS" - ]) +# Try if some desirable compiler flags are supported and append them to SECP_CFLAGS. +# +# These are our own flags, so we append them to our own SECP_CFLAGS variable (instead of CFLAGS) as +# recommended in the automake manual (Section "Flag Variables Ordering"). CFLAGS belongs to the user +# and we are not supposed to touch it. In the Makefile, we will need to ensure that SECP_CFLAGS +# is prepended to CFLAGS when invoking the compiler so that the user always has the last word (flag). +# +# Another advantage of not touching CFLAGS is that the contents of CFLAGS will be picked up by +# libtool for compiling helper executables. For example, when compiling for Windows, libtool will +# generate entire wrapper executables (instead of simple wrapper scripts as on Unix) to ensure +# proper operation of uninstalled programs linked by libtool against the uninstalled shared library. +# These executables are compiled from C source file for which our flags may not be appropriate, +# e.g., -std=c89 flag has lead to undesirable warnings in the past. +# +# TODO We still touch the CFLAGS for --coverage and -O0/-O2. +# TODO We should analogously not touch CPPFLAGS and LDFLAGS but currently there are no issues. +AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [ + # Try to append -Werror=unknown-warning-option to CFLAGS temporarily. Otherwise clang will + # not error out if it gets unknown warning flags and the checks here will always succeed + # no matter if clang knows the flag or not. + SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS="$CFLAGS" + SECP_TRY_APPEND_CFLAGS([-Werror=unknown-warning-option], CFLAGS) + + SECP_TRY_APPEND_CFLAGS([-std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef], $1) # GCC >= 3.0, -Wlong-long is implied by -pedantic. + SECP_TRY_APPEND_CFLAGS([-Wno-overlength-strings], $1) # GCC >= 4.2, -Woverlength-strings is implied by -pedantic. + SECP_TRY_APPEND_CFLAGS([-Wall], $1) # GCC >= 2.95 and probably many other compilers + SECP_TRY_APPEND_CFLAGS([-Wno-unused-function], $1) # GCC >= 3.0, -Wunused-function is implied by -Wall. + SECP_TRY_APPEND_CFLAGS([-Wextra], $1) # GCC >= 3.4, this is the newer name of -W, which we don't use because older GCCs will warn about unused functions. + SECP_TRY_APPEND_CFLAGS([-Wcast-align], $1) # GCC >= 2.95 + SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only + SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0 + + CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS" +]) +SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS) ### ### Define config arguments @@ -360,8 +366,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then fi # If we're not cross-compiling, simply use the same compiler for building the static precompation code. CC_FOR_BUILD="$CC" - CFLAGS_FOR_BUILD="$CFLAGS" + SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS" CPPFLAGS_FOR_BUILD="$CPPFLAGS" + CFLAGS_FOR_BUILD="$CFLAGS" LDFLAGS_FOR_BUILD="$LDFLAGS" else AX_PROG_CC_FOR_BUILD @@ -378,15 +385,7 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then SAVE_LDFLAGS="$LDFLAGS" LDFLAGS="$LDFLAGS_FOR_BUILD" - warn_CFLAGS_FOR_BUILD="-Wall -Wextra -Wno-unused-function" - saved_CFLAGS="$CFLAGS" - CFLAGS="$warn_CFLAGS_FOR_BUILD $CFLAGS" - AC_MSG_CHECKING([if native ${CC_FOR_BUILD} supports ${warn_CFLAGS_FOR_BUILD}]) - AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], - [ AC_MSG_RESULT([yes]) ], - [ AC_MSG_RESULT([no]) - CFLAGS="$saved_CFLAGS" - ]) + SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS_FOR_BUILD) AC_MSG_CHECKING([for working native compiler: ${CC_FOR_BUILD}]) AC_RUN_IFELSE( @@ -394,8 +393,6 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then [working_native_cc=yes], [working_native_cc=no],[:]) - CFLAGS_FOR_BUILD="$CFLAGS" - # Restore the environment cross_compiling=$save_cross_compiling CC="$SAVE_CC" @@ -419,6 +416,7 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then fi AC_SUBST(CC_FOR_BUILD) + AC_SUBST(SECP_CFLAGS_FOR_BUILD) AC_SUBST(CFLAGS_FOR_BUILD) AC_SUBST(CPPFLAGS_FOR_BUILD) AC_SUBST(LDFLAGS_FOR_BUILD) @@ -490,6 +488,7 @@ AC_SUBST(SECP_INCLUDES) AC_SUBST(SECP_LIBS) AC_SUBST(SECP_TEST_LIBS) AC_SUBST(SECP_TEST_INCLUDES) +AC_SUBST(SECP_CFLAGS) AM_CONDITIONAL([ENABLE_COVERAGE], [test x"$enable_coverage" = x"yes"]) AM_CONDITIONAL([USE_TESTS], [test x"$use_tests" != x"no"]) AM_CONDITIONAL([USE_EXHAUSTIVE_TESTS], [test x"$use_exhaustive_tests" != x"no"]) @@ -532,12 +531,14 @@ fi echo echo " valgrind = $enable_valgrind" echo " CC = $CC" +echo " SECP_CFLAGS = $SECP_CFLAGS" echo " CFLAGS = $CFLAGS" echo " CPPFLAGS = $CPPFLAGS" echo " LDFLAGS = $LDFLAGS" echo if test x"$set_precomp" = x"yes"; then echo " CC_FOR_BUILD = $CC_FOR_BUILD" +echo " SECP_CFLAGS_FOR_BUILD = $SECP_CFLAGS_FOR_BUILD" echo " CFLAGS_FOR_BUILD = $CFLAGS_FOR_BUILD" echo " CPPFLAGS_FOR_BUILD = $CPPFLAGS_FOR_BUILD" echo " LDFLAGS_FOR_BUILD = $LDFLAGS_FOR_BUILD" From 595e8a35d80c932f91e810ce889c48b6efbaf890 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 13 May 2021 17:14:56 +0200 Subject: [PATCH 15/26] build: Enable -Wcast-align=strict warning --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index aaca53430..9c3cb37ce 100644 --- a/configure.ac +++ b/configure.ac @@ -99,6 +99,7 @@ AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [ SECP_TRY_APPEND_CFLAGS([-Wno-unused-function], $1) # GCC >= 3.0, -Wunused-function is implied by -Wall. SECP_TRY_APPEND_CFLAGS([-Wextra], $1) # GCC >= 3.4, this is the newer name of -W, which we don't use because older GCCs will warn about unused functions. SECP_TRY_APPEND_CFLAGS([-Wcast-align], $1) # GCC >= 2.95 + SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0 SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0 From 7939cd571c7a236f0d46e5cd7b6529ae29757c5a Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 13 May 2021 18:54:37 +0200 Subject: [PATCH 16/26] build: List *CPPFLAGS before *CFLAGS like on the compiler command line --- configure.ac | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index 9c3cb37ce..0caa0cf5b 100644 --- a/configure.ac +++ b/configure.ac @@ -367,8 +367,8 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then fi # If we're not cross-compiling, simply use the same compiler for building the static precompation code. CC_FOR_BUILD="$CC" - SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS" CPPFLAGS_FOR_BUILD="$CPPFLAGS" + SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS" CFLAGS_FOR_BUILD="$CFLAGS" LDFLAGS_FOR_BUILD="$LDFLAGS" else @@ -379,10 +379,10 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then cross_compiling=no SAVE_CC="$CC" CC="$CC_FOR_BUILD" - SAVE_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS_FOR_BUILD" SAVE_CPPFLAGS="$CPPFLAGS" CPPFLAGS="$CPPFLAGS_FOR_BUILD" + SAVE_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS_FOR_BUILD" SAVE_LDFLAGS="$LDFLAGS" LDFLAGS="$LDFLAGS_FOR_BUILD" @@ -397,14 +397,14 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then # Restore the environment cross_compiling=$save_cross_compiling CC="$SAVE_CC" - CFLAGS="$SAVE_CFLAGS" CPPFLAGS="$SAVE_CPPFLAGS" + CFLAGS="$SAVE_CFLAGS" LDFLAGS="$SAVE_LDFLAGS" if test x"$working_native_cc" = x"no"; then AC_MSG_RESULT([no]) set_precomp=no - m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CFLAGS_FOR_BUILD, CPPFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.]) + m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CPPFLAGS_FOR_BUILD, CFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.]) if test x"$use_ecmult_static_precomputation" = x"yes"; then AC_MSG_ERROR([native compiler ${CC_FOR_BUILD} does not produce working binaries. please_set_for_build]) else @@ -417,9 +417,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then fi AC_SUBST(CC_FOR_BUILD) + AC_SUBST(CPPFLAGS_FOR_BUILD) AC_SUBST(SECP_CFLAGS_FOR_BUILD) AC_SUBST(CFLAGS_FOR_BUILD) - AC_SUBST(CPPFLAGS_FOR_BUILD) AC_SUBST(LDFLAGS_FOR_BUILD) else set_precomp=no @@ -532,15 +532,15 @@ fi echo echo " valgrind = $enable_valgrind" echo " CC = $CC" +echo " CPPFLAGS = $CPPFLAGS" echo " SECP_CFLAGS = $SECP_CFLAGS" echo " CFLAGS = $CFLAGS" -echo " CPPFLAGS = $CPPFLAGS" echo " LDFLAGS = $LDFLAGS" echo if test x"$set_precomp" = x"yes"; then echo " CC_FOR_BUILD = $CC_FOR_BUILD" +echo " CPPFLAGS_FOR_BUILD = $CPPFLAGS_FOR_BUILD" echo " SECP_CFLAGS_FOR_BUILD = $SECP_CFLAGS_FOR_BUILD" echo " CFLAGS_FOR_BUILD = $CFLAGS_FOR_BUILD" -echo " CPPFLAGS_FOR_BUILD = $CPPFLAGS_FOR_BUILD" echo " LDFLAGS_FOR_BUILD = $LDFLAGS_FOR_BUILD" fi From b924e1e605dcf9f9b362531184d16d643cc3baa9 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 13 May 2021 19:34:16 +0200 Subject: [PATCH 17/26] build: Ensure that configure's compile checks default to -O2 Fixes #896. --- .gitignore | 1 + configure.ac | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index b62055a39..79b740db8 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ aclocal.m4 autom4te.cache/ config.log config.status +conftest* *.tar.gz *.la libtool diff --git a/configure.ac b/configure.ac index 0caa0cf5b..3a68e8585 100644 --- a/configure.ac +++ b/configure.ac @@ -8,10 +8,6 @@ AH_TOP([#define LIBSECP256K1_CONFIG_H]) AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/]) AM_INIT_AUTOMAKE([foreign subdir-objects]) -# Set -g if CFLAGS are not already set, which matches the default autoconf -# behavior (see PROG_CC in the Autoconf manual) with the exception that we don't -# set -O2 here because we set it in any case (see further down). -: ${CFLAGS="-g"} LT_INIT # Make the compilation flags quiet unless V=1 is used. @@ -84,7 +80,6 @@ esac # These executables are compiled from C source file for which our flags may not be appropriate, # e.g., -std=c89 flag has lead to undesirable warnings in the past. # -# TODO We still touch the CFLAGS for --coverage and -O0/-O2. # TODO We should analogously not touch CPPFLAGS and LDFLAGS but currently there are no issues. AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [ # Try to append -Werror=unknown-warning-option to CFLAGS temporarily. Otherwise clang will @@ -220,10 +215,14 @@ AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"]) if test x"$enable_coverage" = x"yes"; then AC_DEFINE(COVERAGE, 1, [Define this symbol to compile out all VERIFY code]) - CFLAGS="-O0 --coverage $CFLAGS" + SECP_CFLAGS="-O0 --coverage $SECP_CFLAGS" LDFLAGS="--coverage $LDFLAGS" else - CFLAGS="-O2 $CFLAGS" + # Most likely the CFLAGS already contain -O2 because that is autoconf's default. + # We still add it here because passing it twice is not an issue, and handling + # this case would just add unnecessary complexity (see #896). + SECP_CFLAGS="-O2 $SECP_CFLAGS" + SECP_CFLAGS_FOR_BUILD="-O2 $SECP_CFLAGS_FOR_BUILD" fi if test x"$req_asm" = x"auto"; then From 0302138f7508414e9e5212bc45b4ca4c0e5f081c Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Thu, 6 May 2021 14:02:00 +0200 Subject: [PATCH 18/26] ci: Make compiler warning into errors on CI This also tidies the list of environment variables in .cirrus.yml. --- .cirrus.yml | 21 ++++++++++++++------- configure.ac | 3 +++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 6d63511e6..aca8e0b52 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,21 +1,28 @@ env: - WIDEMUL: auto + ### compiler options + HOST: + # Specific warnings can be disabled with -Wno-error=foo. + # -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual. + WERROR_CFLAGS: -Werror -pedantic-errors + MAKEFLAGS: -j2 + BUILD: check + ### secp256k1 config STATICPRECOMPUTATION: yes ECMULTGENPRECISION: auto ASM: no - BUILD: check + WIDEMUL: auto WITH_VALGRIND: yes EXTRAFLAGS: - HOST: + ### secp256k1 modules + EXPERIMENTAL: no ECDH: no RECOVERY: no SCHNORRSIG: no - EXPERIMENTAL: no - CTIMETEST: yes - BENCH: yes + ### test options TEST_ITERS: + BENCH: yes BENCH_ITERS: 2 - MAKEFLAGS: -j2 + CTIMETEST: yes cat_logs_snippet: &CAT_LOGS always: diff --git a/configure.ac b/configure.ac index 3a68e8585..7172a9f22 100644 --- a/configure.ac +++ b/configure.ac @@ -357,6 +357,9 @@ if test x"$enable_valgrind" = x"yes"; then SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS" fi +# Add -Werror and similar flags passed from the outside (for testing, e.g., in CI) +SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS" + # Handle static precomputation (after everything which modifies CFLAGS and friends) if test x"$use_ecmult_static_precomputation" != x"no"; then if test x"$cross_compiling" = x"no"; then From a1ee83c6546c65d8f5b32acc4a0e1740858ee7d6 Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Mon, 28 Jun 2021 15:44:19 +0200 Subject: [PATCH 19/26] tests_exhaustive: check the result of secp256k1_ecdsa_sign If `secp256k1_ecdsa_sign` fails, the signature which is then loaded by `secp256k1_ecdsa_signature_load` is garbage. Exit early with an error when this occurs. --- src/tests_exhaustive.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index b7c782899..5b9a3035d 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -302,6 +302,7 @@ void test_exhaustive_sign(const secp256k1_context *ctx, const secp256k1_ge *grou if (skip_section(&iter)) continue; for (k = 1; k < EXHAUSTIVE_TEST_ORDER; k++) { /* nonce */ const int starting_k = k; + int ret; secp256k1_ecdsa_signature sig; secp256k1_scalar sk, msg, r, s, expected_r; unsigned char sk32[32], msg32[32]; @@ -310,7 +311,8 @@ void test_exhaustive_sign(const secp256k1_context *ctx, const secp256k1_ge *grou secp256k1_scalar_get_b32(sk32, &sk); secp256k1_scalar_get_b32(msg32, &msg); - secp256k1_ecdsa_sign(ctx, &sig, msg32, sk32, secp256k1_nonce_function_smallint, &k); + ret = secp256k1_ecdsa_sign(ctx, &sig, msg32, sk32, secp256k1_nonce_function_smallint, &k); + CHECK(ret == 1); secp256k1_ecdsa_signature_load(ctx, &r, &s, &sig); /* Note that we compute expected_r *after* signing -- this is important From 2cc3cfa58382582fc26eb91e3243153e1d06ce77 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sun, 4 Jul 2021 02:01:44 +0200 Subject: [PATCH 20/26] Fix -Wmissing-braces warning in clang --- src/modules/schnorrsig/tests_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index 0405d46f4..e804df504 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -122,7 +122,7 @@ void test_schnorrsig_api(void) { secp256k1_xonly_pubkey zero_pk; unsigned char sig[64]; secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; - secp256k1_schnorrsig_extraparams invalid_extraparams = { 0 }; + secp256k1_schnorrsig_extraparams invalid_extraparams = {{ 0 }, NULL, NULL}; /** setup **/ secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE); From 769528f30714a1e5503a7abefad25fd89f0ef237 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sun, 4 Jul 2021 02:03:18 +0200 Subject: [PATCH 21/26] Don't use string literals for char arrays without NUL termination unsigned char foo[4] = "abcd" is not valid C++ because the string literal "abcd" does not fit into foo due to the terminating NUL character. This is valid in C, it will just omit the NUL character. Fixes #962. --- include/secp256k1_schnorrsig.h | 2 +- src/modules/schnorrsig/main_impl.h | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index d68bba62c..74cbcac45 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -85,7 +85,7 @@ typedef struct { void* ndata; } secp256k1_schnorrsig_extraparams; -#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC "\xda\x6f\xb3\x8c" +#define SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC { 0xda, 0x6f, 0xb3, 0x8c } #define SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT {\ SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC,\ NULL,\ diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index e6de73b8a..693b78f03 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -47,6 +47,8 @@ static void secp256k1_nonce_function_bip340_sha256_tagged_aux(secp256k1_sha256 * * by using the correct tagged hash function. */ static const unsigned char bip340_algo[13] = "BIP0340/nonce"; +static const unsigned char schnorrsig_extraparams_magic[4] = SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC; + static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) { secp256k1_sha256 sha; unsigned char masked_key[32]; @@ -194,7 +196,7 @@ int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char if (extraparams != NULL) { ARG_CHECK(secp256k1_memcmp_var(extraparams->magic, - SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC, + schnorrsig_extraparams_magic, sizeof(extraparams->magic)) == 0); noncefp = extraparams->noncefp; ndata = extraparams->ndata; From b5b8e7b7190aed619a1fa83bf1794fddef90346d Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sun, 4 Jul 2021 11:35:52 +0200 Subject: [PATCH 22/26] Don't declare constants twice This is forbidden in C++. --- src/ecmult.h | 1 - src/ecmult_gen.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/ecmult.h b/src/ecmult.h index 7ab617e20..84537bbfe 100644 --- a/src/ecmult.h +++ b/src/ecmult.h @@ -17,7 +17,6 @@ typedef struct { secp256k1_ge_storage (*pre_g_128)[]; /* odd multiples of 2^128*generator */ } secp256k1_ecmult_context; -static const size_t SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE; static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx); static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, void **prealloc); static void secp256k1_ecmult_context_finalize_memcpy(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src); diff --git a/src/ecmult_gen.h b/src/ecmult_gen.h index 539618dcb..05cf4d52c 100644 --- a/src/ecmult_gen.h +++ b/src/ecmult_gen.h @@ -35,7 +35,6 @@ typedef struct { secp256k1_gej initial; } secp256k1_ecmult_gen_context; -static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE; static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context* ctx); static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, void **prealloc); static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context* src); From f698caaff6a263390d7ded5be4751dbc1c862b1e Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Sun, 4 Jul 2021 11:37:06 +0200 Subject: [PATCH 23/26] Use unsigned char consistently for byte arrays C++ does not allow initialization with string literals but we do it in other places and -fpermissive will convince g++ to compile. --- src/modules/schnorrsig/tests_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index e804df504..59357afa9 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -219,7 +219,7 @@ void test_schnorrsig_api(void) { /* Checks that hash initialized by secp256k1_schnorrsig_sha256_tagged has the * expected state. */ void test_schnorrsig_sha256_tagged(void) { - char tag[17] = "BIP0340/challenge"; + unsigned char tag[17] = "BIP0340/challenge"; secp256k1_sha256 sha; secp256k1_sha256 sha_optimized; From 90e83449b2c2e4046af755e35fdeee579a468f31 Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Mon, 5 Jul 2021 10:33:36 +0200 Subject: [PATCH 24/26] ci: Add C++ test --- .cirrus.yml | 20 ++++++++++++++++++++ ci/linux-debian.Dockerfile | 1 + 2 files changed, 21 insertions(+) diff --git a/.cirrus.yml b/.cirrus.yml index aca8e0b52..bf71a7083 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -320,3 +320,23 @@ task: - ./ci/cirrus.sh << : *CAT_LOGS +task: + name: "C++ -fpermissive" + container: + dockerfile: ci/linux-debian.Dockerfile + cpu: 1 + memory: 1G + env: + # ./configure correctly errors out when given CC=g++. + # We hack around this by passing CC=g++ only to make. + CC: gcc + MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive + WERROR_CFLAGS: + EXPERIMENTAL: yes + ECDH: yes + RECOVERY: yes + SCHNORRSIG: yes + << : *MERGE_BASE + test_script: + - ./ci/cirrus.sh + << : *CAT_LOGS diff --git a/ci/linux-debian.Dockerfile b/ci/linux-debian.Dockerfile index 6def91333..2c02ed69d 100644 --- a/ci/linux-debian.Dockerfile +++ b/ci/linux-debian.Dockerfile @@ -13,6 +13,7 @@ RUN apt-get install --no-install-recommends --no-upgrade -y \ git ca-certificates \ make automake libtool pkg-config dpkg-dev valgrind qemu-user \ gcc clang llvm libc6-dbg \ + g++ \ gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan5:i386 \ gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \ From aeece4459977b69962bcd1e1ee8845c18c74ff8f Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 14 Jul 2021 11:12:41 +0200 Subject: [PATCH 25/26] gen_context: Don't use any ASM --- src/gen_context.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gen_context.c b/src/gen_context.c index 8fab7aa49..f9176eb99 100644 --- a/src/gen_context.c +++ b/src/gen_context.c @@ -13,10 +13,11 @@ /* We can't require the precomputed tables when creating them. */ #undef USE_ECMULT_STATIC_PRECOMPUTATION -/* In principle we could use external ASM, but this yields only a minor speedup in +/* In principle we could use ASM, but this yields only a minor speedup in build time and it's very complicated. In particular when cross-compiling, we'd - need to build the external ASM for the build and the host machine. */ + need to build the ASM for the build and the host machine. */ #undef USE_EXTERNAL_ASM +#undef USE_ASM_X86_64 #include "../include/secp256k1.h" #include "assumptions.h" From 6ad66de6802a47687d451ac6d5369dc36c558874 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 27 Jul 2021 18:15:58 +0000 Subject: [PATCH 26/26] rangeproof: add an (unnecessary) variable initialization to shut up CI --- src/modules/rangeproof/rangeproof_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/rangeproof/rangeproof_impl.h b/src/modules/rangeproof/rangeproof_impl.h index 8056f0a78..df9f64dc4 100644 --- a/src/modules/rangeproof/rangeproof_impl.h +++ b/src/modules/rangeproof/rangeproof_impl.h @@ -369,7 +369,7 @@ SECP256K1_INLINE static int secp256k1_rangeproof_rewind_inner(secp256k1_scalar * secp256k1_scalar stmp; unsigned char prep[4096]; unsigned char tmp[32]; - uint64_t value; + uint64_t value = 0; size_t offset; size_t i; size_t j;