From d41e1bed5e1e1f87927ca1e8e6e1c1ad18b1ea7f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 09:41:39 -0400 Subject: [PATCH] Experimentally allow forcing `nix-daemon` trust; use this to test We finally test the status quo of remote build trust in a number of ways. We create a new experimental feature on `nix-daemon` to do so. PR #3921, which improves the situation with trustless remote building, will build upon these changes. This code / tests was pull out of there to make this, so everything is easier to review, and in particular we test before and after so the new behavior in that PR is readily apparent from the testsuite diff alone. --- src/libstore/daemon.cc | 2 + src/libutil/experimental-features.cc | 12 +++- src/libutil/experimental-features.hh | 1 + src/nix/daemon.cc | 63 ++++++++++++++----- tests/build-remote-trustless-after.sh | 2 + tests/build-remote-trustless-should-fail-0.sh | 29 +++++++++ tests/build-remote-trustless-should-pass-0.sh | 9 +++ tests/build-remote-trustless-should-pass-1.sh | 9 +++ tests/build-remote-trustless-should-pass-3.sh | 14 +++++ tests/build-remote-trustless.sh | 14 +++++ tests/local.mk | 4 ++ tests/nix-daemon-untrusting.sh | 3 + 12 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 tests/build-remote-trustless-after.sh create mode 100644 tests/build-remote-trustless-should-fail-0.sh create mode 100644 tests/build-remote-trustless-should-pass-0.sh create mode 100644 tests/build-remote-trustless-should-pass-1.sh create mode 100644 tests/build-remote-trustless-should-pass-3.sh create mode 100644 tests/build-remote-trustless.sh create mode 100755 tests/nix-daemon-untrusting.sh diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0d7ec2af0..af9a76f1e 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -1067,6 +1067,8 @@ void processConnection( opCount++; + debug("performing daemon worker op: %d", op); + try { performOp(tunnelLogger, store, trusted, recursive, clientVersion, from, to, op); } catch (Error & e) { diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 5b4418714..bd1899662 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -12,7 +12,7 @@ struct ExperimentalFeatureDetails std::string_view description; }; -constexpr std::array xpFeatureDetails = {{ +constexpr std::array xpFeatureDetails = {{ { .tag = Xp::CaDerivations, .name = "ca-derivations", @@ -189,6 +189,16 @@ constexpr std::array xpFeatureDetails = {{ runtime dependencies. )", }, + { + .tag = Xp::DaemonTrustOverride, + .name = "daemon-trust-override", + .description = R"( + Allow forcing trusting or not trusting clients with + `nix-daemon`. This is useful for testing, but possibly also + useful for various experiments with `nix-daemon --stdio` + networking. + )", + }, }}; static_assert( diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 8ef66263a..3c00bc4e5 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -28,6 +28,7 @@ enum struct ExperimentalFeature AutoAllocateUids, Cgroups, DiscardReferences, + DaemonTrustOverride, }; /** diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 7ae7b4ea6..c1a91c63d 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -273,8 +273,12 @@ static std::pair authPeer(const PeerInfo & peer) /** * Run a server. The loop opens a socket and accepts new connections from that * socket. + * + * @param forceTrustClientOpt If present, force trusting or not trusted + * the client. Otherwise, decide based on the authentication settings + * and user credentials (from the unix domain socket). */ -static void daemonLoop() +static void daemonLoop(std::optional forceTrustClientOpt) { if (chdir("/") == -1) throw SysError("cannot change current directory"); @@ -317,9 +321,18 @@ static void daemonLoop() closeOnExec(remote.get()); - PeerInfo peer = getPeerInfo(remote.get()); - auto [_trusted, user] = authPeer(peer); - auto trusted = _trusted; + PeerInfo peer { .pidKnown = false }; + TrustedFlag trusted; + std::string user; + + if (forceTrustClientOpt) + trusted = *forceTrustClientOpt; + else { + peer = getPeerInfo(remote.get()); + auto [_trusted, _user] = authPeer(peer); + trusted = _trusted; + user = _user; + }; printInfo((std::string) "accepted connection from pid %1%, user %2%" + (trusted ? " (trusted)" : ""), peer.pidKnown ? std::to_string(peer.pid) : "", @@ -410,38 +423,47 @@ static void forwardStdioConnection(RemoteStore & store) { * Unlike `forwardStdioConnection()` we do process commands ourselves in * this case, not delegating to another daemon. * - * @note `Trusted` is unconditionally passed because in this mode we - * blindly trust the standard streams. Limiting access to those is - * explicitly not `nix-daemon`'s responsibility. + * @param trustClient Whether to trust the client. Forwarded directly to + * `processConnection()`. */ -static void processStdioConnection(ref store) +static void processStdioConnection(ref store, TrustedFlag trustClient) { FdSource from(STDIN_FILENO); FdSink to(STDOUT_FILENO); - processConnection(store, from, to, Trusted, NotRecursive); + processConnection(store, from, to, trustClient, NotRecursive); } /** * Entry point shared between the new CLI `nix daemon` and old CLI * `nix-daemon`. + * + * @param forceTrustClientOpt See `daemonLoop()` and the parameter with + * the same name over there for details. */ -static void runDaemon(bool stdio) +static void runDaemon(bool stdio, std::optional forceTrustClientOpt) { if (stdio) { auto store = openUncachedStore(); - if (auto remoteStore = store.dynamic_pointer_cast()) + // If --force-untrusted is passed, we cannot forward the connection and + // must process it ourselves (before delegating to the next store) to + // force untrusting the client. + if (auto remoteStore = store.dynamic_pointer_cast(); remoteStore && (!forceTrustClientOpt || *forceTrustClientOpt != NotTrusted)) forwardStdioConnection(*remoteStore); else - processStdioConnection(store); + // `Trusted` is passed in the auto (no override case) because we + // cannot see who is on the other side of a plain pipe. Limiting + // access to those is explicitly not `nix-daemon`'s responsibility. + processStdioConnection(store, forceTrustClientOpt.value_or(Trusted)); } else - daemonLoop(); + daemonLoop(forceTrustClientOpt); } static int main_nix_daemon(int argc, char * * argv) { { auto stdio = false; + std::optional isTrustedOpt = std::nullopt; parseCmdLine(argc, argv, [&](Strings::iterator & arg, const Strings::iterator & end) { if (*arg == "--daemon") @@ -452,11 +474,20 @@ static int main_nix_daemon(int argc, char * * argv) printVersion("nix-daemon"); else if (*arg == "--stdio") stdio = true; - else return false; + else if (*arg == "--force-trusted") { + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); + isTrustedOpt = Trusted; + } else if (*arg == "--force-untrusted") { + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); + isTrustedOpt = NotTrusted; + } else if (*arg == "--default-trust") { + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); + isTrustedOpt = std::nullopt; + } else return false; return true; }); - runDaemon(stdio); + runDaemon(stdio, isTrustedOpt); return 0; } @@ -482,7 +513,7 @@ struct CmdDaemon : StoreCommand void run(ref store) override { - runDaemon(false); + runDaemon(false, std::nullopt); } }; diff --git a/tests/build-remote-trustless-after.sh b/tests/build-remote-trustless-after.sh new file mode 100644 index 000000000..19f59e6ae --- /dev/null +++ b/tests/build-remote-trustless-after.sh @@ -0,0 +1,2 @@ +outPath=$(readlink -f $TEST_ROOT/result) +grep 'FOO BAR BAZ' ${remoteDir}/${outPath} diff --git a/tests/build-remote-trustless-should-fail-0.sh b/tests/build-remote-trustless-should-fail-0.sh new file mode 100644 index 000000000..5e3d5ae07 --- /dev/null +++ b/tests/build-remote-trustless-should-fail-0.sh @@ -0,0 +1,29 @@ +source common.sh + +enableFeatures "daemon-trust-override" + +restartDaemon + +[[ $busybox =~ busybox ]] || skipTest "no busybox" + +unset NIX_STORE_DIR +unset NIX_STATE_DIR + +# We first build a dependency of the derivation we eventually want to +# build. +nix-build build-hook.nix -A passthru.input2 \ + -o "$TEST_ROOT/input2" \ + --arg busybox "$busybox" \ + --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. + +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" diff --git a/tests/build-remote-trustless-should-pass-0.sh b/tests/build-remote-trustless-should-pass-0.sh new file mode 100644 index 000000000..2a7ebd8c6 --- /dev/null +++ b/tests/build-remote-trustless-should-pass-0.sh @@ -0,0 +1,9 @@ +source common.sh + +# Remote trusts us +file=build-hook.nix +prog=nix-store +proto=ssh + +source build-remote-trustless.sh +source build-remote-trustless-after.sh diff --git a/tests/build-remote-trustless-should-pass-1.sh b/tests/build-remote-trustless-should-pass-1.sh new file mode 100644 index 000000000..516bdf092 --- /dev/null +++ b/tests/build-remote-trustless-should-pass-1.sh @@ -0,0 +1,9 @@ +source common.sh + +# Remote trusts us +file=build-hook.nix +prog=nix-daemon +proto=ssh-ng + +source build-remote-trustless.sh +source build-remote-trustless-after.sh diff --git a/tests/build-remote-trustless-should-pass-3.sh b/tests/build-remote-trustless-should-pass-3.sh new file mode 100644 index 000000000..40f81da5a --- /dev/null +++ b/tests/build-remote-trustless-should-pass-3.sh @@ -0,0 +1,14 @@ +source common.sh + +enableFeatures "daemon-trust-override" + +restartDaemon + +# Remote doesn't trusts us, but this is fine because we are only +# building (fixed) CA derivations. +file=build-hook-ca-fixed.nix +prog=$(readlink -e ./nix-daemon-untrusting.sh) +proto=ssh-ng + +source build-remote-trustless.sh +source build-remote-trustless-after.sh diff --git a/tests/build-remote-trustless.sh b/tests/build-remote-trustless.sh new file mode 100644 index 000000000..9df44e0c5 --- /dev/null +++ b/tests/build-remote-trustless.sh @@ -0,0 +1,14 @@ +requireSandboxSupport +[[ $busybox =~ busybox ]] || skipTest "no busybox" + +unset NIX_STORE_DIR +unset NIX_STATE_DIR + +remoteDir=$TEST_ROOT/remote + +# Note: ssh{-ng}://localhost bypasses ssh. See tests/build-remote.sh for +# more details. +nix-build $file -o $TEST_ROOT/result --max-jobs 0 \ + --arg busybox $busybox \ + --store $TEST_ROOT/local \ + --builders "$proto://localhost?remote-program=$prog&remote-store=${remoteDir}%3Fsystem-features=foo%20bar%20baz - - 1 1 foo,bar,baz" diff --git a/tests/local.mk b/tests/local.mk index 6cb466e8e..7c3b42599 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -70,6 +70,10 @@ nix_tests = \ check-reqs.sh \ build-remote-content-addressed-fixed.sh \ 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-3.sh \ + build-remote-trustless-should-fail-0.sh \ nar-access.sh \ pure-eval.sh \ eval.sh \ diff --git a/tests/nix-daemon-untrusting.sh b/tests/nix-daemon-untrusting.sh new file mode 100755 index 000000000..bcdb70989 --- /dev/null +++ b/tests/nix-daemon-untrusting.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +exec nix-daemon --force-untrusted "$@"