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

Bump dependencies to fix vulnerability from cargo audit #223

Conversation

wulfraem
Copy link
Contributor

Description of change

Running cargo audit in identity.rs currently producing errors due to vulnerabilities in the current curve25519-dalek version:

Crate:     curve25519-dalek
Version:   3.2.0
Title:     Timing variability in `curve25519-dalek`'s `Scalar29::sub`/`Scalar52::sub`
Date:      2024-06-18
ID:        RUSTSEC-2024-0344
URL:       https://rustsec.org/advisories/RUSTSEC-2024-0344
Solution:  Upgrade to >=4.1.3
Dependency tree:
curve25519-dalek 3.2.0
├── x25519-dalek 1.1.1
│   ├── iota-crypto 0.23.1
│   └── age 0.9.2
│       └── iota-crypto 0.23.1
└── iota-crypto 0.23.1

error: 1 vulnerability found!

Same audit message could be produces when running cargo audit in crypto.rs. This PR bumps the dependencies to fix the vulnerability.

Changes:

  • bump dependencies to latest versions:
    • curve25519-dalek
    • x25519-dalek
    • age
  • remove by now outdated feature u64_backend for
    • curve25519-dalek
    • x25519-dalek
  • enable features for x25519-dalek:
    • static_secrets
    • zeroize
  • fix cargo clippy messages from CI

The features static_secrets and zeroize had to be enabled for x25519-dalek so that identity.rs to be able to use crypto.rs with the updated dependencies.

⚠️ There may be dragons (or not)

Interestingly, all tests run with cargo test --lib --all --all-features --tests completed successfully without the features static_secrets and zeroize, but building identity.rs against a version without those features enabled fails due to:

  • x25519_dalek::StaticSecret not being found in crypto.rs/src/keys/x25519.rs:105
  • not being able to derive Zeroize and ZeroizeOnDrop in crypto.rs/src/keys/x25519.rs:105

So the outward behavior changes without those features.

I'm not entirely sure, if similar behavior changes may affect other dependents similarly for other features.

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix
  • Chore?

How the change has been tested

cargo audit on current dev should produce the message from above, doing the same on this branch should not.

Tested locally and verified with CI.

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@semenov-vladyslav semenov-vladyslav merged commit 489aa0e into iotaledger:dev Jul 15, 2024
6 of 7 checks passed
This was referenced Aug 14, 2024
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