Skip to content

fix: replace ecdsa with cryptography to mitigate Minerva timing attack#400

Open
serge-eric-kalaga wants to merge 1 commit intompdavis:masterfrom
serge-eric-kalaga:fix/remove-ecdsa-timing-vulnerability
Open

fix: replace ecdsa with cryptography to mitigate Minerva timing attack#400
serge-eric-kalaga wants to merge 1 commit intompdavis:masterfrom
serge-eric-kalaga:fix/remove-ecdsa-timing-vulnerability

Conversation

@serge-eric-kalaga
Copy link

Summary

  • Replace the python-ecdsa dependency with cryptography (PyCA) to eliminate exposure to the Minerva timing side-channel attack
  • cryptography relies on OpenSSL which provides hardened, constant-time ECDSA implementations
  • Fix a pre-existing test bug where SECP256R1 was passed as a class instead of an instance

Changes

  • setup.cfg: move cryptography >=3.4.0 from optional extras to install_requires; remove ecdsa
  • requirements.txt: replace ecdsa with cryptography >=3.4.0
  • jose/backends/__init__.py: remove the ecdsa_backend fallback for ECKey
  • jose/utils.py: remove the ecdsa.ecdsa.int_to_string fallback for long_to_bytes
  • tests/algorithms/test_EC.py: fix SECP256R1SECP256R1() (API change in newer cryptography versions)

Security

The python-ecdsa library is vulnerable to the Minerva timing attack, which can leak nonce information during ECDSA signing and potentially allow private key recovery. The upstream maintainers consider side-channel attacks out of scope and have no plans to fix this.

Testing

All existing tests pass (458 passed, 12 skipped).

The python-ecdsa library is vulnerable to the Minerva timing side-channel
attack, which can leak nonce information during ECDSA signing and potentially
lead to private key recovery. The upstream maintainers have no plans to fix it.

- Make `cryptography` a required dependency (was optional)
- Remove `ecdsa` from install_requires and requirements.txt
- Remove ecdsa fallback in backends/__init__.py and utils.py
- Fix pre-existing test bug: SECP256R1 must be instantiated (SECP256R1())
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.

1 participant