Skip to content

evervault-cli - patch 2 dependencies - #248

Open
evervault-dependencies wants to merge 2 commits into
mainfrom
COM-2407/batch-dependency-bumps
Open

evervault-cli - patch 2 dependencies#248
evervault-dependencies wants to merge 2 commits into
mainfrom
COM-2407/batch-dependency-bumps

Conversation

@evervault-dependencies

Copy link
Copy Markdown
Contributor

Fixes COM-2407

@evervault-dependencies
evervault-dependencies requested a review from a team as a code owner July 23, 2026 18:42

@ev-vaultkeeper ev-vaultkeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dependency review — safe to merge (pushed a formatting fix)

Heads up on scope: the title says patch 2 dependencies, but this PR is effectively a major rustls upgrade. Bumping rustls-webpki to 0.103.13 (the security driver) pulls rustls 0.21 → 0.23 and the direct dep tokio-rustls 0.24 → 0.26, and switches the crypto backend to aws-lc-rs (aws-lc-rs/aws-lc-sys are new). Full set: serde_with 3.12→3.21, rustls-webpki 0.101→0.103.13, rustls 0.21→0.23, tokio-rustls 0.24→0.26, serde_json 1.0.138→1.0.151.

Advisories fixed

  • GHSA-965h-392x-2mh5 / GHSA-xgp8-3hg3-c2mh (rustls-webpki, both Low): name-constraint handling for URI/wildcard names. Only reachable after signature verification and require cert misissuance — and this crate uses a custom verifier that bypasses webpki chain validation anyway.
  • GHSA-7gcf-g7xr-8hxj (serde_with, Moderate): KeyValueMap serialize panic. Requires the app to use #[serde_as(as = "KeyValueMap<_>")] on attacker-controlled data — not the case here; serde_with is purely transitive (COSE/attestation crates).

Migration correctness (only consumer: crates/ev-enclave/src/attest)

The custom SubjectAltNameAttestationValidator previously overrode only verify_server_cert and relied on rustls 0.21's default verify_tls12/13_signature + supported_verify_schemes (which still verified the handshake signature against the leaf key). In 0.23 those methods are required, and the PR re-implements them by delegating to rustls::crypto::verify_tls12/13_signature with the default provider's algorithms. Behaviour is preserved: handshake signatures are still verified; only cert-chain trust is intentionally bypassed in favour of attestation-doc validation, exactly as before. CryptoProvider::get_default() is safe here because ClientConfig::builder() (called first) installs the process-default provider, and only one provider feature (aws_lc_rs) is enabled on rustls (verified via cargo tree -e features), so there's no ambiguity/panic. The added Debug impl and the pki_types::InvalidDnsNameError move are both correct for 0.23.

Checks I ran (rustc 1.96.1)

  • cargo build --workspace
  • cargo clippy ✅ (exit 0; only pre-existing ev-cli warnings)
  • cargo fmt --check ❌ on the PR's new code — this is what was failing clippy_check_cli CI. I ran cargo fmt (only reformats the PR's new lines in attest/mod.rs), committed and pushed a19d12a to this branch. It now passes.
  • Tests: the only failures are 403 Forbidden from the sandbox network-egress proxy (attestation endpoint / test APIs are not allow-listed here) — not code regressions. 70 unit tests pass; these network tests should run normally in CI.

One thing to confirm before/at merge

aws-lc-rs/aws-lc-sys require a C compiler + cmake at build time. Please make sure the release pipeline — especially the newly added ARM64 cross-compile targets — has that toolchain and cross-compiles aws-lc-sys cleanly. If cross-compilation is problematic, consider switching tokio-rustls to the ring provider instead of the default aws-lc-rs (default-features = false + ring), which would also keep CryptoProvider::get_default() valid.

minus = { version = "5.6.1", features = ["static_output"] }
exitcode = "1.1.2"
tokio-rustls = { version = "0.24", features = ["dangerous_configuration"] }
tokio-rustls = { version = "0.26" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note: dropping the explicit features here moves rustls to its default aws-lc-rs backend, which adds aws-lc-sys (C/asm) as a build-time dep requiring a C compiler + cmake. Worth double-checking the ARM64 cross-compile release targets build this cleanly; otherwise default-features = false + ring is an alternative that keeps the code below working.

cert: &CertificateDer<'_>,
dss: &DigitallySignedStruct,
) -> Result<HandshakeSignatureValid, Error> {
let provider = CryptoProvider::get_default()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This correctly re-creates rustls 0.21's default handshake-signature verification (the old custom verifier relied on the trait defaults), so signatures are still checked against the leaf cert — behaviour preserved. get_default() is guaranteed Some here because ClientConfig::builder() runs first and installs the process-default provider, and only the aws_lc_rs provider feature is enabled, so the no crypto provider installed branch is unreachable in this flow.

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