Merge branch 'sandbox-profiles' of git://github.com/pikajude/nix

Temporarily allow derivations to describe their full sandbox profile.
This will be eventually scaled back to a more secure setup, see the
discussion at #695

corepkgs/buildenv.nix

@@ -23,10 +23,20 @@ derivation {
   # network traffic, so don't do that.
   preferLocalBuild = true;
 
-  __impureHostDeps = if builtins.currentSystem == "x86_64-darwin" then [
+  __sandboxProfile = ''
-    "/usr/lib/libSystem.dylib"
+    (allow sysctl-read)
-    "/usr/lib/system"
+    (allow file-read*
-  ] else null;
+           (literal "/usr/lib/libSystem.dylib")
+           (literal "/usr/lib/libSystem.B.dylib")
+           (literal "/usr/lib/libobjc.A.dylib")
+           (literal "/usr/lib/libobjc.dylib")
+           (literal "/usr/lib/libauto.dylib")
+           (literal "/usr/lib/libc++abi.dylib")
+           (literal "/usr/lib/libc++.1.dylib")
+           (literal "/usr/lib/libDiagnosticMessagesClient.dylib")
+           (subpath "/usr/lib/system")
+           (subpath "/dev"))
+  '';
 
   inherit chrootDeps;
 }

release.nix

@@ -97,6 +97,10 @@ let
 
         enableParallelBuilding = true;
 
+        __sandboxProfile = lib.sandbox.allowFileRead [
+          "/etc" "/etc/nix/nix.conf" "/private/etc/nix/nix.conf"
+        ];
+
         makeFlags = "profiledir=$(out)/etc/profile.d";
 
         preBuild = "unset NIX_INDENT_MAKE";

src/libstore/build.cc

@@ -778,6 +778,12 @@ private:
     DirsInChroot dirsInChroot;
     typedef map<string, string> Environment;
     Environment env;
+#if SANDBOX_ENABLED
+    typedef string SandboxProfile;
+    SandboxProfile additionalSandboxProfile;
+
+    AutoDelete autoDelSandbox;
+#endif
 
     /* Hash rewriting. */
     HashRewrites rewritesToTmp, rewritesFromTmp;
@@ -1919,6 +1925,9 @@ void DerivationGoal::startBuilder()
         for (auto & i : closure)
             dirsInChroot[i] = i;
 
+#if SANDBOX_ENABLED
+        additionalSandboxProfile = get(drv->env, "__sandboxProfile");
+#else
         string allowed = settings.get("allowed-impure-host-deps", string(DEFAULT_ALLOWED_IMPURE_PREFIXES));
         PathSet allowedPaths = tokenizeString<StringSet>(allowed);
 
@@ -1944,6 +1953,7 @@ void DerivationGoal::startBuilder()
 
             dirsInChroot[i] = i;
         }
+#endif
 
 #if CHROOT_ENABLED
         /* Create a temporary directory in which we set up the chroot
@@ -2439,9 +2449,10 @@ void DerivationGoal::runChild()
         const char *builder = "invalid";
 
         string sandboxProfile;
-        if (isBuiltin(*drv))
+        if (isBuiltin(*drv)) {
             ;
-        else if (useChroot && SANDBOX_ENABLED) {
+#if SANDBOX_ENABLED
+        } else if (useChroot) {
             /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */
             PathSet ancestry;
 
@@ -2471,8 +2482,6 @@ void DerivationGoal::runChild()
             /* This has to appear before import statements */
             sandboxProfile += "(version 1)\n";
 
-            sandboxProfile += (format("(import \"%1%/nix/sandbox-defaults.sb\")\n") % settings.nixDataDir).str();
-
             /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */
             if (settings.get("darwin-log-sandbox-violations", false)) {
                 sandboxProfile += "(deny default)\n";
@@ -2494,13 +2503,8 @@ void DerivationGoal::runChild()
             }
             sandboxProfile += ")\n";
 
-            /* Our inputs (transitive dependencies and any impurities computed above)
+            /* Our inputs (transitive dependencies and any impurities computed above) */
-               Note that the sandbox profile allows file-write* even though it isn't seemingly necessary. First of all, nix's standard user permissioning
+            sandboxProfile += "(allow file-read* process-exec\n";
-               mechanism still prevents builders from writing to input directories, so no security/purity is lost. The reason we allow file-write* is that
-               denying it means the `access` syscall will return EPERM instead of EACCESS, which confuses a few programs that assume (understandably, since
-               it appears to be a violation of the POSIX spec) that `access` won't do that, and don't deal with it nicely if it does. The most notable of
-               these is the entire GHC Haskell ecosystem. */
-            sandboxProfile += "(allow file-read* file-write* process-exec mach-priv-task-port\n";
             for (auto & i : dirsInChroot) {
                 if (i.first != i.second)
                     throw SysError(format("can't map '%1%' to '%2%': mismatched impure paths not supported on darwin"));
@@ -2516,28 +2520,32 @@ void DerivationGoal::runChild()
             }
             sandboxProfile += ")\n";
 
-            /* Our ancestry. N.B: this uses literal on folders, instead of subpath. Without that,
+            /* Allow file-read* on full directory hierarchy to self. Allows realpath() */
-               you open up the entire filesystem because you end up with (subpath "/")
-               Note: file-read-metadata* is not sufficiently permissive for GHC. file-read* is but may
-               be a security hazard.
-               TODO: figure out a more appropriate directive.
-             */
             sandboxProfile += "(allow file-read*\n";
             for (auto & i : ancestry) {
                 sandboxProfile += (format("\t(literal \"%1%\")\n") % i.c_str()).str();
             }
             sandboxProfile += ")\n";
 
+            sandboxProfile += additionalSandboxProfile;
+
             debug("Generated sandbox profile:");
             debug(sandboxProfile);
 
+            Path sandboxFile = drvPath + ".sb";
+            if (pathExists(sandboxFile)) deletePath(sandboxFile);
+            autoDelSandbox.reset(sandboxFile, false);
+
+            writeFile(sandboxFile, sandboxProfile);
+
             builder = "/usr/bin/sandbox-exec";
             args.push_back("sandbox-exec");
-            args.push_back("-p");
+            args.push_back("-f");
-            args.push_back(sandboxProfile);
+            args.push_back(sandboxFile);
             args.push_back("-D");
-            args.push_back((format("_GLOBAL_TMP_DIR=%1%") % globalTmpDir).str());
+            args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir);
             args.push_back(drv->builder);
+#endif
         } else {
             builder = drv->builder.c_str();
             string builderBasename = baseNameOf(drv->builder);

src/libstore/local.mk

@@ -33,4 +33,3 @@ $(d)/local-store.cc: $(d)/schema.sql.hh
 clean-files += $(d)/schema.sql.hh
 
 $(eval $(call install-file-in, $(d)/nix-store.pc, $(prefix)/lib/pkgconfig, 0644))
-$(eval $(call install-file-in, $(d)/sandbox-defaults.sb, $(datadir)/nix, 0644))

src/libstore/sandbox-defaults.sb.in

@@ -1,63 +0,0 @@
-(allow file-read* file-write-data (literal "/dev/null"))
-(allow ipc-posix*)
-(allow mach-lookup (global-name "com.apple.SecurityServer"))
-
-(allow file-read*
-       (literal "/dev/dtracehelper")
-       (literal "/dev/tty")
-       (literal "/dev/autofs_nowait")
-       (literal "/System/Library/CoreServices/SystemVersion.plist")
-       (literal "/private/var/run/systemkeychaincheck.done")
-       (literal "/private/etc/protocols")
-       (literal "/private/var/tmp")
-       (literal "/private/var/db")
-       (subpath "/private/var/db/mds"))
-
-(allow file-read*
-       (subpath "/usr/share/icu")
-       (subpath "/usr/share/locale")
-       (subpath "/usr/share/zoneinfo"))
-
-(allow file-write*
-       (literal "/dev/tty")
-       (literal "/dev/dtracehelper")
-       (literal "/mds"))
-
-(allow file-ioctl (literal "/dev/dtracehelper"))
-
-(allow file-read-metadata
-       (literal "/var")
-       (literal "/tmp")
-       ; symlinks
-       (literal "@sysconfdir@")
-       (literal "@sysconfdir@/nix")
-       (literal "@sysconfdir@/nix/nix.conf")
-       (literal "/etc/resolv.conf")
-       (literal "/private/etc/resolv.conf"))
-
-(allow file-read*
-       (literal "/private@sysconfdir@/nix/nix.conf")
-       (literal "/private/var/run/resolv.conf"))
-
-; some builders use filehandles other than stdin/stdout
-(allow file*
-        (subpath "/dev/fd")
-        (literal "/dev/ptmx")
-        (regex #"^/dev/[pt]ty.*$"))
-
-; allow everything inside TMP
-(allow file* process-exec
-       (subpath (param "_GLOBAL_TMP_DIR"))
-       (subpath "/private/tmp"))
-
-(allow process-fork)
-(allow sysctl-read)
-(allow signal (target same-sandbox))
-
-; allow getpwuid (for git and other packages)
-(allow mach-lookup
-       (global-name "com.apple.system.notification_center")
-       (global-name "com.apple.system.opendirectoryd.libinfo"))
-
-; allow local networking
-(allow network* (local ip) (remote unix-socket))

src/libutil/util.cc

@@ -599,6 +599,8 @@ string drainFD(int fd)
 //////////////////////////////////////////////////////////////////////
 
 
+AutoDelete::AutoDelete() : del{false} {}
+
 AutoDelete::AutoDelete(const string & p, bool recursive) : path(p)
 {
     del = true;
@@ -626,6 +628,12 @@ void AutoDelete::cancel()
     del = false;
 }
 
+void AutoDelete::reset(const Path & p, bool recursive) {
+    path = p;
+    this->recursive = recursive;
+    del = true;
+}
+
 
 
 //////////////////////////////////////////////////////////////////////

src/libutil/util.hh

@@ -199,9 +199,11 @@ class AutoDelete
     bool del;
     bool recursive;
 public:
+    AutoDelete();
     AutoDelete(const Path & p, bool recursive = true);
     ~AutoDelete();
     void cancel();
+    void reset(const Path & p, bool recursive = true);
     operator Path() const { return path; }
 };