Skip to content

Commit

Permalink
Fix symlink/canonical path exception conflicts
Browse files Browse the repository at this point in the history 
This fixes an issue on Linux where adding an exception for a path would
override all previously set permissions for the canonical path if the
old and new uncanonicalized paths did not match.
  • Loading branch information
@cd-work
cd-work committed Oct 30, 2023
1 parent 5fc4253 commit 4efaab9
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 132 deletions.
10 changes: 10 additions & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ name = "fs_readonly"
path = "tests/fs_readonly.rs"
harness = false

[[test]]
name = "fs_restrict_child"
path = "tests/fs_restrict_child.rs"
harness = false

[[test]]
name = "fs_write_also_read"
path = "tests/fs_write_also_read.rs"
Expand All @@ -69,6 +74,11 @@ name = "fs_broken_symlink"
path = "tests/fs_broken_symlink.rs"
harness = false

[[test]]
name = "fs_symlink_dir_separate_perms"
path = "tests/fs_symlink_dir_separate_perms.rs"
harness = false

[[test]]
name = "fs_null"
path = "tests/fs_null.rs"
Expand Down
164 changes: 136 additions & 28 deletions src/linux/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
use std::collections::HashMap;
use std::io::Error as IoError;
use std::path::PathBuf;
use std::os::unix::ffi::OsStrExt;
use std::path::{Component, Path, PathBuf};
use std::{env, io};

use crate::error::{Error, Result};
use crate::linux::namespaces::MountAttrFlags;
Expand All @@ -15,34 +17,12 @@ mod seccomp;
/// Linux sandboxing.
#[derive(Default)]
pub struct LinuxSandbox {
bind_mounts: HashMap<PathBuf, MountAttrFlags>,
env_exceptions: Vec<String>,
path_exceptions: PathExceptions,
allow_networking: bool,
full_env: bool,
}

impl LinuxSandbox {
/// Add or modify a bind mount.
///
/// This will add a new bind mount with the specified permission if it does
/// not exist already.
///
/// If the bind mount already exists, it will *ADD* the additional
/// permissions.
fn update_bind_mount(&mut self, path: PathBuf, write: bool, execute: bool) {
let flags =
self.bind_mounts.entry(path).or_insert(MountAttrFlags::RDONLY | MountAttrFlags::NOEXEC);

if write {
flags.remove(MountAttrFlags::RDONLY);
}

if execute {
flags.remove(MountAttrFlags::NOEXEC);
}
}
}

impl Sandbox for LinuxSandbox {
fn new() -> Self {
Self::default()
Expand All @@ -60,9 +40,9 @@ impl Sandbox for LinuxSandbox {
}

match exception {
Exception::Read(path) => self.update_bind_mount(path, false, false),
Exception::WriteAndRead(path) => self.update_bind_mount(path, true, false),
Exception::ExecuteAndRead(path) => self.update_bind_mount(path, false, true),
Exception::Read(path) => self.path_exceptions.update(path, false, false),
Exception::WriteAndRead(path) => self.path_exceptions.update(path, true, false),
Exception::ExecuteAndRead(path) => self.path_exceptions.update(path, false, true),
Exception::Environment(key) => self.env_exceptions.push(key),
Exception::FullEnvironment => self.full_env = true,
Exception::Networking => self.allow_networking = true,
Expand All @@ -78,7 +58,7 @@ impl Sandbox for LinuxSandbox {
}

// Setup namespaces.
namespaces::create_namespaces(self.allow_networking, self.bind_mounts)?;
namespaces::create_namespaces(self.allow_networking, self.path_exceptions)?;

// Setup system call filters.
SyscallFilter::apply()?;
Expand All @@ -93,6 +73,62 @@ impl Sandbox for LinuxSandbox {
}
}

/// Path permissions required for the sandbox.
#[derive(Default)]
pub(crate) struct PathExceptions {
bind_mounts: HashMap<PathBuf, MountAttrFlags>,
symlinks: Vec<(PathBuf, PathBuf)>,
}

impl PathExceptions {
/// Add or modify a path's exceptions.
///
/// This will add a new bind mount for the canonical path with the specified
/// permission if it does not exist already.
///
/// If the bind mount already exists, it will *ADD* the additional
/// permissions.
fn update(&mut self, path: PathBuf, write: bool, execute: bool) {
// Use canonical path for indexing.
//
// This ensures that a symlink and its target are treated like the same path for
// exceptions.
//
// If the home path cannot be accessed, we ignore the exception.
let canonical_path = match path.canonicalize() {
Ok(path) => path,
Err(_) => return,
};

// Store original symlink path to create it if necessary.
if path_has_symlinks(&path) {
// Normalize symlink's path.
let absolute = match absolute(&path) {
Ok(absolute) => absolute,
Err(_) => return,
};
let normalized = normalize_path(&absolute);

self.symlinks.push((normalized, canonical_path.clone()));
}

// Update bind mount's permission flags.

let flags = self
.bind_mounts
.entry(canonical_path)
.or_insert(MountAttrFlags::RDONLY | MountAttrFlags::NOEXEC);

if write {
flags.remove(MountAttrFlags::RDONLY);
}

if execute {
flags.remove(MountAttrFlags::NOEXEC);
}
}
}

/// Prevent suid/sgid.
fn no_new_privs() -> Result<()> {
let result = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) };
Expand All @@ -102,3 +138,75 @@ fn no_new_privs() -> Result<()> {
_ => Err(IoError::last_os_error().into()),
}
}

