3daab83a60 refactor: remove ret from secp256k1_ec_pubkey_serialize (kevkevinpal)
Pull request description:
This is a follow-up to https://github.com/bitcoin-core/secp256k1/pull/1774#discussion_r2539737079
It is pretty straightforward to remove `ret` and to just return either `0` or `1`
ACKs for top commit:
real-or-random:
utACK 3daab83a60
theStack:
ACK 3daab83a60
Tree-SHA512: ce598d917455a2d25297436bf2b900a9e88a638617cb79ca22e467135035c334b6815911fe4429ff44dbd877e6d10a346d0b37f2e5a7459e5b35854023832d27
secp256k1_memclear has the side effect of undefining bytes for
valgrind checks. In some cases, we may want to zero bytes
but allow subsequent reads. So we split memclear into
memclear_explicit, which makes no guarantees about the content
of the buffer on return, and memzero_explicit, which guarantees
zero value on return.
Change the memset in partial_sign to use memzero_explicit.
These function aliases have been described as DEPRECATED in the public
API docs already many years ago (see #701, commit 41fc7856), and in
addition explicit deprecation warnings are shown by the compiler at
least since the first official release 0.2.0 (see PR #1089, commit
fc94a2da), so it should be fine to just remove them by now.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
Due to similarity to the public API function `secp256k1_ec_pubkey_serialize`,
public API flags like `SECP256K1_EC_COMPRESSED` are sometimes mistakingly
passed to newly proposed code (this is currently the case for several modules in
secp256k1-zkp, see https://github.com/BlockstreamResearch/secp256k1-zkp/pull/300).
which is currently not detected. To avoid this in the future, a VERIFY_CHECK
is added to check that the `compressed` argument is either 0 or 1.
Quoting sipa (see https://github.com/bitcoin-core/secp256k1/pull/1479#discussion_r1790079414):
"When performing an EC multiplication A = aG for secret a, the resulting
_affine_ coordinates of A are presumed to not leak information about a (ECDLP),
but the same is not necessarily true for the Jacobian coordinates that come
out of our multiplication algorithm."
For the ECDH point multiplication result, the result in Jacobi coordinates should be
cleared not only to avoid leaking the scalar, but even more so as it's a representation
of the resulting shared secret.
This gives the caller more control about whether the state should
be cleaned (= should be considered secret). Moreover, it gives the
caller the possibility to clean a hash struct without finalizing it.
All of the invocations of secp256k1_memclear() operate on stack
memory and happen after the function is done with the memory object.
This commit replaces existing memset() invocations and also adds
secp256k1_memclear() to code locations where clearing was missing;
there is no guarantee that this commit covers all code locations
where clearing is necessary.
Co-Authored-By: isle2983 <isle2983@yahoo.com>
This gets rid of an untested code path. Resolves#1352.
secp256k1_ge_storage is a struct with two secp256k1_fe_storage fields.
The C standard allows the compiler to add padding between the fields and
at the end of the struct, but no sane compiler in the end would do this:
The only reason to add padding is to ensure alignment, but such padding
is never necessary between two fields of the same type.
Similarly, secp256k1_fe_storage is a struct with a single array of
uintXX_t. No padding is allowed between array elements. Again, C allows
the compiler to insert padding at the end of the struct, but there's no
absolute reason to do so in this case.
For the uintXX_t itself, this guaranteed to have no padding bits, i.e.,
it's guaranteed to have exactly XX bits.
So I claim that for any existing compiler in the real world,
sizeof(secp256k1_ge_storage) == 64.
The scheme implemented is described below, and largely follows the paper
"SwiftEC: Shallue–van de Woestijne Indifferentiable Function To Elliptic Curves",
by Chavez-Saab, Rodriguez-Henriquez, and Tibouchi
(https://eprint.iacr.org/2022/759).
A new 64-byte public key format is introduced, with the property that *every*
64-byte array is an encoding for a non-infinite curve point. Each curve point
has roughly 2^256 distinct encodings. This permits disguising public keys as
uniformly random bytes.
The new API functions:
* secp256k1_ellswift_encode: convert a normal public key to an ellswift 64-byte
public key, using additional entropy to pick among the many possible
encodings.
* secp256k1_ellswift_decode: convert an ellswift 64-byte public key to a normal
public key.
* secp256k1_ellswift_create: a faster and safer equivalent to calling
secp256k1_ec_pubkey_create + secp256k1_ellswift_encode.
* secp256k1_ellswift_xdh: x-only ECDH directly on ellswift 64-byte public keys,
where the key encodings are fed to the hash function.
The scheme itself is documented in secp256k1_ellswift.h.
The output ge is normalized when sizeof(secp256k1_ge_storage) = 64
but not when it's not 64. ARG_CHECK at the end of the function
assumes normalization. So normalize ge in the other code path too.
f3126fdfec norm arg: remove prove edge tests which are now covered by vectors (Jonas Nick)
847ed9ecb2 norm arg: add verification to prove vectors (Jonas Nick)
cf797ed2a4 norm arg: add prove test vectors (Jonas Nick)
095c1e749c norm arg: add prove_const to tests (Jonas Nick)
bf7bf8a64f norm arg: split norm_arg_zero into prove_edge and verify_zero_len (Jonas Nick)
a70c4d4a8a norm arg: add test vector for |n| = 0 (Jonas Nick)
f5e4b16f0f norm arg: add test vector for sign bit malleability (Jonas Nick)
c0de361fc5 norm arg: allow X and R to be the point at infinity (Jonas Nick)
f22834f202 norm arg: add verify vector for n = [0], l = [0] (Jonas Nick)
d8e7f3763b musig: move ge_{serialize,parse}_ext to module-independent file (Jonas Nick)
Pull request description:
ACKs for top commit:
Liam-Eagen:
ACK f3126fd
Tree-SHA512: 1aad86521fce74435beabe7690c7fcc38ad9ae7a884ddcab69ef825b573433f700723a7672d29df1b4465bc33d5957b6a46f657f988cfd2cc73fa94a3472357d
