Revert "crosvm: Embed seccomp filters into binary"

This reverts commit a78f92cdb2.

Reason for revert: doesn't build on Chrome OS

Original change's description:
> crosvm: Embed seccomp filters into binary
>
> Seccomp policy files will now pre-compile to bpf bytecode for
> target architecture and embedded in the crosvm binary.
> When minijail is not checked out in crosvm tree as a submodule,
> MINIJAIL_DIR environment variable needs to be specified for the
> policy compiler to run.
>
> TEST=all tests passed, vm runs fine with sandbox on and no separate
> policy files present.
> BUG=b:235858187
>
> Change-Id: Ia801966df0a8adfdc4a80f5899e33121fe45e5f9
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3774318
> Reviewed-by: Dennis Kempin <denniskempin@google.com>
> Commit-Queue: Zihan Chen <zihanchen@google.com>
> Tested-by: Zihan Chen <zihanchen@google.com>

Bug: b:235858187
Change-Id: Ia81e43185d5f16bd061b6d0290befb4642c44548
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3813056
Tested-by: Junichi Uekawa <uekawa@chromium.org>
Commit-Queue: Junichi Uekawa <uekawa@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>

Cargo.toml

@@ -229,9 +229,6 @@ tube_transporter = { path = "tube_transporter" }
 winapi = "*"
 win_util = { path = "win_util"}
 
-[build-dependencies]
-cc = "*"
-
 [dev-dependencies]
 base = "*"
 

build.rs

