From d171090530f4a2a79efec2c385bee1a10844c706 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 25 Jul 2019 09:37:57 -0400 Subject: [PATCH 1/4] =?UTF-8?q?Disable=20CLONE=5FNEWUSER=20when=20it?= =?UTF-8?q?=E2=80=99s=20unavailable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some kernels disable "unpriveleged user namespaces". This is unfortunate, but we can still use mount namespaces. Anyway, since each builder has its own nixbld user, we already have most of the benefits of user namespaces. --- src/libstore/build.cc | 14 ++++++++++++-- src/nix/run.cc | 5 ++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index cf6428e12..c10005839 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2302,10 +2302,20 @@ void DerivationGoal::startBuilder() flags |= CLONE_NEWNET; pid_t child = clone(childEntry, stack + stackSize, flags, this); - if (child == -1 && errno == EINVAL) + if (child == -1 && errno == EINVAL) { /* Fallback for Linux < 2.13 where CLONE_NEWPID and CLONE_PARENT are not allowed together. */ - child = clone(childEntry, stack + stackSize, flags & ~CLONE_NEWPID, this); + flags &= ~CLONE_NEWPID; + child = clone(childEntry, stack + stackSize, flags, this); + } + if (child == -1 && (errno == EPERM || errno == EINVAL)) { + /* Some distros patch Linux to not allow unpriveleged + * user namespaces. If we get EPERM or EINVAL, try + * without CLONE_NEWUSER and see if that works. + */ + flags &= ~CLONE_NEWUSER; + child = clone(childEntry, stack + stackSize, flags, this); + } if (child == -1) throw SysError("cloning builder process"); writeFull(builderOut.writeSide.get(), std::to_string(child) + "\n"); diff --git a/src/nix/run.cc b/src/nix/run.cc index 35b763345..90b76d666 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -199,7 +199,10 @@ void chrootHelper(int argc, char * * argv) uid_t gid = getgid(); if (unshare(CLONE_NEWUSER | CLONE_NEWNS) == -1) - throw SysError("setting up a private mount namespace"); + /* Try with just CLONE_NEWNS in case user namespaces are + specifically disabled. */ + if (unshare(CLONE_NEWNS) == -1) + throw SysError("setting up a private mount namespace"); /* Bind-mount realStoreDir on /nix/store. If the latter mount point doesn't already exists, we have to create a chroot From 11d853462925d0b57fe956962e07edf5751fd4c3 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 25 Jul 2019 14:29:58 -0400 Subject: [PATCH 2/4] Use sandbox fallback when cloning fails in builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When sandbox-fallback = true (the default), the Nix builder will fall back to disabled sandbox mode when the kernel doesn’t allow users to set it up. This prevents hard errors from occuring in tricky places, especially the initial installer. To restore the previous behavior, users can set: sandbox-fallback = false in their /etc/nix/nix.conf configuration. --- src/libstore/build.cc | 12 +++++++++++- src/libstore/globals.hh | 3 +++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index c10005839..dd08ce7d7 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2316,13 +2316,22 @@ void DerivationGoal::startBuilder() flags &= ~CLONE_NEWUSER; child = clone(childEntry, stack + stackSize, flags, this); } + /* Otherwise exit with EPERM so we can handle this in the + parent. This is only done when sandbox-fallback is set + to true (the default). */ + if (child == -1 && (errno == EPERM || errno == EINVAL) && settings.sandboxFallback) + _exit(EPERM); if (child == -1) throw SysError("cloning builder process"); writeFull(builderOut.writeSide.get(), std::to_string(child) + "\n"); _exit(0); }, options); - if (helper.wait() != 0) + int res = helper.wait(); + if (res == EPERM && settings.sandboxFallback) { + useChroot = false; + goto fallback; + } else if (res != 0) throw Error("unable to start build process"); userNamespaceSync.readSide = -1; @@ -2353,6 +2362,7 @@ void DerivationGoal::startBuilder() } else #endif { + fallback: options.allowVfork = !buildUser && !drv->isBuiltin(); pid = startProcess([&]() { runChild(); diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 0af8215d1..cc9534b27 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -209,6 +209,9 @@ public: "The paths to make available inside the build sandbox.", {"build-chroot-dirs", "build-sandbox-paths"}}; + Setting sandboxFallback{this, true, "sandbox-fallback", + "Whether to disable sandboxing when the kernel doesn't allow it."}; + Setting extraSandboxPaths{this, {}, "extra-sandbox-paths", "Additional paths to make available inside the build sandbox.", {"build-extra-chroot-dirs", "build-extra-sandbox-paths"}}; From 9a0855bbb6546e792848e551e79f8efc40782eeb Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Tue, 30 Jul 2019 17:52:42 -0400 Subject: [PATCH 3/4] =?UTF-8?q?Don=E2=80=99t=20rely=20on=20EPERM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startProcess does not appear to send the exit code to the helper correctly. Not sure why this is, but it is probably safe to just fallback on all sandbox errors. --- src/libstore/build.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index dd08ce7d7..0f71e7511 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2320,7 +2320,7 @@ void DerivationGoal::startBuilder() parent. This is only done when sandbox-fallback is set to true (the default). */ if (child == -1 && (errno == EPERM || errno == EINVAL) && settings.sandboxFallback) - _exit(EPERM); + _exit(1); if (child == -1) throw SysError("cloning builder process"); writeFull(builderOut.writeSide.get(), std::to_string(child) + "\n"); @@ -2328,7 +2328,7 @@ void DerivationGoal::startBuilder() }, options); int res = helper.wait(); - if (res == EPERM && settings.sandboxFallback) { + if (res != 0 && settings.sandboxFallback) { useChroot = false; goto fallback; } else if (res != 0) From 5c06a8d3283139140e765b5f10ad7102a6a3e964 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Fri, 23 Aug 2019 20:24:39 -0400 Subject: [PATCH 4/4] Reset tmpDirInSandbox for unsandboxed --- src/libstore/build.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0f71e7511..96e9b8edd 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2330,6 +2330,7 @@ void DerivationGoal::startBuilder() int res = helper.wait(); if (res != 0 && settings.sandboxFallback) { useChroot = false; + tmpDirInSandbox = tmpDir; goto fallback; } else if (res != 0) throw Error("unable to start build process");