1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2024-09-19 23:03:53 -04:00

Merge branch 'best-effort-supplementary-groups' into overlayfs-store

This commit is contained in:
John Ericson 2023-05-08 14:47:46 -04:00
commit 9c9f5f0d12
7 changed files with 87 additions and 29 deletions

View file

@ -30,6 +30,7 @@ Check out the [security policy](https://github.com/NixOS/nix/security/policy).
You can use [labels](https://github.com/NixOS/nix/labels) to filter for relevant topics.
2. Search for related issues that cover what you're going to work on. It could help to mention there that you will work on the issue.
Pull requests addressing issues labeled ["idea approved"](https://github.com/NixOS/nix/labels/idea%20approved) are especially welcomed by maintainers and will receive prioritised review.
3. Check the [Nix reference manual](https://nixos.org/manual/nix/unstable/contributing/hacking.html) for information on building Nix and running its tests.

View file

@ -258,6 +258,8 @@ static int main_build_remote(int argc, char * * argv)
connected:
close(5);
assert(sshStore);
std::cerr << "# accept\n" << storeUri << "\n";
auto inputs = readStrings<PathSet>(source);
@ -286,23 +288,48 @@ connected:
uploadLock = -1;
auto drv = store->readDerivation(*drvPath);
std::optional<BuildResult> optResult;
// If we don't know whether we are trusted (e.g. `ssh://`
// stores), we assume we are. This is necessary for backwards
// compat.
bool trustedOrLegacy = ({
std::optional trusted = sshStore->isTrustedClient();
!trusted || *trusted;
});
// See the very large comment in `case wopBuildDerivation:` in
// `src/libstore/daemon.cc` that explains the trust model here.
//
// This condition mirrors that: that code enforces the "rules" outlined there;
// we do the best we can given those "rules".
if (trustedOrLegacy || drv.type().isCA()) {
// Hijack the inputs paths of the derivation to include all
// the paths that come from the `inputDrvs` set. We dont do
// that for the derivations whose `inputDrvs` is empty
// because:
//
// 1. Its not needed
//
// 2. Changing the `inputSrcs` set changes the associated
// output ids, which break CA derivations
if (!drv.inputDrvs.empty())
drv.inputSrcs = store->parseStorePathSet(inputs);
optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv);
auto & result = *optResult;
if (!result.success())
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
} else {
copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute);
auto res = sshStore->buildPathsWithResults({ DerivedPath::Built { *drvPath, OutputsSpec::All {} } });
// One path to build should produce exactly one build result
assert(res.size() == 1);
optResult = std::move(res[0]);
}
auto outputHashes = staticOutputHashes(*store, drv);
// Hijack the inputs paths of the derivation to include all the paths
// that come from the `inputDrvs` set.
// We dont do that for the derivations whose `inputDrvs` is empty
// because
// 1. Its not needed
// 2. Changing the `inputSrcs` set changes the associated output ids,
// which break CA derivations
if (!drv.inputDrvs.empty())
drv.inputSrcs = store->parseStorePathSet(inputs);
auto result = sshStore->buildDerivation(*drvPath, drv);
if (!result.success())
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
std::set<Realisation> missingRealisations;
StorePathSet missingPaths;
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) {
@ -311,6 +338,8 @@ connected:
auto thisOutputId = DrvOutput{ thisOutputHash, outputName };
if (!store->queryRealisation(thisOutputId)) {
debug("missing output %s", outputName);
assert(optResult);
auto & result = *optResult;
auto i = result.builtOutputs.find(outputName);
assert(i != result.builtOutputs.end());
auto & newRealisation = i->second;

View file

@ -907,15 +907,10 @@ void LocalDerivationGoal::startBuilder()
openSlave();
/* Drop additional groups here because we can't do it
after we've created the new user namespace. FIXME:
this means that if we're not root in the parent
namespace, we can't drop additional groups; they will
be mapped to nogroup in the child namespace. There does
not seem to be a workaround for this. (But who can tell
from reading user_namespaces(7)?)
See also https://lwn.net/Articles/621612/. */
if (getuid() == 0 && setgroups(0, 0) == -1)
throw SysError("setgroups failed");
after we've created the new user namespace. */
if (settings.dropSupplementaryGroups)
if (setgroups(0, 0) == -1)
throw SysError("setgroups failed");
ProcessOptions options;
options.cloneFlags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_PARENT | SIGCHLD;

View file

@ -515,6 +515,25 @@ public:
Setting<bool> sandboxFallback{this, true, "sandbox-fallback",
"Whether to disable sandboxing when the kernel doesn't allow it."};
Setting<bool> dropSupplementaryGroups{this, getuid() == 0, "drop-supplementary-groups",
R"(
Whether to drop supplementary groups when building with sandboxing.
This is normally a good idea if we are root and have the capability to
do so.
But if this "root" is mapped from a non-root user in a larger
namespace, we won't be able drop additional groups; they will be
mapped to nogroup in the child namespace. There does not seem to be a
workaround for this.
(But who can tell from reading user_namespaces(7)? See also https://lwn.net/Articles/621612/.)
TODO: It might be good to create a middle ground option that allows
`setgroups` to fail if all additional groups are "nogroup" / the value
of `/proc/sys/fs/overflowuid`. This would handle the common
nested-sandboxing case identified above.
)"};
#if __linux__
Setting<std::string> sandboxShmSize{
this, "50%", "sandbox-dev-shm-size",

View file

@ -17,13 +17,13 @@ nix-build build-hook.nix -A passthru.input2 \
--store "$TEST_ROOT/local" \
--option system-features bar
# Now when we go to build that downstream derivation, Nix will fail
# because we cannot trustlessly build input-addressed derivations with
# `inputDrv` dependencies.
# Now when we go to build that downstream derivation, Nix will try to
# copy our already-build `input2` to the remote store. That store object
# is input-addressed, so this will fail.
file=build-hook.nix
prog=$(readlink -e ./nix-daemon-untrusting.sh)
proto=ssh-ng
expectStderr 1 source build-remote-trustless.sh \
| grepQuiet "you are not privileged to build input-addressed derivations"
| grepQuiet "cannot add path '[^ ]*' because it lacks a signature by a trusted key"

View file

@ -0,0 +1,13 @@
source common.sh
enableFeatures "daemon-trust-override"
restartDaemon
# Remote doesn't trust us
file=build-hook.nix
prog=$(readlink -e ./nix-daemon-untrusting.sh)
proto=ssh-ng
source build-remote-trustless.sh
source build-remote-trustless-after.sh

View file

@ -72,6 +72,7 @@ nix_tests = \
build-remote-content-addressed-floating.sh \
build-remote-trustless-should-pass-0.sh \
build-remote-trustless-should-pass-1.sh \
build-remote-trustless-should-pass-2.sh \
build-remote-trustless-should-pass-3.sh \
build-remote-trustless-should-fail-0.sh \
nar-access.sh \