bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.09k stars 1.01k forks source link

Support more Schnorr schemes besides BIP340 #1070

Closed Sajjon closed 2 years ago

Sajjon commented 2 years ago

There are two different Schnorr signature variants in bitcoin-core/secp256k1, the "default" one named secp256k1_schnorrsig_sign and another more customizable with the appropriate name secp256k1_schnorrsig_sign_custom. Both functions calls secp256k1_schnorrsig_sign_internal. Using secp256k1_schnorrsig_sign_custom we can pass custom nonce data through the secp256k1_schnorrsig_extraparams parameter, which has a member for a custom nonce function, secp256k1_nonce_function_hardened noncefp. This looks great at first glance! Looks like I can use bitcoin-core/secp256k1 as a provider of a great Schnorr sig implementation! However, the secp256k1_schnorrsig_sign_custom is not as customizable as it could be, or as I want to be, because secp256k1_schnorrsig_sign_internal hardcodes the algo to bip340_algo (with value "BIP0340/nonce"), when calling the nonce function:

    ret &= !!noncefp(buf, msg, msglen, seckey, pk_buf, bip340_algo, sizeof(bip340_algo), ndata);

I propose we change this hardcoded value, to allow consumers of this amazing lib, to pass along a custom algo string.

I've recently started writing a Swift wrapper called K1. The earliest (AFAIK) adopter of Schnorr signatures amongst cryptocurrency community is Zilliqa, which has been using Schnorr since 2017/2018. The Zilliqa-JS javascript library has in earlier commits bundled 1000 test vectors which I thought to be useful.

I would also like to make it possible for my library K1 to replace my own, unsafe, EC library called EllipticCurveKit, which currently is used (potentially unsafe for end users, which they are informed about) by the iOS Zilliqa wallet Zhip (open source) (also on AppStore).

The bad news is that Zilliqa uses the ALGO name "Schnorr+SHA256 ". So I cannot replace EllipticCurveKit in Zhip with K1 until I'm able to pass a custom ("Schnorr+SHA256 ") ALGO to secp256k1_schnorrsig_sign_custom.

Furthermore, Zilliqa uses this HMAC-DRBG as nonce function, but I should be able to implement that in C and pass through the noncefp member in secp256k1_schnorrsig_extraparams, right?

Thanks!

Sajjon commented 2 years ago

If PRs are welcome, I might give it a go? That is, if you all think it a good idea to support this?

sipa commented 2 years ago

You can provide a custom nonce function to accomplish this. It could ignore the algo passed, and use whatever prefix it wants.

Sajjon commented 2 years ago

Ah right, got it, thanks!

Sajjon commented 2 years ago

@sipa On closer inspection, actually it is not enough to pass a custom Nonce Function, because secp256k1_schnorrsig_sign_internal calls secp256k1_schnorrsig_challenge, which in turn initializes the SHA256 hasher to some hardcoded fixed midstate using call secp256k1_schnorrsig_sha256_tagged (being SHA256("BIP0340/challenge")||SHA256("BIP0340/challenge") as per code documentation).

So the "challenge" function would also need to be passed.

I experimented by modifying the challenge function to the same as Zilliqa uses and managed to produce the same r component of the signature (strangely enough Zilliqa hashes 33 bytes compression of public point, where as bitcoin-core/secp256k1 only uses the X component of points, i.e. 32 bytes...).

sipa commented 2 years ago

I don't understand why you can't precompute the initial state for your prefix just like we did for ours?

Sajjon commented 2 years ago

@sipa In a fork of bitcoin-core/secp256k1, yes, I can (or otherwise bundled and modified bitcoin-core/secp256k1 source). But I'm not talking about that use case, I'm talking about using bitcoin-core/secp256k1 unchanged, as a dependency and relying on secp256k1_schnorrsig_sign_custom for customizable Schnorr signing. But as I wrote above, the secp256k1_schnorrsig_challenge is hardcoded to use SHA256("BIP0340/challenge")||SHA256("BIP0340/challenge") as initial state of SHA256.

So: I'm asking if it would be possible to add yet another function pointer to secp256k1_schnorrsig_extraparams, being a pointer for a customizable secp256k1_schnorrsig_challenge and instead of having:

void secp256k1_schnorrsig_challenge_fp(
secp256k1_scalar* e, 
const unsigned char *r32, 
const unsigned char *msg, 
size_t msglen, 
const unsigned char *pubkey32
);

The declaration could be

void secp256k1_schnorrsig_challenge_fp(
  secp256k1_scalar* e, // Out
secp256k1_ge *r, //  r := Curve.G * k
  const unsigned char *msg,
  size_t msglen,
  secp256k1_pubkey *publicKey
  );

Which is more customizable... since we get access to r instead of r.x and the whole publicKey instead of publicKey.X.

And then secp256k1_schnorrsig_challenge would still act as the default value of type secp256k1_schnorrsig_challenge_fp if no custom was provided (and in the case of secp256k1_schnorrsig_sign being used instead of secp256k1_schnorrsig_sign_custom).

sipa commented 2 years ago

Oh this isn't about the nonce function anymore, but the challenge hash.

Yes, that's hardcoded. The schnorrsig module implements the signature scheme defined in BIP340, not other schemes.

Sajjon commented 2 years ago

@sipa yeah sorry if I was unclear.

Ok got it. Hmmm. Is that something we might wanna reconsider?

This amazing library would add great value to the whole crypto-community if it could support more Schnorr schemes besides BIP340, and I think support for custom Challenge function pointer might be enough for many. At least for Zilliqa.

real-or-random commented 2 years ago

So: I'm asking if it would be possible to add yet another function pointer to secp256k1_schnorrsig_extraparams, being a pointer for a customizable secp256k1_schnorrsig_challenge and instead of having:

This has a huge footgun potential because you leak the secret key if you sign twice with the same nonce but different challenge hashes.

Besides, I don't think it would solve your problem. AFAICT from a glimpse at the code you linked, their scheme does not use x-only public keys, and the signatures are different: The first component is a hash and not a point as in BIP340. "Schnorr signatures" are an abstract concept. If you want a concrete scheme, you need to make a few design decisions and those in BIP340 are different than what you're looking for.

This library is made for the Bitcoin universe. If you want something else, you need to fork it.

Sajjon commented 2 years ago

@real-or-random thanks! Yeah I figured there are risks if not done right.

Hopefully we will see many other projects adopting BIP340 instead of using there own Challenge fn etc.

Thanks!