From 6e1e15ffec69de8a3954100389f1d41494f8da8d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 7 Apr 2023 11:13:23 -0400 Subject: [PATCH] Fix it! --- src/build-remote/build-remote.cc | 19 +++++++++------ tests/build-remote-trustless-after.sh | 2 ++ tests/build-remote-trustless-should-fail-0.sh | 24 +++++++++++++++---- tests/build-remote-trustless-should-pass-0.sh | 4 ++-- tests/build-remote-trustless-should-pass-1.sh | 4 ++-- tests/build-remote-trustless-should-pass-2.sh | 4 ++-- tests/build-remote-trustless-should-pass-3.sh | 6 ++--- tests/build-remote-trustless.sh | 14 +++++------ 8 files changed, 48 insertions(+), 29 deletions(-) create mode 100644 tests/build-remote-trustless-after.sh diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 649ad3e7c..3d4dbc3d6 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -290,14 +290,19 @@ connected: auto drv = store->readDerivation(*drvPath); std::optional optResult; - if (sshStore->isTrustedClient() || drv.type().isCA()) { - // Hijack the inputs paths of the derivation to include all the paths - // that come from the `inputDrvs` set. - // We don’t do that for the derivations whose `inputDrvs` is empty - // because + // If we don't know whether we are trusted (e.g. `ssh://` + // stores), we assume we are. This is neccessary for backwards + // compat. + if (std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) || drv.type().isCA()) { + // Hijack the inputs paths of the derivation to include all + // the paths that come from the `inputDrvs` set. We don’t do + // that for the derivations whose `inputDrvs` is empty + // because: + // // 1. It’s not needed - // 2. Changing the `inputSrcs` set changes the associated output ids, - // which break CA derivations + // + // 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); 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 index b72c134e7..b5cedb544 100644 --- a/tests/build-remote-trustless-should-fail-0.sh +++ b/tests/build-remote-trustless-should-fail-0.sh @@ -1,11 +1,25 @@ source common.sh -# We act as if remote trusts us, but it doesn't. This fails since we are -# building input-addressed derivations with `buildDerivation`, which -# depends on trust. +[[ $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 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 -trusting=true -! source build-remote-trustless.sh +expectStderr 1 source build-remote-trustless.sh \ + | grepQuiet "cannot add path '[^ ]*' because it lacks a signature by a trusted key" diff --git a/tests/build-remote-trustless-should-pass-0.sh b/tests/build-remote-trustless-should-pass-0.sh index 82c3f4520..2a7ebd8c6 100644 --- a/tests/build-remote-trustless-should-pass-0.sh +++ b/tests/build-remote-trustless-should-pass-0.sh @@ -1,9 +1,9 @@ source common.sh -# Remote trusts us but we pretend it doesn't. +# Remote trusts us file=build-hook.nix prog=nix-store proto=ssh -trusting=false 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 index 22c304bc2..516bdf092 100644 --- a/tests/build-remote-trustless-should-pass-1.sh +++ b/tests/build-remote-trustless-should-pass-1.sh @@ -1,9 +1,9 @@ source common.sh -# Remote trusts us but we pretend it doesn't. +# Remote trusts us file=build-hook.nix prog=nix-daemon proto=ssh-ng -trusting=false source build-remote-trustless.sh +source build-remote-trustless-after.sh diff --git a/tests/build-remote-trustless-should-pass-2.sh b/tests/build-remote-trustless-should-pass-2.sh index 941ddca05..6383f5489 100644 --- a/tests/build-remote-trustless-should-pass-2.sh +++ b/tests/build-remote-trustless-should-pass-2.sh @@ -1,9 +1,9 @@ source common.sh -# Remote doesn't trust us nor do we think it does +# Remote doesn't trust us file=build-hook.nix prog=$(readlink -e ./nix-daemon-untrusting.sh) proto=ssh-ng -trusting=false 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 index 0c4e191fc..c3ec359fb 100644 --- a/tests/build-remote-trustless-should-pass-3.sh +++ b/tests/build-remote-trustless-should-pass-3.sh @@ -1,10 +1,10 @@ source common.sh -# We act as if remote trusts us, but it doesn't. This is fine because we -# are only building (fixed) CA derivations. +# 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 -trusting=true 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 index 8260c4dcd..9df44e0c5 100644 --- a/tests/build-remote-trustless.sh +++ b/tests/build-remote-trustless.sh @@ -1,16 +1,14 @@ -if ! canUseSandbox; then exit; fi -if ! [[ $busybox =~ busybox ]]; then exit; fi +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 -L -v -f $file -o $TEST_ROOT/result --max-jobs 0 \ +nix-build $file -o $TEST_ROOT/result --max-jobs 0 \ --arg busybox $busybox \ --store $TEST_ROOT/local \ - --builders "$proto://localhost?remote-program=$prog&trusting=$trusting&remote-store=$TEST_ROOT/remote%3Fsystem-features=foo%20bar%20baz - - 1 1 foo,bar,baz" - -outPath=$(readlink -f $TEST_ROOT/result) - -grep 'FOO BAR BAZ' $TEST_ROOT/${subDir}/local${outPath} + --builders "$proto://localhost?remote-program=$prog&remote-store=${remoteDir}%3Fsystem-features=foo%20bar%20baz - - 1 1 foo,bar,baz"