[Backport release-24.05] apptainer, singularity: precede system-level bin paths in `defaultPath` and fix `singularity` image running #324426

github-actions · 2024-07-03T22:11:04Z

Bot-based backport to release-24.05, triggered by a label in #306730.

Before merging, ensure that this backport is acceptable for the release.
- Even as a non-commiter, if you find that it is not acceptable, leave a comment.

Prefix the upstream-given defalutPath value over the one constructed by defaultPathInputs. Make SUID'ed binaries searchable out-of-the-box non-NixOS platforms. (cherry picked from commit 409cbbe)

Use systemBinPaths as the new way to specify system bin paths, especifally for SUID'ed binaries. Deprecate arguments setuidmapPath and setgidmapPath in favour of systemBinPaths. Add NixOS configuration option programs.singularity.systemBinPath, with "/run/wrappers/bin" included by default. (cherry picked from commit f6d9b4b)

Add a Nixpkgs 24.05 release note entry explaining the introduction of `systemBinPaths` argument, the prioritization of the original (FHS) `defaultPath` values, and the deprecation of arguments `newuidmapPath`, `newgidmapPath` and NixOS configuration option `programs.singularity.enableFakeroot`. (cherry picked from commit dbcf7cf)

SomeoneSerge · 2024-07-03T22:14:09Z

nixos/doc/manual/release-notes/rl-2405.section.md

@@ -224,6 +224,19 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m

 - `appimageTools.wrapAppImage` now creates the binary at `$out/bin/${pname}` rather than `$out/bin/${pname}-${version}`, which will break downstream workarounds.

+- `apptainer` and `singularity` now prioritize system-wide `PATH` over those constructed from dependent packages when searching for third-party utilities. The `PATH` to search for third-party utilities, known as `defaultPath` inside Apptainer/Singularity source code, is now constructed from the following sources, ordered by their precedence:


My memory's a bit hazy, I now forget why we wanted the system PATH to take the priority?

Because the fusermount (or fusermount3) executable needs its SUID bit set to be able to execute by unprivileged users, and that such utilities are usually provided system-wide.

There are quite a few similar utilities. Some other examples includes newuidmap and newgidmap needed by the --fakeroot feature.

It also makes Apptainer and SingularityCE work out of the box in FHS environments, where the images are mostly run.

H'm I see how /run/wrapper/bin is correct, but I'm still dizzy as to why FHS isn't the last priority

But ok this isn't exactly the question for backporting

but I'm still dizzy as to why FHS isn't the last priority

That's indeed an unusual design choice for Nix packages.

As Apptainer and SingularityCE rely more and more on system-level utilities and even the ones with SUID bit set, it becomes much easier to utilize them from the environment instead of inventing more arguments like newuidmapPath and newgidmapPath that unprivileged users need to override to get Apptainer/SingularityCE to function correctly.

ShamrockLee · 2024-07-31T18:11:27Z

Hi, @SomeoneSerge! Is there anything I could do to push this PR forward?

SomeoneSerge · 2024-07-31T21:28:17Z

nixos/doc/manual/release-notes/rl-2405.section.md

The question for backporting is what are the interfaces we're changing (we're changing some priorities), how we justify that, and whom do we affect.

Is it reasonable to assume these two packages would only be actively used on NixOS? Because in FHS distributions they probably have their own...

Is it reasonable to assume these two packages would only be actively used on NixOS? Because in FHS distributions they probably have their own...

We reverse the priority of FHS system-level PATH and makeBinPath-generated PATH, preceding the FHS one. This is based on the assumption that apptainer and singularity will be used often in FHS distributions.

To reserve room of customization, we define the systemBinPaths argument in the override interface, the PATH it constructed will precede the above two. The non-empty default value [ "/run/wrappers/bin" ] is to make the package usable in NixOS without overriding.

Ok but somehow the FHS is now the first priority, instead of the security wrappers being the first priority?

Before

For defaultPaths where Apptainer/SingularityCE search for system-level utilities:

PATH from defaultPathInputs by lib.makeBinPath, consisting of dependent Nix packages

PATH from the upstream's original defaultPath values, the FHS system-level PATHs

For makeWrapper:

PATH from defaultPathInputs by lib.makeBinPath, consisting of dependent Nix packages

PATH from the run-time environment

If both arguments newuidmapPath and newgidmapPath are specified, a local wrapper package privileged-un-utils will be inserted into defaultPathInputs. That's how --fakeroot support of Apptainer/SingularityCE is enabled when using the NixOS module programs.singularity. Without using the NxOS module, people need to override newuidmapPath and newgidmapPath manually, pointing them to the newuidmap and newgidmap executables with SUID bit set to be able to have --fakeroot support.

After

For defaultPaths where Apptainer/SingularityCE search for system-level utilities:

PATH from systemBinPaths, default to "/run/wrappers/bin"

PATH from the upstream's original defaultPath values, the FHS system-level PATHs

PATH from defaultPathInputs by lib.makeBinPath, consisting of dependent Nix packages

For makeWrapper:

PATH from systemBinPaths, default to "/run/wrappers/bin"

PATH from the run-time environment

PATH from defaultPathInputs by lib.makeBinPath, consisting of dependent Nix packages

For compatibility, if newuidmapPath and newgidmapPath are both set, "${privileged-un-utils}/bin" is prepended to systemBinPaths.

The question for backporting is what are the interfaces we're changing (we're changing some priorities), how we justify that, and whom do we affect.

The most significant change would be the priorities of each source to form the PATH. Other interface changes (adding the systemBinPaths arguments and programs.singularity.systemBinPaths NixOS module option) are backward compatible.

The compatibility layer for the newuidmapPath and newgidmapPath arguments is explained above. As for the NixOS module, programs.singularity.enableFakeroot no longer has the effect of setting newuidmapPath and newgidmapPath, as "/run/wrappers/bin" is added to programs.singularity.systemBinPathsby default, making--fakeroot` support available.

whom do we affect

I could think of an edge case that an upset user wants only the newuidmap and newgidmap with the SUID bit set to be exposed to Apptainer/SingularityCE but not the whole /run/wrappers/bin or system-wide FHS PATH. Still, it doesn't sound reasonably practical to persuade such a level of isolation while sacrificing the functionality of Apptainer/SingularityCE.

Thanks. I now see that security wrappers on nixos take the highest priority.

The only part I'm concerned about is PATH (env) taking priority over makeBinPath [ ... ].

Ok I suppose this has to be, because PATH is going to be the only way FHS distributions expose their own setuid executables. What I don't like is PATH is less deterministic, it's not just used for explicit overrides

ShamrockLee · 2024-07-31T22:10:49Z

Is it possible to drop the last commit that adds the deprecation warnings? This one is to guide people to use the new interface, and does not mean to be backported.

wolfgangwalther · 2024-12-05T20:04:43Z

Does it still make sense to backport this at this stage in the release-cycle?

I'm leaning to "no".

ShamrockLee · 2024-12-06T02:41:45Z

Does it still make sense to backport this at this stage in the release-cycle?

I think not. It's not a security fix, and we don't seem to fix things on previous stable branches unless it's security-related.

ShamrockLee added 3 commits July 3, 2024 22:11

apptainer, singularity: prioritize original defaultPath

2153023

Prefix the upstream-given defalutPath value over the one constructed by defaultPathInputs. Make SUID'ed binaries searchable out-of-the-box non-NixOS platforms. (cherry picked from commit 409cbbe)

github-actions bot mentioned this pull request Jul 3, 2024

apptainer, singularity: precede system-level bin paths in defaultPath and fix singularity image running #306730

Merged

13 tasks

SomeoneSerge reviewed Jul 3, 2024

View reviewed changes

ofborg bot requested a review from jbedo July 3, 2024 23:41

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jul 3, 2024

SomeoneSerge reviewed Jul 31, 2024

View reviewed changes

SomeoneSerge force-pushed the backport-306730-to-release-24.05 branch from 77442cf to 8993f1f Compare July 31, 2024 22:21

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 31, 2024

ofborg bot requested a review from ShamrockLee July 31, 2024 23:44

wolfgangwalther requested review from ShamrockLee and SomeoneSerge and removed request for ShamrockLee December 3, 2024 21:24

ShamrockLee closed this Dec 6, 2024

wolfgangwalther deleted the backport-306730-to-release-24.05 branch December 6, 2024 07:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in `defaultPath` and fix `singularity` image running #324426

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in `defaultPath` and fix `singularity` image running #324426

github-actions bot commented Jul 3, 2024

SomeoneSerge Jul 3, 2024

ShamrockLee Jul 4, 2024 •

edited

Loading

SomeoneSerge Jul 31, 2024 •

edited

Loading

SomeoneSerge Jul 31, 2024

ShamrockLee Aug 1, 2024

ShamrockLee commented Jul 31, 2024 •

edited

Loading

SomeoneSerge Jul 31, 2024

SomeoneSerge Jul 31, 2024

ShamrockLee Jul 31, 2024

SomeoneSerge Jul 31, 2024

ShamrockLee Aug 1, 2024 •

edited

Loading

ShamrockLee Aug 1, 2024 •

edited

Loading

SomeoneSerge Aug 1, 2024

SomeoneSerge Aug 1, 2024

ShamrockLee commented Jul 31, 2024

wolfgangwalther commented Dec 5, 2024

ShamrockLee commented Dec 6, 2024

		@@ -224,6 +224,19 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m

		- `appimageTools.wrapAppImage` now creates the binary at `$out/bin/${pname}` rather than `$out/bin/${pname}-${version}`, which will break downstream workarounds.

		- `apptainer` and `singularity` now prioritize system-wide `PATH` over those constructed from dependent packages when searching for third-party utilities. The `PATH` to search for third-party utilities, known as `defaultPath` inside Apptainer/Singularity source code, is now constructed from the following sources, ordered by their precedence:

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in defaultPath and fix singularity image running #324426

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in defaultPath and fix singularity image running #324426

Conversation

github-actions bot commented Jul 3, 2024

Choose a reason for hiding this comment

ShamrockLee Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

SomeoneSerge Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShamrockLee commented Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShamrockLee Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

ShamrockLee Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShamrockLee commented Jul 31, 2024

wolfgangwalther commented Dec 5, 2024

ShamrockLee commented Dec 6, 2024

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in `defaultPath` and fix `singularity` image running #324426

[Backport release-24.05] apptainer, singularity: precede system-level bin paths in `defaultPath` and fix `singularity` image running #324426

ShamrockLee Jul 4, 2024 •

edited

Loading

SomeoneSerge Jul 31, 2024 •

edited

Loading

ShamrockLee commented Jul 31, 2024 •

edited

Loading

ShamrockLee Aug 1, 2024 •

edited

Loading

ShamrockLee Aug 1, 2024 •

edited

Loading