-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow sign-to-contract commitments in schnorrsigs [̶a̶l̶t̶e̶r̶n̶a̶t̶i̶v̶e̶]̶ #589
Conversation
That workflow doesn't make it look as easy to implement s2c anti-sidechannel blinding, if I understand it. |
Hm, I've done a quick and dirty rebase of the anti-nonce-sidechannel and it seems straight forward. It's even a bit simpler on the client side. |
I opened PR #590 to show the anti nonce-sidechannel protocol can be built on this PR in a similar way. |
Concept ACK |
I guess this is a more general discussion because it's also in ECDSA: |
@real-or-random IIRC it's actually used by joinmarket to do ECDH with the R value for a stealth payment donation feature. (like one of the outputs pays to P+H(kP)G for some pubkey P). The payments can be found by the recipient simply scanning every signature in the chain, performing ECDH with it, and checking the outputs... Less cosmically RFC6979 is kind of absurd and actually makes signing measurably slower compared to just a simple hash-- for applications that care about signing speed (like matt's betterhash) substituting it is probably a good idea. Not sure who if anyone is doing that right now, but some things probably should be. (Though those same applications should probably also do batch nonce generation and other stuff that wouldn't really be simple with the current nonce function interface) Re: footgunnery, I haven't yet seen anyone footgun themselves that way. Creating a working nonce function takes a fair amount of work, so I think it's likely that if anyone bothers they at least have a fighting chance to get it right. I think the best mental model is that we're trying to defend against users that are equal parts lazy and ignorant. Give them a nonce argument and they will set the nonce to private key xor messagehash (actual example which demonstrates lazy+ignorant), but give them a nonce function argument and they'll leave it null unless they actually have a real reason to do otherwise. :) |
Okay, convinced.
That's indeed a good way to think about it. Maybe it's still a good idea to make it more prominent in the docs that people should use the default unless they really know what they are doing. |
Sounds fine to me. I also think we could explicitly say that the nonce function isn't an interface that we particularly care about keeping stable. |
|
src/secp256k1.c
Outdated
return secp256k1_gej_is_infinity(&pj); | ||
} | ||
|
||
static char s2c_opening_magic[8] = { 0x5d, 0x05, 0x20, 0xb8, 0xb7, 0xf2, 0xb1, 0x68 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it's char
but in the struct it's unsigned char
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to uint64_t
because there's less room for mistakes and it's easier to read.
secp256k1_ge_set_gej(&r, &rj); | ||
if (s2c_opening != NULL) { | ||
secp256k1_s2c_opening_init(s2c_opening); | ||
if (s2c_data32 != NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a check if s2c_opening == NULL || s2c_data32 == NULL
? Because if that's true I think it means the user made a mistake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean CHECK((s2c_opening != NULL) == (s2c_data32 != NULL))
(both should be NULL or both should be non-NULL)? Yes, I'll add an ARG_CHECK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
include/secp256k1.h
Outdated
/* Public nonce before applying the sign-to-contract commitment */ | ||
secp256k1_pubkey original_pubnonce; | ||
/* Integer indicating if signing algorithm negated the nonce */ | ||
unsigned char nonce_is_negated; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a computational optimization to avoid computing the jacobi symbol on secp256k1_schnorrsig_verify_s2c_commit
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we could could compute the ec_commitment of original_pubnonce
and data
then negate if this is not a valid nonce. But this prevents batch verification. I will mention this in the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comment to explain this
(I also prefer this design over #588) |
include/secp256k1.h
Outdated
/* Byte indicating if signing algorithm negated the nonce. Alternatively when | ||
* verifying we could compute the EC commitment of original_pubnonce and the | ||
* data and negate if this would not be a valid nonce. But this would prevent | ||
* batch verification of sign-to-contract commitments. */ | ||
unsigned char nonce_is_negated; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, feel free to ignore: Boolean values are typically represented by int
and not unsigned char
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but ints are the worst. Since nonce_is_negated
is only ever 0 or 1 I changed this to int.
779458a
to
4798484
Compare
This PR is quite outdated. Sign-to-contract should use the schnorrsig_sign_custom API. PR #1018 is a WIP continuation of this work. Closing. |
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
Adapted from bitcoin-core#589. Co-authored-by: Marko Bencun <mbencun+pgp@gmail.com>
Adapted from bitcoin-core#589. The data is hashed using a tagged hash with the "s2c/schnorr/data" tag, which is consistent with the data hashing done in the ECDSA version of sign-to-contract (in ElementsProject/secp256k1-zkp), where the "s2c/ecdsa/data" tag is used. Similarly, the tweak hash tag is "s2c/schnorr/point". Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
This is an alternative to #588 as requested by @real-or-random which I also slightly prefer(EDIT: this seems to be generally preferred now). The main difference is that the sign-to-contract commitment step happens in the signature function and not the nonce function. Also thenonce_is_negated
argument inschnorrsig_sign
is replaced by ans2c_opening
object. A new argument toschnorrsig_sign
is added calleds2c_data
. There's no need to add a context argument to nonce functions. I also added parsing and serialization fors2c_opening
s. Manual initialization ofs2c_opening
is not necessary anymore.Example: