Skip to content
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

BIP-340 signature verification #10

Merged
merged 6 commits into from
Jan 8, 2024
Merged

BIP-340 signature verification #10

merged 6 commits into from
Jan 8, 2024

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Jan 5, 2024

Depends on #9. Will remain as draft until #9 is merged.

The VerifySignature verifies the provided BIP-340 signature for the message against the group public key. The function returns true and nil error when the signature is valid. The function returns false and an error when the signature is invalid. The error provides a detailed explanation of why the signature verification failed.

VerifySignature implements Verify(pk, m, sig) function defined in BIP-340. One important design decision is to accept public key as a *Point instead of just the X coordinate. From BIP-340:

"Note that the correctness of verification relies on the fact that lift_x always returns a point with an even Y coordinate. A hypothetical verification algorithm that treats points as public keys, and takes the point P directly as input would fail any time a point with odd Y is used. While it is possible to correct for this by negating points with odd Y coordinate before further processing, this would result in a scheme where every (message, signature) pair is valid for two public keys (a type of malleability that exists for ECDSA as well, but we don't wish to retain). We avoid these problems by treating just the X coordinate as public key."

In our specific case, we define FROST ciphersuite that will operate on the same types as the [FROST] algorithm. This is a requirement to make the ciphersuite used generic for [FROST].

Accepting the public key as a point and signature as a struct outputted from the aggregate function makes the code easier to follow as no conversions have to be made before the verification. Also, from our specific perspective, it does not make a difference where the conversion is made and where we strip the public key's Y coordinate information: between the aggregation and before the verification or inside the verification.

This is a draft implementation. The remaining BIP-340 test vectors need to be
added, TODOs need to be addressed, and one failing test needs to be fixed.

The VerifySignature verifies the provided BIP-340 signature for the message
against the group public key. The function returns true and nil error when the
signature is valid. The function returns false and an error when the signature
is invalid. The error provides a detailed explanation on why the signature
verification failed.

VerifySignature implements Verify(pk, m, sig) function as defined in BIP-340.

One important design decision is to accept *Signature and *Point into Verify
function instead of bytes, as in the prototype. This has the implication. To
ensure consistency and that we'll not return positive verification result for
(x,y) that is not on the curve, we need to verify y coordinate even though
BIP-340 verification is not really interested in it. I think this is acceptable
given the complexity of byte operations is hidden behind a ciphersuite and
inside the protocol code, we'll operate on known domain objects.
Added missing test vectors with one test vector failing - this requires
further investigation. Explained why we accept public key's Y coordinate
in parameters as opposed to what is proposed in BIP-340. tl;dr; support
for other ciphersuites and in FROST it does not matter where we strip Y
coordinate information: after aggregate and before the verification or
inside the verification.
EncodePoint is important for hashing, especially H2. It is not
important for the signature verification calls (in our current model) or
for the curve operations.
Comment on lines 496 to 501
signature: "4A298DACAE57395A15D0795DDBFD1DCB564DA82B0F269BC70A74F8220429BA1D69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B",
publicKeyX: "DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659",
message: "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89",
isValid: false,
expectedErr: "point R is infinite",
},
Copy link
Member Author

@pdyraga pdyraga Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test vector is problematic. In BIP-340 test vectors, the comment is:

sig[0:32] is not an X coordinate on the curve

So I would expect the test vector to fail on point R is infinite check. It does fail later on coordinate R.x != r. I checked and the point seems to be a valid point on the curve with [x,y] as
[101293062680523315514373137351023114440902235251657644508821325047911886333529, 95491709537915294920828256998521669146617750390665870859237534620269297521559]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature verification algorithm in BIP-340 does not check that the 32 public key bytes in the signature (rb in the code, r in the BIP) match a valid point on the curve. This check is effectively implicit in the algorithm later, because G and P are known valid curve points, so the point R that is calculated is also a valid point. Thus, if r is not a valid point's X-coordinate, R.X != r.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that makes sense! Fixed in 929f34d.

@pdyraga pdyraga changed the title Draft implementation of BIP-340 signature verification BIP-340 signature verification Jan 8, 2024
@pdyraga pdyraga requested a review from eth-r January 8, 2024 12:33
This test was previously failing.

The signature verification algorithm in BIP-340 does not check that the 32
public key bytes in the signature match a valid point on the curve. This check
is effectively implicit in the algorithm later, because G and P are known valid
curve points, so the point R that is calculated is also a valid point. Thus, if
r is not a valid point's X-coordinate, R.X != r.
Base automatically changed from aggregate to main January 8, 2024 15:38
This does not make any difference to the result but it is preferable to
stay in the field order for the clarity's sake.
@pdyraga
Copy link
Member Author

pdyraga commented Jan 8, 2024

#9 is merged now so I am undrafting this PR.

@pdyraga pdyraga marked this pull request as ready for review January 8, 2024 15:45
Copy link
Contributor

@eth-r eth-r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@eth-r eth-r merged commit 952dabd into main Jan 8, 2024
2 checks passed
@pdyraga pdyraga deleted the bip340-verify branch January 8, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants