docs-pin batch: self-signed validity, pin format-exactness, non-Clone decision, negotiation note (C-1, C-4, N-3, N-4)
- C-1: SelfSignedCert + generate_self_signed_cert document the rcgen-default validity (1975→4096, never expires in practice), no SANs (CA verification fails as designed — pairs with SHA256: pinning), and the additive not_before/not_after route if tighter validity is ever wanted - C-4: RemoteIdentity::fingerprint + FingerprintPinVerifier document case-/format-exactness (pins produced by fingerprint_from_cert_der, lowercase hex, exact ed25519:/SHA256: prefixes); written from the ADR-007-corrected chain — same-format-but-wrong pins fail closed at the pin compare, cross-format mismatches fail earlier at cert-type negotiation (the review's original pin-compare mechanism superseded) - N-3: TlsServerConfig doc records the keep-non-Clone-for-v1 decision (API identical across feature configurations; conditional Clone is its own trap); N-3 closed in review 001 - N-4: "Client-cert-type negotiation" section on VerifyPresentedCertVerifier (cross-referenced from AcceptAnyCertVerifier): requires_raw_public_keys() stays trait-default false on both server verifiers — do not fix to true (rejects X.509 clients); the raw-only-offer rejection can only arise from a foreign resolver with only_raw_public_keys() == true — interop boundary of the request-not-require shape, fail-closed, not a downgrade; pinned by server_verifiers_keep_requires_raw_public_keys_default_false - Work item 5: FingerprintPinVerifier's pop cross-ref now names both server verifiers (default VerifyPresentedCertVerifier verifies per ADR-008; AcceptAnyCertVerifier escape hatch does not) Verification: cargo test (default, 69 lib tests) and --all-features (78 lib tests, 1 new), clippy --all-targets --all-features -D warnings, fmt --check, doc --no-deps warning-free — all green
This commit is contained in:
@@ -31,6 +31,19 @@ SPKI under the X.509 offer). The pending remediation tasks
|
||||
against that tree — see the correction notes in each task file and in
|
||||
§U-1/§U-2 below.
|
||||
|
||||
**Status update (2026-09-12, docs-pin batch): C-1, C-4, N-3, N-4 are
|
||||
closed** by `tasks/docs-pin-c1-c4-n3-n4.md` (completed) as rustdoc
|
||||
pins: the self-signed cert's never-expires/no-SAN facts (§C-1), the
|
||||
pin's case-/format-exactness with the ADR-007 negotiation-earlier
|
||||
failure point (§C-4), the non-Clone decision recorded on
|
||||
`TlsServerConfig` (§N-3, closed as decided), and the
|
||||
request-not-require negotiation boundary on the server verifiers
|
||||
(§N-4, with the pinned
|
||||
`server_verifiers_keep_requires_raw_public_keys_default_false` unit
|
||||
test). The review's original §C-4 mechanism ("rejected at pin
|
||||
comparison") is superseded by ADR-007 for the cross-format case — the
|
||||
docs are written from the corrected chain.
|
||||
|
||||
**One correction to this review, found during the S-1 remediation:** the
|
||||
N-4 parenthetical "alknet's own client resolver offers both types" is
|
||||
**inaccurate** — rustls 0.23.41 and 0.23.44 both send
|
||||
@@ -567,6 +580,12 @@ feature is its own trap (a consumer compiling both configurations
|
||||
would see an API difference). Recorded as an observation, not a
|
||||
change request; the all-features posture is the one the freeze pins.
|
||||
|
||||
**Remediation status (2026-09-12): closed by decision** —
|
||||
`tasks/docs-pin-c1-c4-n3-n4.md` (completed): `TlsServerConfig` stays
|
||||
non-`Clone` for v1, the reasoning is recorded in the type's rustdoc
|
||||
(src/server.rs), and this finding is closed. Revisit only if a
|
||||
concrete consumer demands it.
|
||||
|
||||
## N-4 [minor, docs] — `AcceptAnyCertVerifier::root_hint_subjects() == &[]` means clients see no CA hints; combined with `client_auth_mandatory() == false` this is the correct request-not-require shape, but `requires_raw_public_keys()` (the trait's third relevant knob, rustls `src/verify.rs:143`, default `false`) is never overridden — correct here (the verifier accepts both X.509 and RFC 7250 client certs), worth one doc sentence so the next reader doesn't "fix" it to `true` for the raw-key peers
|
||||
|
||||
The raw-key client path works because rustls negotiates the
|
||||
|
||||
@@ -241,6 +241,21 @@ impl rustls::client::ResolvesClientCert for NoClientCertResolver {
|
||||
/// connection is rejected). The fingerprint IS the trust anchor — there is no
|
||||
/// CA verification and no name verification, only the fingerprint pin.
|
||||
///
|
||||
/// # The pin is case- and format-exact (review 001 §C-4)
|
||||
///
|
||||
/// Produce the pin with [`fingerprint_from_cert_der`]: lowercase hex and
|
||||
/// the exact `ed25519:` / `SHA256:` prefixes. The comparison against the
|
||||
/// fingerprint computed from the presented cert is exact-string, so
|
||||
/// same-format-but-wrong pins (uppercase hex, a lowercase `sha256:`
|
||||
/// prefix, garbage) construct fine and fail closed at the pin compare —
|
||||
/// rejected at handshake, never a downgrade. A mismatched *format*
|
||||
/// (e.g. an `ed25519:` pin for an X.509 server) fails even earlier, at
|
||||
/// cert-type negotiation: the pin format also selects the server
|
||||
/// cert-type offer (ADR-007 — see `requires_raw_public_keys` below), so
|
||||
/// the wrong-format pairing aborts with a handshake alert before this
|
||||
/// verifier's pin compare is reached. Match the prefix to the peer's
|
||||
/// actual cert kind; both failure modes are fail-closed.
|
||||
///
|
||||
/// Handshake signatures are still verified (using the aws-lc-rs default
|
||||
/// signature verification algorithms): the presenter must prove possession
|
||||
/// of the private key corresponding to the pinned public key, so the pin
|
||||
@@ -250,9 +265,12 @@ impl rustls::client::ResolvesClientCert for NoClientCertResolver {
|
||||
/// *certificate* being used by a party that does not hold the matching
|
||||
/// private key.
|
||||
///
|
||||
/// This verifier checks proof-of-possession. The **server-side**
|
||||
/// `AcceptAnyCertVerifier` does not — see that type's docs (S-1 in
|
||||
/// `docs/reviews/001-implementation-review.md`, OQ-TLS-09).
|
||||
/// This verifier checks proof-of-possession. Server-side, the **default**
|
||||
/// `VerifyPresentedCertVerifier` verifies it too (ADR-008, resolving
|
||||
/// OQ-TLS-09); the `AcceptAnyCertVerifier` escape hatch does not — see
|
||||
/// those types' docs (S-1 in `docs/reviews/001-implementation-review.md`).
|
||||
///
|
||||
/// [`fingerprint_from_cert_der`]: crate::fingerprint_from_cert_der
|
||||
pub struct FingerprintPinVerifier {
|
||||
fingerprint: String,
|
||||
supported: rustls::crypto::WebPkiSupportedAlgorithms,
|
||||
|
||||
@@ -35,6 +35,27 @@ use crate::identity::TlsIdentity;
|
||||
pub struct RemoteIdentity {
|
||||
/// The pinned fingerprint: `ed25519:<hex>` for raw-key remotes,
|
||||
/// `SHA256:<hex>` for X.509 cert remotes (alknet ADR-030 §6).
|
||||
///
|
||||
/// **Case- and format-exact** (review 001 §C-4): the pin must be
|
||||
/// produced by [`fingerprint_from_cert_der`] — lowercase hex, the
|
||||
/// exact `ed25519:` / `SHA256:` prefixes — because the verifier's
|
||||
/// comparison is exact-string against the fingerprint it computes
|
||||
/// from the presented cert. A config author cannot mis-case a pin
|
||||
/// and get away with it:
|
||||
///
|
||||
/// - Same-format-but-wrong pins (uppercase hex, lowercase `sha256:`
|
||||
/// prefix, garbage) construct fine and fail closed at the pin
|
||||
/// compare (handshake rejection — never a downgrade).
|
||||
/// - Mismatched *format* fails earlier, at cert-type negotiation
|
||||
/// ([ADR-007](../docs/architecture/decisions/007-cert-type-negotiation.md)):
|
||||
/// an `ed25519:` pin makes the client offer raw-key-only server
|
||||
/// cert types so it aborts against an X.509 server before the pin
|
||||
/// compare is reached, and a `SHA256:` pin against a raw-key
|
||||
/// server likewise fails at negotiation. Same fail-closed verdict,
|
||||
/// earlier failure point — match the prefix to the peer's actual
|
||||
/// cert kind.
|
||||
///
|
||||
/// [`fingerprint_from_cert_der`]: crate::fingerprint_from_cert_der
|
||||
pub fingerprint: String,
|
||||
}
|
||||
|
||||
|
||||
@@ -19,7 +19,15 @@ use crate::TlsError;
|
||||
|
||||
/// Server-side TLS configuration, transport-agnostic. Built once from a
|
||||
/// [`TlsIdentity`] + ALPN list, shared across transports via
|
||||
/// `Arc` (not `Clone` — it holds the ACME task's `JoinHandle`).
|
||||
/// [`Arc`] (not `Clone` — it holds the ACME task's
|
||||
/// `JoinHandle`).
|
||||
///
|
||||
/// Not `Clone` for v1 (review 001 §N-3, decided): even under
|
||||
/// `default = []`, where the struct carries no ACME `JoinHandle`, the
|
||||
/// non-Clone posture is kept so the API is identical across feature
|
||||
/// configurations — conditional `Clone` would be an API difference
|
||||
/// between feature sets; share via [`Arc`] and revisit
|
||||
/// only if a concrete consumer demands it.
|
||||
#[allow(dead_code)]
|
||||
pub struct TlsServerConfig {
|
||||
pub(crate) rustls_config: rustls::ServerConfig,
|
||||
@@ -233,14 +241,32 @@ pub fn build_rustls_server_config(
|
||||
|
||||
/// The cert material behind the [`TlsIdentity::SelfSigned`] server path:
|
||||
/// a single rcgen-generated DER cert plus its PKCS#8 private key.
|
||||
///
|
||||
/// Validity facts (review 001 §C-1, verified against rcgen 0.13's
|
||||
/// `CertificateParams::default()`): the generated cert is valid
|
||||
/// 1975→4096 — i.e. **never expires in practice** — and carries **no
|
||||
/// SANs**, so CA verification of it fails as expected for a dev cert.
|
||||
/// Pinning its `SHA256:` fingerprint therefore outlives any plausible
|
||||
/// deployment by design; if a tighter validity is ever wanted,
|
||||
/// `CertificateParams::not_before` / `not_after` are additive params.
|
||||
pub struct SelfSignedCert {
|
||||
/// The self-signed leaf certificate (DER).
|
||||
/// The self-signed leaf certificate (DER). No SANs, validity
|
||||
/// 1975→4096 (see the type's doc).
|
||||
pub cert_chain: Vec<rustls::pki_types::CertificateDer<'static>>,
|
||||
/// The matching private key (PKCS#8 DER).
|
||||
pub private_key: rustls::pki_types::PrivateKeyDer<'static>,
|
||||
}
|
||||
|
||||
/// Generate a self-signed dev certificate (rcgen), in-memory.
|
||||
///
|
||||
/// Dev-posture facts (review 001 §C-1): rcgen's `default()` params make
|
||||
/// the cert **valid 1975→4096** (never expires in practice) and leave it
|
||||
/// **without SANs** — CA verification of it always fails, so this cert
|
||||
/// pairs only with fingerprint-pinning (`SHA256:` of the DER) or with
|
||||
/// clients that don't verify against a CA. It can never *fail* the way
|
||||
/// an ACME or ops-managed cert can. If a tighter validity is ever
|
||||
/// wanted, `not_before` / `not_after` are additive
|
||||
/// `CertificateParams` fields.
|
||||
pub fn generate_self_signed_cert() -> Result<SelfSignedCert, TlsError> {
|
||||
use rcgen::{CertificateParams, KeyPair};
|
||||
let key_pair = KeyPair::generate()?;
|
||||
@@ -290,10 +316,36 @@ pub fn generate_self_signed_cert() -> Result<SelfSignedCert, TlsError> {
|
||||
/// Clients that present no cert are unaffected — there is nothing to
|
||||
/// verify, and `client_auth_mandatory() == false`.
|
||||
///
|
||||
/// # Client-cert-type negotiation (ADR-007, review 001 §N-4)
|
||||
///
|
||||
/// `requires_raw_public_keys()` stays on the rustls trait default
|
||||
/// (`false`) on **both** this verifier and [`AcceptAnyCertVerifier`] —
|
||||
/// do not "fix" it to `true`. `false` is the request-but-don't-require
|
||||
/// shape: the verifier *requests* a client cert and accepts **both cert
|
||||
/// types** (X.509 or RFC 7250 raw key); setting `true` would reject
|
||||
/// every X.509 client. Post-ADR-007, the crate's own
|
||||
/// [`RawKeyClientCertResolver`](crate::RawKeyClientCertResolver)
|
||||
/// presents the SPKI under the **X.509 offer** unconditionally
|
||||
/// (`only_raw_public_keys() == false` — the extension is an offer
|
||||
/// format, not an identity statement), 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 (rustls `server/hs.rs::process_cert_type_extension`'s
|
||||
/// `(false, true, false)` arm) can therefore only arise from a
|
||||
/// *foreign* rustls resolver with `only_raw_public_keys() == true` — an
|
||||
/// interop boundary of the request-not-require shape: it fails closed,
|
||||
/// never a downgrade. The full mechanism lives in
|
||||
/// [ADR-007](../docs/architecture/decisions/007-cert-type-negotiation.md)
|
||||
/// — do not re-derive it here. 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 by this verifier.
|
||||
///
|
||||
/// `supported_verify_schemes()` returns the same nine-scheme list as
|
||||
/// `AcceptAnyCertVerifier` (the load-bearing list, pinned by an
|
||||
/// exact-list test). `requires_raw_public_keys()` stays `false` — the
|
||||
/// verifier accepts both cert types (ADR-007).
|
||||
/// verifier accepts both cert types (ADR-007), pinned by
|
||||
/// `server_verifiers_keep_requires_raw_public_keys_default_false`.
|
||||
///
|
||||
/// Use [`AcceptAnyCertVerifier`] only if a deployment deliberately
|
||||
/// needs the no-pop posture (e.g. an auth layer that owns
|
||||
@@ -439,6 +491,11 @@ fn nine_supported_verify_schemes() -> Vec<rustls::SignatureScheme> {
|
||||
/// X.509 remotes, fingerprint pinning for known peers, fail closed for
|
||||
/// unknown raw keys. Unlike the client-side pin verifier, this type has
|
||||
/// **no proof-of-possession check** (see above).
|
||||
///
|
||||
/// For the client-cert-type negotiation boundary (`requires_raw_public_keys()`
|
||||
/// stays `false`, raw-only offers rejected), see
|
||||
/// [`VerifyPresentedCertVerifier`]'s "Client-cert-type negotiation" section —
|
||||
/// the same shape and rationale apply verbatim to this type.
|
||||
pub struct AcceptAnyCertVerifier;
|
||||
|
||||
impl std::fmt::Debug for AcceptAnyCertVerifier {
|
||||
@@ -638,6 +695,21 @@ mod tests {
|
||||
assert!(verifier.root_hint_subjects().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_verifiers_keep_requires_raw_public_keys_default_false() {
|
||||
use rustls::server::danger::ClientCertVerifier;
|
||||
assert!(
|
||||
!AcceptAnyCertVerifier.requires_raw_public_keys(),
|
||||
"the request-not-require shape must accept both cert types; requires_raw_public_keys() == true would reject every X.509 client (ADR-007, review 001 N-4)"
|
||||
);
|
||||
let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider());
|
||||
let verifier = VerifyPresentedCertVerifier::new(&provider);
|
||||
assert!(
|
||||
!verifier.requires_raw_public_keys(),
|
||||
"the default verifier must keep the trait default; a raw-only demand is the interop boundary documented under ADR-007"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accept_any_cert_verifier_verifies_any_client_cert() {
|
||||
use rustls::pki_types::{CertificateDer, UnixTime};
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
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: pending
|
||||
status: completed
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
@@ -151,8 +151,98 @@ question):
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills this during implementation.
|
||||
- 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
|
||||
|
||||
> Agent fills this on completion.
|
||||
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.
|
||||
Reference in New Issue
Block a user