// Copied from Rust's STD:
// https://github.com/rust-lang/rust/blob/42faef503f3e765120ca0ef06991337668eafc32/library/std/src/sys/unix/path.rs#L23C1-L63C2
//
// Licensed under MIT:
// https://github.com/rust-lang/rust/blob/master/LICENSE-MIT
//
/// Make a POSIX path absolute without changing its semantics.
fn absolute(path: &Path) -> io::Result<PathBuf> {
// This is mostly a wrapper around collecting `Path::components`, with
// exceptions made where this conflicts with the POSIX specification.
// See 4.13 Pathname Resolution, IEEE Std 1003.1-2017
// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13

// Get the components, skipping the redundant leading "." component if it
// exists.
let mut components = path.strip_prefix(".").unwrap_or(path).components();
let path_os = path.as_os_str().as_bytes();

let mut normalized = if path.is_absolute() {
// "If a pathname begins with two successive <slash> characters, the
// first component following the leading <slash> characters may be
// interpreted in an implementation-defined manner, although more than
// two leading <slash> characters shall be treated as a single <slash>
// character."
if path_os.starts_with(b"//") && !path_os.starts_with(b"///") {
components.next();
PathBuf::from("//")
} else {
PathBuf::new()
}
} else {
env::current_dir()?
};
normalized.extend(components);

// "Interfaces using pathname resolution may specify additional constraints
// when a pathname that does not name an existing directory contains at
// least one non- <slash> character and contains one or more trailing
// <slash> characters".
// A trailing <slash> is also meaningful if "a symbolic link is
// encountered during pathname resolution".
if path_os.ends_with(b"/") {
normalized.push("");
}

Ok(normalized)
}

/// Normalize path components, stripping out `.` and `..`.
fn normalize_path(path: &Path) -> PathBuf {
let mut normalized = PathBuf::new();

for component in path.components() {
match component {
Component::Prefix(_) => unreachable!("impl does not consider windows"),
Component::RootDir => normalized.push("/"),
Component::CurDir => continue,
Component::ParentDir => {
normalized.pop();
},
Component::Normal(segment) => normalized.push(segment),
}
}

normalized
}

/// Check if a path contains any symlinks.
fn path_has_symlinks(path: &Path) -> bool {
path.ancestors().any(|path| path.read_link().is_ok())
}
112 changes: 8 additions & 104 deletions src/linux/namespaces.rs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
//! Linux namespaces.
use std::cmp::Ordering;
use std::collections::HashMap;
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};
use std::path::{Path, PathBuf, Component};
use std::{env, mem, ptr};

use bitflags::bitflags;

use crate::error::Result;
use crate::linux::PathExceptions;

