diff --git a/Cargo.lock b/Cargo.lock index 7ec51c61..8a13b7d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1144,6 +1144,7 @@ dependencies = [ "didbot-identity", "didbot-key", "didbot-lexicon", + "didbot-onboarding", "didbot-pds", "didbot-serve", "jacquard-common", diff --git a/crates/didbot-claim/Cargo.toml b/crates/didbot-claim/Cargo.toml index 874a1a62..48911d3f 100644 --- a/crates/didbot-claim/Cargo.toml +++ b/crates/didbot-claim/Cargo.toml @@ -15,6 +15,7 @@ path = "src/bin/didbot-claim.rs" [dependencies] didbot-http.workspace = true didbot-claim-check.workspace = true +didbot-onboarding.workspace = true didbot-identity.workspace = true didbot-key.workspace = true didbot-lexicon.workspace = true diff --git a/crates/didbot-claim/src/bin/didbot-claim.rs b/crates/didbot-claim/src/bin/didbot-claim.rs index a04c2b43..6ed353ed 100644 --- a/crates/didbot-claim/src/bin/didbot-claim.rs +++ b/crates/didbot-claim/src/bin/didbot-claim.rs @@ -61,9 +61,10 @@ use std::time::Duration; use didbot_claim::describe::HttpServerDescriber; use didbot_claim::identify::HttpDocumentFetcher; use didbot_claim::orchestrate::{claim, ClaimError}; -use didbot_claim::preflight::{preflight, SystemResolver}; +use didbot_claim::preflight::preflight; use didbot_claim::session_store::FileAuthStore; use didbot_claim::tls::ReqwestTlsProbe; +use didbot_onboarding::NativeEnvironment; use jacquard_common::session::SessionKey; use jacquard_oauth::atproto::AtprotoClientMetadata; use jacquard_oauth::client::OAuthClient; @@ -181,19 +182,12 @@ fn parse_args(args: &[String]) -> Result, String> { /// `--check`: the read-only half, alone. /// -/// Deliberately shares [`didbot_claim::preflight::preflight`] with nothing — -/// it *is* the thing a real claim runs first, so an operator who sees this -/// pass and then watches a claim fail on the same check has found a genuine -/// race rather than two implementations disagreeing. +/// The checks are `didbot-onboarding`'s, which is the same list the policy +/// page runs — so an operator who sees this pass and then watches a claim +/// fail on the same check has found a genuine race rather than two +/// implementations disagreeing. async fn check(hostname: &str) -> Result<(), String> { - let http = didbot_http::client(); - let report = preflight( - &SystemResolver, - &HttpDocumentFetcher::new(http.clone()), - &HttpServerDescriber::new(http), - hostname, - ) - .await; + let report = preflight(&NativeEnvironment::new(), hostname).await; println!("{hostname}"); print!("{report}"); diff --git a/crates/didbot-claim/src/preflight.rs b/crates/didbot-claim/src/preflight.rs index 510e4cf7..78d02c17 100644 --- a/crates/didbot-claim/src/preflight.rs +++ b/crates/didbot-claim/src/preflight.rs @@ -1,9 +1,12 @@ //! Whether a server can be claimed at all, checked from the outside. //! -//! Everything here answers one question — *can a stranger reach this -//! server, and is it the server it says it is* — and answers it from the -//! operator's own machine, which is the only place the answer means -//! anything. +//! Everything here answers one question — *can a stranger reach this server, +//! and is it the server it says it is* — and answers it from the operator's +//! own machine, which is the only place the answer means anything. +//! +//! The checks themselves are [`didbot_onboarding`]'s, and so is the order +//! they run in. This module is the projection a claim needs: the steps a +//! claim waits on, and the report `--check` prints. //! //! # Why this is not on the server //! @@ -30,193 +33,69 @@ //! //! # What this does not do //! -//! It does not *fix* anything. Keeping a zone this deployment manages -//! correct — noticing a missing record and republishing it — is a standing -//! obligation the server owns in every state, forever, and a separate state -//! machine; `plan/onboarding.md` carries the write-up. This module only ever -//! reports. -//! -//! # Two-valued, deliberately -//! -//! A check here is `Passed` or `Failed`, never "not looked at yet". Every -//! one of them runs to completion before this function returns, so a third -//! value would describe a state that cannot exist. That is the difference -//! between a synchronous command and a background probe: `didbot_fsm::Gates` -//! is three-valued because `didbot_reconcile` acts across ticks and needs -//! `Pending`, and nothing here does. +//! It reports. Keeping a zone this deployment manages correct — noticing a +//! missing record and republishing it — is a standing obligation the server +//! owns in every state, forever, and a separate state machine; +//! `plan/onboarding.md` carries the write-up. use std::fmt; -use std::net::IpAddr; -use std::time::Duration; -use crate::describe::ServerDescriber; -use crate::identify::DocumentFetcher; +use didbot_onboarding::{run_steps, Check, Environment, Run, Step, Target, Verdict}; -/// How long any one check waits before calling it a failure. +/// The steps a claim waits on, in the order they run. /// -/// A check is a question about reachability, so a probe that hangs *is* the -/// answer; a generous timeout only buys a longer wait for the same -/// conclusion. This is the per-check bound for `--check`, which is a -/// one-shot report — the claim flow proper waits much longer for TLS, on -/// purpose, because there it is waiting out an ACME order rather than -/// reporting a fact. See `didbot_claim::tls::wait_for_tls`. -pub const CHECK_TIMEOUT: Duration = Duration::from_secs(10); - -/// One thing that has to be true before a server can be claimed. -/// -/// Ordered as they are checked, which is also the order they fail in -/// practice: a name that does not resolve cannot present a certificate, and -/// a server that does not answer HTTPS cannot serve a document. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -pub enum Check { - /// The hostname resolves, through an ordinary resolver. - /// - /// Checked from the operator's machine because that is where a claim is - /// written from and where every other consumer of this server will be - /// looking from. What it deliberately does *not* assert is that the - /// address belongs to any particular host: a server behind a load - /// balancer, a NAT or an anycast address correctly resolves to something - /// that is not itself, and the checks below reaching it is a stronger - /// test than any comparison would have been. - Dns, - /// TLS completes and something answers HTTPS on that name. - /// - /// Established by making a real request and caring only that a response - /// came back — any status. The chain and the name are verified against - /// the platform trust store as part of connecting, so a response *at - /// all* is the proof. - Tls, - /// `https:///.well-known/did.json` serves a DID document whose - /// `id` is `did:web:` and whose `#atproto` verification method - /// is a public key atproto verifies commits with. - /// - /// The document a claim will name, and the key it will bind - /// `subjectKey` to. If this fails, there is nothing for a - /// `bot.did.operator` record to point at. The key is checked here and - /// not only at write time so that `--check` and a real claim agree: - /// see [`crate::identify::signing_key`] for why an unreadable key is a - /// refusal rather than something to write and find out about later. - DidDocument, - /// `com.atproto.server.describeServer` agrees with that document. - /// - /// The check that catches two deployments answering one name, which is - /// the failure mode invisible from either of them. See - /// [`crate::describe`]. - ServerDescription, -} - -impl Check { - /// Every check, in the order [`preflight`] runs them. - pub const ALL: &'static [Check] = &[ - Check::Dns, - Check::Tls, - Check::DidDocument, - Check::ServerDescription, - ]; - - /// The kebab-case name used in output. - pub const fn as_str(self) -> &'static str { - match self { - Check::Dns => "dns", - Check::Tls => "tls", - Check::DidDocument => "did-document", - Check::ServerDescription => "server-description", - } - } -} - -impl fmt::Display for Check { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} - -/// What one [`Check`] found. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Outcome { - /// The outside world agrees. - Passed, - /// It does not, and this is what an operator reads to know which of the - /// check's failure modes this is. - Failed { - /// Why, in a sentence somebody reads once and acts on. - reason: String, - }, - /// An earlier check failed and this one was not attempted, because its - /// answer would have been noise rather than information — a certificate - /// cannot be presented for a name that does not resolve. - /// - /// Distinct from [`Self::Failed`] on purpose. Reporting "TLS failed" for - /// a hostname with no DNS sends an operator to look at certificates when - /// the problem is a delegation. - Skipped { - /// The check whose failure made this one moot. - after: Check, - }, -} - -impl Outcome { - /// Whether this check is satisfied. Only [`Self::Passed`] is. - pub fn passed(&self) -> bool { - matches!(self, Outcome::Passed) - } - - /// The reason, for a failure. - pub fn reason(&self) -> Option<&str> { - match self { - Outcome::Failed { reason } => Some(reason), - _ => None, - } - } -} - -impl fmt::Display for Outcome { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Outcome::Passed => f.write_str("ok"), - Outcome::Failed { reason } => write!(f, "FAILED: {reason}"), - Outcome::Skipped { after } => write!(f, "skipped ({after} failed first)"), - } - } -} - -/// Every check and what it found, in [`Check::ALL`] order. +/// The four a `bot.did.operator` record cannot be written without: the name +/// resolves, something answers on it, the document it serves is this +/// server's, and `describeServer` agrees. The rest of +/// [`didbot_onboarding::Step::ALL`] is about a server that has already been +/// claimed, so a command whose whole job is writing the claim does not wait +/// on it. +pub const CLAIM_STEPS: [Step; 4] = [Step::Zone, Step::Host, Step::Document, Step::Description]; + +/// Every check in [`CLAIM_STEPS`] and what it found. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Report { /// The hostname that was checked. pub hostname: String, - outcomes: Vec, + run: Run, } impl Report { - /// What one check found. - pub fn outcome(&self, check: Check) -> &Outcome { - let index = Check::ALL - .iter() - .position(|candidate| *candidate == check) - .expect("a Check appears in its own ALL"); - &self.outcomes[index] + /// What one check found, if it ran. + #[must_use] + pub fn outcome(&self, check: Check) -> Option<&Verdict> { + self.run.verdict(check) } /// Every check and its outcome, in order. - pub fn all(&self) -> impl Iterator { - Check::ALL.iter().copied().zip(self.outcomes.iter()) + pub fn all(&self) -> impl Iterator { + self.run + .checks() + .map(|outcome| (outcome.check, &outcome.verdict)) } /// Whether every check passed, which is the whole question this module /// answers. + #[must_use] pub fn claimable(&self) -> bool { - self.outcomes.iter().all(Outcome::passed) + self.run.passed() } /// The first check that failed, if any. /// /// The first rather than all of them, because the later ones are - /// [`Outcome::Skipped`] and a caller reporting "fix this" wants the + /// [`Verdict::Blocked`] and a caller reporting "fix this" wants the /// cause, not the consequences. + #[must_use] pub fn first_failure(&self) -> Option<(Check, &str)> { - self.all() - .find_map(|(check, outcome)| outcome.reason().map(|reason| (check, reason))) + self.run.first_failure() + } + + /// The run behind this report, for a caller that wants the steps rather + /// than the lines. + #[must_use] + pub fn run(&self) -> &Run { + &self.run } } @@ -230,379 +109,16 @@ impl fmt::Display for Report { } } -/// Resolves a hostname to addresses. The seam a test substitutes; -/// [`SystemResolver`] is the only production implementation. -// `async fn` in a public trait forgoes an auto `Send` bound on its future -- -// fine here for the same reason `crate::tls::TlsProbe` gives: every use is -// generic, never a trait object, and nothing here spawns the future onto -// another task. -#[allow(async_fn_in_trait)] -pub trait HostResolver { - /// Resolves `hostname`, or reports why it could not. - async fn resolve(&self, hostname: &str) -> Result, String>; -} - -/// Resolves through the operating system's own resolver. -/// -/// Deliberately not a DNS client of this crate's own. The answer that -/// matters is the one an ordinary consumer's stub resolver gets, and the -/// operator's machine is the closest available stand-in for one — vendoring -/// a resolver and a root hint list would answer a subtly different question -/// than "will this work for people." -#[derive(Debug, Default, Clone, Copy)] -pub struct SystemResolver; - -impl HostResolver for SystemResolver { - async fn resolve(&self, hostname: &str) -> Result, String> { - let target = format!("{hostname}:443"); - match tokio::time::timeout(CHECK_TIMEOUT, tokio::net::lookup_host(target)).await { - Err(_) => Err(format!( - "the resolver did not answer within {CHECK_TIMEOUT:?}" - )), - Ok(Err(error)) => Err(error.to_string()), - Ok(Ok(addrs)) => Ok(addrs.map(|addr| addr.ip()).collect()), - } - } -} - -/// Runs every check against `hostname` and reports what it found. -/// -/// Never writes anything and never authenticates: this is the read-only -/// half of `didbot-claim`, exposed as `didbot-claim --check ` and -/// run again as the first half of a real claim. An operator who runs it gets -/// the same answers the claim flow would get, without an OAuth round trip -/// and without a record. -/// -/// # Why a failure stops the run +/// Runs [`CLAIM_STEPS`] against `hostname` and reports what they found. /// -/// Later checks are [`Outcome::Skipped`] rather than attempted, because -/// their failures would be *consequences* and reporting four failures for -/// one cause is how an operator ends up debugging the wrong thing. The -/// dependency is real in every case: a certificate cannot be presented for a -/// name that does not resolve, and a document cannot be served over a -/// connection that does not complete. -pub async fn preflight(resolver: &R, fetcher: &F, describer: &D, hostname: &str) -> Report -where - R: HostResolver, - F: DocumentFetcher, - D: ServerDescriber, -{ - let mut outcomes = Vec::with_capacity(Check::ALL.len()); - let mut failed_at: Option = None; - - // DNS. - let dns = match resolver.resolve(hostname).await { - Ok(addrs) if addrs.is_empty() => Outcome::Failed { - reason: format!("{hostname} resolves to no addresses"), - }, - Ok(_) => Outcome::Passed, - Err(error) => Outcome::Failed { - reason: format!("{hostname} does not resolve: {error}"), - }, - }; - if !dns.passed() { - failed_at = Some(Check::Dns); - } - outcomes.push(dns); - - // TLS, and then the document, over the same fetcher: resolving the - // `did:web` document *is* an HTTPS request, so a fetch that succeeds - // proves both. They stay separate checks because they fail for different - // reasons and send an operator to different places. - let url = format!("https://{hostname}/.well-known/did.json"); - let fetched = match failed_at { - Some(after) => { - outcomes.push(Outcome::Skipped { after }); - outcomes.push(Outcome::Skipped { after }); - outcomes.push(Outcome::Skipped { after }); - return Report { - hostname: hostname.to_owned(), - outcomes, - }; - } - None => fetcher.fetch(&url).await, - }; - - let document = match &fetched { - Ok(body) => { - outcomes.push(Outcome::Passed); - match crate::identify::parse_document(hostname, &url, body) - .map_err(|error| error.to_string()) - .and_then(|document| { - crate::identify::signing_key(hostname, &document) - .map_err(|error| error.to_string())?; - Ok(document) - }) { - Ok(document) => { - outcomes.push(Outcome::Passed); - Some(document) - } - Err(reason) => { - outcomes.push(Outcome::Failed { reason }); - failed_at = Some(Check::DidDocument); - None - } - } - } - Err(error) => { - outcomes.push(Outcome::Failed { - reason: format!("no usable HTTPS connection to {hostname}: {error}"), - }); - outcomes.push(Outcome::Skipped { after: Check::Tls }); - failed_at = Some(Check::Tls); - None - } - }; - - // describeServer, cross-checked against the document. - let description = match (&document, failed_at) { - (Some(document), _) => { - match crate::describe::confirm(describer, hostname, &document.id).await { - Ok(()) => Outcome::Passed, - Err(error) => Outcome::Failed { - reason: error.to_string(), - }, - } - } - (None, Some(after)) => Outcome::Skipped { after }, - // Unreachable: `document` is `None` only when something above - // failed. Handled rather than panicking, because a report that - // reached this arm somehow is still more useful than a crash. - (None, None) => Outcome::Skipped { - after: Check::DidDocument, - }, - }; - outcomes.push(description); - +/// Never writes anything and never authenticates: this is the read-only half +/// of `didbot-claim`, exposed as `didbot-claim --check ` and run +/// again as the first half of a real claim. An operator who runs it gets the +/// same answers the claim flow would get, without an OAuth round trip and +/// without a record. +pub async fn preflight(env: &E, hostname: &str) -> Report { Report { hostname: hostname.to_owned(), - outcomes, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use didbot_identity::resolve::ResolveError; - - struct Resolver(Result, String>); - impl HostResolver for Resolver { - async fn resolve(&self, _hostname: &str) -> Result, String> { - self.0.clone() - } - } - - struct Fetcher(Result); - impl DocumentFetcher for Fetcher { - async fn fetch(&self, _url: &str) -> Result { - self.0.clone() - } - } - - struct Describer(Result); - impl ServerDescriber for Describer { - async fn describe(&self, _hostname: &str) -> Result { - self.0.clone() - } - } - - fn addr() -> Vec { - vec!["203.0.113.7".parse().expect("a literal address")] - } - - /// A document that would really pass: the `id` the hostname implies and - /// an `#atproto` key a claim could bind `subjectKey` to. Both, because - /// `--check` reporting claimable for a document a real claim then - /// refuses is the one thing this module promises does not happen. - fn document(host: &str) -> String { - document_with_key( - host, - Some( - &didbot_key::SigningKey::generate() - .verifying_key() - .to_multibase(), - ), - ) - } - - fn document_with_key(host: &str, key: Option<&str>) -> String { - let did = format!("did:web:{host}"); - let mut value = serde_json::json!({ - "@context": ["https://www.w3.org/ns/did/v1"], - "id": did, - "verificationMethod": [], - }); - if let Some(key) = key { - value["verificationMethod"] = serde_json::json!([{ - "id": format!("{did}#atproto"), - "type": "Multikey", - "controller": did, - "publicKeyMultibase": key, - }]); - } - value.to_string() - } - - /// `--check` and a real claim read the same document the same way. A - /// document with no `#atproto` key, or one naming something that is not - /// an atproto public key, is not claimable -- and reporting it as - /// claimable would send an operator through an OAuth consent screen to - /// reach a refusal `--check` already had the answer to. - #[tokio::test] - async fn a_document_without_a_usable_signing_key_is_not_claimable() { - let host = "pds.example.test"; - for key in [None, Some("zNotARealKeyAtAll")] { - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Ok(document_with_key(host, key))), - &Describer(Ok(format!("did:web:{host}"))), - host, - ) - .await; - assert!(!report.claimable(), "{key:?} was reported claimable"); - let (check, _) = report.first_failure().expect("a failure"); - assert_eq!(check, Check::DidDocument, "{key:?}"); - // The document is the cause, so the check below it is a - // consequence and must not be attempted. - assert_eq!( - report.outcome(Check::ServerDescription), - &Outcome::Skipped { - after: Check::DidDocument - }, - "{key:?}" - ); - } - } - - #[tokio::test] - async fn everything_passing_is_claimable() { - let host = "pds.example.test"; - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Ok(document(host))), - &Describer(Ok(format!("did:web:{host}"))), - host, - ) - .await; - assert!(report.claimable(), "{report}"); - assert_eq!(report.first_failure(), None); - } - - /// The property the whole design turns on: one cause, one failure, and - /// the consequences reported as skipped rather than as three more - /// problems to debug. - #[tokio::test] - async fn a_dns_failure_skips_everything_downstream() { - let report = preflight( - &Resolver(Err("NXDOMAIN".into())), - &Fetcher(Ok(document("pds.example.test"))), - &Describer(Ok("did:web:pds.example.test".into())), - "pds.example.test", - ) - .await; - assert!(!report.claimable()); - let (check, reason) = report.first_failure().expect("a failure"); - assert_eq!(check, Check::Dns); - assert!(reason.contains("NXDOMAIN"), "{reason}"); - for later in [Check::Tls, Check::DidDocument, Check::ServerDescription] { - assert_eq!( - report.outcome(later), - &Outcome::Skipped { after: Check::Dns }, - "{later} should not have been attempted" - ); - } - } - - #[tokio::test] - async fn resolving_to_nothing_is_a_dns_failure() { - let report = preflight( - &Resolver(Ok(Vec::new())), - &Fetcher(Ok(document("pds.example.test"))), - &Describer(Ok("did:web:pds.example.test".into())), - "pds.example.test", - ) - .await; - assert_eq!( - report.first_failure().map(|(check, _)| check), - Some(Check::Dns) - ); - } - - #[tokio::test] - async fn an_unreachable_server_fails_tls_and_not_the_document() { - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Err(ResolveError::Transport { - url: "https://pds.example.test/.well-known/did.json".into(), - message: "connection refused".into(), - })), - &Describer(Ok("did:web:pds.example.test".into())), - "pds.example.test", - ) - .await; - assert_eq!( - report.first_failure().map(|(check, _)| check), - Some(Check::Tls) - ); - assert_eq!( - report.outcome(Check::DidDocument), - &Outcome::Skipped { after: Check::Tls } - ); - } - - /// A server answering HTTPS with a document for somebody else's name. - #[tokio::test] - async fn a_document_naming_another_host_fails_the_document_check() { - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Ok(document("someone-else.example.test"))), - &Describer(Ok("did:web:pds.example.test".into())), - "pds.example.test", - ) - .await; - assert!(report.outcome(Check::Tls).passed(), "TLS did answer"); - assert_eq!( - report.first_failure().map(|(check, _)| check), - Some(Check::DidDocument) - ); - } - - /// The failure that is invisible from either server involved: two - /// deployments answering one name. - #[tokio::test] - async fn a_describe_server_mismatch_is_its_own_failure() { - let host = "pds.example.test"; - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Ok(document(host))), - &Describer(Ok("did:web:a-different-deployment.example.test".into())), - host, - ) - .await; - assert!(report.outcome(Check::DidDocument).passed()); - let (check, reason) = report.first_failure().expect("a failure"); - assert_eq!(check, Check::ServerDescription); - assert!(reason.contains("a-different-deployment"), "{reason}"); - } - - #[tokio::test] - async fn the_report_renders_every_check_in_order() { - let host = "pds.example.test"; - let report = preflight( - &Resolver(Ok(addr())), - &Fetcher(Ok(document(host))), - &Describer(Ok(format!("did:web:{host}"))), - host, - ) - .await; - let rendered = report.to_string(); - let mut cursor = 0; - for check in Check::ALL { - let at = rendered - .find(check.as_str()) - .unwrap_or_else(|| panic!("{check} is missing from the report:\n{rendered}")); - assert!(at >= cursor, "{check} is out of order:\n{rendered}"); - cursor = at; - } + run: run_steps(env, &Target::new(hostname), &CLAIM_STEPS).await, } } diff --git a/crates/didbot-fsm/src/lib.rs b/crates/didbot-fsm/src/lib.rs index af3175ca..a97e80d9 100644 --- a/crates/didbot-fsm/src/lib.rs +++ b/crates/didbot-fsm/src/lib.rs @@ -47,13 +47,13 @@ //! //! An earlier version of this crate carried a three-valued precondition //! type, then dropped it: the lifecycle that wanted it blocked on DNS, TLS -//! and its own `did.json`, those checks moved to `didbot_claim::preflight` -//! on the operator's machine, and `preflight` runs them synchronously and -//! needs only two values. +//! and its own `did.json`, those checks moved to `didbot_onboarding` on the +//! operator's machine, where a run is synchronous and a check is answered or +//! waiting on a step below it. //! //! `didbot_reconcile::ZoneReconciler` is the caller that brings the concept -//! back, and it needs the third value for a reason `preflight` does not -//! have: it *acts*. A repair it has not attempted yet and a repair it +//! back, and it needs the third value for a reason an onboarding run does +//! not have: it *acts*. A repair it has not attempted yet and a repair it //! attempted and could not make are different facts about the zone, and //! collapsing them makes the first tick of a healthy process //! indistinguishable from an outage — which is exactly the input its @@ -527,9 +527,9 @@ impl fmt::Display for Gate { /// causal order — read the zone before comparing it, compare it before /// repairing it — and one failure reports as one cause rather than as every /// downstream gate it made unanswerable. This is the same shape -/// `didbot_claim::preflight` uses when it skips the checks below a failed -/// one; the difference is that this set persists across ticks, which is why -/// it needs a value for "not attempted yet." +/// `didbot_onboarding` uses when it blocks the steps below a failed one; the +/// difference is that this set persists across ticks, which is why it needs +/// a value for "not attempted yet." /// /// A gate is addressed by its `&'static str` name, which the caller owns. /// Naming one that was not declared is a caller bug rather than a runtime diff --git a/crates/didbot-onboarding/tests/steps.rs b/crates/didbot-onboarding/tests/steps.rs index 0456fe46..f596b2e6 100644 --- a/crates/didbot-onboarding/tests/steps.rs +++ b/crates/didbot-onboarding/tests/steps.rs @@ -27,8 +27,14 @@ enum Broken { Host, /// The certificate expired. Certificate, + /// The name exists and publishes no address. + ZoneNoAddresses, /// The document names somebody else. Document, + /// The document publishes no `#atproto` key at all. + DocumentMissingKey, + /// It publishes one that is not a key atproto verifies commits with. + DocumentUnusableKey, /// Neither handle record answers. Handle, /// `describeServer` names another deployment. @@ -47,10 +53,12 @@ impl Broken { /// The step this breakage is meant to fail. fn step(self) -> Step { match self { - Broken::Zone => Step::Zone, + Broken::Zone | Broken::ZoneNoAddresses => Step::Zone, Broken::Host => Step::Host, Broken::Certificate => Step::Certificate, - Broken::Document => Step::Document, + Broken::Document | Broken::DocumentMissingKey | Broken::DocumentUnusableKey => { + Step::Document + } Broken::Handle => Step::Handle, Broken::Description => Step::Description, Broken::Identity => Step::Identity, @@ -60,11 +68,14 @@ impl Broken { } } - const ALL: [Broken; 10] = [ + const ALL: [Broken; 13] = [ Broken::Zone, + Broken::ZoneNoAddresses, Broken::Host, Broken::Certificate, Broken::Document, + Broken::DocumentMissingKey, + Broken::DocumentUnusableKey, Broken::Handle, Broken::Description, Broken::Identity, @@ -107,23 +118,36 @@ impl Fixture { } else { DID }; - serde_json::json!({ + let mut value = serde_json::json!({ "@context": ["https://www.w3.org/ns/did/v1"], "id": did, "alsoKnownAs": [format!("at://{HOST}")], - "verificationMethod": [{ - "id": format!("{did}#atproto"), - "type": "Multikey", - "controller": did, - "publicKeyMultibase": self.key, - }], + "verificationMethod": [], "service": [{ "id": format!("{did}#atproto_pds"), "type": "AtprotoPersonalDataServer", "serviceEndpoint": format!("https://{HOST}"), }], - }) - .to_string() + }); + // The claimed server's document is its own answer, so the key in it + // is whatever it chose to serve. A string that is not an atproto + // public key has to refuse rather than be reported claimable: + // `didbot_pds::Ownership` compares `subjectKey` for exact equality + // against the key the server really holds. + let key = match self.broken { + Some(Broken::DocumentMissingKey) => None, + Some(Broken::DocumentUnusableKey) => Some("zNotARealKeyAtAll"), + _ => Some(self.key.as_str()), + }; + if let Some(key) = key { + value["verificationMethod"] = serde_json::json!([{ + "id": format!("{did}#atproto"), + "type": "Multikey", + "controller": did, + "publicKeyMultibase": key, + }]); + } + value.to_string() } fn status(&self) -> String { @@ -198,6 +222,7 @@ impl Environment for Fixture { RecordKind::Address if self.is(Broken::Zone) => { Err(Unavailable::failed("NXDOMAIN: the name does not exist")) } + RecordKind::Address if self.is(Broken::ZoneNoAddresses) => Ok(Vec::new()), RecordKind::Address => Ok(vec!["203.0.113.7".to_owned()]), // A zone cut one label above the deployment's own name, which is // where a real delegation sits. diff --git a/crates/didbot-pds/src/server_state.rs b/crates/didbot-pds/src/server_state.rs index 417bef6b..f93e5151 100644 --- a/crates/didbot-pds/src/server_state.rs +++ b/crates/didbot-pds/src/server_state.rs @@ -70,8 +70,8 @@ //! DNS, TLS and `did.json` and blocked on them. That was the wrong vantage //! point: a server can only confirm what it *wrote*, and what matters is //! whether anyone else can see it. Those checks belong where somebody is -//! actually looking from, which is the operator's machine — -//! `didbot_claim::preflight`. +//! actually looking from, which is the operator's machine and the policy +//! page — `didbot_onboarding`. //! //! **DNS reconciliation.** Keeping a zone this deployment manages correct is //! a standing obligation in every state, not a bootstrap step: if a record diff --git a/crates/didbot-reconcile/src/lib.rs b/crates/didbot-reconcile/src/lib.rs index c7ebbc29..21802595 100644 --- a/crates/didbot-reconcile/src/lib.rs +++ b/crates/didbot-reconcile/src/lib.rs @@ -11,9 +11,9 @@ //! //! # What it is, against what `didbot-claim` is //! -//! `didbot_claim::preflight` **observes from outside**: it runs on the -//! operator's machine, resolves through a real resolver, and reports. It -//! exists because a server checking its own DNS confirms only what it wrote. +//! `didbot_onboarding` **observes from outside**: it runs on the operator's +//! machine, resolves through a real resolver, and reports. It exists because +//! a server checking its own DNS confirms only what it wrote. //! //! This **repairs from inside**, which is the thing no outside observer can //! do, and it is honest about the limit that makes the two complementary: diff --git a/docs/server-lifecycle.md b/docs/server-lifecycle.md index cf64628b..ef001022 100644 --- a/docs/server-lifecycle.md +++ b/docs/server-lifecycle.md @@ -117,9 +117,10 @@ burst multiply outbound requests, which is the wrong quantity to bound. The server knows two things about itself and checks nothing else: whether it holds its own signing key, and whether its own poll found a claim. -Everything external — DNS resolving, TLS terminating, the `did:web` document -serving, `describeServer` agreeing — is checked by `didbot_claim::preflight`, -on the operator's own machine, and reported by `didbot-claim --check`. A +Everything external — the zone delegated, DNS resolving, TLS terminating, the +`did:web` document serving, `describeServer` agreeing — is checked by +`didbot_onboarding`, from wherever somebody is looking: the operator's own +machine through `didbot-claim --check`, or the policy page. A server checking its own DNS confirms what it wrote; a server checking its own TLS is often checking a loopback path; a server checking its own `did.json` proves it can talk to itself, and the failure worth catching (a *different*