-
Notifications
You must be signed in to change notification settings - Fork 43
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
BIP340: Variable-length messages / domain separation #221
Conversation
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.
Concept ACK. The explanation for domain separation turned out nicely.
aa35f62
to
a04ff12
Compare
Forced-push to address @jonasnick's comments. |
7d527e7
to
4a1ace6
Compare
Updated. This also takes the #211 into account now. |
@jonasnick @sipa Would be nice to have ACKs on the method before I'll generate test vectors. |
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.
Method ACK
You seem to have a bunch of unrelated commits in here? |
|
@real-or-random Oops, I didn't realize this was opened against my repo. |
@real-or-random I've updated my bip-taproot branch, in case you want to update. |
4a1ace6
to
70a6791
Compare
rebased |
Concept ACK |
cc @LLFourn |
ACK. 33 bytes seems straightforward and easy to explain. It looks like in |
bip-0340.mediawiki
Outdated
|
||
==== Messages of Arbitrary Size ==== | ||
|
||
The signature scheme specified in this BIP accepts byte strings of arbitrary size as input messages.<ref>In theory, the message size is restricted due to the fact that SHA256 accepts byte strings only up to size of 2^56-1 bytes.</ref> |
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.
Where does the number 2^56-1 come from?
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.
SHA256 accepts messages of length up to 2^64-1 bits, see https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf Figure 1.
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.
Ah right, tricky.
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.
2^64 bits is 2^61 bytes
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.
Ah right, tricky... Sigh.
edit: I seem to be married to this mistake, I made the same mistake here: bitcoin-core/secp256k1#731 (review)
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
70a6791
to
a577322
Compare
@jonasnick suggested in jonasnick#24 (comment) to have an explicit upper limit of message size, e.g., 2^48 (we could also make it 2^60 or even 2^62): Pros:
Cons:
My opinion: I don't think "Pro 4" is helpful in practice: Even if we have an upper bound of 2^48, this does not guarantee that all implementation will accept messages of this size. Many implementations will want reject them, e.g., implementations on 32-bit platforms. Similarly, I think a limit wouldn't help make verification identical across implementations (again because implementations will have their own upper bounds). So I tend to think that "Noone will ever sign a message of size 2048 PiB." convinces me. My standpoint is that we're just giving a generic algorithms, any application or protocol using the BIP will need to specify message formats, and this include a maximum message size, which will probably be well below 2^48. If formal methods need a limit, they could still introduce one in their theorems, and then the restricted theorem would still cover any real world protocol. |
An upper bound on the message length that's much smaller than 2^61 would allow claiming that BIP-musig can sign any message that BIP-schnorr can or that BIP-half-agg can verify any signature that BIP-schnorr can. With the assumption that "Noone will ever hash an input of size 2048 PiB" (slightly strong than "Noone will ever sign a message of size 2048 PiB") then you can still make this claim (even with formal methods). I'm fine working under this assumption to avoid complexity. If you'd want to make use of an upper bound like 2^48 in a half aggregation spec you'd need to do annoying caclulations to figure out how many signatures you can actually verify and put an upper bound on that as well (the current draft already has an upper bound on it due to potential integer overflows). Stating the upper bound that stems from SHA256 explicitly (close to |
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.
ACK mod nit
bip-0340/test-vectors.csv
Outdated
15,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,,71535DB165ECD9FBBC046E5FFAEA61186BB6AD436732FCCC25291A55895464CF6069CE26BF03466228F19A3A62DB8A649F2D560FAC652827D1AF0574E427AB63,TRUE,message of size 0 bytes (added 2022-08) | ||
16,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,11,08A20A0AFEF64124649232E0693C583AB1B9934AE63B4C3511F3AE1134C6A303EA3173BFEA6683BD101FA5AA5DBC1996FE7CACFC5A577D33EC14564CEC2BACBF,TRUE,message of size 1 byte (added 2022-08) | ||
17,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,0102030405060708090A0B0C0D0E0F1011,5130F39A4059B43BC7CAC09A19ECE52B5D8699D1A71E3C52DA9AFDB6B50AC370C4A482B77BF960F8681540E25B6771ECE1E5A37FD80E5A51897C5566A97EA5A5,TRUE,message of size 17 bytes (added 2022-08) | ||
18,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999,403B12B0D8555A344175EA7EC746566303321E5DBFA8BE6F091635163ECA79A8585ED3E3170807E7C03B720FC54C7B23897FCBA0E9D0B4A06894CFD249F22367,TRUE,message of size 100 bytes (added 2022-08) | ||
19,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999,F1D320D7DE4D5FF365A8CE6CF0AD580B9358F96F46DFBE141E22128B75919C3E47B4420DB40806AD1CF6BF0FCADE8A2D51634DEDB7E118E9AE896DD4D6A504BB,TRUE,message of size 1000 bytes (added 2022-08) |
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: is there a particular reason why you chose these test vectors specifically? In BIP-musig we only have tests for 32-byte and 0-byte. I see no particular reason how an implemtnation can pass those two test vectors but not a vector with length 1000.
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.
The purpose of the 100 byte and 1000 byte vectors is to catch implementation that that truncate messages (e.g., at 32 bytes because they only reserve 32 bytes internally). I can remove the 1000 bytes vector if you think it's overkill.
And the 17 was just to have another case < 32 which is not a corner case.
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.
The purpose of the 100 byte and 1000 byte vectors is to catch implementation that that truncate messages (e.g., at 32 bytes because they only reserve 32 bytes internally)
Good point. I added a vector with a message > 32 bytes to bip-musig2.
Wrt. the 1000 bytes vector, I'd remove it because I find it desirable to have a minimal set of test vectors, but it's fine with me either way.
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.
removed the 1000 bytes vector
a577322
to
05bd175
Compare
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.
ACK 05bd175
bip-0340.mediawiki
Outdated
If large messages are not pre-hashed, | ||
the algorithms of the signature scheme will perform more hashing internally. | ||
In particular, the hashing performed by the signing algorithm will process the message twice, | ||
which leads to performance penalties for large messages. |
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.
Specifically, if the post-padding size of the message exceeds 128/(2-a) bytes, where a is the ratio between the hashing speed in the caller and the hashing speed in libsecp256k1 (a<1 meaning the caller is faster), then pre-hashing in the caller is faster.
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.
Are you suggesting that we add the exact condition?
a is the ratio between the hashing speed in the caller and the hashing speed in libsecp256k1 (a<1 meaning the caller is faster)
Yes, assuming libsecp256k1 is used at all.
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.
I mostly calculated this because I was curious. But perhaps it's useful to be specific (if you assume a=1), by saying that generally precomputing is faster for messages over 55 bytes?
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.
I can add a footnote but what exactly do you mean by "post-padded"?
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.
SHA256 adds between 9 and 72 bytes of padding IIRC (ending with a multiple of 64 bytes). So by post-padded size of the message I mean 64*floor((msglen+72)/64)
.
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.
Suggested footnote:
- Typically, messages of 56 bytes or longer enjoy a performance benefit from pre-hashing, assuming the hashing speed inside the signature scheme matches that of the pre-hashing done inside the calling application.
bip-0340.mediawiki
Outdated
(as for example done in [[bip-0341.mediawiki|BIP341]].) | ||
|
||
Since pre-hashing may not always be desirable, | ||
e.g. when actual messages are shorter than 32 bytes, |
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.
e.g. when actual messages are shorter than 32 bytes, | |
e.g., when actual messages are shorter than 32 bytes, |
bip-0340.mediawiki
Outdated
If large messages are not pre-hashed, | ||
the algorithms of the signature scheme will perform more hashing internally. | ||
In particular, the hashing performed by the signing algorithm will process the message twice, | ||
which leads to performance penalties for large messages. |
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.
I can add a footnote but what exactly do you mean by "post-padded"?
This could also cite https://ed25519.cr.yp.to/eddsa-20150704.pdf for its last paragraph. |
bip-0340.mediawiki
Outdated
|
||
As a best practice, we recommend applications to use exactly one of the following methods to pre-process application messages before passing it to the signature scheme: | ||
* Either, pre-hash the application message using ''hash<sub>name</sub>'', where ''name'' identifies the context uniquely (e.g., "foo-app/signed-bar"), | ||
* or prefix the actual message with a 33-byte string that identifies the context uniquely (e.g. the UTF-8 enconding of "foo-app/signed-bar", padded with null bytes to 33 bytes). |
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.
Typo: enconding
bip-0340.mediawiki
Outdated
|
||
To help implementors understand updates to this BIP, we keep a list of substantial changes. | ||
|
||
* 2022-08: Allow messages of arbitrary size |
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.
Update...
bip-0340/test-vectors.csv
Outdated
@@ -14,3 +14,7 @@ index,secret key,public key,aux_rand,message,signature,verification result,comme | |||
12,,DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659,,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B,FALSE,sig[0:32] is equal to field size | |||
13,,DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659,,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E177769FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141,FALSE,sig[32:64] is equal to curve order | |||
14,,FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30,,243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89,6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E17776969E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B,FALSE,public key is not a valid X coordinate because it exceeds the field size | |||
15,0340034003400340034003400340034003400340034003400340034003400340,778CAA53B4393AC467774D09497A87224BF9FAB6F6E68B23086497324D6FD117,0000000000000000000000000000000000000000000000000000000000000000,,71535DB165ECD9FBBC046E5FFAEA61186BB6AD436732FCCC25291A55895464CF6069CE26BF03466228F19A3A62DB8A649F2D560FAC652827D1AF0574E427AB63,TRUE,message of size 0 bytes (added 2022-08) |
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.
Code for generating these should be added to bip-0340/test-vectors.py
.
Concept ACK apart from nits. |
05bd175
to
e4b87fb
Compare
addressed all nits by @sipa
I made the effort to mention their main arguments instead (reliance on collision-resistance, need to keep the message in memory while signing). And then there's no need to cite this, this is common knowledge... |
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.
ACK mod nits
bip-0340.mediawiki
Outdated
(as for example done in [[bip-0341.mediawiki|BIP341]].) | ||
|
||
Since pre-hashing may not always be desirable, | ||
e.g., when actual messages are shorter than 32 bytes,<ref>Another reason to omit pre-hashing is to protect against certain types of cryptanalytic advances against the hash function used for pre-hashing: If pre-hashing is used, an attacker that can find collisions in the pre-hashing function can necessarily forge signatures under chosen-message attacks. If pre-hashing is not used, an attacker that can find collisions in SHA256 (as used inside the signature scheme) may not be able to forge signatures. However, this seeming advantage is mostly moot in the context Bitcoin, which already relies on collision resistance of SHA256 in other places, e.g., for transaction hashes.</ref> |
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: s/moot/irrelevant
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.
done
@@ -249,6 +249,20 @@ def vector14(): | |||
|
|||
return (None, pubkey, None, msg, sig, "FALSE", "public key is not a valid X coordinate because it exceeds the field size") | |||
|
|||
def varlen_vector(msg_int): | |||
seckey = bytes_from_int(int(16 * "0340", 16)) |
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.
seckey = default_seckey
for consistency?
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.
I think some variance in the vectors won't hurt.
e4b87fb
to
e2ab39a
Compare
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.
ACK e2ab39a
e2ab39a
to
6467520
Compare
forced-push to address nits by @sipa |
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.
ACK 6467520
Do you want to just PR it to the bips repo now? |
No, we have more local changes here: bitcoin/bips@master...sipa:bips:bip-taproot |
Opened BIPs repo PR: bitcoin#1446 |
Solves #207.
I tried to avoid going into collision-resistance vs random-prefix resistance. It's hard to explain, and the situation is not entirely clear, e.g., due to https://eprint.iacr.org/2015/509.pdf which claims a flaw in the paper http://www.neven.org/papers/schnorr.pdf which cite in the BIP (we may want to add a footnote to the BIP...).
TODO:
@sipa Can you update the
bip-taproot
brach tobitcoin/master