Merge 1b2c828d57 into aa1629ca35

src/libstore/globals.cc

@@ -86,12 +86,12 @@ Settings::Settings()
     }
 
 #if (defined(__linux__) || defined(__FreeBSD__)) && defined(SANDBOX_SHELL)
-    sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL);
+    sandboxPaths = { { "/bin/sh", SANDBOX_SHELL } };
 #endif
 
     /* chroot-like behavior from Apple's sandbox */
 #ifdef __APPLE__
-    sandboxPaths = tokenizeString<StringSet>("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
+    sandboxPaths.setDefault("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
     allowedImpureHostPrefixes = tokenizeString<StringSet>("/System/Library /usr/lib /dev /bin/sh");
 #endif
 }
@@ -296,6 +296,94 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s
     });
 }
 
+NLOHMANN_JSON_SERIALIZE_ENUM(SandboxPath::MountOpt, {
+    {SandboxPath::MountOpt::ro, "ro"},
+#ifdef __linux__
+    {SandboxPath::MountOpt::nodev, "nodev"},
+    {SandboxPath::MountOpt::noexec, "noexec"},
+    {SandboxPath::MountOpt::nosuid, "nosuid"},
+    {SandboxPath::MountOpt::noatime, "noatime"},
+    {SandboxPath::MountOpt::nodiratime, "nodiratime"},
+    {SandboxPath::MountOpt::relatime, "relatime"},
+    {SandboxPath::MountOpt::strictatime, "strictatime"},
+    {SandboxPath::MountOpt::private_, "private"},
+    {SandboxPath::MountOpt::slave, "slave"},
+    {SandboxPath::MountOpt::unbindable, "unbindable"},
+#endif
+});
+
+NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(SandboxPath, source, optional, readOnly, options);
+
+/**
+ * Parses either old ("path[=source][?]" strings) or new (json object) format
+ * sandbox-paths. Legacy format supports only a subset of available settings.
+ */
+SandboxPaths SandboxPath::parse(const std::string_view & str, const std::string & ctx)
+{
+    SandboxPaths res;
+
+    auto add = [&](std::string target, SandboxPath v) {
+        if (target == "")
+            throw UsageError("setting '%s' is an object whose keys are paths and paths cannot be empty", ctx);
+        target = canonPath(std::move(target));
+        if (v.source == "") v.source = target;
+        if (!res.try_emplace(target, std::move(v)).second)
+            throw UsageError("Sandbox path declared twice in '%s': %s", ctx, target);
+    };
+
+    if (str.starts_with('{')) {
+        for (auto & [k, v] : nlohmann::json::parse(str, nullptr, false, true).template get<SandboxPaths>())
+            add(k, std::move(v));
+    } else {
+        for (std::string_view s : tokenizeString<Strings>(str)) {
+            bool optional = s.ends_with('?');
+            if (optional) s.remove_suffix(1);
+            if (size_t eq = s.find('='); eq != s.npos) {
+                add(std::string(s, 0, eq), { std::string(s.data() + eq + 1, s.size() - eq - 1), optional });
+            } else {
+                add(std::string(s), { "", optional });
+            }
+        }
+    }
+    return res;
+}
+
+template<> SandboxPaths BaseSetting<SandboxPaths>::parse(const std::string & str) const
+{
+    return SandboxPath::parse(str, this->name);
+}
+
+template<> struct BaseSetting<SandboxPaths>::trait
+{
+    static constexpr bool appendable = true;
+};
+
+/* Omits keys that are set to their default values. */
+template<> std::string BaseSetting<SandboxPaths>::to_string() const
+{
+    if (value.empty())
+        return "";
+    nlohmann::json res = nlohmann::json::object();
+    for (const auto & [k, v] : value) {
+        auto po = nlohmann::json::object();
+        if (v.source != "" && v.source != k) po.emplace("source", v.source);
+        if (v.optional) po.emplace("optional", v.optional);
+        if (v.readOnly) po.emplace("readOnly", v.readOnly);
+        if (!v.options.empty()) po.emplace("options", v.options);
+        res.emplace(k, std::move(po));
+    }
+    return res.dump();
+}
+
+template<> void BaseSetting<SandboxPaths>::appendOrSet(SandboxPaths newValue, bool append)
+{
+    if (!append) value.clear();
+    for (auto & [k, v] : newValue)
+        value.insert_or_assign(std::move(k), std::move(v));
+}
+
+template class BaseSetting<SandboxPaths>;
+
 unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
 {
     if (str == "auto") return std::max(1U, std::thread::hardware_concurrency());

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

@@ -5,6 +5,9 @@
 #include <limits>
 
 #include <sys/types.h>
+#ifdef __linux__
+#include <sys/mount.h>
+#endif
 
 #include "nix/util/types.hh"
 #include "nix/util/configuration.hh"
@@ -18,6 +21,72 @@ namespace nix {
 
 typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode;
 
+struct SandboxPath;
+using SandboxPaths = std::map<Path, SandboxPath, std::less<>>;
+struct SandboxPath
+{
+public:
+    typedef enum {
+#ifdef __linux__
+        ro = MS_RDONLY,
+        nodev = MS_NODEV,
+        noexec = MS_NOEXEC,
+        nosuid = MS_NOSUID,
+        noatime = MS_NOATIME,
+        nodiratime = MS_NODIRATIME,
+        relatime = MS_RELATIME,
+        strictatime = MS_STRICTATIME, /* overrides any atime/relatime */
+        private_ = MS_PRIVATE,
+        slave = MS_SLAVE,
+        unbindable = MS_UNBINDABLE
+#else
+        ro  // FIXME: do any options make sense on other that linux?
+#endif
+    } MountOpt;
+
+#ifdef __linux__
+    /* Options to set when readOnly=true */
+    constexpr static MountOpt readOnlyDefaults[] = { ro, nodev, noexec, nosuid, noatime };
+
+    /* Only one atime option should be enabled at a time. Same for propagation
+     * style.*/
+    constexpr static std::pair<uint64_t, const char*> exclusiveOptionMasks[] = {
+        {MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME, "option-atime"},
+        {MS_SHARED | MS_PRIVATE | MS_SLAVE, "propagation"},
+    };
+#endif
+
+    Path source;
+
+    /**
+     * Ignore path if source is missing.
+     */
+    bool optional;
+
+    /**
+     * Enables MS_RDONLY, NODEV, NOSUID, NOEXEC and NOATIME. You can get finer
+     * control with 'options' instead.
+     * */
+    bool readOnly;
+
+    std::vector<MountOpt> options;
+
+    SandboxPath(std::string source = "", bool optional = false,
+        bool readOnly = false, std::vector<MountOpt> options = { }) :
+        source(std::string(std::move(source))), optional(optional),
+        readOnly(readOnly), options(std::move(options)) { }
+
+    /* This is to enable the full implicit conversion from e.g. const char[],
+     * even when binding a reference. Code can specify paths with literals and
+     * nothing extra. (Have angried the C++ deities with this? Seems like
+     * there should be a better way?) */
+    template<typename S, typename = std::enable_if_t<std::is_convertible_v<S, Path>>>
+    SandboxPath(S&& source, bool optional = false, bool readOnly = false) :
+        SandboxPath(Path(std::forward<S>(source)), optional, readOnly) { }
+
+    static SandboxPaths parse(const std::string_view & str, const std::string& = "(unknown)");
+};
+
 struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
 {
     MaxBuildJobsSetting(Config * options,
@@ -629,19 +698,76 @@ public:
         )",
         {"build-use-chroot", "build-use-sandbox"}};
 
-    Setting<PathSet> sandboxPaths{
+    Setting<SandboxPaths> sandboxPaths{
         this, {}, "sandbox-paths",
         R"(
-          A list of paths bind-mounted into Nix sandbox environments. You can
+          Paths to bind-mount into Nix sandbox environments.
-          use the syntax `target=source` to mount a path in a different
+          Two syntaxes can be used:
-          location in the sandbox; for instance, `/bin=/nix-bin` will mount
-          the path `/nix-bin` as `/bin` inside the sandbox. If *source* is
-          followed by `?`, then it is 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
+          1. Original (old) syntax: Strings separated by whitespace. Entries
-          the sandbox as well.
+             are parsed as `TARGET[=SOURCE][?]`. Only the `TARGET` path is
+             required.
+
+             `SOURCE` can be set following an equals sign (`=`) to specify a
+             different source path (the value of `TARGET` is used by default
+             for the source path as well). For instance, `/bin=/nix-bin` would
+             mount path `/nix-bin` in `/bin` inside the sandbox.
+
+             A `?` suffix can be used to make it not an error if the `SOURCE`
+             path does not exist. Without it an error is raised for an
+             unavailable path. For instance, `/dev/nvidiactl?` specifies that
+             `/dev/nvidiactl` will only be mounted in the sandbox if it exists
+             in the host filesystem.
+
+          2. JSON syntax (new): Using this form more configurable settings
+             become available. All paths are specified in a single JSON object
+             so that every key is a target path inside the sandbox and the
+             corresponding values can contain additional (platform-specific)
+             settings.
+
+          For instance:
+
+          ```nix
+          sandbox-paths = {"/bin/sh":{}}                         # /bin/sh
+          sandbox-paths = {"/bin/sh":{"source":"/usr/bin/bash"}} # /bin/sh=/usr/bin/bash
+          sandbox-paths = {"/etc/nix/netrc":{"optional":true}}   # /etc/nix/netrc?
+          ```
+
+          Additional per-path options are available on Linux:
+
+          - `readOnly` (boolean)
+
+            When this is `true`, the bind-mount is made read-only and
+            additional mount-point flags are enabled. In particular these
+            options are enabled by this flag: `ro`, `nosuid`, `nodev`,
+            `noexec` and `noatime`.
+
+          - `options` (string array)
+
+            This setting can be used to add/modify (some) mount(-point) flags
+            directly. In addition to flags used by `readOnly` the following
+            flags can also be used: `nodiratime`, `relatime`, `strictatime`,
+            `unbindable`, `private`, `slave`.
+
+          Full example:
+
+          ```nix
+          sandbox-paths = {
+            "/path/to" : {
+              "source"   : "/path/from",                  # ()
+              "optional" : true,                          # (false)
+              "readOnly" : true,                          # (false)
+              "options"  : [ "optionA", "optionB", ... ], # ()
+            },
+
+            # ...
+          }
+          ```
+
+          > **Note:**
+          >
+          > If the source is in the Nix store, then its closure will
+          > be added to the sandbox as well.
 
           Depending on how Nix was built, the default value for this option
           may be empty or provide `/bin/sh` as a bind-mount of `bash`.

src/libstore/path.cc

@@ -77,6 +77,9 @@ StorePath StorePath::random(std::string_view name)
 
 StorePath MixStoreDirMethods::parseStorePath(std::string_view path) const
 {
+    if (path.empty())
+        throw BadStorePath("empty path is not a valid store path");
+
     // On Windows, `/nix/store` is not a canonical path. More broadly it
     // is unclear whether this function should be using the native
     // notion of a canonical path at all. For example, it makes to

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

@@ -105,14 +105,8 @@ protected:
     /**
      * Stuff we need to pass to initChild().
      */
-    struct ChrootPath {
+
-        Path source;
+    typedef SandboxPaths PathsInChroot;
-        bool optional;
-        ChrootPath(Path source = "", bool optional = false)
-            : source(source), optional(optional)
-        { }
-    };
-    typedef std::map<Path, ChrootPath> PathsInChroot; // maps target path to source path
 
     typedef StringMap Environment;
     Environment env;
@@ -849,24 +843,14 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
 
     /* Allow a user-configurable set of directories from the
        host file system. */
-    for (auto i : settings.sandboxPaths.get()) {
+    for (const auto & [k, v] : settings.sandboxPaths.get())
-        if (i.empty()) continue;
+        pathsInChroot.insert_or_assign(k, v);
-        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()))
     {
         throw Error("`sandbox-build-dir` must not contain the storeDir");
     }
-    pathsInChroot[tmpDirInSandbox()] = tmpDir;
+    pathsInChroot.insert_or_assign(tmpDirInSandbox(), tmpDir);
 
     /* Add the closure of store paths to the chroot. */
     StorePathSet closure;
@@ -936,11 +920,8 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
                 if (line == "") {
                     state = stBegin;
                 } else {
-                    auto p = line.find('=');
+                    for (const auto & [k, v] : SandboxPath::parse(line, "extra-sandbox-paths (via pre-build-hook)"))
-                    if (p == std::string::npos)
+                        pathsInChroot.try_emplace(k, v);
-                        pathsInChroot[line] = line;
-                    else
-                        pathsInChroot[line.substr(0, p)] = line.substr(p + 1);
                 }
             }
         }

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

@@ -121,21 +121,44 @@ static void setupSeccomp()
 #  endif
 }
 
-static void doBind(const Path & source, const Path & target, bool optional = false)
+static uint64_t mergeIntoMountOpts(uint64_t oset, uint64_t o) {
+    for (const auto & [mask, _] : SandboxPath::exclusiveOptionMasks)
+        if (o & mask) return (oset & ~mask) | o;
+    return oset | o;
+};
+
+template<typename Iterable>
+static uint64_t combineMountOpts(uint64_t init, const Iterable& opts)
 {
-    debug("bind mounting '%1%' to '%2%'", source, target);
+    return std::transform_reduce(std::begin(opts), std::end(opts), init, mergeIntoMountOpts,
+        [](const SandboxPath::MountOpt & o) { return static_cast<uint64_t>(o); });
+};
+
+static void doBind(const SandboxPath & v, const Path & target)
+{
+    debug("bind mounting '%1%' to '%2%'", v.source, target);
 
     auto bindMount = [&]() {
-        if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1)
+        if (mount(v.source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1)
-            throw SysError("bind mount from '%1%' to '%2%' failed", source, target);
+            throw SysError("bind mount from '%1%' to '%2%' failed", v.source, target);
+
+        // set extra options when wanted
+        auto flags = v.readOnly ? combineMountOpts(0, SandboxPath::readOnlyDefaults) : 0;
+        flags = combineMountOpts(flags, v.options);
+        if (flags != 0) {
+            // initial mount wouldn't respect MS_RDONLY, must remount
+            debug("remounting '%s' with flags: %d", target, flags);
+            if (mount("", target.c_str(), "", MS_REMOUNT | MS_BIND | flags, 0) == -1)
+                throw SysError("mount: updating bind-mount flags of '%s' failed", target);
+        }
     };
 
-    auto maybeSt = maybeLstat(source);
+    auto maybeSt = maybeLstat(v.source);
     if (!maybeSt) {
-        if (optional)
+        if (v.optional)
             return;
         else
-            throw SysError("getting attributes of path '%1%'", source);
+            throw SysError("getting attributes of path '%1%'", v.source);
     }
     auto st = *maybeSt;
 
@@ -145,7 +168,7 @@ static void doBind(const Path & source, const Path & target, bool optional = fal
     } else if (S_ISLNK(st.st_mode)) {
         // Symlinks can (apparently) not be bind-mounted, so just copy it
         createDirs(dirOf(target));
-        copyFile(std::filesystem::path(source), std::filesystem::path(target), false);
+        copyFile(std::filesystem::path(v.source), std::filesystem::path(target), false);
     } else {
         createDirs(dirOf(target));
         writeFile(target, "");
@@ -311,7 +334,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder
         printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootParentDir);
 
         if (mkdir(chrootParentDir.c_str(), 0700) == -1)
-            throw SysError("cannot create '%s'", chrootRootDir);
+            throw SysError("cannot create '%s'", chrootParentDir);
 
         chrootRootDir = chrootParentDir + "/root";
 
@@ -672,7 +695,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder
             // For backwards-compatibility, resolve all the symlinks in the
             // chroot paths.
             auto canonicalPath = canonPath(i, true);
-            pathsInChroot.emplace(i, canonicalPath);
+            pathsInChroot.try_emplace(i, canonicalPath);
         }
 
         /* Bind-mount all the directories from the "host"
@@ -694,7 +717,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder
             } else
 #  endif
             {
-                doBind(i.second.source, chrootRootDir + i.first, i.second.optional);
+                doBind(i.second, chrootRootDir + i.first);
             }
         }