diff --git a/Cargo.lock b/Cargo.lock index c91d2f7..debffa7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1570,6 +1570,7 @@ dependencies = [ "tracing", "tracing-actix-web", "tracing-subscriber", + "url", "urlencoding", "utoipa", "utoipa-swagger-ui", diff --git a/Cargo.toml b/Cargo.toml index 55a294c..6e8f01c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ tracing-subscriber = { version = "0.3", features = [ "fmt", "json", ] } +url = "2.5.8" urlencoding = "2.1.3" utoipa = { version = "5.4.0", features = ["actix_extras", "chrono", "uuid"] } utoipa-swagger-ui = { version = "9.0.2", features = ["actix-web"] } diff --git a/src/config.rs b/src/config.rs index 76cf5df..be0d09d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,6 +8,7 @@ use tokio::sync::mpsc::Sender; use crate::{ endpoints::mods::IndexQueryParams, + pin_dns::PinDnsResolver, s3_worker::S3WorkerTask, storage::{LocalBackend, PrivateDisk, PublicDisk, S3Backend, S3Configuration}, types::{ @@ -35,6 +36,7 @@ pub struct AppData { mods_cache: Cache>>, http_client: reqwest::Client, + pin_dns_http_client: reqwest::Client, s3_sender: OnceLock>, } @@ -84,6 +86,13 @@ pub async fn build_config() -> anyhow::Result { None }; + let pin_dns_http_client = reqwest::Client::builder() + .dns_resolver(Arc::new(PinDnsResolver)) + .pool_max_idle_per_host(4) + .connect_timeout(Duration::from_secs(10)) + .read_timeout(Duration::from_secs(30)) + .build()?; + Ok(AppData { db: pool, app_url: app_url.clone(), @@ -114,6 +123,7 @@ pub async fn build_config() -> anyhow::Result { .connect_timeout(Duration::from_secs(10)) .read_timeout(Duration::from_secs(30)) .build()?, + pin_dns_http_client, s3_sender: OnceLock::new(), }) } @@ -193,6 +203,21 @@ impl AppData { &self.http_client } + /// Client that allows pinning DNS queries to a certain ip address. + /// Useful for preventing Server Side Request Forgery. + /// + /// To pin an address, you have to use pin_dns::PINNED_ADDR. + /// + /// Basically, if you have a URL as user input, *always* use this client. + /// + /// The downside is that you get worse DNS performance, doesn't matter when + /// security is involved though. + /// + /// For an example, check mod_zip::download() + pub fn pin_dns_http_client(&self) -> &reqwest::Client { + &self.pin_dns_http_client + } + pub fn init_s3_sender(&self, sender: Sender) { self.s3_sender .set(sender) diff --git a/src/endpoints/mod_versions.rs b/src/endpoints/mod_versions.rs index c4cde46..b459d67 100644 --- a/src/endpoints/mod_versions.rs +++ b/src/endpoints/mod_versions.rs @@ -353,7 +353,12 @@ pub async fn create_version( .filter(|c| c.is_ascii() && *c != '\0') .collect(); - let bytes = download_mod(data.http_client(), &download_link, data.max_download_mb()).await?; + let bytes = download_mod( + data.pin_dns_http_client(), + &download_link, + data.max_download_mb(), + ) + .await?; let json = ModJson::from_zip(&bytes, &download_link, make_accepted) .inspect_err(|e| tracing::error!("Failed to parse mod.json: {e}"))?; if json.id != the_mod.id { @@ -575,7 +580,7 @@ pub async fn update_version( } let bytes = mod_zip::download_mod_hash_comp( - data.http_client(), + data.pin_dns_http_client(), &version.download_link, &version.hash, data.max_download_mb(), diff --git a/src/endpoints/mods.rs b/src/endpoints/mods.rs index 2b53f41..696e8f1 100644 --- a/src/endpoints/mods.rs +++ b/src/endpoints/mods.rs @@ -218,7 +218,7 @@ pub async fn create( let dev = auth.developer()?; let mut pool = data.db().acquire().await?; let bytes = mod_zip::download_mod( - data.http_client(), + data.pin_dns_http_client(), &payload.download_link, data.max_download_mb(), ) diff --git a/src/main.rs b/src/main.rs index f6746f9..3c9316b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,6 +20,7 @@ mod jobs; mod logging; mod mod_zip; mod openapi; +mod pin_dns; mod s3_worker; mod storage; mod types; diff --git a/src/mod_zip.rs b/src/mod_zip.rs index 6a5b039..3989b82 100644 --- a/src/mod_zip.rs +++ b/src/mod_zip.rs @@ -1,15 +1,24 @@ +use std::cell::Cell; use std::io::Seek; use std::io::{BufReader, Cursor, Read}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, ToSocketAddrs}; use actix_web::web::Bytes; use image::codecs::png::PngDecoder; use image::codecs::png::PngEncoder; use image::{DynamicImage, GenericImageView}; use image::{ImageEncoder, ImageError}; +use url::Url; use zip::ZipArchive; use zip::read::ZipFile; use zip::result::ZipError; +use crate::pin_dns::PINNED_ADDR; + +const DOWNLOAD_DENYLIST_DOMAINS: [&str; 1] = ["localhost"]; +const DOWNLOAD_DENYLIST_TLDS: [&str; 4] = [".host", ".lan", ".local", ".internal"]; +const MAX_REDIRECTS: u8 = 10; + #[derive(thiserror::Error, Debug)] pub enum ModZipError { #[error("I/O error: {0}")] @@ -22,6 +31,12 @@ pub enum ModZipError { SerdeJsonError(#[from] serde_json::Error), #[error("Invalid mod logo: {0}")] InvalidLogo(String), + #[error("Download link is invalid")] + InvalidModFileUrl, + #[error("Too many redirects when downloading .geode file")] + TooManyRedirects, + #[error("Invalid Location header on download URL redirect")] + InvalidRedirect, #[error(".geode file hash mismatch: {0} doesn't match {1}")] ModFileHashMismatch(String, String), #[error("Failed to fetch .geode file: {0}")] @@ -156,37 +171,243 @@ async fn download( url: &str, limit_mb: u32, ) -> Result { - let limit_bytes: u64 = limit_mb as u64 * 1_000_000; - let mut response = http_client - .get(url) - .send() - .await - .inspect_err(|e| tracing::error!("Failed to fetch .geode file: {e}"))? - .error_for_status() - .inspect_err(|e| tracing::error!("Failed to fetch .geode file: {e}"))?; + let mut current_url = Url::parse(url).map_err(|_| ModZipError::InvalidModFileUrl)?; - // Check Content-Length, but the server can lie about this, so we'll also stream the file - // If the header is somehow unavailable, we'll just check the size when streaming - let content_length = response.content_length().unwrap_or(0); + tracing::debug!("fetching mod from {current_url}"); - if content_length > limit_bytes { - let len_mb = content_length / 1_000_000; - return Err(ModZipError::ModFileTooLarge(len_mb, limit_mb.into())); - } + let limit_bytes: u64 = limit_mb as u64 * 1_000_000; + + for i in 0..MAX_REDIRECTS { + tracing::debug!("starting hop {}", i + 1); + let addrs = validate_download_url(¤t_url)?; + let port = current_url.port_or_known_default().unwrap_or(443); + let addr = std::net::SocketAddr::new(addrs[0], port); + tracing::debug!("DNS validated as {addr}"); + + // Pin the validated ip address in our cool custom resolver + let response = PINNED_ADDR.scope(Cell::new(Some(addr)), async { + http_client.get(url) + .send() + .await + .inspect_err(|e| tracing::error!("Failed to fetch .geode file: {e}")) + }).await?; + + if response.status().is_redirection() { + let location = response + .headers() + .get(reqwest::header::LOCATION) + .ok_or(ModZipError::InvalidRedirect)? + .to_str() + .map_err(|_| ModZipError::InvalidRedirect)?; + current_url = current_url + .join(location) + .map_err(|_| ModZipError::InvalidRedirect)?; + continue; + } - let mut data: Vec = Vec::with_capacity(content_length as usize); + let mut response = response + .error_for_status() + .inspect_err(|e| tracing::error!("Failed to fetch .geode file: {e}"))?; - let mut streamed: u64 = 0; - while let Some(chunk) = response.chunk().await? { - streamed += chunk.len() as u64; + // Check Content-Length, but the server can lie about this, so we'll also stream the file + // If the header is somehow unavailable, we'll just check the size when streaming + let content_length = response.content_length().unwrap_or(0); - if streamed > limit_bytes { - let len_mb = streamed / 1_000_000; + if content_length > limit_bytes { + let len_mb = content_length / 1_000_000; return Err(ModZipError::ModFileTooLarge(len_mb, limit_mb.into())); } - data.extend_from_slice(&chunk); + let mut data: Vec = Vec::with_capacity(content_length as usize); + + let mut streamed: u64 = 0; + while let Some(chunk) = response.chunk().await? { + streamed += chunk.len() as u64; + + if streamed > limit_bytes { + let len_mb = streamed / 1_000_000; + return Err(ModZipError::ModFileTooLarge(len_mb, limit_mb.into())); + } + + data.extend_from_slice(&chunk); + } + + return Ok(Bytes::from(data)); + } + + Err(ModZipError::TooManyRedirects) +} + +/// Hopefully this gets rid of all nasty ips +fn is_disallowed_ip(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => { + v4.is_loopback() + || v4.is_private() + || v4.is_link_local() // covers 169.254.169.254 cloud metadata + || v4.is_unspecified() + || v4.is_broadcast() + || v4.is_documentation() + || is_shared_nat(v4) // 100.64.0.0/10 CGNAT + } + IpAddr::V6(v6) => { + v6.is_loopback() + || v6.is_unspecified() + || is_unique_local(v6) // fc00::/7 + || is_ipv6_link_local(v6) // fe80::/10 + || v6 + .to_ipv4_mapped() + .is_some_and(|v4| is_disallowed_ip(IpAddr::V4(v4))) + } + } +} + +/// Denies ipv4 like `100.64.0.0/10` +fn is_shared_nat(v4: Ipv4Addr) -> bool { + let o = v4.octets(); + o[0] == 100 && (o[1] & 0b1100_0000) == 0b0100_0000 +} + +/// Denies ipv6 like `fc00::/7` +fn is_unique_local(v6: Ipv6Addr) -> bool { + (v6.segments()[0] & 0xfe00) == 0xfc00 +} + +/// Denies ipv6 like `fe80::/10` +fn is_ipv6_link_local(v6: Ipv6Addr) -> bool { + (v6.segments()[0] & 0xffc0) == 0xfe80 +} + +fn allowed_scheme(url: &Url) -> bool { + matches!(url.scheme(), "http" | "https") +} + +fn ends_with_label(host: &str, suffix_with_dot: &str) -> bool { + let label = &suffix_with_dot[1..]; + host == label || host.ends_with(suffix_with_dot) +} + +fn is_denied_host(host: &str) -> bool { + DOWNLOAD_DENYLIST_DOMAINS.contains(&host) || DOWNLOAD_DENYLIST_TLDS.iter().any(|&i| ends_with_label(host, i)) +} + +fn validate_download_url(url: &Url) -> Result, ModZipError> { + // First, validate the domain + if !allowed_scheme(url) { + return Err(ModZipError::InvalidModFileUrl); + } + + let domain = url.domain().ok_or(ModZipError::InvalidModFileUrl)?; + + if is_denied_host(domain) { + return Err(ModZipError::InvalidModFileUrl); + } + + // Now resolve and validate the IP itself to make sure + // the DNS isn't pointing to something bad + let host = url + .host_str() + .ok_or(ModZipError::InvalidModFileUrl) + .inspect_err(|_| tracing::warn!("host_str() returned None - very weird!"))?; + let port = url.port_or_known_default().unwrap_or(443); + + let addrs: Vec = (host, port) + .to_socket_addrs() + .map_err(|_| ModZipError::InvalidModFileUrl)? + .map(|s| s.ip()) + .collect(); + + if addrs.is_empty() { + tracing::warn!("{host}:{port} failed DNS resolution"); + return Err(ModZipError::InvalidModFileUrl); + } + + if let Some(ip) = addrs.iter().find(|&ip| is_disallowed_ip(*ip)) { + tracing::warn!("{host}:{port} resolved to disallowed ip {ip}"); + return Err(ModZipError::InvalidModFileUrl); + } + + Ok(addrs) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::net::IpAddr; + + #[test] + fn rejects_non_http_schemes() { + assert!(!allowed_scheme(&Url::parse("file:///etc/passwd").unwrap())); + assert!(!allowed_scheme(&Url::parse("ftp://example.com/x").unwrap())); + + assert!(allowed_scheme(&Url::parse("https://example.com").unwrap())); + assert!(allowed_scheme(&Url::parse("http://example.com").unwrap())); } - Ok(Bytes::from(data)) + #[test] + fn exact_domain_match() { + assert!(is_denied_host("localhost")); + } + + #[test] + fn exact_domain_is_case_sensitive_by_design() { + // url::Url normalizes host to lowercase during parsing, so this + // function assumes lowercase input. Document that assumption here. + assert!(!is_denied_host("LOCALHOST")); + } + + #[test] + fn tld_exact_match() { + // host == the bare suffix itself, no subdomain + assert!(is_denied_host("internal")); + assert!(is_denied_host("local")); + } + + #[test] + fn tld_subdomain_match() { + assert!(is_denied_host("foo.internal")); + assert!(is_denied_host("service.lan")); + assert!(is_denied_host("printer.local")); + assert!(is_denied_host("db.host")); + assert!(is_denied_host("deep.nested.sub.internal")); + } + + #[test] + fn allows_unrelated_domains() { + assert!(!is_denied_host("example.com")); + assert!(!is_denied_host("github.com")); + assert!(!is_denied_host("sub.example.com")); + } + + #[test] + fn rejects_ip_literal_hosts() { + assert!(validate_download_url(&Url::parse("http://127.0.0.1/x").unwrap()).is_err()); + assert!(validate_download_url(&Url::parse("http://[::1]/download").unwrap()).is_err()); + } + + #[test] + fn flags_private_and_special_ranges() { + let cases: &[(&str, bool)] = &[ + ("127.0.0.1", true), + ("10.0.0.5", true), + ("172.16.0.1", true), + ("192.168.1.1", true), + ("169.254.169.254", true), // cloud metadata + ("100.64.0.1", true), // CGNAT + ("8.8.8.8", false), + ("1.1.1.1", false), + ]; + for (ip, expected) in cases { + let addr: IpAddr = ip.parse().unwrap(); + assert_eq!(is_disallowed_ip(addr), *expected, "failed for {ip}"); + } + } + + #[test] + fn flags_ipv6_special_ranges() { + assert!(is_disallowed_ip("::1".parse().unwrap())); + assert!(is_disallowed_ip("fc00::1".parse().unwrap())); + assert!(is_disallowed_ip("fe80::1".parse().unwrap())); + assert!(!is_disallowed_ip("2606:4700:4700::1111".parse().unwrap())); // public + } } diff --git a/src/pin_dns.rs b/src/pin_dns.rs new file mode 100644 index 0000000..b4d1fba --- /dev/null +++ b/src/pin_dns.rs @@ -0,0 +1,47 @@ +use std::net::{SocketAddr, ToSocketAddrs}; + +use reqwest::dns::{Name, Resolve, Resolving}; + +tokio::task_local! { + /// Used to pin an IP address for the resolver. + /// + /// # Example (assumes you have an http client that uses PinDnsResolver) + /// + /// ```rust + /// // Let's assume addr is an address we confirmed is safe to call. + /// let addr = std::net::SocketAddr::new("127.0.0.1", port); + /// + /// let response = PINNED_ADDR.scope(Cell::new(Some(addr)), async { + /// http_client.get(url).send().await + /// }).await?; + /// ``` + /// + /// Make sure to only run **the one request you need** inside the callback. + /// This design is a little brittle, but works for our purposes. + pub static PINNED_ADDR: std::cell::Cell>; +} + +#[derive(Clone, Default)] +pub struct PinDnsResolver; + +impl Resolve for PinDnsResolver { + fn resolve(&self, name: Name) -> Resolving { + Box::pin(async move { + tracing::debug!("resolving {} with PinDnsResolver", name.as_str()); + + if let Ok(Some(addr)) = PINNED_ADDR.try_with(|c| c.get()) { + tracing::debug!("pinned {} to {addr} successfully", name.as_str()); + return Ok( + Box::new(std::iter::once(addr)) as Box + Send> + ); + } + + tracing::debug!("didn't have a PINNED_ADDR, falling back to OS resolver"); + // Fallback to the usual OS resolver otherwise + (name.as_str(), 0) + .to_socket_addrs() + .map(|it| Box::new(it) as Box + Send>) + .map_err(|e| e.into()) + }) + } +} diff --git a/src/s3_worker.rs b/src/s3_worker.rs index 9b7e117..764b343 100644 --- a/src/s3_worker.rs +++ b/src/s3_worker.rs @@ -110,8 +110,12 @@ async fn migrate_one( version: &str, version_id: i32, ) -> anyhow::Result<()> { - let bytes = - mod_zip::download_mod(data.http_client(), original_url, data.max_download_mb()).await?; + let bytes = mod_zip::download_mod( + data.pin_dns_http_client(), + original_url, + data.max_download_mb(), + ) + .await?; process_task( data,