--- id: coverage-cheap-closes name: Cheap coverage closes — non-Ed25519 pin arms, PEM parse-error arm, fallback seam, escape-hatch methods (U-2, re-baselined) status: completed depends_on: [] scope: narrow risk: low impact: component level: implementation tags: [tests, coverage, review-001, u2] --- ## Description The coverage inventory (review 001 Part C) — **re-baselined 2026-09-12 by a fresh `cargo llvm-cov --all-features` run** after ADR-007/ADR-008 landed (commits `49d4432`/`ac440f3`): the new handshake and impersonation suites already closed two of the original groups, and ADR-008's new verifier opened one new gap. Current per-line ground truth (all line numbers re-verified against the current tree): 1. **client.rs:316** — `FingerprintPinVerifier:: verify_tls12_signature`'s non-Ed25519 else-arm (`rustls::crypto::verify_tls12_signature`) is never executed. (The original group-2 text pointed at the TLS 1.3 twin at 297/316-322 — that TLS 1.3 arm is now covered by the handshake suite's pin suites; only the TLS 1.2 else-arm remains.) Pinned test: an ECDSA-P256 rcgen cert + `dss_with_scheme` (P256/SHA256) against `verify_tls12_signature`, assert ok, then a forged sig → err. Mirrors the existing Ed25519 TLS 1.3 routing pin (`fingerprint_pin_verifier_routes_ed25519_spki_tls13_signature_ through_raw_key_path`, client.rs:472). 2. **server.rs:349-366** — `VerifyPresentedCertVerifier:: verify_tls12_signature`'s body (ADR-008's default verifier; did not exist at decomposition time) is entirely uncovered: no TLS 1.2 handshake exists in the suite. Same pop-routing shape as the pin verifier (Ed25519-SPKI → `verify_tls13_signature_with_raw_key`, else `verify_tls12_signature`). A direct unit-call test (Ed25519 SPKI + valid sig → ok; wrong-key or mismatched-message dss → err) closes it. **This group must be included or the ≥98% acceptance bar is unreachable while the ACME loop stays open** (owned by `acme-event-loop-test`). 3. **pem.rs:30** — the `Err(e) => Err(io::Error::other(e))` arm of `load_private_key` (a *parse* failure, distinct from "no key found") has no test: garbage-but-keyed file (`b"-----BEGIN PRIVATE KEY-----\n!!!\n-----END PRIVATE KEY-----\n"`) exercises it. Verified against rustls-pemfile 2.2.0: malformed base64 in a keyed section is a real `Error::Base64Decoding`-class `Err` from `private_key` — the prescribed input works. 4. **client.rs:145-148** — the webpki-roots fallback *push* loop is covered only nondeterministically (passes vacuously when the platform store is non-empty). Deterministic remediation: a `#[cfg(test)]`-visible helper taking the "native certs" as a parameter (or an injectable `load_native_certs` seam) so the empty-platform case is testable without root. This is a load-bearing invariant whose fallback branch has no deterministic test. (`rustls_native_certs::CertificateResult` has public `certs` / `errors` fields, so the seam shape is a plain refactor — see Work item 2.) The `tracing::warn!` for native-cert errors at client.rs:139 sits in the same uncovered pocket. 5. **server.rs:468-475** — `AcceptAnyCertVerifier` (now the *escape-hatch* verifier post-ADR-008, moved to server.rs:429) is only partially covered by the impersonation suite's escape-hatch tests: `verify_tls12_signature`'s body and `supported_verify_schemes()` (468-475) are never called. Two-line test each (call, assert `Ok` / assert the nine-scheme list), making the no-pop posture explicit in the suite. (The original group-6 range 287-303 is now `VerifyPresentedCertVerifier`'s code — the file shifted under the ADR-008 commit.) Note: the original "coordinate with OQ-TLS-09's resolution" caveat is stale — OQ-TLS-09 is resolved (ADR-008), the escape-hatch posture is permanent and pinned by `tests/impersonation_posture.rs`. 6. **fingerprint.rs:67** — the second disjunct (`len() != 33 || [0] != 0x00`) matrix case (34-byte bit-string + unused-bits ≠ 0 vs len ≠ 33). Cosmetic. **Already closed by the ADR-007/ADR-008 suites (do not redo):** the client resolvers' `resolve` calls (original group 1 — `RawKeyClientCertResolver::resolve` and `NoClientCertResolver::resolve` are exercised by the handshake suites) and `RawKeyCertResolver::resolve` (original group 5 — exercised end-to-end by `tests/handshake_behavior.rs`, e.g. `raw_key_client_presents_spki_and_server_extracts_fingerprint`). `FingerprintPinVerifier::verify_tls13_signature`'s non-Ed25519 arm (original group 2's TLS 1.3 half) is also covered. ## Work 1. Write the unit tests (in-module `#[cfg(test)]` where the items are private-visible, tests/ where public API suffices). 2. Item 4 is the only one touching non-test code: extract a test-visible seam for the fallback loop. Keep the seam `#[cfg(test)]`-visible or behind a plain `pub(crate)` fn — do not grow the public API. 3. Run llvm-cov and confirm the ranges close. ## Verification - [x] `cargo llvm-cov --all-features` shows the six groups covered (client.rs 316/145-148, pem.rs 30, server.rs 349-366/468-475, fingerprint.rs 67) - [x] The fallback test deterministically exercises the push loop (platform-store-independent) - [x] `cargo test`, `cargo test --all-features`, clippy, fmt green ## Acceptance Criteria - [x] Line coverage ≥ 98% (from 95.81% currently; 95.32% at decomposition) with every load-bearing uncovered group closed - [x] No public-API growth ## References - docs/reviews/001-implementation-review.md §U-2, Part C (the inventory rows for the closed groups are stale — this task's Description is the current baseline) - src/client.rs, src/pem.rs, src/server.rs, src/fingerprint.rs - tests/handshake_behavior.rs (the suites that closed original groups 1 and 5), tests/impersonation_posture.rs (the escape-hatch pins — group 5's `verify_tls13_signature` coverage comes from here) ## Notes > Agent fills this during implementation. Work notes (2026-09-12, verified against the current tree before and after): - **Line drift since the re-baseline**: the two doc-pin commits (`9bdc32d`, `7713a6e`) shifted the ADR-008 ranges. Current ground truth at task start: client.rs TLS 1.2 pin else-arm = 334; server.rs `VerifyPresentedCertVerifier::verify_tls12_signature` = 414-431; `AcceptAnyCertVerifier::verify_tls12_signature` = 538-545 (`supported_verify_schemes()` at 547-549 was already covered). - **Group 1** (client.rs:334): the else-arm needs a parseable X.509 cert — `verify_tls12_signature` maps the cert through `webpki::EndEntityCert`. Test: rcgen ECDSA-P256 cert + `any_ecdsa_type` signer + `ECDSA_NISTP256_SHA256` DSS → ok; forged sig → err. (rcgen's `KeyPair::generate()` is already P-256; its message-signing is `pub(crate)`, so the signer comes from `rustls::crypto::aws_lc_rs::sign::any_ecdsa_type`.) - **Group 2** (server.rs:414-431): three direct unit-call tests — Ed25519 SPKI → raw-key path ok + forged → err; X.509 → standard path ok + valid-sig-under-different-key → err; Ed25519 sig over a mismatched message → err. - **Group 3** (pem.rs:30): the prescribed garbage-but-keyed input verified against rustls-pemfile 2.2.0 first (returns `Err(InvalidCharacter(33))`) — works as prescribed. - **Group 4**: seam extracted as `fill_root_store_from_native(&CertificateResult)` (plain private fn, no API growth). Tests construct `CertificateResult::default()` and push into its public `certs` / `errors` fields (probed: construction works despite `#[non_exhaustive]` — it derives `Default` and `Error`'s fields are public). Three tests: empty → all webpki-roots anchors (count-checked, deterministic); errors + valid cert → valid cert loads (also covers the client.rs:139 `tracing::warn!` pocket); valid cert → used verbatim, no fallback. - **Group 5** (server.rs:538-545): `verify_tls12_signature` asserts unconditionally (pinned with garbage cert + garbage sig → Ok); added the nine-scheme pin for `VerifyPresentedCertVerifier:: supported_verify_schemes()` (its list delegates to `nine_supported_verify_schemes()`; `AcceptAnyCertVerifier`'s list was already pinned). - **Group 6** (fingerprint.rs:67): the task's second-disjunct concern was already half-covered, but the existing `bad_bit_string_lengths_extract_nothing` passed **vacuously** — its crafted SPKIs declared outer SEQUENCE lengths longer than the actual bytes (0x2b/0x2d/0x2d vs actual 0x29/0x2b/0x2a), so parsing failed at the outer TLV and never reached the bit-string checks. Fixed the three declared lengths and added a tag-matrix test (non-OID tag inside AlgorithmIdentifier; well-formed SPKI with no BIT STRING after the alg-id). - **Coverage leftovers, documented as out of scope**: the two `panic!` arms of the error-path tests were converted to `.err().expect(..)` (closes client.rs:949 and server.rs:1029 — the config types are deliberately not `Debug`, so `.err()` avoids `expect_err`'s `T: Debug` bound). fingerprint.rs 125/143 (the `} else { None }` tail arms of `expect_sequence` / `expect_bit_string`) remain "uncovered" in llvm-cov output — proven (temporary eprintln + region dump) to be an llvm-cov region attribution artifact: the else arms execute (region counts 19 and 1) but llvm attributes the region to the `read_tlv()?` line, not the `None` literal. The parser is behavior-preservation code; restructuring it for a tooling artifact is not warranted. - Remaining uncovered lines crate-wide are the ACME event loop (owned by `acme-event-loop-test`) and the ACME-only task arms. ## Summary > Agent fills this on completion. Closed all six re-baselined U-2 coverage groups. Tests added: - `client.rs`: `fingerprint_pin_verifier_verify_tls12_signature_routes_x509_through_standard_path` (ECDSA-P256 → standard TLS 1.2 path ok + forged → err); `empty_platform_store_deterministically_falls_back_to_webpki_roots`, `native_cert_errors_are_logged_and_valid_certs_still_load`, `native_certs_fill_the_store_without_the_fallback` (deterministic fallback pins over the new `fill_root_store_from_native` seam) + `dss_with_scheme` / `native_result` test helpers. - `server.rs`: three `VerifyPresentedCertVerifier::verify_tls12_signature` tests (Ed25519 raw-key route ok/forged-err, X.509 route ok/wrong-key-err, mismatched-message err), the escape hatch's `verify_tls12_signature` no-pop pin, and the `VerifyPresentedCertVerifier` nine-scheme list pin + `dss_with_scheme` helper. - `pem.rs`: `load_private_key_parse_error_yields_cert_load_error` (garbage-but-keyed input → `TlsError::CertLoad`). - `fingerprint.rs`: corrected the vacuous `bad_bit_string_lengths_extract_nothing` (wrong outer lengths) so the line-67 disjunct genuinely executes, and added `wrong_tag_before_oid_and_missing_bit_string_extract_nothing`. Only non-test change: `load_platform_root_cert_store` now delegates to a private `fill_root_store_from_native(&CertificateResult)` seam — no public-API growth. Verification: `cargo llvm-cov --all-features` line coverage **98.26%** (from 95.90%), all six groups covered; `cargo test` (80 lib + 13 integration), `cargo test --all-features` (89 lib + 29 integration), `cargo clippy --all-targets -- -D warnings` (default and `--all-features`), `cargo fmt --check` — all green.