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

@@ -34,6 +34,16 @@ name = "fs_readonly"
 path = "tests/fs_readonly.rs"
 harness = false
 
+[[test]]
+name = "fs_remount"
+path = "tests/fs_remount.rs"
+harness = false
+
+[[test]]
+name = "fs_remount_symlink"
+path = "tests/fs_remount_symlink.rs"
+harness = false
+
 [[test]]
 name = "fs_write_also_read"
 path = "tests/fs_write_also_read.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,25 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
     // aren't created outside the sandbox.
     mount_tmpfs(&new_root_c)?;
 
+    // Canonicalize paths, to 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 let Ok(target) = path.read_link() {
+                symlinks.push((path, target));
+            }
+
+            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 +96,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 +113,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 +136,42 @@ 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 (path, target) in symlinks {
+        // Ensure path is 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,
+        };
+
+        // Ignore symlinks if a parent bind mount exists.
+        let unrooted_path = path.strip_prefix("/").unwrap();
+        let dst = new_root.join(unrooted_path);
+        if dst.symlink_metadata().is_ok() {
+            continue;
+        }
+
+        // Create all parent directories.
+        let parent = match path.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 +194,9 @@ 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.
+        // TODO: symlink_metadata().is_ok()?
+        //
+        // Skip nodes that already exist.
         if dst.exists() {
             continue;
         }
@@ -170,7 +222,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 +364,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;
     }
 }
 

tests/fs_remount.rs

@@ -0,0 +1,29 @@
+use std::fs;
+
+use birdcage::{Birdcage, Exception, Sandbox};
+
+fn main() {
+    const FILE_CONTENT: &str = "expected content";
+
+    // Setup our test tree.
+    let tempdir = tempfile::tempdir().unwrap().into_path();
+    let tempfile = tempdir.join("target-file");
+    fs::write(&tempfile, FILE_CONTENT.as_bytes()).unwrap();
+
+    // Setup sandbox, allowing write to dir, but only read for the file.
+    let mut birdcage = Birdcage::new();
+    birdcage.add_exception(Exception::WriteAndRead(tempdir.clone())).unwrap();
+    birdcage.add_exception(Exception::Read(tempfile.clone())).unwrap();
+    birdcage.lock().unwrap();
+
+    // Write access to directory works.
+    fs::create_dir(tempdir.join("boop")).unwrap();
+
+    // Read access to file works.
+    let content = fs::read_to_string(&tempfile).unwrap();
+    assert_eq!(content, FILE_CONTENT);
+
+    // Write access to file is denied.
+    let result = fs::write(&tempfile, "no");
+    assert!(result.is_err());
+}

tests/fs_remount_symlink.rs

@@ -0,0 +1,32 @@
+use std::fs;
+use std::os::unix::fs as unixfs;
+
+use birdcage::{Birdcage, Exception, Sandbox};
+
+fn main() {
+    const FILE_CONTENT: &str = "expected content";
+
+    // Setup our test tree.
+
+    let root = tempfile::tempdir().unwrap().into_path();
+
+    let lib = root.join("usr").join("lib");
+    fs::create_dir_all(&lib).unwrap();
+    let file = lib.join("os-release");
+
+    let etc = root.join("etc");
+    fs::create_dir(&etc).unwrap();
+    fs::write(&file, FILE_CONTENT.as_bytes()).unwrap();
+    let symlink = etc.join("os-release");
+    unixfs::symlink("../usr/lib/os-release", &symlink).unwrap();
+
+    // Setup sandbox, ensuring sandbox can be created.
+    let mut birdcage = Birdcage::new();
+    birdcage.add_exception(Exception::Read(etc.clone())).unwrap();
+    birdcage.add_exception(Exception::Read(symlink.clone())).unwrap();
+    birdcage.lock().unwrap();
+
+    // Ensure we can read from the symlink.
+    let content = fs::read_to_string(symlink).unwrap();
+    assert_eq!(content, FILE_CONTENT);
+}