From 8f56724e2bc00b7301cb4324a7ace7713ab56dd3 Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Sun, 1 Jun 2025 09:54:55 +0200 Subject: [PATCH 1/5] make sandbox path bind mount read-only on request Signed-off-by: Samuli Thomasson --- src/libstore/include/nix/store/globals.hh | 19 +++++---- src/libstore/unix/build/derivation-builder.cc | 42 +++++++++++-------- .../unix/build/linux-derivation-builder.cc | 7 +++- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 00d929236..42b5ac2c0 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -632,13 +632,18 @@ public: Setting sandboxPaths{ this, {}, "sandbox-paths", R"( - A list of paths bind-mounted into Nix sandbox environments. You can - use the syntax `target=source` to mount a path in a different - 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. + A list of paths bind-mounted into Nix sandbox environments. Use the + syntax `target[=source][:ro][?]` to control the mount: + + - `=source` will mount a different path at target location; for + instance, `/bin=/nix-bin` will mount the path `/nix-bin` as `/bin` + inside the sandbox. + + - `: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. diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 1f15cc9e9..89ccf2d2d 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -108,8 +108,9 @@ protected: struct ChrootPath { Path source; bool optional; - ChrootPath(Path source = "", bool optional = false) - : source(source), optional(optional) + bool rdonly; + ChrootPath(Path source = "", bool optional = false, bool rdonly = false) + : source(source), optional(optional), rdonly(rdonly) { } }; typedef std::map 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; - 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}; + addPathWithOptions(i); } if (hasPrefix(store.storeDir, tmpDirInSandbox())) { @@ -936,11 +946,7 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() if (line == "") { state = stBegin; } else { - auto p = line.find('='); - if (p == std::string::npos) - pathsInChroot[line] = line; - else - pathsInChroot[line.substr(0, p)] = line.substr(p + 1); + addPathWithOptions(line); } } } diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index c27b87163..2e3424358 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/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); From 6041531716c29dc7ad219299e2b28782ba75bb4f Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Mon, 2 Jun 2025 01:53:59 +0200 Subject: [PATCH 2/5] throw BadStorePath for empty string some of the functional tests that run config check wouldn't tolerate some oddities in `PATH`, something like a null string entry (e.g. `::` in the middle or a trailing `:`) I think. This allows the test pass. Signed-off-by: Samuli Thomasson --- src/libstore/path.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/path.cc b/src/libstore/path.cc index d989b1caa..595cf77e1 100644 --- a/src/libstore/path.cc +++ b/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 From c9651a0cbd00f55ad94e850a141a5d90bb1cb585 Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Fri, 13 Jun 2025 17:07:08 +0200 Subject: [PATCH 3/5] sandbox-paths: rewrite read-only paths to use json config format Signed-off-by: Samuli Thomasson --- src/libstore/globals.cc | 92 +++++++++++- src/libstore/include/nix/store/globals.hh | 131 ++++++++++++++++-- src/libstore/unix/build/derivation-builder.cc | 41 ++---- .../unix/build/linux-derivation-builder.cc | 32 ++++- 4 files changed, 243 insertions(+), 53 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index de5128347..a7297fc69 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -86,12 +86,12 @@ Settings::Settings() } #if (defined(__linux__) || defined(__FreeBSD__)) && defined(SANDBOX_SHELL) - sandboxPaths = tokenizeString("/bin/sh=" SANDBOX_SHELL); + sandboxPaths = SandboxPaths { { "/bin/sh", SandboxPath(SANDBOX_SHELL) } }; #endif /* chroot-like behavior from Apple's sandbox */ #ifdef __APPLE__ - sandboxPaths = tokenizeString("/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("/System/Library /usr/lib /dev /bin/sh"); #endif } @@ -296,6 +296,94 @@ template<> void BaseSetting::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 (strings) or new (json object) format sandbox-paths. + */ +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()) + add(k, std::move(v)); + } else { + /* Parses legacy format sandbox-path e.g. "path[=source][?]". + * This format supports only a subset of options available with JSON format. */ + for (std::string_view s : tokenizeString(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::parse(const std::string & str) const +{ + return SandboxPath().parse(str, this->name); +} + +template<> struct BaseSetting::trait +{ + static constexpr bool appendable = true; +}; + +/* Omits keys that are set to their default values. */ +template<> std::string BaseSetting::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::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; + unsigned int MaxBuildJobsSetting::parse(const std::string & str) const { if (str == "auto") return std::max(1U, std::thread::hardware_concurrency()); diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 42b5ac2c0..4b08cd6ff 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -5,6 +5,9 @@ #include #include +#ifdef __linux__ +#include +#endif #include "nix/util/types.hh" #include "nix/util/configuration.hh" @@ -18,6 +21,58 @@ namespace nix { typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode; +struct SandboxPath; +using SandboxPaths = std::map>; +struct SandboxPath +{ + 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 */ + static constexpr std::array readOnlyDefaults = { + ro, nodev, noexec, nosuid, noatime + }; +#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 options; + + SandboxPath(const Path & source = "", + bool optional = false, bool readOnly = false, const std::vector & options = { }) + : source(source), optional(optional), readOnly(readOnly), options(options) { }; + SandboxPath(const char * source) : SandboxPath(Path(source)) { }; + + static SandboxPaths parse(const std::string_view & str, const std::string& = "(unknown)"); +}; + struct MaxBuildJobsSetting : public BaseSetting { MaxBuildJobsSetting(Config * options, @@ -629,24 +684,76 @@ public: )", {"build-use-chroot", "build-use-sandbox"}}; - Setting sandboxPaths{ + Setting sandboxPaths{ this, {}, "sandbox-paths", R"( - A list of paths bind-mounted into Nix sandbox environments. Use the - syntax `target[=source][:ro][?]` to control the mount: + Paths to bind-mount into Nix sandbox environments. + Two syntaxes can be used: - - `=source` will mount a different path at target location; for - instance, `/bin=/nix-bin` will mount the path `/nix-bin` as `/bin` - inside the sandbox. + 1. Original (old) syntax: Strings separated by whitespace. Entries + are parsed as `TARGET[=SOURCE][?]`. Only the `TARGET` path is + required. - - `:ro` makes the mount read-only (Linux only). + `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. - - `?` 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. + 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. - If the source is in the Nix store, then its closure will be added to - the sandbox as well. + 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`. diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 89ccf2d2d..406f57586 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -105,15 +105,8 @@ protected: /** * Stuff we need to pass to initChild(). */ - struct ChrootPath { - Path source; - bool optional; - bool rdonly; - ChrootPath(Path source = "", bool optional = false, bool rdonly = false) - : source(source), optional(optional), rdonly(rdonly) - { } - }; - typedef std::map PathsInChroot; // maps target path to source path + + typedef SandboxPaths PathsInChroot; // maps target path to source path typedef StringMap Environment; Environment env; @@ -848,35 +841,16 @@ 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()) { - addPathWithOptions(i); - } + for (const auto & [k, v] : settings.sandboxPaths.get()) + pathsInChroot.insert_or_assign(k, v); + 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; @@ -946,7 +920,8 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() if (line == "") { state = stBegin; } else { - addPathWithOptions(line); + for (const auto & [k, v] : SandboxPath().parse(line)) + pathsInChroot.try_emplace(k, v); } } } diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 2e3424358..6f4ec5dbb 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -121,18 +121,38 @@ static void setupSeccomp() # endif } -static void doBind(const Path & source, const Path & target, bool optional = false, bool rdonly = false) +static auto combineMountOpts(auto init, auto iter) { + return std::transform_reduce(iter.cbegin(), iter.cend(), init, + [](unsigned long a, unsigned long b) { + if (b & (MS_NOATIME | MS_RELATIME | MS_NODIRATIME | MS_STRICTATIME)) { + return (a & ~(MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME)) | b; + } + return a | b; + }, + [](const SandboxPath::MountOpt & o) { return static_cast(o); }); +}; + + +static void doBind(const SandboxPath & pval, const Path & target) +{ + auto source = pval.source; + auto optional = pval.optional; 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) + // set extra options when wanted + auto flags = pval.readOnly ? combineMountOpts(0, pval.readOnlyDefaults) : 0; + flags = combineMountOpts(flags, pval.options); + if (flags != 0) { // 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)); + 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); @@ -677,7 +697,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" @@ -699,7 +719,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder } else # endif { - doBind(i.second.source, chrootRootDir + i.first, i.second.optional); + doBind(i.second, chrootRootDir + i.first); } } From b01dcf8985109cac40445c6a97674c4ca4ab9d66 Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Sat, 14 Jun 2025 02:05:36 +0200 Subject: [PATCH 4/5] improve linux chroot-related error message Signed-off-by: Samuli Thomasson --- src/libstore/unix/build/linux-derivation-builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 6f4ec5dbb..8bf5e3d3b 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -336,7 +336,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"; From 1b2c828d57e72cc2319ad86a477eb9dee39946da Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Sat, 14 Jun 2025 11:58:04 +0200 Subject: [PATCH 5/5] clean up Signed-off-by: Samuli Thomasson --- src/libstore/globals.cc | 32 +++++++------- src/libstore/include/nix/store/globals.hh | 28 +++++++++--- src/libstore/unix/build/derivation-builder.cc | 4 +- .../unix/build/linux-derivation-builder.cc | 44 +++++++++---------- 4 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index a7297fc69..5fa759eca 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -86,7 +86,7 @@ Settings::Settings() } #if (defined(__linux__) || defined(__FreeBSD__)) && defined(SANDBOX_SHELL) - sandboxPaths = SandboxPaths { { "/bin/sh", SandboxPath(SANDBOX_SHELL) } }; + sandboxPaths = { { "/bin/sh", SANDBOX_SHELL } }; #endif /* chroot-like behavior from Apple's sandbox */ @@ -297,25 +297,26 @@ template<> void BaseSetting::convertToArg(Args & args, const std::s } NLOHMANN_JSON_SERIALIZE_ENUM(SandboxPath::MountOpt, { - {SandboxPath::MountOpt::ro, "ro"}, + {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::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"}, + {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 (strings) or new (json object) format sandbox-paths. + * 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) { @@ -334,15 +335,14 @@ SandboxPaths SandboxPath::parse(const std::string_view & str, const std::string for (auto & [k, v] : nlohmann::json::parse(str, nullptr, false, true).template get()) add(k, std::move(v)); } else { - /* Parses legacy format sandbox-path e.g. "path[=source][?]". - * This format supports only a subset of options available with JSON format. */ for (std::string_view s : tokenizeString(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 + } else { add(std::string(s), { "", optional }); + } } } return res; @@ -350,7 +350,7 @@ SandboxPaths SandboxPath::parse(const std::string_view & str, const std::string template<> SandboxPaths BaseSetting::parse(const std::string & str) const { - return SandboxPath().parse(str, this->name); + return SandboxPath::parse(str, this->name); } template<> struct BaseSetting::trait diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 4b08cd6ff..92c469ec8 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -25,6 +25,7 @@ struct SandboxPath; using SandboxPaths = std::map>; struct SandboxPath { +public: typedef enum { #ifdef __linux__ ro = MS_RDONLY, @@ -39,14 +40,19 @@ struct SandboxPath slave = MS_SLAVE, unbindable = MS_UNBINDABLE #else - ro // FIXME: do any options make sense on other that linux? + ro // FIXME: do any options make sense on other that linux? #endif } MountOpt; #ifdef __linux__ /* Options to set when readOnly=true */ - static constexpr std::array readOnlyDefaults = { - ro, nodev, noexec, nosuid, noatime + 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 exclusiveOptionMasks[] = { + {MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME, "option-atime"}, + {MS_SHARED | MS_PRIVATE | MS_SLAVE, "propagation"}, }; #endif @@ -65,10 +71,18 @@ struct SandboxPath std::vector options; - SandboxPath(const Path & source = "", - bool optional = false, bool readOnly = false, const std::vector & options = { }) - : source(source), optional(optional), readOnly(readOnly), options(options) { }; - SandboxPath(const char * source) : SandboxPath(Path(source)) { }; + SandboxPath(std::string source = "", bool optional = false, + bool readOnly = false, std::vector 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>> + SandboxPath(S&& source, bool optional = false, bool readOnly = false) : + SandboxPath(Path(std::forward(source)), optional, readOnly) { } static SandboxPaths parse(const std::string_view & str, const std::string& = "(unknown)"); }; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 406f57586..2deb5d6a6 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -106,7 +106,7 @@ protected: * Stuff we need to pass to initChild(). */ - typedef SandboxPaths PathsInChroot; // maps target path to source path + typedef SandboxPaths PathsInChroot; typedef StringMap Environment; Environment env; @@ -920,7 +920,7 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() if (line == "") { state = stBegin; } else { - for (const auto & [k, v] : SandboxPath().parse(line)) + for (const auto & [k, v] : SandboxPath::parse(line, "extra-sandbox-paths (via pre-build-hook)")) pathsInChroot.try_emplace(k, v); } } diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 8bf5e3d3b..7e877bda8 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -121,32 +121,30 @@ static void setupSeccomp() # endif } -static auto combineMountOpts(auto init, auto iter) -{ - return std::transform_reduce(iter.cbegin(), iter.cend(), init, - [](unsigned long a, unsigned long b) { - if (b & (MS_NOATIME | MS_RELATIME | MS_NODIRATIME | MS_STRICTATIME)) { - return (a & ~(MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME)) | b; - } - return a | b; - }, - [](const SandboxPath::MountOpt & o) { return static_cast(o); }); +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; }; - -static void doBind(const SandboxPath & pval, const Path & target) +template +static uint64_t combineMountOpts(uint64_t init, const Iterable& opts) { - auto source = pval.source; - auto optional = pval.optional; - 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(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) - throw SysError("bind mount from '%1%' to '%2%' failed", source, target); + if (mount(v.source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) + throw SysError("bind mount from '%1%' to '%2%' failed", v.source, target); // set extra options when wanted - auto flags = pval.readOnly ? combineMountOpts(0, pval.readOnlyDefaults) : 0; - flags = combineMountOpts(flags, pval.options); + 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); @@ -155,12 +153,12 @@ static void doBind(const SandboxPath & pval, const Path & 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; @@ -170,7 +168,7 @@ static void doBind(const SandboxPath & pval, const Path & target) } 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, "");