Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions crates/tinybox-linux/src/sandbox/args.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! Building the `bwrap` command line, and the cgroup wrapper around it.
//!
//! Pure functions, as with the Docker backend: what a sandbox binds, unshares,
//! and limits *is* the security boundary, so every one of those decisions
//! should be a value a test can assert on rather than something that only
//! happens at runtime.
//! Command construction, as with the Docker backend: what a sandbox binds,
//! unshares, and limits *is* the security boundary, so every one of those
//! decisions should be a value a test can assert on rather than something that
//! only happens at runtime. Runtime capability probes are kept separate from
//! the pure argument builders.

use std::path::Path;

use tinybox_core::{BoxSpec, Error, ExecRequest, Resources, Result, WorkspaceSource};
use tinybox_core::{BoxSpec, Capability, Error, ExecRequest, Resources, Result, WorkspaceSource};

use super::NAME;

Expand All @@ -21,6 +21,7 @@ pub const WORKSPACE_MOUNT: &str = "/workspace";
/// user can read; this exposes the system's programs and libraries and nothing
/// else.
const USR: &str = "/usr";
const SYSTEMD_EXPAND_ENVIRONMENT_FLAG: &[u8] = b"--expand-environment";

/// Files a normal command expects to exist.
///
Expand Down Expand Up @@ -72,15 +73,20 @@ fn unsupported(kind: &'static str) -> Error {
/// Returns [`Error::EmptyCommand`] when the request names no program, and
/// [`Error::UnsupportedWorkspaceSource`] when the spec names a source this
/// backend cannot bind.
pub(super) fn exec(spec: &BoxSpec, request: &ExecRequest, limits: bool) -> Result<Vec<String>> {
pub(super) fn exec(
spec: &BoxSpec,
request: &ExecRequest,
limits: bool,
supports_expand_environment: bool,
) -> Result<Vec<String>> {
if request.argv.is_empty() {
return Err(Error::EmptyCommand {
sandbox: NAME.to_owned(),
});
}

let mut argv = if limits {
cgroup_prefix(&spec.resources)
cgroup_prefix(&spec.resources, supports_expand_environment)
} else {
Vec::new()
};
Expand All @@ -95,8 +101,11 @@ pub(super) fn exec(spec: &BoxSpec, request: &ExecRequest, limits: bool) -> Resul
/// also why limits are opt-in: a machine with no systemd user session cannot
/// provide them, and a sandbox that accepted a memory cap it could not apply
/// would be claiming something untrue.
fn cgroup_prefix(resources: &Resources) -> Vec<String> {
vec![
pub(super) fn cgroup_prefix(
resources: &Resources,
supports_expand_environment: bool,
) -> Vec<String> {
let mut argv = vec![
"systemd-run".to_owned(),
"--user".to_owned(),
"--scope".to_owned(),
Expand All @@ -120,7 +129,34 @@ fn cgroup_prefix(resources: &Resources) -> Vec<String> {
"--property".to_owned(),
format!("TasksMax={}", resources.pids_max),
"--".to_owned(),
]
];
if supports_expand_environment {
// The shell receives the command as an argument, but newer systemd
// releases expand `${VAR}` references before the shell can interpret
// its own quoting. Keep arbitrary user commands byte-for-byte intact.
// This option was added in v254; older releases do not expand scope
// command arguments by default, so omitting it preserves compatibility.
argv.splice(3..3, ["--expand-environment=no".to_owned()]);
}
argv
}

pub(super) fn parse_expand_environment_help(
success: bool,
stdout: &[u8],
stderr: &[u8],
) -> Result<bool> {
if !success {
return Err(Error::Unsupported {
sandbox: NAME.to_owned(),
capability: Capability::ResourceLimits,
});
}
Ok([stdout, stderr].into_iter().any(|stream| {
stream
.windows(SYSTEMD_EXPAND_ENVIRONMENT_FLAG.len())
.any(|window| window == SYSTEMD_EXPAND_ENVIRONMENT_FLAG)
}))
}

/// The `bwrap` invocation itself.
Expand Down
16 changes: 15 additions & 1 deletion crates/tinybox-linux/src/sandbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,21 @@ impl Sandbox for NamespaceSandbox {
});
}

let argv = args::exec(&info.spec, request, self.limits)?;
let supports_expand_environment = if self.limits {
let probe = self
.host
.run(&ExecRequest::new(["systemd-run", "--help"]).with_env("LC_ALL", "C"))
.await?;
args::parse_expand_environment_help(probe.succeeded(), &probe.stdout, &probe.stderr)?
} else {
false
};
let argv = args::exec(
&info.spec,
request,
self.limits,
supports_expand_environment,
)?;
// A command that runs and fails is a result, so the raw output is
// returned rather than being turned into a backend error.
self.host.run(&ExecRequest::new(argv)).await
Expand Down
93 changes: 93 additions & 0 deletions crates/tinybox-linux/src/sandbox/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,37 @@ impl Host for RecordingHost {

async fn run(&self, request: &ExecRequest) -> Result<ExecOutput> {
self.seen().push(request.argv.clone());
if request.argv == ["systemd-run", "--help"] {
return Ok(ExecOutput::new(
0,
b"--expand-environment=BOOL".to_vec(),
Vec::new(),
));
}
Ok(ExecOutput::new(0, b"ran".to_vec(), Vec::new()))
}
}

#[derive(Debug, Default)]
struct FailedProbeHost {
seen: Mutex<Vec<Vec<String>>>,
}

#[async_trait]
impl Host for FailedProbeHost {
fn name(&self) -> &'static str {
"failed-probe"
}

async fn run(&self, request: &ExecRequest) -> Result<ExecOutput> {
self.seen
.lock()
.unwrap_or_else(PoisonError::into_inner)
.push(request.argv.clone());
Ok(ExecOutput::new(1, Vec::new(), b"probe failed".to_vec()))
}
}

fn sandbox() -> (NamespaceSandbox, Arc<RecordingHost>) {
let host = Arc::new(RecordingHost::default());
(
Expand Down Expand Up @@ -300,6 +327,7 @@ async fn limits_wrap_the_sandbox_in_a_systemd_scope() -> Result<()> {
assert_eq!(argv.first().map(String::as_str), Some("systemd-run"));
assert!(has(&argv, "--user"));
assert!(has(&argv, "--scope"));
assert!(has(&argv, "--expand-environment=no"));
let properties = argv
.iter()
.enumerate()
Expand All @@ -324,6 +352,71 @@ async fn limits_wrap_the_sandbox_in_a_systemd_scope() -> Result<()> {
Ok(())
}

#[test]
fn systemd_scope_environment_flag_is_added_only_when_supported() {
let resources = Resources {
cpu_millis: 1_000,
memory_bytes: 64 * 1024 * 1024,
pids_max: 16,
disk_bytes: 1024 * 1024,
};

let supported = args::cgroup_prefix(&resources, true);
assert!(has(&supported, "--expand-environment=no"));
assert_eq!(
supported.get(3).map(String::as_str),
Some("--expand-environment=no")
);

let legacy = args::cgroup_prefix(&resources, false);
assert!(!has(&legacy, "--expand-environment=no"));
assert_eq!(legacy.get(3).map(String::as_str), Some("--quiet"));
}

#[test]
fn systemd_environment_flag_probe_distinguishes_legacy_and_fails_closed() {
assert_eq!(
args::parse_expand_environment_help(true, b"--help", b""),
Ok(false)
);
assert_eq!(
args::parse_expand_environment_help(true, b"--expand-environment=BOOL", b""),
Ok(true)
);
assert_eq!(
args::parse_expand_environment_help(false, b"--expand-environment=BOOL", b""),
Err(Error::Unsupported {
sandbox: NAME.to_owned(),
capability: tinybox_core::Capability::ResourceLimits,
})
);
}

#[tokio::test]
async fn failed_systemd_option_probe_refuses_without_launching_workload() -> Result<()> {
let host = Arc::new(FailedProbeHost::default());
let sandbox =
NamespaceSandbox::new(host.clone(), Arc::new(MemoryStore::new())).with_cgroup_limits();
let info = sandbox.create(&spec()?).await?;

let result = sandbox
.exec(&info.id, &ExecRequest::new(["echo", "must not run"]))
.await;

assert_eq!(
result,
Err(Error::Unsupported {
sandbox: NAME.to_owned(),
capability: Capability::ResourceLimits,
})
);
assert_eq!(
*host.seen.lock().unwrap_or_else(PoisonError::into_inner),
[vec!["systemd-run".to_owned(), "--help".to_owned()]]
);
Ok(())
}

#[tokio::test]
async fn without_limits_there_is_no_scope_at_all() -> Result<()> {
let (sandbox, host) = sandbox();
Expand Down
28 changes: 28 additions & 0 deletions crates/tinybox-linux/tests/live_namespaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,34 @@ async fn live_a_memory_limit_is_actually_enforced() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn live_systemd_scope_preserves_shell_dollar_literals() -> Result<()> {
if !enabled() {
return Ok(());
}
let dir = workspace()?;
let sandbox = NamespaceSandbox::new(Arc::new(LocalHost::new()), Arc::new(MemoryStore::new()))
.with_cgroup_limits();
let info = sandbox.create(&spec(dir.path())?).await?;
let output = sandbox
.exec(
&info.id,
&ExecRequest::new(["/bin/sh", "-c", "printf '%s\\n' '${cgroup_path}'"]),
)
.await?;
sandbox.destroy(&info.id).await?;

if !output.succeeded() {
return Err(Error::Backend {
sandbox: "namespace".to_owned(),
operation: "preserve a literal dollar reference in a systemd scope",
message: output.stderr_lossy().trim().to_owned(),
});
}
assert_eq!(output.stdout_lossy(), "${cgroup_path}\n");
Ok(())
}

#[tokio::test]
async fn live_a_box_is_only_a_record_so_destroying_it_leaves_the_workspace() -> Result<()> {
if !enabled() {
Expand Down
Loading