libstore/build: use an allowlist approach to syscall filtering

Previously, system call filtering (to prevent builders from storing files with
setuid/setgid permission bits or extended attributes) was performed using a
blocklist. While this looks simple at first, it actually carries significant
security and maintainability risks: after all, the kernel may add new syscalls
to achieve the same functionality one is trying to block, and it can even be
hard to actually add the syscall to the blocklist when building against a C
library that doesn't know about it yet. For a recent demonstration of this
happening in practice to Nix, see the introduction of fchmodat2 [0] [1].

The allowlist approach does not share the same drawback. While it does require
a rather large list of harmless syscalls to be maintained in the codebase,
failing to update this list (and roll out the update to all users) in time has
rather benign effects; at worst, very recent programs that already rely on new
syscalls will fail with an error the same way they would on a slightly older
kernel that doesn't support them yet. Most importantly, no unintended new ways
of performing dangerous operations will be silently allowed.

Another possible drawback is reduced system call performance due to the larger
filter created by the allowlist requiring more computation [2]. However, this
issue has not convincingly been demonstrated yet in practice, for example in
systemd or various browsers.

This commit tries to keep the behavior as close to unchanged as possible. Only
newer syscalls that are not supported by glibc 2.38 (as found in NixOS 23.11)
are blocked. Since this includes fchmodat2, the compatibility code added for
handling this syscall can be removed too.

[0] https://github.com/NixOS/nixpkgs/issues/300635
[1] https://github.com/NixOS/nix/issues/10424
[2] https://github.com/flatpak/flatpak/pull/4462#issuecomment-1061690607

Change-Id: I541be3ea9b249bcceddfed6a5a13ac10b11e16ad

src/libexpr/eval.cc

@@ -4,6 +4,7 @@
 #include "primops.hh"
 #include "print-options.hh"
 #include "shared.hh"
+#include "suggestions.hh"
 #include "types.hh"
 #include "store-api.hh"
 #include "derivations.hh"
