Fix symlink handling with Linux sandbox

CHANGELOG.md

@@ -6,6 +6,12 @@ The sections should follow the order `Packaging`, `Added`, `Changed`, `Fixed` an
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [Unreleased]
+
+### Fixed
+
+- (Linux) Sandbox exceptions for symbolic links
+
 ## [v0.5.0] - 2023-10-13
 
 ### Changed

Cargo.toml

@@ -24,6 +24,21 @@ name = "exec"
 path = "tests/exec.rs"
 harness = false
 
+[[test]]
+name = "exec_symlinked_dir"
+path = "tests/exec_symlinked_dir.rs"
+harness = false
+
+[[test]]
+name = "exec_symlinked_file"
+path = "tests/exec_symlinked_file.rs"
+harness = false
+
+[[test]]
+name = "exec_symlinked_dirs_exec"
+path = "tests/exec_symlinked_dirs_exec.rs"
+harness = false
+
 [[test]]
 name = "fs"
 path = "tests/fs.rs"

src/lib.rs

@@ -63,6 +63,9 @@ pub trait Sandbox: Sized {
     /// This exception opens up the sandbox to allow access for the specified
     /// operation. Once an exception is added, it is **not** possible to
     /// prohibit access to this resource without creating a new sandbox.
+    ///
+    /// Exceptions added for symlinks will also automatically apply to the
+    /// symlink's target.
     fn add_exception(&mut self, exception: Exception) -> Result<&mut Self>;
 
     /// Apply the sandbox restrictions to the current process.

src/linux/namespaces.rs

@@ -6,6 +6,7 @@ use std::ffi::{CStr, CString};
 use std::fs::{self, File};
 use std::io::Error as IoError;
 use std::os::unix::ffi::OsStrExt;
+use std::os::unix::fs as unixfs;
 use std::path::{Component, Path, PathBuf};
 use std::{env, io, mem, ptr};
 
@@ -67,8 +68,29 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
     // aren't created outside the sandbox.
     mount_tmpfs(&new_root_c)?;
 
+    // Canonicalize paths and resolve symlinks.
+    //
+    // If the working directory cannot be accessed, we ignore relative paths.
+    let mut symlinks = Vec::new();
+    let mut bind_mounts = bind_mounts
+        .into_iter()
+        .filter_map(|(path, exception)| {
+            let canonicalized = path.canonicalize().ok()?;
+
+            // Store original symlink path to create it if necessary.
+            if path_has_symlinks(&path) {
+                // Normalize symlink's path.
+                let absolute = absolute(&path).ok()?;
+                let normalized = normalize_path(&absolute);
+
+                symlinks.push((normalized, canonicalized.clone()));
+            }
+
+            Some((canonicalized, exception))
+        })
+        .collect::<Vec<_>>();
+
     // Sort bind mounts by shortest length, to create parents before their children.
-    let mut bind_mounts = bind_mounts.into_iter().collect::<Vec<_>>();
     bind_mounts.sort_unstable_by(|(a_path, a_flags), (b_path, b_flags)| {
         match a_path.components().count().cmp(&b_path.components().count()) {
             Ordering::Equal => (a_path, a_flags).cmp(&(b_path, b_flags)),
@@ -78,13 +100,6 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
 
     // Bind mount all allowed directories.
     for (path, flags) in bind_mounts {
-        // Ensure all paths are absolute, without following symlinks.
-        let path = match absolute(&path) {
-            Ok(path) => normalize_path(&path),
-            // Ignore relative paths if we cannot access the working directory.
-            Err(_) => continue,
-        };
-
         let src_c = CString::new(path.as_os_str().as_bytes()).unwrap();
 
         // Get bind mount destination.
@@ -102,6 +117,9 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
         update_mount_flags(&dst_c, flags | MountAttrFlags::NOSUID)?;
     }
 
+    // Ensure original symlink paths are available.
+    create_symlinks(&new_root, symlinks)?;
+
     // Bind mount old procfs.
     let old_proc_c = CString::new("/proc").unwrap();
     let new_proc = new_root.join("proc");
@@ -122,6 +140,35 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
     Ok(())
 }
 
+/// Create missing symlinks.
+///
+/// If the parent directory of a symlink is mapped, we do not need to map the
+/// symlink ourselves and it's not possible to mount on top of it anyway. So
+/// here we make sure that symlinks are created if no bind mount was created for
+/// their parent directory.
+fn create_symlinks(new_root: &Path, symlinks: Vec<(PathBuf, PathBuf)>) -> Result<()> {
+    for (symlink, target) in symlinks {
+        // Ignore symlinks if a parent bind mount exists.
+        let unrooted_path = symlink.strip_prefix("/").unwrap();
+        let dst = new_root.join(unrooted_path);
+        if dst.symlink_metadata().is_ok() {
+            continue;
+        }
+
+        // Create all parent directories.
+        let parent = match symlink.parent() {
+            Some(parent) => parent,
+            None => continue,
+        };
+        copy_tree(parent, new_root)?;
+
+        // Create the symlink.
+        unixfs::symlink(target, dst)?;
+    }
+
+    Ok(())
+}
+
 /// Replicate a directory tree under a different directory.
 ///
 /// This will create all missing empty diretories and copy their permissions
@@ -144,7 +191,7 @@ fn copy_tree(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> Result<()> {
         src_sub = src_sub.join(component);
         dst = dst.join(component);
 
-        // Skip directories that already exist.
+        // Skip nodes that already exist.
         if dst.exists() {
             continue;
         }
@@ -170,7 +217,7 @@ fn mount_tmpfs(dst: &CStr) -> Result<()> {
     let flags = MountFlags::empty();
     let fstype = CString::new("tmpfs").unwrap();
     let res = unsafe {
-        libc::mount(fstype.as_ptr(), dst.as_ptr(), fstype.as_ptr(), flags.bits(), ptr::null())
+        libc::mount(ptr::null(), dst.as_ptr(), fstype.as_ptr(), flags.bits(), ptr::null())
     };
 
     if res == 0 {
@@ -312,6 +359,8 @@ bitflags! {
         /// Make this mount private. Mount and unmount events do not propagate into or
         /// out of this mount.
         const PRIVATE = libc::MS_PRIVATE;
+        /// Do not follow symbolic links when resolving paths.
+        const NOSYMFOLLOW = 256;
     }
 }
 
@@ -475,3 +524,8 @@ fn normalize_path(path: &Path) -> PathBuf {
 
     normalized
 }
+
+/// Check if a path contains any symlinks.
+fn path_has_symlinks(path: &Path) -> bool {
+    path.ancestors().any(|path| path.read_link().is_ok())
+}

tests/exec_symlinked_dir.rs

@@ -0,0 +1,31 @@
+use std::os::unix::fs as unixfs;
+use std::path::PathBuf;
+use std::process::Command;
+
+use birdcage::{Birdcage, Exception, Sandbox};
+
+fn main() {
+    // Create symlinked executable dir.
+    let tempdir = tempfile::tempdir().unwrap().into_path();
+    let symlink_dir = tempdir.join("bin");
+    unixfs::symlink("/usr/bin", &symlink_dir).unwrap();
+
+    let mut birdcage = Birdcage::new();
+    birdcage.add_exception(Exception::ExecuteAndRead(symlink_dir.clone())).unwrap();
+    if PathBuf::from("/lib64").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib64".into())).unwrap();
+    }
+    if PathBuf::from("/lib").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib".into())).unwrap();
+    }
+    birdcage.lock().unwrap();
+
+    // Ensure symlinked dir's executable works.
+    let symlink_dir_exec = symlink_dir.join("true");
+    let cmd = Command::new(symlink_dir_exec).status().unwrap();
+    assert!(cmd.success());
+
+    // Ensure original dir's executable works.
+    let cmd = Command::new("/usr/bin/true").status().unwrap();
+    assert!(cmd.success());
+}

tests/exec_symlinked_dirs_exec.rs

@@ -0,0 +1,31 @@
+use std::os::unix::fs as unixfs;
+use std::path::PathBuf;
+use std::process::Command;
+
+use birdcage::{Birdcage, Exception, Sandbox};
+
+fn main() {
+    // Create symlinked executable dir.
+    let tempdir = tempfile::tempdir().unwrap().into_path();
+    let symlink_dir = tempdir.join("bin");
+    let symlink_dir_exec = symlink_dir.join("true");
+    unixfs::symlink("/usr/bin", &symlink_dir).unwrap();
+
+    let mut birdcage = Birdcage::new();
+    birdcage.add_exception(Exception::ExecuteAndRead(symlink_dir_exec.clone())).unwrap();
+    if PathBuf::from("/lib64").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib64".into())).unwrap();
+    }
+    if PathBuf::from("/lib").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib".into())).unwrap();
+    }
+    birdcage.lock().unwrap();
+
+    // Ensure symlinked dir's executable works.
+    let cmd = Command::new(symlink_dir_exec).status().unwrap();
+    assert!(cmd.success());
+
+    // Ensure original dir's executable works.
+    let cmd = Command::new("/usr/bin/true").status().unwrap();
+    assert!(cmd.success());
+}

tests/exec_symlinked_file.rs

@@ -0,0 +1,33 @@
+use std::fs;
+use std::os::unix::fs as unixfs;
+use std::path::PathBuf;
+use std::process::Command;
+
+use birdcage::{Birdcage, Exception, Sandbox};
+
+fn main() {
+    // Create symlinked executable.
+    let tempdir = tempfile::tempdir().unwrap().into_path();
+    let exec_dir = tempdir.join("bin");
+    fs::create_dir(&exec_dir).unwrap();
+    let symlink_exec = exec_dir.join("true");
+    unixfs::symlink("/usr/bin/true", &symlink_exec).unwrap();
+
+    let mut birdcage = Birdcage::new();
+    birdcage.add_exception(Exception::ExecuteAndRead(symlink_exec.clone())).unwrap();
+    if PathBuf::from("/lib64").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib64".into())).unwrap();
+    }
+    if PathBuf::from("/lib").exists() {
+        birdcage.add_exception(Exception::ExecuteAndRead("/lib".into())).unwrap();
+    }
+    birdcage.lock().unwrap();
+
+    // Ensure symlinked executable works.
+    let cmd = Command::new(symlink_exec).status().unwrap();
+    assert!(cmd.success());
+
+    // Ensure original executable works.
+    let cmd = Command::new("/usr/bin/true").status().unwrap();
+    assert!(cmd.success());
+}