Merge "Fix /etc/group having desynced IDs from the actual UID in the sandbox" into main

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;
 }