@@ -1,158 +0,0 @@
-// Copyright 2022 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#[cfg(unix)]
-use std::env;
-use std::fs;
-use std::path::Path;
-#[cfg(unix)]
-use std::path::PathBuf;
-use std::process::Command;
-
-fn generate_preprocessed(minijail_dir: &Path, out_dir: &Path) {
-    let env_cc = cc::Build::new()
-        .get_compiler()
-        .path()
-        .as_os_str()
-        .to_owned();
-
-    Command::new(minijail_dir.join("gen_constants.sh"))
-        .env("CC", &env_cc)
-        .env("SRC", &minijail_dir)
-        .arg(out_dir.join("libconstants.gen.c"))
-        .spawn()
-        .unwrap()
-        .wait()
-        .expect("Generate kernel constant table failed");
-
-    Command::new(minijail_dir.join("gen_syscalls.sh"))
-        .env("CC", &env_cc)
-        .env("SRC", &minijail_dir)
-        .arg(out_dir.join("libsyscalls.gen.c"))
-        .spawn()
-        .unwrap()
-        .wait()
-        .expect("Generate syscall table failed");
-}
-
-fn generate_llvm_ir(minijail_dir: &Path, out_dir: &Path, target: &str) {
-    Command::new("clang")
-        .arg("-target")
-        .arg(target)
-        .arg("-S")
-        .arg("-emit-llvm")
-        .arg("-I")
-        .arg(minijail_dir)
-        .arg(out_dir.join("libconstants.gen.c"))
-        .arg(out_dir.join("libsyscalls.gen.c"))
-        .current_dir(&out_dir)
-        .spawn()
-        .unwrap()
-        .wait()
-        .expect("Convert kernel constants and syscalls to llvm ir failed");
-}
-
-fn generate_constants_json(minijail_dir: &Path, out_dir: &Path) {
-    Command::new(minijail_dir.join("tools/generate_constants_json.py"))
-        .arg("--output")
-        .arg(out_dir.join("constants.json"))
-        .arg(out_dir.join("libconstants.gen.ll"))
-        .arg(out_dir.join("libsyscalls.gen.ll"))
-        .spawn()
-        .unwrap()
-        .wait()
-        .expect("Generate constants.json failed");
-}
-
-fn rewrite_policies(seccomp_policy_path: &Path, rewrote_policy_folder: &Path) {
-    for entry in fs::read_dir(seccomp_policy_path).unwrap() {
-        let policy_file = entry.unwrap();
-        let policy_file_content = fs::read_to_string(policy_file.path()).unwrap();
-        let policy_file_content_rewrote =
-            policy_file_content.replace("/usr/share/policy/crosvm", ".");
-        fs::write(
-            rewrote_policy_folder.join(policy_file.file_name()),
-            policy_file_content_rewrote,
-        )
-        .unwrap();
-    }
-}
-
-fn compile_policies(out_dir: &Path, rewrote_policy_folder: &Path, minijail_dir: &Path) {
-    let compiled_policy_folder = out_dir.join("policy_output");
-    fs::create_dir_all(&compiled_policy_folder).unwrap();
-    let mut include_all_bytes = String::from("std::collections::HashMap::from([\n");
-    for entry in fs::read_dir(&rewrote_policy_folder).unwrap() {
-        let policy_file = entry.unwrap();
-        if policy_file.path().extension().unwrap() == "policy" {
-            let output_file_path = compiled_policy_folder.join(
-                policy_file
-                    .path()
-                    .with_extension("bpf")
-                    .file_name()
-                    .unwrap(),
-            );
-            Command::new(minijail_dir.join("tools/compile_seccomp_policy.py"))
-                .arg("--arch-json")
-                .arg(out_dir.join("constants.json"))
-                .arg("--default-action")
-                .arg("trap")
-                .arg(policy_file.path())
-                .arg(&output_file_path)
-                .spawn()
-                .unwrap()
-                .wait()
-                .expect("Compile bpf failed");
-            let s = format!(
-                r#"("{}", include_bytes!("{}").to_vec()),"#,
-                policy_file.path().file_stem().unwrap().to_str().unwrap(),
-                output_file_path.to_str().unwrap()
-            );
-            include_all_bytes += s.as_str();
-        }
-    }
-    include_all_bytes += "])";
-    fs::write(out_dir.join("bpf_includes.in"), include_all_bytes).unwrap();
-}
-
-#[cfg(unix)]
-fn main() {
-    println!("cargo:rerun-if-changed=build.rs");
-    println!("cargo:rerun-if-changed=seccomp");
-
-    let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
-    let src_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
-    let minijail_dir = if let Ok(minijail_dir_env) = env::var("MINIJAIL_DIR") {
-        PathBuf::from(minijail_dir_env)
-    } else {
-        src_dir.join("third_party/minijail")
-    };
-
-    let target = env::var("TARGET").unwrap();
-
-    generate_preprocessed(&minijail_dir, &out_dir);
-    generate_llvm_ir(&minijail_dir, &out_dir, &target);
-    generate_constants_json(&minijail_dir, &out_dir);
-
-    // check policies exist for target architecuture
-    let seccomp_arch_name = match env::var("CARGO_CFG_TARGET_ARCH").unwrap().as_str() {
-        "armv7" => "arm".to_owned(),
-        x => x.to_owned(),
-    };
-    let seccomp_policy_path = src_dir.join("seccomp").join(&seccomp_arch_name);
-    assert!(
-        seccomp_policy_path.is_dir(),
-        "Seccomp policy dir doesn't exist"
-    );
-
-    let rewrote_policy_folder = out_dir.join("policy_input");
-    fs::create_dir_all(&rewrote_policy_folder).unwrap();
-    rewrite_policies(&seccomp_policy_path, &rewrote_policy_folder);
-    compile_policies(&out_dir, &rewrote_policy_folder, &minijail_dir);
-}
-
-#[cfg(not(unix))]
-fn main() {
-    println!("cargo:rerun-if-changed=build.rs");
-}

docs/book/src/building_crosvm.md

@@ -172,13 +172,19 @@ The `--quick` variant will skip some slower checks, like building for other plat
 
 ## Known issues
 
+- By default, crosvm is running devices in sandboxed mode, which requires seccomp policy files to be
+  set up. For local testing it is often easier to `--disable-sandbox` to run everything in a single
+  process.
 - If your Linux header files are too old, you may find minijail rejecting seccomp filters for
