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

3 files changed, 95 insertions, 22 deletions

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 <nixpkgs> { };
+    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;
 }