@@ -1426,11 +1427,13 @@ static std::string showAttrPath(EvalState & state, Env & env, const AttrPath & a
 
 void ExprSelect::eval(EvalState & state, Env & env, Value & v)
 {
-    Value vTmp;
-    PosIdx pos2;
-    Value * vAttrs = &vTmp;
+    Value vFirst;
 
-    e->eval(state, env, vTmp);
+    // Pointer to the current attrset Value in this select chain.
+    Value * vCurrent = &vFirst;
+    // Position for the current attrset Value in this select chain.
+    PosIdx posCurrent;
+    e->eval(state, env, vFirst);
 
     try {
         auto dts = state.debugRepl
@@ -1443,48 +1446,75 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
                 showAttrPath(state, env, attrPath))
             : nullptr;
 
-        for (auto & i : attrPath) {
+        for (auto const & currentAttrName : attrPath) {
             state.nrLookups++;
-            Bindings::iterator j;
-            auto name = getName(i, state, env);
-            if (def) {
-                state.forceValue(*vAttrs, pos);
-                if (vAttrs->type() != nAttrs ||
-                    (j = vAttrs->attrs->find(name)) == vAttrs->attrs->end())
-                {
-                    def->eval(state, env, v);
+
+            Symbol const name = getName(currentAttrName, state, env);
+
+            state.forceValue(*vCurrent, pos);
+
+            if (vCurrent->type() != nAttrs) {
+
+                // If we have an `or` provided default,
+                // then this is allowed to not be an attrset.
+                if (def != nullptr) {
+                    this->def->eval(state, env, v);
                     return;
                 }
-            } else {
-                state.forceAttrs(*vAttrs, pos, "while selecting an attribute");
-                if ((j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) {
-                    std::set<std::string> allAttrNames;
-                    for (auto & attr : *vAttrs->attrs)
-                        allAttrNames.insert(state.symbols[attr.name]);
-                    auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]);
-                    state.error<EvalError>("attribute '%1%' missing", state.symbols[name])
-                        .atPos(pos).withSuggestions(suggestions).withFrame(env, *this).debugThrow();
-                }
+
+                // Otherwise, we must type error.
+                state.error<TypeError>(
+                    "expected a set but found %s: %s",
+                    showType(*vCurrent),
+                    ValuePrinter(state, *vCurrent, errorPrintOptions)
+                ).withTrace(pos, "while selecting an attribute").debugThrow();
             }
-            vAttrs = j->value;
-            pos2 = j->pos;
-            if (state.countCalls) state.attrSelects[pos2]++;
+
+            // Now that we know this is actually an attrset, try to find an attr
+            // with the selected name.
+            Bindings::iterator attrIt = vCurrent->attrs->find(name);
+            if (attrIt == vCurrent->attrs->end()) {
+
+                // If we have an `or` provided default, then we'll use that.
+                if (def != nullptr) {
+                    this->def->eval(state, env, v);
+                    return;
+                }
+
+                // Otherwise, missing attr error.
+                std::set<std::string> allAttrNames;
+                for (auto const & attr : *vCurrent->attrs) {
+                    allAttrNames.insert(state.symbols[attr.name]);
+                }
+                auto suggestions = Suggestions::bestMatches(allAttrNames, state.symbols[name]);
+                state.error<EvalError>("attribute '%s' missing", state.symbols[name])
+                    .atPos(pos)
+                    .withSuggestions(suggestions)
+                    .withFrame(env, *this)
+                    .debugThrow();
+            }
+
+            // If we're here, then we successfully found the attribute.
+            // Set our currently operated-on attrset to this one, and keep going.
+            vCurrent = attrIt->value;
+            posCurrent = attrIt->pos;
+            if (state.countCalls) state.attrSelects[posCurrent]++;
         }
 
-        state.forceValue(*vAttrs, (pos2 ? pos2 : this->pos ) );
+        state.forceValue(*vCurrent, (posCurrent ? posCurrent : this->pos));
 
     } catch (Error & e) {
-        if (pos2) {
-            auto pos2r = state.positions[pos2];
+        if (posCurrent) {
+            auto pos2r = state.positions[posCurrent];
             auto origin = std::get_if<SourcePath>(&pos2r.origin);
             if (!(origin && *origin == state.derivationInternal))
-                state.addErrorTrace(e, pos2, "while evaluating the attribute '%1%'",
+                state.addErrorTrace(e, posCurrent, "while evaluating the attribute '%1%'",
                     showAttrPath(state, env, attrPath));
         }
         throw;
     }
 
-    v = *vAttrs;
+    v = *vCurrent;
 }
 
 

src/libexpr/nixexpr.hh

@@ -157,8 +157,18 @@ struct ExprInheritFrom : ExprVar
 struct ExprSelect : Expr
 {
     PosIdx pos;
-    std::unique_ptr<Expr> e, def;
+
+    /** The expression attributes are being selected on. e.g. `foo` in `foo.bar.baz`. */
+    std::unique_ptr<Expr> e;
+
+    /** A default value specified with `or`, if the selected attr doesn't exist.
+     * e.g. `bix` in `foo.bar.baz or bix`
+     */
+    std::unique_ptr<Expr> def;
+
+    /** The path of attributes being selected. e.g. `bar.baz` in `foo.bar.baz.` */
     AttrPath attrPath;
+
     ExprSelect(const PosIdx & pos, std::unique_ptr<Expr> e, AttrPath attrPath, std::unique_ptr<Expr> def) : pos(pos), e(std::move(e)), def(std::move(def)), attrPath(std::move(attrPath)) { };
     ExprSelect(const PosIdx & pos, std::unique_ptr<Expr> e, Symbol name) : pos(pos), e(std::move(e)) { attrPath.push_back(AttrName(name)); };
     PosIdx getPos() const override { return pos; }

src/libmain/shared.cc

@@ -378,7 +378,7 @@ RunPager::RunPager()
 RunPager::~RunPager()
 {
     try {
-        if (pid != -1) {
+        if (pid) {
             std::cout.flush();
             dup2(std_out, STDOUT_FILENO);
             pid.wait();

src/libstore/build/hook-instance.cc

@@ -79,7 +79,7 @@ HookInstance::~HookInstance()
 {
     try {
         toHook.writeSide.reset();
-        if (pid != -1) pid.kill();
+        if (pid) pid.kill();
     } catch (...) {
         ignoreException();
     }

src/libstore/build/local-derivation-goal.cc

@@ -50,9 +50,6 @@
 #endif
 
 #if __APPLE__
-#include <spawn.h>
-#include <sys/sysctl.h>
-
 /* This definition is undocumented but depended upon by all major browsers. */
 extern "C" int sandbox_init_with_parameters(const char *profile, uint64_t flags, const char *const parameters[], char **errorbuf);
 #endif
@@ -139,7 +136,7 @@ LocalStore & LocalDerivationGoal::getLocalStore()
 
 void LocalDerivationGoal::killChild()
 {
-    if (pid != -1) {
+    if (pid) {
         worker.childTerminated(this);
 
         /* If we're using a build user, then there is a tricky race
@@ -147,7 +144,7 @@ void LocalDerivationGoal::killChild()
            done its setuid() to the build user uid, then it won't be
            killed, and we'll potentially lock up in pid.wait().  So
            also send a conventional kill to the child. */
-        ::kill(-pid, SIGKILL); /* ignore the result */
+        ::kill(-pid.get(), SIGKILL); /* ignore the result */
 
         killSandbox(true);
 
@@ -160,19 +157,7 @@ void LocalDerivationGoal::killChild()
 
 void LocalDerivationGoal::killSandbox(bool getStats)
 {
-    if (cgroup) {
-        #if __linux__
-        auto stats = destroyCgroup(*cgroup);
-        if (getStats) {
-            buildResult.cpuUser = stats.cpuUser;
-            buildResult.cpuSystem = stats.cpuSystem;
-        }
-        #else
-        abort();
-        #endif
-    }
-
-    else if (buildUser) {
+    if (buildUser) {
         auto uid = buildUser->getUID();
         assert(uid != 0);
         killUser(uid);
@@ -944,7 +929,7 @@ void LocalDerivationGoal::startBuilder()
             if (usingUserNamespace)
                 options.cloneFlags |= CLONE_NEWUSER;
 
-            pid_t child = startProcess([&]() { runChild(); }, options);
+            pid_t child = startProcess([&]() { runChild(); }, options).release();
 
             writeFull(sendPid.writeSide.get(), fmt("%d\n", child));
             _exit(0);
@@ -965,7 +950,7 @@ void LocalDerivationGoal::startBuilder()
 
         auto ss = tokenizeString<std::vector<std::string>>(readLine(sendPid.readSide.get()));
         assert(ss.size() == 1);
-        pid = string2Int<pid_t>(ss[0]).value();
+        pid = Pid{string2Int<pid_t>(ss[0]).value()};
 
         if (usingUserNamespace) {
             /* Set the UID/GID mapping of the builder's user namespace
@@ -975,13 +960,13 @@ void LocalDerivationGoal::startBuilder()
             uid_t hostGid = buildUser ? buildUser->getGID() : getgid();
             uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1;
 
-            writeFile("/proc/" + std::to_string(pid) + "/uid_map",
+            writeFile("/proc/" + std::to_string(pid.get()) + "/uid_map",
                 fmt("%d %d %d", sandboxUid(), hostUid, nrIds));
 
             if (!buildUser || buildUser->getUIDCount() == 1)
-                writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny");
+                writeFile("/proc/" + std::to_string(pid.get()) + "/setgroups", "deny");
 
-            writeFile("/proc/" + std::to_string(pid) + "/gid_map",
+            writeFile("/proc/" + std::to_string(pid.get()) + "/gid_map",
                 fmt("%d %d %d", sandboxGid(), hostGid, nrIds));
         } else {
             debug("note: not using a user namespace");
@@ -1004,19 +989,19 @@ void LocalDerivationGoal::startBuilder()
 
         /* Save the mount- and user namespace of the child. We have to do this
            *before* the child does a chroot. */
-        sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY)};
+        sandboxMountNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/mnt", pid.get()).c_str(), O_RDONLY)};
         if (sandboxMountNamespace.get() == -1)
             throw SysError("getting sandbox mount namespace");
 
         if (usingUserNamespace) {
-            sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY)};
+            sandboxUserNamespace = AutoCloseFD{open(fmt("/proc/%d/ns/user", pid.get()).c_str(), O_RDONLY)};
             if (sandboxUserNamespace.get() == -1)
                 throw SysError("getting sandbox user namespace");
         }
 
         /* Move the child into its own cgroup. */
         if (cgroup)
-            writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid));
+            writeFile(*cgroup + "/cgroup.procs", fmt("%d", pid.get()));
 
         /* Signal the builder that we've updated its user namespace. */
         writeFull(userNamespaceSync.writeSide.get(), "1");
@@ -1584,7 +1569,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path)
                entering its mount namespace, which is not possible
                in multithreaded programs. So we do this in a
                child process.*/
-            Pid child(startProcess([&]() {
+            Pid child = startProcess([&]() {
 
                 if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1))
                     throw SysError("entering sandbox user namespace");
@@ -1595,7 +1580,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path)
                 doBind(source, target);
 
                 _exit(0);
-            }));
+            });
 
             int status = child.wait();
             if (status != 0)
@@ -2672,31 +2657,8 @@ void LocalDerivationGoal::runChild()
             }
         }
 
-#if __APPLE__
-        posix_spawnattr_t attrp;
-
-        if (posix_spawnattr_init(&attrp))
-            throw SysError("failed to initialize builder");
-
-        if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC))
-            throw SysError("failed to initialize builder");
-
-        if (drv->platform == "aarch64-darwin") {
-            // Unset kern.curproc_arch_affinity so we can escape Rosetta
-            int affinity = 0;
-            sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity));
-
-            cpu_type_t cpu = CPU_TYPE_ARM64;
-            posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL);
-        } else if (drv->platform == "x86_64-darwin") {
-            cpu_type_t cpu = CPU_TYPE_X86_64;
-            posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL);
-        }
-
-        posix_spawn(NULL, drv->builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
-#else
-        execve(drv->builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
-#endif
+        execBuilder(drv->builder, args, envStrs);
+        // execBuilder should not return
 
         throw SysError("executing '%1%'", drv->builder);
 
@@ -2712,6 +2674,11 @@ void LocalDerivationGoal::runChild()
     }
 }
 
