From 6da0389d0fac359ca6c752ab9b174b3764b8547f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 21 Jan 2026 15:59:30 +0100 Subject: [PATCH] libstore: move netrc/ca init outside of the build child process this will make it easier to turn build sandbox processes into helpers. the point they were at was effectivly unsandboxed except for a few fds that were redirected by commonExecveingChildInit, which only made logs of any errors that much harder to convey from the child to the parent. Change-Id: I67006eb33e1e13311bb8d14e6a0c3d5e6baf0c13 --- lix/libstore/build/local-derivation-goal.cc | 71 ++++++++++++--------- lix/libstore/build/local-derivation-goal.hh | 9 ++- lix/libstore/platform/linux.cc | 11 +++- lix/libstore/platform/linux.hh | 6 +- 4 files changed, 62 insertions(+), 35 deletions(-) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index d21b043b6..159657e79 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -836,8 +837,31 @@ try { buildResult.startTime = time(0); + /* Make the contents of netrc and the CA certificate bundle + available to builtin:fetchurl (which may run under a + different uid and/or in a sandbox). */ + std::optional netrcData; + std::optional caFileData; + if (drv->isBuiltin() && drv->builder == "builtin:fetchurl" && !derivationType->isSandboxed()) { + try { + netrcData = readFile(settings.netrcFile); + } catch (SysError &) { + netrcData = ""; + } + + try { + caFileData = readFile(settings.caFile); + } catch (SysError &) { + caFileData = ""; + } + } + + if (!derivationType->isSandboxed()) { + setupConfiguredCertificateAuthority(); + } + /* Fork a child to build the package. */ - pid = startChild(std::move(builderOut)); + pid = startChild(netrcData, caFileData, std::move(builderOut)); /* parent */ pid.setSeparatePG(true); @@ -873,14 +897,18 @@ try { co_return result::current_exception(); } -Pid LocalDerivationGoal::startChild(AutoCloseFD logPTY) +Pid LocalDerivationGoal::startChild( + const std::optional & netrcData, + const std::optional & caFileData, + AutoCloseFD logPTY +) { return startProcess([&]() { if (dup2(logPTY.get(), STDERR_FILENO) == -1) { throw SysError("failed to redirect build output to log file"); } closeOnExec(STDERR_FILENO, false); - runChild(); + runChild(netrcData, caFileData); }); } @@ -1115,8 +1143,9 @@ void LocalDerivationGoal::chownToBuilder(const AutoCloseFD & fd) throw SysError("cannot change ownership of file '%1%'", fd.guessOrInventPath()); } - -void LocalDerivationGoal::runChild() +void LocalDerivationGoal::runChild( + const std::optional & netrcData, const std::optional & caFileData +) { /* Warning: in the child we should absolutely not make any SQLite calls! */ @@ -1127,25 +1156,6 @@ void LocalDerivationGoal::runChild() commonExecveingChildInit(); - /* Make the contents of netrc and the CA certificate bundle - available to builtin:fetchurl (which may run under a - different uid and/or in a sandbox). */ - std::string netrcData; - std::string caFileData; - if (drv->isBuiltin() && drv->builder == "builtin:fetchurl" && !derivationType->isSandboxed()) { - try { - netrcData = readFile(settings.netrcFile); - } catch (SysError &) { } - - try { - caFileData = readFile(settings.caFile); - } catch (SysError &) { } - } - - if (!derivationType->isSandboxed()) { - setupConfiguredCertificateAuthority(); - } - const bool setUser = prepareChildSetup(); if (sys::chdir(tmpDirInSandbox) == -1) { @@ -1217,14 +1227,17 @@ void LocalDerivationGoal::runChild() for (auto & e : drv2.env) e.second = rewriteStrings(e.second, inputRewrites); - if (drv->builder == "builtin:fetchurl") - builtinFetchurl(drv2, netrcData, caFileData); - else if (drv->builder == "builtin:buildenv") + if (drv->builder == "builtin:fetchurl") { + assert(netrcData.has_value()); + assert(caFileData.has_value()); + builtinFetchurl(drv2, netrcData.value(), caFileData.value()); + } else if (drv->builder == "builtin:buildenv") { builtinBuildenv(drv2); - else if (drv->builder == "builtin:unpack-channel") + } else if (drv->builder == "builtin:unpack-channel") { builtinUnpackChannel(drv2); - else + } else { throw Error("unsupported builtin builder '%1%'", drv->builder.substr(8)); + } _exit(0); } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) writeFull(STDERR_FILENO, e.what() + std::string("\n")); diff --git a/lix/libstore/build/local-derivation-goal.hh b/lix/libstore/build/local-derivation-goal.hh index 4b21ae39f..870122306 100644 --- a/lix/libstore/build/local-derivation-goal.hh +++ b/lix/libstore/build/local-derivation-goal.hh @@ -214,7 +214,8 @@ struct LocalDerivationGoal : public DerivationGoal /** * Run the builder's process. */ - void runChild(); + void + runChild(const std::optional & netrcData, const std::optional & caFileData); /** * Check that the derivation outputs all exist and register them @@ -311,7 +312,11 @@ protected: * Create a new process that runs `openSlave` and `runChild` * On some platforms this process is created with sandboxing flags. */ - virtual Pid startChild(AutoCloseFD logPTY); + virtual Pid startChild( + const std::optional & netrcData, + const std::optional & caFileData, + AutoCloseFD logPTY + ); kj::Promise> handleRawChild() noexcept; kj::Promise>> handleRawChildStream() noexcept; diff --git a/lix/libstore/platform/linux.cc b/lix/libstore/platform/linux.cc index 28ae74bdd..c6c94ed63 100644 --- a/lix/libstore/platform/linux.cc +++ b/lix/libstore/platform/linux.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -1237,7 +1238,11 @@ bool LinuxLocalDerivationGoal::prepareChildSetup() return false; } -Pid LinuxLocalDerivationGoal::startChild(AutoCloseFD logPTY) +Pid LinuxLocalDerivationGoal::startChild( + const std::optional & netrcData, + const std::optional & caFileData, + AutoCloseFD logPTY +) { #if HAVE_SECCOMP // Our seccomp filter program is surprisingly expensive to compile (~10ms). @@ -1248,7 +1253,7 @@ Pid LinuxLocalDerivationGoal::startChild(AutoCloseFD logPTY) // If we're not sandboxing no need to faff about, use the fallback if (!useChroot) { - return LocalDerivationGoal::startChild(std::move(logPTY)); + return LocalDerivationGoal::startChild(netrcData, caFileData, std::move(logPTY)); } /* Set up private namespaces for the build: @@ -1324,7 +1329,7 @@ Pid LinuxLocalDerivationGoal::startChild(AutoCloseFD logPTY) options.cloneFlags |= CLONE_NEWUSER; } - pid_t child = startProcess([&]() { runChild(); }, options).release(); + pid_t child = startProcess([&]() { runChild(netrcData, caFileData); }, options).release(); writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); _exit(0); diff --git a/lix/libstore/platform/linux.hh b/lix/libstore/platform/linux.hh index 988b49c73..4f6c92a75 100644 --- a/lix/libstore/platform/linux.hh +++ b/lix/libstore/platform/linux.hh @@ -73,7 +73,11 @@ private: * Start child process in new namespaces, * create /etc/passwd and /etc/group based on discovered uid/gid */ - Pid startChild(AutoCloseFD logPTY) override; + Pid startChild( + const std::optional & netrcData, + const std::optional & caFileData, + AutoCloseFD logPTY + ) override; /** * Kill all processes by build user.