From a20062d0f703e464d6df74b109ae4caf4a8a03ef Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 23 Oct 2026 13:27:40 +0000 Subject: [PATCH] Fetches refuse internal addresses unless fetch_internal lists them (#320) ipx's clients had no destination policy and followed redirects anywhere. Adding a feed returned the status or first line of whatever answered at the address; an enclosure declared as audio was downloaded whole from any URL and served back through /media; /api/art fetched any image a feed named. Feeds come from publishers and anyone signed in, so loopback, the Docker network and the LAN were all reachable. ipx-feed's guard is a connector layer: after the name resolves and the connection opens, before the request is written, it looks at the peer's address and refuses anything off the public internet unless config.toml's [general] fetch_internal lists it. Being on the connection, it covers every request on ctx.client and feed_client, each redirect hop, IP literals (which never reach a DNS resolver) and a name that resolves differently the second time. reqwest's rustls connection reports the peer address; a test run by hand checks HTTPS gets through, since were it lost every https fetch would be refused. The directory is on the server's own network by design (http://ipx-directory:8100), so it moves to local_client, unguarded, set by an admin. The browser suite and load tests serve their feeds on loopback and list 127.0.0.1. Not covered: librqbit's own connections (trackers, peers), which do not go through these clients. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 3 + Cargo.toml | 2 + crates/feed/Cargo.toml | 3 + crates/feed/src/guard.rs | 269 ++++++++++++++++++++++++++++++++++++ crates/feed/src/lib.rs | 2 + crates/ipx/CHANGELOG.md | 1 + crates/ipx/src/config.rs | 5 + crates/ipx/src/directory.rs | 2 +- crates/ipx/src/main.rs | 14 +- docs/configuration.md | 9 +- tests/load/run.js | 2 + tests/ui/global-setup.js | 2 + 12 files changed, 311 insertions(+), 3 deletions(-) create mode 100644 crates/feed/src/guard.rs diff --git a/Cargo.lock b/Cargo.lock index 510370f..fa5075a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1930,6 +1930,8 @@ dependencies = [ "anyhow", "atom_syndication", "chrono", + "http", + "hyper-util", "opml", "pulldown-cmark", "quick-xml 0.42.0", @@ -1937,6 +1939,7 @@ dependencies = [ "rss", "serde_json", "tokio", + "tower", "tracing", "url", ] diff --git a/Cargo.toml b/Cargo.toml index 7f89f16..9e1c50e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,8 @@ opentelemetry_sdk = { version = "0.33", default-features = false, features = ["t opml = "1.1.6" pulldown-cmark = { version = "0.13.4", default-features = false, features = ["html"] } mimalloc = "0.1.52" +http = "1.5.0" +hyper-util = { version = "0.1.20", features = ["client-legacy"] } percent-encoding = "2.3.2" quick-xml = { version = "0.42.0", features = ["escape-html"] } reqwest = { version = "0.13.5", default-features = false, features = ["rustls", "http2", "gzip", "brotli", "zstd", "stream", "json", "charset", "system-proxy"] } diff --git a/crates/feed/Cargo.toml b/crates/feed/Cargo.toml index 883307e..8114acb 100644 --- a/crates/feed/Cargo.toml +++ b/crates/feed/Cargo.toml @@ -8,12 +8,15 @@ edition.workspace = true anyhow.workspace = true atom_syndication.workspace = true chrono.workspace = true +http.workspace = true +hyper-util.workspace = true opml.workspace = true pulldown-cmark.workspace = true quick-xml.workspace = true reqwest.workspace = true rss.workspace = true serde_json.workspace = true +tower.workspace = true tracing.workspace = true url.workspace = true diff --git a/crates/feed/src/guard.rs b/crates/feed/src/guard.rs new file mode 100644 index 0000000..3fe53c3 --- /dev/null +++ b/crates/feed/src/guard.rs @@ -0,0 +1,269 @@ +//! Keeps fetches off internal addresses (#320). A feed, an episode, its artwork and a .torrent are +//! all at addresses a publisher or any signed-in person names, and redirects go anywhere, so +//! without this the server fetched from its own loopback, its Docker network and the LAN, and +//! handed back what answered. +//! +//! The check is on the connection, after the name is resolved and before a byte of the request +//! is sent, so it covers every request, every redirect, an address written as an IP, and a name +//! that resolves to an internal address only on the second lookup. + +use std::future::Future; +use std::net::{IpAddr, Ipv4Addr}; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; + +use anyhow::{Context as _, Result}; +use hyper_util::client::legacy::connect::{Connection, HttpInfo}; + +type BoxError = Box; + +/// Internal addresses and networks a fetch may reach all the same: config.toml's `fetch_internal`, +/// as `192.168.1.20` or `192.168.1.0/24`. Names are not taken: the check sees only the address. +#[derive(Clone, Debug, Default)] +pub struct Allowed(Arc>); + +impl Allowed { + pub fn parse(list: &[String]) -> Result { + list.iter().map(|s| parse_net(s)).collect::>>().map(|v| Self(Arc::new(v))) + } + + pub fn permits(&self, ip: IpAddr) -> bool { + let ip = canonical(ip); + !is_internal(ip) || self.0.iter().any(|&(net, bits)| contains(net, bits, ip)) + } +} + +fn parse_net(s: &str) -> Result<(IpAddr, u8)> { + let bad = || format!("fetch_internal: {s:?} is not an address or a network such as 192.168.1.0/24"); + let (addr, bits) = s.trim().split_once('/').map_or((s.trim(), None), |(a, b)| (a, Some(b))); + let ip = canonical(addr.parse::().with_context(bad)?); + let max = if ip.is_ipv4() { 32 } else { 128 }; + let bits = match bits { + Some(b) => b.parse::().ok().filter(|n| *n <= max).with_context(bad)?, + None => max, + }; + Ok((ip, bits)) +} + +/// An IPv4 address written as IPv6 (`::ffff:127.0.0.1`) is the IPv4 one. +fn canonical(ip: IpAddr) -> IpAddr { + match ip { + IpAddr::V6(v6) => v6.to_ipv4_mapped().map_or(ip, IpAddr::V4), + v4 => v4, + } +} + +fn contains(net: IpAddr, bits: u8, ip: IpAddr) -> bool { + match (net, ip) { + (IpAddr::V4(n), IpAddr::V4(i)) => { + let mask = u32::MAX.checked_shl(32 - u32::from(bits)).unwrap_or(0); + u32::from(n) & mask == u32::from(i) & mask + } + (IpAddr::V6(n), IpAddr::V6(i)) => { + let mask = u128::MAX.checked_shl(128 - u32::from(bits)).unwrap_or(0); + u128::from(n) & mask == u128::from(i) & mask + } + _ => false, + } +} + +/// Not on the public internet: loopback, private, link-local, shared (CGNAT), multicast and +/// reserved ranges, and their IPv6 counterparts. `IpAddr::is_global` would say this, but is +/// unstable. +// shortcut: 6to4 (2002::/16) and Teredo addresses that wrap an internal IPv4 one pass; check the +// embedded address if an IPv6 route to them ever exists here. +pub fn is_internal(ip: IpAddr) -> bool { + match canonical(ip) { + IpAddr::V4(a) => { + let [o0, o1, o2, _] = a.octets(); + a.is_private() + || a.is_loopback() + || a.is_link_local() + || a.is_unspecified() + || a.is_broadcast() + || a.is_multicast() + || o0 == 0 + || o0 >= 240 + || (o0 == 100 && o1 & 0xc0 == 64) + || (o0 == 192 && o1 == 0 && o2 == 0) + } + IpAddr::V6(a) => { + let s = a.segments(); + a.is_loopback() + || a.is_unspecified() + || a.is_multicast() + || s[0] & 0xfe00 == 0xfc00 + || s[0] & 0xffc0 == 0xfe80 + // NAT64 carries an IPv4 address in its last 32 bits. + || (s[0] == 0x64 && s[1] == 0xff9b && is_internal(IpAddr::V4(Ipv4Addr::from(u128::from(a) as u32)))) + } + } +} + +/// A connector layer for `reqwest::ClientBuilder::connector_layer`: a connection to an address +/// `Allowed` does not permit fails before the request is sent. +#[derive(Clone)] +pub struct Guard(Allowed); + +impl Guard { + pub fn new(allowed: Allowed) -> Self { + Self(allowed) + } +} + +impl tower::Layer for Guard { + type Service = Guarded; + + fn layer(&self, inner: S) -> Guarded { + Guarded { inner, allowed: self.0.clone() } + } +} + +#[derive(Clone)] +pub struct Guarded { + inner: S, + allowed: Allowed, +} + +impl tower::Service for Guarded +where + S: tower::Service, + S::Response: Connection, + S::Error: Into, + S::Future: Send + 'static, +{ + type Response = S::Response; + type Error = BoxError; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx).map_err(Into::into) + } + + fn call(&mut self, req: R) -> Self::Future { + let connecting = self.inner.call(req); + let allowed = self.allowed.clone(); + Box::pin(async move { + let conn = connecting.await.map_err(Into::into)?; + let mut extras = http::Extensions::new(); + conn.connected().get_extras(&mut extras); + // Refused when the peer is unknown too: a connection this cannot check is one it + // cannot vouch for. + match extras.get::().map(|i| i.remote_addr().ip()) { + Some(ip) if allowed.permits(ip) => Ok(conn), + Some(ip) => { + Err(format!("{ip} is an internal address, not fetched (fetch_internal in config.toml)").into()) + } + None => Err("the connection's address is unknown, so it is not fetched".into()), + } + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ip(s: &str) -> IpAddr { + s.parse().unwrap() + } + + #[test] + fn internal_addresses_are_known() { + for a in [ + "127.0.0.1", + "10.1.2.3", + "172.16.0.1", + "192.168.1.130", + "169.254.169.254", + "100.64.0.1", + "0.0.0.0", + "224.0.0.1", + "255.255.255.255", + "::1", + "::", + "fd00::1", + "fe80::1", + "::ffff:127.0.0.1", + "64:ff9b::a00:1", + ] { + assert!(is_internal(ip(a)), "{a}"); + } + for a in ["1.1.1.1", "8.8.8.8", "2606:4700:4700::1111", "64:ff9b::808:808", "100.128.0.1"] { + assert!(!is_internal(ip(a)), "{a}"); + } + } + + #[test] + fn the_allowlist_lets_through_what_it_names_and_no_more() { + let a = Allowed::parse(&["192.168.1.0/24".into(), "127.0.0.1".into(), "fd00::/8".into()]).unwrap(); + assert!(a.permits(ip("192.168.1.20"))); + assert!(a.permits(ip("127.0.0.1"))); + assert!(a.permits(ip("::ffff:127.0.0.1")), "the same address written as IPv6"); + assert!(a.permits(ip("fd12::1"))); + assert!(!a.permits(ip("192.168.2.1"))); + assert!(!a.permits(ip("127.0.0.2"))); + assert!(a.permits(ip("1.1.1.1")), "public addresses need no listing"); + assert!(Allowed::default().permits(ip("1.1.1.1"))); + assert!(!Allowed::default().permits(ip("10.0.0.1"))); + for bad in ["nas.lan", "10.0.0.0/33", "10.0.0.0/x", ""] { + assert!(Allowed::parse(&[bad.into()]).is_err(), "{bad}"); + } + } + + async fn answers() -> String { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + while let Ok((mut sock, _)) = listener.accept().await { + let _ = sock.read(&mut [0u8; 1024]).await; + let _ = sock.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 6\r\n\r\nsecret").await; + } + }); + format!("http://{addr}/") + } + + fn client(allowed: &[&str]) -> reqwest::Client { + let allowed = Allowed::parse(&allowed.iter().map(|s| s.to_string()).collect::>()).unwrap(); + reqwest::Client::builder().connector_layer(Guard::new(allowed)).build().unwrap() + } + + #[tokio::test] + async fn a_fetch_from_loopback_is_refused_unless_listed() { + let url = answers().await; + let err = client(&[]).get(&url).send().await.unwrap_err(); + assert!(format!("{err:?}").contains("internal address"), "{err:?}"); + let body = client(&["127.0.0.1"]).get(&url).send().await.unwrap().text().await.unwrap(); + assert_eq!(body, "secret"); + } + + #[tokio::test] + async fn a_redirect_to_loopback_is_refused() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let inner = answers().await; + let listener = tokio::net::TcpListener::bind("127.0.0.2:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + while let Ok((mut sock, _)) = listener.accept().await { + let _ = sock.read(&mut [0u8; 1024]).await; + let r = format!("HTTP/1.1 302 Found\r\nLocation: {inner}\r\nContent-Length: 0\r\n\r\n"); + let _ = sock.write_all(r.as_bytes()).await; + } + }); + // 127.0.0.2 stands for a public address: listed, it is let through, and the redirect it + // sends to 127.0.0.1 is not. + let err = client(&["127.0.0.2"]).get(format!("http://{addr}/")).send().await.unwrap_err(); + assert!(format!("{err:?}").contains("127.0.0.1 is an internal address"), "{err:?}"); + } + + /// Run by hand: `cargo test -p ipx-feed guard -- --ignored`. HTTPS gives the peer's address + /// too; were it lost under TLS, every https fetch would be refused. + #[tokio::test] + #[ignore = "reaches the internet"] + async fn https_is_checked_and_let_through() { + let r = client(&[]).get("https://example.com/").send().await.unwrap(); + assert!(r.status().is_success()); + } +} diff --git a/crates/feed/src/lib.rs b/crates/feed/src/lib.rs index 1e31343..1b6e80c 100644 --- a/crates/feed/src/lib.rs +++ b/crates/feed/src/lib.rs @@ -1,6 +1,8 @@ //! Feed fetching and parsing, shared by ipx and the directory (#202). Replaces //! FeedData.__getFeed / __getEntries. +pub mod guard; + use anyhow::{Context, Result, anyhow}; use reqwest::StatusCode; use reqwest::header::{ETAG, IF_MODIFIED_SINCE, IF_NONE_MATCH, LAST_MODIFIED}; diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index da1f3f2..160658b 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A downloaded file opens in the browser only when it is audio, video, an image other than SVG, a PDF or plain text, and is saved otherwise. An SVG or web page a podcast sent ran its script as iPX, signed in as whoever opened it (#319). - An OPML nested more than 100 levels deep is refused, uploaded or fetched. A deep enough one stopped iPX, and a subscribed one stopped it again each time it restarted (#322). - A `.torrent` file larger than 10 MB, or slower than the torrent stall limit, is refused. iPX read one into memory however large it was (#330). +- Feeds, episodes, artwork and `.torrent` files are no longer fetched from internal addresses (loopback, the LAN, Docker's networks), redirects included, unless config.toml's `fetch_internal` lists them. Adding a feed could read what a service inside the network answered (#320). ## [1.4.0] - 2026-10-19 diff --git a/crates/ipx/src/config.rs b/crates/ipx/src/config.rs index 08433ba..b0534d8 100644 --- a/crates/ipx/src/config.rs +++ b/crates/ipx/src/config.rs @@ -56,6 +56,10 @@ pub struct General { /// How far back a new Signal looks, in days (#276): each item is a call to TypeSafe, and a /// month of production's feeds was about 8,000. pub signal_days: u64, + /// Internal addresses and networks feeds, episodes and artwork may still be fetched from, as + /// `["192.168.1.20", "10.0.0.0/8"]`: everything off the public internet is refused otherwise, + /// since a feed or anyone signed in names what is fetched (#320). Addresses, not names. + pub fetch_internal: Vec, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] @@ -215,6 +219,7 @@ impl Default for General { directory_share: false, signals_on_server: false, signal_days: 7, + fetch_internal: vec![], } } } diff --git a/crates/ipx/src/directory.rs b/crates/ipx/src/directory.rs index b814716..0088da2 100644 --- a/crates/ipx/src/directory.rs +++ b/crates/ipx/src/directory.rs @@ -70,7 +70,7 @@ async fn call( ) -> Result { let (base, token) = target(&ctx.cfg()).ok_or(DirError::NotConfigured)?; let mut req = ctx - .client + .local_client .request(method, format!("{base}{path}")) .bearer_auth(token) .timeout(std::time::Duration::from_secs(20)); diff --git a/crates/ipx/src/main.rs b/crates/ipx/src/main.rs index 5bed27f..3491336 100644 --- a/crates/ipx/src/main.rs +++ b/crates/ipx/src/main.rs @@ -129,8 +129,12 @@ pub struct Ctx { /// across an await. pub cfg: std::sync::RwLock>, pub db: db::Db, + /// For anything a feed or a person names: refuses internal addresses (#320). pub client: reqwest::Client, - /// For feeds only: follows no redirects, so `feed::fetch` sees each hop and can tell a feed + /// For the services this server is set up to call, which are on its own network by design: + /// the directory, as `http://ipx-directory:8100`. Never for an address a feed or a person names. + pub local_client: reqwest::Client, + /// For feeds only, refusing internal addresses as `client` does: follows no redirects, so `feed::fetch` sees each hop and can tell a feed /// that moved for good from one sent elsewhere for now. pub feed_client: reqwest::Client, pub out: Emitter, @@ -353,6 +357,7 @@ async fn main() -> Result<()> { Command::Add { .. } | Command::Rm { .. } | Command::Import { .. } | Command::User { cmd: UserCmd::Rm { .. } } ); let (events, _) = broadcast::channel(1024); + let guard = feed::guard::Guard::new(feed::guard::Allowed::parse(&cfg.general.fetch_internal)?); let ctx = Arc::new(Ctx { cfg: std::sync::RwLock::new(std::sync::Arc::new(cfg)), db, @@ -363,12 +368,18 @@ async fn main() -> Result<()> { // Two minutes without a byte is a stalled download, however long it is: unbounded, one // held the worker, and every scan behind it, for good, while the daemon read healthy (#152). .read_timeout(std::time::Duration::from_secs(120)) + .connector_layer(guard.clone()) + .build()?, + local_client: reqwest::Client::builder() + .user_agent(concat!("ipx/", env!("CARGO_PKG_VERSION"))) + .connect_timeout(std::time::Duration::from_secs(10)) .build()?, feed_client: reqwest::Client::builder() .user_agent(concat!("ipx/", env!("CARGO_PKG_VERSION"))) .connect_timeout(std::time::Duration::from_secs(10)) .read_timeout(feed::FEED_SILENCE) .redirect(reqwest::redirect::Policy::none()) + .connector_layer(guard) .build()?, out: if is_daemon { Emitter::socket(events.clone(), false) } else { Emitter::terminal() }, torrents: tokio::sync::OnceCell::new(), @@ -2629,6 +2640,7 @@ mod tests { cfg: std::sync::RwLock::new(Arc::new(cfg)), db: db::Db::memory().await.unwrap(), client: reqwest::Client::new(), + local_client: reqwest::Client::new(), feed_client: reqwest::Client::builder().redirect(reqwest::redirect::Policy::none()).build().unwrap(), out: Emitter::terminal(), torrents: tokio::sync::OnceCell::new(), diff --git a/docs/configuration.md b/docs/configuration.md index e972a10..2178cc6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -46,10 +46,11 @@ max_age_days = 30 # 0 = keep forever max_new_per_check = 3 # per feed, per scan. 0 = unlimited media_types = ["audio", "video"] art_cache_mb = 500 # artwork kept on disk. 0 = none +fetch_internal = [] # internal addresses fetches may reach, e.g. ["192.168.1.0/24"] ``` `schedule`, `max_total_gb`, `max_age_days`, `max_new_per_check`, `media_types` and `art_cache_mb` move into the -database as described above; `download_dir`, `socket` and `organize` stay in config.toml. +database as described above; `download_dir`, `socket`, `organize` and `fetch_internal` stay in config.toml. * **`schedule`** — how often feeds are re-checked. A feed's own `` still wins when it asks to be polled *less* often, and a per-feed `schedule` overrides both. Without a schedule of its own, @@ -77,6 +78,12 @@ database as described above; `download_dir`, `socket` and `organize` stay in con * **`media_types`** — top-level MIME types taken automatically. Anything else is still listed and can be fetched by hand; blog feeds put each article's header image in an ``, and without this the disk fills with artwork. Empty takes everything. +* **`fetch_internal`** — internal addresses and networks that feeds, episodes, artwork and + `.torrent` files may still be fetched from, as `["192.168.1.20", "10.0.0.0/8"]`. Anything off + the public internet (loopback, the LAN, Docker's networks, link-local) is refused otherwise, + redirects included, because a feed's publisher or anyone signed in chooses what is fetched. List + addresses, not names: a feed on `nas.lan` needs `nas.lan`'s address here. The directory is not + affected. Empty by default; read at start. ## `[torrent]` diff --git a/tests/load/run.js b/tests/load/run.js index 1eba5ce..17908d9 100644 --- a/tests/load/run.js +++ b/tests/load/run.js @@ -134,6 +134,8 @@ download_dir = "${root}/downloads" socket = "${root}/ipx.sock" schedule = "every 60m" max_new_per_check = 1 +# The generated feeds are on loopback, which ipx refuses otherwise (#320). +fetch_internal = ["127.0.0.1"] [torrent] enabled = false diff --git a/tests/ui/global-setup.js b/tests/ui/global-setup.js index f18c147..0da3311 100644 --- a/tests/ui/global-setup.js +++ b/tests/ui/global-setup.js @@ -32,6 +32,8 @@ download_dir = "${path.join(root, 'downloads')}" socket = "${path.join(root, 'ipx.sock')}" schedule = "every 60m" max_new_per_check = 1 +# The fixture feeds are on loopback, which ipx refuses otherwise (#320). +fetch_internal = ["127.0.0.1"] directory_url = "${DIRECTORY}" directory_token = "${DIRECTORY_TOKEN}" directory_share = true