Skip to content

Commit

Permalink
Limit permissions for Android images.
Browse files Browse the repository at this point in the history
Remove the use of the `--privileged` flag for Android images and instead use an seccomp permissions. The provided profile is derived from the docker documentation, with slight modifications to allow `clone` and `clone3`.

The documentation is [docker seccomp](https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile), which details the syscalls blocked by docker. The same is true for podman. We merely modified these settings to allow `personality` syscall, which then allows us to use our Android images.

On Windows with Docker Desktop, we currently have an issue where Docker tries to read the seccomp profile, and then interpret that as the path, rather than load the profile from the path, which is tracked by the following issue:
docker/for-win#12760

On Podman (not inside WSL2), we have a separate issue where it expects a WSL path to be provided for the seccomp profile, despite the path being provided for the host.
  • Loading branch information
Alexhuszagh committed Jun 8, 2022
1 parent 94adafe commit 61c4bb7
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

- #762 - re-enabled `x86_64-unknown-dragonfly` target.
- #747 - reduced android image sizes.
- #746 - limit image permissions for android images.
- #377 - update WINE versions to 7.0.
- #734 - patch `arm-unknown-linux-gnueabihf` to build for ARMv6, and add architecture for crosstool-ng-based images.
- #709 - Update Emscripten targets to `emcc` version 3.1.10
Expand Down
45 changes: 40 additions & 5 deletions src/docker.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::io::Write;
use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus};
use std::{env, fs};