/// Path for mount namespace's new root.
const NEW_ROOT: &str = "/tmp/birdcage-root";
Expand All @@ -23,10 +23,7 @@ const NEW_ROOT: &str = "/tmp/birdcage-root";
///
/// Additionally it will isolate network access if `allow_networking` is
/// `false`.
pub fn create_namespaces(
allow_networking: bool,
bind_mounts: HashMap<PathBuf, MountAttrFlags>,
) -> Result<()> {
pub fn create_namespaces(allow_networking: bool, exceptions: PathExceptions) -> Result<()> {
// Get EUID/EGID outside of the namespace.
let uid = unsafe { libc::geteuid() };
let gid = unsafe { libc::getegid() };
Expand All @@ -37,7 +34,7 @@ pub fn create_namespaces(
}

// Isolate filesystem and procfs.
create_mount_namespace(bind_mounts)?;
create_mount_namespace(exceptions)?;

// Drop root user mapping and ensure abstract namespace is cleared.
create_user_namespace(uid, gid, Namespaces::empty())?;
Expand All @@ -49,7 +46,7 @@ pub fn create_namespaces(
///
/// This will deny access to any path which isn't part of `bind_mounts`. Allowed
/// paths are mounted according to their bind mount flags.
fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Result<()> {
fn create_mount_namespace(exceptions: PathExceptions) -> Result<()> {
// Create mount namespace to allow creation of new mounts.
create_user_namespace(0, 0, Namespaces::MOUNT)?;

Expand All @@ -68,29 +65,8 @@ 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: Vec<_> = exceptions.bind_mounts.into_iter().collect();
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 Down Expand Up @@ -118,7 +94,7 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountAttrFlags>) -> Resu
}

// Ensure original symlink paths are available.
create_symlinks(&new_root, symlinks)?;
create_symlinks(&new_root, exceptions.symlinks)?;

// Bind mount old procfs.
let old_proc_c = CString::new("/proc").unwrap();
Expand Down Expand Up @@ -457,75 +433,3 @@ bitflags! {
const SYSVSEM = libc::CLONE_SYSVSEM;
}
}

// Copied from Rust's STD:
// https://github.com/rust-lang/rust/blob/42faef503f3e765120ca0ef06991337668eafc32/library/std/src/sys/unix/path.rs#L23C1-L63C2
//
// Licensed under MIT:
// https://github.com/rust-lang/rust/blob/master/LICENSE-MIT
//
/// Make a POSIX path absolute without changing its semantics.
fn absolute(path: &Path) -> io::Result<PathBuf> {
// This is mostly a wrapper around collecting `Path::components`, with
// exceptions made where this conflicts with the POSIX specification.
// See 4.13 Pathname Resolution, IEEE Std 1003.1-2017
// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13

// Get the components, skipping the redundant leading "." component if it
// exists.
let mut components = path.strip_prefix(".").unwrap_or(path).components();
let path_os = path.as_os_str().as_bytes();

let mut normalized = if path.is_absolute() {
// "If a pathname begins with two successive <slash> characters, the
// first component following the leading <slash> characters may be
// interpreted in an implementation-defined manner, although more than
// two leading <slash> characters shall be treated as a single <slash>
// character."
if path_os.starts_with(b"//") && !path_os.starts_with(b"///") {
components.next();
PathBuf::from("//")
} else {
PathBuf::new()
}
} else {
env::current_dir()?
};
normalized.extend(components);

// "Interfaces using pathname resolution may specify additional constraints
// when a pathname that does not name an existing directory contains at
// least one non- <slash> character and contains one or more trailing
// <slash> characters".
// A trailing <slash> is also meaningful if "a symbolic link is
// encountered during pathname resolution".
if path_os.ends_with(b"/") {
normalized.push("");
}

Ok(normalized)
}

/// Normalize path components, stripping out `.` and `..`.
fn normalize_path(path: &Path) -> PathBuf {
let mut normalized = PathBuf::new();

for component in path.components() {
match component {
Component::Prefix(_) => unreachable!("impl does not consider windows"),
Component::RootDir => normalized.push("/"),
Component::CurDir => continue,
Component::ParentDir => {
normalized.pop();
},
Component::Normal(segment) => normalized.push(segment),
}
}

normalized
}

/// Check if a path contains any symlinks.
fn path_has_symlinks(path: &Path) -> bool {
path.ancestors().any(|path| path.read_link().is_ok())
}
Loading

0 comments on commit 4efaab9

Please sign in to comment.