+void LocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs)
+{
+    execve(builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
+}
+
 
 SingleDrvOutputs LocalDerivationGoal::registerOutputs()
 {

src/libstore/build/local-derivation-goal.hh

@@ -178,7 +178,28 @@ struct LocalDerivationGoal : public DerivationGoal
 
     friend struct RestrictedStore;
 
-    using DerivationGoal::DerivationGoal;
+    /**
+     * Create a LocalDerivationGoal without an on-disk .drv file,
+     * possibly a platform-specific subclass
+     */
+    static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
+        const StorePath & drvPath,
+        const OutputsSpec & wantedOutputs,
+        Worker & worker,
+        BuildMode buildMode
+    );
+
+    /**
+     * Create a LocalDerivationGoal for an on-disk .drv file,
+     * possibly a platform-specific subclass
+     */
+    static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
+        const StorePath & drvPath,
+        const BasicDerivation & drv,
+        const OutputsSpec & wantedOutputs,
+        Worker & worker,
+        BuildMode buildMode
+    );
 
     virtual ~LocalDerivationGoal() noexcept(false) override;
 
@@ -282,7 +303,7 @@ struct LocalDerivationGoal : public DerivationGoal
      * Kill any processes running under the build user UID or in the
      * cgroup of the build.
      */
