Closed
Conversation
Instead of having the starting point of the ecmult_gen computation be offset, do it with the final point. This enables reasoning over the set of points reachable in intermediary computations, which can be leveraged by potential future optimization. Because the final point is in affine coordinates, its projective blinding is no longer possible. It will be reintroduced again in a different way, in a later commit. Also introduce some more comments and more descriptive names.
The old code overwrote the input at the start of the function, making a call like secp256k1_scalar_inverse(&x,&x) always fail.
This introduces the signed-digit multi-comb multiplication algorithm for constant-time G multiplications (ecmult_gen). It is based on section 3.3 of "Fast and compact elliptic-curve cryptography" by Mike Hamburg (see https://eprint.iacr.org/2012/309). Original implementation by Peter Dettman, with changes by Pieter Wuille to use scalars for recoding, and additional comments.
It is unnecessary to recompute this term needed by the SDMC algorithm for every multiplication; move it into the context scalar_offset value instead.
The old code would trigger UB when count=32.
The existing code needs to deal with the edge case that bit_pos >= 256, which would lead to an out-of-bounds read from secp256k1_scalar. Instead, recode the scalar into an array of uint32_t with enough zero padding at the end to alleviate the issue. This also simplifies the code, and is necessary for a security improvement in a follow-up commit. Original code by Peter Dettman, with modifications by Pieter Wuille.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
… algorithm 4c341f8 Add changelog entry for SDMC (Pieter Wuille) a043940 Permit COMB_BITS < 256 for exhaustive tests (Pieter Wuille) 39b2f2a Add test case for ecmult_gen recoded = {-1,0,1} (Pieter Wuille) 644e86d Reintroduce projective blinding (Pieter Wuille) 07810d9 Reduce side channels from single-bit reads (Peter Dettman) a0d32b5 Optimization: use Nx32 representation for recoded bits (Peter Dettman) e03dcc4 Make secp256k1_scalar_get_bits support 32-bit reads (Pieter Wuille) 5005abe Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t (Pieter Wuille) 6247f48 Optimization: avoid unnecessary doublings in precomputation (Peter Dettman) 15d0cca Optimization: first table lookup needs no point addition (Pieter Wuille) 7a33db3 Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset (Pieter Wuille) ed2a056 Provide 3 configurations accessible through ./configure (Pieter Wuille) 5f7be9f Always generate tables for current (blocks,teeth) config (Pieter Wuille) fde1dfc Signed-digit multi-comb ecmult_gen algorithm (Peter Dettman) 486518b Make exhaustive tests's scalar_inverse(&x,&x) work (Pieter Wuille) ab45c3e Initial gej blinding -> final ge blinding (Pieter Wuille) aa00a6b Introduce CEIL_DIV macro and use it (Tim Ruffing) Pull request description: ACKs for top commit: real-or-random: reACK 4c341f8 jonasnick: ACK 4c341f8 stratospher: ACK 4c341f8. Did [these benchmarks](bitcoin-core/secp256k1#1058 (comment)) and saw a 12.4% on gcc 13.2.0 and 11.5% on clang 15.0.0. Also summarised how the precomputed table generation works [here](https://github.com/stratospher/blogosphere/blob/main/sdmc.md) for future me :) Tree-SHA512: 9a11138e4fb98b98e85c82cd46ed78b29fbe63d6efe61654ef519a64b1e175d63395a8a931c1646f9df8c7daacd796d5fe2384899d5a13a2c7ed2ded696ceed5
Co-authored-by: Tim Ruffing <crypto@timruffing.de> Co-authored-by: Russell O'Connor <roconnor@blockstream.io>
7d2591c Add secp256k1_pubkey_sort (Jonas Nick) Pull request description: This PR adds a `secp256k1_pubkey_sort` function the the public API which was originally part of the musig PR (#1479). However, I opened a separate PR because it adds internal functions that are also used by the WIP silent payments module. ACKs for top commit: sipa: ACK 7d2591c josibake: ACK bitcoin-core/secp256k1@7d2591c real-or-random: ACK 7d2591c Tree-SHA512: d0e4464dc9cd4bdb35cc5d9bb4c37a7b71233328319165d49bc940d8d3394a2d74a43d2f73ee7bfe8f3f90a466ee8afcdca75cfbbf3969e218d76b89f4af55fb
Instead of having the starting point of the ecmult_gen computation be offset, do it with the final point. This enables reasoning over the set of points reachable in intermediary computations, which can be leveraged by potential future optimization. Because the final point is in affine coordinates, its projective blinding is no longer possible. It will be reintroduced again in a different way, in a later commit. Also introduce some more comments and more descriptive names.
The old code overwrote the input at the start of the function, making a call like secp256k1_scalar_inverse(&x,&x) always fail.
This introduces the signed-digit multi-comb multiplication algorithm for constant-time G multiplications (ecmult_gen). It is based on section 3.3 of "Fast and compact elliptic-curve cryptography" by Mike Hamburg (see https://eprint.iacr.org/2012/309). Original implementation by Peter Dettman, with changes by Pieter Wuille to use scalars for recoding, and additional comments.
It is unnecessary to recompute this term needed by the SDMC algorithm for every multiplication; move it into the context scalar_offset value instead.
The old code would trigger UB when count=32.
The existing code needs to deal with the edge case that bit_pos >= 256, which would lead to an out-of-bounds read from secp256k1_scalar. Instead, recode the scalar into an array of uint32_t with enough zero padding at the end to alleviate the issue. This also simplifies the code, and is necessary for a security improvement in a follow-up commit. Original code by Peter Dettman, with modifications by Pieter Wuille.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Merge bitcoin-core/secp256k1#1518: Add secp256k1_pubkey_sort
This PR can be recreated with ./contrib/sync-upstream.sh -b master range bb528cf.
Tip: Use git show --remerge-diff to show the changes manually added to the merge commit.