unix::CommandExt::groups and root_directory

libs
1

Having looked at the Command and unix::CommandExt docs, I find that they are actually lacking. Currently, in rust, you need to use .pre_exec to set a list of groups or to chroot before executing the command. Both are suboptimal for the following reasons:

  • You need to allocate in order to call chroot(2) (I will not start a rant about OsStr being entirely useless, but suffice it to say, I am not the biggest fan of it).
  • You almost always need to allocate in order to set a suplementary groups list.
  • When the pre_exec closure is called, the directory has already been changed, which is incorrect when presented with chroot(2), since it will leave it at the same physical working directory (which may be outside the new root directory), so in order to use chroot, you also have to use chdir, requiring more allocations.
  • It is also not defined the order that changes of uid and gid will take place wrt. pre_exec, which means that you can't use .uid if you plan to do anything that needs elevation because if it occurs before pre_exec, all privileged operations will be all but guaranteed to EPERM, including root_directory and groups.
  • pre_exec also cannot use closures that capture by-move, which is understandable, but prevents taking ownership of the Vec or CString (which could then be forgotten in the child), which could be dropped in the parent (To avoid having to leak or borrow, which limits or prevents returning the Command, in forwarding methods that operate on Command).
  • groups closely mirrors the presence of gid. It doesn't necessarily make sense to set the primary group idea while leaving all suplementary group ids untouched.
  • chroot is potetionally useful in privileged code, and is very difficult to use correctly as-is with CommandExt, for the reasons described above.

Thus, I propose two additional functions be added to std::os::unix::process::CommandExt as follows:

groups

fn groups<A: AsRef<[u32]>>(&mut self,groups: A) -> &mut Self

Sets the supplementary groups of the child process to the the list provided by groups.

root_directory

fn root_directory<S: AsRef<Path>>(&mut self,dir: S) -> &mut Self;

Sets the Root directory of the the child process.

Notes

root_directory applies to both current_directory and the program name for the command. The new directory is the path name within the root directory. If the path passed to current_directory is relative, it is resolved relative to /. If no call to self.current_directory is made, then the working directory in the child will be /.

2

groups has already been added: Add setgroups to std::os::unix::process::CommandExt by slo1 · Pull Request #72160 · rust-lang/rust · GitHub

I'd love to see support for chroot, for setsid, and for arbitrary file descriptor support.

In the meantime, I'd suggest the unshare crate.

3

I didn't see that in the docs. Apparently it was because I'm on the stable docs, not nightly. That seems to do what I need in terms of groups.

I'd be willing to write the appropriate proposal for any or all of those if I can be pointed in the right direction :smiley:. I would assume the former two would be easier, since there's less about them that can be bikeshedded.

I'll check that out. In my case, I'm not working on a public api, so that switch would be reasonable.

4

I think it's fine if this stuff stays as a crate - among other things, it gets hard to avoid e.g. having Linux specific APIs and a lot people will want that, others might want generic Unix etc. - if it's in std then it becomes painful for porters.

5

Well, there is prior art for linux specific stuff, the std::os::linux module. setsid, is in POSIX. chroot is not (I thought it was), but it is SVr4 (though I don't know how portable that makes it to cfg(unix) targets).

One thing that would be nice and generic would be a different pre_exec that is guaranteed to run before the other things are done (and that accepts FnOnce under the strict safety requirement that "You must not drop things that free").

6

Independent of what gets added, the exact order of operations in between fork and exec should be documented (and corrected, if necessary, to ensure that each operation actually works as intended).

7

We just talked about this in today's libs team meeting. We broadly agreed that while we wouldn't want to have extensive mechanisms that might involve selecting and implementing a policy (e.g. constructing and configuring namespaces of various kinds), chroot and setsid would be fine, as would other reasonably simple things that just require a single call between fork and exec.

As with many relatively simple APIs, these don't require a full RFC, just a pull request adding the code (including documentation, tests, and a feature gate). (Note that on most systems root_directory won't be testable except as root.)

8

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.