-    void killSandbox(bool getStats);
+    virtual void killSandbox(bool getStats);
 
     /**
      * Create alternative path calculated from but distinct from the
@@ -299,6 +320,16 @@ struct LocalDerivationGoal : public DerivationGoal
      * rewrites caught everything
      */
     StorePath makeFallbackPath(OutputNameView outputName);
+
+protected:
+    using DerivationGoal::DerivationGoal;
+
+    /**
+     * Execute the builder, replacing the current process.
+     * Generally this means an `execve` call.
+     */
+    virtual void execBuilder(std::string builder, Strings args, Strings envStrs);
+
 };
 
 }

src/libstore/build/substitution-goal.cc

@@ -217,6 +217,7 @@ void PathSubstitutionGoal::tryToRun()
     promise = std::promise<void>();
 
     thr = std::thread([this]() {
+        auto & fetchPath = subPath ? *subPath : storePath;
         try {
             ReceiveInterrupts receiveInterrupts;
 
@@ -226,10 +227,17 @@ void PathSubstitutionGoal::tryToRun()
             Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()});
             PushActivity pact(act.id);
 
-            copyStorePath(*sub, worker.store,
-                subPath ? *subPath : storePath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs);
+            copyStorePath(
+                *sub, worker.store, fetchPath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs
+            );
 
             promise.set_value();
+        } catch (const EndOfFile &) {
+            promise.set_exception(std::make_exception_ptr(EndOfFile(
+                "NAR for '%s' fetched from '%s' is incomplete",
+                sub->printStorePath(fetchPath),
+                sub->getUri()
+            )));
         } catch (...) {
             promise.set_exception(std::current_exception());
         }

src/libstore/build/worker.cc

@@ -65,8 +65,8 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
         return !dynamic_cast<LocalStore *>(&store)
-            ? std::make_shared</* */DerivationGoal>(drvPath, wantedOutputs, *this, buildMode)
-            : std::make_shared<LocalDerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
+            ? std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode)
+            : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, wantedOutputs, *this, buildMode);
     });
 }
 
@@ -76,8 +76,8 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
         return !dynamic_cast<LocalStore *>(&store)
-            ? std::make_shared</* */DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode)
-            : std::make_shared<LocalDerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
+            ? std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode)
+            : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, drv, wantedOutputs, *this, buildMode);
     });
 }
 

src/libstore/platform.cc

@@ -1,4 +1,5 @@
 #include "local-store.hh"
+#include "build/local-derivation-goal.hh"
 
 #if __linux__
 #include "platform/linux.hh"
@@ -19,4 +20,43 @@ std::shared_ptr<LocalStore> LocalStore::makeLocalStore(const Params & params)
     return std::shared_ptr<LocalStore>(new FallbackLocalStore(params));
 #endif
 }
+
+std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
+    const StorePath & drvPath,
+    const OutputsSpec & wantedOutputs,
+    Worker & worker,
+    BuildMode buildMode
+)
+{
+#if __linux__
+    return std::make_shared<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode);
+#elif __APPLE__
+    return std::make_shared<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode);
+#else
+    return std::make_shared<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode);
+#endif
+}
+
+std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
+    const StorePath & drvPath,
+    const BasicDerivation & drv,
+    const OutputsSpec & wantedOutputs,
+    Worker & worker,
+    BuildMode buildMode
+)
+{
+#if __linux__
+    return std::make_shared<LinuxLocalDerivationGoal>(
+        drvPath, drv, wantedOutputs, worker, buildMode
+    );
+#elif __APPLE__
+    return std::make_shared<DarwinLocalDerivationGoal>(
+        drvPath, drv, wantedOutputs, worker, buildMode
+    );
+#else
+    return std::make_shared<FallbackLocalDerivationGoal>(
+        drvPath, drv, wantedOutputs, worker, buildMode
+    );
+#endif
+}
 }

