make sandbox path bind mount read-only on request

Signed-off-by: Samuli Thomasson <samuli.thomasson@pm.me>

src/libstore/include/nix/store/globals.hh

@@ -632,13 +632,18 @@ public:
     Setting<PathSet> sandboxPaths{
         this, {}, "sandbox-paths",
         R"(
-          A list of paths bind-mounted into Nix sandbox environments. You can
+          A list of paths bind-mounted into Nix sandbox environments. Use the
-          use the syntax `target=source` to mount a path in a different
+          syntax `target[=source][:ro][?]` to control the mount:
-          location in the sandbox; for instance, `/bin=/nix-bin` will mount
+
-          the path `/nix-bin` as `/bin` inside the sandbox. If *source* is
+          - `=source` will mount a different path at target location; for
-          followed by `?`, then it is not an error if *source* does not exist;
+            instance, `/bin=/nix-bin` will mount the path `/nix-bin` as `/bin`
-          for example, `/dev/nvidiactl?` specifies that `/dev/nvidiactl` will
+            inside the sandbox.
-          only be mounted in the sandbox if it exists in the host filesystem.
+
+          - `:ro` makes the mount read-only (Linux only).
+
+          - `?` makes it not an error if *source* does not exist; for example,
+            `/dev/nvidiactl?` specifies that `/dev/nvidiactl` will only be
+            mounted in the sandbox if it exists in the host filesystem.
 
           If the source is in the Nix store, then its closure will be added to
           the sandbox as well.

src/libstore/unix/build/derivation-builder.cc

@@ -108,8 +108,9 @@ protected:
     struct ChrootPath {
         Path source;
         bool optional;
-        ChrootPath(Path source = "", bool optional = false)
+        bool rdonly;
-            : source(source), optional(optional)
+        ChrootPath(Path source = "", bool optional = false, bool rdonly = false)
+            : source(source), optional(optional), rdonly(rdonly)
         { }
     };
     typedef std::map<Path, ChrootPath> PathsInChroot; // maps target path to source path
@@ -847,20 +848,29 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
 {
     PathsInChroot pathsInChroot;
 
+    auto addPathWithOptions = [&](std::string s) {
+        if (s.empty()) return;
+        bool optional = false;
+        bool rdonly = false;
+        if (s[s.size() - 1] == '?') {
+            optional = true;
+            s.pop_back();
+        }
+        if (s.size() > 3 && s.substr(s.size() - 3) == ":ro") {
+            rdonly = true;
+            s.resize(s.size() - 3);
+        }
+        size_t p = s.find('=');
+        if (p == std::string::npos)
+            pathsInChroot[s] = {s, optional, rdonly};
+        else
+            pathsInChroot[s.substr(0, p)] = {s.substr(p + 1), optional, rdonly};
+    };
+
     /* Allow a user-configurable set of directories from the
        host file system. */
     for (auto i : settings.sandboxPaths.get()) {
-        if (i.empty()) continue;
+        addPathWithOptions(i);
-        bool optional = false;
-        if (i[i.size() - 1] == '?') {
-            optional = true;
-            i.pop_back();
-        }
-        size_t p = i.find('=');
-        if (p == std::string::npos)
-            pathsInChroot[i] = {i, optional};
-        else
-            pathsInChroot[i.substr(0, p)] = {i.substr(p + 1), optional};
     }
     if (hasPrefix(store.storeDir, tmpDirInSandbox()))
     {
@@ -936,11 +946,7 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
                 if (line == "") {
                     state = stBegin;
                 } else {
-                    auto p = line.find('=');
+                    addPathWithOptions(line);
-                    if (p == std::string::npos)
-                        pathsInChroot[line] = line;
-                    else
-                        pathsInChroot[line.substr(0, p)] = line.substr(p + 1);
                 }
             }
         }

src/libstore/unix/build/linux-derivation-builder.cc

@@ -121,13 +121,18 @@ static void setupSeccomp()
 #  endif
 }
 
-static void doBind(const Path & source, const Path & target, bool optional = false)
+static void doBind(const Path & source, const Path & target, bool optional = false, bool rdonly = false)
 {
     debug("bind mounting '%1%' to '%2%'", source, target);
 
     auto bindMount = [&]() {
         if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1)
             throw SysError("bind mount from '%1%' to '%2%' failed", source, target);
+
+        if (rdonly)
+            // initial mount wouldn't respect MS_RDONLY, must remount
+            if (mount("", target.c_str(), "", MS_REMOUNT | MS_BIND | MS_RDONLY, 0) == -1)
+                throw (SysError("making bind mount '%s' read-only failed", target));
     };
 
     auto maybeSt = maybeLstat(source);