From a495dcebab66c09a2de12faf8118a28ad30071f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Kj=C3=A4ll?= Date: Sat, 11 Apr 2026 15:02:39 +0200 Subject: [PATCH 1/2] In ServeDir, change the behaviour for handling symlinks that point outside the base_dir to return errors, and introduce a new setting that will allow it (on linux) --- tower-http/Cargo.toml | 5 ++ tower-http/src/services/fs/serve_dir/mod.rs | 20 +++++ .../src/services/fs/serve_dir/open_file.rs | 84 +++++++++++++++++-- tower-http/src/services/fs/serve_dir/tests.rs | 73 +++++++++++++++- tower-http/src/services/fs/serve_file.rs | 28 +++++++ 5 files changed, 200 insertions(+), 10 deletions(-) diff --git a/tower-http/Cargo.toml b/tower-http/Cargo.toml index 4c1e92e9..3b186e8d 100644 --- a/tower-http/Cargo.toml +++ b/tower-http/Cargo.toml @@ -39,6 +39,10 @@ tracing = { version = "0.1", default-features = false, optional = true } httpdate = { version = "1.0", optional = true } uuid = { version = "1.0", features = ["v4"], optional = true } +# linux only dependencies +[target.'cfg(target_os = "linux")'.dependencies] +rustix = { version = "1", features = ["fs"] } + [dev-dependencies] brotli = "8" bytes = "1" @@ -55,6 +59,7 @@ tokio = { version = "1", features = ["full"] } tower = { version = "0.5", features = ["buffer", "util", "retry", "make", "timeout"] } tracing-subscriber = "0.3" zstd = "0.13" +tempfile = "3" [features] default = [] diff --git a/tower-http/src/services/fs/serve_dir/mod.rs b/tower-http/src/services/fs/serve_dir/mod.rs index c9c4dcbc..02789b97 100644 --- a/tower-http/src/services/fs/serve_dir/mod.rs +++ b/tower-http/src/services/fs/serve_dir/mod.rs @@ -40,6 +40,10 @@ const DEFAULT_CAPACITY: usize = 65536; /// existing file (`/file.html/something`) /// - We don't have necessary permissions to read the file /// +/// On linux, if a file is a symlink to a file outside the base directory, +/// a 500 error will be returned, unless `follow_symlinks_outside_base` +/// is set to true. +/// /// # Example /// /// ``` @@ -59,6 +63,7 @@ pub struct ServeDir { variant: ServeVariant, fallback: Option, call_fallback_on_method_not_allowed: bool, + follow_symlinks_outside_base: bool, } impl ServeDir { @@ -79,6 +84,7 @@ impl ServeDir { }, fallback: None, call_fallback_on_method_not_allowed: false, + follow_symlinks_outside_base: false, } } @@ -93,6 +99,7 @@ impl ServeDir { variant: ServeVariant::SingleFile { mime }, fallback: None, call_fallback_on_method_not_allowed: false, + follow_symlinks_outside_base: false, } } } @@ -217,6 +224,7 @@ impl ServeDir { variant: self.variant, fallback: Some(new_fallback), call_fallback_on_method_not_allowed: self.call_fallback_on_method_not_allowed, + follow_symlinks_outside_base: self.follow_symlinks_outside_base, } } @@ -249,6 +257,16 @@ impl ServeDir { self } + /// Start following symlinks outside the base directory. + /// + /// Warning: if you have a writeable directory inside the base directory, this + /// means that a local user can exfiltrate any file that the server process have + /// read access to. + pub fn follow_symlinks_outside_base(mut self, follow_symlinks_outside_base: bool) -> Self { + self.follow_symlinks_outside_base = follow_symlinks_outside_base; + self + } + /// Call the service and get a future that contains any `std::io::Error` that might have /// happened. /// @@ -381,6 +399,8 @@ impl ServeDir { negotiated_encodings, range_header, buf_chunk_size, + self.base.clone(), + self.follow_symlinks_outside_base, )); ResponseFuture::open_file_future(open_file_future, fallback_and_request) diff --git a/tower-http/src/services/fs/serve_dir/open_file.rs b/tower-http/src/services/fs/serve_dir/open_file.rs index 9dd1e012..78425b99 100644 --- a/tower-http/src/services/fs/serve_dir/open_file.rs +++ b/tower-http/src/services/fs/serve_dir/open_file.rs @@ -40,6 +40,7 @@ pub(super) enum FileRequestExtent { Head(Metadata), } +#[allow(clippy::too_many_arguments)] pub(super) async fn open_file( variant: ServeVariant, mut path_to_file: PathBuf, @@ -47,6 +48,8 @@ pub(super) async fn open_file( negotiated_encodings: Vec<(Encoding, QValue)>, range_header: Option, buf_chunk_size: usize, + base_path: PathBuf, + follow_symlinks_outside_base: bool, ) -> io::Result { let if_unmodified_since = req .headers() @@ -110,15 +113,21 @@ pub(super) async fn open_file( last_modified, }))) } else { - let (mut file, maybe_encoding) = - match open_file_with_fallback(path_to_file, negotiated_encodings).await { - Ok(result) => result, + let (mut file, maybe_encoding) = match open_file_with_fallback( + path_to_file, + negotiated_encodings, + &base_path, + follow_symlinks_outside_base, + ) + .await + { + Ok(result) => result, - Err(err) if is_invalid_filename_error(&err) => { - return Ok(OpenFileOutput::InvalidFilename) - } - Err(err) => return Err(err), - }; + Err(err) if is_invalid_filename_error(&err) => { + return Ok(OpenFileOutput::InvalidFilename) + } + Err(err) => return Err(err), + }; let meta = file.metadata().await?; let last_modified = meta.modified().ok().map(LastModified::from); @@ -226,17 +235,74 @@ fn preferred_encoding( preferred_encoding } +fn canonicalize_and_openat2(base_path: &Path, path: &Path) -> io::Result { + let (path, base_path2) = if base_path.is_file() || !base_path.exists() { + let base_path = base_path.parent().unwrap().canonicalize()?; + let path = path + .canonicalize()? + .strip_prefix(&base_path) + .map_err(|err| io::Error::new(io::ErrorKind::Other, err))? + .to_owned(); + ( + path, + rustix::fs::open( + base_path, + rustix::fs::OFlags::RDONLY, + rustix::fs::Mode::empty(), + )?, + ) + } else { + let base_path = base_path.canonicalize()?; + let path = path + .canonicalize()? + .strip_prefix(&base_path) + .map_err(|err| io::Error::new(io::ErrorKind::Other, err))? + .to_owned(); + ( + path, + rustix::fs::open( + base_path, + rustix::fs::OFlags::RDONLY, + rustix::fs::Mode::empty(), + )?, + ) + }; + + rustix::fs::openat2( + base_path2, + &path, + rustix::fs::OFlags::RDONLY, + rustix::fs::Mode::empty(), + rustix::fs::ResolveFlags::BENEATH, + ) + .map(std::fs::File::from) + .map(File::from) + .map_err(|err| io::Error::new(io::ErrorKind::Other, err)) +} + // Attempts to open the file with any of the possible negotiated_encodings in the // preferred order. If none of the negotiated_encodings have a corresponding precompressed // file the uncompressed file is used as a fallback. async fn open_file_with_fallback( mut path: PathBuf, mut negotiated_encoding: Vec<(Encoding, QValue)>, + base_path: &Path, + follow_symlinks_outside_base: bool, ) -> io::Result<(File, Option)> { let (file, encoding) = loop { // Get the preferred encoding among the negotiated ones. let encoding = preferred_encoding(&mut path, &negotiated_encoding); - match (File::open(&path).await, encoding) { + let file = { + #[cfg(target_os = "linux")] + if follow_symlinks_outside_base { + File::open(&path).await + } else { + canonicalize_and_openat2(base_path, &path) + } + #[cfg(not(target_os = "linux"))] + File::open(&path).await + }; + match (file, encoding) { (Ok(file), maybe_encoding) => break (file, maybe_encoding), (Err(err), Some(encoding)) if err.kind() == io::ErrorKind::NotFound => { // Remove the extension corresponding to a precompressed file (.gz, .br, .zz) diff --git a/tower-http/src/services/fs/serve_dir/tests.rs b/tower-http/src/services/fs/serve_dir/tests.rs index e220197a..336ff488 100644 --- a/tower-http/src/services/fs/serve_dir/tests.rs +++ b/tower-http/src/services/fs/serve_dir/tests.rs @@ -10,7 +10,9 @@ use http_body::Body as HttpBody; use http_body_util::BodyExt; use std::convert::Infallible; use std::fs; -use std::io::Read; +use std::fs::{create_dir, File}; +use std::io::{Read, Write}; +use std::os::unix::fs::symlink; use tower::{service_fn, ServiceExt}; #[tokio::test] @@ -627,6 +629,23 @@ async fn read_partial_errs_on_bad_range() { ) } +#[tokio::test] +async fn read_partial_errs_on_bad_range_follow_symlinks() { + let svc = ServeDir::new("..").follow_symlinks_outside_base(true); + let req = Request::builder() + .uri("/README.md") + .header("Range", "bytes=-1-15") + .body(Body::empty()) + .unwrap(); + let res = svc.oneshot(req).await.unwrap(); + assert_eq!(res.status(), StatusCode::RANGE_NOT_SATISFIABLE); + let file_contents = std::fs::read("../README.md").unwrap(); + assert_eq!( + res.headers()["content-range"], + &format!("bytes */{}", file_contents.len()) + ) +} + #[tokio::test] async fn accept_encoding_identity() { let svc = ServeDir::new(".."); @@ -877,3 +896,55 @@ async fn calls_fallback_on_null() { assert_eq!(res.headers()["from-fallback"], "1"); } + +#[tokio::test] +async fn returns_500_if_symlink_outside_base() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path().join("base"); + create_dir(&base).unwrap(); + let file_outside = tmp.path().join("file_outside.txt"); + let mut outside = File::create(&file_outside).unwrap(); + write!(outside, "{}", "outside").unwrap(); + let file_inside = base.join("inside.txt"); + symlink(file_outside, file_inside).unwrap(); + + let svc = ServeDir::new(base); + + let request = Request::builder() + .header("Accept-Encoding", "deflate") + .method(Method::GET) + .uri("/inside.txt") + .body(Body::empty()) + .unwrap(); + let res = svc.oneshot(request).await.unwrap(); + + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + assert!(res.headers().get(header::CONTENT_TYPE).is_none()); +} + +#[tokio::test] +async fn allow_symlink_outside_base() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path().join("base"); + create_dir(&base).unwrap(); + let file_outside = tmp.path().join("file_outside.txt"); + let mut outside = File::create(&file_outside).unwrap(); + write!(outside, "{}", "outside").unwrap(); + let file_inside = base.join("inside.txt"); + symlink(file_outside, file_inside).unwrap(); + + let svc = ServeDir::new(base).follow_symlinks_outside_base(true); + + let request = Request::builder() + .header("Accept-Encoding", "deflate") + .method(Method::GET) + .uri("/inside.txt") + .body(Body::empty()) + .unwrap(); + let res = svc.oneshot(request).await.unwrap(); + + assert_eq!(res.status(), StatusCode::OK); + assert!(res.headers().get(header::CONTENT_TYPE).is_some()); + let body = res.into_body().collect().await.unwrap().to_bytes(); + assert_eq!("outside", String::from_utf8_lossy(&body)); +} diff --git a/tower-http/src/services/fs/serve_file.rs b/tower-http/src/services/fs/serve_file.rs index ade3cd15..04b9cbaf 100644 --- a/tower-http/src/services/fs/serve_file.rs +++ b/tower-http/src/services/fs/serve_file.rs @@ -152,7 +152,10 @@ mod tests { use http::{Request, StatusCode}; use http_body_util::BodyExt; use mime::Mime; + use std::fs::{create_dir, File}; use std::io::Read; + use std::io::Write; + use std::os::unix::fs::symlink; use std::str::FromStr; use tokio::io::AsyncReadExt; use tower::ServiceExt; @@ -496,6 +499,31 @@ mod tests { assert!(res.headers().get(header::CONTENT_TYPE).is_none()); } + #[tokio::test] + async fn returns_500_if_symlink_outside_base() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path().join("base"); + create_dir(&base).unwrap(); + let file_outside = tmp.path().join("file_outside.txt"); + let mut outside = File::create(&file_outside).unwrap(); + write!(outside, "{}", "outside").unwrap(); + let file_inside = base.join("inside.txt"); + symlink(file_outside, file_inside).unwrap(); + + let svc = ServeFile::new(base); + + let request = Request::builder() + .header("Accept-Encoding", "deflate") + .method(Method::GET) + .uri("/inside.txt") + .body(Body::empty()) + .unwrap(); + let res = svc.oneshot(request).await.unwrap(); + + assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR); + assert!(res.headers().get(header::CONTENT_TYPE).is_none()); + } + #[tokio::test] async fn last_modified() { let svc = ServeFile::new("../README.md"); From 79ef5b036fe80359b9c840b65fddd7b78891034d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Kj=C3=A4ll?= Date: Sat, 11 Apr 2026 19:28:32 +0200 Subject: [PATCH 2/2] review feedback: switch follow_symlinks_outside_base to be default true --- tower-http/src/services/fs/serve_dir/mod.rs | 18 ++++++++---------- .../src/services/fs/serve_dir/open_file.rs | 1 + tower-http/src/services/fs/serve_dir/tests.rs | 5 +++-- tower-http/src/services/fs/serve_file.rs | 12 +++++++++++- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tower-http/src/services/fs/serve_dir/mod.rs b/tower-http/src/services/fs/serve_dir/mod.rs index 02789b97..29c017e3 100644 --- a/tower-http/src/services/fs/serve_dir/mod.rs +++ b/tower-http/src/services/fs/serve_dir/mod.rs @@ -40,9 +40,9 @@ const DEFAULT_CAPACITY: usize = 65536; /// existing file (`/file.html/something`) /// - We don't have necessary permissions to read the file /// -/// On linux, if a file is a symlink to a file outside the base directory, -/// a 500 error will be returned, unless `follow_symlinks_outside_base` -/// is set to true. +/// If the requested file is a symlink, it will be followed, even outside `base`. +/// On Linux this behaviour can be modified to only allow symlinks below `base` +/// with `follow_symlinks_outside_base`. /// /// # Example /// @@ -84,7 +84,7 @@ impl ServeDir { }, fallback: None, call_fallback_on_method_not_allowed: false, - follow_symlinks_outside_base: false, + follow_symlinks_outside_base: true, } } @@ -99,7 +99,7 @@ impl ServeDir { variant: ServeVariant::SingleFile { mime }, fallback: None, call_fallback_on_method_not_allowed: false, - follow_symlinks_outside_base: false, + follow_symlinks_outside_base: true, } } } @@ -257,11 +257,9 @@ impl ServeDir { self } - /// Start following symlinks outside the base directory. - /// - /// Warning: if you have a writeable directory inside the base directory, this - /// means that a local user can exfiltrate any file that the server process have - /// read access to. + /// Disallow following symlinks outside the base directory on Linux. + /// Setting this to false on Linux introduces a dependency on at least + /// kernel version 5.6, as the openat2 syscall was introduced in that version. pub fn follow_symlinks_outside_base(mut self, follow_symlinks_outside_base: bool) -> Self { self.follow_symlinks_outside_base = follow_symlinks_outside_base; self diff --git a/tower-http/src/services/fs/serve_dir/open_file.rs b/tower-http/src/services/fs/serve_dir/open_file.rs index 78425b99..1cfa47fd 100644 --- a/tower-http/src/services/fs/serve_dir/open_file.rs +++ b/tower-http/src/services/fs/serve_dir/open_file.rs @@ -235,6 +235,7 @@ fn preferred_encoding( preferred_encoding } +#[cfg(target_os = "linux")] fn canonicalize_and_openat2(base_path: &Path, path: &Path) -> io::Result { let (path, base_path2) = if base_path.is_file() || !base_path.exists() { let base_path = base_path.parent().unwrap().canonicalize()?; diff --git a/tower-http/src/services/fs/serve_dir/tests.rs b/tower-http/src/services/fs/serve_dir/tests.rs index 336ff488..bab43702 100644 --- a/tower-http/src/services/fs/serve_dir/tests.rs +++ b/tower-http/src/services/fs/serve_dir/tests.rs @@ -898,6 +898,7 @@ async fn calls_fallback_on_null() { } #[tokio::test] +#[cfg(target_os = "linux")] async fn returns_500_if_symlink_outside_base() { let tmp = tempfile::tempdir().unwrap(); let base = tmp.path().join("base"); @@ -908,7 +909,7 @@ async fn returns_500_if_symlink_outside_base() { let file_inside = base.join("inside.txt"); symlink(file_outside, file_inside).unwrap(); - let svc = ServeDir::new(base); + let svc = ServeDir::new(base).follow_symlinks_outside_base(false); let request = Request::builder() .header("Accept-Encoding", "deflate") @@ -933,7 +934,7 @@ async fn allow_symlink_outside_base() { let file_inside = base.join("inside.txt"); symlink(file_outside, file_inside).unwrap(); - let svc = ServeDir::new(base).follow_symlinks_outside_base(true); + let svc = ServeDir::new(base); let request = Request::builder() .header("Accept-Encoding", "deflate") diff --git a/tower-http/src/services/fs/serve_file.rs b/tower-http/src/services/fs/serve_file.rs index 04b9cbaf..64ad9740 100644 --- a/tower-http/src/services/fs/serve_file.rs +++ b/tower-http/src/services/fs/serve_file.rs @@ -105,6 +105,16 @@ impl ServeFile { Self(self.0.with_buf_chunk_size(chunk_size)) } + /// Disallow following symlinks outside the base directory on Linux. + /// Setting this to false on Linux introduces a dependency on at least + /// kernel version 5.6, as the openat2 syscall was introduced in that version. + pub fn follow_symlinks_outside_base(self, follow_symlinks_outside_base: bool) -> Self { + Self( + self.0 + .follow_symlinks_outside_base(follow_symlinks_outside_base), + ) + } + /// Call the service and get a future that contains any `std::io::Error` that might have /// happened. /// @@ -510,7 +520,7 @@ mod tests { let file_inside = base.join("inside.txt"); symlink(file_outside, file_inside).unwrap(); - let svc = ServeFile::new(base); + let svc = ServeFile::new(base).follow_symlinks_outside_base(false); let request = Request::builder() .header("Accept-Encoding", "deflate")