src/libstore/platform/darwin.cc

@@ -6,6 +6,7 @@
 #include <sys/proc_info.h>
 #include <sys/sysctl.h>
 #include <libproc.h>
+#include <spawn.h>
 
 #include <regex>
 
@@ -220,4 +221,29 @@ void DarwinLocalStore::findPlatformRoots(UncheckedRoots & unchecked)
         }
     }
 }
+
+void DarwinLocalDerivationGoal::execBuilder(std::string builder, Strings args, Strings envStrs)
+{
+    posix_spawnattr_t attrp;
+
+    if (posix_spawnattr_init(&attrp))
+        throw SysError("failed to initialize builder");
+
+    if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETEXEC))
+        throw SysError("failed to initialize builder");
+
+    if (drv->platform == "aarch64-darwin") {
+        // Unset kern.curproc_arch_affinity so we can escape Rosetta
+        int affinity = 0;
+        sysctlbyname("kern.curproc_arch_affinity", NULL, NULL, &affinity, sizeof(affinity));
+
+        cpu_type_t cpu = CPU_TYPE_ARM64;
+        posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL);
+    } else if (drv->platform == "x86_64-darwin") {
+        cpu_type_t cpu = CPU_TYPE_X86_64;
+        posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL);
+    }
+
+    posix_spawn(NULL, builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
+}
 }

src/libstore/platform/darwin.hh

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "build/local-derivation-goal.hh"
 #include "gc-store.hh"
 #include "local-store.hh"
 
@@ -32,4 +33,19 @@ private:
     void findPlatformRoots(UncheckedRoots & unchecked) override;
 };
 
+/**
+ * Darwin-specific implementation of LocalDerivationGoal
+ */
+class DarwinLocalDerivationGoal : public LocalDerivationGoal
+{
+public:
+    using LocalDerivationGoal::LocalDerivationGoal;
+
+private:
+    /**
+     * Set process flags to enter or leave rosetta, then execute the builder
+     */
+    void execBuilder(std::string builder, Strings args, Strings envStrs) override;
+};
+
 }

src/libstore/platform/fallback.hh

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "build/local-derivation-goal.hh"
 #include "local-store.hh"
 
 namespace nix {
@@ -28,4 +29,14 @@ public:
     }
 };
 
+/**
+ * Fallback platform implementation of LocalDerivationGoal
+ * Exists so we can make LocalDerivationGoal constructor protected
+ */
+class FallbackLocalDerivationGoal : public LocalDerivationGoal
+{
+public:
+    using LocalDerivationGoal::LocalDerivationGoal;
+};
+
 }

src/libstore/platform/linux.cc

@@ -1,3 +1,4 @@
+#include "cgroup.hh"
 #include "gc-store.hh"
 #include "signals.hh"
 #include "platform/linux.hh"
@@ -114,4 +115,17 @@ void LinuxLocalStore::findPlatformRoots(UncheckedRoots & unchecked)
     readFileRoots("/proc/sys/kernel/fbsplash", unchecked);
     readFileRoots("/proc/sys/kernel/poweroff_cmd", unchecked);
 }
+
+void LinuxLocalDerivationGoal::killSandbox(bool getStats)
+{
+    if (cgroup) {
+        auto stats = destroyCgroup(*cgroup);
+        if (getStats) {
+            buildResult.cpuUser = stats.cpuUser;
+            buildResult.cpuSystem = stats.cpuSystem;
+        }
+    } else {
+        LocalDerivationGoal::killSandbox(getStats);
+    }
+}
 }

src/libstore/platform/linux.hh

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "build/local-derivation-goal.hh"
 #include "gc-store.hh"
 #include "local-store.hh"
 
@@ -32,4 +33,16 @@ private:
     void findPlatformRoots(UncheckedRoots & unchecked) override;
 };
 
+/**
+ * Linux-specific implementation of LocalDerivationGoal
+ */
+class LinuxLocalDerivationGoal : public LocalDerivationGoal
+{
+public:
+    using LocalDerivationGoal::LocalDerivationGoal;
+
+private:
+    void killSandbox(bool getStatus) override;
+};
+
 }

src/libstore/ssh.cc

@@ -131,7 +131,7 @@ Path SSHMaster::startMaster()
 
     auto state(state_.lock());
 
-    if (state->sshMaster != -1) return state->socketPath;
+    if (state->sshMaster) return state->socketPath;
 
     state->socketPath = (Path) *state->tmpDir + "/ssh.sock";
 

src/libstore/store-api.cc

@@ -1072,8 +1072,6 @@ void copyStorePath(
         });
         TeeSink tee { sink, progressSink };
         srcStore.narFromPath(storePath, tee);
