Tim Ruffing [Wed, 29 Jul 2020 13:18:30 +0000 (15:18 +0200)]
Merge #778: secp256k1_gej_double_nonzero supports infinity
18d36327fddad18ba81af2cf7fe6c8a16952dc22 secp256k1_gej_double_nonzero supports infinity (Pieter Wuille)
Pull request description:
Our existing function `secp256k1_gej_double_nonzero` actually supports infinity if only it wouldn't check that the input isn't infinity.
Drop the check, rename it to `secp256k1_gej_double`, and adapt the tests.
ACKs for top commit:
real-or-random:
ACK
18d36327fddad18ba81af2cf7fe6c8a16952dc22 I looked at the diff and ran tests locally
gmaxwell:
ACK
18d36327fddad18ba81af2cf7fe6c8a16952dc22
Tree-SHA512: 79dc42099c318f0bdfe7961495ab3fbbe87551c3cc373557a371914bb65638b129ddfd360e694959349f184e2d71a540abdbef04211e7eb70ee17b691632b915
Jonas Nick [Wed, 29 Jul 2020 13:06:12 +0000 (13:06 +0000)]
Merge #779: travis: Fix argument quoting for ./configure
9e49a9b2552b7b865ebc43cfd13c9767de65cb4b travis: Fix argument quoting for ./configure (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
ACK
9e49a9b2552b7b865ebc43cfd13c9767de65cb4b
Tree-SHA512: 53efa7134de978912d604bc9685bc779f98e2d72e5f77636595676aa420c04fc934a6bb9d560d74b58197943ab86708d3b913e79bc3dfb856681b26dda8724b3
Tim Ruffing [Wed, 29 Jul 2020 06:50:42 +0000 (08:50 +0200)]
travis: Fix argument quoting for ./configure
When $USE_HOST or $EXTRAFLAGS are empty, we pass (due to quoting) an
empty string as a parameter to ./configure, which then believes we want
to use a deprecated syntax for specifing a host or a target and yells at us:
> configure: WARNING: you should use --build, --host, --target
The fixes are:
- $EXTRAFLAGS could contain multiple flags and should not be quoted at all.
- We can get rid of $USE_HOST by specifying --host="$HOST" directly.
Pieter Wuille [Wed, 29 Jul 2020 01:12:14 +0000 (18:12 -0700)]
secp256k1_gej_double_nonzero supports infinity
Tim Ruffing [Tue, 28 Jul 2020 14:10:58 +0000 (16:10 +0200)]
Merge #772: Improve constant-timeness on PowerPC
67a429f31fd3d1b37c5365cc58b70588b8645d62 Suppress a harmless variable-time optimization by clang in _int_cmov (Tim Ruffing)
5b196338f0c8dc07bf0eece37b46d8686c4da3ce Remove redundant "? 1 : 0" after comparisons in scalar code (Tim Ruffing)
Pull request description:
Attempt at resolving #771 .
This surprisingly seems to improve the situation at least for the compilers available on godbolt.
ACKs for top commit:
gmaxwell:
ACK
67a429f31fd3d1b37c5365cc58b70588b8645d62
elichai:
tACK
67a429f31fd3d1b37c5365cc58b70588b8645d62
Tree-SHA512: ee8b0c86831ec8c3d5a9abcad773ed8a0f267e5c47012e4e1423b10a64c26b4cf6e3c466c3df765ba7e636787a3fe134d633926d67b599287f12c51be924f478
Tim Ruffing [Tue, 28 Jul 2020 10:34:35 +0000 (12:34 +0200)]
Merge #774: tests: Abort if malloc() fails during context cloning tests
2e1b9e0458317d03b682c1f5dd63aedb52c86b04 tests: Abort if malloc() fails during context cloning tests (Tim Ruffing)
Pull request description:
Found by the clang static analyzer.
This is the worst true positive that it found. I feel somewhat proud.
ACKs for top commit:
elichai:
tACK
2e1b9e0458317d03b682c1f5dd63aedb52c86b04
Tree-SHA512: bf9a3b6c2b8beaafd230ece00a9a69dd884a35b6d2243502ebfded3f77a454e80ef922791bd48c17aa4814a275550957071c045912080a616dd5ed704a70aab7
Tim Ruffing [Mon, 27 Jul 2020 11:43:28 +0000 (13:43 +0200)]
tests: Abort if malloc() fails during context cloning tests
Found by the clang static analyzer.
This is the worst true positive that it found. I feel somewhat proud.
Tim Ruffing [Mon, 27 Jul 2020 12:35:05 +0000 (14:35 +0200)]
Suppress a harmless variable-time optimization by clang in _int_cmov
Follow up on
52a03512c1d800603b5c923c1a28bdba12dadb30
Tim Ruffing [Fri, 24 Jul 2020 22:28:10 +0000 (00:28 +0200)]
Remove redundant "? 1 : 0" after comparisons in scalar code
This prevents GCC from generating branches on PowerPC in certain
cases.
Fixes #771.
Tim Ruffing [Sun, 26 Jul 2020 10:18:17 +0000 (12:18 +0200)]
Merge #741: Remove unnecessary sign variable from wnaf_const
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
Tim Ruffing [Sun, 26 Jul 2020 09:05:08 +0000 (11:05 +0200)]
Merge #773: Fix some compile problems on weird/old compilers.
1309c03c45beece646a7d21fdb6a0e3d38adee2b Fix some compile problems on weird/old compilers. (Gregory Maxwell)
Pull request description:
The visibility attribute is a GCC 4+ feature.
GCC 2.95 also warns about the unsigned/signed comparision.
ACKs for top commit:
real-or-random:
ACK
1309c03c45beece646a7d21fdb6a0e3d38adee2b I inspected the diff
Tree-SHA512: b5a5175416b67b2619f68ad82a208052ad678955e59c2f3457799abd1dd6fd817c40f6bc2941b2bda207c6f58ad0fbe46221a2f92b726e824702c4c0b177377c
Gregory Maxwell [Sun, 26 Jul 2020 05:25:14 +0000 (05:25 +0000)]
Fix some compile problems on weird/old compilers.
The visibility attribute is a GCC 4+ feature.
GCC 2.95 also warns about the unsigned/signed comparision.
Jonas Nick [Tue, 21 Jul 2020 19:12:43 +0000 (19:12 +0000)]
Merge #769: Undef HAVE___INT128 in basic-config.h to fix gen_context compilation
22e578bb11fe62d3e8ac05b5278a076bf7f2fa2e Undef HAVE___INT128 in basic-config.h to fix gen_context compilation (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
ACK
22e578bb11fe62d3e8ac05b5278a076bf7f2fa2e
Tree-SHA512: 91e11c3feade13923a01c30025b7f01d0cb6d7d88cd7a19d490373d2fb4552f2ca1ab0d9138096268999bcbfd51ef3c9af64ec8ab0dc8ee2fa60be16d2b5af64
Tim Ruffing [Tue, 21 Jul 2020 09:09:23 +0000 (11:09 +0200)]
Undef HAVE___INT128 in basic-config.h to fix gen_context compilation
Fixes #768.
Jonas Nick [Mon, 29 Jun 2020 08:38:20 +0000 (08:38 +0000)]
Merge #765: remove dead store in ecdsa_signature_parse_der_lax
f00d6575ca0dcca11e085b20afa4d73dc8742ddc remove dead store in ecdsa_signature_parse_der_lax (fanquake)
Pull request description:
ACKs for top commit:
elichai:
utACK
f00d6575ca0dcca11e085b20afa4d73dc8742ddc, it does look like we don't use that assignment
jonasnick:
ACK
f00d6575ca0dcca11e085b20afa4d73dc8742ddc
Tree-SHA512: 9aa54c901f299341c309411b0247720f5152a131dd346c19be7ee21865e3a822e8cf91b869e28ef6288adaf31660bc2e18874e304052468a9be6b7027674af30
fanquake [Mon, 29 Jun 2020 05:17:24 +0000 (13:17 +0800)]
remove dead store in ecdsa_signature_parse_der_lax
This change was made in bitcoin/bitcoin without upstreaming. So this is
a followup to the comment here:
https://github.com/bitcoin/bitcoin/pull/19228#issuecomment-
641795558.
See also: https://github.com/bitcoin/bitcoin/pull/11073.
Tim Ruffing [Mon, 15 Jun 2020 14:00:10 +0000 (16:00 +0200)]
Merge #759: Fix uninitialized variables in ecmult_multi test
2e7fc5b5372067ecd33b2304e8c88ed6de98ff13 Fix uninitialized variables in ecmult_multi test (Jonas Nick)
Pull request description:
Fixes #756
ACKs for top commit:
real-or-random:
ACK
2e7fc5b5372067ecd33b2304e8c88ed6de98ff13 I inspected the diff. I did not test it and I did not check whether if makes the warning go away
elichai:
tACK
2e7fc5b5372067ecd33b2304e8c88ed6de98ff13
Tree-SHA512: 674400134f5487236f5b6e8b3020b346d43662511628cdf6dd1bd7ba1de985bf93f5be11f5650f250ff37b5f87eb4b01d90ed53d41193c05a420d3f5a2d63470
Jonas Nick [Mon, 15 Jun 2020 09:02:14 +0000 (09:02 +0000)]
Fix uninitialized variables in ecmult_multi test
Tim Ruffing [Mon, 8 Jun 2020 13:44:06 +0000 (15:44 +0200)]
Merge #755: Recovery signing: add to constant time test, and eliminate non ct operators
28609507e70a172dd8f39de4aa55f851452fc0b4 Add tests for the cmov implementations (Elichai Turkel)
73596a85a2ab9c885e58a7a2a8876355a6ae68e4 Add ecdsa_sign_recoverable to the ctime tests (Elichai Turkel)
2876af4f8da952e39c06bc229d68cd4892ea2c85 Split ecdsa_sign logic into a new function and use it from ecdsa_sign and recovery (Elichai Turkel)
Pull request description:
Hi,
The recovery module was overlooked in #708 and #710, so this adds it to the `valgrind_ctime_test` and replaces the secret dependent branching with the cmovs,
I created a new function `secp256k1_ecdsa_sign_inner` (feel free to bikeshed) which does the logic both for ecdsa_sign and for ecdsa_sign_recoverable, such that next time when things get changed/improved in ecdsa it will affect the recoverable signing too.
ACKs for top commit:
jonasnick:
ACK
28609507e70a172dd8f39de4aa55f851452fc0b4
real-or-random:
ACK
28609507e70a172dd8f39de4aa55f851452fc0b4 read the diff, tested with valgrind including ctime tests
Tree-SHA512: 4730301dcb62241d79f18eb8fed7e9ab0e20d1663a788832cb6cf4126baa7075807dc31896764b6f82d52742fdb636abc6b75e4344c6f117305904c628a5ad59
Elichai Turkel [Sun, 31 May 2020 11:14:05 +0000 (14:14 +0300)]
Add tests for the cmov implementations
Elichai Turkel [Tue, 26 May 2020 21:38:46 +0000 (00:38 +0300)]
Add ecdsa_sign_recoverable to the ctime tests
Elichai Turkel [Tue, 26 May 2020 21:37:59 +0000 (00:37 +0300)]
Split ecdsa_sign logic into a new function and use it from ecdsa_sign and recovery
Tim Ruffing [Tue, 2 Jun 2020 16:03:42 +0000 (18:03 +0200)]
Merge #754: Fix uninit values passed into cmov
f79a7adcf555ccc78b591850ea15c64fbfbca152 Add valgrind uninit check to cmovs output (Elichai Turkel)
a39c2b09de304b8f24716b59219ae37c2538c242 Fixed UB(arithmetics on uninit values) in cmovs (Elichai Turkel)
Pull request description:
This should fix #753.
Used @peterdettman's solution here for the `ECMULT_CONST_TABLE_GET_GE` https://github.com/bitcoin-core/secp256k1/issues/753#issuecomment-
631316091
and in ecdsa_sign I initialize `s` and `r` to a zero scalar.
The second commit adds a valgrind check to the cmovs that could've caught this (in ecdsa_sign, not in ecmult_const because there's a scalar clear there under `VERIFY_SETUP`)
ACKs for top commit:
sipa:
utACK
f79a7adcf555ccc78b591850ea15c64fbfbca152
jonasnick:
ACK
f79a7adcf555ccc78b591850ea15c64fbfbca152
real-or-random:
ACK
f79a7adcf555ccc78b591850ea15c64fbfbca152
Tree-SHA512: 6fd7b7c84f392bda733a973f4dcfc12bf1478aac2591e2c87b69e637847d3b063c4243cc8feccaffc3a5824c18183a5e66bd4251c2322abaf63bb6439b38defe
Elichai Turkel [Wed, 20 May 2020 12:12:09 +0000 (15:12 +0300)]
Add valgrind uninit check to cmovs output
Tim Ruffing [Fri, 22 May 2020 11:30:25 +0000 (13:30 +0200)]
Merge #752: autoconf: Use ":" instead of "dnl" as a noop
5e8747ae2a0c915d079837d238f8b84841a4ce5c autoconf: Use ":" instead of "dnl" as a noop (Tim Ruffing)
Pull request description:
Fixes #424.
Top commit has no ACKs.
Tree-SHA512: a83664afbc6ca1254c4767161bfbec82f3489a8a248ba7a5a46ed9ec2a39232cf92f504accadd4dbb1a6ea4791dbf7f0e1f030e51f02f49eb9a38a2e509ee6c2
Elichai Turkel [Wed, 20 May 2020 12:09:13 +0000 (15:09 +0300)]
Fixed UB(arithmetics on uninit values) in cmovs
Jonas Nick [Mon, 18 May 2020 19:38:41 +0000 (19:38 +0000)]
Merge #750: Add macOS to the CI
71757da5ccece100b1eca6c70b4d87e2542cff97 Explictly pass SECP256K1_BENCH_ITERS to the benchmarks in travis.sh (Elichai Turkel)
99bd661d71fe17be7b3de4707a0264c41a63ebe8 Replace travis_wait with a loop printing "\a" to stdout every minute (Elichai Turkel)
bc818b160cdaaccff2162206cc15915fa5f0cca8 Bump travis Ubuntu from xenial(16.04) to bionic(18.04) (Elichai Turkel)
0c5ff9066e6fa41b1fbd5d0b8c2f02e8a04e96ea Add macOS support to travis (Elichai Turkel)
b6807d91d83e9597ffecec999eb761b8571a1f26 Move travis script into a standalone sh file (Elichai Turkel)
Pull request description:
ACKs for top commit:
real-or-random:
ACK
71757da5ccece100b1eca6c70b4d87e2542cff97 I inspected the diff
jonasnick:
ACK
71757da5ccece100b1eca6c70b4d87e2542cff97
Tree-SHA512: e8fab725ef5ed98c795f39d7f26b5d967a6bd730d40eb7d9793986858bf34770b0350c1b7b1d14ae608dfff9375a0750ec67c8e6d0d4b562ab917f5e645aa67b
Tim Ruffing [Mon, 18 May 2020 10:27:14 +0000 (12:27 +0200)]
autoconf: Use ":" instead of "dnl" as a noop
Fixes #424.
Elichai Turkel [Thu, 7 May 2020 13:07:37 +0000 (16:07 +0300)]
Explictly pass SECP256K1_BENCH_ITERS to the benchmarks in travis.sh
Elichai Turkel [Sun, 3 May 2020 15:01:28 +0000 (18:01 +0300)]
Replace travis_wait with a loop printing "\a" to stdout every minute
Elichai Turkel [Sat, 2 May 2020 19:06:46 +0000 (22:06 +0300)]
Bump travis Ubuntu from xenial(16.04) to bionic(18.04)
Elichai Turkel [Sat, 2 May 2020 19:06:04 +0000 (22:06 +0300)]
Add macOS support to travis
Elichai Turkel [Sat, 2 May 2020 18:58:42 +0000 (21:58 +0300)]
Move travis script into a standalone sh file
Tim Ruffing [Thu, 30 Apr 2020 16:11:53 +0000 (18:11 +0200)]
Merge #701: Make ec_ arithmetic more consistent and add documentation
7e3952ae82af2a98619de74614f2d3f0ff072941 Clarify documentation of tweak functions. (Jonas Nick)
89853a0f2e2b324bd32e78641aeaad6d1e427e81 Make tweak function documentation more consistent. (Jonas Nick)
41fc78560223aa035d9fe2cbeedb3ee632c740e2 Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul (Jonas Nick)
22911ee6da7197c45226ab4bcd84b8c2773baf9f Rename private key to secret key in public API (with the exception of function names) (Jonas Nick)
5a73f14d6cfab043f94b2032d6958e15f84065fe Mention that value is unspecified for In/Out parameters if the function returns 0 (Jonas Nick)
f03df0e6d7d272808c9d27df40bb92716b341d03 Define valid ECDSA keys in the documentation of seckey_verify (Jonas Nick)
5894e1f1df74b23435cfd1e9a8b25f22ac631ebe Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul (Jonas Nick)
8f814cddb94f4018646569143d10ebbb98daa454 Add test for boundary conditions of scalar_set_b32 with respect to overflows (Jonas Nick)
3fec9826086aa45ebbac1ff6fc3bb7b25ca78b1d Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify (Jonas Nick)
9ab2cbe0ebf8dfafd5499ea7a79c35f0d0cfe5e2 Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key (Jonas Nick)
Pull request description:
Fixes #671. Supersedes #668.
This PR unifies handling of invalid secret keys by introducing a new function `scalar_set_b32_secret` which returns false if the b32 overflows or is 0. By using this in `privkey_{negate, tweak_add, tweak_mul}` these function will now return 0 if the secret key is invalid which matches the behavior of `ecdsa_sign` and `pubkey_create`.
Instead of deciding whether to zeroize the secret key on failure, I only added documentation for now that the value is undefined on failure.
ACKs for top commit:
real-or-random:
ACK
7e3952ae82af2a98619de74614f2d3f0ff072941 I read the diff carefully and tested the changes
apoelstra:
ACK
7e3952ae82af2a98619de74614f2d3f0ff072941
Tree-SHA512: 8e9a66799cd3b6ec1c3acb731d6778035417e3dca9300d840e2437346ff0ac94f0c9be4de20aa2fac9bb4ae2f8a36d4e6a34795a640b9cfbfee8311decb102f0
Jonas Nick [Fri, 17 Apr 2020 18:06:47 +0000 (18:06 +0000)]
Remove unnecessary sign variable from wnaf_const
Jonas Nick [Fri, 17 Apr 2020 18:05:50 +0000 (18:05 +0000)]
Fix test_constant_wnaf for -1 and add a test for it.
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.
Jonas Nick [Sat, 18 Apr 2020 12:22:39 +0000 (12:22 +0000)]
Merge #732: Retry if r is zero during signing
37ed51a7eafa5fd9955ceb66213251cdd8b8b8c4 Make ecdsa_sig_sign constant-time again after reverting
25e3cfb (Tim Ruffing)
93d343bfc5323e56f6a60cb41d60b96368cc09c7 Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign" (Tim Ruffing)
Pull request description:
ACKs for top commit:
elichai:
ACK
37ed51a7eafa5fd9955ceb66213251cdd8b8b8c4 makes sense.
jonasnick:
ACK
37ed51a7eafa5fd9955ceb66213251cdd8b8b8c4
Tree-SHA512: 82b5b8e29f48e84fd7a0681b62923d3bd87d724b38ef18e8c7969b0dcc5a405ebb26c14b5c5f4c7ba0ccabd152d1531d217809d1daf40872fe0c1e079b55c64b
Tim Ruffing [Sat, 18 Apr 2020 11:19:47 +0000 (13:19 +0200)]
Merge #742: Fix typo in ecmult_const_impl.h
4e284655d99c8f0ee030aa29bcd036e5e5bd2a40 Fix typo in ecmult_const_impl.h (f-daniel)
Pull request description:
Fix small typo in the reference given for the wNAF method ( `secp256k1_wnaf_const`)
ACKs for top commit:
real-or-random:
ACK
4e284655d99c8f0ee030aa29bcd036e5e5bd2a40 trivial
Tree-SHA512: d6c3daa0384fc1bba36a46933641c97661b18e88c711343e2c8f91f39aa707eddd0ba77c8d5c43aaead883eeed7f4458ed1dec228d692d713572231aa6010fb0
f-daniel [Sat, 18 Apr 2020 10:53:06 +0000 (12:53 +0200)]
Fix typo in ecmult_const_impl.h
Fix small typo in the reference given for the wNAF method
Tim Ruffing [Wed, 15 Apr 2020 20:37:08 +0000 (22:37 +0200)]
Merge #740: Make recovery/main_impl.h non-executable
ffef45c98a55934ab555ecf96f0b19d0e4cfe830 Make recovery/main_impl.h non-executable (Elichai Turkel)
Pull request description:
Opened it because of https://github.com/bitcoin/bitcoin/pull/18650
I assume it doesn't matter?
But because I'm not sure I preferred to open this then let the info go away in case someone thinks it does matter.
ACKs for top commit:
real-or-random:
ACK
ffef45c98a55934ab555ecf96f0b19d0e4cfe830
Tree-SHA512: 381aed7f99fd739f4059b2e526ba9cd75b55b4fa86c9cc040fbf6b93055ce8558cc69c4ccf5d8a422b17022ca376cc9a608cf5af8d5841d62c5953f40825f5ff
Elichai Turkel [Wed, 15 Apr 2020 20:14:06 +0000 (23:14 +0300)]
Make recovery/main_impl.h non-executable
Jonas Nick [Mon, 13 Apr 2020 19:52:01 +0000 (19:52 +0000)]
Merge #735: build: fix OpenSSL EC detection on macOS
3b7d26b23c9811deee244ede7bb344bb43544153 build: add SECP_TEST_INCLUDES to bench_verify CPPFLAGS (fanquake)
84b5fc5bc34cfb6e9df4f06b3b6fc98c3373dd0e build: fix OpenSSL EC detection on macOS (fanquake)
Pull request description:
ACKs for top commit:
real-or-random:
ACK
3b7d26b23c9811deee244ede7bb344bb43544153 the diff looks good to me
jonasnick:
ACK
3b7d26b23c9811deee244ede7bb344bb43544153
Tree-SHA512: 381aed7f99fd739f4059b2e526ba9cd75b55b4fa86c9cc040fbf6b93055ce8558cc69c4ccf5d8a422b17022ca376cc9a608cf5af8d5841d62c5953f40825f5ff
fanquake [Thu, 9 Apr 2020 09:22:56 +0000 (17:22 +0800)]
build: add SECP_TEST_INCLUDES to bench_verify CPPFLAGS
This is needed so that bench_verify gets CRYPTO_CPPFLAGS, which would
otherwise not be included, at least on macOS.
fanquake [Thu, 9 Apr 2020 07:55:11 +0000 (15:55 +0800)]
build: fix OpenSSL EC detection on macOS
Tim Ruffing [Tue, 31 Mar 2020 12:57:19 +0000 (14:57 +0200)]
Make ecdsa_sig_sign constant-time again after reverting
25e3cfb
Tim Ruffing [Tue, 31 Mar 2020 12:28:48 +0000 (14:28 +0200)]
Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign"
This reverts commit
25e3cfbf9b52d2f5afa543f967a73aa8850d2038. The reverted
commit was probably based on the assumption that this is about the touched
checks cover the secret nonce k instead of r, which is the x-coord of the public
nonce. A signature with a zero r is invalid by the spec, so we should return 0
to make the caller retry with a different nonce. Overflow is not an issue.
Fixes #720.
Jonas Nick [Fri, 20 Mar 2020 14:14:11 +0000 (14:14 +0000)]
Clarify documentation of tweak functions.
In particular, mention that the functions return 0 if seckey or tweak are
invalid (as opposed to saying "should" or "must" be valid).
Jonas Nick [Thu, 19 Mar 2020 21:52:37 +0000 (21:52 +0000)]
Make tweak function documentation more consistent.
Do this by adding a newline after the first sentence and aligning the rest.
Jonas Nick [Thu, 19 Dec 2019 15:02:29 +0000 (15:02 +0000)]
Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul
Jonas Nick [Tue, 17 Dec 2019 17:10:11 +0000 (17:10 +0000)]
Rename private key to secret key in public API (with the exception of function names)
Jonas Nick [Tue, 17 Dec 2019 17:06:03 +0000 (17:06 +0000)]
Mention that value is unspecified for In/Out parameters if the function returns 0
Jonas Nick [Tue, 17 Dec 2019 17:05:42 +0000 (17:05 +0000)]
Define valid ECDSA keys in the documentation of seckey_verify
Jonas Nick [Tue, 17 Dec 2019 16:52:07 +0000 (16:52 +0000)]
Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul
Jonas Nick [Tue, 8 Oct 2019 09:11:16 +0000 (09:11 +0000)]
Add test for boundary conditions of scalar_set_b32 with respect to overflows
Jonas Nick [Tue, 17 Dec 2019 15:56:09 +0000 (15:56 +0000)]
Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify
Jonas Nick [Tue, 17 Dec 2019 15:32:00 +0000 (15:32 +0000)]
Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key
Jonas Nick [Fri, 27 Mar 2020 18:09:07 +0000 (18:09 +0000)]
Merge #728: Suppress a harmless variable-time optimization by clang in memczero
01993878bb2e7f24f42dac84d6949242143bb7f8 Add test for memczero() (Tim Ruffing)
52a03512c1d800603b5c923c1a28bdba12dadb30 Suppress a harmless variable-time optimization by clang in memczero (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
ACK
01993878bb2e7f24f42dac84d6949242143bb7f8
Tree-SHA512: ed385f6e3909299b9979254bd0208a9d0c68caa9f57039e392aa0d5424ed8002c13f8c037bfbd2697c2f6b54af5731209eba9725c5e88be3ee3077c159d134e2
Tim Ruffing [Fri, 27 Mar 2020 09:54:09 +0000 (10:54 +0100)]
Add test for memczero()
Tim Ruffing [Wed, 25 Mar 2020 15:04:49 +0000 (16:04 +0100)]
Suppress a harmless variable-time optimization by clang in memczero
This has been not been caught by the new constant-time tests because
valgrind currently gives us a zero exit code even if finds errors, see
https://github.com/bitcoin-core/secp256k1/pull/723#discussion_r388246806 .
This commit also simplifies the arithmetic in memczero.
Note that the timing leak here was the bit whether a secret key was
out of range. This leak is harmless and not exploitable. It is just
our overcautious practice to prefer constant-time code even here.
Jonas Nick [Tue, 24 Mar 2020 15:53:24 +0000 (15:53 +0000)]
Merge #722: Context isn't freed in the ECDH benchmark
85b35afa765ba23ac3682cf15800769b0a3b834d Add running benchmarks regularly and under valgrind in travis (Elichai Turkel)
ca4906b02e69644d83e13b3c09cc9147fc1232a5 Pass num of iters to benchmarks as variable, and define envvar (Elichai Turkel)
02dd5f1bbb3af3660ecff276c3a108371979b67c free the ctx at the end of bench_ecdh (Elichai Turkel)
Pull request description:
ACKs for top commit:
real-or-random:
ACK
85b35afa765ba23ac3682cf15800769b0a3b834d I looked at the diff and tested the ecdh benchmark using valgrind
jonasnick:
ACK
85b35afa765ba23ac3682cf15800769b0a3b834d
Tree-SHA512: 029456d2c8f6c2c45c689279683ae30b067872bbfaee76a657f7fc3a059e2dffcde09ed29e3610de3adb055405126b6c3656c7ab5f5aaa1f45af2b32d4693ec4
Tim Ruffing [Fri, 20 Mar 2020 15:48:45 +0000 (16:48 +0100)]
Merge #700: Allow overriding default flags
ca739cba238cdd7c513abfc719b0b0eb957c9458 Compile with optimization flag -O2 by default instead of -O3 (Jonas Nick)
83fb1bcef49b1c12ef349f62d90bfcc83f0f7398 Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual) (Jonas Nick)
ecba8138ec163f5ad4c303df9f8744810a3dfc03 Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables (Jonas Nick)
613c34cd869e56dee2ea5fb701f05b07da7069c8 Remove test in configure.ac because it doesn't have an effect (Jonas Nick)
Pull request description:
Right now, it's not easy to reduce the optimization level with `CFLAGS` because `configure` overwrites any optimization flag with `-O3`. The [automake documentation](https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html) states that:
> The reason ‘$(CPPFLAGS)’ appears after ‘$(AM_CPPFLAGS)’ or ‘$(mumble_CPPFLAGS)’ in the compile command is that users should always have the last say.
and also that it's incorrect to redefine CFLAGS in the first place
> You should never redefine a user variable such as CPPFLAGS in Makefile.am. [...] You should not add options to these user variables within configure either, for the same reason
With this PR `CFLAGS` is still redefined, but user-provided flags appear after the default `CFLAGS` which means that they override the default flags (at least in clang and gcc). Otherwise, the default configuration is not changed. This also means that if CFLAGS are defined by the user, then -g is not added (which does not seem to make much sense). In order to keep the `-O3` despite the reordering we need to explicitly tell autoconf to not append `-O2` by setting the default to `-g` with `: ${CFLAGS="-g"}` as per [the manual](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#C-Compiler) (EDIT: link fix).
ACKs for top commit:
real-or-random:
ACK
ca739cba238cdd7c513abfc719b0b0eb957c9458
theuni:
ACK
ca739cba238cdd7c513abfc719b0b0eb957c9458.
elichai:
ACK
ca739cba238cdd7c513abfc719b0b0eb957c9458
Tree-SHA512: be92589faa461d245203385d44b489c7d6917b0c68472b8d7576806c0250cf5ff61d5c99ce04eebb8ff5279b9987185d4e5d2da979683fb1c489fdf3e5b59630
Elichai Turkel [Wed, 4 Mar 2020 14:35:31 +0000 (16:35 +0200)]
Add running benchmarks regularly and under valgrind in travis
Elichai Turkel [Wed, 4 Mar 2020 13:13:35 +0000 (15:13 +0200)]
Pass num of iters to benchmarks as variable, and define envvar
Elichai Turkel [Wed, 4 Mar 2020 12:14:51 +0000 (14:14 +0200)]
free the ctx at the end of bench_ecdh
Tim Ruffing [Tue, 3 Mar 2020 15:49:20 +0000 (16:49 +0100)]
Merge #708: Constant-time behaviour test using valgrind memtest.
08fb6c49261f8aefaaa8ea2ca6d84a53e037e812 Run valgrind_ctime_test in travis (Jonas Nick)
3d2302257f19533932cd53547e9745b6283a907d Constant-time behaviour test using valgrind memtest. (Gregory Maxwell)
Pull request description:
Valgrind does bit-level tracking of the "uninitialized" status of memory,
property tracks memory which is tainted by any uninitialized memory, and
warns if any branch or array access depends on an uninitialized bit.
That is exactly the verification we need on secret data to test for
constant-time behaviour. All we need to do is tell valgrind our
secret key is actually uninitialized memory.
This adds a valgrind_ctime_test which is compiled if valgrind is installed:
Run it with libtool --mode=execute:
$ libtool --mode=execute valgrind ./valgrind_ctime_test
ACKs for top commit:
sipa:
ACK
08fb6c49261f8aefaaa8ea2ca6d84a53e037e812
real-or-random:
ACK
08fb6c49261f8aefaaa8ea2ca6d84a53e037e812
jonasnick:
ACK
08fb6c49261f8aefaaa8ea2ca6d84a53e037e812
Tree-SHA512: d2eb829fb09f43ad1af70898e0eb9cf3f002c6bc418eca9e3e01a9c2c6e87c092aed23d6b0f311ddccbce1cce5f8ef39162cf9b2e68b83d160bc3d249e881493
Jonas Nick [Wed, 12 Feb 2020 10:20:38 +0000 (10:20 +0000)]
Run valgrind_ctime_test in travis
Gregory Maxwell [Wed, 8 Jan 2020 11:56:15 +0000 (11:56 +0000)]
Constant-time behaviour test using valgrind memtest.
Valgrind does bit-level tracking of the "uninitialized" status of memory,
property tracks memory which is tainted by any uninitialized memory, and
warns if any branch or array access depends on an uninitialized bit.
That is exactly the verification we need on secret data to test for
constant-time behaviour. All we need to do is tell valgrind our
secret key is actually uninitialized memory.
This adds a valgrind_ctime_test which is compiled if valgrind is installed:
Run it with libtool --mode=execute:
$ libtool --mode=execute valgrind ./valgrind_ctime_test
Tim Ruffing [Mon, 24 Feb 2020 13:02:44 +0000 (14:02 +0100)]
Merge #710: Eliminate harmless non-constant time operations on secret data.
7b50483ad789081ba158799e5b94330f62932607 Adds a declassify operation to aid constant-time analysis. (Gregory Maxwell)
34a67c773b0871e5797c7ab506d004e80911f120 Eliminate harmless non-constant time operations on secret data. (Gregory Maxwell)
Pull request description:
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)
ACKs for top commit:
sipa:
utACK
7b50483ad789081ba158799e5b94330f62932607
real-or-random:
ACK
7b50483ad789081ba158799e5b94330f62932607 I read the code carefully and tested it
jonasnick:
reACK
7b50483ad789081ba158799e5b94330f62932607
Tree-SHA512: 0776c3a86e723d2f97b9b9cb31d0d0e59dfcf308093b3f46fbc859f73f9957f3fa977d03b57727232040368d058701ef107838f9b1ec98f925ec78ddad495c4e
Tim Ruffing [Sun, 23 Feb 2020 08:20:47 +0000 (09:20 +0100)]
Merge #718: Clarify that a secp256k1_ecdh_hash_function must return 0 or 1
eb45ef33842ead425137d589521dc231ee92a10d Clarify that a secp256k1_ecdh_hash_function must return 0 or 1 (Tim Ruffing)
Pull request description:
and improve style of the ECDH docs.
ACKs for top commit:
sipa:
utACK
eb45ef33842ead425137d589521dc231ee92a10d
jonasnick:
ACK
eb45ef33842ead425137d589521dc231ee92a10d
elichai:
ACK
eb45ef33842ead425137d589521dc231ee92a10d
apoelstra:
utACK https://github.com/bitcoin-core/secp256k1/pull/718/commits/
eb45ef33842ead425137d589521dc231ee92a10d
Tree-SHA512: fa1e34fbbe2fd53b633c48c70fbd9d6eec4be1303b660ff87945d49333264ef5c28a4db9407161907697f37ca657a1ee7b50e58861689de526ad4d685dedeae6
Gregory Maxwell [Sat, 11 Jan 2020 13:31:50 +0000 (13:31 +0000)]
Adds a declassify operation to aid constant-time analysis.
ECDSA signing has a retry loop for the exceptionally unlikely case
that S==0. S is not a secret at this point and this case is so
rare that it will never be observed but branching on it will trip
up tools analysing if the code is constant time with respect to
secrets.
Derandomized ECDSA can also loop on k being zero or overflowing,
and while k is a secret these cases are too rare (1:2^255) to
ever observe and are also of no concern.
This adds a function for marking memory as no-longer-secret and
sets it up for use with the valgrind memcheck constant-time
test.
Gregory Maxwell [Sat, 11 Jan 2020 01:01:05 +0000 (01:01 +0000)]
Eliminate harmless non-constant time operations on secret data.
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)
Jonas Nick [Wed, 19 Feb 2020 14:07:54 +0000 (14:07 +0000)]
Compile with optimization flag -O2 by default instead of -O3
Tim Ruffing [Mon, 10 Feb 2020 11:55:30 +0000 (12:55 +0100)]
Clarify that a secp256k1_ecdh_hash_function must return 0 or 1
and improve style of the ECDH docs.
Tim Ruffing [Mon, 10 Feb 2020 11:06:17 +0000 (12:06 +0100)]
Merge #714: doc: document the length requirements of output parameter.
4b48a431060948dc5e29aa590d646a72aa138968 doc: document the length requirements of output parameter. (Rusty Russell)
Pull request description:
It's subtle, since it is actually only touched by hashfp (though
we assert it's non-NULL), but give explicit advice in the default
case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ACKs for top commit:
jonasnick:
ACK
4b48a431060948dc5e29aa590d646a72aa138968
real-or-random:
ACK
4b48a431060948dc5e29aa590d646a72aa138968 diff inspection
Tree-SHA512: d6bedb495e46b27ac9b558e77d814884d782ea78569a2296688eccf374bc880d13846546ad449c2a677865cf6ed56fcbc8be58c21f9daca5084831074e20d769
Tim Ruffing [Mon, 10 Feb 2020 10:59:06 +0000 (11:59 +0100)]
Merge #682: Remove Java Native Interface
642cd062bdd2d28a8a84d4cb6dedbfe435ee5869 Remove Java Native Interface (Jonas Nick)
Pull request description:
This was discussed in #508. The main reasons are that the existing Java Native Interface (JNI) bindings would need way more work to remain useful to Java developers but the maintainers and regular contributors of libsecp are not very familiar with Java (and evidently are motivated enough to improve the situation). We don't know who relies on these bindings with the exception of ACINQ who have their own fork at https://github.com/ACINQ/secp256k1/tree/jni-embed/src/java (@sstone). Bitcoinj can optionally use the libsecp bindings.
Ideally, there would be a separate repository owned by Java developers with just the bindings. Until this exists, Java developers relying on libsecp can use ACINQs fork or an older commit of libsecp.
ACKs for top commit:
real-or-random:
ACK
642cd062bdd2d28a8a84d4cb6dedbfe435ee5869 I read the diff
real-or-random:
ACK
642cd062bdd2d28a8a84d4cb6dedbfe435ee5869 I read the diff, and I verified that the diff to
7d9066a66c0f13cabb0c4f71aca30edd3494f0d5, which has been ACKed by sipa, is only the additonal removal of ax_jni_include_dir.m4
Tree-SHA512: 9e573f2b01897bd5f301707062b41de53424517b537ce0834d9049d003cfd73fa1bcc024b543256016e4c9a1126f7c7fef559b84dc4914083b5a2d0ad5e57ea8
Rusty Russell [Mon, 10 Feb 2020 00:41:11 +0000 (11:11 +1030)]
doc: document the length requirements of output parameter.
It's subtle, since it is actually only touched by hashfp (though
we assert it's non-NULL), but give explicit advice in the default
case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Jonas Nick [Fri, 24 Jan 2020 12:34:50 +0000 (12:34 +0000)]
Merge #713: Docstrings
dabfea7e217b129d10f0f787722626f388dddb5a field: extend docstring of secp256k1_fe_normalize (Marko Bencun)
dc7d8fd9e2576c399a7ed25aa98b0f23ffac6766 scalar: extend docstring of secp256k1_scalar_set_b32 (Marko Bencun)
Pull request description:
ACKs for top commit:
real-or-random:
ACK
dabfea7e217b129d10f0f787722626f388dddb5a
jonasnick:
ACK
dabfea7
Tree-SHA512: aeed17dad281296e46d94007c864ba07f41a347525b049385a0a71640de84c3094bcc51d2fb4132b2a8f575acfe8ae53d7e28790bf328cac1892d040a9c50f70
Marko Bencun [Thu, 16 Jan 2020 15:52:09 +0000 (16:52 +0100)]
field: extend docstring of secp256k1_fe_normalize
Marko Bencun [Thu, 16 Jan 2020 15:48:49 +0000 (16:48 +0100)]
scalar: extend docstring of secp256k1_scalar_set_b32
Pieter Wuille [Wed, 15 Jan 2020 15:55:20 +0000 (07:55 -0800)]
Merge #704: README: add a section for test coverage
acb7f97eb82dfbbdb797354e1550b910055b4422 README: add a section for test coverage (Marko Bencun)
Pull request description:
It is a hassle to figure out the exact commands to create a good
report.
ACKs for top commit:
real-or-random:
ACK
acb7f97eb82dfbbdb797354e1550b910055b4422
sipa:
ACK
acb7f97eb82dfbbdb797354e1550b910055b4422
Tree-SHA512: d39f3e0b289229b2ce085406f6d716fdd54038df9ee5273a18a05140d1eddd4149149e881cc7a13f2126347217b9c56a0c12adf558c49879c5f556695242afc6
Marko Bencun [Sun, 29 Dec 2019 20:52:01 +0000 (21:52 +0100)]
README: add a section for test coverage
It is a hassle to figure out the exact commands to create a good
report.
Pieter Wuille [Tue, 14 Jan 2020 21:24:52 +0000 (13:24 -0800)]
Merge #709: Remove secret-dependant non-constant time operation in ecmult_const.
d567b779fe446fd18820a9d2968ecb703c8dea19 Clarify comments about use of rzr on ge functions and abs function. (Gregory Maxwell)
2241ae6d14df187e2c8d6fe5b44e3d850474af38 Remove secret-dependant non-constant time operation in ecmult_const. (Gregory Maxwell)
Pull request description:
ECMULT_CONST_TABLE_GET_GE was branching on its secret input.
Also makes secp256k1_gej_double_var implemented as a wrapper
on secp256k1_gej_double_nonzero instead of the other way
around. This wasn't a constant time bug but it was fragile
and could easily become one in the future if the double_var
algorithm is changed.
ACKs for top commit:
real-or-random:
ACK
d567b779fe446fd18820a9d2968ecb703c8dea19 I read the diff carefully and tested the code with ECDH enabled and various settings, also on valgrind
sipa:
ACK
d567b779fe446fd18820a9d2968ecb703c8dea19
Tree-SHA512: f00a921dcc6cc024cfb3ac1a34c1be619b96f1f17ec0ee0f3ff4ea02035ee288e55469491ed3183e2c4e5560cc068c10aafb657dff95a610706e5b9a8cd13966
Gregory Maxwell [Thu, 9 Jan 2020 13:07:36 +0000 (13:07 +0000)]
Clarify comments about use of rzr on ge functions and abs function.
Gregory Maxwell [Wed, 8 Jan 2020 14:58:28 +0000 (14:58 +0000)]
Remove secret-dependant non-constant time operation in ecmult_const.
ECMULT_CONST_TABLE_GET_GE was branching on its secret input.
Also makes secp256k1_gej_double_var implemented as a wrapper
on secp256k1_gej_double_nonzero instead of the other way
around. This wasn't a constant time bug but it was fragile
and could easily become one in the future if the double_var
algorithm is changed.
Jonas Nick [Tue, 29 Oct 2019 12:23:32 +0000 (12:23 +0000)]
Remove Java Native Interface
Jonas Nick [Tue, 17 Dec 2019 12:41:44 +0000 (12:41 +0000)]
Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual)
Jonas Nick [Tue, 17 Dec 2019 12:37:48 +0000 (12:37 +0000)]
Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables
Jonas Nick [Tue, 17 Dec 2019 12:34:26 +0000 (12:34 +0000)]
Remove test in configure.ac because it doesn't have an effect
Pieter Wuille [Sun, 29 Dec 2019 15:00:39 +0000 (07:00 -0800)]
Merge #703: Overhaul README.md
2e759ec753446aab0272ba32c5f1b7dc3a4dc75c Overhaul README.md (Tim Ruffing)
Pull request description:
* Update feature list
* Be more positive about the state and quality of the library
* Mention ECDSA key operations explicitly in short library description
* Say "secret key" instead of "private key"
cc @gmaxwell who suggested a similar wording for the disclaimer.
ACKs for top commit:
sipa:
ACK
2e759ec753446aab0272ba32c5f1b7dc3a4dc75c
jonasnick:
ACK
2e759ec753446aab0272ba32c5f1b7dc3a4dc75c
Tree-SHA512: 2e1c87e7fa28d9dab682af227f845e7d48ac79a9fbe10be47ae4567abc2e066ba2f852c000db7d697ece8e4bbeeb851ea647465f870ac29dc3654031bf15a1ad
Tim Ruffing [Fri, 20 Dec 2019 16:25:14 +0000 (17:25 +0100)]
Overhaul README.md
* Update feature list
* Be more positive about the state and quality of the library
* Mention ECDSA key operations explicitly in short library description
* Say "secret key" instead of "private key
* Define "experimental"
Co-Authored-By: Gregory Maxwell <greg@xiph.org>
Tim Ruffing [Fri, 13 Dec 2019 12:16:29 +0000 (13:16 +0100)]
Merge #689: Remove "except in benchmarks" exception for fp math
bde2a32286c697dd1056aa3eb1ea2a5353f0bede Convert bench.h to fixed-point math (Wladimir J. van der Laan)
Pull request description:
Convert `bench.h` to fixed-point math, removing all use of float math from the repository:
- Use 64-bit integer microsecond timestamps
- Use decimal fixed-point math for formatting numbers
It turned out to be a little trickier than I expected because of formatting and rounding. But, output should be the same before and after.
I used the following to test the number formatting: https://gist.github.com/laanwj/
f971bfbe018e39c19677a21ff954d0c7
ACKs for top commit:
real-or-random:
ACK
bde2a32286c697dd1056aa3eb1ea2a5353f0bede I've read the code in detail and I've tested it. I haven't explicitly tested the formatting function with known/hardcoded inputs.
Tree-SHA512: 41ab6024b88c65a4b194272097c70d527bedb396dc7ab9d3d93165f1a19d31092798370f66399443a8d5393d0a6dcf5825679de5a325550865cfdef3586bf64c
Wladimir J. van der Laan [Tue, 5 Nov 2019 13:05:56 +0000 (14:05 +0100)]
Convert bench.h to fixed-point math
- Use 64-bit integer microsecond timestamps
- Use fixed-point math for formatting numbers
Then, remove "except in benchmarks" exception from `README.md`.
Jonas Nick [Tue, 26 Nov 2019 19:10:02 +0000 (19:10 +0000)]
Merge #679: Add SECURITY.md
78c38363412db3ea1cd1f0cc42dd1624c078ee32 Add SECURITY.md (Jonas Nick)
Pull request description:
Fixes #646
WIP because the secp256k1-security@bitcoincore.org email address doesn't exist yet. But it seems like the right place for vulnerability reports. security@bitcoincore.org would have the downside that it perhaps reaches more people than necessary. Ideally secp256k1-security would just forward to the three maintainers listed in SECURITY.md. @sipa @apoelstra is it okay to put you there? Fwiw I'm opting out for now because three people should be enough.
@sipa do you know who to talk to about adding secp256k1-security@bitcoincore.org and the specifics about how it would work?
ACKs for top commit:
real-or-random:
ACK
78c38363412db3ea1cd1f0cc42dd1624c078ee32 I looked at the diff and verified my fingerprint
Tree-SHA512: 53a989615665cf8cf0c6a70d3bc2c4b71b68178cae40b2a7881aa9eba24732d126ba1e258a9fc127c69b47bb3025943097300cfcbbe18736cbf92ff4f3a901e0
Tim Ruffing [Mon, 25 Nov 2019 14:03:15 +0000 (15:03 +0100)]
Merge #685: Fix issue where travis does not show the ./tests seed…
a0771d1 Explicitly disable buffering for stderr in tests (Jonas Nick)
fb424fb Make travis show the ./tests seed by removing stdout buffering and always cat tests.log after a travis run. (Jonas Nick)
Pull request description:
…by removing stdout buffering and always cat tests.log after a travis run. Fixes #645.
I noticed that according to the [doc](https://www.gnu.org/software/automake/manual/html_node/Parallel-Test-Harness.html) tests.log should contain stdout as well as stderr. But it doesn't because stdout isn't flushed. I removed buffering completely to avoid having to call `fflush` twice.
Travis is instructed to always show the seed which seems helpful with `after_script` by `cat`ing `./tests.log`. In case the tests fail it looks like https://travis-ci.org/jonasnick/secp256k1/jobs/
606446234.
ACKs for commit a0771d:
real-or-random:
ACK
a0771d15e67d3fe6ac1791f81d9731f73c550e5e I looked at the diff and checked that it does not break the tests
Tree-SHA512: 3ba37c2d9169867112981bba3d56680000651ef22ef684c3703f26ed3f71bf415fb23875d30059c8247ea9520c9cfad2c9207badf1b33da8fa3b7b7235a8bf16
Jonas Nick [Mon, 25 Nov 2019 10:17:02 +0000 (10:17 +0000)]
Explicitly disable buffering for stderr in tests
Jonas Nick [Sat, 2 Nov 2019 14:06:36 +0000 (14:06 +0000)]
Make travis show the ./tests seed by removing stdout buffering and always cat tests.log after a travis run.
Jonas Nick [Mon, 25 Nov 2019 10:18:44 +0000 (10:18 +0000)]
Merge #690: Add valgrind check to travis
dd98cc988f0fb3a0ab10bf1a4e28d2fbffd6c1e7 travis: Added a valgrind test without endro and enabled recovery+ecdh (Elichai Turkel)
b4c1382a87dde22d0a5075e56fb7f5d2a09f7cc7 Add valgrind check to travis (Elichai Turkel)
Pull request description:
As discussed in https://github.com/bitcoin-core/secp256k1/pull/687
This adds valgrind check to the repo.
It doesn't run on recovery+ecdh because of the time.
No openssl because of uninitialized mem.
I debated between with and without ASM, but decided with ASM because it might be more fragile(?).
I wasn't sure if I should pass `-DVALGRIND` via `CFLAGS` or `CPPFLAGS`, it seems like because this is only C then there shouldn't even be `CPPFLAGS` but looks like we use `CPPFLAGS` in other places for the preprocessor definitions.
If people are worried about the time it takes we can mark it as `allow_failure` although I don't think it's a problem here because there's only a handful of PRs and they're usually open for weeks.
ACKs for top commit:
real-or-random:
ACK
dd98cc988f0fb3a0ab10bf1a4e28d2fbffd6c1e7 I looked at the diff
jonasnick:
ACK
dd98cc988f0fb3a0ab10bf1a4e28d2fbffd6c1e7
Tree-SHA512: 72d7f1f4c8dd4c58501ac1003b28296d6fd140a8f7711e9e3b3c04a3fbce358ff1c89d2e1d1c5489d7668d3019981264c5cadecae3d9b48cd38c9463e287d8ad
Jonas Nick [Mon, 18 Nov 2019 20:09:05 +0000 (20:09 +0000)]
Merge #678: Preventing compiler optimizations in benchmarks without a memory fence
362bb25608dbcd724a07dd5170c4ebe081c3dd84 Modified bench_scalar_split so it won't get optimized out (Elichai Turkel)
73a30c6b58f078b42a03a222c55bfe8b4dd86a2b Added accumulators and checks on benchmarks so they won't get optimized out (Elichai Turkel)
Pull request description:
As asked https://github.com/bitcoin-core/secp256k1/pull/667#issuecomment-
546885951 this is the parts of #667 that don't require an assembly memory fence.
I splitted them to 2 commits, one with obvious easy ones. and another that changes the logic a bit to achieve this (See https://github.com/bitcoin-core/secp256k1/pull/667#discussion_r337248398 )
ACKs for top commit:
jonasnick:
ACK
362bb256
real-or-random:
ACK
362bb25608dbcd724a07dd5170c4ebe081c3dd84 I read the diff and I ran the benchmarks
Tree-SHA512: d5e47f5d64c3b035155276f057671ceb7f5852f24c7102fee4d0141aabebf882039f3eae0d152bae89d0603bc09fa6ad9f7bc6b8c0f74a668ee252c727517804
Elichai Turkel [Sat, 9 Nov 2019 11:40:45 +0000 (13:40 +0200)]
travis: Added a valgrind test without endro and enabled recovery+ecdh
Elichai Turkel [Thu, 7 Nov 2019 19:31:59 +0000 (21:31 +0200)]
Add valgrind check to travis
This page took 0.087896 seconds and 4 git commands to generate.