diff --git a/knot2/README.md b/knot2/README.md index b9b5305b..30ed55ed 100644 --- a/knot2/README.md +++ b/knot2/README.md @@ -193,6 +193,8 @@ That talks to the knot by container name, which needs both containers on a singl Set `xrpc.trusted_proxy_header = "x-forwarded-for"` when doing this, otherwise every client looks like it comes from the proxy and the ratelimiter wil treat them as one very busy mister. Only set it behind a proxy the operator controls, since a direct client can like, invent that header. +Add `xrpc.trusted_proxies = ["fd00:1::4", "10.89.0.4"]` for example, one entry per address that the proxy connects from, so the knot honors that header from the proxy alone & ratelimits anyone else by the address they connected from. + The knot can also terminate TLS itself (and that's the only way to get its HTTP3 support) because a plain TCP frontend can't proxy QUIC. Using a certificate the operator already manages: ```toml diff --git a/knot2/crates/knot-bench/benches/pack.rs b/knot2/crates/knot-bench/benches/pack.rs index 30b13d10..b4ffcdde 100644 --- a/knot2/crates/knot-bench/benches/pack.rs +++ b/knot2/crates/knot-bench/benches/pack.rs @@ -202,10 +202,12 @@ fn push(bencher: Bencher, commits: u32) { fn archive(bencher: Bencher, commits: u32) { let history = build_history(spec_for(commits)); let request = build_archive_request(history.tip()); - let bytes = upload_archive(history.repo(), &request).unwrap().len(); - bencher - .counter(BytesCount::new(bytes)) - .bench_local(|| upload_archive(history.repo(), &request).unwrap()); + let bytes = upload_archive(history.repo(), &request, knot_git::ArchiveLimit::default()) + .unwrap() + .len(); + bencher.counter(BytesCount::new(bytes)).bench_local(|| { + upload_archive(history.repo(), &request, knot_git::ArchiveLimit::default()).unwrap() + }); } struct FreshTarget { diff --git a/knot2/crates/knot-config/src/lib.rs b/knot2/crates/knot-config/src/lib.rs index 02c0005f..b5adee8a 100644 --- a/knot2/crates/knot-config/src/lib.rs +++ b/knot2/crates/knot-config/src/lib.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::net::SocketAddr; +use std::net::{AddrParseError, IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; use std::sync::OnceLock; use std::time::Duration; @@ -237,6 +237,11 @@ pub struct XrpcConfig { #[config(env = "KNOT_XRPC_MAX_RESPONSE_BYTES", default = 5_242_880)] pub max_response_bytes: u64, + /// Upper bound on bytes that a single archive spools, + /// across all our surfaces: the sh.tangled.repo.archive query, + /// `git archive --remote` over SSH, + /// and the smart HTTP archive route. + /// The knot will refuse writing smth that would blast an archive past this bound. #[config(env = "KNOT_XRPC_MAX_ARCHIVE_BYTES", default = 1_073_741_824)] pub max_archive_bytes: u64, @@ -303,6 +308,20 @@ pub struct XrpcConfig { #[config(env = "KNOT_XRPC_TRUSTED_PROXY_HEADER")] pub trusted_proxy_header: Option, + /// IP addresses whose `trusted_proxy_header` the knot honors, + /// without a port, + /// for ex the loopback address of a reverse proxy on the same host. + /// The knot rate-limits a request from any other address + /// by its own socket address and ignores the header. + /// Leave empty to honor the header from every peer, + /// which is safe *only* if nothing but the proxy can reach this knot. + #[config( + env = "KNOT_XRPC_TRUSTED_PROXIES", + parse_env = parse_trusted_proxies, + default = [] + )] + pub trusted_proxies: Vec, + #[config(env = "KNOT_XRPC_EVENTS_REPLAY_BUFFER", default = 4096)] pub events_replay_buffer: u32, @@ -420,6 +439,14 @@ fn parse_admins(raw: &str) -> Result, knot_types::ParseError> { .collect() } +fn parse_trusted_proxies(raw: &str) -> Result, AddrParseError> { + raw.split(',') + .map(str::trim) + .filter(|item| !item.is_empty()) + .map(str::parse) + .collect() +} + impl KnotConfig { pub fn object_format(&self) -> Option { knot_types::ObjectFormat::from_capability(&self.git.object_format) @@ -803,6 +830,10 @@ impl KnotConfig { .as_ref() .filter(|header| !is_http_token(header)) .map(|_| "xrpc.trusted_proxy_header isn't valid HTTP header name".to_string()), + check( + self.xrpc.trusted_proxy_header.is_some() || self.xrpc.trusted_proxies.is_empty(), + "xrpc.trusted_proxies needs xrpc.trusted_proxy_header, the header the knot honors from those addresses", + ), self.acl .legacy_admin_secret_env .as_deref() @@ -1188,6 +1219,7 @@ mod tests { fork_max_pack_bytes: 1_073_741_824, fork_fetch_timeout_ms: 600_000, trusted_proxy_header: None, + trusted_proxies: Vec::new(), events_replay_buffer: 4_096, events_replay_bytes: 67_108_864, events_max_subscribers: 256, @@ -1542,6 +1574,11 @@ mod tests { |config| config.homepage.path = Some(PathBuf::from("homepage.html")), "homepage.path must be absolute path", ), + ( + "trusted_proxies_without_the_header_the_knot_honors", + |config| config.xrpc.trusted_proxies = vec!["127.0.0.1".parse().unwrap()], + "needs xrpc.trusted_proxy_header", + ), ]; cases.iter().for_each(|(label, mutate, expected)| { let mut config = sample(); @@ -1610,6 +1647,22 @@ mod tests { assert!(parse_admins("not-a-did").is_err()); } + #[test] + fn trusted_proxies_parse_from_comma_separated_env() { + assert_eq!( + parse_trusted_proxies("127.0.0.1, ::1").unwrap(), + vec![ + "127.0.0.1".parse::().unwrap(), + "::1".parse::().unwrap() + ] + ); + assert!(parse_trusted_proxies("").unwrap().is_empty()); + assert!( + parse_trusted_proxies("127.0.0.1:5555").is_err(), + "xrpc.trusted_proxies takes bare IP addresses, so a port must fail to parse" + ); + } + #[test] fn http_limits_map_from_config() { let limits = sample().http_limits(); diff --git a/knot2/crates/knot-edge/src/lib.rs b/knot2/crates/knot-edge/src/lib.rs index 7dd3c217..5248b784 100644 --- a/knot2/crates/knot-edge/src/lib.rs +++ b/knot2/crates/knot-edge/src/lib.rs @@ -290,7 +290,7 @@ mod tests { RequestTimeout::from_millis(NonZeroU64::new(30_000).unwrap()), BodyInactivityTimeout::from_millis(NonZeroU64::new(30_000).unwrap()), WriteRequestTimeout::from_millis(NonZeroU64::new(1_800_000).unwrap()), - None, + knot_types::ProxyTrust::default(), ) .prepare(&CancellationToken::new()) } @@ -312,7 +312,7 @@ mod tests { RequestTimeout::from_millis(NonZeroU64::new(30_000).unwrap()), BodyInactivityTimeout::from_millis(NonZeroU64::new(30_000).unwrap()), WriteRequestTimeout::from_millis(NonZeroU64::new(1_800_000).unwrap()), - None, + knot_types::ProxyTrust::default(), ) .prepare(&CancellationToken::new()) } diff --git a/knot2/crates/knot-edge/src/robustness.rs b/knot2/crates/knot-edge/src/robustness.rs index 1142cc1e..7357d3cb 100644 --- a/knot2/crates/knot-edge/src/robustness.rs +++ b/knot2/crates/knot-edge/src/robustness.rs @@ -1,6 +1,6 @@ use std::net::{IpAddr, SocketAddr}; use std::num::{NonZeroU32, NonZeroU64}; -use std::sync::Arc; +use std::sync::{Arc, OnceLock}; use std::time::Duration; use axum::Router; @@ -10,7 +10,8 @@ use axum::extract::{ConnectInfo, State}; use axum::middleware::{Next, from_fn_with_state}; use axum::response::{IntoResponse, Response}; use governor::middleware::NoOpMiddleware; -use http::{HeaderName, Method, Request, StatusCode}; +use http::{Method, Request, StatusCode}; +use knot_types::ProxyTrust; use tokio_util::sync::CancellationToken; use tower::limit::GlobalConcurrencyLimitLayer; use tower::load_shed::LoadShedLayer; @@ -77,7 +78,7 @@ pub struct EdgeGuards { request_timeout: RequestTimeout, body_timeout: BodyInactivityTimeout, write_request_timeout: WriteRequestTimeout, - proxy_header: Option, + proxy_trust: ProxyTrust, } impl EdgeGuards { @@ -88,7 +89,7 @@ impl EdgeGuards { request_timeout: RequestTimeout, body_timeout: BodyInactivityTimeout, write_request_timeout: WriteRequestTimeout, - proxy_header: Option, + proxy_trust: ProxyTrust, ) -> Self { Self { rate, @@ -97,12 +98,12 @@ impl EdgeGuards { request_timeout, body_timeout, write_request_timeout, - proxy_header, + proxy_trust, } } pub(crate) fn prepare(self, shutdown: &CancellationToken) -> GuardLayers { - let governor = build_governor(self.rate, self.burst, self.proxy_header); + let governor = build_governor(self.rate, self.burst, self.proxy_trust); spawn_state_cleanup(Arc::clone(&governor), shutdown.clone()); GuardLayers { governor, @@ -131,40 +132,53 @@ struct TimeoutBudget { extended: Duration, } +#[derive(Clone, Default)] +struct IgnoredHeaderNotice(Arc>); + +impl IgnoredHeaderNotice { + fn report(&self, peer: IpAddr) { + if self.0.set(peer).is_ok() { + tracing::warn!( + %peer, + "a peer outside xrpc.trusted_proxies sent xrpc.trusted_proxy_header, so the knot ignored the header and rate-limits that peer by the address it connected from. Add this address to xrpc.trusted_proxies if it is the reverse proxy, since a proxy reaches the knot over one address family and listing the other one silently loses the header. This warning reports the first such peer only." + ); + } + } +} + #[derive(Clone)] struct ProxyAwareIp { - header: Option, + trust: ProxyTrust, + ignored_header: IgnoredHeaderNotice, } impl KeyExtractor for ProxyAwareIp { type Key = IpAddr; fn extract(&self, request: &Request) -> Result { - let from_header = self - .header - .as_ref() - .and_then(|header| knot_types::forwarded_peer(request.headers(), header)); - from_header - .or_else(|| { - request - .extensions() - .get::>() - .map(|info| info.0.ip()) - }) - .ok_or(GovernorError::UnableToExtractKey) + let socket = request + .extensions() + .get::>() + .map(|info| info.0.ip()); + let key = self.trust.peer_key(request.headers(), socket); + if let Some(peer) = key.ignored_header() { + self.ignored_header.report(peer); + } + key.address().ok_or(GovernorError::UnableToExtractKey) } } fn build_governor( rate: RequestsPerSecond, burst: BurstSize, - proxy_header: Option, + proxy_trust: ProxyTrust, ) -> Arc { let mut builder = GovernorConfigBuilder::default(); builder.period(rate.period()).burst_size(burst.0.get()); let config = builder .key_extractor(ProxyAwareIp { - header: proxy_header, + trust: proxy_trust, + ignored_header: IgnoredHeaderNotice::default(), }) .finish() .expect("a non-zero rate period and burst size always yield a governor config"); @@ -257,7 +271,7 @@ mod tests { inflight: u32, request_timeout_ms: u64, body_timeout_ms: u64, - proxy_header: Option<&str>, + proxy_trust: ProxyTrust, ) -> EdgeGuards { guards_with_write( rate, @@ -266,7 +280,7 @@ mod tests { request_timeout_ms, body_timeout_ms, request_timeout_ms, - proxy_header, + proxy_trust, ) } @@ -278,7 +292,7 @@ mod tests { request_timeout_ms: u64, body_timeout_ms: u64, write_request_timeout_ms: u64, - proxy_header: Option<&str>, + proxy_trust: ProxyTrust, ) -> EdgeGuards { EdgeGuards::new( RequestsPerSecond::new(NonZeroU32::new(rate).unwrap()), @@ -287,10 +301,32 @@ mod tests { RequestTimeout::from_millis(NonZeroU64::new(request_timeout_ms).unwrap()), BodyInactivityTimeout::from_millis(NonZeroU64::new(body_timeout_ms).unwrap()), WriteRequestTimeout::from_millis(NonZeroU64::new(write_request_timeout_ms).unwrap()), - proxy_header.map(|header| HeaderName::from_bytes(header.as_bytes()).unwrap()), + proxy_trust, + ) + } + + fn forwarded_for() -> http::HeaderName { + http::HeaderName::from_static("x-forwarded-for") + } + + fn trusting_any_peer() -> ProxyTrust { + ProxyTrust::new(Some(forwarded_for()), knot_types::TrustedProxies::default()) + } + + fn trusting_loopback() -> ProxyTrust { + ProxyTrust::new( + Some(forwarded_for()), + knot_types::TrustedProxies::new(["127.0.0.1".parse::().unwrap()]), ) } + fn extractor(trust: ProxyTrust) -> ProxyAwareIp { + ProxyAwareIp { + trust, + ignored_header: IgnoredHeaderNotice::default(), + } + } + fn guarded_router(router: Router, guards: EdgeGuards) -> Router { apply(router, guards.prepare(&CancellationToken::new())) } @@ -309,9 +345,7 @@ mod tests { #[test] fn the_extractor_keys_on_the_trusted_proxy_header_when_configured() { - let extractor = ProxyAwareIp { - header: Some(HeaderName::from_static("x-forwarded-for")), - }; + let extractor = extractor(trusting_any_peer()); let request = Request::get("/") .header("x-forwarded-for", "203.0.113.7, 198.51.100.4") .extension(ConnectInfo(SocketAddr::from(([127, 0, 0, 1], 5000)))) @@ -326,7 +360,7 @@ mod tests { #[test] fn the_extractor_ignores_a_forgeable_header_when_no_proxy_is_trusted() { - let extractor = ProxyAwareIp { header: None }; + let extractor = extractor(ProxyTrust::default()); let request = Request::get("/") .header("x-forwarded-for", "203.0.113.7") .extension(ConnectInfo(SocketAddr::from(([10, 0, 0, 9], 5000)))) @@ -340,10 +374,30 @@ mod tests { } #[test] - fn the_extractor_falls_back_to_the_peer_when_the_trusted_header_is_absent() { - let extractor = ProxyAwareIp { - header: Some(HeaderName::from_static("x-forwarded-for")), + fn the_extractor_keys_an_unlisted_peer_on_its_socket_however_it_fills_the_header() { + let extractor = extractor(trusting_loopback()); + let forged = |host| { + Request::get("/") + .header("x-forwarded-for", "198.51.100.4") + .extension(ConnectInfo(SocketAddr::from(([127, 0, 0, host], 5000)))) + .body(()) + .unwrap() }; + assert_eq!( + extractor.extract(&forged(1)).unwrap(), + "198.51.100.4".parse::().unwrap(), + "the listed proxy relayed this one, so the extractor keys on the header address" + ); + assert_eq!( + extractor.extract(&forged(9)).unwrap(), + "127.0.0.9".parse::().unwrap(), + "an unlisted peer picked its own token bucket by forging the header" + ); + } + + #[test] + fn the_extractor_falls_back_to_the_peer_when_the_trusted_header_is_absent() { + let extractor = extractor(trusting_any_peer()); let request = Request::get("/") .extension(ConnectInfo(SocketAddr::from(([10, 0, 0, 9], 5000)))) .body(()) @@ -356,7 +410,7 @@ mod tests { #[test] fn the_extractor_fails_when_no_peer_can_be_identified() { - let extractor = ProxyAwareIp { header: None }; + let extractor = extractor(ProxyTrust::default()); let request = Request::get("/").body(()).unwrap(); assert!(matches!( extractor.extract(&request), @@ -364,11 +418,80 @@ mod tests { )); } + #[test] + fn a_missing_connect_info_fails_closed_rather_than_taking_the_header_on_trust() { + let listed = extractor(trusting_loopback()); + let headed = || { + Request::get("/") + .header("x-forwarded-for", "198.51.100.4") + .body(()) + .unwrap() + }; + assert!( + matches!( + listed.extract(&headed()), + Err(GovernorError::UnableToExtractKey) + ), + "with no socket to match against the allowlist the extractor identifies no client" + ); + assert_eq!( + extractor(trusting_any_peer()).extract(&headed()).unwrap(), + "198.51.100.4".parse::().unwrap(), + "an operator who lists no proxy already told the knot to take the header from anyone" + ); + } + + #[test] + fn the_first_unlisted_peer_sending_the_header_is_reported_once() { + let extractor = extractor(trusting_loopback()); + let forged = |host| { + Request::get("/") + .header("x-forwarded-for", "198.51.100.4") + .extension(ConnectInfo(SocketAddr::from(([203, 0, 113, host], 5000)))) + .body(()) + .unwrap() + }; + extractor.extract(&forged(7)).unwrap(); + extractor.extract(&forged(9)).unwrap(); + assert_eq!( + extractor.ignored_header.0.get(), + Some(&"203.0.113.7".parse::().unwrap()), + "a wrong-family allowlist sends every request down this path, so only the first peer is reported" + ); + } + + #[test] + fn a_listed_proxy_and_a_headerless_request_report_nothing() { + let listed = extractor(trusting_loopback()); + listed + .extract( + &Request::get("/") + .header("x-forwarded-for", "198.51.100.4") + .extension(ConnectInfo(SocketAddr::from(([127, 0, 0, 1], 5000)))) + .body(()) + .unwrap(), + ) + .unwrap(); + listed + .extract( + &Request::get("/") + .extension(ConnectInfo(SocketAddr::from(([203, 0, 113, 7], 5000)))) + .body(()) + .unwrap(), + ) + .unwrap(); + assert_eq!( + listed.ignored_header.0.get(), + None, + "neither a relayed request or a request without the header says anything about the allowlist" + ); + } + #[tokio::test] async fn a_well_behaved_request_passes_every_guard() { let app = guarded_router( Router::new().route("/", get(|| async { "ok" })), - guards(50, 200, 1_024, 60_000, 30_000, None), + guards(50, 200, 1_024, 60_000, 30_000, ProxyTrust::default()), ); let status = app .oneshot(from_peer(get_request(), 1)) @@ -382,7 +505,7 @@ mod tests { async fn a_burst_beyond_the_per_ip_limit_is_rejected_with_429() { let app = guarded_router( Router::new().route("/", get(|| async { "ok" })), - guards(1, 2, 1_024, 60_000, 30_000, None), + guards(1, 2, 1_024, 60_000, 30_000, ProxyTrust::default()), ); let first = app .clone() @@ -432,7 +555,7 @@ mod tests { "ok" }), ), - guards(10_000, 10_000, 1, 60_000, 30_000, None), + guards(10_000, 10_000, 1, 60_000, 30_000, ProxyTrust::default()), ); let holder = { let app = app.clone(); @@ -469,7 +592,7 @@ mod tests { "ok" }), ), - guards(10_000, 10_000, 1_024, 80, 30_000, None), + guards(10_000, 10_000, 1_024, 80, 30_000, ProxyTrust::default()), ); let status = app .oneshot(from_peer( @@ -500,7 +623,15 @@ mod tests { "ok" }), ), - guards_with_write(10_000, 10_000, 1_024, 80, 30_000, 5_000, None), + guards_with_write( + 10_000, + 10_000, + 1_024, + 80, + 30_000, + 5_000, + ProxyTrust::default(), + ), ); let push = app .clone() @@ -539,7 +670,7 @@ mod tests { async fn a_stalled_request_body_is_cut_and_never_hangs() { let app = guarded_router( Router::new().route("/upload", post(|_body: Bytes| async { "ok" })), - guards(10_000, 10_000, 1_024, 60_000, 80, None), + guards(10_000, 10_000, 1_024, 60_000, 80, ProxyTrust::default()), ); let body = Body::from_stream( futures::stream::once(async { diff --git a/knot2/crates/knot-git/src/archive.rs b/knot2/crates/knot-git/src/archive.rs index d648299e..14b41bac 100644 --- a/knot2/crates/knot-git/src/archive.rs +++ b/knot2/crates/knot-git/src/archive.rs @@ -1,11 +1,25 @@ +use std::io::{Seek, SeekFrom, Write}; use std::sync::atomic::AtomicBool; use gix::bstr::BString; use knot_types::{Oid, ParseError}; use crate::error::{GitError, backend}; +use crate::objects::MAX_TREE_DEPTH; use crate::repo::Repo; +const TAR_BLOCK: u64 = 512; + +knot_types::scalar_newtype! { + pub struct ArchiveLimit(u64); +} + +impl Default for ArchiveLimit { + fn default() -> Self { + Self::new(1024 * 1024 * 1024) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ArchiveFormat { Tar, @@ -65,32 +79,160 @@ impl Repo { tree: Oid, format: ArchiveFormat, prefix: Option<&ArchivePrefix>, - mut out: impl std::io::Write + std::io::Seek, + limit: ArchiveLimit, + out: impl std::io::Write + std::io::Seek, ) -> Result<(), GitError> { + self.bound_archive_source(tree.object_id(), limit, MAX_TREE_DEPTH, &mut 0)?; let (stream, _index) = self .git() .worktree_stream(tree.object_id()) .map_err(backend)?; let interrupt = AtomicBool::new(false); - self.git() - .worktree_archive( - stream, - &mut out, - gix::progress::Discard, - &interrupt, - gix_archive::Options { - format: format.gix(), - tree_prefix: prefix.map(|prefix| BString::from(prefix.as_str())), - modification_time: 0, - }, - ) - .map_err(backend) + let mut spool = BoundedSpool { + inner: out, + position: 0, + limit, + overflowed: false, + }; + let written = self.git().worktree_archive( + stream, + &mut spool, + gix::progress::Discard, + &interrupt, + gix_archive::Options { + format: format.gix(), + tree_prefix: prefix.map(|prefix| BString::from(prefix.as_str())), + modification_time: 0, + }, + ); + match (written, spool.overflowed) { + (_, true) => Err(GitError::ArchiveTooLarge { limit }), + (Ok(()), false) => Ok(()), + (Err(error), false) => Err(backend(error)), + } + } + + fn bound_archive_source( + &self, + tree: gix::ObjectId, + limit: ArchiveLimit, + nesting: usize, + spooled: &mut u64, + ) -> Result<(), GitError> { + if nesting == 0 { + return Err(GitError::DepthExceeded("tree nesting")); + } + if tree == gix::ObjectId::empty_tree(self.git().object_hash()) { + return Ok(()); + } + let object = self.git().find_tree(tree).map_err(backend)?; + let decoded = object + .decode() + .map_err(|error| GitError::Decode(error.to_string()))?; + decoded.entries.iter().try_for_each(|entry| { + let oid = entry.oid.to_owned(); + *spooled = spooled.saturating_add(TAR_BLOCK); + match entry.mode.kind() { + _ if *spooled > limit.get() => Err(GitError::ArchiveTooLarge { limit }), + gix::objs::tree::EntryKind::Commit => Ok(()), + gix::objs::tree::EntryKind::Tree => { + self.bound_archive_source(oid, limit, nesting - 1, spooled) + } + _ => { + let content = self.blob_size(Oid::from(oid))?; + *spooled = spooled.saturating_add(content.next_multiple_of(TAR_BLOCK)); + match *spooled > limit.get() { + true => Err(GitError::ArchiveTooLarge { limit }), + false => Ok(()), + } + } + } + }) + } +} + +struct BoundedSpool { + inner: W, + position: u64, + limit: ArchiveLimit, + overflowed: bool, +} + +impl Write for BoundedSpool { + fn write(&mut self, data: &[u8]) -> std::io::Result { + let remaining = self.limit.get().saturating_sub(self.position); + if data.len() as u64 > remaining { + self.overflowed = true; + return Err(std::io::Error::from(std::io::ErrorKind::WriteZero)); + } + let written = self.inner.write(data)?; + self.position = self.position.saturating_add(written as u64); + Ok(written) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } +} + +impl Seek for BoundedSpool { + fn seek(&mut self, pos: SeekFrom) -> std::io::Result { + let position = self.inner.seek(pos)?; + self.position = position; + Ok(position) } } #[cfg(test)] mod tests { - use super::ArchivePrefix; + use super::{ArchiveLimit, ArchivePrefix, BoundedSpool}; + use std::io::{Seek, SeekFrom, Write}; + + fn spool(limit: u64) -> BoundedSpool>> { + BoundedSpool { + inner: std::io::Cursor::new(Vec::new()), + position: 0, + limit: ArchiveLimit::new(limit), + overflowed: false, + } + } + + #[test] + fn the_spool_refuses_the_write_that_would_pass_the_limit() { + let mut spool = spool(8); + assert!(spool.write_all(b"12345678").is_ok()); + assert!(!spool.overflowed); + assert!(spool.write_all(b"9").is_err()); + assert!(spool.overflowed); + assert_eq!( + spool.inner.into_inner(), + b"12345678", + "the refused write never reaches the inner writer" + ); + } + + #[test] + fn a_seek_backwards_re_credits_the_budget_the_zip_writer_rewinds_over() { + let mut spool = spool(8); + spool.write_all(b"12345678").unwrap(); + spool.seek(SeekFrom::Start(4)).unwrap(); + assert_eq!(spool.position, 4); + spool + .write_all(b"abcd") + .expect("rewriting bytes already counted stays within the limit"); + assert!(!spool.overflowed); + } + + #[test] + fn a_write_whose_length_would_overflow_the_position_is_refused() { + let mut spool = spool(u64::MAX); + spool.position = u64::MAX; + assert!( + spool.write_all(b"1").is_err(), + "the position saturates at u64::MAX, so the spool must refuse the write" + ); + assert!(spool.overflowed); + } #[test] fn a_plain_nested_prefix_is_accepted() { diff --git a/knot2/crates/knot-git/src/error.rs b/knot2/crates/knot-git/src/error.rs index b85dd888..a5e95248 100644 --- a/knot2/crates/knot-git/src/error.rs +++ b/knot2/crates/knot-git/src/error.rs @@ -39,6 +39,8 @@ pub enum GitError { ReservedDid(String), #[error("{0} exceeds maximum supported depth")] DepthExceeded(&'static str), + #[error("archive exceeds the {} byte limit", limit.get())] + ArchiveTooLarge { limit: crate::ArchiveLimit }, #[error("revision walk: {0}")] RevWalk(String), #[error("upload-pack selection exceeded its {0}")] diff --git a/knot2/crates/knot-git/src/lib.rs b/knot2/crates/knot-git/src/lib.rs index 29a640a6..0ef29958 100644 --- a/knot2/crates/knot-git/src/lib.rs +++ b/knot2/crates/knot-git/src/lib.rs @@ -12,7 +12,7 @@ mod reads; mod repo; mod staging; -pub use archive::{ArchiveFormat, ArchivePrefix}; +pub use archive::{ArchiveFormat, ArchiveLimit, ArchivePrefix}; pub use bitmap::{reachable_via_bitmap, verbatim_clone_pack, write_bitmap, write_midx_bitmap}; pub use error::{GitError, SelectionLimit}; pub use maintenance::{PackRefsReport, ReflogReport}; diff --git a/knot2/crates/knot-git/tests/config_isolation.rs b/knot2/crates/knot-git/tests/config_isolation.rs index 0c0aa2c8..82ea6995 100644 --- a/knot2/crates/knot-git/tests/config_isolation.rs +++ b/knot2/crates/knot-git/tests/config_isolation.rs @@ -1,7 +1,7 @@ use std::path::Path; use std::sync::atomic::AtomicBool; -use knot_git::ArchiveFormat; +use knot_git::{ArchiveFormat, ArchiveLimit}; use knot_types::Oid; mod common; @@ -80,8 +80,14 @@ fn a_filter_driver_pulled_in_by_an_include_never_runs_for_a_served_archive() { let bare = layout.open(&did).unwrap(); let tree = bare.peel_to_tree(head).unwrap(); let mut out = std::io::Cursor::new(Vec::new()); - bare.write_archive(tree, ArchiveFormat::Tar, None, &mut out) - .unwrap(); + bare.write_archive( + tree, + ArchiveFormat::Tar, + None, + ArchiveLimit::new(u64::MAX), + &mut out, + ) + .unwrap(); let served = out.into_inner(); assert!( @@ -100,3 +106,37 @@ fn a_filter_driver_pulled_in_by_an_include_never_runs_for_a_served_archive() { "the knot ran a filter driver defined by config outside the repository" ); } + +#[test] +fn a_pushed_replace_ref_never_substitutes_an_object_the_knot_reads() { + let (_scan, work_dir, layout, did) = seeded(); + let work = work_dir.path(); + let bare_path = layout.repo_path(&did).unwrap(); + + commit_file(work, "payload.txt", "kelp\n", "seed"); + git(work, &["push", "-q", bare_path.to_str().unwrap(), "main"]); + let original = Oid::from_hex(&git(work, &["rev-parse", "HEAD:payload.txt"])).unwrap(); + commit_file(work, "payload.txt", "pwned\n", "second"); + git(work, &["push", "-q", bare_path.to_str().unwrap(), "main"]); + let substitute = Oid::from_hex(&git(work, &["rev-parse", "HEAD:payload.txt"])).unwrap(); + + git( + &bare_path, + &[ + "update-ref", + &format!("refs/replace/{}", original.to_hex()), + &substitute.to_hex(), + ], + ); + + assert_eq!( + git(&bare_path, &["cat-file", "blob", &original.to_hex()]), + "pwned", + "git read the replaced object as itself, so this fixture never armed the substitution" + ); + assert_eq!( + layout.open(&did).unwrap().read_blob(original).unwrap(), + b"kelp\n", + "a pushed replace ref rewrote what the knot serves for an object" + ); +} diff --git a/knot2/crates/knot-git/tests/reads.rs b/knot2/crates/knot-git/tests/reads.rs index 44b91479..fba900d2 100644 --- a/knot2/crates/knot-git/tests/reads.rs +++ b/knot2/crates/knot-git/tests/reads.rs @@ -628,6 +628,7 @@ fn archives_round_trip_through_tar() { tree, knot_git::ArchiveFormat::TarGz, Some(&knot_git::ArchivePrefix::new("squid-main/").unwrap()), + knot_git::ArchiveLimit::new(u64::MAX), &mut out, ) .unwrap(); @@ -647,6 +648,81 @@ fn archives_round_trip_through_tar() { ); } +#[test] +fn an_archive_stops_at_its_limit_instead_of_spooling_the_whole_tree() { + let (_scan, work_dir, layout, did) = seed_rich(); + let work = work_dir.path(); + let bare = layout.open(&did).unwrap(); + let head = Oid::from_hex(&git(work, &["rev-parse", "HEAD"])).unwrap(); + let tree = bare.peel_to_tree(head).unwrap(); + + [ + knot_git::ArchiveFormat::Tar, + knot_git::ArchiveFormat::TarGz, + knot_git::ArchiveFormat::Zip, + ] + .iter() + .for_each(|format| { + let mut out = std::io::Cursor::new(Vec::new()); + let refused = bare.write_archive( + tree, + *format, + None, + knot_git::ArchiveLimit::new(512), + &mut out, + ); + assert!( + matches!(refused, Err(knot_git::GitError::ArchiveTooLarge { .. })), + "a {format:?} archive past its limit must be refused, got {refused:?}" + ); + assert!( + out.into_inner().len() <= 512, + "the {format:?} writer took bytes past the limit before the refusal" + ); + }); +} + +#[test] +fn a_compressible_tree_is_measured_before_the_compressor_ever_sees_it() { + let scan = tempfile::tempdir().unwrap(); + let layout = Layout::new(scan.path()); + let did = RepoDid::new("did:plc:whelk").unwrap(); + layout.create(&did).unwrap(); + let bare_path = layout.repo_path(&did).unwrap(); + + let work_dir = tempfile::tempdir().unwrap(); + let work = work_dir.path(); + git(work, &["init", "-q", "-b", "main"]); + commit_file( + work, + "kelp.txt", + &"kelp\n".repeat(200_000), + "one very compressible blob", + ); + git(work, &["push", "-q", bare_path.to_str().unwrap(), "main"]); + + let bare = layout.open(&did).unwrap(); + let head = Oid::from_hex(&git(work, &["rev-parse", "HEAD"])).unwrap(); + let tree = bare.peel_to_tree(head).unwrap(); + + let mut out = std::io::Cursor::new(Vec::new()); + let refused = bare.write_archive( + tree, + knot_git::ArchiveFormat::TarGz, + None, + knot_git::ArchiveLimit::new(64 * 1024), + &mut out, + ); + assert!( + matches!(refused, Err(knot_git::GitError::ArchiveTooLarge { .. })), + "a tree that gzips under the limit still costs its full size to read, so it must be refused, got {refused:?}" + ); + assert!( + out.into_inner().is_empty(), + "the knot must refuse before the compressor writes a byte" + ); +} + #[test] fn a_filename_with_a_backslash_is_addressable() { let (_scan, layout, did, bare_path, _head) = seed_main(); diff --git a/knot2/crates/knot-pack/src/archive.rs b/knot2/crates/knot-pack/src/archive.rs index 82a566d9..6b8e6188 100644 --- a/knot2/crates/knot-pack/src/archive.rs +++ b/knot2/crates/knot-pack/src/archive.rs @@ -1,6 +1,6 @@ use std::io::{self, Read, Seek, SeekFrom}; -use knot_git::{ArchiveFormat, ArchivePrefix, Repo}; +use knot_git::{ArchiveFormat, ArchiveLimit, ArchivePrefix, Repo}; use knot_types::Oid; use crate::error::PackError; @@ -15,10 +15,11 @@ struct Request { pub fn stream( repo: &Repo, request: &[u8], + limit: ArchiveLimit, sink: &mut dyn FnMut(&[u8]) -> io::Result<()>, ) -> Result<(), PackError> { let args = parse_arguments(request)?; - match build(repo, &args) { + match build(repo, &args, limit) { Ok(mut spool) => { let mut head = Vec::new(); pkt::write_data(&mut head, b"ACK\n")?; @@ -109,7 +110,7 @@ fn format_from(value: &str) -> ArchiveFormat { } } -fn build(repo: &Repo, args: &[String]) -> Result { +fn build(repo: &Repo, args: &[String], limit: ArchiveLimit) -> Result { let request = interpret(args)?; let id = repo .resolve_revision(&request.treeish) @@ -119,8 +120,14 @@ fn build(repo: &Repo, args: &[String]) -> Result { .peel_to_tree(commit) .map_err(|error| PackError::Pack(error.to_string()))?; let mut spool = tempfile::tempfile().map_err(|error| PackError::Pack(error.to_string()))?; - repo.write_archive(tree, request.format, request.prefix.as_ref(), &mut spool) - .map_err(|error| PackError::Pack(error.to_string()))?; + repo.write_archive( + tree, + request.format, + request.prefix.as_ref(), + limit, + &mut spool, + ) + .map_err(|error| PackError::Pack(error.to_string()))?; spool .seek(SeekFrom::Start(0)) .map_err(|error| PackError::Pack(error.to_string()))?; diff --git a/knot2/crates/knot-pack/src/lib.rs b/knot2/crates/knot-pack/src/lib.rs index de6198a9..33502b0b 100644 --- a/knot2/crates/knot-pack/src/lib.rs +++ b/knot2/crates/knot-pack/src/lib.rs @@ -219,14 +219,19 @@ pub fn receive_preflight(request: &[u8]) -> Preflight { pub fn upload_archive_streamed( repo: &Repo, request: &[u8], + limit: knot_git::ArchiveLimit, sink: &mut dyn FnMut(&[u8]) -> io::Result<()>, ) -> Result<(), PackError> { - archive::stream(repo, request, sink) + archive::stream(repo, request, limit, sink) } -pub fn upload_archive(repo: &Repo, request: &[u8]) -> Result, PackError> { +pub fn upload_archive( + repo: &Repo, + request: &[u8], + limit: knot_git::ArchiveLimit, +) -> Result, PackError> { let mut buf = Vec::new(); - upload_archive_streamed(repo, request, &mut |chunk| { + upload_archive_streamed(repo, request, limit, &mut |chunk| { buf.extend_from_slice(chunk); Ok(()) })?; @@ -340,15 +345,45 @@ struct PackState { cache: Arc, catalog: Arc, hostname: KnotHostname, + archive_limit: knot_git::ArchiveLimit, +} + +pub struct EdgeConfig { + pub layout: Layout, + pub resolver: Arc, + pub receive: Option>, + pub handle_resolver: Option>, + pub pack_slots: PackSlots, + pub cache: CacheConfig, + pub catalog: Arc, + pub hostname: KnotHostname, + pub clock: Arc, + pub archive_limit: knot_git::ArchiveLimit, +} + +impl EdgeConfig { + pub fn serving(layout: Layout, resolver: Arc, clock: Arc) -> Self { + Self { + layout, + resolver, + receive: None, + handle_resolver: None, + pack_slots: PackSlots::new(knot_resource::threads().get()), + cache: CacheConfig::default(), + catalog: Arc::new(Catalog::defaults()), + hostname: default_hostname().clone(), + clock, + archive_limit: knot_git::ArchiveLimit::default(), + } + } + + pub fn with_pack_slots(self, pack_slots: PackSlots) -> Self { + Self { pack_slots, ..self } + } } pub fn router(layout: Layout, resolver: Arc, clock: Arc) -> Router { - router_with_pack_slots( - layout, - resolver, - PackSlots::new(knot_resource::threads().get()), - clock, - ) + serving_router(EdgeConfig::serving(layout, resolver, clock)) } pub fn router_with_pack_slots( @@ -357,33 +392,21 @@ pub fn router_with_pack_slots( pack_slots: PackSlots, clock: Arc, ) -> Router { - let state = pack_state( - layout, - resolver, - None, - None, - pack_slots, - CacheConfig::default(), - Arc::new(Catalog::defaults()), - default_hostname().clone(), - clock, - ); + serving_router(EdgeConfig::serving(layout, resolver, clock).with_pack_slots(pack_slots)) +} + +fn serving_router(config: EdgeConfig) -> Router { + let state = pack_state(config); write_routes(state.clone()).merge(advertisement_routes(state).into_router()) } -#[allow(clippy::too_many_arguments)] -pub fn edge_routes( - layout: Layout, - resolver: Arc, - receive: Option>, - handle_resolver: Option>, - pack_slots: PackSlots, - cache: CacheConfig, - catalog: Arc, - hostname: KnotHostname, - clock: Arc, -) -> (Router, knot_edge::ZeroRttRoutes) { - let state = pack_state( +pub fn edge_routes(config: EdgeConfig) -> (Router, knot_edge::ZeroRttRoutes) { + let state = pack_state(config); + (write_routes(state.clone()), advertisement_routes(state)) +} + +fn pack_state(config: EdgeConfig) -> PackState { + let EdgeConfig { layout, resolver, receive, @@ -393,22 +416,8 @@ pub fn edge_routes( catalog, hostname, clock, - ); - (write_routes(state.clone()), advertisement_routes(state)) -} - -#[allow(clippy::too_many_arguments)] -fn pack_state( - layout: Layout, - resolver: Arc, - receive: Option>, - handle_resolver: Option>, - pack_slots: PackSlots, - cache: CacheConfig, - catalog: Arc, - hostname: KnotHostname, - clock: Arc, -) -> PackState { + archive_limit, + } = config; PackState { layout, resolver, @@ -418,6 +427,7 @@ fn pack_state( cache: cache::PackCache::new(cache, clock), catalog, hostname, + archive_limit, } } @@ -830,10 +840,15 @@ async fn archive_dispatch( body: Vec, ) -> Result { let permit = state.pack_slots.acquire().await; - Ok(archive_response(repo, body, permit)) + Ok(archive_response(repo, body, state.archive_limit, permit)) } -fn archive_response(repo: Repo, body: Vec, permit: SlotPermit) -> Response { +fn archive_response( + repo: Repo, + body: Vec, + limit: knot_git::ArchiveLimit, + permit: SlotPermit, +) -> Response { let (tx, rx) = mpsc::channel::>(16); tokio::task::spawn_blocking(move || { let _permit = permit; @@ -841,7 +856,7 @@ fn archive_response(repo: Repo, body: Vec, permit: SlotPermit) -> Response { tx.blocking_send(Ok(Bytes::copy_from_slice(chunk))) .map_err(|_| io::Error::other("client disconnected")) }; - if let Err(error) = upload_archive_streamed(&repo, &body, &mut sink) { + if let Err(error) = upload_archive_streamed(&repo, &body, limit, &mut sink) { let _ = tx.blocking_send(Err(io::Error::other(error.to_string()))); } }); diff --git a/knot2/crates/knot-pack/tests/git_client.rs b/knot2/crates/knot-pack/tests/git_client.rs index 6a3d22c1..478349f4 100644 --- a/knot2/crates/knot-pack/tests/git_client.rs +++ b/knot2/crates/knot-pack/tests/git_client.rs @@ -483,7 +483,7 @@ async fn http_upload_archive_serves_a_framed_tar_and_guards_refuse_cob_raw_oids_ args.iter() .for_each(|arg| request.extend(pkt(arg.as_bytes()))); request.extend_from_slice(b"0000"); - knot_pack::upload_archive(&repo, &request).unwrap() + knot_pack::upload_archive(&repo, &request, knot_git::ArchiveLimit::default()).unwrap() }; let raw_arg = format!("argument {}\n", tree.to_hex()); @@ -527,6 +527,63 @@ async fn http_upload_archive_serves_a_framed_tar_and_guards_refuse_cob_raw_oids_ ); } +#[tokio::test(flavor = "multi_thread")] +async fn http_upload_archive_honors_the_configured_archive_limit() { + use tower::ServiceExt as _; + + let scan = tempfile::tempdir().unwrap(); + let layout = Layout::new(scan.path()); + let did = RepoDid::new("did:plc:limpet").unwrap(); + layout.create(&did).unwrap(); + let bare = layout.repo_path(&did).unwrap(); + + let scratch = tempfile::tempdir().unwrap(); + let work = scratch.path().join("work"); + seed_repo(&work, bare.to_str().unwrap(), "README.md", "archive me\n"); + + let (write_routes, _advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { + pack_slots: knot_resource::PackSlots::new(1), + archive_limit: knot_git::ArchiveLimit::new(512), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + serve_dids(), + Arc::new(knot_runtime::SystemClock), + ) + }); + + let mut framed = Vec::new(); + framed.extend(pkt(b"argument --format=tar\n")); + framed.extend(pkt(b"argument HEAD\n")); + framed.extend_from_slice(b"0000"); + let response = write_routes + .oneshot( + axum::http::Request::builder() + .method("POST") + .uri(format!("/{}/git-upload-archive", did.as_str())) + .header( + header::CONTENT_TYPE, + "application/x-git-upload-archive-request", + ) + .body(Body::from(framed)) + .unwrap(), + ) + .await + .unwrap(); + let body = http_body_util::BodyExt::collect(response.into_body()) + .await + .unwrap() + .to_bytes(); + let text = String::from_utf8_lossy(&body); + assert!( + text.contains("NACK") && text.contains("archive exceeds the 512 byte limit"), + "an archive past the state's limit must be declined, got {text:?}" + ); + assert!( + !contains(&body, b"README.md"), + "the declined archive mustn't leak the tree it refused to serve" + ); +} + #[tokio::test(flavor = "multi_thread")] async fn push_over_http_is_refused() { let scan = tempfile::tempdir().unwrap(); diff --git a/knot2/crates/knot-pack/tests/h3_conformance.rs b/knot2/crates/knot-pack/tests/h3_conformance.rs index 719dfd4b..32396dbd 100644 --- a/knot2/crates/knot-pack/tests/h3_conformance.rs +++ b/knot2/crates/knot-pack/tests/h3_conformance.rs @@ -13,7 +13,7 @@ use knot_edge::{ RequiresFullHandshake, StaticCertPaths, TlsSetup, WriteRequestTimeout, }; use knot_git::Layout; -use knot_pack::{CacheConfig, RepoLookup, RepoResolver, RepoTarget}; +use knot_pack::{RepoLookup, RepoResolver, RepoTarget}; use knot_types::{ObjectFormat, RepoDid}; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; use rustls::crypto::aws_lc_rs; @@ -92,7 +92,7 @@ fn edge_config(addr: SocketAddr, cert: PathBuf, key: PathBuf) -> EdgeConfig { RequestTimeout::from_millis(nz64(120_000)), BodyInactivityTimeout::from_millis(nz64(120_000)), WriteRequestTimeout::from_millis(nz64(1_800_000)), - None, + knot_types::ProxyTrust::default(), ), tls: Some(TlsSetup { source: CertSource::Static(StaticCertPaths { @@ -164,17 +164,14 @@ async fn stand_up(layout: Layout, certdir: &Path) -> Edge { for _ in 0..8 { let addr: SocketAddr = format!("127.0.0.1:{}", free_port()).parse().unwrap(); let (cert, key, cert_der) = write_self_signed(certdir); - let (write_routes, advertisement) = knot_pack::edge_routes( - layout.clone(), - serve_dids(), - None, - None, - knot_resource::PackSlots::new(4), - CacheConfig::default(), - Arc::new(knot_messages::Catalog::defaults()), - knot_pack::default_hostname().clone(), - Arc::new(knot_runtime::SystemClock), - ); + let (write_routes, advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { + pack_slots: knot_resource::PackSlots::new(4), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + serve_dids(), + Arc::new(knot_runtime::SystemClock), + ) + }); let log: Arc>> = Arc::new(Mutex::new(Vec::new())); let sink = log.clone(); let recorded = write_routes.layer(axum::middleware::from_fn( diff --git a/knot2/crates/knot-pack/tests/handle_owner.rs b/knot2/crates/knot-pack/tests/handle_owner.rs index da47b68f..a9c8b4a2 100644 --- a/knot2/crates/knot-pack/tests/handle_owner.rs +++ b/knot2/crates/knot-pack/tests/handle_owner.rs @@ -6,7 +6,7 @@ use axum::body::Body; use axum::http::{Request, StatusCode}; use http_body_util::BodyExt; use knot_git::Layout; -use knot_pack::{CacheConfig, HandleResolver, RepoLookup, RepoResolver, RepoTarget}; +use knot_pack::{HandleResolver, RepoLookup, RepoResolver, RepoTarget}; use knot_types::{AccountDid, Handle, OwnerDid, RepoDid}; use tower::ServiceExt; @@ -38,17 +38,15 @@ fn repo_resolver() -> Arc { } fn build(layout: &Layout, handle_resolver: Option>) -> axum::Router { - let (_write, advertisement) = knot_pack::edge_routes( - layout.clone(), - repo_resolver(), - None, + let (_write, advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { handle_resolver, - knot_resource::PackSlots::new(4), - CacheConfig::default(), - Arc::new(knot_messages::Catalog::defaults()), - knot_pack::default_hostname().clone(), - Arc::new(knot_runtime::SystemClock), - ); + pack_slots: knot_resource::PackSlots::new(4), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + repo_resolver(), + Arc::new(knot_runtime::SystemClock), + ) + }); advertisement.into_router() } diff --git a/knot2/crates/knot-server/src/main.rs b/knot2/crates/knot-server/src/main.rs index e550aa42..b22f80e9 100644 --- a/knot2/crates/knot-server/src/main.rs +++ b/knot2/crates/knot-server/src/main.rs @@ -279,7 +279,7 @@ async fn main() -> anyhow::Result<()> { .context("server.listen_max_connections must be greater than zero")?, ); // A header name that doesn't parse will never match, - // `effective_peer` falls back to socket, + // `ProxyTrust::client_peer` falls back to socket, // and every request in the world shares // the proxy's address + its one ratelimit bucket. // So... better to refuse to start. @@ -290,6 +290,15 @@ async fn main() -> anyhow::Result<()> { .map(|header| axum::http::HeaderName::from_bytes(header.as_bytes())) .transpose() .context("xrpc.trusted_proxy_header isn't a valid HTTP header name")?; + let trusted_proxies = + knot_types::TrustedProxies::new(config.xrpc.trusted_proxies.iter().copied()); + let proxy_trust = knot_types::ProxyTrust::new(trusted_proxy_header, trusted_proxies); + if proxy_trust.trusts_any_peer() && !http_addr.ip().is_loopback() { + tracing::warn!( + bind = %http_addr, + "xrpc.trusted_proxy_header is set without xrpc.trusted_proxies while the HTTP surface takes connections from off-host, so a client that reaches this knot without passing the proxy can forge the header and pick its own rate-limit bucket. List the proxy's address in xrpc.trusted_proxies." + ); + } let edge_guards = knot_edge::EdgeGuards::new( knot_edge::RequestsPerSecond::new( NonZeroU32::new(config.server.listen_rate_limit_per_second) @@ -315,7 +324,7 @@ async fn main() -> anyhow::Result<()> { NonZeroU64::new(config.server.listen_write_request_timeout_ms) .context("server.listen_write_request_timeout_ms must be greater than zero")?, ), - trusted_proxy_header.clone(), + proxy_trust.clone(), ); let tls_setup = build_tls_setup(&config, &hostname).context("assemble TLS configuration")?; if config.tls.http3 && tls_setup.is_none() { @@ -325,7 +334,7 @@ async fn main() -> anyhow::Result<()> { } if tls_setup.is_none() && config.xrpc.trusted_proxy_header.is_none() { tracing::warn!( - "running plaintext behind a reverse proxy without xrpc.trusted_proxy_header. Per-IP rate limiting will key on the proxy socket address, throttling all clients as one. Set xrpc.trusted_proxy_header to the header your proxy appends." + "running plaintext behind a reverse proxy without xrpc.trusted_proxy_header. Per-IP rate limiting will key on the proxy socket address, throttling all clients as one. Set xrpc.trusted_proxy_header to the header your proxy appends, and xrpc.trusted_proxies to the address it connects from." ); } if config.tls.acme_enabled && http_addr.port() != 443 { @@ -502,20 +511,21 @@ async fn main() -> anyhow::Result<()> { let slots = knot_resource::Slots::for_machine(); - let ssh_base = knot_ssh::SshState::new( - layout.clone(), - Arc::clone(&index), - Arc::clone(&atproto), + let ssh_base = knot_ssh::SshState::new(knot_ssh::SshConfig { + layout: layout.clone(), + index: Arc::clone(&index), + atproto: Arc::clone(&atproto), knot_actor, - Arc::clone(&events), - hostname.clone(), - appview_endpoint.clone(), - admins.clone(), + events: Arc::clone(&events), + hostname: hostname.clone(), + appview: appview_endpoint.clone(), + admins: admins.clone(), admission, - byte_limits.pack, - budgets.languages_push, - ci_logs.clone(), - ) + max_pack_bytes: byte_limits.pack, + archive_limit: byte_limits.archive, + languages_push_budget: budgets.languages_push, + ci_logs: ci_logs.clone(), + }) .with_maintenance(maintenance_handle.clone()) .with_limits(pack_limits) .with_slots(slots.clone()) @@ -541,7 +551,7 @@ async fn main() -> anyhow::Result<()> { limiter: Arc::new(knot_xrpc::PreAuthLimiter::with_config(xrpc_limits)), cob_locks: Arc::new(knot_xrpc::CobLocks::default()), reservations, - trusted_proxy_header, + proxy_trust, committer, byte_limits, budgets, @@ -573,17 +583,16 @@ async fn main() -> anyhow::Result<()> { let handle_resolver: Arc = Arc::new(AtprotoHandleResolver { atproto: Arc::clone(&atproto), }); - let (write_routes, early_data_safe) = knot_pack::edge_routes( - layout, - resolver, - Some(receive_advertiser), - Some(handle_resolver), - slots.pack.clone(), - pack_cache_config, - Arc::clone(&catalog), - xrpc_state.knot_hostname.clone(), - Arc::new(SystemClock), - ); + let (write_routes, early_data_safe) = knot_pack::edge_routes(knot_pack::EdgeConfig { + receive: Some(receive_advertiser), + handle_resolver: Some(handle_resolver), + pack_slots: slots.pack.clone(), + cache: pack_cache_config, + catalog: Arc::clone(&catalog), + hostname: xrpc_state.knot_hostname.clone(), + archive_limit: byte_limits.archive, + ..knot_pack::EdgeConfig::serving(layout, resolver, Arc::new(SystemClock)) + }); let legacy_admin_routes = legacy_admin.map(|secret| { tracing::warn!( route = knot_xrpc::legacy_admin::ADD_MEMBER_ROUTE, diff --git a/knot2/crates/knot-sim/src/harness.rs b/knot2/crates/knot-sim/src/harness.rs index 46f36edb..bf727e9d 100644 --- a/knot2/crates/knot-sim/src/harness.rs +++ b/knot2/crates/knot-sim/src/harness.rs @@ -732,7 +732,7 @@ fn assemble_router(parts: StateParts) -> Router { PerActorQuota::new(256), GlobalQuota::new(256), )), - trusted_proxy_header: None, + proxy_trust: knot_types::ProxyTrust::default(), committer: Committer { name: AuthorName::new("knot"), email: Email::new("knot@nel.pet"), diff --git a/knot2/crates/knot-sim/tests/common/mod.rs b/knot2/crates/knot-sim/tests/common/mod.rs index f0019339..84dae1cf 100644 --- a/knot2/crates/knot-sim/tests/common/mod.rs +++ b/knot2/crates/knot-sim/tests/common/mod.rs @@ -73,7 +73,7 @@ pub fn edge_config(addr: SocketAddr, cert: PathBuf, key: PathBuf) -> EdgeConfig RequestTimeout::from_millis(nz64(120_000)), BodyInactivityTimeout::from_millis(nz64(120_000)), WriteRequestTimeout::from_millis(nz64(1_800_000)), - None, + knot_types::ProxyTrust::default(), ), tls: Some(TlsSetup { source: CertSource::Static(StaticCertPaths { diff --git a/knot2/crates/knot-sim/tests/h3.rs b/knot2/crates/knot-sim/tests/h3.rs index a6a2513b..7b9dad22 100644 --- a/knot2/crates/knot-sim/tests/h3.rs +++ b/knot2/crates/knot-sim/tests/h3.rs @@ -11,7 +11,7 @@ use common::Edge; use http::Method; use knot_edge::RequiresFullHandshake; use knot_git::Layout; -use knot_pack::{CacheConfig, RepoLookup, RepoResolver, RepoTarget}; +use knot_pack::{RepoLookup, RepoResolver, RepoTarget}; use knot_types::{ObjectFormat, RepoDid}; const PINNED_DATE: &str = "2026-06-20T12:00:00+00:00"; @@ -192,17 +192,14 @@ async fn cloned_set( let certdir = tempfile::tempdir().unwrap(); let clonedir = tempfile::tempdir().unwrap(); let edge = common::serve_edge(certdir.path(), || { - let (write_routes, advertisement) = knot_pack::edge_routes( - layout.clone(), - serve_dids(), - None, - None, - knot_resource::PackSlots::new(4), - CacheConfig::default(), - Arc::new(knot_messages::Catalog::defaults()), - knot_pack::default_hostname().clone(), - Arc::new(knot_runtime::SystemClock), - ); + let (write_routes, advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { + pack_slots: knot_resource::PackSlots::new(4), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + serve_dids(), + Arc::new(knot_runtime::SystemClock), + ) + }); (RequiresFullHandshake::new(write_routes), advertisement) }) .await; diff --git a/knot2/crates/knot-sim/tests/lfs_roundtrip.rs b/knot2/crates/knot-sim/tests/lfs_roundtrip.rs index 5774b2eb..ca93ba67 100644 --- a/knot2/crates/knot-sim/tests/lfs_roundtrip.rs +++ b/knot2/crates/knot-sim/tests/lfs_roundtrip.rs @@ -308,20 +308,21 @@ async fn spawn(published_line: String, with_h3: bool) -> World { ), )); let ssh_state = Arc::new( - knot_ssh::SshState::new( - layout.clone(), - Arc::clone(&index), - Arc::clone(&atproto), - knot_types::ActorId::from_secp256k1(actor_signer().public_key().as_bytes()), - Arc::clone(&events), - KnotHostname::new("nel.pet").unwrap(), - knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), - BTreeSet::from([AccountDid::new(OWNER_DID).unwrap()]), - AdmissionPolicy::Closed, - knot_xrpc::MaxWireBytes::new(1 << 30), - knot_xrpc::LanguagesPushBudget::new(Duration::from_secs(2)), - None, - ) + knot_ssh::SshState::new(knot_ssh::SshConfig { + layout: layout.clone(), + index: Arc::clone(&index), + atproto: Arc::clone(&atproto), + knot_actor: knot_types::ActorId::from_secp256k1(actor_signer().public_key().as_bytes()), + events: Arc::clone(&events), + hostname: KnotHostname::new("nel.pet").unwrap(), + appview: knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), + admins: BTreeSet::from([AccountDid::new(OWNER_DID).unwrap()]), + admission: AdmissionPolicy::Closed, + max_pack_bytes: knot_xrpc::MaxWireBytes::new(1 << 30), + archive_limit: knot_git::ArchiveLimit::default(), + languages_push_budget: knot_xrpc::LanguagesPushBudget::new(Duration::from_secs(2)), + ci_logs: None, + }) .with_lfs(lfs.clone(), 16), ); let ssh_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -356,7 +357,7 @@ async fn spawn(published_line: String, with_h3: bool) -> World { knot_xrpc::PerActorQuota::new(16), knot_xrpc::GlobalQuota::new(16), )), - trusted_proxy_header: None, + proxy_trust: knot_types::ProxyTrust::default(), committer: knot_xrpc::Committer { name: AuthorName::new("Tangled"), email: Email::new("noreply@tangled.sh"), @@ -405,17 +406,15 @@ async fn spawn(published_line: String, with_h3: bool) -> World { }) }; let advertiser = knot_xrpc::receive_advertiser(Arc::clone(&xrpc_state)); - let (write_routes, advertisement) = knot_pack::edge_routes( - layout.clone(), - Arc::clone(&resolver), - Some(Arc::clone(&advertiser)), - None, - knot_resource::PackSlots::new(4), - knot_pack::CacheConfig::default(), - Arc::new(knot_messages::Catalog::defaults()), - knot_pack::default_hostname().clone(), - Arc::new(knot_runtime::SystemClock), - ); + let (write_routes, advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { + receive: Some(Arc::clone(&advertiser)), + pack_slots: knot_resource::PackSlots::new(4), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + Arc::clone(&resolver), + Arc::new(knot_runtime::SystemClock), + ) + }); let router = write_routes .merge(advertisement.into_router()) .merge(knot_xrpc::router(Arc::clone(&xrpc_state))); @@ -428,17 +427,15 @@ async fn spawn(published_line: String, with_h3: bool) -> World { true => { let certdir = tempfile::tempdir().unwrap(); let edge = common::serve_edge(certdir.path(), || { - let (write_routes, advertisement) = knot_pack::edge_routes( - layout.clone(), - Arc::clone(&resolver), - Some(Arc::clone(&advertiser)), - None, - knot_resource::PackSlots::new(4), - knot_pack::CacheConfig::default(), - Arc::new(knot_messages::Catalog::defaults()), - knot_pack::default_hostname().clone(), - Arc::new(knot_runtime::SystemClock), - ); + let (write_routes, advertisement) = knot_pack::edge_routes(knot_pack::EdgeConfig { + receive: Some(Arc::clone(&advertiser)), + pack_slots: knot_resource::PackSlots::new(4), + ..knot_pack::EdgeConfig::serving( + layout.clone(), + Arc::clone(&resolver), + Arc::new(knot_runtime::SystemClock), + ) + }); let app = RequiresFullHandshake::new( write_routes.merge(knot_xrpc::router(Arc::clone(&xrpc_state))), ); diff --git a/knot2/crates/knot-sim/tests/ssh.rs b/knot2/crates/knot-sim/tests/ssh.rs index be2ff918..389d32cb 100644 --- a/knot2/crates/knot-sim/tests/ssh.rs +++ b/knot2/crates/knot-sim/tests/ssh.rs @@ -183,20 +183,23 @@ async fn spawn(published_line: String) -> Server { knot_events::ReplayBytes::new(16 << 20).unwrap(), ), )); - let state = Arc::new(knot_ssh::SshState::new( - layout.clone(), + let state = Arc::new(knot_ssh::SshState::new(knot_ssh::SshConfig { + layout: layout.clone(), index, atproto, - actor_for_seed(1), - Arc::clone(&events), - knot_types::KnotHostname::new("knot.test").unwrap(), - knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), - std::collections::BTreeSet::new(), - knot_types::AdmissionPolicy::Closed, - knot_xrpc::MaxWireBytes::new(1 << 30), - knot_xrpc::LanguagesPushBudget::new(std::time::Duration::from_secs(2)), - None, - )); + knot_actor: actor_for_seed(1), + events: Arc::clone(&events), + hostname: knot_types::KnotHostname::new("knot.test").unwrap(), + appview: knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), + admins: std::collections::BTreeSet::new(), + admission: knot_types::AdmissionPolicy::Closed, + max_pack_bytes: knot_xrpc::MaxWireBytes::new(1 << 30), + archive_limit: knot_git::ArchiveLimit::default(), + languages_push_budget: knot_xrpc::LanguagesPushBudget::new(std::time::Duration::from_secs( + 2, + )), + ci_logs: None, + })); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let port = listener.local_addr().unwrap().port(); tokio::spawn(async move { diff --git a/knot2/crates/knot-ssh/examples/ephemeral_knot.rs b/knot2/crates/knot-ssh/examples/ephemeral_knot.rs index 8888e77b..7424a5f1 100644 --- a/knot2/crates/knot-ssh/examples/ephemeral_knot.rs +++ b/knot2/crates/knot-ssh/examples/ephemeral_knot.rs @@ -205,20 +205,21 @@ async fn main() { .public_key() .as_bytes(), ); - let state = Arc::new(knot_ssh::SshState::new( + let state = Arc::new(knot_ssh::SshState::new(knot_ssh::SshConfig { layout, index, atproto, - actor, + knot_actor: actor, events, - KnotHostname::new("knot.test").unwrap(), - knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), - BTreeSet::new(), - AdmissionPolicy::Closed, - knot_pack::MaxWireBytes::new(1 << 34), - knot_postreceive::LanguagesPushBudget::new(Duration::from_secs(2)), - None, - )); + hostname: KnotHostname::new("knot.test").unwrap(), + appview: knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), + admins: BTreeSet::new(), + admission: AdmissionPolicy::Closed, + max_pack_bytes: knot_pack::MaxWireBytes::new(1 << 34), + archive_limit: knot_git::ArchiveLimit::default(), + languages_push_budget: knot_postreceive::LanguagesPushBudget::new(Duration::from_secs(2)), + ci_logs: None, + })); let listener = TcpListener::bind(("127.0.0.1", port)).await.unwrap(); let bound = listener.local_addr().unwrap().port(); diff --git a/knot2/crates/knot-ssh/src/exec.rs b/knot2/crates/knot-ssh/src/exec.rs index 09b59168..baf0e1c0 100644 --- a/knot2/crates/knot-ssh/src/exec.rs +++ b/knot2/crates/knot-ssh/src/exec.rs @@ -422,6 +422,7 @@ async fn serve_upload_archive( let (tx, mut rx) = mpsc::channel::>(16); let layout = state.layout.clone(); let did = repo_did.clone(); + let archive_limit = state.archive_limit; let handle = tokio::task::spawn_blocking(move || -> Result<(), PackError> { let _permit = permit; let repo = layout.open(&did)?; @@ -429,7 +430,7 @@ async fn serve_upload_archive( tx.blocking_send(chunk.to_vec()) .map_err(|_| std::io::Error::other("client disconnected")) }; - knot_pack::upload_archive_streamed(&repo, &request, &mut sink) + knot_pack::upload_archive_streamed(&repo, &request, archive_limit, &mut sink) }); let mut writer = channel.make_writer(); diff --git a/knot2/crates/knot-ssh/src/lib.rs b/knot2/crates/knot-ssh/src/lib.rs index c6d49238..a23dce52 100644 --- a/knot2/crates/knot-ssh/src/lib.rs +++ b/knot2/crates/knot-ssh/src/lib.rs @@ -11,7 +11,7 @@ use std::time::Duration; use knot_atproto::Atproto; use knot_events::EventLog; -use knot_git::Layout; +use knot_git::{ArchiveLimit, Layout}; use knot_index::Index; use knot_maintenance::MaintenanceHandle; use knot_pack::{MaxWireBytes, PackLimits}; @@ -57,6 +57,7 @@ pub struct SshState { admission: AdmissionPolicy, limits: PackLimits, max_pack_bytes: MaxWireBytes, + archive_limit: ArchiveLimit, languages_push_budget: LanguagesPushBudget, ci_logs: Option, slots: Slots, @@ -74,22 +75,39 @@ pub(crate) struct LfsRuntime { pub(crate) peer_slots: Arc, } +pub struct SshConfig { + pub layout: Layout, + pub index: Arc, + pub atproto: Arc>, + pub knot_actor: ActorId, + pub events: Arc>, + pub hostname: KnotHostname, + pub appview: AppviewEndpoint, + pub admins: BTreeSet, + pub admission: AdmissionPolicy, + pub max_pack_bytes: MaxWireBytes, + pub archive_limit: ArchiveLimit, + pub languages_push_budget: LanguagesPushBudget, + pub ci_logs: Option, +} + impl SshState { - #[allow(clippy::too_many_arguments)] - pub fn new( - layout: Layout, - index: Arc, - atproto: Arc>, - knot_actor: ActorId, - events: Arc>, - hostname: KnotHostname, - appview: AppviewEndpoint, - admins: BTreeSet, - admission: AdmissionPolicy, - max_pack_bytes: MaxWireBytes, - languages_push_budget: LanguagesPushBudget, - ci_logs: Option, - ) -> Self { + pub fn new(config: SshConfig) -> Self { + let SshConfig { + layout, + index, + atproto, + knot_actor, + events, + hostname, + appview, + admins, + admission, + max_pack_bytes, + archive_limit, + languages_push_budget, + ci_logs, + } = config; Self { layout, index, @@ -102,6 +120,7 @@ impl SshState { admission, limits: PackLimits::default(), max_pack_bytes, + archive_limit, languages_push_budget, ci_logs, slots: Slots::for_machine(), diff --git a/knot2/crates/knot-ssh/tests/ssh_push.rs b/knot2/crates/knot-ssh/tests/ssh_push.rs index a7899473..6062be1c 100644 --- a/knot2/crates/knot-ssh/tests/ssh_push.rs +++ b/knot2/crates/knot-ssh/tests/ssh_push.rs @@ -7,7 +7,7 @@ use futures::stream::StreamExt; use knot_atproto::Atproto; use knot_cob::{CobHome, CobStore}; use knot_cobs::{CollaboratorsChange, Grant, MembersChange, Registration, RegistryChange}; -use knot_git::{Layout, Repo}; +use knot_git::{ArchiveLimit, Layout, Repo}; use knot_index::Index; use knot_pack::MaxWireBytes; use knot_postreceive::LanguagesPushBudget; @@ -205,13 +205,21 @@ async fn spawn_server_with( max_pack_bytes: MaxWireBytes, warm: bool, ) -> (Server, Arc) { - let (server, index, _, _) = spawn_server_core(published_line, max_pack_bytes, warm, None).await; + let (server, index, _, _) = spawn_server_core( + published_line, + max_pack_bytes, + ArchiveLimit::default(), + warm, + None, + ) + .await; (server, index) } async fn spawn_server_core( published_line: String, max_pack_bytes: MaxWireBytes, + archive_limit: ArchiveLimit, warm: bool, lfs: Option, ) -> ( @@ -291,20 +299,21 @@ async fn spawn_server_core( knot_events::ReplayBytes::new(16 << 20).unwrap(), ), )); - let base = knot_ssh::SshState::new( - layout.clone(), - Arc::clone(&index), + let base = knot_ssh::SshState::new(knot_ssh::SshConfig { + layout: layout.clone(), + index: Arc::clone(&index), atproto, - actor_for_seed(1), - Arc::clone(&events), - knot_types::KnotHostname::new("knot.test").unwrap(), - knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), - std::collections::BTreeSet::new(), - knot_types::AdmissionPolicy::Closed, + knot_actor: actor_for_seed(1), + events: Arc::clone(&events), + hostname: knot_types::KnotHostname::new("knot.test").unwrap(), + appview: knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), + admins: std::collections::BTreeSet::new(), + admission: knot_types::AdmissionPolicy::Closed, max_pack_bytes, - LanguagesPushBudget::new(std::time::Duration::from_secs(2)), - None, - ); + archive_limit, + languages_push_budget: LanguagesPushBudget::new(std::time::Duration::from_secs(2)), + ci_logs: None, + }); let state = Arc::new(match lfs { Some(handle) => base.with_lfs(handle, 2), None => base, @@ -470,9 +479,20 @@ struct Fixture { } async fn fixture() -> Fixture { + fixture_with_archive_limit(ArchiveLimit::default()).await +} + +async fn fixture_with_archive_limit(archive_limit: ArchiveLimit) -> Fixture { let scratch = tempfile::tempdir().unwrap(); let (key_path, public_line) = keygen(scratch.path(), "client"); - let (server, index) = spawn_server(public_line, MaxWireBytes::new(1 << 30)).await; + let (server, index, _, _) = spawn_server_core( + public_line, + MaxWireBytes::new(1 << 30), + archive_limit, + true, + None, + ) + .await; let url = format!( "ssh://git@127.0.0.1:{}/{OWNER_DID}/{REPO_NAME}", server.port @@ -1081,20 +1101,21 @@ async fn launch( knot_events::ReplayBytes::new(16 << 20).unwrap(), ), )); - let state = Arc::new(knot_ssh::SshState::new( + let state = Arc::new(knot_ssh::SshState::new(knot_ssh::SshConfig { layout, index, atproto, - actor_for_seed(77), + knot_actor: actor_for_seed(77), events, - knot_types::KnotHostname::new("knot.test").unwrap(), - knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), - std::collections::BTreeSet::new(), - knot_types::AdmissionPolicy::Closed, - MaxWireBytes::new(1 << 30), - LanguagesPushBudget::new(std::time::Duration::from_secs(2)), - None, - )); + hostname: knot_types::KnotHostname::new("knot.test").unwrap(), + appview: knot_types::AppviewEndpoint::new("https://tangled.test").unwrap(), + admins: std::collections::BTreeSet::new(), + admission: knot_types::AdmissionPolicy::Closed, + max_pack_bytes: MaxWireBytes::new(1 << 30), + archive_limit: ArchiveLimit::default(), + languages_push_budget: LanguagesPushBudget::new(std::time::Duration::from_secs(2)), + ci_logs: None, + })); let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let port = listener.local_addr().unwrap().port(); tokio::spawn(async move { @@ -1339,7 +1360,7 @@ async fn git_archive_remote_over_ssh_streams_a_tar_of_the_tree() { let fx = fixture().await; seed_work(&fx.work); let (ok, out) = push(&fx.work, &fx.url, &fx.key_path, &["main"]).await; - assert!(ok, "seeding push must land before archiving:\n{out}"); + assert!(ok, "the seeding push must succeed before archiving:\n{out}"); let out_tar = fx.scratch.path().join("archive.tar"); let (ok, out) = git_ssh( @@ -1365,6 +1386,35 @@ async fn git_archive_remote_over_ssh_streams_a_tar_of_the_tree() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn git_archive_remote_over_ssh_honors_the_configured_archive_limit() { + let fx = fixture_with_archive_limit(ArchiveLimit::new(512)).await; + seed_work(&fx.work); + let (ok, out) = push(&fx.work, &fx.url, &fx.key_path, &["main"]).await; + assert!(ok, "the seeding push must succeed before archiving:\n{out}"); + + let out_tar = fx.scratch.path().join("archive.tar"); + let (ok, out) = git_ssh( + &fx.work, + &fx.key_path, + &[ + "archive", + "--format=tar", + "--remote", + &fx.url, + "-o", + out_tar.to_str().unwrap(), + "HEAD", + ], + ) + .await; + assert!(!ok, "git archive --remote past the limit must fail:\n{out}"); + assert!( + out.contains("archive exceeds the 512 byte limit"), + "the refusal must reach the client over the ssh channel:\n{out}" + ); +} + fn pkt(payload: &[u8]) -> Vec { let mut framed = format!("{:04x}", payload.len() + 4).into_bytes(); framed.extend_from_slice(payload); @@ -1480,6 +1530,7 @@ async fn shutdown_drains_an_in_flight_lfs_transfer_before_exit() { let (server, _index, shutdown, serve_task) = spawn_server_core( public_line, MaxWireBytes::new(1 << 20), + ArchiveLimit::default(), true, Some(handle.clone()), ) diff --git a/knot2/crates/knot-types/src/lib.rs b/knot2/crates/knot-types/src/lib.rs index 1293857f..4a51440d 100644 --- a/knot2/crates/knot-types/src/lib.rs +++ b/knot2/crates/knot-types/src/lib.rs @@ -20,7 +20,7 @@ mod hex; pub use hex::{decode_hex, lowercase_hex}; mod net; -pub use net::forwarded_peer; +pub use net::{PeerKey, ProxyTrust, TrustedProxies}; pub use jacquard_common::CowStr; pub use jacquard_common::DefaultStr; diff --git a/knot2/crates/knot-types/src/net.rs b/knot2/crates/knot-types/src/net.rs index bba146c1..a081033b 100644 --- a/knot2/crates/knot-types/src/net.rs +++ b/knot2/crates/knot-types/src/net.rs @@ -1,9 +1,108 @@ +use std::collections::BTreeSet; use std::net::IpAddr; -use http::HeaderMap; -use http::header::AsHeaderName; +use http::{HeaderMap, HeaderName}; -pub fn forwarded_peer(headers: &HeaderMap, header: K) -> Option { +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TrustedProxies(BTreeSet); + +impl TrustedProxies { + pub fn new(addresses: impl IntoIterator) -> Self { + Self( + addresses + .into_iter() + .map(|peer| peer.to_canonical()) + .collect(), + ) + } + + pub fn trusts(&self, peer: Option) -> bool { + match peer { + _ if self.0.is_empty() => true, + Some(peer) => self.0.contains(&peer.to_canonical()), + None => false, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PeerKey { + Relayed(IpAddr), + Socket(IpAddr), + SocketWithIgnoredHeader(IpAddr), + Unidentified, +} + +impl PeerKey { + pub fn address(self) -> Option { + match self { + Self::Relayed(peer) | Self::Socket(peer) | Self::SocketWithIgnoredHeader(peer) => { + Some(peer) + } + Self::Unidentified => None, + } + } + + pub fn ignored_header(self) -> Option { + match self { + Self::SocketWithIgnoredHeader(peer) => Some(peer), + _ => None, + } + } +} + +#[derive(Debug, Clone, Default)] +pub struct ProxyTrust { + header: Option, + proxies: TrustedProxies, +} + +impl ProxyTrust { + pub fn new(header: Option, proxies: TrustedProxies) -> Self { + Self { header, proxies } + } + + pub fn trusts_any_peer(&self) -> bool { + self.header.is_some() && self.proxies.trusts(None) + } + + pub fn peer_key(&self, headers: &HeaderMap, socket: Option) -> PeerKey { + match (self.relayed_peer(headers, socket), socket) { + (Some(relayed), _) => PeerKey::Relayed(relayed.to_canonical()), + (None, None) => PeerKey::Unidentified, + (None, Some(socket)) => match self.ignores_header_from(headers, socket) { + true => PeerKey::SocketWithIgnoredHeader(socket.to_canonical()), + false => PeerKey::Socket(socket.to_canonical()), + }, + } + } + + pub fn client_peer(&self, headers: &HeaderMap, socket: Option) -> Option { + self.peer_key(headers, socket).address() + } + + pub fn client_peer_of(&self, headers: &HeaderMap, socket: IpAddr) -> IpAddr { + self.peer_key(headers, Some(socket)) + .address() + .unwrap_or(socket.to_canonical()) + } + + fn relayed_peer(&self, headers: &HeaderMap, socket: Option) -> Option { + self.header + .as_ref() + .filter(|_| self.proxies.trusts(socket)) + .and_then(|header| forwarded_peer(headers, header)) + } + + fn ignores_header_from(&self, headers: &HeaderMap, socket: IpAddr) -> bool { + self.header + .as_ref() + .is_some_and(|header| headers.contains_key(header)) + && !self.proxies.trusts(Some(socket)) + } +} + +fn forwarded_peer(headers: &HeaderMap, header: &HeaderName) -> Option { headers .get(header) .and_then(|value| value.to_str().ok()) @@ -16,21 +115,24 @@ pub fn forwarded_peer(headers: &HeaderMap, header: K) -> Option mod tests { use super::*; - use http::HeaderName; - fn headers(value: Option<&str>) -> HeaderMap { value .map(|value| { let mut map = HeaderMap::new(); - map.insert( - HeaderName::from_bytes(b"x-forwarded-for").unwrap(), - value.parse().unwrap(), - ); + map.insert(forwarded_for(), value.parse().unwrap()); map }) .unwrap_or_default() } + fn forwarded_for() -> HeaderName { + HeaderName::from_static("x-forwarded-for") + } + + fn ip(value: &str) -> IpAddr { + value.parse().unwrap() + } + #[test] fn forwarded_peer_takes_the_rightmost_parseable_entry() { [ @@ -42,10 +144,215 @@ mod tests { .iter() .for_each(|&(header, expected)| { assert_eq!( - forwarded_peer(&headers(header), "x-forwarded-for"), - expected.map(|ip| ip.parse::().unwrap()), + forwarded_peer(&headers(header), &forwarded_for()), + expected.map(ip), "{header:?}" ); }); } + + #[test] + fn an_empty_allowlist_trusts_every_peer() { + let anyone = TrustedProxies::default(); + assert!(anyone.trusts(Some(ip("203.0.113.7")))); + assert!(anyone.trusts(None)); + } + + #[test] + fn the_header_applies_only_to_a_peer_on_the_allowlist() { + let proxy = ip("127.0.0.1"); + let forged = headers(Some("198.51.100.4")); + let trust = ProxyTrust::new(Some(forwarded_for()), TrustedProxies::new([proxy])); + let peer = |socket| trust.client_peer(&forged, Some(socket)); + + assert_eq!( + peer(proxy), + Some(ip("198.51.100.4")), + "a request relayed by the listed proxy is limited by the address the proxy recorded" + ); + assert_eq!( + peer(ip("203.0.113.7")), + Some(ip("203.0.113.7")), + "a client reaching the knot directly forged the header and must answer for its socket" + ); + } + + #[test] + fn a_caller_with_a_socket_address_gets_the_same_answer_without_an_option() { + let listed = TrustedProxies::new([ip("127.0.0.1")]); + [ + (ProxyTrust::default(), Some("198.51.100.4")), + ( + ProxyTrust::new(Some(forwarded_for()), listed), + Some("198.51.100.4"), + ), + ( + ProxyTrust::new(Some(forwarded_for()), TrustedProxies::default()), + None, + ), + ] + .into_iter() + .for_each(|(trust, value)| { + let headers = headers(value); + [ip("127.0.0.1"), ip("203.0.113.7"), ip("::ffff:203.0.113.7")] + .into_iter() + .for_each(|socket| { + assert_eq!( + Some(trust.client_peer_of(&headers, socket)), + trust.client_peer(&headers, Some(socket)), + "{trust:?} disagreed with itself for {socket} and header {value:?}" + ); + }); + }); + } + + #[test] + fn a_listed_ipv4_proxy_still_matches_the_v4_mapped_address_a_dual_stack_listener_reports() { + let mapped = ip("::ffff:127.0.0.1"); + assert!( + TrustedProxies::new([ip("127.0.0.1")]).trusts(Some(mapped)), + "binding [::] turns an IPv4 proxy into ::ffff:127.0.0.1 and the allowlist must still match it" + ); + assert!( + TrustedProxies::new([mapped]).trusts(Some(ip("127.0.0.1"))), + "an operator who writes the mapped form must match a plain IPv4 peer too" + ); + assert!( + !TrustedProxies::new([ip("127.0.0.1")]).trusts(Some(ip("::1"))), + "the IPv6 loopback is a different address from the IPv4 one" + ); + } + + #[test] + fn client_peer_falls_back_to_the_socket_whenever_no_header_applies() { + let socket = ip("203.0.113.7"); + [ + (None, Some("198.51.100.4")), + (Some(forwarded_for()), None), + (Some(forwarded_for()), Some("not-an-ip")), + ] + .into_iter() + .for_each(|(header_name, header_value)| { + let trust = ProxyTrust::new(header_name.clone(), TrustedProxies::default()); + assert_eq!( + trust.client_peer(&headers(header_value), Some(socket)), + Some(socket), + "{header_name:?} with {header_value:?}" + ); + }); + } + + #[test] + fn one_address_gets_one_bucket_however_the_listener_spelled_it() { + let trust = ProxyTrust::default(); + assert_eq!( + trust.client_peer(&headers(None), Some(ip("::ffff:203.0.113.7"))), + trust.client_peer(&headers(None), Some(ip("203.0.113.7"))), + "a v4-mapped socket and the plain v4 address are one client, so they share a key" + ); + } + + #[test] + fn client_peer_reports_no_peer_when_an_allowlist_leaves_it_with_neither_source() { + let trust = ProxyTrust::new( + Some(forwarded_for()), + TrustedProxies::new([ip("127.0.0.1")]), + ); + assert_eq!( + trust.client_peer(&headers(Some("198.51.100.4")), None), + None, + "with no socket to check against the allowlist there is no client to key on" + ); + } + + #[test] + fn the_peer_key_separates_an_ignored_header_from_a_request_that_never_sent_one() { + let listed = ProxyTrust::new( + Some(forwarded_for()), + TrustedProxies::new([ip("127.0.0.1")]), + ); + assert_eq!( + listed.peer_key(&headers(Some("198.51.100.4")), Some(ip("203.0.113.7"))), + PeerKey::SocketWithIgnoredHeader(ip("203.0.113.7")), + "an unlisted peer sent the header, which is the address an operator has to see" + ); + assert_eq!( + listed.peer_key(&headers(None), Some(ip("203.0.113.7"))), + PeerKey::Socket(ip("203.0.113.7")), + "a request without the header says nothing about the allowlist" + ); + assert_eq!( + listed.peer_key(&headers(Some("198.51.100.4")), Some(ip("127.0.0.1"))), + PeerKey::Relayed(ip("198.51.100.4")), + "the listed proxy relayed this one" + ); + assert_eq!( + listed.peer_key(&headers(Some("198.51.100.4")), None), + PeerKey::Unidentified + ); + } + + #[test] + fn only_an_ignored_header_reports_an_address_to_warn_about() { + assert_eq!( + PeerKey::SocketWithIgnoredHeader(ip("203.0.113.7")).ignored_header(), + Some(ip("203.0.113.7")) + ); + [ + PeerKey::Relayed(ip("198.51.100.4")), + PeerKey::Socket(ip("203.0.113.7")), + PeerKey::Unidentified, + ] + .into_iter() + .for_each(|key| { + assert_eq!( + key.ignored_header(), + None, + "{key:?} is not a misconfigured allowlist" + ); + }); + } + + #[test] + fn an_ignored_header_still_keys_the_peer_on_its_socket() { + let listed = ProxyTrust::new( + Some(forwarded_for()), + TrustedProxies::new([ip("127.0.0.1")]), + ); + let forged = headers(Some("198.51.100.4")); + assert_eq!( + listed.client_peer(&forged, Some(ip("203.0.113.7"))), + Some(ip("203.0.113.7")) + ); + assert_eq!( + listed.client_peer_of(&forged, ip("::ffff:203.0.113.7")), + ip("203.0.113.7"), + "the reported address stays canonical so the warning and the bucket agree" + ); + } + + #[test] + fn a_populated_allowlist_trusts_only_the_addresses_it_lists() { + let proxies = TrustedProxies::new([ip("127.0.0.1"), ip("::1")]); + assert!(proxies.trusts(Some(ip("127.0.0.1")))); + assert!(proxies.trusts(Some(ip("::1")))); + assert!( + !proxies.trusts(Some(ip("203.0.113.7"))), + "a client reaching the knot directly would pick its own rate-limit bucket" + ); + assert!( + !proxies.trusts(None), + "a peer of None has no address to match against the list" + ); + } + + #[test] + fn only_a_header_without_an_allowlist_trusts_any_peer() { + let listed = TrustedProxies::new([ip("127.0.0.1")]); + assert!( + ProxyTrust::new(Some(forwarded_for()), TrustedProxies::default()).trusts_any_peer() + ); + assert!(!ProxyTrust::new(Some(forwarded_for()), listed).trusts_any_peer()); + assert!(!ProxyTrust::default().trusts_any_peer()); + } } diff --git a/knot2/crates/knot-xrpc/src/error.rs b/knot2/crates/knot-xrpc/src/error.rs index 4060a1a0..0652aa49 100644 --- a/knot2/crates/knot-xrpc/src/error.rs +++ b/knot2/crates/knot-xrpc/src/error.rs @@ -120,6 +120,7 @@ impl From for XrpcError { GitError::AtomicRefs(_) => Self::conflict(message), GitError::UnsafeRepoDid(_) | GitError::ReservedDid(_) => Self::invalid_request(message), GitError::DepthExceeded(_) => Self::invalid_request(message), + GitError::ArchiveTooLarge { .. } => Self::request_too_large(message), GitError::Selection(_) => Self::overloaded(message), // Every oid passed to the object database here came from a ref this // knot already resolved or a tree it already read, so a miss means diff --git a/knot2/crates/knot-xrpc/src/events.rs b/knot2/crates/knot-xrpc/src/events.rs index ea8f6e06..007ce325 100644 --- a/knot2/crates/knot-xrpc/src/events.rs +++ b/knot2/crates/knot-xrpc/src/events.rs @@ -39,11 +39,7 @@ pub(crate) async fn events( Query(query): Query, upgrade: WebSocketUpgrade, ) -> Response { - let peer = state - .trusted_proxy_header - .as_ref() - .and_then(|header| knot_types::forwarded_peer(&headers, header)) - .unwrap_or_else(|| socket_peer.ip()); + let peer = state.proxy_trust.client_peer_of(&headers, socket_peer.ip()); let Some(permit) = state.subscriber_gate.try_admit(peer) else { return XrpcError::overloaded( "knot is serving its maximum number of event subscribers, retry shortly", diff --git a/knot2/crates/knot-xrpc/src/lib.rs b/knot2/crates/knot-xrpc/src/lib.rs index 7a4271f3..65186dc4 100644 --- a/knot2/crates/knot-xrpc/src/lib.rs +++ b/knot2/crates/knot-xrpc/src/lib.rs @@ -57,6 +57,7 @@ use serde_json::json; use knot_atproto::{Atproto, AtprotoError, ServiceJwt}; use knot_events::{EventLog, SubscriberGate}; +pub use knot_git::ArchiveLimit; use knot_git::Layout; use knot_index::{Index, Resolved}; use knot_maintenance::MaintenanceHandle; @@ -97,7 +98,6 @@ knot_types::scalar_newtype! { pub struct PatchLimit(usize); pub struct PatchDecompressedLimit(u64); pub struct ResponseLimit(usize); - pub struct ArchiveLimit(u64); pub struct ForkPackLimit(u64); pub struct TreeReadBudget(ReadBudget); pub struct BlobReadBudget(ReadBudget); @@ -122,7 +122,7 @@ impl Default for ByteLimits { patch: PatchLimit::new(16 * 1024 * 1024), patch_decompressed: PatchDecompressedLimit::new(128 * 1024 * 1024), response: ResponseLimit::new(5 * 1024 * 1024), - archive: ArchiveLimit::new(1024 * 1024 * 1024), + archive: ArchiveLimit::default(), fork_pack: ForkPackLimit::new(1024 * 1024 * 1024), pack: MaxWireBytes::new(8 * 1024 * 1024 * 1024), } @@ -164,7 +164,7 @@ pub struct XrpcState { pub limiter: Arc, pub cob_locks: Arc, pub reservations: Arc, - pub trusted_proxy_header: Option, + pub proxy_trust: knot_types::ProxyTrust, pub committer: Committer, pub byte_limits: ByteLimits, pub budgets: Budgets, @@ -336,7 +336,9 @@ pub(crate) async fn enforce_pre_auth_limit( request: Request, next: Next, ) -> Response { - let peer = effective_peer(&state, socket, request.headers()); + let peer = state + .proxy_trust + .client_peer(request.headers(), socket.ip()); match admit_pre_auth(&state, peer) { Ok(guard) => { let response = next.run(request).await; @@ -347,18 +349,6 @@ pub(crate) async fn enforce_pre_auth_limit( } } -pub(crate) fn effective_peer( - state: &XrpcState, - socket: SocketPeer, - headers: &HeaderMap, -) -> Option { - state - .trusted_proxy_header - .as_ref() - .and_then(|header| knot_types::forwarded_peer(headers, header)) - .or(socket.ip()) -} - pub(crate) fn admit_pre_auth( state: &XrpcState, peer: Option, @@ -501,7 +491,7 @@ pub(crate) async fn authenticate_and_authorize_push( repo: &RepoDid, denied: &str, ) -> Result { - let peer = effective_peer(state, socket, headers); + let peer = state.proxy_trust.client_peer(headers, socket.ip()); let guard = admit_pre_auth(state, peer)?; let actor = state.authenticate_push(headers).await?; guard.refund(); diff --git a/knot2/crates/knot-xrpc/src/reads.rs b/knot2/crates/knot-xrpc/src/reads.rs index a0200ec7..9f8c75fc 100644 --- a/knot2/crates/knot-xrpc/src/reads.rs +++ b/knot2/crates/knot-xrpc/src/reads.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::io::{Seek, SeekFrom}; use std::sync::Arc; use axum::body::Body; @@ -56,7 +55,6 @@ const LIST_REPOS_DEFAULT: usize = 50; const LIST_REPOS_MAX: usize = 1000; const MAX_BLOB_BYTES: u64 = 25 * 1024 * 1024; const MAX_COMPARE_COMMITS: usize = 500; -const ARCHIVE_CAP_MESSAGE: &str = "archive exceeds configured maximum size"; const RAW_CSP: &str = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; pub(crate) fn repo_not_found() -> XrpcError { @@ -1213,40 +1211,6 @@ fn content_disposition(filename: &str) -> String { } } -struct BoundedSpool { - file: std::fs::File, - position: u64, - limit: u64, - tripped: bool, -} - -impl std::io::Write for BoundedSpool { - fn write(&mut self, data: &[u8]) -> std::io::Result { - if self.position.saturating_add(data.len() as u64) > self.limit { - self.tripped = true; - return Err(std::io::Error::new( - std::io::ErrorKind::WriteZero, - ARCHIVE_CAP_MESSAGE, - )); - } - let written = self.file.write(data)?; - self.position += written as u64; - Ok(written) - } - - fn flush(&mut self) -> std::io::Result<()> { - self.file.flush() - } -} - -impl Seek for BoundedSpool { - fn seek(&mut self, pos: SeekFrom) -> std::io::Result { - let position = self.file.seek(pos)?; - self.position = position; - Ok(position) - } -} - fn archive_etag(did: &RepoDid, commit: Oid, format: ArchiveFormat, prefix: &str) -> String { let mut hasher = Sha256::new(); hasher.update(did.as_str().as_bytes()); @@ -1333,7 +1297,7 @@ pub(crate) async fn repo_archive( let temp = run_blocking({ let layout = state.layout.clone(); let did = did.clone(); - let archive_limit = state.byte_limits.archive.get(); + let archive_limit = state.byte_limits.archive; let tree_prefix = knot_git::ArchivePrefix::new(format!("{archive_prefix}/")) .expect("validated prefix with trailing slash stays valid"); move || { @@ -1341,24 +1305,26 @@ pub(crate) async fn repo_archive( let tree = repo.peel_to_tree(resolved)?; let temp = tempfile::NamedTempFile::new() .map_err(|error| XrpcError::internal(format!("cannot spool archive: {error}")))?; - let file = temp + let mut file = temp .reopen() .map_err(|error| XrpcError::internal(format!("cannot spool archive: {error}")))?; - let mut spool = BoundedSpool { - file, - position: 0, - limit: archive_limit, - tripped: false, - }; - repo.write_archive(tree, format.format(), Some(&tree_prefix), &mut spool) - .map_err(|error| match spool.tripped { - true => XrpcError::request_too_large(ARCHIVE_CAP_MESSAGE), + repo.write_archive( + tree, + format.format(), + Some(&tree_prefix), + archive_limit, + &mut file, + ) + .map_err(|error| { + match matches!(error, knot_git::GitError::ArchiveTooLarge { .. }) { + true => XrpcError::from(error), false => XrpcError::named( StatusCode::BAD_REQUEST, "ArchiveError", format!("failed to create archive: {error}"), ), - })?; + } + })?; temp.as_file() .set_modified(pinned_modified(modified_secs)) .map_err(|error| XrpcError::internal(error.to_string()))?; diff --git a/knot2/crates/knot-xrpc/src/tests.rs b/knot2/crates/knot-xrpc/src/tests.rs index 4de708a1..c1aa7134 100644 --- a/knot2/crates/knot-xrpc/src/tests.rs +++ b/knot2/crates/knot-xrpc/src/tests.rs @@ -331,7 +331,7 @@ fn state_from( limiter: Arc::new(crate::PreAuthLimiter::default()), cob_locks: Arc::new(crate::CobLocks::default()), reservations, - trusted_proxy_header: None, + proxy_trust: knot_types::ProxyTrust::default(), committer: crate::Committer { name: AuthorName::new("Tangled"), email: Email::new("noreply@tangled.sh"), diff --git a/knot2/crates/knot-xrpc/tests/common/mod.rs b/knot2/crates/knot-xrpc/tests/common/mod.rs index e3de7dd3..b9b21a03 100644 --- a/knot2/crates/knot-xrpc/tests/common/mod.rs +++ b/knot2/crates/knot-xrpc/tests/common/mod.rs @@ -184,7 +184,7 @@ impl World { PerActorQuota::new(16), GlobalQuota::new(16), )), - trusted_proxy_header: None, + proxy_trust: knot_types::ProxyTrust::default(), committer: knot_xrpc::Committer { name: AuthorName::new("Tangled"), email: Email::new("noreply@tangled.sh"), diff --git a/knot2/example.toml b/knot2/example.toml index dcaffc46..ab2088d1 100644 --- a/knot2/example.toml +++ b/knot2/example.toml @@ -175,7 +175,14 @@ # Default value: 5242880 #max_response_bytes = 5242880 +# Upper bound on bytes that a single archive spools, +# across all our surfaces: the sh.tangled.repo.archive query, +# `git archive --remote` over SSH, +# and the smart HTTP archive route. +# The knot will refuse writing smth that would blast an archive past this bound. +# # Can also be specified via environment variable `KNOT_XRPC_MAX_ARCHIVE_BYTES`. +# # Default value: 1073741824 #max_archive_bytes = 1073741824 @@ -266,6 +273,19 @@ # Can also be specified via environment variable `KNOT_XRPC_TRUSTED_PROXY_HEADER`. #trusted_proxy_header = +# IP addresses whose `trusted_proxy_header` the knot honors, +# without a port, +# for ex the loopback address of a reverse proxy on the same host. +# The knot rate-limits a request from any other address +# by its own socket address and ignores the header. +# Leave empty to honor the header from every peer, +# which is safe *only* if nothing but the proxy can reach this knot. +# +# Can also be specified via environment variable `KNOT_XRPC_TRUSTED_PROXIES`. +# +# Default value: [] +#trusted_proxies = [] + # Can also be specified via environment variable `KNOT_XRPC_EVENTS_REPLAY_BUFFER`. # Default value: 4096 #events_replay_buffer = 4096