14c9739a1fb485bb56dbe3447132a37bcbef4e22 tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs (Tim Ruffing)
4a19668c37bc77d0165f4a1c0e626e321e9c4a09 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs (Tim Ruffing)
45b6468d7e3ed9849ed474c71e9a9479de1a77db Have secp256k1_ge_set_all_gej_var initialize all fields. Previous behaviour would not initialize r->y values in the case where infinity is passed in. Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity. (Russell O'Connor)
31c0f6de413e521731ad0e63424431b3dd49cec8 Have secp256k1_gej_double_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)
dd6c3de322740a3054cf6a1994a38dc8f201b473 Have secp256k1_ge_set_gej_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)
Pull request description:
Previous behaviour would not initialize `r->x` and `r->y` values in the case where infinity is passed in.
ACKs for top commit:
gmaxwell:
ACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
sipa:
utACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
real-or-random:
ACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
Tree-SHA512: 2e779b767f02e348af4bbc62aa9871c3d1d29e61a6c643c879c49f2de27556a3588850acd2f7c7483790677597d01064025e14befdbf29e783f57996fe4430f9
Add a new run_inverse_tests that replaces all existing field/scalar inverse tests,
and tests a few identities for fixed inputs, small numbers (-999...999), random
inputs (structured and unstructured), as well as comparing with the output of
secp256k1_fe_inv_all_var.
This adds tests for the modinv{32,64}_impl.h directly (before the functions are used
inside the field/scalar code). It uses a naive implementation of modular multiplication
and gcds in order to verify the modular inverses themselves.
This was detected while running the tests with the `-Wconditional-uninitialized` flag
```
./autogen.sh
CC=clang CFLAGS="-Wconditional-uninitialized" ./configure
make check
```
The resulting warning is a false positive, but setting the value to -1
ensures that the CHECK below will fail if recid is never written to.
As identified in #829 and #833. Fixes#829.
Since we touch this anyway, this commit additionally makes the
identifiers in the benchmark files a little bit more consistent.
The VERIFY macro turns on various paranoid consistency checks, but
the complete functionality should still be tested without it.
This also adds a couple of static test points for extremely small
split inputs/outputs. The existing bounds vectors already check
extremely large outputs.
f431b3f28ac95a3645ad5a6dc96b878fa30a1de3 valgrind_ctime_test: Add schnorrsig_sign (Jonas Nick)
16ffa9d97cef93f49544b016339c107882f9a1c3 schnorrsig: Add taproot test case (Jonas Nick)
8dfd53ee3fa059562483d1867815f78b9e00d947 schnorrsig: Add benchmark for sign and verify (Jonas Nick)
4e43520026f5bcd182d21f0759bac159ef47bb62 schnorrsig: Add BIP-340 compatible signing and verification (Jonas Nick)
7332d2db6b62fda851f9ed8adbfda187a875b84e schnorrsig: Add BIP-340 nonce function (Jonas Nick)
7a703fd97db0161bae07ef84513ddde6e0d27353 schnorrsig: Init empty experimental module (Jonas Nick)
eabd9bc46a31c0da6db6d88840eadbe9006447b1 Allow initializing tagged sha256 (Jonas Nick)
6fcb5b845d2832ce019d60507033f74426290768 extrakeys: Add keypair_xonly_tweak_add (Jonas Nick)
58254463f9a2e96d893157a341c9953c440fdf60 extrakeys: Add keypair struct with create, pub and pub_xonly (Jonas Nick)
f0010349b876bc6b3f0a6ec6c8bad0b12ca17b51 Separate helper functions for pubkey_create and seckey_tweak_add (Jonas Nick)
910d9c284c33b77774a9316d4524f313357d441c extrakeys: Add xonly_pubkey_tweak_add & xonly_pubkey_tweak_add_test (Jonas Nick)
176bfb1110147b5dca1834ea071acc846fb1cab3 Separate helper function for ec_pubkey_tweak_add (Jonas Nick)
4cd2ee474d178bd1b5602486104db346a7562c67 extrakeys: Add xonly_pubkey with serialize, parse and from_pubkey (Jonas Nick)
47e6618e11813cfabe91f0909ca031f960cb7dd4 extrakeys: Init empty experimental module (Jonas Nick)
3e08b02e2a78f2a1fc457efab665db8ab8085373 Make the secp256k1_declassify argument constant (Jonas Nick)
Pull request description:
This PR implements signing, verification and batch verification as described in [BIP-340](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki) in an experimental module named `schnorrsig`. It includes the test vectors and a benchmarking tool.
This PR also adds a module `extrakeys` that allows [BIP-341](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki)-style key tweaking.
(Adding ChaCha20 as a CSPRNG and batch verification was moved to PR #760).
In order to enable the module run `./configure` with `--enable-experimental --enable-module-schnorrsig`.
Based on apoelstra's work.
ACKs for top commit:
gmaxwell:
ACK f431b3f28ac95a3645ad5a6dc96b878fa30a1de3 (exactly matches the previous post-fixup version which I have already reviewed and tested)
sipa:
ACK f431b3f28ac95a3645ad5a6dc96b878fa30a1de3
real-or-random:
ACK f431b3f28ac95a3645ad5a6dc96b878fa30a1de3 careful code review
Tree-SHA512: e15e849c7bb65cdc5d7b1d6874678e275a71e4514de9d5432ec1700de3ba92aa9f381915813f4729057af152d90eea26aabb976ed297019c5767e59cf0bbc693
47a7b8382fd6f1458d859b315cf3bcd3b9790b68 Clear field elements when writing infinity (Elichai Turkel)
61d1ecb02847be9d65ffe9df2d2408d85f3a0711 Added test with additions resulting in infinity (Elichai Turkel)
Pull request description:
Currently if `secp256k1_gej_add_var` / `secp256k1_gej_add_ge_var` /` secp256k1_gej_add_zinv_var` receive `P + (-P)` it will set `gej->infinity = 1` but doesn't call initialize the field elements.
Notice that this is the only branch in the function that results in an uninitialized output.
By using `secp256k1_gej_set_infinity()` it will set the field elements to zero while also setting the infinity flag.
I also added a test that fails with valgrind on current master but passes with the fix.
EDIT: This isn't a bug or something necessary, I just personally found this helpful.
ACKs for top commit:
real-or-random:
ACK 47a7b8382fd6f1458d859b315cf3bcd3b9790b68
Tree-SHA512: cdc2efc242a1b04b4f081183c07d4b2602cdba705e6b30b548df4e115e54fb97691f4b1a28f142f02d5e523c020721337a297b17d732acde147b910f5c53bd0a
60f7f2de5de917c2bee32a4cd79cc3818b6a94a0 Don't assume that ALIGNMENT > 1 in tests (Tim Ruffing)
ada6361dece4265823478e0019a8c196e9285a26 Use ROUND_TO_ALIGN in scratch_create (Jonas Nick)
8ecc6ce50ead28a0b8bab2f1fb18a58ee5204a13 Add check preventing rounding to alignment from wrapping around in scratch_alloc (Jonas Nick)
4edaf06fb02a9ac9cd115e0c967bb0ef35cae01d Add check preventing integer multiplication wrapping around in scratch_max_allocation (Jonas Nick)
Pull request description:
This PR increases the general robustness of scratch spaces. It does not fix an existing vulnerability because scratch spaces aren't used anywhere in master. Additionally, it must be prevented anyway that an attacker has (indirect) control over the arguments touched in this PR.
ACKs for top commit:
sipa:
ACK 60f7f2de5de917c2bee32a4cd79cc3818b6a94a0
Tree-SHA512: ecdd794b55a01d1d6d24098f3abff34cb8bb6f33737ec4ec93714aa631c9d397b213cc3603a916ad79f4b09d6b2f8973bf87fc07b81b25a530cc72c4dbafaba9
37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 Remove unnecessary sign variable from wnaf_const (Jonas Nick)
6bb0b77e158fc2f9e56e4b65b08bcb660d4c588b Fix test_constant_wnaf for -1 and add a test for it. (Jonas Nick)
Pull request description:
There currently is a single branch in the `ecmul_const` function that is not being exercised by the tests. This branch is unreachable and therefore I'm suggesting to remove it.
For your convenience the paper the wnaf algorithm can be found [here (The Width-w NAF Method Provides Small Memory and Fast Elliptic Scalar Multiplications Secure against Side Channel Attacks)](http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.563.1267&rep=rep1&type=pdf). Similarly, unless I'm missing something important, I don't see how their algorithm needs to consider `sign(u[i-1])` unless `d` can be negative - which doesn't make much sense to me either.
ACKs for top commit:
real-or-random:
ACK 37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 I verified the correctness of the change and claimed invariant by manual inspection. I tested the code, both with 32bit and 64bit scalars.
Tree-SHA512: 9db45f76bd881d00a81923b6d2ae1c3e0f49a82a5d55347f01e1ce4e924d9a3bf55483a0697f25039c327e33edca6796ba3205c068d9f2f99aa5d655e46b15be
Before, test_constant_wnaf used scalar_cadd_bit to correct for the skew. But
this function does not correctly deal with overflows which is why num = -1
couldn't be tested.
This commit also adds tests for 0, 1/2 and 1/2-1 as they are corner cases
in constant_wnaf.
There were several places where the code was non-constant time
for invalid secret inputs. These are harmless under sane use
but get in the way of automatic const-time validation.
(Nonce overflow in signing is not addressed, nor is s==0 in
signing)
