From 826e262ac145fc40899fd6100dddbcde75b93cf3 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Thu, 21 Sep 2023 19:48:44 +1000 Subject: [PATCH] Add support for running rustc in a sandbox --- CONFIG.md | 16 +++++- src/config.rs | 10 ++++ src/crate_index.rs | 2 +- src/main.rs | 3 ++ src/proxy.rs | 14 ++++- src/proxy/subprocess.rs | 15 +++++- src/sandbox.rs | 104 ++++++++++++++++++++++++++++++++------ src/sandbox/bubblewrap.rs | 2 +- test_crates/cackle.toml | 3 ++ 9 files changed, 147 insertions(+), 22 deletions(-) diff --git a/CONFIG.md b/CONFIG.md index 3ae5f9b..ee45bd5 100644 --- a/CONFIG.md +++ b/CONFIG.md @@ -103,7 +103,7 @@ kind = "Bubblewrap" Here we declare that we'd like to use `Bubblewrap` (installed as `bwrap`) as our sandbox. Bubblewrap is currently the only supported kind of sandbox. The sandbox will be used for running build scripts -(build.rs). +(build.rs), running tests (with `cargo acl test`) and optionally for sandboxing rustc. If for some reason you don't want to sandbox a particular build script, you can disable the sandbox just for that build script. @@ -159,6 +159,20 @@ test.sandbox.make_writable = [ ] ``` +### Sandboxing rustc + +Currently rustc isn't sandboxed by default. You can enable it as follows: + +```toml +[rustc.sandbox] +kind = "Bubblewrap" +``` + +Sandboxing rustc will mean that all proc macros get sandboxed. Controlling the sandbox on a +per-proc-macro basis unfortunately isn't supported yet, but hopefully will in future. This means +that if you have for example one proc macro that needs network access, you'd need to enable network +access for the whole rustc sandbox, which means that all proc macros would have network access. + ## Importing API definitions from an external crate If you depend on a crate that publishes `cackle/export.toml`, you can import API definitions from diff --git a/src/config.rs b/src/config.rs index f904e1c..f476383 100644 --- a/src/config.rs +++ b/src/config.rs @@ -47,6 +47,9 @@ pub(crate) struct RawConfig { #[serde(default)] pub(crate) sandbox: SandboxConfig, + + #[serde(default)] + pub(crate) rustc: RustcConfig, } /// The name of a package. Doesn't include any version information. @@ -93,6 +96,13 @@ pub(crate) struct SandboxConfig { pub(crate) make_writable: Vec, } +#[derive(Deserialize, Serialize, Debug, Default, Clone, PartialEq, Eq, Hash)] +#[serde(deny_unknown_fields)] +pub(crate) struct RustcConfig { + #[serde(default)] + pub(crate) sandbox: SandboxConfig, +} + #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq, Default, Hash)] #[serde(deny_unknown_fields)] pub(crate) struct ApiConfig { diff --git a/src/crate_index.rs b/src/crate_index.rs index c508dc5..8ea1187 100644 --- a/src/crate_index.rs +++ b/src/crate_index.rs @@ -70,7 +70,7 @@ pub(crate) struct PackageInfo { /// metadata. Subprocesses need to know which packages are non-unique so that they can correctly /// form PackageIds, which need this information so that we can only print package versions when /// there are multiple versions of that package. -const MULTIPLE_VERSION_PKG_NAMES_ENV: &str = "CACKLE_MULTIPLE_VERSION_PKG_NAMES"; +pub(crate) const MULTIPLE_VERSION_PKG_NAMES_ENV: &str = "CACKLE_MULTIPLE_VERSION_PKG_NAMES"; impl CrateIndex { pub(crate) fn new(dir: &Path) -> Result { diff --git a/src/main.rs b/src/main.rs index 2ce2afa..0850a45 100644 --- a/src/main.rs +++ b/src/main.rs @@ -207,6 +207,7 @@ struct Cackle { config_path: PathBuf, checker: Arc>, tmpdir: Arc, + target_dir: PathBuf, args: Arc, event_sender: Sender, ui_join_handle: JoinHandle>, @@ -264,6 +265,7 @@ impl Cackle { ui_join_handle, crate_index, tmpdir, + target_dir, abort_sender, cargo_output_waiter: None, }) @@ -367,6 +369,7 @@ impl Cackle { let cargo_runner = proxy::CargoRunner { manifest_dir: &root_path, tmpdir: self.tmpdir.path(), + target_dir: &self.target_dir, config: &config, args: &args, crate_index: &crate_index, diff --git a/src/proxy.rs b/src/proxy.rs index 6b658ff..4edf27d 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -49,9 +49,18 @@ pub(crate) mod errors; pub(crate) mod rpc; pub(crate) mod subprocess; -const SOCKET_ENV: &str = "CACKLE_SOCKET_PATH"; +pub(crate) const SOCKET_ENV: &str = "CACKLE_SOCKET_PATH"; const CONFIG_PATH_ENV: &str = "CACKLE_CONFIG_PATH"; const ORIG_LINKER_ENV: &str = "CACKLE_ORIG_LINKER"; +pub(crate) const TARGET_DIR: &str = "CACKLE_TARGET_DIR"; +pub(crate) const MANIFEST_DIR: &str = "CACKLE_MANIFEST_DIR"; + +/// Environment variables that we need to allow through to rustc when we run rustc in a sandbox. +pub(crate) const RUSTC_ENV_VARS: &[&str] = &[ + SOCKET_ENV, + CONFIG_PATH_ENV, + crate::crate_index::MULTIPLE_VERSION_PKG_NAMES_ENV, +]; pub(crate) struct CargoRunner<'a> { pub(crate) manifest_dir: &'a Path, @@ -59,6 +68,7 @@ pub(crate) struct CargoRunner<'a> { pub(crate) config: &'a Config, pub(crate) args: &'a Args, pub(crate) crate_index: &'a CrateIndex, + pub(crate) target_dir: &'a Path, } #[derive(Default)] @@ -126,6 +136,8 @@ impl<'a> CargoRunner<'a> { command .env(SOCKET_ENV, &ipc_path) .env(CONFIG_PATH_ENV, config_path) + .env(TARGET_DIR, self.target_dir) + .env(MANIFEST_DIR, self.manifest_dir) .env("RUSTC_WRAPPER", cackle_exe()?); self.crate_index.add_internal_env(&mut command); diff --git a/src/proxy/subprocess.rs b/src/proxy/subprocess.rs index b76a46a..fd3c52f 100644 --- a/src/proxy/subprocess.rs +++ b/src/proxy/subprocess.rs @@ -11,12 +11,14 @@ use super::CONFIG_PATH_ENV; use crate::config::permissions::PermSel; use crate::config::permissions::Permissions; use crate::config::Config; +use crate::config::RustcConfig; use crate::crate_index::CrateKind; use crate::crate_index::CrateSel; use crate::link_info::LinkInfo; use crate::location::SourceLocation; use crate::outcome::Outcome; use crate::proxy::rpc::RpcClient; +use crate::sandbox::RustcSandboxInputs; use crate::unsafe_checker; use anyhow::anyhow; use anyhow::bail; @@ -140,7 +142,7 @@ fn proxy_binary( let config = SubprocessConfig::from_env()?; let perm_sel = PermSel::for_non_build_output(crate_sel); let sandbox_config = config.permissions.sandbox_config_for_package(&perm_sel); - let Some(sandbox) = crate::sandbox::from_config(&sandbox_config, &orig_bin, &perm_sel)? + let Some(sandbox) = crate::sandbox::for_perm_sel(&sandbox_config, &orig_bin, &perm_sel)? else { // Config says to run without a sandbox. return Ok(Command::new(&orig_bin).status()?.into()); @@ -227,7 +229,14 @@ impl RustcRunner { .permissions .unsafe_permitted_for_crate(&self.crate_sel); let mut command = self.get_command(unsafe_permitted)?; - let output = command.output()?; + let output = + match crate::sandbox::for_rustc(&config.rustc, &RustcSandboxInputs::from_env()?)? { + Some(mut sandbox) => { + sandbox.ro_bind(&cackle_exe()?); + sandbox.run(&command)? + } + None => command.output()?, + }; let mut unsafe_locations = Vec::new(); if output.status.code() == Some(0) { @@ -363,6 +372,7 @@ fn default_linker() -> String { #[derive(Deserialize, Serialize, PartialEq, Eq, Debug)] pub(crate) struct SubprocessConfig { permissions: Permissions, + rustc: RustcConfig, } impl SubprocessConfig { @@ -376,6 +386,7 @@ impl SubprocessConfig { pub(crate) fn from_full_config(full_config: &Config) -> Self { Self { permissions: full_config.permissions.clone(), + rustc: full_config.raw.rustc.clone(), } } diff --git a/src/sandbox.rs b/src/sandbox.rs index 423afb4..ba553ec 100644 --- a/src/sandbox.rs +++ b/src/sandbox.rs @@ -1,4 +1,5 @@ use crate::config::permissions::PermSel; +use crate::config::RustcConfig; use crate::config::SandboxConfig; use crate::config::SandboxKind; use anyhow::bail; @@ -56,15 +57,12 @@ pub(crate) trait Sandbox { fn display_to_run(&self, command: &Command) -> Box; } -pub(crate) fn from_config( - config: &SandboxConfig, - bin_path: &Path, - perm_sel: &PermSel, -) -> Result>> { +pub(crate) fn from_config(config: &SandboxConfig) -> Result>> { let mut sandbox = match &config.kind { None | Some(SandboxKind::Disabled) => return Ok(None), Some(SandboxKind::Bubblewrap) => Box::::default(), }; + let home = PathBuf::from(std::env::var("HOME").context("Couldn't get HOME env var")?); // We allow access to the root of the filesystem, but only selected parts of the user's home // directory. The home directory is where sensitive stuff is most likely to live. e.g. access @@ -86,15 +84,7 @@ pub(crate) fn from_config( // Allow read access to the crate's root source directory. sandbox.ro_bind(Path::new(&get_env("CARGO_MANIFEST_DIR")?)); - // Allow read access to the build directory. This contains the bin file being executed and - // possibly other binaries. - if let Some(build_dir) = build_directory(bin_path) { - sandbox.ro_bind(build_dir); - } - // Allow write access to OUT_DIR. - if let Ok(out_dir) = std::env::var("OUT_DIR") { - sandbox.writable_bind(Path::new(&out_dir)); - } + // LD_LIBRARY_PATH is set when running `cargo test` on crates that normally compile as // cdylibs - e.g. proc macros. If we don't pass it through, those tests will fail to find // runtime dependencies. @@ -104,13 +94,13 @@ pub(crate) fn from_config( for dir in &config.bind_writable { if !dir.exists() { bail!( - "Sandbox config for `{perm_sel}` says to bind directory `{}`, but that doesn't exist", + "Sandbox config says to bind directory `{}`, but that doesn't exist", dir.display() ); } if !dir.is_dir() { bail!( - "Sandbox config for `{perm_sel}` says to bind directory `{}`, but that isn't a directory", + "Sandbox config says to bind directory `{}`, but that isn't a directory", dir.display() ); } @@ -132,6 +122,88 @@ pub(crate) fn from_config( // permitted prevents DNS lookups on some systems. sandbox.tmpfs(Path::new("/run")); } + + Ok(Some(sandbox)) +} + +/// Information extracted from the rustc command line that's relevant to running it in a sandbox. +#[derive(Default)] +pub(crate) struct RustcSandboxInputs { + output_directories: Vec, + input_directories: Vec, +} + +impl RustcSandboxInputs { + pub(crate) fn from_env() -> Result { + let mut result = Self::default(); + let mut next_is_out = false; + let target_dir = PathBuf::from(get_env(crate::proxy::TARGET_DIR)?); + let manifest_dir = PathBuf::from(get_env(crate::proxy::MANIFEST_DIR)?); + result.input_directories.push(manifest_dir); + for arg in std::env::args() { + if next_is_out { + result.output_directories.push(arg.into()); + next_is_out = false; + continue; + } + next_is_out = arg == "--out-dir"; + if let Some(rest) = arg.strip_prefix("incremental=") { + result.output_directories.push(rest.into()); + } + } + if let Ok(socket_path) = std::env::var(crate::proxy::SOCKET_ENV) { + if let Some(dir) = Path::new(&socket_path).parent() { + result.output_directories.push(dir.to_owned()); + } + } + result + .output_directories + .retain(|d| !d.starts_with(&target_dir)); + result.output_directories.push(target_dir); + Ok(result) + } +} + +pub(crate) fn for_rustc( + config: &RustcConfig, + inputs: &RustcSandboxInputs, +) -> Result>> { + let Some(mut sandbox) = from_config(&config.sandbox)? else { + return Ok(None); + }; + for dir in &inputs.input_directories { + sandbox.ro_bind(dir); + } + for dir in &inputs.output_directories { + sandbox.writable_bind(dir); + } + for env in crate::proxy::RUSTC_ENV_VARS { + sandbox.pass_env(env); + } + Ok(Some(sandbox)) +} + +pub(crate) fn for_perm_sel( + config: &SandboxConfig, + bin_path: &Path, + perm_sel: &PermSel, +) -> Result>> { + let Some(mut sandbox) = from_config(config) + .with_context(|| format!("Failed to build sandbox config for `{perm_sel}`"))? + else { + return Ok(None); + }; + + // Allow read access to the build directory. This contains the bin file being executed and + // possibly other binaries. + if let Some(build_dir) = build_directory(bin_path) { + sandbox.ro_bind(build_dir); + } + // Allow write access to OUT_DIR. + if let Ok(out_dir) = std::env::var("OUT_DIR") { + sandbox.writable_bind(Path::new(&out_dir)); + } + Ok(Some(sandbox)) } diff --git a/src/sandbox/bubblewrap.rs b/src/sandbox/bubblewrap.rs index cd2d53a..92a7d39 100644 --- a/src/sandbox/bubblewrap.rs +++ b/src/sandbox/bubblewrap.rs @@ -113,7 +113,7 @@ impl Display for CommandDisplay { write!(f, "{}", self.command.get_program().to_string_lossy())?; for arg in self.command.get_args() { let arg = arg.to_string_lossy(); - if arg.contains(' ') || arg.is_empty() { + if arg.contains(' ') || arg.contains('"') || arg.is_empty() { // Use debug print, since that gives us quotes. write!(f, " {:?}", arg)?; } else { diff --git a/test_crates/cackle.toml b/test_crates/cackle.toml index 51450b2..3169f94 100644 --- a/test_crates/cackle.toml +++ b/test_crates/cackle.toml @@ -16,6 +16,9 @@ import_std = [ [sandbox] kind = "Bubblewrap" +[rustc.sandbox] +kind = "Bubblewrap" + # Restrict access to an entire crate. [api.res1] include = [