Upstream PRs #696 #795 #793 #787 #798 #805 #648 #806 #799 #699 #797 #102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3929536 Test travis s390x (big endian) (Pieter Wuille) Pull request description: ACKs for top commit: real-or-random: ACK 3929536 Travis works and says it's big endian Tree-SHA512: 939b98fe369e575e8bf56899a28cb5aafdb9ccfaaee3cb611027e053edc8220d2787c34359cd01508899b8b7e105c89853a4ab44c382252538c797d00c09345b
So far this has not been needed, as it's only used by the static precomputation which always builds with 32-bit fields. This prepares for the ability to have __int128 detected on the C side, breaking that restriction.
Instead of supporting configuration of the field and scalar size independently,
both are now controlled by the availability of a 64x64->128 bit multiplication
(currently only through __int128). This is autodetected from the C code through
__SIZEOF_INT128__, but can be overridden using configure's
--with-test-override-wide-multiply, or by defining
USE_FORCE_WIDEMUL_{INT64,INT128} manually.
Libcrypto isn't useful here and on some systems UB in OpenSSL's init causes failures. Fixes #775.
57d3a3c Avoid linking libcrypto in the valgrind ct test. (Gregory Maxwell) Pull request description: Libcrypto isn't useful here and on some systems UB in OpenSSL's init causes failures. Fixes #775. ACKs for top commit: real-or-random: ACK 57d3a3c elichai: tACK 57d3a3c Tree-SHA512: 0b10b3e9cc0871a9a93271c72be9d1663ea163745071cb4951a99664c048ab5b6f46bb7cff36e7000e8fb26df7ee164f536f61210bece376478f9f774f34e83d
This does not fix any particular issue but it's preferable to not rely on autoconf. This avoids endianness mess for users on BE hosts if they use their build without autoconf. The macros are carefully written to err on the side of the caution, e.g., we #error if the user manually configures a different endianness than what we detect.
…ailability 79f1f7a Autodetect __int128 availability on the C side (Pieter Wuille) 0d7727f Add SECP256K1_FE_STORAGE_CONST_GET to 5x52 field (Pieter Wuille) Pull request description: This PR does two things: * It removes the ability to select the 5x52 field with a 8x32 scalar, or the 10x26 field with a 4x64 scalar. It's both 128-bit wide versions, or neither. * The choice is made automatically by the C code, unless overridden by a USE_FORCE_WIDEMUL_INT{64,128} define (which is available through `configure` with a hidden option --with-test-override-wide-multiplication={auto,int64,int128}). This reduces the reliance on autoconf for this performance-critical configuration option, and also reduces the number of different combinations to test. This removes one theoretically useful combination: if you had x86_64 asm but no __int128 support in your compiler, it was possible to use the 64-bit field before but the 32-bit scalar. I think this doesn't matter as all compilers/systems that support (our) x86_64 asm also support __int128. Furthermore, #767 will break this. As an unexpected side effect, this also means the `gen_context` static precomputation tool will now use __int128 based implementations when available (which required an addition to the 5x52 field; see first commit). ACKs for top commit: real-or-random: ACK 79f1f7a diff looks good and tests pass elichai: tACK 79f1f7a Tree-SHA512: 4171732668e5c9cae5230e3a43dd6df195567e1232b89c12c5db429986b6519bb4d77334cb0bac8ce13a00a24dfffdff69b46c89b4d59bc6d297a996ea4efd3d
…ianness 0dccf98 Use preprocessor macros instead of autoconf to detect endianness (Tim Ruffing) Pull request description: This does not fix any particular issue but it's preferable to not rely on autoconf. This avoids endianness mess for users on BE hosts if they use their build without autoconf. The macros are carefully written to err on the side of the caution, e.g., we #error if the user manually configures a different endianness than what we detect. Supersedes #770 . ACKs for top commit: sipa: ACK 0dccf98 gmaxwell: ACK 0dccf98 Tree-SHA512: 6779458de5cb6eaef2ac37f9d4b8fa6c9b299f58f6e5b72f2b0d7e36c12ea06074e483acfb85085a147e0f4b51cd67d897f61a67250ec1cea284a0f7680eb2e8
Also permit it being overridden by explicitly passing SECP256K1_{BIG,LITTLE}_ENDIAN
7c06899 Compile-time check assumptions on integer types (Pieter Wuille) 02b6c87 Add support for (signed) __int128 (Pieter Wuille) Pull request description: A compile-time check is implemented in a new `src/assumptions.h` which verifies several aspects that are implementation-defined in C: * size of bytes * conversion between unsigned and (negative) signed types * right-shifts of negative signed types. ACKs for top commit: gmaxwell: ACK 7c06899 real-or-random: ACK 7c06899 code review and tested Tree-SHA512: 3903251973681c88d64d4af0f6cb40fde11eb436804c5b6202c3715b78b1a48bcb287f601b394fd0b503437e3832ba011885e992fe65098b33edc430d9b1f67d
This had two things in it-- tests for the scalar/field code and constant time signing and keygen. The signing and keygen have been thoroughly constant time for years and there are now powerful tests to verify it... no further work on constant-time is needed at least on ordinary platforms (other sidechannels-- sure). The scalar and field code have extensive tests. They could use better static test vectors but they're well tested. TODOs for the project are currently better documented on github right now. This file could return in the future with current info, if needed.
1c32519 Remove the extremely outdated TODO file. (Gregory Maxwell) Pull request description: This had two things in it-- tests for the scalar/field code and constant time signing and keygen. The signing and keygen have been thoroughly constant time for years and there are now powerful tests to verify it... no further work on constant-time is needed at least on ordinary platforms (other sidechannels-- sure). The scalar and field code have extensive tests. They could use better static test vectors but they're well tested. TODOs for the project are currently better documented on github right now. This file could return in the future with current info, if needed. ACKs for top commit: real-or-random: ACK bitcoin-core/secp256k1@1c32519 Tree-SHA512: 65c730ad2816b28991cdb74df6da4671abe76a74a0d0b306f13612b4bbe9b54f9a623b18fc288e0ec13572d9fdbab6f376ce7aafc9fe601644239629b84fb15c
60f7f2d Don't assume that ALIGNMENT > 1 in tests (Tim Ruffing) ada6361 Use ROUND_TO_ALIGN in scratch_create (Jonas Nick) 8ecc6ce Add check preventing rounding to alignment from wrapping around in scratch_alloc (Jonas Nick) 4edaf06 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 60f7f2d Tree-SHA512: ecdd794b55a01d1d6d24098f3abff34cb8bb6f33737ec4ec93714aa631c9d397b213cc3603a916ad79f4b09d6b2f8973bf87fc07b81b25a530cc72c4dbafaba9
bceefd6 Add test logs to gitignore (Jake Rawsthorne) Pull request description: Was just running the tests for bitcoin-core/secp256k1#558 and noticed these logs weren't ignored ACKs for top commit: real-or-random: ACK bitcoin-core/secp256k1@bceefd6 sipa: ACK bceefd6 Tree-SHA512: 690906bc80abc547e1ef78d8654900c2f4054fd8cb8c2e0a6f6b95a5875930b8e1e3a69a5dca86b198e4a2601788f584c8b2ff6f5a85da230b12954e07aeff37
…s + SHA256 selftest 8bc6aef Add SHA256 selftest (Pieter Wuille) 5e5fb28 Use additional system macros to figure out endianness (Pieter Wuille) Pull request description: These are all the architecture macros I could find with known endianness. Use those as a fallback when __BYTE_ORDER__ isn't available. See bitcoin-core/secp256k1#787 (comment) It also adds a SHA256 selftest, so that improperly overriding the endianness detection will be detected at runtime. ACKs for top commit: real-or-random: ACK 8bc6aef I read the diff, and tested that the self-test passes/fails with/without the correct endianness setting gmaxwell: ACK 8bc6aef looks good and I also ran the tests on MIPS-BE and verified that forcing it to LE makes the runtime test fail. Tree-SHA512: aca4cebcd0715dcf5b58f5763cb4283af238987f43bd83a650e38e127f348131692b2eed7ae5b2ae96046d9b971fc77c6ab44467689399fe470a605c3458ecc5
47a7b83 Clear field elements when writing infinity (Elichai Turkel) 61d1ecb 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 47a7b83 Tree-SHA512: cdc2efc242a1b04b4f081183c07d4b2602cdba705e6b30b548df4e115e54fb97691f4b1a28f142f02d5e523c020721337a297b17d732acde147b910f5c53bd0a
The _x and _y suffices are confusing; they don't actually correspond to X and Y coordinates. Instead replace them with arrays.
Also increase the number of fe inputs.
Also make the num_jacobi benchmark use the scalar order as modulus, instead of a random number.
cb5524a Add benchmark for secp256k1_ge_set_gej_var (Pieter Wuille) 5c6af60 Make jacobi benchmarks vary inputs (Pieter Wuille) d0fdd5f Randomize the Z coordinates in bench_internal (Pieter Wuille) c7a3424 Rename bench_internal variables (Pieter Wuille) Pull request description: ACKs for top commit: real-or-random: ACK cb5524a jonasnick: ACK cb5524a Tree-SHA512: 0cbcfffebebf563cf9a1bd951394a0419503ffd43a2d0df4c99e4a839c89c8454925314f7e7eee0c01bce94b6dfeab935f36cc27f9bfc878f702313d455db7e1
|
Need to add a commit changing the use of |
jonasnick
force-pushed
the
temp-merge-797
branch
from
0031b1c
to
8b70795
Compare
real-or-random
approved these changes
Oct 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[upstream PR #696]: Run a Travis test on s390x (big endian)
[upstream PR #795]: Avoid linking libcrypto in the valgrind ct test.
[upstream PR #793]: Make scalar/field choice depend on C-detected __int128 availability
[upstream PR #787]: Use preprocessor macros instead of autoconf to detect endianness
[upstream PR #798]: Check assumptions on integer implementation at compile time
[upstream PR #805]: Remove the extremely outdated TODO file.
[upstream PR #648]: Prevent ints from wrapping around in scratch space functions
[upstream PR #806]: Trivial: Add test logs to gitignore
[upstream PR #799]: Add fallback LE/BE for architectures with known endianness + SHA256 selftest
[upstream PR #699]: Initialize field elements when resulting in infinity
[upstream PR #797]: Fix Jacobi benchmarks and other benchmark improvements
Next PR will be schnorrsig.