-    }, [&]() {
-           throw EndOfFile("NAR for '%s' fetched from '%s' is incomplete", srcStore.printStorePath(storePath), srcStore.getUri());
     });
 
     dstStore.addToStore(*info, *source, repair, checkSigs);

src/libutil/namespaces.cc

@@ -94,12 +94,7 @@ bool userNamespacesSupported()
     static auto res = [&]() -> bool
     {
         try {
-            Pid pid = startProcess([&]()
-            {
-                _exit(0);
-            }, {
-                .cloneFlags = CLONE_NEWUSER
-            });
+            Pid pid = startProcess([&]() { _exit(0); }, {.cloneFlags = CLONE_NEWUSER});
 
             auto r = pid.wait();
             assert(!r);
@@ -120,8 +115,7 @@ bool mountAndPidNamespacesSupported()
     {
         try {
 
-            Pid pid = startProcess([&]()
-            {
+            Pid pid = startProcess([&]() {
                 /* Make sure we don't remount the parent's /proc. */
                 if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1)
                     _exit(1);

src/libutil/processes.cc

@@ -35,9 +35,19 @@ Pid::Pid()
 }
 
 
-Pid::Pid(pid_t pid)
-    : pid(pid)
+Pid::Pid(Pid && other) : pid(other.pid), separatePG(other.separatePG), killSignal(other.killSignal)
 {
+    other.pid = -1;
+}
+
+
+Pid & Pid::operator=(Pid && other)
+{
+    Pid tmp(std::move(other));
+    std::swap(pid, tmp.pid);
+    std::swap(separatePG, tmp.separatePG);
+    std::swap(killSignal, tmp.killSignal);
+    return *this;
 }
 
 
@@ -47,20 +57,6 @@ Pid::~Pid() noexcept(false)
 }
 
 
-void Pid::operator =(pid_t pid)
-{
-    if (this->pid != -1 && this->pid != pid) kill();
-    this->pid = pid;
-    killSignal = SIGKILL; // reset signal to default
-}
-
-
-Pid::operator pid_t()
-{
-    return pid;
-}
-
-
 int Pid::kill()
 {
     assert(pid != -1);
@@ -131,7 +127,7 @@ void killUser(uid_t uid)
        users to which the current process can send signals.  So we
        fork a process, switch to uid, and send a mass kill. */
 
-    Pid pid = startProcess([&]() {
+    Pid pid{startProcess([&]() {
 
         if (setuid(uid) == -1)
             throw SysError("setting uid");
@@ -153,7 +149,7 @@ void killUser(uid_t uid)
         }
 
         _exit(0);
-    });
+    })};
 
     int status = pid.wait();
     if (status != 0)
@@ -187,7 +183,7 @@ static int childEntry(void * arg)
 #endif
 
 
-pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
+Pid startProcess(std::function<void()> fun, const ProcessOptions & options)
 {
     std::function<void()> wrapper = [&]() {
         logger = makeSimpleLogger();
@@ -231,7 +227,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
 
     if (pid == -1) throw SysError("unable to fork");
 
-    return pid;
+    return Pid{pid};
 }
 
 std::string runProgram(Path program, bool searchPath, const Strings & args,
@@ -294,7 +290,7 @@ void runProgram2(const RunOptions & options)
     }
 
     /* Fork. */
-    Pid pid = startProcess([&]() {
+    Pid pid{startProcess([&]() {
         if (options.environment)
             replaceEnv(*options.environment);
         if (options.standardOut && dup2(out.writeSide.get(), STDOUT_FILENO) == -1)
@@ -328,7 +324,7 @@ void runProgram2(const RunOptions & options)
             execv(options.program.c_str(), stringsToCharPtrs(args_).data());
 
         throw SysError("executing '%1%'", options.program);
-    }, processOptions);
+    }, processOptions)};
 
     out.writeSide.close();
 

src/libutil/processes.hh

@@ -26,16 +26,18 @@ class Pid
     int killSignal = SIGKILL;
 public:
     Pid();
-    Pid(pid_t pid);
+    explicit Pid(pid_t pid): pid(pid) {}
+    Pid(Pid && other);
+    Pid & operator=(Pid && other);
     ~Pid() noexcept(false);
-    void operator =(pid_t pid);
-    operator pid_t();
+    explicit operator bool() const { return pid != -1; }
     int kill();
     int wait();
 
     void setSeparatePG(bool separatePG);
     void setKillSignal(int signal);
     pid_t release();
+    pid_t get() const { return pid; }
 };
 
 /**
@@ -60,7 +62,8 @@ struct ProcessOptions
     int cloneFlags = 0;
 };
 
-pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());
+[[nodiscard]]
+Pid startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());
 
 
 /**

src/libutil/serialise.cc

@@ -266,20 +266,17 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun)
 }
 
 
-std::unique_ptr<Source> sinkToSource(
-    std::function<void(Sink &)> fun,
-    std::function<void()> eof)
+std::unique_ptr<Source> sinkToSource(std::function<void(Sink &)> fun)
 {
     struct SinkToSource : Source
     {
         typedef boost::coroutines2::coroutine<std::string> coro_t;
 
         std::function<void(Sink &)> fun;
-        std::function<void()> eof;
         std::optional<coro_t::pull_type> coro;
 
-        SinkToSource(std::function<void(Sink &)> fun, std::function<void()> eof)
-            : fun(fun), eof(eof)
+        SinkToSource(std::function<void(Sink &)> fun)
+            : fun(fun)
         {
         }
 
@@ -298,7 +295,9 @@ std::unique_ptr<Source> sinkToSource(
                 });
             }
 
-            if (!*coro) { eof(); abort(); }
+            if (!*coro) {
+                throw EndOfFile("coroutine has finished");
+            }
 
             if (pos == cur.size()) {
                 if (!cur.empty()) {
@@ -317,7 +316,7 @@ std::unique_ptr<Source> sinkToSource(
         }
     };
 
-    return std::make_unique<SinkToSource>(fun, eof);
+    return std::make_unique<SinkToSource>(fun);
 }
 
 

src/libutil/serialise.hh

@@ -338,11 +338,7 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun);
  * Convert a function that feeds data into a Sink into a Source. The
  * Source executes the function as a coroutine.
  */
-std::unique_ptr<Source> sinkToSource(
-    std::function<void(Sink &)> fun,
-    std::function<void()> eof = []() {
-        throw EndOfFile("coroutine has finished");
-    });
+std::unique_ptr<Source> sinkToSource(std::function<void(Sink &)> fun);
 
 
 void writePadding(size_t len, Sink & sink);

src/libutil/unix-domain-socket.cc

@@ -65,7 +65,7 @@ static void bindConnectProcHelper(
     if (path.size() + 1 >= sizeof(addr.sun_path)) {
         Pipe pipe;
         pipe.create();
-        Pid pid = startProcess([&] {
+        Pid pid{startProcess([&] {
             try {
                 pipe.readSide.close();
                 Path dir = dirOf(path);
@@ -83,7 +83,7 @@ static void bindConnectProcHelper(
             } catch (...) {
                 writeFull(pipe.writeSide.get(), "-1\n");
             }
-        });
+        })};
         pipe.writeSide.close();
         auto errNo = string2Int<int>(chomp(drainFD(pipe.readSide.get())));
         if (!errNo || *errNo == -1)

src/nix/daemon.cc

@@ -369,7 +369,7 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
                 processConnection(openUncachedStore(), from, to, trusted, NotRecursive);
 
                 exit(0);
-            }, options);
+            }, options).release();
 
         } catch (Interrupted & e) {
             return;

tests/functional/meson.build

@@ -182,6 +182,7 @@ functional_tests_scripts = [
   'debugger.sh',
   'test-libstoreconsumer.sh',
   'extra-sandbox-profile.sh',
+  'substitute-truncated-nar.sh',
 ]
 
 # Plugin tests require shared libraries support.

tests/functional/repl_characterization/test-session.cc

@@ -22,7 +22,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args)
     procStdout.create();
 
     // This is separate from runProgram2 because we have different IO requirements
-    pid_t pid = startProcess([&]() {
+    auto pid = startProcess([&]() {
         if (dup2(procStdout.writeSide.get(), STDOUT_FILENO) == -1) {
             throw SysError("dupping stdout");
         }
@@ -42,7 +42,7 @@ RunningProcess RunningProcess::start(std::string executable, Strings args)
     procStdin.readSide.close();
 
     return RunningProcess{
-        .pid = pid,
+        .pid = std::move(pid),
         .procStdin = std::move(procStdin),
         .procStdout = std::move(procStdout),
     };

tests/functional/repl_characterization/test-session.hh

@@ -7,13 +7,14 @@
 #include <string>
 
 #include "file-descriptor.hh"
+#include "processes.hh"
 #include "tests/terminal-code-eater.hh"
 
 namespace nix {
 
 struct RunningProcess
 {
-    pid_t pid;
+    Pid pid;
     Pipe procStdin;
     Pipe procStdout;
 

tests/functional/substitute-truncated-nar.sh

@@ -0,0 +1,29 @@
+source common.sh
+
+BINARY_CACHE=file://$cacheDir
+
+build() {
+    nix-build --no-out-link "$@" --expr 'derivation {
+        name = "text";
+        system = builtins.currentSystem;
+        builder = "/bin/sh";
+        args = [ "-c" "echo some text to make the nar less empty > $out" ];
+    }'
+}
+
+path=$(build)
+nix copy --to "$BINARY_CACHE" "$path"
+nix-collect-garbage >/dev/null 2>&1
+
+nar=0bylmx35yjy2b1b4k7gjsl7i4vc03cpmryb41grfb1mp40n3hifl.nar.xz
+
+[ -e $cacheDir/nar/$nar ] || fail "long nar missing?"
+
+xzcat $cacheDir/nar/$nar > $TEST_HOME/tmp
+truncate -s $(( $(stat -c %s $TEST_HOME/tmp) - 10 )) $TEST_HOME/tmp
+xz < $TEST_HOME/tmp > $cacheDir/nar/$nar
+
+# Copying back '$path' from the binary cache. This should fail as it is truncated
+if build --option substituters "$BINARY_CACHE" --option require-sigs false -j0; then
+    fail "Importing a truncated nar should fail"
+fi

tests/unit/libutil/serialise.cc

@@ -0,0 +1,166 @@
+#include "serialise.hh"
+#include "error.hh"
+#include "fmt.hh"
+#include "pos-table.hh"
+#include "ref.hh"
+#include "types.hh"
+
+#include <limits.h>
+#include <gtest/gtest.h>
+
+#include <numeric>
+
+namespace nix {
+
+TEST(Sink, uint64_t)
+{
+    StringSink s;
+    s << 42;
+    ASSERT_EQ(s.s, std::string({42, 0, 0, 0, 0, 0, 0, 0}));
+}
+
+TEST(Sink, string_view)
+{
+    StringSink s;
+    s << "";
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            0, 0, 0, 0, 0, 0, 0, 0,
+            // data (omitted)
+        })
+    );
+    // clang-format on
+
+    s = {};
+    s << "test";
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            4, 0, 0, 0, 0, 0, 0, 0,
+            // data
+            't', 'e', 's', 't',
+            // padding
+            0, 0, 0, 0,
+        })
+    );
+    // clang-format on
+
+    s = {};
+    s << "longer string";
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            13, 0, 0, 0, 0, 0, 0, 0,
+            // data
+            'l', 'o', 'n', 'g', 'e', 'r', ' ', 's', 't', 'r', 'i', 'n', 'g',
+            // padding
+            0, 0, 0,
+        })
+    );
+    // clang-format on
+}
+
+TEST(Sink, StringSet)
+{
+    StringSink s;
+    s << StringSet{};
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            0, 0, 0, 0, 0, 0, 0, 0,
+            // data (omitted)
+        })
+    );
+    // clang-format on
+
+    s = {};
+    s << StringSet{"a", ""};
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            2, 0, 0, 0, 0, 0, 0, 0,
+            // data ""
+            0, 0, 0, 0, 0, 0, 0, 0,
+            // data "a"
+            1, 0, 0, 0, 0, 0, 0, 0, 'a', 0, 0, 0, 0, 0, 0, 0,
+        })
+    );
+    // clang-format on
+}
+
+TEST(Sink, Strings)
+{
+    StringSink s;
+    s << Strings{};
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            0, 0, 0, 0, 0, 0, 0, 0,
+            // data (omitted)
+        })
+    );
+    // clang-format on
+
+    s = {};
+    s << Strings{"a", ""};
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            // length
+            2, 0, 0, 0, 0, 0, 0, 0,
+            // data "a"
+            1, 0, 0, 0, 0, 0, 0, 0, 'a', 0, 0, 0, 0, 0, 0, 0,
+            // data ""
+            0, 0, 0, 0, 0, 0, 0, 0,
+        })
+    );
+    // clang-format on
+}
+
+TEST(Sink, Error)
+{
+    PosTable pt;
+    auto o = pt.addOrigin(Pos::String{make_ref<std::string>("test")}, 4);
+
+    StringSink s;
+    s << Error{ErrorInfo{
+        .level = lvlInfo,
+        .msg = HintFmt("foo"),
+        .pos = pt[pt.add(o, 1)],
+        .traces = {{.pos = pt[pt.add(o, 2)], .hint = HintFmt("b %1%", "foo")}},
+    }};
+    // NOTE position of the error and all traces are ignored
+    // by the wire format
+    // clang-format off
+    ASSERT_EQ(
+        s.s,
+        std::string({
+            5, 0, 0, 0, 0, 0, 0, 0, 'E', 'r', 'r', 'o', 'r', 0, 0, 0,
+            3, 0, 0, 0, 0, 0, 0, 0,
+            5, 0, 0, 0, 0, 0, 0, 0, 'E', 'r', 'r', 'o', 'r', 0, 0, 0,
+            3, 0, 0, 0, 0, 0, 0, 0, 'f', 'o', 'o', 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0,
+            1, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0,
+            16, 0, 0, 0, 0, 0, 0, 0,
+            'b', ' ', '\x1b', '[', '3', '5', ';', '1', 'm', 'f', 'o', 'o', '\x1b', '[', '0', 'm',
+        })
+    );
+    // clang-format on
+}
+
+}

tests/unit/meson.build

@@ -49,6 +49,7 @@ libutil_tests_sources = files(
   'libutil/paths-setting.cc',
   'libutil/pool.cc',
   'libutil/references.cc',
+  'libutil/serialise.cc',
   'libutil/suggestions.cc',
   'libutil/tests.cc',
   'libutil/url.cc',