From 4dcd1d6f0e71970f84c5e4dbf8f1776cf62fbd98 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 14 Oct 2026 20:25:18 -0400 Subject: [PATCH] fix(index): hold every account-named read to public https addresses One HTTP client builder now decides where a read may connect. Under the jetstream path, DID documents, handles, profiles, registrations, records and pictures go over https to public addresses only (the metadata service on IPv4 and IPv6 included), through at most three redirects checked the same way; the deployment's own relay is read wherever it is, and `--private-addresses` lifts the guard for a development stack. Co-Authored-By: Claude Opus 5.5 Change-Id: Ib32b13122678fff4ad46aacb5eba40015e8a9279 --- crates/vibescrobble-index/src/avatar.rs | 198 ++------ crates/vibescrobble-index/src/backfill.rs | 11 +- .../src/bin/vibescrobble-index.rs | 40 +- crates/vibescrobble-index/src/http.rs | 476 +++++++++++++++++- crates/vibescrobble-index/src/identity.rs | 36 ++ crates/vibescrobble-index/src/lib.rs | 4 +- crates/vibescrobble-index/src/sse.rs | 5 +- crates/vibescrobble-index/src/tls.rs | 5 +- crates/vibescrobble-query/tests/avatar.rs | 5 +- docs/running-locally.md | 8 +- spike/stack/run.sh | 1 + 11 files changed, 581 insertions(+), 208 deletions(-) diff --git a/crates/vibescrobble-index/src/avatar.rs b/crates/vibescrobble-index/src/avatar.rs index af0b8cf8..37688d9a 100644 --- a/crates/vibescrobble-index/src/avatar.rs +++ b/crates/vibescrobble-index/src/avatar.rs @@ -14,10 +14,9 @@ //! every URL can be served `immutable`. //! //! Where those requests go is whatever a DID document says, which for an -//! arbitrary account is its owner's to choose. So they go through a client of -//! their own ([`Fetcher`]): https only, to public addresses only (a name that -//! resolves anywhere else is refused before a connection is opened), with no -//! redirects, short timeouts and a byte cap. +//! arbitrary account is its owner's to choose. So they go through a +//! [`crate::http::Client`] that reaches [`Reach::Public`] only ([`Fetcher`]), +//! with short timeouts and a byte cap. //! //! What is held is held in memory, one picture per account with its CID //! beside it, and no more than [`MAX_HELD`] of them. A profile that cannot be @@ -29,7 +28,6 @@ use std::collections::{HashMap, HashSet}; use std::io::Cursor; -use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -45,7 +43,7 @@ use serde::Deserialize; use tokio::sync::RwLock; use tracing::{debug, info, warn}; -use crate::http::{HttpGet, CONNECT_TIMEOUT}; +use crate::http::{refusal_in, Client, HttpGet, Reach, Unsafe}; use crate::nsid; use crate::resolve::DidResolver; use crate::server::ServerRef; @@ -134,7 +132,7 @@ pub enum Refused { /// otherwise non-public address. #[error("{0} is not a public address")] Private(String), - /// The server answered with a redirect, which is not followed. + /// The server answered with a redirect that named nowhere to go. #[error("redirected ({0}), which is not followed")] Redirect(u16), /// The server answered, but not with what was asked for. @@ -179,7 +177,7 @@ struct Held { #[derive(Debug)] struct Upstream { base: String, - http: reqwest::Client, + http: Client, } impl Avatars { @@ -193,12 +191,8 @@ impl Avatars { /// not hold, and keeps what it is given: a query service's. #[must_use] pub fn upstream(index: &str) -> Self { - let http = reqwest::Client::builder() - .connect_timeout(CONNECT_TIMEOUT) - .timeout(TIMEOUT) - .redirect(reqwest::redirect::Policy::none()) - .build() - .expect("a client with timeouts builds"); + // The deployment's own index, on its own network. + let http = Client::new(Reach::Any, Some(TIMEOUT)); Self { held: Arc::default(), upstream: Some(Arc::new(Upstream { @@ -320,14 +314,11 @@ pub async fn serve( } } -/// Reads `url` with `http`, refusing a redirect and stopping at `cap` bytes. -async fn read_capped( - http: &reqwest::Client, - url: &str, - cap: usize, -) -> Result<(u16, Bytes), Refused> { +/// Reads `url` with `http`, stopping at `cap` bytes. +async fn read_capped(http: &Client, url: &str, cap: usize) -> Result<(u16, Bytes), Refused> { let mut response = http .get(url) + .map_err(Refused::from)? .send() .await .map_err(|err| unreachable(&err))?; @@ -351,23 +342,28 @@ async fn read_capped( Ok((status.as_u16(), body.into())) } -/// A transport error, or the address refusal it is wrapping. +/// A transport error, or the guard's refusal it is wrapping. fn unreachable(err: &reqwest::Error) -> Refused { - let mut source: Option<&(dyn std::error::Error + 'static)> = Some(err); - while let Some(cause) = source { - if let Some(private) = cause.downcast_ref::() { - return Refused::Private(private.0.clone()); + match refusal_in(err) { + Some(refusal) => refusal.into(), + None => Refused::Unreachable(err.to_string()), + } +} + +impl From for Refused { + fn from(refusal: Unsafe) -> Self { + match refusal { + Unsafe::NotAUrl(url) => Self::Unreachable(format!("{url} is not a url")), + Unsafe::NotHttps(url) => Self::NotHttps(url), + Unsafe::Private(host) => Self::Private(host), } - source = cause.source(); } - Refused::Unreachable(err.to_string()) } /// The client that talks to accounts' servers. #[derive(Debug, Clone)] pub struct Fetcher { - http: reqwest::Client, - guarded: bool, + http: Client, } impl Default for Fetcher { @@ -377,50 +373,29 @@ impl Default for Fetcher { } impl Fetcher { - /// https only, to public addresses only, with no redirects. + /// [`Reach::Public`]: https only, to public addresses only. #[must_use] pub fn new() -> Self { - Self::build(true) + Self::reaching(Reach::Public) } /// The same, but to any address over http or https: for a development /// server on loopback, and for tests of everything but the guard. #[must_use] pub fn trusting_any_address() -> Self { - Self::build(false) - } - - fn build(guarded: bool) -> Self { - // No proxy: a proxy resolves the name itself, and the address check - // below would be checking nothing. - let mut builder = crate::tls::builder() - .no_proxy() - .connect_timeout(CONNECT_TIMEOUT) - .timeout(TIMEOUT) - .redirect(reqwest::redirect::Policy::none()); - if guarded { - builder = builder.dns_resolver(Arc::new(PublicOnly)); + Self::reaching(Reach::Any) + } + + /// What `reach` allows. + #[must_use] + pub fn reaching(reach: Reach) -> Self { + Self { + http: Client::new(reach, Some(TIMEOUT)), } - let http = builder.build().expect("a client with timeouts builds"); - Self { http, guarded } } /// Reads `url`, whatever the status, up to `cap` bytes. async fn fetch(&self, url: &str, cap: usize) -> Result<(u16, Bytes), Refused> { - if self.guarded { - let parsed = - reqwest::Url::parse(url).map_err(|err| Refused::Unreachable(err.to_string()))?; - if parsed.scheme() != "https" { - return Err(Refused::NotHttps(url.to_owned())); - } - // A literal address is dialled without being resolved, so the - // resolver below never sees it. - let host = parsed.host_str().unwrap_or_default(); - let literal = host.trim_start_matches('[').trim_end_matches(']'); - if let Some(ip) = literal.parse::().ok().filter(|ip| !is_public(*ip)) { - return Err(Refused::Private(ip.to_string())); - } - } read_capped(&self.http, url, cap).await } @@ -559,83 +534,6 @@ pub fn shrink(blob: &[u8]) -> Result, Refused> { Ok(jpeg) } -/// Whether `ip` is somewhere on the public internet: not loopback, private, -/// link-local, shared, reserved, multicast or documentation space, and not an -/// IPv6 address wrapping one of those. -#[must_use] -pub fn is_public(ip: IpAddr) -> bool { - match ip { - IpAddr::V4(v4) => public_v4(v4), - IpAddr::V6(v6) => { - if let Some(v4) = v6.to_ipv4_mapped() { - return public_v4(v4); - } - let s = v6.segments(); - !(v6.is_loopback() - || v6.is_unspecified() - || v6.is_multicast() - // ::a.b.c.d, the deprecated IPv4-compatible form. - || s[..6] == [0; 6] - // Unique local, fc00::/7. - || (s[0] & 0xfe00) == 0xfc00 - // Link-local, fe80::/10. - || (s[0] & 0xffc0) == 0xfe80 - // Documentation, 2001:db8::/32. - || (s[0] == 0x2001 && s[1] == 0x0db8) - // NAT64 and 6to4, which carry an IPv4 address inside. - || (s[0] == 0x0064 && s[1] == 0xff9b) - || s[0] == 0x2002) - } - } -} - -fn public_v4(ip: Ipv4Addr) -> bool { - let [a, b, c, _] = ip.octets(); - !(ip.is_private() - || ip.is_loopback() - || ip.is_link_local() - || ip.is_broadcast() - || ip.is_documentation() - || ip.is_unspecified() - || ip.is_multicast() - || a == 0 - // Shared address space, 100.64.0.0/10. - || (a == 100 && (b & 0xc0) == 64) - // IETF protocol assignments, 192.0.0.0/24. - || (a == 192 && b == 0 && c == 0) - // Benchmarking, 198.18.0.0/15. - || (a == 198 && (b & 0xfe) == 18) - // Reserved, 240.0.0.0/4. - || a >= 240) -} - -/// A name that resolved to no public address. -#[derive(Debug, thiserror::Error)] -#[error("{0} resolves to no public address")] -struct PrivateName(String); - -/// Resolves a name to its public addresses only, refusing one that has none. -/// -/// The check is made on the addresses the connection is opened to, not on an -/// earlier lookup, so a name that answers differently the second time cannot -/// slip a private address in between. -#[derive(Debug)] -struct PublicOnly; - -impl reqwest::dns::Resolve for PublicOnly { - fn resolve(&self, name: reqwest::dns::Name) -> reqwest::dns::Resolving { - let host = name.as_str().to_owned(); - Box::pin(async move { - let found = tokio::net::lookup_host((host.as_str(), 0)).await?; - let public: Vec = found.filter(|addr| is_public(addr.ip())).collect(); - if public.is_empty() { - return Err(Box::new(PrivateName(host)) as Box); - } - Ok(Box::new(public.into_iter()) as reqwest::dns::Addrs) - }) - } -} - /// Reads `did`'s profile and brings the picture it is drawn with up to date, /// announcing the operator or agent again if it changed. /// @@ -729,32 +627,6 @@ async fn accounts(shared: &Shared) -> (Vec, HashSet) { mod tests { use super::*; - #[test] - fn only_public_addresses_are_public() { - for private in [ - "127.0.0.1", - "10.1.2.3", - "172.17.0.2", - "192.168.1.1", - "169.254.169.254", - "100.64.0.1", - "0.0.0.0", - "240.0.0.1", - "::1", - "::", - "fd00::1", - "fe80::1", - "::ffff:127.0.0.1", - "::ffff:169.254.169.254", - "64:ff9b::a9fe:a9fe", - ] { - assert!(!is_public(private.parse().expect(private)), "{private}"); - } - for public in ["1.1.1.1", "34.230.20.1", "2606:4700:4700::1111"] { - assert!(is_public(public.parse().expect(public)), "{public}"); - } - } - #[test] fn a_path_names_the_did_and_the_cid() { assert_eq!( diff --git a/crates/vibescrobble-index/src/backfill.rs b/crates/vibescrobble-index/src/backfill.rs index b39de6b2..b1bf7598 100644 --- a/crates/vibescrobble-index/src/backfill.rs +++ b/crates/vibescrobble-index/src/backfill.rs @@ -315,7 +315,9 @@ impl Overheard { /// again when the stream loses its place. #[derive(Debug, Clone)] pub struct Relisting { - /// Reads the listing and each repository. + /// Reads the listing. The relay is this deployment's own, so this may + /// reach it wherever it is; each repository is read with the registry's + /// transport, which reaches only what an account may name. pub indexer: Indexer, /// The relay to list. pub relay: ServerRef, @@ -342,7 +344,7 @@ pub struct Relisting { /// no server can be named for, as on the stream. The two passes are /// [`backfill`]'s, for the same reasons. A read running beside the stream is /// given what it has said, as `overheard`. -pub async fn from_relay( +pub async fn from_relay( shared: &Shared, registry: &mut Registry, relisting: &Relisting, @@ -353,6 +355,7 @@ pub async fn from_relay( let mut held: Vec = Vec::new(); let mut minted: Vec = Vec::new(); let relay = &relisting.relay; + let accounts = Indexer::new(registry.resolver().http().clone()); let listed = relisting .indexer @@ -377,7 +380,7 @@ pub async fn from_relay( let reads: Vec = { let registry: &Registry = registry; stream::iter(batch) - .map(|listed| read(registry, &relisting.indexer, listed, relisting.records)) + .map(|listed| read(registry, &accounts, listed, relisting.records)) .buffer_unordered(READERS) .collect() .await @@ -427,7 +430,7 @@ pub async fn from_relay( } /// [`from_relay`], said on `/health` while it runs and logged when it ends. -pub async fn refill( +pub async fn refill( shared: &Shared, registry: &mut Registry, relisting: &Relisting, diff --git a/crates/vibescrobble-index/src/bin/vibescrobble-index.rs b/crates/vibescrobble-index/src/bin/vibescrobble-index.rs index 6f880eb8..0b1080bd 100644 --- a/crates/vibescrobble-index/src/bin/vibescrobble-index.rs +++ b/crates/vibescrobble-index/src/bin/vibescrobble-index.rs @@ -61,6 +61,7 @@ use vibescrobble_index::backfill::Relisting; use vibescrobble_index::directory::{ServerDirectory, StaticDirectory, Target}; use vibescrobble_index::dns::SystemDns; use vibescrobble_index::firehose::{subscribe, Update}; +use vibescrobble_index::http::Reach; use vibescrobble_index::identity::{Identities, Registry}; use vibescrobble_index::intake::Intake; use vibescrobble_index::jetstream; @@ -140,6 +141,10 @@ Options: and the servers its relay crawls; needs --jetstream --plc The PLC directory a did:plc is resolved through (default https://plc.directory) + --private-addresses With --jetstream, read what accounts name over http + and from loopback and private addresses too. Every + account names its own URLs, so this is for a + development stack only; --pds always reads this way --identities Append-only log of every account ever resolved, so one that has ended can still be named after a restart --cluster-every Seconds between clustering passes (default 2, 0 is off) @@ -181,6 +186,7 @@ struct Args { limit: usize, backfill: bool, backfill_limit: usize, + private_addresses: bool, cluster: Duration, prune: Duration, model: String, @@ -204,6 +210,7 @@ impl Default for Args { limit: DEFAULT_LIMIT, backfill: false, backfill_limit: DEFAULT_BACKFILL, + private_addresses: false, cluster: Duration::from_secs(DEFAULT_CLUSTER), prune: vibescrobble_index::activity::DEFAULT_PRUNE, model: Model2Vec::DEFAULT.to_owned(), @@ -287,6 +294,7 @@ fn parse_args>(mut args: I) -> Result { .map_err(|_| format!("--feed-limit: `{raw}` is not a count"))?; } "--backfill" => parsed.backfill = true, + "--private-addresses" => parsed.private_addresses = true, "--backfill-limit" => { let raw = value()?; parsed.backfill_limit = raw @@ -315,6 +323,9 @@ fn parse_args>(mut args: I) -> Result { "--relay is read alongside --jetstream; --pds reads servers directly".to_owned(), ); } + if parsed.private_addresses && parsed.jetstream.is_none() { + return Err("--private-addresses is for --jetstream; --pds reads any address".to_owned()); + } if parsed.network.is_some() && parsed.jetstream.is_none() { return Err("--network is read alongside --jetstream".to_owned()); } @@ -345,7 +356,6 @@ async fn main() -> ExitCode { } async fn run(args: Args) -> Result<(), String> { - let http = ReqwestGet::new(); // Loaded before anything connects, and fatal if it fails. A process that // came up without the model would answer every request with no groups, // which reads as "nobody is working together" rather than as a fault. @@ -379,8 +389,11 @@ async fn run(args: Args) -> Result<(), String> { // relay that crawls every server; `--pds` is one server read directly, // which is what a machine with a development deployment on it has. if let Some(endpoint) = args.jetstream.clone() { - return run_jetstream(args, endpoint, http, shared).await; + return run_jetstream(args, endpoint, shared).await; } + // The servers followed are the ones configured, which in development are + // on loopback. + let http = ReqwestGet::trusting_any_address(); let configured = StaticDirectory::parse(&args.servers) .map_err(|err| format!("--pds: {err}"))? @@ -756,12 +769,7 @@ async fn cluster_loop(shared: Shared, every: Duration, keeping: Option Result<(), String> { +async fn run_jetstream(args: Args, endpoint: String, shared: Shared) -> Result<(), String> { let addr: SocketAddr = args .listen .parse() @@ -774,6 +782,18 @@ async fn run_jetstream( .transpose() .map_err(|err| format!("--relay: {err}"))?; + // Every account names its own server and its own document's address, + // so what they name is read over https from public addresses only. The + // relay is this deployment's own, and is read wherever it is. + let reach = if args.private_addresses { + warn!("--private-addresses: reading what accounts name from any address"); + Reach::Any + } else { + Reach::Public + }; + let http = ReqwestGet::reaching(reach); + let relay_http = ReqwestGet::trusting_any_address(); + let identities = open_identities(&args)?; let registry = Registry::new(resolver(&http, &args.plc), identities); @@ -802,7 +822,7 @@ async fn run_jetstream( args.groups.clone(), )); let _pruner = start_pruning(&shared, args.prune).await; - let _avatars = start_avatars(&shared, &http, &args.plc, avatar::Fetcher::new()); + let _avatars = start_avatars(&shared, &http, &args.plc, avatar::Fetcher::reaching(reach)); let listener = TcpListener::bind(addr) .await .map_err(|err| format!("could not bind {addr}: {err}"))?; @@ -834,7 +854,7 @@ async fn run_jetstream( // stream starts from now. A relay that does not answer is asked again // until it does: on a deploy it is restarting too. let relisting = relay.map(|relay| Relisting { - indexer: Indexer::new(http), + indexer: Indexer::new(relay_http), relay, repositories: args.backfill_limit, records: HISTORY, diff --git a/crates/vibescrobble-index/src/http.rs b/crates/vibescrobble-index/src/http.rs index 7aa1c0df..983c4468 100644 --- a/crates/vibescrobble-index/src/http.rs +++ b/crates/vibescrobble-index/src/http.rs @@ -7,6 +7,8 @@ //! malicious ones a real server would have to be persuaded to send. use std::future::Future; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::sync::Arc; use std::time::Duration; /// How long any connection may take to open. @@ -23,13 +25,254 @@ pub const MAX_BODY: usize = 4 * 1024 * 1024; /// is dropped and reconnected. pub const IDLE_TIMEOUT: Duration = Duration::from_secs(60); -/// A client for streams that stay open: a connect timeout and no total one. -/// Readers of it apply [`IDLE_TIMEOUT`] themselves. -pub fn stream_client() -> reqwest::Client { - crate::tls::builder() - .connect_timeout(CONNECT_TIMEOUT) - .build() - .expect("a client with a connect timeout builds") +/// The most redirects one request follows. A handle's well-known or a +/// `did:web` document may sit behind one; nothing this reads needs a chain. +pub const MAX_REDIRECTS: usize = 3; + +/// Where a [`Client`] may connect. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Reach { + /// Wherever an account's own document or records point: https only, to + /// public addresses only, through redirects that are held to the same. + /// Any account on the network can name any URL, so this is what reads + /// for one use. + Public, + /// Anywhere, over http or https: this deployment's own services, which + /// its operator names, and a development server on loopback. + Any, +} + +/// Why a [`Client`] would not send a request. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum Unsafe { + /// Not a URL at all. + #[error("{0} is not a url")] + NotAUrl(String), + /// [`Reach::Public`] reads over https only. + #[error("not https: {0}")] + NotHttps(String), + /// The host is, or resolves only to, a loopback, private, link-local or + /// otherwise non-public address. + #[error("{0} is not a public address")] + Private(String), +} + +/// The one way this crate builds an HTTP client. +/// +/// Every outbound read goes through one of these, so where it may connect is +/// decided here and not by each caller. Under [`Reach::Public`] a URL is +/// checked before anything is sent ([`Self::get`]); every name is resolved +/// to its public addresses only, on the connection itself, so a name that +/// answers differently the second time cannot slip a private address in +/// between; and each redirect is checked like the first URL. No proxy is +/// used: a proxy resolves the name itself, and the address check would be +/// checking nothing. +#[derive(Debug, Clone)] +pub struct Client { + http: reqwest::Client, + guard: Option, +} + +impl Client { + /// A client that reaches `reach`, gives up on a connection after + /// [`CONNECT_TIMEOUT`] and on a whole request after `timeout`, if given. + #[must_use] + pub fn new(reach: Reach, timeout: Option) -> Self { + let guard = match reach { + Reach::Public => Some(Guard::PUBLIC), + Reach::Any => None, + }; + Self::build(guard, timeout) + } + + fn build(guard: Option, timeout: Option) -> Self { + let mut builder = crate::tls::builder() + .no_proxy() + .connect_timeout(CONNECT_TIMEOUT); + if let Some(timeout) = timeout { + builder = builder.timeout(timeout); + } + builder = match guard { + Some(guard) => builder + .dns_resolver(Arc::new(Admitting(guard.admits))) + .redirect(reqwest::redirect::Policy::custom(move |attempt| { + if attempt.previous().len() > MAX_REDIRECTS { + return attempt.error(format!("more than {MAX_REDIRECTS} redirects")); + } + match guard.check(attempt.url()) { + Ok(()) => attempt.follow(), + Err(refusal) => attempt.error(refusal), + } + })), + None => builder.redirect(reqwest::redirect::Policy::limited(MAX_REDIRECTS)), + }; + let http = builder.build().expect("a client with timeouts builds"); + Self { http, guard } + } + + /// Whether this client holds every request to [`Reach::Public`]. + #[must_use] + pub fn guarded(&self) -> bool { + self.guard.is_some() + } + + /// A GET of `url`, or why it may not be sent. + /// + /// A literal address is dialled without being resolved, so it is checked + /// here; a name is checked as it resolves. + pub fn get(&self, url: &str) -> Result { + if let Some(guard) = &self.guard { + let parsed = reqwest::Url::parse(url).map_err(|_| Unsafe::NotAUrl(url.to_owned()))?; + guard.check(&parsed)?; + } + Ok(self.http.get(url)) + } +} + +/// Why `err` happened, when it was a guard refusing a redirect or a name. +#[must_use] +pub fn refusal_in(err: &reqwest::Error) -> Option { + let mut source: Option<&(dyn std::error::Error + 'static)> = Some(err); + while let Some(cause) = source { + if let Some(refusal) = cause.downcast_ref::() { + return Some(refusal.clone()); + } + source = cause.source(); + } + None +} + +/// `err` and everything it wraps, which is where a redirect's or a +/// connection's actual reason is. +fn describe(err: &reqwest::Error) -> String { + let mut described = err.to_string(); + let mut source = std::error::Error::source(err); + while let Some(cause) = source { + let cause_text = cause.to_string(); + if !described.contains(&cause_text) { + described.push_str(": "); + described.push_str(&cause_text); + } + source = cause.source(); + } + described +} + +/// What a guarded client may reach. +#[derive(Debug, Clone, Copy)] +struct Guard { + https_only: bool, + admits: fn(IpAddr) -> bool, +} + +impl Guard { + const PUBLIC: Self = Self { + https_only: true, + admits: is_public, + }; + + fn check(&self, url: &reqwest::Url) -> Result<(), Unsafe> { + match url.scheme() { + "https" => {} + "http" if !self.https_only => {} + _ => return Err(Unsafe::NotHttps(url.to_string())), + } + let Some(host) = url.host_str() else { + return Err(Unsafe::NotAUrl(url.to_string())); + }; + let literal = host.trim_start_matches('[').trim_end_matches(']'); + if let Ok(ip) = literal.parse::() { + if !(self.admits)(ip) { + return Err(Unsafe::Private(ip.to_string())); + } + } + Ok(()) + } +} + +/// Resolves a name to the addresses a [`Guard`] admits, refusing one that +/// has none. +/// +/// The check is made on the addresses the connection is opened to, not on an +/// earlier lookup. +#[derive(Debug)] +struct Admitting(fn(IpAddr) -> bool); + +impl reqwest::dns::Resolve for Admitting { + fn resolve(&self, name: reqwest::dns::Name) -> reqwest::dns::Resolving { + let host = name.as_str().to_owned(); + let admits = self.0; + Box::pin(async move { + let found = tokio::net::lookup_host((host.as_str(), 0)).await?; + let admitted: Vec = found.filter(|addr| admits(addr.ip())).collect(); + if admitted.is_empty() { + return Err( + Box::new(Unsafe::Private(host)) as Box + ); + } + Ok(Box::new(admitted.into_iter()) as reqwest::dns::Addrs) + }) + } +} + +/// Whether `ip` is somewhere on the public internet: not loopback, private, +/// link-local (which holds the cloud metadata service, `169.254.169.254`), +/// unique local (`fd00:ec2::254`, its IPv6 address), shared, reserved, +/// multicast or documentation space, and not an IPv6 address wrapping one of +/// those. +#[must_use] +pub fn is_public(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => public_v4(v4), + IpAddr::V6(v6) => { + if let Some(v4) = v6.to_ipv4_mapped() { + return public_v4(v4); + } + let s = v6.segments(); + !(v6.is_loopback() + || v6.is_unspecified() + || v6.is_multicast() + // ::a.b.c.d, the deprecated IPv4-compatible form. + || s[..6] == [0; 6] + // Unique local, fc00::/7. + || (s[0] & 0xfe00) == 0xfc00 + // Link-local, fe80::/10. + || (s[0] & 0xffc0) == 0xfe80 + // Documentation, 2001:db8::/32. + || (s[0] == 0x2001 && s[1] == 0x0db8) + // NAT64 and 6to4, which carry an IPv4 address inside. + || (s[0] == 0x0064 && s[1] == 0xff9b) + || s[0] == 0x2002) + } + } +} + +fn public_v4(ip: Ipv4Addr) -> bool { + let [a, b, c, _] = ip.octets(); + !(ip.is_private() + || ip.is_loopback() + || ip.is_link_local() + || ip.is_broadcast() + || ip.is_documentation() + || ip.is_unspecified() + || ip.is_multicast() + || a == 0 + // Shared address space, 100.64.0.0/10. + || (a == 100 && (b & 0xc0) == 64) + // IETF protocol assignments, 192.0.0.0/24. + || (a == 192 && b == 0 && c == 0) + // Benchmarking, 198.18.0.0/15. + || (a == 198 && (b & 0xfe) == 18) + // Reserved, 240.0.0.0/4. + || a >= 240) +} + +/// A client for streams that stay open, to this deployment's own services: +/// a connect timeout and no total one. Readers of it apply [`IDLE_TIMEOUT`] +/// themselves. +#[must_use] +pub fn stream_client() -> Client { + Client::new(Reach::Any, None) } /// A response that reached a status code. @@ -107,10 +350,10 @@ where /// /// Every request is bounded by [`CONNECT_TIMEOUT`], [`REQUEST_TIMEOUT`] and /// [`MAX_BODY`], so a host that answers slowly or at length costs one failed -/// request and not the caller. +/// request and not the caller. [`Self::new`] reaches [`Reach::Public`] only. #[derive(Debug, Clone)] pub struct ReqwestGet { - http: reqwest::Client, + http: Client, } impl Default for ReqwestGet { @@ -120,27 +363,39 @@ impl Default for ReqwestGet { } impl ReqwestGet { - /// Builds a client with this crate's timeouts and [`crate::tls`]'s roots. + /// Reads what accounts name: https, public addresses only. + #[must_use] pub fn new() -> Self { - let http = crate::tls::builder() - .connect_timeout(CONNECT_TIMEOUT) - .timeout(REQUEST_TIMEOUT) - .build() - .expect("a client with timeouts builds"); - Self { http } + Self::reaching(Reach::Public) + } + + /// Reads anywhere: for this deployment's own relay, and for a + /// development server on loopback. + #[must_use] + pub fn trusting_any_address() -> Self { + Self::reaching(Reach::Any) } - /// Builds a client over a caller-supplied one, for a proxy or other - /// timeouts. [`MAX_BODY`] still applies. - pub fn with_client(http: reqwest::Client) -> Self { - Self { http } + /// Reads what `reach` allows, with this crate's timeouts. + #[must_use] + pub fn reaching(reach: Reach) -> Self { + Self { + http: Client::new(reach, Some(REQUEST_TIMEOUT)), + } } } impl HttpGet for ReqwestGet { async fn get(&self, url: &str) -> Result { - let fail = |err: reqwest::Error| TransportError::new(url, err.to_string()); - let mut response = self.http.get(url).send().await.map_err(fail)?; + let fail = |err: reqwest::Error| match refusal_in(&err) { + Some(refusal) => TransportError::new(url, refusal.to_string()), + None => TransportError::new(url, describe(&err)), + }; + let request = self + .http + .get(url) + .map_err(|refusal| TransportError::new(url, refusal.to_string()))?; + let mut response = request.send().await.map_err(fail)?; let status = response.status().as_u16(); if response .content_length() @@ -184,6 +439,181 @@ pub(crate) fn preview(body: &str, limit: usize) -> String { mod tests { use super::*; + #[test] + fn only_public_addresses_are_public() { + for private in [ + "127.0.0.1", + "10.1.2.3", + "172.17.0.2", + "192.168.1.1", + // The cloud metadata service, over IPv4 and IPv6. + "169.254.169.254", + "fd00:ec2::254", + "100.64.0.1", + "0.0.0.0", + "240.0.0.1", + "::1", + "::", + "fd00::1", + "fe80::1", + "::ffff:127.0.0.1", + "::ffff:169.254.169.254", + "64:ff9b::a9fe:a9fe", + ] { + assert!(!is_public(private.parse().expect(private)), "{private}"); + } + for public in ["1.1.1.1", "34.230.20.1", "2606:4700:4700::1111"] { + assert!(is_public(public.parse().expect(public)), "{public}"); + } + } + + #[test] + fn a_public_client_refuses_a_private_or_plain_url_before_sending() { + let client = Client::new(Reach::Public, Some(REQUEST_TIMEOUT)); + for (url, refusal) in [ + ("https://127.0.0.1:3000/xrpc/x", "127.0.0.1"), + ("https://10.0.0.7/", "10.0.0.7"), + ("https://172.16.4.4/", "172.16.4.4"), + ("https://192.168.1.1/", "192.168.1.1"), + ( + "https://169.254.169.254/latest/meta-data/", + "169.254.169.254", + ), + // The same address written as one number, which a URL parser + // normalises before anything is dialled. + ("https://2852039166/latest/meta-data/", "169.254.169.254"), + ("https://[fd00:ec2::254]/latest/meta-data/", "fd00:ec2::254"), + ( + "https://[::ffff:169.254.169.254]/", + "::ffff:169.254.169.254", + ), + ("https://[::1]:3000/", "::1"), + ("https://[fe80::1]/", "fe80::1"), + ] { + let refused = client.get(url).expect_err(url); + assert_eq!(refused, Unsafe::Private(refusal.to_owned()), "{url}"); + } + assert!(matches!( + client.get("http://relay:2470/xrpc/com.atproto.sync.listRepos"), + Err(Unsafe::NotHttps(_)) + )); + assert!(matches!( + client.get("file:///etc/passwd"), + Err(Unsafe::NotHttps(_)) + )); + assert!(client.get("https://bsky.social/xrpc/x").is_ok()); + + let any = Client::new(Reach::Any, None); + assert!(any.get("http://relay:2470/").is_ok()); + assert!(any.get("http://127.0.0.1:3000/").is_ok()); + } + + /// Serves on loopback: `/to/` redirects there, and anything else + /// answers with its own path. + async fn redirecting() -> SocketAddr { + use axum::extract::Path; + use axum::response::Redirect; + use axum::routing::get; + let app = axum::Router::new() + .route( + "/to/{*target}", + get(|Path(target): Path| async move { + Redirect::temporary(&target.replacen(":/", "://", 1)) + }), + ) + .fallback(|uri: axum::http::Uri| async move { uri.path().to_owned() }); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + tokio::spawn(async move { axum::serve(listener, app).await }); + addr + } + + #[tokio::test] + async fn a_redirect_is_held_to_the_guard_of_the_first_url() { + let addr = redirecting().await; + // The public guard refuses a loopback server outright, so this one + // admits 127.0.0.1 alone and stands in for "public": everything + // else is what a redirect must not reach. + let http = ReqwestGet { + http: Client::build( + Some(Guard { + https_only: false, + admits: |ip| ip == IpAddr::V4(Ipv4Addr::LOCALHOST), + }), + Some(REQUEST_TIMEOUT), + ), + }; + let landed = http + .get(&format!("http://{addr}/to/http:/{addr}/landed")) + .await + .expect("a redirect to an admitted address is followed"); + assert_eq!(landed.body, "/landed"); + + for (target, refusal) in [ + ( + "http:/169.254.169.254/latest/meta-data/".to_owned(), + "169.254.169.254 is not a public address", + ), + ( + "http:/[fd00:ec2::254]/latest/meta-data/".to_owned(), + "fd00:ec2::254 is not a public address", + ), + ( + format!("http:/[::1]:{}/", addr.port()), + "::1 is not a public address", + ), + ( + "http:/10.0.0.7/".to_owned(), + "10.0.0.7 is not a public address", + ), + ] { + let url = format!("http://{addr}/to/{target}"); + let refused = http.get(&url).await.expect_err(&url); + assert!( + refused.reason.contains(refusal), + "{url}: {}", + refused.reason + ); + } + + let ftp = format!("http://{addr}/to/ftp:/example.com/"); + assert!(http.get(&ftp).await.is_err()); + + // As many redirects as are followed, and one more. + let mut url = format!("http://{addr}/landed"); + for _ in 0..MAX_REDIRECTS { + url = format!("http://{addr}/to/{}", url.replacen("://", ":/", 1)); + } + let landed = http.get(&url).await.expect("a chain of the most followed"); + assert_eq!(landed.body, "/landed"); + url = format!("http://{addr}/to/{}", url.replacen("://", ":/", 1)); + let refused = http.get(&url).await.expect_err("too many redirects"); + assert!( + refused + .reason + .contains(&format!("more than {MAX_REDIRECTS} redirects")), + "{}", + refused.reason + ); + } + + #[tokio::test] + async fn a_name_that_resolves_only_to_private_addresses_is_never_dialled() { + let addr = redirecting().await; + let http = ReqwestGet::new(); + let refused = http + .get(&format!("https://localhost:{}/", addr.port())) + .await + .expect_err("loopback"); + assert!( + refused.reason.contains("localhost is not a public address"), + "{}", + refused.reason + ); + } + #[test] fn preview_leaves_a_short_body_alone() { assert_eq!(preview(" {\"ok\":true} ", 100), "{\"ok\":true}"); @@ -208,7 +638,7 @@ mod tests { let addr = listener.local_addr().expect("addr"); tokio::spawn(async move { axum::serve(listener, app).await }); - let http = ReqwestGet::new(); + let http = ReqwestGet::trusting_any_address(); let small = http.get(&format!("http://{addr}/small")).await; assert_eq!(small.expect("small").body, "{}"); let huge = http.get(&format!("http://{addr}/huge")).await; diff --git a/crates/vibescrobble-index/src/identity.rs b/crates/vibescrobble-index/src/identity.rs index e9fffedc..8804b484 100644 --- a/crates/vibescrobble-index/src/identity.rs +++ b/crates/vibescrobble-index/src/identity.rs @@ -718,6 +718,42 @@ mod tests { .expect("a document") } + /// What an account names is read through the guarded client: a profile + /// on the metadata service, and a document on a plain or private host, + /// are refused before anything is sent. + #[tokio::test] + async fn an_account_cannot_point_the_index_at_a_private_address() { + let registry = Registry::new( + crate::resolve::DidResolver::new(crate::http::ReqwestGet::new()), + Identities::new(), + ); + let did = "did:plc:nlzmjyfv6loqtxyzvdcznwgf"; + for pds in [ + "https://169.254.169.254", + "https://[fd00:ec2::254]", + "https://10.0.0.7", + "http://relay:2470", + ] { + let server = ServerRef::parse(pds).expect(pds); + let refused = registry.read_bot(did, &server).await.expect_err(pds); + assert!( + refused.contains("not a public address") || refused.contains("not https"), + "{pds}: {refused}" + ); + } + let refused = registry + .resolver() + .resolve("did:web:localhost%3A3000") + .await + .expect_err("a document on loopback"); + assert!( + refused + .to_string() + .contains("localhost is not a public address"), + "{refused}" + ); + } + #[test] fn an_account_is_remembered_with_its_handle() { let mut store = Identities::new(); diff --git a/crates/vibescrobble-index/src/lib.rs b/crates/vibescrobble-index/src/lib.rs index 67f1cb4e..5b297c23 100644 --- a/crates/vibescrobble-index/src/lib.rs +++ b/crates/vibescrobble-index/src/lib.rs @@ -72,7 +72,9 @@ //! use std::sync::Arc; //! use vibescrobble_index::{FeedSource, Indexer, ReqwestGet, ServerDirectory, StaticDirectory}; //! -//! let http = Arc::new(ReqwestGet::new()); +//! // A development server on loopback; `ReqwestGet::new` reads public +//! // addresses only. +//! let http = Arc::new(ReqwestGet::trusting_any_address()); //! let (targets, refused) = StaticDirectory::parse("http://localhost:3000")? //! .targets() //! .await diff --git a/crates/vibescrobble-index/src/sse.rs b/crates/vibescrobble-index/src/sse.rs index 8b1639a2..b416d667 100644 --- a/crates/vibescrobble-index/src/sse.rs +++ b/crates/vibescrobble-index/src/sse.rs @@ -15,7 +15,7 @@ use std::time::Duration; use tokio::sync::mpsc; use tracing::{debug, warn}; -use crate::http::{stream_client, TransportError, IDLE_TIMEOUT}; +use crate::http::{stream_client, Client, TransportError, IDLE_TIMEOUT}; /// How long to wait before reconnecting a stream that ended. pub const RETRY: Duration = Duration::from_secs(3); @@ -80,7 +80,7 @@ pub async fn frames(url: String, out: mpsc::Sender) { /// alternative is a dependency that also knows about retry policies and /// last-event-ids that nothing here uses. pub async fn read_frames( - http: &reqwest::Client, + http: &Client, url: &str, out: &mpsc::Sender, ) -> Result<(), TransportError> { @@ -90,6 +90,7 @@ pub async fn read_frames( let response = http .get(url) + .map_err(|refusal| TransportError::new(url, refusal.to_string()))? .send() .await .map_err(|err| TransportError::new(url, err.to_string()))?; diff --git a/crates/vibescrobble-index/src/tls.rs b/crates/vibescrobble-index/src/tls.rs index a08f50c5..e974ed20 100644 --- a/crates/vibescrobble-index/src/tls.rs +++ b/crates/vibescrobble-index/src/tls.rs @@ -37,8 +37,9 @@ fn extra_pem() -> Option> { } /// A `reqwest` client builder that trusts [`EXTRA_CA_CERTS`] beside its own -/// roots. Callers add their own timeouts. -pub fn builder() -> reqwest::ClientBuilder { +/// roots. Only [`crate::http::Client`] builds on it, so that where a client +/// may connect is decided in one place. +pub(crate) fn builder() -> reqwest::ClientBuilder { let mut builder = reqwest::Client::builder(); if let Some(pem) = extra_pem() { match reqwest::Certificate::from_pem_bundle(&pem) { diff --git a/crates/vibescrobble-query/tests/avatar.rs b/crates/vibescrobble-query/tests/avatar.rs index bba6cdca..d337907c 100644 --- a/crates/vibescrobble-query/tests/avatar.rs +++ b/crates/vibescrobble-query/tests/avatar.rs @@ -144,7 +144,10 @@ fn set_avatar(fake: &Fake, bytes: Vec, declared: usize) -> String { async fn setup() -> (Shared, DidResolver, Fake) { let fake = Fake::default(); let addr = fake_server(fake.clone()).await; - let resolver = DidResolver::with_plc_directory(ReqwestGet::new(), &format!("http://{addr}")); + let resolver = DidResolver::with_plc_directory( + ReqwestGet::trusting_any_address(), + &format!("http://{addr}"), + ); let shared = Shared::new(); let seen = shared.view.lock().await.see_operator(Operator { did: OPERATOR.to_owned(), diff --git a/docs/running-locally.md b/docs/running-locally.md index 95d215ed..f5cc3f2c 100644 --- a/docs/running-locally.md +++ b/docs/running-locally.md @@ -205,8 +205,12 @@ document names, every ten minutes for each operator and each agent not dormant, and serves the picture re-encoded at 144 pixels square, holding at most 2048. A new picture gets a new path, so each one is cached as immutable. Run with `--pds`, the index reads profiles from any address, which lets a -server on loopback work. Run with `--jetstream`, it reads them only over -https, only from public addresses, and never follows a redirect. An account +server on loopback work. Run with `--jetstream`, everything it reads because an +account named it — DID documents, handles, profiles, registrations, records +and pictures — goes only over https, only to public addresses, and through at +most three redirects held to the same; `--private-addresses` lifts that for a +development stack. The relay `--relay` names is the deployment's own, and is +read wherever it is. An account with no avatar has no `avatar`, and the page draws it a generated portrait. Against the index's `--prune` window (24 hours unless told otherwise, 1 to diff --git a/spike/stack/run.sh b/spike/stack/run.sh index 8e778088..3d21cfe0 100755 --- a/spike/stack/run.sh +++ b/spike/stack/run.sh @@ -121,6 +121,7 @@ serve) # (see plan/order.md), and it is not what this stack is here to exercise. "$REPO/target/debug/vibescrobble-index" \ --jetstream ws://127.0.0.1:8080 \ + --private-addresses \ --identities "$STATE/identities.jsonl" \ --cluster-every 0 >"$STATE/index.log" 2>&1 & echo $! > "$STATE/index.pid" -- knotmirror 0.0.1