Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to set jailed process uid #103

Open
akhramov opened this issue May 5, 2021 · 3 comments
Open

Unable to set jailed process uid #103

akhramov opened this issue May 5, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@akhramov
Copy link
Contributor

akhramov commented May 5, 2021

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

  • Attempt to change stdlib (unrealistically)
  • exec.jail_user. Well, not quite. It's not uid, not sure if it works for jail_attach.
  • just create another hook to call setuid there!

WDYT?

@akhramov akhramov added the bug Something isn't working label May 5, 2021
@akhramov
Copy link
Contributor Author

akhramov commented May 5, 2021

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
Copy link
Contributor

Hmm, this is annoying :/

@ryanavella
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants