Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify path handling for external binaries, from sylabs 272 #6224

Merged
merged 3 commits into from
Oct 19, 2021

Conversation

DrDaveD
Copy link
Collaborator

@DrDaveD DrDaveD commented Oct 12, 2021

This pulls in sylabs pr

This also includes a commit imported from

The original description was:

Prior to this PR, we were very inconsistent in how we locate external binaries. E.g. cryptsetup is found via a buildcfg setting, or a singularity.conf value. mksquashfs is found via $PATH or a singularity.conf value. In different portions of code unsquashfs may be found only via $PATH or by manipulation of the mksquashfs singularity.conf value. Some GPU lib finding and overlay creation operations looked for binaries on the user's complete $PATH. Other lookups on $PATH were after it is santized as a pre-run on the action commands.

This PR:

  • Removes the forced PATH sanitization when action commands are run. The user's PATH is now kept in the pre-container environment. This does change our security model a bit. In Singularity 2.x there were a lot of calls across a collection of shell scripts, python, compiled code. Sanitizing PATH to default admin controlled locations only was a means to avoid calling nasty binaries. In 3.x we explicitly call out to a bounded number of executables from our Go code. We should explicitly check root owns things that will be executed in a privileged context that could be dangerous. Outside of this context, allowing binaries to be found on a full user PATH makes it easier for singularity to be installed by a package manager, or to call binaries in non-standard locations - such as when a cluster provides a newer squashfs-tools etc.

  • Ensures external binaries are found via a bin.FindBin(..) call, so that behaviour for finding a binary can be managed in one place in the code.

  • Uses singularity.conf directives for the cryptsetup, go, ldconfig, mksquashfs, nvidia-container-cli, unsquashfs paths. These are most likely to need customization for some environments.
    - If set, the specified path is used at runtime.
    - If unset, we search PATH plus sensible default bin/sbin dirs.
    - At ./mconfig time we discover these binaries and write their paths into the installed singularity.conf.

  • For other external binaries, search PATH plus sensible default bin/sbin dirs.

  • Check cryptsetup for root ownership explicitly as we now look on the user PATH. Previously we were assuming that a cryptsetup in the default sanitized PATH is safe.

Prior to this PR, we were very inconsistent in how we locate external
binaries. E.g. `cryptsetup` is found via a buildcfg setting, or a
`singularity.conf` value. `mksquashfs` is found via `$PATH` or a
`singularity.conf` value. In different locations `unsquashfs` may be
found only via `$PATH` or by manipulation of the `mksquashfs`
`singularity.conf` value.

This PR:

* Removes the forced `PATH` sanitization when action commands are
  run. The user's `PATH` is now kept in the pre-container
  environment. This does change our security model a bit. In Singularity
  2.x there were a lot of calls across a collection of shell scripts,
  python, compiled code. Santizing `PATH` was a means to avoid calling
  nasty binaries. In 3.x we explicitly call out to specific executables
  from our Go code. We should explicitly check `root` owns things that
  will be executed in a privileged context that could be
  dangerous. Outside of this context, allowing binaries to be found on a
  full user `PATH` makes it easier for singularity to be installed by a
  package manager, or to call binaries in non-standard locations - such
  as when a cluster provides a newer `squashfs-tools` etc.

* Ensures external binaries are found via a `bin.FindBin(..)` call, so
  that behaviour for finding a binary can be managed in one place in the
  code.

* Uses `singularity.conf` directives for the `cryptsetup`, `go`,
  `ldconfig`, `mksquashfs`, `nvidia-container-cli`, `unsquashfs`
  paths. These are most likely to need customization for some environments.
  * If set, the specified path is used at runtime.
  * If unset, we search `PATH` plus sensible default bin/sbin dirs.
  * At `./mconfig` time we discover these binaries and write their
    paths into the installed `sinularity.conf`.

* For other external binaries, search `PATH` plus sensible default
  bin/sbin dirs.

* Check `cryptsetup` for root ownership explicitly as we now look on
  the user `PATH`. Previously we were assuming that a `cryptsetup` in
  the default sanitized `PATH` is safe.

 - Fixes sylabs/singularity#178
Removes buildcfg dependency from pkg directory
Removes buildcfg dependency on pkg directory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants