Skip to content

Commit

Permalink
Fix symlink handling with Linux sandbox
Browse files Browse the repository at this point in the history
This fixes an issue where dangling symbolic links would cause errors
while locking down the sandbox since the `mount` syscall is not able to
resolve the link.

To ensure symlinks are mapped correctly, the Linux sandbox now maps
every exception to its canonicalized path, granting access to the
symlink's **TARGET**. Then if necessary the symlink pointing to this
file is created to ensure access through the exception's original path.
  • Loading branch information
cd-work committed Oct 25, 2023
1 parent c2655d8 commit 6b220da
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 10 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
74 changes: 64 additions & 10 deletions src/linux/namespaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)),
Expand All @@ -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.
Expand All @@ -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");
Expand All @@ -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
Expand All @@ -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;
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/fs_remount.rs
Original file line number Diff line number Diff line change
@@ -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());
}
32 changes: 32 additions & 0 deletions tests/fs_remount_symlink.rs
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit 6b220da

Please sign in to comment.