evervault-cli - patch 2 dependencies - #248
Conversation
There was a problem hiding this comment.
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):KeyValueMapserialize panic. Requires the app to use#[serde_as(as = "KeyValueMap<_>")]on attacker-controlled data — not the case here;serde_withis 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-existingev-cliwarnings)cargo fmt --check❌ on the PR's new code — this is what was failingclippy_check_cliCI. I rancargo fmt(only reformats the PR's new lines inattest/mod.rs), committed and pusheda19d12ato this branch. It now passes.- Tests: the only failures are
403 Forbiddenfrom 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" } |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
Fixes COM-2407