diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index e22a522a2..2935b9da9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -912,15 +912,13 @@ void LocalDerivationGoal::startBuilder() openSlave(); /* Drop additional groups here because we can't do it - after we've created the new user namespace. FIXME: - this means that if we're not root in the parent - namespace, we can't drop additional groups; they will - be mapped to nogroup in the child namespace. There does - not seem to be a workaround for this. (But who can tell - from reading user_namespaces(7)?) - See also https://lwn.net/Articles/621612/. */ - if (getuid() == 0 && setgroups(0, 0) == -1) - throw SysError("setgroups failed"); + after we've created the new user namespace. */ + if (setgroups(0, 0) == -1) { + if (errno != EPERM) + throw SysError("setgroups failed"); + if (settings.requireDropSupplementaryGroups) + throw Error("setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); + } ProcessOptions options; options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD; diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index ec8625020..d4b8fb1f9 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -524,6 +524,24 @@ public: Setting sandboxFallback{this, true, "sandbox-fallback", "Whether to disable sandboxing when the kernel doesn't allow it."}; + Setting requireDropSupplementaryGroups{this, getuid() == 0, "require-drop-supplementary-groups", + R"( + Following the principle of least privilege, + Nix will attempt to drop supplementary groups when building with sandboxing. + + However this can fail under some circumstances. + For example, if the user lacks the `CAP_SETGID` capability. + Search `setgroups(2)` for `EPERM` to find more detailed information on this. + + If you encounter such a failure, setting this option to `false` will let you ignore it and continue. + But before doing so, you should consider the security implications carefully. + Not dropping supplementary groups means the build sandbox will be less restricted than intended. + + This option defaults to `true` when the user is root + (since `root` usually has permissions to call setgroups) + and `false` otherwise. + )"}; + #if __linux__ Setting sandboxShmSize{ this, "50%", "sandbox-dev-shm-size", diff --git a/tests/build-remote.sh b/tests/build-remote.sh index 78e12b477..d2a2132c1 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -1,6 +1,7 @@ requireSandboxSupport [[ $busybox =~ busybox ]] || skipTest "no busybox" +# Avoid store dir being inside sandbox build-dir unset NIX_STORE_DIR unset NIX_STATE_DIR diff --git a/tests/common.sh b/tests/common.sh index 8941671d6..7b0922c9f 100644 --- a/tests/common.sh +++ b/tests/common.sh @@ -4,7 +4,7 @@ if [[ -z "${COMMON_SH_SOURCED-}" ]]; then COMMON_SH_SOURCED=1 -source "$(readlink -f "$(dirname "${BASH_SOURCE[0]}")")/common/vars-and-functions.sh" +source "$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")/common/vars-and-functions.sh" if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then startDaemon fi diff --git a/tests/common/vars-and-functions.sh.in b/tests/common/vars-and-functions.sh.in index a9e6c802f..dc7ce13cc 100644 --- a/tests/common/vars-and-functions.sh.in +++ b/tests/common/vars-and-functions.sh.in @@ -4,7 +4,7 @@ if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1 -export PS4='+(${BASH_SOURCE[0]}:$LINENO) ' +export PS4='+(${BASH_SOURCE[0]-$0}:$LINENO) ' export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default} export NIX_STORE_DIR diff --git a/tests/hermetic.nix b/tests/hermetic.nix new file mode 100644 index 000000000..4c9d7a51f --- /dev/null +++ b/tests/hermetic.nix @@ -0,0 +1,56 @@ +{ busybox, seed }: + +with import ./config.nix; + +let + contentAddressedByDefault = builtins.getEnv "NIX_TESTS_CA_BY_DEFAULT" == "1"; + caArgs = if contentAddressedByDefault then { + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + } else {}; + + mkDerivation = args: + derivation ({ + inherit system; + builder = busybox; + args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")]; + } // removeAttrs args ["builder" "meta" "passthru"] + // caArgs) + // { meta = args.meta or {}; passthru = args.passthru or {}; }; + + input1 = mkDerivation { + shell = busybox; + name = "hermetic-input-1"; + buildCommand = "echo hi-input1 seed=${toString seed}; echo FOO > $out"; + }; + + input2 = mkDerivation { + shell = busybox; + name = "hermetic-input-2"; + buildCommand = "echo hi; echo BAR > $out"; + }; + + input3 = mkDerivation { + shell = busybox; + name = "hermetic-input-3"; + buildCommand = '' + echo hi-input3 + read x < ${input2} + echo $x BAZ > $out + ''; + }; + +in + + mkDerivation { + shell = busybox; + name = "hermetic"; + passthru = { inherit input1 input2 input3; }; + buildCommand = + '' + read x < ${input1} + read y < ${input3} + echo "$x $y" > $out + ''; + } diff --git a/tests/local.mk b/tests/local.mk index 173bc84b3..df20f3dd7 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -95,6 +95,7 @@ nix_tests = \ misc.sh \ dump-db.sh \ linux-sandbox.sh \ + supplementary-groups.sh \ build-dry.sh \ structured-attrs.sh \ shell.sh \ diff --git a/tests/supplementary-groups.sh b/tests/supplementary-groups.sh new file mode 100644 index 000000000..d18fb2414 --- /dev/null +++ b/tests/supplementary-groups.sh @@ -0,0 +1,37 @@ +source common.sh + +requireSandboxSupport +[[ $busybox =~ busybox ]] || skipTest "no busybox" +if ! command -p -v unshare; then skipTest "Need unshare"; fi +needLocalStore "The test uses --store always so we would just be bypassing the daemon" + +unshare --mount --map-root-user bash <