Sandbox adds a /bin/sh binding even if enable-embedded-sandbox-shell is false #525

Closed
opened 2024-09-15 02:51:59 +00:00 by dareka826 · 3 comments

Describe the bug

In src/libstore/globals.cc these lines add a binding to SANDBOX_SHELL as /bin/sh even if HAVE_EMBEDDED_SANDBOX_SHELL isn't defined:

#if defined(__linux__) && defined(SANDBOX_SHELL)
    sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL);
#endif

Whether SANDBOX_SHELL is defined doesn't depend on the value of enable-embedded-sandbox-shell, just on whether the busybox program was found.

From src/libstore/meson.build:

if busybox.found()
  cpp_str_defines += {
    'SANDBOX_SHELL': busybox.full_path()
  }
endif

From the main meson.build:

sandbox_shell = get_option('sandbox-shell')
sandbox_shell_required = sandbox_shell != 'busybox' and host_machine.system() == 'linux'
busybox = find_program(sandbox_shell, required : sandbox_shell_required, native : false)

From the main meson.options:

option('sandbox-shell', type : 'string', value : 'busybox',
  description : 'path to a statically-linked shell to use as /bin/sh in sandboxes (usually busybox)',
)

The required field of find_program is only false if the sandbox-shell option remains at the default of "busybox", but that means that if busybox is detected as installed on the system, then it always gets bound to /bin/sh in the sandbox.

Steps To Reproduce

  1. Compile lix with enable-embedded-sandbox-shell set to false and busybox present on the host system
meson setup -Denable-embedded-sandbox-shell=false build
meson compile -C build
  1. Download/compile a static busybox for the test nix expression
  2. Compile statically this code as argv0 for the test nix expression:
#include <unistd.h>
#include <stdio.h>

// argv[1] - new argv0
// argv[2] - program to exec
// argv[3..] - program args
int main(int argc, char *argv[]) {
    if (argc < 3) {
        fputs("Usage: ./argv0  new_argv0  program  args...\n", stderr);
        return -1;
    }

    char *argv0 = argv[1];
    char *prog  = argv[2];

    argv[2] = argv0; // New start of argv

    return execvp(prog, &argv[2]);
}
  1. Build this expression:
let
  bb = ./busybox;
  a0 = ./argv0;
in
  derivation {
    system = "x86_64-linux";
    name = "sandbox-test";
    builder = a0;
    args = [
      "ash"
      bb
      "-c"
      "${a0} test ${bb} -e /bin/sh && ${a0} md5sum ${bb} /bin/sh"
    ];
  }

My output:

$ nix-build ./test.nix
this derivation will be built:
  /nix/store/81m3dak7xwmvjdzh25dlh1vhkdjslglk-sandbox-test.drv
building '/nix/store/81m3dak7xwmvjdzh25dlh1vhkdjslglk-sandbox-test.drv'...
b5975dfc1223b0c7ac80e7a01e75d62f  /bin/sh
error: builder [...] failed to produce output [...]

$ md5sum /bin/busybox
b5975dfc1223b0c7ac80e7a01e75d62f  /bin/busybox

Running strings build/src/libstore/liblixstore.so | grep busybox should show whether the binding /bin/sh=/usr/bin/busybox is embedded into libstore.

Expected behavior

Disabling enable-embedded-sandbox-shell should also prevent the SANBDOX_SHELL being mounted as /bin/sh.

nix --version output

nix (Lix, like Nix) 2.92.0-dev

Specifically, commit 727258241.

