change std::process to drop supplementary groups based on CAP_SETGID by GrigorenkoPV · Pull Request #121650

change std::process to drop supplementary groups based on CAP_SETGID by GrigorenkoPV · Pull Request #121650 · rust-lang/rust

rustbot added O-unix

Operating system: Unix-like

S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.

labels

Feb 26, 2024

GrigorenkoPV added a commit to GrigorenkoPV/rust that referenced this pull request

Feb 29, 2024

GrigorenkoPV added a commit to GrigorenkoPV/rust that referenced this pull request

Mar 13, 2024

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Mar 13, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 14, 2024

…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117118 ([AIX] Remove AixLinker's debuginfo() implementation)
 - rust-lang#121650 (change std::process to drop supplementary groups based on CAP_SETGID)
 - rust-lang#121764 (Make incremental sessions identity no longer depend on the crate names provided by source code)
 - rust-lang#122212 (Copy byval argument to alloca if alignment is insufficient)
 - rust-lang#122322 (coverage: Initial support for branch coverage instrumentation)
 - rust-lang#122373 (Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification`  and  `unused_imports`)
 - rust-lang#122479 (Implement `Duration::as_millis_{f64,f32}`)
 - rust-lang#122487 (Rename `StmtKind::Local` variant into `StmtKind::Let`)
 - rust-lang#122498 (Update version of cc crate)
 - rust-lang#122503 (Make `SubdiagMessageOp` well-formed)

r? `@ghost`
`@rustbot` modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 15, 2024

…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117118 ([AIX] Remove AixLinker's debuginfo() implementation)
 - rust-lang#121650 (change std::process to drop supplementary groups based on CAP_SETGID)
 - rust-lang#121764 (Make incremental sessions identity no longer depend on the crate names provided by source code)
 - rust-lang#122212 (Copy byval argument to alloca if alignment is insufficient)
 - rust-lang#122322 (coverage: Initial support for branch coverage instrumentation)
 - rust-lang#122373 (Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification`  and  `unused_imports`)
 - rust-lang#122479 (Implement `Duration::as_millis_{f64,f32}`)
 - rust-lang#122487 (Rename `StmtKind::Local` variant into `StmtKind::Let`)
 - rust-lang#122498 (Update version of cc crate)
 - rust-lang#122503 (Make `SubdiagMessageOp` well-formed)

r? `@ghost`
`@rustbot` modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 15, 2024

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.