--- id: fix-accept-any-cert-verifier-posture name: S-1 remediation — AcceptAnyCertVerifier no-pop posture (doc + ADR/OQ + permanent probe) status: completed depends_on: [] scope: narrow risk: medium impact: project level: implementation tags: [security, docs, review-001, s1] --- ## Description The `AcceptAnyCertVerifier` (src/server.rs:257-318) never checks the client's CertificateVerify signature — no proof-of-possession — so any party that sees a peer's public cert/SPKI can complete a handshake *as that peer*, and the server hands the auth layer the victim's fingerprint. Confirmed by executable probe during review 001 and **re-verified by a fresh probe in the 001-decomposition session** (both X.509 and RFC 7250 raw-key variants complete the handshake with the victim's cert + the attacker's signing key; `peer_certificates()` yields the victim's cert and the server-extracted fingerprint equals the victim's). The verifier behavior itself is the alknet-inherited design (alknet ADR-034) and is behavior-preserving — this is a **documentation/posture** finding, not a code-change mandate. What's missing: 1. The load-bearing doc on `AcceptAnyCertVerifier` (src/server.rs:242-256) states "does not verify the presented cert against a CA" but nowhere states the signature is unverified. To a consumer that reads as the *only* gap, when the actual consequence is: **presented identity is spoofable by anyone with the public bytes**. 2. No ADR/OQ records which layer owns proof-of-possession on the server path. 3. The spoofable posture has no permanent test pinning it — a refactor that *adds* pop (or upstream changes the verifier contract) would land green, silently changing the documented behavior. ## Work 1. **Doc on `AcceptAnyCertVerifier`**: state explicitly that the client's CertificateVerify signature is not verified (no proof-of-possession), that consequently the fingerprint the server extracts is attacker-suppliable from observed peer fingerprints, and point at the two safe patterns the auth layer can own: - challenge-response over the established channel bound to the cert's public key, or - a server-side verifier that *does* verify the CertificateVerify (the same routing the client-side `FingerprintPinVerifier` implements — the crate has the code, just not on this type). 2. **N-1 companion line on `FingerprintPinVerifier`** (src/client.rs:232-236): fix the "stolen-but-stale fingerprint" phrasing (the cert is presented fresh each time — the real threat is a stolen *private key*) and add the cross-reference: this verifier checks proof-of-possession; the server-side `AcceptAnyCertVerifier` does not. 3. **Record OQ-TLS-09** in `docs/architecture/open-questions.md`: which layer owns proof-of-possession on the server path? Options: (a) keep request-but-don't-require + auth-layer challenge-response; (b) additive `VerifyPresentedCertVerifier` sibling (request-and-verify — same nine-scheme list, permissive `verify_client_cert`, signature methods delegate to `rustls::crypto::verify_tls{12,13}_signature(_with_raw_key)`); (c) make the auth layer's fingerprint resolution possession-checked. Until decided, the S-1 doc note is the honest description. If option (b) is chosen, it must land before the first consumer. 4. **Permanent behavior-pin test** (tests/, tcp-feature-gated): the impersonation probe as a documented test — server config via `build_rustls_server_config`, a client presenting the victim's cert bytes with an attacker signer completes the handshake and `peer_certificates()` yields the victim's fingerprint. Assert the spoofable posture in both directions (X.509 + raw-key SPKI). If a future change adds pop, this test fails and forces the doc/ADR update. Probe shape: rustls `ServerConnection`/`ClientConnection` pair over a duplex, `complete_io` on each — the 001-decomposition session's probe (deleted after its run) is the reference; a client-side test-local accept-anything verifier keeps the probe independent of the pin path. ## Verification - [x] The doc note exists on `AcceptAnyCertVerifier` and states the no-pop consequence in one reading - [x] The N-1 companion line exists on `FingerprintPinVerifier` - [x] OQ-TLS-09 recorded with status `open` and the three options - [x] The permanent probe test passes and asserts the spoofable fingerprint end-to-end (both cert types) - [x] `cargo test`, `cargo clippy --all-targets -- -D warnings`, `cargo fmt --check`, `cargo doc --no-deps` green - [x] `cargo test --all-features` green (the probe is tcp-gated) ## Acceptance Criteria - [x] A consumer reading only the rustdoc understands that presented identity is spoofable without caller-enforced possession - [x] The spoofable posture is pinned by an executed test, not just prose - [x] The ownership question is tracked (OQ-TLS-09) — not silently inherited ## References - docs/reviews/001-implementation-review.md §S-1 (the finding) and §N-1 - src/server.rs:242-318 (`AcceptAnyCertVerifier`), src/client.rs:225-323 (`FingerprintPinVerifier` — the pop-complete routing to mirror) - alknet ADR-034 (the inherited request-but-don't-require shape) - docs/architecture/open-questions.md (where OQ-TLS-09 lands) ## Notes - The probe used a tokio duplex pair + `TlsConnector`/`TlsAcceptor` (tokio-rustls, `tcp`-gated) rather than raw `complete_io` — same shape, cleaner await-based flow. The attacker client uses a test-local accept-anything `ServerCertVerifier` and a fixed `ResolvesClientCert` presenting the victim's bytes with the attacker's signer, `only_raw_public_keys() == false` in both variants (the cert-type negotiation stays X.509-typed, exactly the review-probe shape; verified against rustls 0.23.44's `validate_client_cert_type_extension` — an offer of only `[RawPublicKey]` would fail `IncorrectCertificateTypeExtension` against `requires_raw_public_keys() == false`, so the SPKI is presented under the default X.509 offer and passes through unparsed). - N-4 (the `requires_raw_public_keys()` doc note) is deliberately NOT in this task — it belongs to `docs-pin-c1-c4-n3-n4`. While verifying the probe shape, the review's N-4 parenthetical ("alknet's own client resolver offers both types") was found inaccurate: rustls 0.23.41 AND 0.23.44 send `client_certificate_types = [RawPublicKey]` whenever the resolver's `only_raw_public_keys()` is true — there is no "offers both types" behavior in either version. Task 6 should write the N-4 note from the rustls sources, not from the review's parenthetical. - Docs synced beyond the type docs: `docs/architecture/server.md` § `AcceptAnyCertVerifier` and `docs/architecture/client.md` § `FingerprintPinVerifier` now carry the S-1 posture + the N-1 cross-reference. ## Summary **Landed (this task):** 1. `src/server.rs` — `AcceptAnyCertVerifier` doc rewritten: a "# The presented signature is NOT verified (no proof-of-possession)" section stating the spoofable-fingerprint consequence, the two safe patterns (challenge-response / a verifying verifier), the OQ-TLS-09 pointer, and the pinning-test reference. 2. `src/client.rs` — `FingerprintPinVerifier` doc: the "stolen-but-stale fingerprint" phrasing replaced (the cert is presented fresh each handshake; the signature check defeats use of a stolen/observed cert by a party without the private key) plus the cross-reference: this verifier checks pop; the server-side `AcceptAnyCertVerifier` does not. 3. `docs/architecture/open-questions.md` — OQ-TLS-09 recorded, status **open**, the three options (a/b/c), the pinning-test constraint, and the deferral note (the decision needs the auth-layer design or an API call before the first consumer). 4. `tests/impersonation_posture.rs` (tcp-gated) — the impersonation probe made permanent, both variants: X.509 victim cert + attacker key, and RFC 7250 victim SPKI + attacker key; each asserts the handshake completes, application data flows, and the server-extracted fingerprint equals the victim's. **Verification:** 68 default / 77 all-features tests green (+2 new); clippy `-D warnings` clean (default + all-features); fmt clean; `cargo doc --no-deps` warning-free.