Skip to content

Commit

Permalink
Experimentally allow forcing nix-daemon trust; use this to test
Browse files Browse the repository at this point in the history
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 NixOS#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.
  • Loading branch information
Ericson2314 committed Apr 17, 2023
1 parent 3f9589f commit d41e1be
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 17 deletions.
2 changes: 2 additions & 0 deletions src/libstore/daemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion src/libutil/experimental-features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct ExperimentalFeatureDetails
std::string_view description;
};

constexpr std::array<ExperimentalFeatureDetails, 11> xpFeatureDetails = {{
constexpr std::array<ExperimentalFeatureDetails, 12> xpFeatureDetails = {{
{
.tag = Xp::CaDerivations,
.name = "ca-derivations",
Expand Down Expand Up @@ -189,6 +189,16 @@ constexpr std::array<ExperimentalFeatureDetails, 11> 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(
Expand Down
1 change: 1 addition & 0 deletions src/libutil/experimental-features.hh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ enum struct ExperimentalFeature
AutoAllocateUids,
Cgroups,
DiscardReferences,
DaemonTrustOverride,
};

/**
Expand Down
63 changes: 47 additions & 16 deletions src/nix/daemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,12 @@ static std::pair<TrustedFlag, std::string> 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<TrustedFlag> forceTrustClientOpt)
{
if (chdir("/") == -1)
throw SysError("cannot change current directory");
Expand Down Expand Up @@ -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) : "<unknown>",
Expand Down Expand Up @@ -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> store)
static void processStdioConnection(ref<Store> 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<TrustedFlag> forceTrustClientOpt)
{
if (stdio) {
auto store = openUncachedStore();

if (auto remoteStore = store.dynamic_pointer_cast<RemoteStore>())
// 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>(); 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<TrustedFlag> isTrustedOpt = std::nullopt;

parseCmdLine(argc, argv, [&](Strings::iterator & arg, const Strings::iterator & end) {
if (*arg == "--daemon")
Expand All @@ -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;
}
Expand All @@ -482,7 +513,7 @@ struct CmdDaemon : StoreCommand

void run(ref<Store> store) override
{
runDaemon(false);
runDaemon(false, std::nullopt);
}
};

Expand Down
2 changes: 2 additions & 0 deletions tests/build-remote-trustless-after.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
outPath=$(readlink -f $TEST_ROOT/result)
grep 'FOO BAR BAZ' ${remoteDir}/${outPath}
29 changes: 29 additions & 0 deletions tests/build-remote-trustless-should-fail-0.sh
Original file line number Diff line number Diff line change
@@ -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"
9 changes: 9 additions & 0 deletions tests/build-remote-trustless-should-pass-0.sh
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions tests/build-remote-trustless-should-pass-1.sh
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions tests/build-remote-trustless-should-pass-3.sh
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions tests/build-remote-trustless.sh
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 4 additions & 0 deletions tests/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
3 changes: 3 additions & 0 deletions tests/nix-daemon-untrusting.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh

exec nix-daemon --force-untrusted "$@"

0 comments on commit d41e1be

Please sign in to comment.