Unrolled build for rust-lang#121650

rust-timer · web-flow · commit 3137f94f2687 · 2024-03-14T22:05:13.000-04:00
Rollup merge of rust-lang#121650 - GrigorenkoPV:cap_setgid, r=Amanieu change std::process to drop supplementary groups based on CAP_SETGID A trivial rebase of rust-lang#95982 Should fix rust-lang#39186 (from what I can tell) Original description: > Fixes rust-lang#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 rust-lang#88716 but there was a bit of uncertainty from `@Amanieu` here: [rust-lang#88716 (comment)](rust-lang#88716 (comment)) > > > 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 rust-lang#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.
diff --git 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
@@ -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))?;
             }
