--- id: docs-pin-c1-c4-n3-n4 name: Docs + pin batch — self-signed validity, fingerprint pin format-exactness, Clone-under-no-acme, cert-type negotiation note (C-1, C-4, N-3, N-4) status: completed depends_on: [] scope: narrow risk: low impact: component level: implementation tags: [docs, review-001, c1, c4, n3, n4] --- ## Description Four doc/doc+pin findings that close as rustdoc (plus one decided question): 1. **C-1** — `generate_self_signed_cert()` produces a cert valid 1975→4096 (rcgen's `CertificateParams::default()` — verified against rcgen 0.13.2 source, certificate.rs:94-96) and has no SANs. "Dev cert" silently meaning "never expires" is a documented-behavior gap. One doc line on `generate_self_signed_cert` (and `SelfSignedCert`): the generated cert never expires in practice, carries no SANs, and pins against CA verification by design; if a tighter validity is ever wanted, `not_before`/`not_after` are additive params. 2. **C-4** — `RemoteIdentity::fingerprint` accepts any string and the pin comparison is exact-string (src/client.rs:293): uppercase-hex pins, `sha256:` lowercase prefix, and cross-prefix pins (`SHA256:`-of-an-ed25519-remote) construct fine and reject at handshake — fail-closed, never a downgrade, but a config-author trap. Doc line on `RemoteIdentity::fingerprint` (and/or `FingerprintPinVerifier`): pins must be produced by `fingerprint_from_cert_der` — case- and format-exact (`ed25519:` / `SHA256:`); malformed pins fail closed at handshake. **Update for ADR-007 (commit `49d4432`): the pin format is no longer only a verification selector — it also selects the cert-type offer (`ed25519:` → offer `[RawPublicKey]`, `SHA256:` → default X.509 offer), and a cross-format mismatch now fails earlier, at cert-type negotiation, before the pin compare is even reached (pinned by `tests/handshake_behavior.rs`: `ed25519_pin_against_x509_server_fails_closed_at_negotiation`). The review's original C-4 mechanism ("rejected at pin comparison") is superseded — same fail-closed verdict, earlier failure point. Write the doc line from that chain: mismatched *format* fails at negotiation (a config error, ADR-007); same-format-but-wrong pins (case, garbage) construct and fail at the pin compare.** 3. **N-3** — `TlsServerConfig` is not `Clone` even under `default = []` where the struct has no `JoinHandle` field. The all-features posture is what the API freeze pins (conditional `Clone` is its own trap: an API difference between feature configurations). Decision: keep non-Clone for v1; record the reasoning in the type's doc (one sentence) and close the question — revisit only with a concrete consumer demanding it. 4. **N-4** — **re-scoped 2026-09-12: the mechanism analysis this item originally asked for is already documented.** ADR-007 (`docs/architecture/decisions/007-cert-type-negotiation.md`, commit `49d4432`) records the whole negotiation chain, and `RawKeyClientCertResolver`'s doc block (src/client.rs:152-171) carries the client-side mechanism note (`only_raw_public_keys() == false` → X.509 offer; the `(false, true, false)` arm reasoning). Do not re-derive or duplicate that analysis. What remains is a short server-verifier note (a sibling section in the `AcceptAnyCertVerifier` and `VerifyPresentedCertVerifier` docs, or one shared paragraph referenced from both): - `requires_raw_public_keys()` stays `false` on **both** server verifiers — do not "fix" it to `true` (that would reject X.509 clients; the request-but-don't-require shape accepts both cert types). - Post-ADR-007, the crate's own `RawKeyClientCertResolver` presents the SPKI under the **X.509 offer** unconditionally (`client.rs:187`, `raw_public_keys: false`), so a raw-key client against this crate's servers never sends a raw-only offer. The `IncorrectCertificateTypeExtension` rejection of a raw-only client offer (`(false, true, false)` arm) can now only arise from a *foreign* rustls resolver that sets `only_raw_public_keys() == true` — state it as an interop boundary of the request-not-require shape, fail-closed, not a downgrade. - A raw-key *server* presentation (`RawKeyCertResolver`, `only_raw_public_keys() == true` on the server-cert side) is the other knob (`server_certificate_types`) and is unaffected. Without the note, the next reader may "fix" the verifier to `true` and break X.509 clients, or mis-diagnose a foreign raw-only client failure as a bug. ## Work 1. C-1 doc lines (`generate_self_signed_cert` + `SelfSignedCert`). 2. C-4 doc lines (`RemoteIdentity::fingerprint`, `FingerprintPinVerifier` — format-exactness + fail-closed, with the ADR-007 negotiation-earlier failure point per the Description). 3. N-3: the one-sentence decision note on `TlsServerConfig`'s doc; close N-3 in the review's finding list (no code change). 4. N-4: the short server-verifier negotiation note per the re-scoped item 4 in the Description (a sibling section on `AcceptAnyCertVerifier` + `VerifyPresentedCertVerifier`, or one shared paragraph referenced from both). If desired, add a pinned unit test asserting `requires_raw_public_keys() == false` stays default (it's a trait default — the test just documents the choice). 5. While in these docs: `FingerprintPinVerifier`'s pop cross-ref (src/client.rs:253-255) currently says only "the **server-side** `AcceptAnyCertVerifier` does not [verify possession]" — post-ADR-008 the *default* server verifier `VerifyPresentedCertVerifier` does; mention both (the default verifies, the escape hatch does not). 6. Cross-check the review's finding numbering so each doc change cites its finding. ## Verification - [ ] All four doc notes exist and render (`cargo doc --no-deps` warning-free) - [ ] The N-3 decision is recorded (doc + this task's Summary) - [ ] No code-behavior change beyond an optional unit pin - [ ] `cargo test`, clippy, fmt, doc green ## Acceptance Criteria - [ ] A config author reading the rustdoc cannot mis-case a pin, pin the wrong prefix, or expect a never-expiring dev cert to expire - [ ] The raw-only-client interop boundary is documented before a consumer hits it (short note per the re-scope; the full mechanism lives in ADR-007) ## References - docs/reviews/001-implementation-review.md §C-1, §C-4, §N-3, §N-4 (§N-4 — the correction blocks are historical context now; ADR-007 is the authoritative record of the negotiation mechanism), and the §Status block - src/server.rs:223-257 (the self-signed helper + `SelfSignedCert`), src/server.rs:18-24 (`TlsServerConfig` — where the N-3 note goes), src/client.rs:253-255 (the pop cross-ref), src/credentials.rs:36-38 (`RemoteIdentity::fingerprint`), src/identity.rs (SelfSigned doc) - ADR-007 (`docs/architecture/decisions/007-cert-type-negotiation.md`) — the authoritative negotiation record; ADR-008 (the verifier default change that makes `VerifyPresentedCertVerifier` part of the N-4 note) - rustls 0.23.44 `server/hs.rs::process_cert_type_extension` (the negotiation table) and `client/hs.rs` (the `[RawPublicKey]`-only offer rule) — consult only if extending the analysis beyond what ADR-007 records - tests/impersonation_posture.rs (its fixed resolver pins `only_raw_public_keys() == false` — post-ADR-007 that is also the crate resolver's unconditional shape), tests/handshake_behavior.rs (`ed25519_pin_against_x509_server_fails_closed_at_negotiation` — the negotiation fail-closed pin) ## Notes - C-1: doc blocks added on both `SelfSignedCert` and `generate_self_signed_cert` (src/server.rs): the cert is valid 1975→4096 (rcgen 0.13's `CertificateParams::default()`) — never expires in practice — carries no SANs, so CA verification of it fails as expected; it pairs with `SHA256:` fingerprint pinning (a pin that outlives any plausible deployment by design) and can never *fail* the way an ACME/ops-managed cert can; `not_before`/`not_after` are additive if tighter validity is ever wanted. The `cert_chain` field doc repeats the validity/no-SAN facts briefly. - C-4: two doc blocks written from the ADR-007-corrected chain: - `RemoteIdentity::fingerprint` (src/credentials.rs): the pin must be produced by `fingerprint_from_cert_der`, case- and format-exact (lowercase hex, `ed25519:` / `SHA256:` prefixes); same-format-but-wrong pins (uppercase hex, lowercase `sha256:`, garbage) construct and fail closed at the pin compare; mismatched *format* fails earlier, at cert-type negotiation (ADR-007), before the pin compare is reached — same fail-closed verdict, earlier failure point. - `FingerprintPinVerifier` (src/client.rs): a "The pin is case- and format-exact" section covering the same chain from the verifier's side (exact-string compare; the pin format also selects the server cert-type offer per ADR-007, so a wrong-format pairing aborts at negotiation). - N-3: one decision paragraph added to `TlsServerConfig`'s doc (src/server.rs): not `Clone` for v1, kept so the API is identical across feature configurations (conditional `Clone` would be an API difference between feature sets); share via `Arc`, revisit only if a concrete consumer demands it. N-3 closed in review 001 (`docs/reviews/001-implementation-review.md` §N-3 remediation status, plus a status-block note). - N-4: a "Client-cert-type negotiation (ADR-007, review 001 §N-4)" section added on `VerifyPresentedCertVerifier` (the default verifier), stating: `requires_raw_public_keys()` stays on the trait default `false` on both server verifiers — do not "fix" it to `true` (that would reject every X.509 client); the crate's own `RawKeyClientCertResolver` presents the SPKI under the X.509 offer unconditionally, so a raw-only client offer rejection (`IncorrectCertificateTypeExtension`, rustls `server/hs.rs::process_cert_type_extension`'s `(false, true, false)` arm) can only arise from a *foreign* resolver with `only_raw_public_keys() == true` — an interop boundary of the request-not-require shape, fail-closed, not a downgrade; the raw-key *server* side (`RawKeyCertResolver`, `server_certificate_types`) is the other knob and unaffected. `AcceptAnyCertVerifier` carries a cross-reference paragraph ("the same shape and rationale apply verbatim") instead of a duplicate. No mechanism re-derivation — ADR-007 stays the authoritative record. Pinned by a new unit test `server_verifiers_keep_requires_raw_public_keys_default_false` (src/server.rs tests) asserting the trait-default `false` on both verifiers. - Work item 5 (pop cross-ref): `FingerprintPinVerifier`'s closing paragraph now says the **default** `VerifyPresentedCertVerifier` verifies possession (ADR-008, resolving OQ-TLS-09) and the `AcceptAnyCertVerifier` escape hatch does not — the pre-ADR-008 sentence that named only `AcceptAnyCertVerifier` is gone. - Work item 6 (finding citations): every doc block names its finding (§C-1, §C-4, §N-3, §N-4) and review 001's status block records the batch closure, including the §C-4 supersession note. - Review 001 updates: §N-3 marked "closed by decision"; Status block gained a "Status update (2026-09-12, docs-pin batch)" paragraph. - ADR-007 doc-link style note: rustdoc renders the `../docs/architecture/decisions/007-cert-type-negotiation.md` relative links from `src/credentials.rs` / `src/server.rs`; verified present in the rendered HTML (`cargo doc --no-deps` warning-free, links resolve to the repo file path). ## Summary All four doc/doc+pin findings closed as rustdoc (plus one decided question), no code-behavior change beyond the optional unit pin: 1. `src/server.rs` — C-1 validity/no-SAN facts on `SelfSignedCert` + `generate_self_signed_cert`; N-3 one-sentence non-Clone decision on `TlsServerConfig`; N-4 "Client-cert-type negotiation" section on `VerifyPresentedCertVerifier` + cross-ref on `AcceptAnyCertVerifier`; new pinned unit test `server_verifiers_keep_requires_raw_public_keys_default_false` (asserts the trait-default `false` on both server verifiers). 2. `src/credentials.rs` — C-4 format-exactness block on `RemoteIdentity::fingerprint` (pin produced by `fingerprint_from_cert_der`; same-format-but-wrong → pin-compare rejection; cross-format → ADR-007 negotiation abort). 3. `src/client.rs` — C-4 "case- and format-exact" section on `FingerprintPinVerifier` (with the ADR-007 offer-selection chain); work item 5: the pop cross-ref now names both server verifiers (default verifies per ADR-008, escape hatch does not). 4. `docs/reviews/001-implementation-review.md` — §N-3 closed by decision; Status block records the C-1/C-4/N-3/N-4 closure and the §C-4 mechanism supersession. Verification: `cargo test` (default, 69 lib + integration) and `cargo test --all-features` (78 lib tests, new pin test passing), `cargo clippy --all-targets --all-features -- -D warnings`, `cargo fmt --check`, `cargo doc --no-deps` warning-free — all green.