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

Schnorr signature support #30

Merged
merged 6 commits into from
Jan 21, 2025
Merged

Conversation

1ma
Copy link

@1ma 1ma commented Jan 21, 2025

Solves #28

This PR imports the SchnorrSignature class and its supporting code from public-square/phpecc. I just made minimal changes to pass the psalm linter, fix some PHP 8.4 deprecations and make the code compatible with PHP 7.1.

The test vectors used in SchnorrSignatureTest are based on the reference test vectors in Bitcoin Improvement Proposal 340. On a cursory look it doesn't seem like Wycheproof has any additional test vectors for Schnorr signatures.

1ma added 6 commits January 21, 2025 00:58
phpecc/phpecc@master...public-square:phpecc:main

The changes in Point.php seem almost all related to linting. The new $windowSize
attribute with a setter and getter is not used anywhere, so I haven't imported
the changes in that class.
@paragonie-security paragonie-security merged commit 432bb18 into paragonie:master Jan 21, 2025
6 checks passed
// scalar is private key if Y is even, otherwise it's order - scalar
$scalar = $d;

if ($isEvenY === false) {

Choose a reason for hiding this comment

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

I would recommend avoiding branches in signing.

$point = $generator->mul($d);

// is the Y coordinate even?
$isEvenY = gmp_cmp(gmp_mod($point->getY(), 2), 0) === 0;

Choose a reason for hiding this comment

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

Just a head's up, gmp_cmp() is timing-leaky.

@1ma 1ma deleted the schnorr branch January 21, 2025 16:53
@paragonie-security
Copy link

I've fixed the timing leaks in the master branch, and will cut a release soon.

Please feel free to report these to the source repository for this Schnorr implementation.

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