## Describe the bug In `src/libstore/globals.cc` these lines add a binding to `SANDBOX_SHELL` as `/bin/sh` even if `HAVE_EMBEDDED_SANDBOX_SHELL` isn't defined: ```cpp #if defined(__linux__) && defined(SANDBOX_SHELL) sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL); #endif ``` Whether `SANDBOX_SHELL` is defined doesn't depend on the value of `enable-embedded-sandbox-shell`, just on whether the busybox program was found. From `src/libstore/meson.build`: ``` if busybox.found() cpp_str_defines += { 'SANDBOX_SHELL': busybox.full_path() } endif ``` From the main `meson.build`: ``` sandbox_shell = get_option('sandbox-shell') sandbox_shell_required = sandbox_shell != 'busybox' and host_machine.system() == 'linux' busybox = find_program(sandbox_shell, required : sandbox_shell_required, native : false) ``` From the main `meson.options`: ``` option('sandbox-shell', type : 'string', value : 'busybox', description : 'path to a statically-linked shell to use as /bin/sh in sandboxes (usually busybox)', ) ``` The `required` field of `find_program` is only false if the `sandbox-shell` option remains at the default of `"busybox"`, but that means that if busybox is detected as installed on the system, then it always gets bound to `/bin/sh` in the sandbox. ## Steps To Reproduce 1. Compile lix with `enable-embedded-sandbox-shell` set to false and busybox present on the host system ``` meson setup -Denable-embedded-sandbox-shell=false build meson compile -C build ``` 2. Download/compile a static busybox for the test nix expression 3. Compile statically this code as argv0 for the test nix expression: ```c #include <unistd.h> #include <stdio.h> // argv[1] - new argv0 // argv[2] - program to exec // argv[3..] - program args int main(int argc, char *argv[]) { if (argc < 3) { fputs("Usage: ./argv0 new_argv0 program args...\n", stderr); return -1; } char *argv0 = argv[1]; char *prog = argv[2]; argv[2] = argv0; // New start of argv return execvp(prog, &argv[2]); } ``` 4. Build this expression: ```nix let bb = ./busybox; a0 = ./argv0; in derivation { system = "x86_64-linux"; name = "sandbox-test"; builder = a0; args = [ "ash" bb "-c" "${a0} test ${bb} -e /bin/sh && ${a0} md5sum ${bb} /bin/sh" ]; } ``` My output: ``` $ nix-build ./test.nix this derivation will be built: /nix/store/81m3dak7xwmvjdzh25dlh1vhkdjslglk-sandbox-test.drv building '/nix/store/81m3dak7xwmvjdzh25dlh1vhkdjslglk-sandbox-test.drv'... b5975dfc1223b0c7ac80e7a01e75d62f /bin/sh error: builder [...] failed to produce output [...] $ md5sum /bin/busybox b5975dfc1223b0c7ac80e7a01e75d62f /bin/busybox ``` Running `strings build/src/libstore/liblixstore.so | grep busybox` should show whether the binding `/bin/sh=/usr/bin/busybox` is embedded into libstore. ## Expected behavior Disabling `enable-embedded-sandbox-shell` should also prevent the `SANBDOX_SHELL` being mounted as `/bin/sh`. ## `nix --version` output ``` nix (Lix, like Nix) 2.92.0-dev ``` Specifically, commit 727258241.
dareka826 added the
bug
label 2024-09-15 02:51:59 +00:00
Author

This breaks many build scripts that use /bin/sh when the system busybox prefers it's own applets from the binaries on PATH.

This breaks many build scripts that use `/bin/sh` when the system busybox prefers it's own applets from the binaries on `PATH`.
Owner

If I am correctly understanding this bug filing, it is not a bug. The /bin/sh in the sandbox is not intended to be used except for bootstrapping, and is the only binary actually in the sandbox. If your build scripts are broken in the sandbox, consider using patchShebangs since they shouldn't depend on /bin/sh to begin with.

Our builds only ship the embedded shell on static builds: df0137226d/package.nix (L225)

Here, embedded means the sandbox shell binary is actually included within the nix binary itself then extracted at runtime. However, all nix-based Lix builds provide a /bin/sh that is busybox inside the sandbox, and that is effectively a standard expectation of the derivation build environment: a fairly minimal busybox that can be used for bootstrapping; the embedding just changes from whence it comes.

Granted, perhaps it should be removed since it is a major sandbox impurity that it comes from the nix implementation, but that would be a very breaking change that is very low on the priority list.

So the "expected" behaviour in this bug filing is very much unexpected behaviour in my view.

If I am correctly understanding this bug filing, it is not a bug. The /bin/sh in the sandbox *is not intended to be used except for bootstrapping*, and is the only binary actually in the sandbox. If your build scripts are broken in the sandbox, consider using `patchShebangs` since they shouldn't depend on /bin/sh to begin with. Our builds only ship the *embedded* shell on static builds: https://git.lix.systems/lix-project/lix/src/df0137226d7db4bfac455076be9f37dae6f01ffe/package.nix#L225 Here, embedded means the sandbox shell *binary* is actually included within the nix binary itself then extracted at runtime. However, all nix-based Lix builds provide a /bin/sh that is busybox inside the sandbox, and that is effectively a standard expectation of the derivation build environment: a [fairly minimal busybox](https://git.lix.systems/lix-project/lix/src/c14486ae8d3bbc862c625d948a6b2f4dc0927d5b/flake.nix#L175-L198) that can be used for bootstrapping; the embedding just changes from whence it comes. Granted, perhaps it *should* be removed since it is a major sandbox impurity that it comes from the nix implementation, but that would be a very breaking change that is very low on the priority list. So the "expected" behaviour in this bug filing is very much unexpected behaviour in my view.
Author

Ah, that makes sense. I don't know why I thought the embedded sandbox shell should influence that. But I do think there should be an option for disabling the embedding of system's busybox if present. I'm trying to bootstrap some derivations without using nixpkgs and I'd rather disable system's /bin/sh for this phase than patch every package I'm building.

Ah, that makes sense. I don't know why I thought the embedded sandbox shell should influence that. But I do think there should be an option for disabling the embedding of system's busybox if present. I'm trying to bootstrap some derivations without using nixpkgs and I'd rather disable system's /bin/sh for this phase than patch every package I'm building.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix#525
No description provided.