fubarnetes / libjail-rs

Rust implementation of a FreeBSD jail library
https://crates.io/crates/jail
BSD 3-Clause "New" or "Revised" License
54 stars 12 forks source link

Unable to set jailed process uid #103

Open akhramov opened 3 years ago

akhramov commented 3 years ago

Describe the bug

Inability to set uid of a jailed process.

To Reproduce Consider the following use-case: I'm trying to change uid of a process running inside the jail. For that purpose I use std::os::unix::process::CommandExt.uid.

In code:

let stopped_jail = StoppedJail::new(&path)
    .name("container 42")
    .param("vnet", Value::Int(1))
    .param("enforce_statfs", Value::Int(1))
    .unwrap();

Command::new(command)
    .jail(&jail)
    .uid(uid)
    .gid(gid)
    .spawn()
    .unwrap();

The spawn call returns EPERM error.

Expected behavior The spawn call succeeds

Additional context Underlying issue is jail_attach call. Per man page

The jail_attach() and jail_remove() system calls will fail if:

[EPERM] A user other than the super-user attempted to attach to or remove a jail.

stdlib calls setuid here, before calling pre-exec hooks here. Since the process uid set to a non-priveleged user, alas, we fail.

Possible workarounds

Either

WDYT?

akhramov commented 3 years ago

FWIW a working workaround

use std::{
    io::Error, os::unix::process::CommandExt as StdCommandExt,
    process::Command,
};

use libc::{gid_t, setgid, setuid, uid_t};

// A workaround for https://github.com/fubarnetes/libjail-rs/issues/103
pub trait CommandExt {
    fn uid(&mut self, uid: u32) -> &mut Command;
    fn gid(&mut self, gid: u32) -> &mut Command;
}

impl CommandExt for Command {
    fn uid(&mut self, uid: u32) -> &mut Command {
        unsafe {
            self.pre_exec(move || {
                if setuid(uid as uid_t) < 0 {
                    return Err(Error::last_os_error());
                }

                Ok(())
            });
        }

        self
    }

    fn gid(&mut self, gid: u32) -> &mut Command {
        StdCommandExt::gid(self, gid)
    }
}
fabianfreyer commented 3 years ago

Hmm, this is annoying :/

ryanavella commented 1 year ago

Could this uid workaround method be added to the jail::Jailed trait? It's a breaking change, but I don't think would present much friction to downstream users, especially if we gave it a different name like jail_uid so that it doesn't conflict with CommandExt::uid.