fix(tls): migrate off rustls-pemfile onto rustls-pki-types, drop RUSTSEC-2025-0134 ignore (task #66)#338
Merged
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe TLS configuration now parses certificates, private keys, and CA bundles with ChangesTLS PKI migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…es (task #66) rustls-pemfile 2.2.0 is unmaintained (RUSTSEC-2025-0134) with no safe drop-in upgrade in the 2.x line. It was ignore-listed in deny.toml/.cargo/audit.toml when the supply-chain CI gate was wired up (#326) specifically because the real fix required touching src/tls.rs, out of scope for that CI-wiring PR. This closes that follow-up. build_tls_config (src/tls.rs) now parses PEM directly via rustls-pki-types' PemObject trait, already present transitively through rustls's own pki_types re-export: CertificateDer::pem_reader_iter for cert chains (server cert + optional CA chain for mTLS) and PrivateKeyDer::from_pem_reader for the private key. rustls-pki-types is promoted to a direct optional dependency (features = ["std"], needed for the reader-based PemObject methods) gated behind the same runtime-tokio/runtime-monoio feature flags rustls-pemfile used. No behavior change: same fail-loud io::Error wrapping per stage (TLS cert file/parse, TLS key file/parse, CA cert parse), same SIGHUP hot-reload path (src/tls.rs spawn_sighup_reload_thread), same cipher-suite allowlist and mTLS client-verifier logic — untouched. Test coverage gap found during migration: the existing suite only exercised missing-file paths (NotFound), never a genuine DER/PEM decode failure — the exact seam this migration touches. Added two tests exercising well-formed- PEM-markers-but-corrupt-body input directly against the new parser: test_build_tls_config_garbage_cert_parse_error and test_build_tls_config_garbage_key_parse_error, both asserting the expected "TLS cert parse" / "TLS key parse" error context survives the migration. All 11 tls:: tests pass (including the two openssl-backed real-cert reload tests, unchanged). rustls-pemfile is fully removed from Cargo.toml/Cargo.lock. The RUSTSEC-2025-0134 ignore entries are removed from deny.toml and .cargo/audit.toml; `cargo deny check advisories licenses bans sources` (0.19.6) and `cargo audit` (0.22.2) both pass clean against the resulting lockfile with no ignore needed for this advisory. Also updated .add/dependencies.allowlist and .github/dependabot.yml's crypto-tls group to reflect the dependency swap. Verified: cargo fmt --check; cargo clippy -- -D warnings (default features); cargo clippy --no-default-features --features runtime-tokio,jemalloc -- -D warnings; cargo test --lib tls:: (11/11 passed, both feature sets); cargo deny check advisories licenses bans sources; cargo audit. task: #66 author: Tin Dang <tindang.ht97@gmail.com>
TinDang97
force-pushed
the
fix/t66-rustls-pki-types
branch
from
July 15, 2026 16:21
257de50 to
2211cb0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Task #66: migrate off the unmaintained
rustls-pemfilecrate ontorustls-pki-typesPEM parsing, and drop the RUSTSEC-2025-0134 ignore from the supply-chain gate.What changed
src/tls.rs: certificate/key loading now usesCertificateDer::pem_file_iter/PrivateKeyDer::from_pem_file(covers server TLS init and the SIGHUP hot-reload path); error behavior stays fail-loud with equivalent diagnostics; parsing-seam tests added (cert chains, PKCS8/RSA/EC keys, garbage input).Cargo.toml/Cargo.lock:rustls-pemfileremoved from the dependency tree entirely.deny.toml+.cargo/audit.toml: RUSTSEC-2025-0134 ignore removed —cargo deny check advisoriesandcargo auditpass without it..add/dependencies.allowlist,.github/dependabot.yml: dependency bookkeeping updated to match.Verification
cargo fmt --check✅cargo clippy -- -D warnings✅ (default features)cargo clippy --no-default-features --features runtime-tokio,jemalloc -- -D warnings✅Summary by CodeRabbit