use crate::cargo::CargoMetadata;
use crate::extensions::{CommandExt, SafeCommand};
use crate::file::write_file;
use crate::id;
use crate::{errors::*, file};
use crate::{Config, Target};
Expand All @@ -14,16 +16,23 @@ const DOCKER_IMAGES: &[&str] = &include!(concat!(env!("OUT_DIR"), "/docker-image
const CROSS_IMAGE: &str = "ghcr.io/cross-rs";
const DOCKER: &str = "docker";
const PODMAN: &str = "podman";
// secured profile based off the docker documentation for denied syscalls:
// https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile
// note that we've allow listed `clone` and `clone3`, which is necessary
// to fork the process, and which podman allows by default.
const SECCOMP: &str = include_str!("seccomp.json");

// determine if the container engine is docker. this fixes issues with
// any aliases (#530), and doesn't fail if an executable suffix exists.
fn get_is_docker(ce: std::path::PathBuf, verbose: bool) -> Result<bool> {
fn get_engine_type(ce: std::path::PathBuf, verbose: bool) -> Result<(bool, bool)> {
let stdout = Command::new(ce)
.arg("--help")
.run_and_get_stdout(verbose)?
.to_lowercase();

Ok(stdout.contains("docker") && !stdout.contains("emulate"))
let is_docker = stdout.contains("docker") && !stdout.contains("emulate");
let is_podman = stdout.contains("podman");
Ok((is_docker, is_podman))
}

fn get_container_engine() -> Result<std::path::PathBuf, which::Error> {
Expand Down Expand Up @@ -172,7 +181,8 @@ pub fn run(
let runner = config.runner(target)?;

let mut docker = docker_command("run")?;
let is_docker = get_is_docker(get_container_engine().unwrap(), verbose)?;
#[allow(unused_variables)] // is_podman, target_os = "windows"
let (is_docker, is_podman) = get_engine_type(get_container_engine().unwrap(), verbose)?;

for ref var in config.env_passthrough(target)? {
validate_env_var(var)?;
Expand Down Expand Up @@ -233,8 +243,33 @@ pub fn run(

docker.arg("--rm");

if target.needs_docker_privileged() {
docker.arg("--privileged");
// docker uses seccomp now on all installations
if target.needs_docker_seccomp() {
let seccomp = if is_docker && cfg!(target_os = "windows") {
// docker on windows fails due to a bug in reading the profile
// https://github.com/docker/for-win/issues/12760
"unconfined".to_string()
} else {
#[allow(unused_mut)] // target_os = "windows"
let mut path = env::current_dir()
.wrap_err("couldn't get current directory")?
.canonicalize()
.wrap_err_with(|| "when canonicalizing current_dir".to_string())?
.join("target")
.join(target.triple())
.join("seccomp.json");
if !path.exists() {
write_file(&path, false)?.write_all(SECCOMP.as_bytes())?;
}
#[cfg(target_os = "windows")]
if is_podman {
// podman weirdly expects a WSL path here, and fails otherwise
path = wslpath(&path, verbose)?;
}
path.display().to_string()
};

docker.args(&["--security-opt", &format!("seccomp={}", seccomp)]);
}

// We need to specify the user for Docker, but not for Podman.
Expand Down
17 changes: 16 additions & 1 deletion src/file.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fs::File;
use std::fs::{self, File};
use std::io::Read;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -35,3 +35,18 @@ fn _canonicalize(path: &Path) -> Result<PathBuf> {
Path::new(&path).canonicalize().map_err(Into::into)
}
}

pub fn write_file(path: impl AsRef<Path>, overwrite: bool) -> Result<File> {
let path = path.as_ref();
fs::create_dir_all(
&path.parent().ok_or_else(|| {
eyre::eyre!("could not find parent directory for `{}`", path.display())
})?,
)
.wrap_err_with(|| format!("couldn't create directory `{}`", path.display()))?;
fs::OpenOptions::new()
.write(true)
.create_new(!overwrite)
.open(&path)
.wrap_err(format!("could't write to file `{}`", path.display()))
}
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl Target {
!native && (self.is_linux() || self.is_windows() || self.is_bare_metal())
}

fn needs_docker_privileged(&self) -> bool {
fn needs_docker_seccomp(&self) -> bool {
let arch_32bit = self.triple().starts_with("arm")
|| self.triple().starts_with("i586")
|| self.triple().starts_with("i686");
Expand Down
166 changes: 166 additions & 0 deletions src/seccomp.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
{
"defaultAction": "SCMP_ACT_ALLOW",
"syscalls": [
{
"names": [
"add_key",
"get_kernel_syms",
"keyctl",
"move_pages",
"nfsservctl",
"perf_event_open",
"pivot_root",
"query_module",
"request_key",
"sysfs",
"_sysctl",
"uselib",
"userfaultfd",
"ustat"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1
},
{
"names": [
"acct"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_PACCT"
]
}
},
{
"names": [
"bpf",
"lookup_dcookie",
"mount",
"quotactl",
"quotactl_fd",
"setns",
"swapon",
"swapoff",
"umount",
"umount2",
"unshare",
"vm86",
"vm86old",
"pciconfig_read",
"pciconfig_write",
"salinfo_log_open",
"salinfo_event_open",
"sys_cacheflush",
"rtas"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_ADMIN"
]
}
},
{
"names": [
"clock_adjtime",
"clock_settime",
"settimeofday",
"stime"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_TIME"
]
}
},
{
"names": [
"create_module",
"delete_module",
"finit_module",
"init_module"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_MODULE"
]
}
},
{
"names": [
"get_mempolicy",
"mbind",
"set_mempolicy"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_NICE"
]
}
},
{
"names": [
"ioperm",
"iopl"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_RAWIO"
]
}
},
{
"names": [
"kcmp",
"process_vm_readv",
"process_vm_writev",
"ptrace"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_PTRACE"
]
}
},
{
"names": [
"kexec_file_load",
"kexec_load",
"reboot"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_SYS_BOOT"
]
}
},
{
"names": [
"name_to_handle_at",
"open_by_handle_at"
],
"action": "SCMP_ACT_ERRNO",
"errnoRet": 1,
"excludes": {
"caps": [
"CAP_DAC_READ_SEARCH"
]
}
}
]
}

0 comments on commit 61c4bb7

Please sign in to comment.