From 9909a175bf1602e7bb4ebfc1c9befeaa56da1fb4 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Sat, 4 May 2024 00:55:15 -0700 Subject: [PATCH] Fix /etc/group having desynced IDs from the actual UID in the sandbox This was found when `logrotate.conf` failed to build in a NixOS system with: /nix/store/26zdl4pyw5qazppj8if5lm8bjzxlc07l-coreutils-9.3/bin/id: cannot find name for group ID 30000 This was surprising because it seemed to mean that /etc/group was busted in the sandbox. Indeed it was: root:x:0: nixbld:!:100: nogroup:x:65534: We diagnosed this to sandboxUid() being called before usingUserNamespace() was called, in setting up /etc/group inside the sandbox. This code desperately needs refactoring. We also moved the /etc/group code to be with the /etc/passwd code, but honestly this code is all spaghetti'd all over the place and needs some more serious tidying than we did here. We also moved some checks to be earlier to improve locality with where the things they are checking come from. Change-Id: Ie29798771f3593c46ec313a32960fa955054aceb --- src/libstore/build/local-derivation-goal.cc | 49 ++++++++------- tests/nixos/broken-userns.nix | 66 +++++++++++++++++++++ tests/nixos/default.nix | 2 + 3 files changed, 95 insertions(+), 22 deletions(-) create mode 100644 tests/nixos/broken-userns.nix diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4278fab85..da1db5771 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -215,17 +215,6 @@ void LocalDerivationGoal::tryLocalBuild() #endif } - #if __linux__ - if (useChroot) { - if (!mountAndPidNamespacesSupported()) { - if (!settings.sandboxFallback) - throw Error("this system does not support the kernel namespaces that are required for sandboxing; use '--no-sandbox' to disable sandboxing"); - debug("auto-disabling sandboxing because the prerequisite namespaces are not available"); - useChroot = false; - } - } - #endif - if (useBuildUsers()) { if (!buildUser) buildUser = acquireUserLock(parsedDrv->useUidRange() ? 65536 : 1, useChroot); @@ -239,6 +228,26 @@ void LocalDerivationGoal::tryLocalBuild() } } + #if __linux__ + // FIXME: should user namespaces being unsupported also require + // sandbox-fallback to be allowed? I don't think so, since they aren't a + // huge security win to have enabled. + usingUserNamespace = userNamespacesSupported(); + + if (useChroot) { + if (!mountAndPidNamespacesSupported()) { + if (!settings.sandboxFallback) + throw Error("this system does not support the kernel namespaces that are required for sandboxing; use '--no-sandbox' to disable sandboxing. Pass --debug for diagnostics on what is broken."); + debug("auto-disabling sandboxing because the prerequisite namespaces are not available"); + useChroot = false; + } + + if (!usingUserNamespace && !buildUser) { + throw Error("cannot perform a sandboxed build because user namespaces are not available.\nIn this Lix's configuration, user namespaces are required due to either being non-root, or build-users-group being disabled without also enabling auto-allocate-uids"); + } + } + #endif + actLock.reset(); try { @@ -697,13 +706,6 @@ void LocalDerivationGoal::startBuilder() if (parsedDrv->useUidRange() && (!buildUser || buildUser->getUIDCount() < 65536)) throw Error("feature 'uid-range' requires the setting '%s' to be enabled", settings.autoAllocateUids.name); - /* Declare the build user's group so that programs get a consistent - view of the system (e.g., "id -gn"). */ - writeFile(chrootRootDir + "/etc/group", - fmt("root:x:0:\n" - "nixbld:!:%1%:\n" - "nogroup:x:65534:\n", sandboxGid())); - /* Create /etc/hosts with localhost entry. */ if (derivationType->isSandboxed()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); @@ -911,8 +913,6 @@ void LocalDerivationGoal::startBuilder() userNamespaceSync.create(); - usingUserNamespace = userNamespacesSupported(); - Pipe sendPid; sendPid.create(); @@ -981,8 +981,6 @@ void LocalDerivationGoal::startBuilder() fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); } else { debug("note: not using a user namespace"); - if (!buildUser) - throw Error("cannot perform a sandboxed build because user namespaces are not enabled; check /proc/sys/user/max_user_namespaces"); } /* Now that we now the sandbox uid, we can write @@ -993,6 +991,13 @@ void LocalDerivationGoal::startBuilder() "nobody:x:65534:65534:Nobody:/:/noshell\n", sandboxUid(), sandboxGid(), settings.sandboxBuildDir)); + /* Declare the build user's group so that programs get a consistent + view of the system (e.g., "id -gn"). */ + writeFile(chrootRootDir + "/etc/group", + fmt("root:x:0:\n" + "nixbld:!:%1%:\n" + "nogroup:x:65534:\n", sandboxGid())); + /* Save the mount- and user namespace of the child. We have to do this *before* the child does a chroot. */ sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY)}; diff --git a/tests/nixos/broken-userns.nix b/tests/nixos/broken-userns.nix new file mode 100644 index 000000000..a48467193 --- /dev/null +++ b/tests/nixos/broken-userns.nix @@ -0,0 +1,66 @@ +# Lix should be able to build derivations that want working NSS, even with +# broken user namespaces support +{ ... }: +let + testDerivation = builtins.toFile "test.nix" '' + { cacheBreak }: + let pkgs = import { }; + in + pkgs.runCommand "test" { } ''' + # ''${cacheBreak} + id -g + id -u + echo "GROUP" + cat /etc/group + echo "PASSWD" + cat /etc/passwd + + username=$(id -un) + groupname=$(id -gn) + [[ "$username" =~ nixbld* ]] + [[ "$groupname" =~ nixbld* ]] + touch $out + ''' + ''; +in +{ + name = "broken-userns"; + + nodes.machine = + { + config, + lib, + pkgs, + ... + }: + { + virtualisation.writableStore = true; + nix.settings.substituters = lib.mkForce [ ]; + nix.nixPath = [ "nixpkgs=${lib.cleanSource pkgs.path}" ]; + virtualisation.additionalPaths = [ + pkgs.stdenvNoCC + testDerivation + ]; + }; + + testScript = + { nodes }: + '' + start_all() + + # Building it normally should work + machine.succeed(r""" + nix-build --argstr cacheBreak 1 --store daemon ${testDerivation} + """) + + # Building it with broken userns should also work + machine.succeed(r""" + # break user ns + sysctl -w user.max_user_namespaces=0 + """) + machine.systemctl("restart nix-daemon") + machine.succeed(r""" + nix-build --argstr cacheBreak 2 --store daemon ${testDerivation} + """) + ''; +} diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix index 3d0a1f0c6..987463b07 100644 --- a/tests/nixos/default.nix +++ b/tests/nixos/default.nix @@ -164,4 +164,6 @@ in symlinkResolvconf = runNixOSTestFor "x86_64-linux" ./symlink-resolvconf.nix; rootInSandbox = runNixOSTestFor "x86_64-linux" ./root-in-sandbox; + + broken-userns = runNixOSTestFor "x86_64-linux" ./broken-userns.nix; }