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

Replace crypto lib ecdsa with coincurve #312

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pnowosie
Copy link

This PR replaces ecdsa with coincurve for crypto signing

  • ecdsa, being a pure Python implementation, is vulnerable to side-channel attacks
  • Ensured signature format aligns with expected canonical 64-bytes
  • All tests passing, maintaining functionality

@pnowosie pnowosie requested review from a team as code owners January 14, 2025 09:16
@pnowosie pnowosie force-pushed the pnowosie/replace-ecdsa branch 2 times, most recently from 5b55752 to cfe0efb Compare January 14, 2025 11:18
@pnowosie pnowosie force-pushed the pnowosie/replace-ecdsa branch from cfe0efb to 5b89330 Compare January 14, 2025 11:21
@@ -28,6 +28,7 @@ pytest-asyncio = "^0.23.6"
python-dotenv = "^1.0.1"
twine = "^4.0.2"
black = "^24.4.0"
ecdsa = "^0.19.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to remove this dependency entirely?

Copy link
Author

Choose a reason for hiding this comment

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

Definitelly 👍 It's removed in c9d92f1

For clarity ecdsa is no longer direct dependency but still will be pulled as bip-utils depends of it.
Based on Snyk Advisor, bip-utils does not have these vulnerabilities and is considered secure.

@pnowosie pnowosie requested a review from ttl33 January 15, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants