diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-03-14 20:00:17 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-03-14 20:00:17 +0100 |
| commit | eaa8dafe1aa80427943708657f4b5a926c4b4e1f (patch) | |
| tree | 41b76e999cd9aa586edcb6e1c31de170fb7b0404 | |
| parent | 68ca795286f8c7c4a61c59f88330e916f0a11f7e (diff) | |
| parent | 3a6af84fca1b41614887748e9fdb0e2b2a13fb96 (diff) | |
| download | rust-eaa8dafe1aa80427943708657f4b5a926c4b4e1f.tar.gz rust-eaa8dafe1aa80427943708657f4b5a926c4b4e1f.zip | |
Rollup merge of #121650 - GrigorenkoPV:cap_setgid, r=Amanieu
change std::process to drop supplementary groups based on CAP_SETGID A trivial rebase of #95982 Should fix #39186 (from what I can tell) Original description: > Fixes #88716 > > * Before this change, when a process was given a uid via `std::os::unix::process::CommandExt.uid`, there would be a `setgroups` call (when the process runs) to clear supplementary groups for the child **if the parent was root** (to remove potentially unwanted permissions). > * After this change, supplementary groups are cleared if we have permission to do so, that is, if we have the CAP_SETGID capability. > > This new behavior was agreed upon in #88716 but there was a bit of uncertainty from `@Amanieu` here: [#88716 (comment)](https://github.com/rust-lang/rust/issues/88716#issuecomment-973366600) > > > I agree with this change, but is it really necessary to ignore an EPERM from setgroups? If you have permissions to change UID then you should also have permissions to change groups. I would feel more comfortable if we documented set_uid as requiring both UID and GID changing permissions. > > The way I've currently written it, we ignore an EPERM as that's what #88716 originally suggested. I'm not at all an expert in any of this so I'd appreciate feedback on whether that was the right way to go.
| -rw-r--r-- | library/std/src/os/unix/process.rs | 7 | ||||
| -rw-r--r-- | library/std/src/sys/pal/unix/process/process_unix.rs | 14 |
2 files changed, 18 insertions, 3 deletions
diff --git a/library/std/src/os/unix/process.rs b/library/std/src/os/unix/process.rs index e45457b2e42..72ea54bd772 100644 --- a/library/std/src/os/unix/process.rs +++ b/library/std/src/os/unix/process.rs @@ -39,6 +39,13 @@ pub trait CommandExt: Sealed { /// Sets the child process's user ID. This translates to a /// `setuid` call in the child process. Failure in the `setuid` /// call will cause the spawn to fail. + /// + /// # Notes + /// + /// This will also trigger a call to `setgroups(0, NULL)` in the child + /// process if no groups have been specified. + /// This removes supplementary groups that might have given the child + /// unwanted permissions. #[stable(feature = "rust1", since = "1.0.0")] fn uid(&mut self, id: UserId) -> &mut process::Command; diff --git a/library/std/src/sys/pal/unix/process/process_unix.rs b/library/std/src/sys/pal/unix/process/process_unix.rs index 97cbd1929d3..f017d39d804 100644 --- a/library/std/src/sys/pal/unix/process/process_unix.rs +++ b/library/std/src/sys/pal/unix/process/process_unix.rs @@ -330,14 +330,22 @@ impl Command { if let Some(u) = self.get_uid() { // When dropping privileges from root, the `setgroups` call // will remove any extraneous groups. We only drop groups - // if the current uid is 0 and we weren't given an explicit + // if we have CAP_SETGID and we weren't given an explicit // set of groups. If we don't call this, then even though our // uid has dropped, we may still have groups that enable us to // do super-user things. //FIXME: Redox kernel does not support setgroups yet #[cfg(not(target_os = "redox"))] - if libc::getuid() == 0 && self.get_groups().is_none() { - cvt(libc::setgroups(0, crate::ptr::null()))?; + if self.get_groups().is_none() { + let res = cvt(libc::setgroups(0, crate::ptr::null())); + if let Err(e) = res { + // Here we ignore the case of not having CAP_SETGID. + // An alternative would be to require CAP_SETGID (in + // addition to CAP_SETUID) for setting the UID. + if e.raw_os_error() != Some(libc::EPERM) { + return Err(e.into()); + } + } } cvt(libc::setuid(u as uid_t))?; } |