-  containing unknown syscalls. You can try removing the offending lines from the filter file and
+  containing unknown syscalls. You can try removing the offending lines from the filter file, or add
-  recompile or add `--seccomp-log-failures` to the crosvm command line to turn these into warnings.
+  `--seccomp-log-failures` to the crosvm command line to turn these into warnings. Note that this
-  Using this option also requires you to specify path to seccomp policiy source files with
+  option will also stop minijail from killing processes that violate the seccomp rule, making the
-  `--seccomp-policy-dir` and adhere to (or modify) the hardcoded absolute include paths in them.
+  sandboxing much less aggressive.
-  Note that this option will also stop minijail from killing processes that violate the seccomp
+- Seccomp policy files have hardcoded absolute paths. You can either fix up the paths locally, or
-  rule, making the sandboxing much less aggressive.
+  set up an awesome hacky symlink:
+  `sudo mkdir /usr/share/policy && sudo ln -s /path/to/crosvm/seccomp/x86_64 /usr/share/policy/crosvm`.
+  We'll eventually build the precompiled policies
+  [into the crosvm binary](http://crbug.com/1052126).
 - Devices can't be jailed if `/var/empty` doesn't exist. `sudo mkdir -p /var/empty` to work around
   this for now.
 - You need read/write permissions for `/dev/kvm` to run tests or other crosvm instances. Usually

src/crosvm/config.rs

@@ -66,6 +66,7 @@ cfg_if::cfg_if! {
 
         static KVM_PATH: &str = "/dev/kvm";
         static VHOST_NET_PATH: &str = "/dev/vhost-net";
+        static SECCOMP_POLICY_DIR: &str = "/usr/share/policy/crosvm";
     } else if #[cfg(windows)] {
         use base::{Event, Tube};
 
@@ -525,12 +526,18 @@ fn jail_config_default_pivot_root() -> PathBuf {
     PathBuf::from(option_env!("DEFAULT_PIVOT_ROOT").unwrap_or("/var/empty"))
 }
 
+#[cfg(unix)]
+fn jail_config_default_seccomp_policy_dir() -> Option<PathBuf> {
+    Some(PathBuf::from(SECCOMP_POLICY_DIR))
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, serde_keyvalue::FromKeyValues)]
 #[serde(deny_unknown_fields, rename_all = "kebab-case")]
 pub struct JailConfig {
     #[serde(default = "jail_config_default_pivot_root")]
     pub pivot_root: PathBuf,
     #[cfg(unix)]
+    #[serde(default = "jail_config_default_seccomp_policy_dir")]
     pub seccomp_policy_dir: Option<PathBuf>,
     #[serde(default)]
     pub seccomp_log_failures: bool,
@@ -541,7 +548,7 @@ impl Default for JailConfig {
         JailConfig {
             pivot_root: jail_config_default_pivot_root(),
             #[cfg(unix)]
-            seccomp_policy_dir: None,
+            seccomp_policy_dir: jail_config_default_seccomp_policy_dir(),
             seccomp_log_failures: false,
         }
     }
@@ -2189,7 +2196,7 @@ mod tests {
             JailConfig {
                 pivot_root: jail_config_default_pivot_root(),
                 #[cfg(unix)]
-                seccomp_policy_dir: None,
+                seccomp_policy_dir: jail_config_default_seccomp_policy_dir(),
                 seccomp_log_failures: false,
             }
         );

src/crosvm/sys/unix/jail_helpers.rs

@@ -13,18 +13,10 @@ use libc::c_ulong;
 use libc::gid_t;
 use libc::uid_t;
 use minijail::Minijail;
-use once_cell::sync::Lazy;
 
 use crate::crosvm::config::JailConfig;
 
-static EMBEDDED_BPFS: Lazy<std::collections::HashMap<&str, Vec<u8>>> = Lazy::new(|| {
+#[allow(dead_code)]
-    if cfg!(unix) {
-        include!(concat!(env!("OUT_DIR"), "/bpf_includes.in"))
-    } else {
-        std::collections::HashMap::<&str, Vec<u8>>::new()
-    }
-});
-
 pub(super) struct SandboxConfig<'a> {
     pub(super) limit_caps: bool,
     pub(super) log_failures: bool,
@@ -75,55 +67,27 @@ pub(super) fn create_base_minijail(
         // Don't allow the device to gain new privileges.
         j.no_new_privs();
 
-        if let Some(seccomp_policy_path) = config.seccomp_policy_path {
+        // By default we'll prioritize using the pre-compiled .bpf over the .policy
-            // By default we'll prioritize using the pre-compiled .bpf over the
+        // file (the .bpf is expected to be compiled using "trap" as the failure
-            // .policy file (the .bpf is expected to be compiled using "trap" as the
+        // behavior instead of the default "kill" behavior).
-            // failure behavior instead of the default "kill" behavior) when a policy
+        // Refer to the code comment for the "seccomp-log-failures"
-            // path is supplied in the command line arugments. Otherwise the built-in
+        // command-line parameter for an explanation about why the |log_failures|
-            // pre-compiled policies will be used.
+        // flag forces the use of .policy files (and the build-time alternative to
-            // Refer to the code comment for the "seccomp-log-failures"
+        // this run-time flag).
-            // command-line parameter for an explanation about why the |log_failures|
+        let bpf_policy_file = config.seccomp_policy_path.unwrap().with_extension("bpf");
-            // flag forces the use of .policy files (and the build-time alternative to
+        if bpf_policy_file.exists() && !config.log_failures {
-            // this run-time flag).
+            j.parse_seccomp_program(&bpf_policy_file)
-            let bpf_policy_file = seccomp_policy_path.with_extension("bpf");
+                .context("failed to parse precompiled seccomp policy")?;
-            if bpf_policy_file.exists() && !config.log_failures {
-                j.parse_seccomp_program(&bpf_policy_file).with_context(|| {
-                    format!(
-                        "failed to parse precompiled seccomp policy: {}",
-                        bpf_policy_file.display()
-                    )
-                })?;
-            } else {
-                // Use TSYNC only for the side effect of it using SECCOMP_RET_TRAP,
-                // which will correctly kill the entire device process if a worker
-                // thread commits a seccomp violation.
-                j.set_seccomp_filter_tsync();
-                if config.log_failures {
-                    j.log_seccomp_filter_failures();
-                }
-                let bpf_policy_file = seccomp_policy_path.with_extension("policy");
-                j.parse_seccomp_filters(&bpf_policy_file).with_context(|| {
-                    format!(
-                        "failed to parse seccomp policy: {}",
-                        bpf_policy_file.display()
-                    )
-                })?;
-            }
         } else {
-            let bpf_program = EMBEDDED_BPFS
+            // Use TSYNC only for the side effect of it using SECCOMP_RET_TRAP,
-                .get(&config.seccomp_policy_name)
+            // which will correctly kill the entire device process if a worker
-                .with_context(|| {
+            // thread commits a seccomp violation.
-                    format!(
+            j.set_seccomp_filter_tsync();
-                        "failed to find embedded seccomp policy: {}",
+            if config.log_failures {
-                        &config.seccomp_policy_name
+                j.log_seccomp_filter_failures();
-                    )
+            }
-                })?;
+            j.parse_seccomp_filters(&config.seccomp_policy_path.unwrap().with_extension("policy"))
-            j.parse_seccomp_bytes(bpf_program).with_context(|| {
+                .context("failed to parse seccomp policy")?;
-                format!(
-                    "failed to parse embedded seccomp policy: {}",
-                    &config.seccomp_policy_name
-                )
-            })?;
         }
         j.use_seccomp_filter();
         // Don't do init setup.