From 85e93d7b874f99730387714394bb60407cf138d5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 6 Jun 2017 18:44:49 +0200 Subject: [PATCH] Always use the Darwin sandbox Even with "build-use-sandbox = false", we now use sandboxing with a permissive profile that allows everything except the creation of setuid/setgid binaries. --- .gitignore | 4 +- src/libstore/build.cc | 170 ++++++++++++++++--------------- src/libstore/local.mk | 6 +- src/libstore/sandbox-defaults.sb | 2 + src/libstore/sandbox-minimal.sb | 5 + 5 files changed, 100 insertions(+), 87 deletions(-) create mode 100644 src/libstore/sandbox-minimal.sb diff --git a/.gitignore b/.gitignore index 1cf7a3c35..616308738 100644 --- a/.gitignore +++ b/.gitignore @@ -48,9 +48,7 @@ perl/Makefile.config /src/libexpr/nix.tbl # /src/libstore/ -/src/libstore/schema.sql.gen.hh -/src/libstore/sandbox-defaults.sb.gen.hh -/src/libstore/sandbox-network.sb.gen.hh +/src/libstore/*.gen.hh /src/nix/nix diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 55c8ac588..d12a1a791 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2611,104 +2611,102 @@ void DerivationGoal::runChild() const char *builder = "invalid"; - string sandboxProfile; if (drv->isBuiltin()) { ; } #if __APPLE__ - else if (useChroot) { - /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ - PathSet ancestry; + else { + /* This has to appear before import statements. */ + std::string sandboxProfile = "(version 1)\n"; - /* We build the ancestry before adding all inputPaths to the store because we know they'll - all have the same parents (the store), and there might be lots of inputs. This isn't - particularly efficient... I doubt it'll be a bottleneck in practice */ - for (auto & i : dirsInChroot) { - Path cur = i.first; - while (cur.compare("/") != 0) { - cur = dirOf(cur); - ancestry.insert(cur); + if (useChroot) { + + /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */ + PathSet ancestry; + + /* We build the ancestry before adding all inputPaths to the store because we know they'll + all have the same parents (the store), and there might be lots of inputs. This isn't + particularly efficient... I doubt it'll be a bottleneck in practice */ + for (auto & i : dirsInChroot) { + Path cur = i.first; + while (cur.compare("/") != 0) { + cur = dirOf(cur); + ancestry.insert(cur); + } } - } - /* And we want the store in there regardless of how empty dirsInChroot. We include the innermost - path component this time, since it's typically /nix/store and we care about that. */ - Path cur = worker.store.storeDir; - while (cur.compare("/") != 0) { - ancestry.insert(cur); - cur = dirOf(cur); - } + /* And we want the store in there regardless of how empty dirsInChroot. We include the innermost + path component this time, since it's typically /nix/store and we care about that. */ + Path cur = worker.store.storeDir; + while (cur.compare("/") != 0) { + ancestry.insert(cur); + cur = dirOf(cur); + } - /* Add all our input paths to the chroot */ - for (auto & i : inputPaths) - dirsInChroot[i] = i; + /* Add all our input paths to the chroot */ + for (auto & i : inputPaths) + dirsInChroot[i] = i; - /* This has to appear before import statements */ - sandboxProfile += "(version 1)\n"; + /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */ + if (settings.darwinLogSandboxViolations) { + sandboxProfile += "(deny default)\n"; + } else { + sandboxProfile += "(deny default (with no-log))\n"; + } - /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */ - if (settings.darwinLogSandboxViolations) { - sandboxProfile += "(deny default)\n"; - } else { - sandboxProfile += "(deny default (with no-log))\n"; - } - - sandboxProfile += - #include "sandbox-defaults.sb.gen.hh" - ; - - if (fixedOutput) sandboxProfile += - #include "sandbox-network.sb.gen.hh" + #include "sandbox-defaults.sb.gen.hh" ; - /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms - to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ - Path globalTmpDir = canonPath(getEnv("TMPDIR", "/tmp"), true); + if (fixedOutput) + sandboxProfile += + #include "sandbox-network.sb.gen.hh" + ; - /* They don't like trailing slashes on subpath directives */ - if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); - - /* Our rwx outputs */ - sandboxProfile += "(allow file-read* file-write* process-exec\n"; - for (auto & i : missingPaths) { - sandboxProfile += (format("\t(subpath \"%1%\")\n") % i.c_str()).str(); - } - sandboxProfile += ")\n"; - - /* Our inputs (transitive dependencies and any impurities computed above) - - without file-write* allowed, access() incorrectly returns EPERM - */ - sandboxProfile += "(allow file-read* file-write* process-exec\n"; - for (auto & i : dirsInChroot) { - if (i.first != i.second.source) - throw Error(format( - "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin") - % i.first % i.second.source); - - string path = i.first; - struct stat st; - if (lstat(path.c_str(), &st)) { - if (i.second.optional && errno == ENOENT) - continue; - throw SysError(format("getting attributes of path ‘%1%’") % path); + /* Our rwx outputs */ + sandboxProfile += "(allow file-read* file-write* process-exec\n"; + for (auto & i : missingPaths) { + sandboxProfile += (format("\t(subpath \"%1%\")\n") % i.c_str()).str(); } - if (S_ISDIR(st.st_mode)) - sandboxProfile += (format("\t(subpath \"%1%\")\n") % path).str(); - else - sandboxProfile += (format("\t(literal \"%1%\")\n") % path).str(); - } - sandboxProfile += ")\n"; + sandboxProfile += ")\n"; - /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ - sandboxProfile += "(allow file-read*\n"; - for (auto & i : ancestry) { - sandboxProfile += (format("\t(literal \"%1%\")\n") % i.c_str()).str(); - } - sandboxProfile += ")\n"; + /* Our inputs (transitive dependencies and any impurities computed above) - sandboxProfile += additionalSandboxProfile; + without file-write* allowed, access() incorrectly returns EPERM + */ + sandboxProfile += "(allow file-read* file-write* process-exec\n"; + for (auto & i : dirsInChroot) { + if (i.first != i.second.source) + throw Error(format( + "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin") + % i.first % i.second.source); + + string path = i.first; + struct stat st; + if (lstat(path.c_str(), &st)) { + if (i.second.optional && errno == ENOENT) + continue; + throw SysError(format("getting attributes of path ‘%1%’") % path); + } + if (S_ISDIR(st.st_mode)) + sandboxProfile += (format("\t(subpath \"%1%\")\n") % path).str(); + else + sandboxProfile += (format("\t(literal \"%1%\")\n") % path).str(); + } + sandboxProfile += ")\n"; + + /* Allow file-read* on full directory hierarchy to self. Allows realpath() */ + sandboxProfile += "(allow file-read*\n"; + for (auto & i : ancestry) { + sandboxProfile += (format("\t(literal \"%1%\")\n") % i.c_str()).str(); + } + sandboxProfile += ")\n"; + + sandboxProfile += additionalSandboxProfile; + } else + sandboxProfile += + #include "sandbox-minimal.sb.gen.hh" + ; debug("Generated sandbox profile:"); debug(sandboxProfile); @@ -2717,6 +2715,13 @@ void DerivationGoal::runChild() writeFile(sandboxFile, sandboxProfile); + /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms + to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */ + Path globalTmpDir = canonPath(getEnv("TMPDIR", "/tmp"), true); + + /* They don't like trailing slashes on subpath directives */ + if (globalTmpDir.back() == '/') globalTmpDir.pop_back(); + builder = "/usr/bin/sandbox-exec"; args.push_back("sandbox-exec"); args.push_back("-f"); @@ -2725,12 +2730,13 @@ void DerivationGoal::runChild() args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir); args.push_back(drv->builder); } -#endif +#else else { builder = drv->builder.c_str(); string builderBasename = baseNameOf(drv->builder); args.push_back(builderBasename); } +#endif for (auto & i : drv->args) args.push_back(rewriteStrings(i, inputRewrites)); diff --git a/src/libstore/local.mk b/src/libstore/local.mk index c0cc91c26..36b270f2e 100644 --- a/src/libstore/local.mk +++ b/src/libstore/local.mk @@ -36,7 +36,9 @@ libstore_CXXFLAGS = \ $(d)/local-store.cc: $(d)/schema.sql.gen.hh -$(d)/build.cc: $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh +sandbox-headers = $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh $(d)/sandbox-minimal.sb.gen.hh + +$(d)/build.cc: $(sandbox-headers) %.gen.hh: % @echo 'R"foo(' >> $@.tmp @@ -44,6 +46,6 @@ $(d)/build.cc: $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh @echo ')foo"' >> $@.tmp @mv $@.tmp $@ -clean-files += $(d)/schema.sql.gen.hh $(d)/sandbox-defaults.sb.gen.hh $(d)/sandbox-network.sb.gen.hh +clean-files += $(d)/schema.sql.gen.hh $(sandbox-headers) $(eval $(call install-file-in, $(d)/nix-store.pc, $(prefix)/lib/pkgconfig, 0644)) diff --git a/src/libstore/sandbox-defaults.sb b/src/libstore/sandbox-defaults.sb index 0292f5ee8..d63c8f813 100644 --- a/src/libstore/sandbox-defaults.sb +++ b/src/libstore/sandbox-defaults.sb @@ -1,5 +1,7 @@ (define TMPDIR (param "_GLOBAL_TMP_DIR")) +(deny default) + ; Disallow creating setuid/setgid binaries, since that ; would allow breaking build user isolation. (deny file-write-setugid) diff --git a/src/libstore/sandbox-minimal.sb b/src/libstore/sandbox-minimal.sb new file mode 100644 index 000000000..65f5108b3 --- /dev/null +++ b/src/libstore/sandbox-minimal.sb @@ -0,0 +1,5 @@ +(allow default) + +; Disallow creating setuid/setgid binaries, since that +; would allow breaking build user isolation. +(deny file-write-setugid)