Skip to content

Commit

Permalink
Set the user and group IDs for the sandboxed process
Browse files Browse the repository at this point in the history
This commit fixes some issues with `write_uid_gid_maps()` which in turn
allows us to finally assign a proper user ID and group ID to our
sandboxed process, so now the `Sandbox::uid()` and `Sandbox::gid()`
builder methods actually do something. If the user doesn't specify them,
the sandbox defaults to inheriting the same user and group as the
parent process.

There were a handful of issues with coaxing the `openat` library to do
the right thing, first of which was described in tailhook/openat#26.
Another unsolved issue is that `writeln!()` and `write!()` sometimes
strangely fails when writing to files opened with
`openat::Dir::write_file()`, namely it throws an
`std::io::ErrorKind::InvalidInput` with the message "Invalid argument".
The panic itself appears to be coming from `std::io::Write::write_fmt()`
specfically when invoked on `/proc/self/{uid_map,gid_map}`.

Thankfully, when I use `<File as Write>::write_all()`, it works just
fine, and I can verify from within the sandboxed process that the UIDs
and GIDs are indeed set up correctly from the write. Strange, but at
least this works. :shrug:
  • Loading branch information
ebkalderon committed Nov 23, 2019
1 parent dc0e22f commit 6c89aad
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
22 changes: 22 additions & 0 deletions src/os/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,28 @@ pub fn create_sandbox(config: &Sandbox, command: &mut Command) -> Result<Child,
net::setup_loopback_device()?;
}

if !IS_PRIVILEGED {
// In the unprivileged case we have to write the uid/gid maps in the child, because
// we have no caps in the parent.

// TODO: Like with `bwrap`, we might have to first map the `SANDBOX_UID` and
// `SANDBOX_GID` to 0, otherwise we can't mount the devpts filesystem because root
// is not mapped. Later, we will create another child user namespace and map back
// to the real uid. But before we write this code, we should investigate whether
// this hack should even be necessary to perform conditionally, or perhaps we could
// just allow device access all the time.

creds::write_uid_gid_map(
SANDBOX_UID,
SANDBOX_GID,
REAL_UID,
REAL_GID,
None,
true,
false,
)?;
}

// FIXME: Commands are currently statically forced to run in either inherit or piped
// mode until the `std::command::Command` builder offers some way to extract its fields
// and inspect them at run-time.
Expand Down
26 changes: 14 additions & 12 deletions src/os/linux/creds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,24 @@ pub unsafe fn write_uid_gid_map(
let pid = pid
.map(|pid| pid.to_string())
.unwrap_or_else(|| "self".to_string());
PROC_DIR

let proc = PROC_DIR
.as_ref()
.ok_or_else(|| Error::new(ErrorKind::NotFound, "Expected /proc descriptor to be open"))
.and_then(|proc| proc.sub_dir(format!("{}/ns", pid)))?
.ok_or_else(|| Error::new(ErrorKind::NotFound, "Expected /proc to be open"))?;

proc.read_link(pid).and_then(|path| proc.sub_dir(&path))?
};

let uid_map = if map_root && parent_uid != 0 && sandbox_uid != 0 {
format!("0 {} 1\n{} {} 1", OVERFLOW_UID, sandbox_uid, parent_uid)
format!("0 {} 1\n{} {} 1\n", OVERFLOW_UID, sandbox_uid, parent_uid)
} else {
format!("{} {} 1", sandbox_uid, parent_uid)
format!("{} {} 1\n", sandbox_uid, parent_uid)
};

let gid_map = if map_root && parent_gid != 0 && sandbox_gid != 0 {
format!("0 {} 1\n{} {} 1", OVERFLOW_GID, sandbox_gid, parent_gid)
} else {
format!("{} {} 1", sandbox_gid, parent_gid)
format!("{} {} 1\n", sandbox_gid, parent_gid)
};

// We have to be root to be allowed to write to the uid map for setuid apps, so temporary set
Expand All @@ -62,13 +64,13 @@ pub unsafe fn write_uid_gid_map(
};

ns_dir
.write_file("uid_map", 0)
.and_then(|mut f| writeln!(f, "{}", uid_map))
.update_file("uid_map", 0)
.and_then(|mut file| file.write_all(uid_map.as_bytes()))
.map_err(|_| Error::new(ErrorKind::Other, "Failed to set up uid map"))?;

if deny_groups {
let setgroups = ns_dir.write_file("setgroups", 0);
if let Err(err) = setgroups.and_then(|mut f| writeln!(f, "deny")) {
let setgroups = ns_dir.update_file("setgroups", 0);
if let Err(err) = setgroups.and_then(|mut file| file.write_all(b"deny\n")) {
// If /proc/[pid]/setgroups does not exist, assume we are
// running a linux kernel < 3.19, i.e. we live with the
// vulnerability known as CVE-2014-8989 in older kernels
Expand All @@ -81,8 +83,8 @@ pub unsafe fn write_uid_gid_map(
}

ns_dir
.write_file("gid_map", 0)
.and_then(|mut f| writeln!(f, "{}", gid_map))
.update_file("gid_map", 0)
.and_then(|mut file| file.write_all(gid_map.as_bytes()))
.map_err(|_| Error::new(ErrorKind::Other, "Failed to set up gid map"))?;

if let Some(old) = old_fsuid {
Expand Down

0 comments on commit 6c89aad

Please sign in to comment.