Merge remote-tracking branch 'upstream/master' into single-ca-drv-build

12 files changed, 179 insertions, 31 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index c3d01eea3..0ddba6384 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -800,7 +800,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
         Hash h = newHashAllowEmpty(*outputHash, ht);
 
         auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName);
-        if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath);
+        drv.env["out"] = state.store->printStorePath(outPath);
         drv.outputs.insert_or_assign("out", DerivationOutput {
                 .output = DerivationOutputCAFixed {
                     .hash = FixedOutputHash {
@@ -814,7 +814,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
     else if (contentAddressed) {
         HashType ht = parseHashType(outputHashAlgo);
         for (auto & i : outputs) {
-            if (!jsonObject) drv.env[i] = hashPlaceholder(i);
+            drv.env[i] = hashPlaceholder(i);
             drv.outputs.insert_or_assign(i, DerivationOutput {
                 .output = DerivationOutputCAFloating {
                     .method = ingestionMethod,
@@ -832,7 +832,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
            that changes in the set of output names do get reflected in
            the hash. */
         for (auto & i : outputs) {
-            if (!jsonObject) drv.env[i] = "";
+            drv.env[i] = "";
             drv.outputs.insert_or_assign(i,
                 DerivationOutput {
                     .output = DerivationOutputInputAddressed {
@@ -847,7 +847,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
 
         for (auto & i : outputs) {
             auto outPath = state.store->makeOutputPath(i, h, drvName);
-            if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath);
+            drv.env[i] = state.store->printStorePath(outPath);
             drv.outputs.insert_or_assign(i,
                 DerivationOutput {
                     .output = DerivationOutputInputAddressed {
diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 8b6e047f1..5ca0f8521 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -269,7 +269,7 @@ struct GitInputScheme : InputScheme
                 // modified dirty file?
                 input.attrs.insert_or_assign(
                     "lastModified",
-                    haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "HEAD" })) : 0);
+                    haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0);
 
                 return {
                     Tree(store->printStorePath(storePath), std::move(storePath)),
@@ -421,7 +421,7 @@ struct GitInputScheme : InputScheme
 
         auto storePath = store->addToStore(name, tmpDir, FileIngestionMethod::Recursive, htSHA256, filter);
 
-        auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", input.getRev()->gitRev() }));
+        auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", "--no-show-signature", input.getRev()->gitRev() }));
 
         Attrs infoAttrs({
             {"rev", input.getRev()->gitRev()},
diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc
index a4eeebaab..f35ddb522 100644
--- a/src/libstore/daemon.cc
+++ b/src/libstore/daemon.cc
@@ -454,8 +454,46 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         readDerivation(from, *store, drv, Derivation::nameFromPath(drvPath));
         BuildMode buildMode = (BuildMode) readInt(from);
         logger->startWork();
-        if (!trusted)
-            throw Error("you are not privileged to build derivations");
+
+        /* Content-addressed derivations are trustless because their output paths
+           are verified by their content alone, so any derivation is free to
+           try to produce such a path.
+
+           Input-addressed derivation output paths, however, are calculated
+           from the derivation closure that produced them---even knowing the
+           root derivation is not enough. That the output data actually came
+           from those derivations is fundamentally unverifiable, but the daemon
+           trusts itself on that matter. The question instead is whether the
+           submitted plan has rights to the output paths it wants to fill, and
+           at least the derivation closure proves that.
+
+           It would have been nice if input-address algorithm merely depended
+           on the build time closure, rather than depending on the derivation
+           closure. That would mean input-addressed paths used at build time
+           would just be trusted and not need their own evidence. This is in
+           fact fine as the same guarantees would hold *inductively*: either
+           the remote builder has those paths and already trusts them, or it
+           needs to build them too and thus their evidence must be provided in
+           turn.  The advantage of this variant algorithm is that the evidence
+           for input-addressed paths which the remote builder already has
+           doesn't need to be sent again.
+
+           That said, now that we have floating CA derivations, it is better
+           that people just migrate to those which also solve this problem, and
+           others. It's the same migration difficulty with strictly more
+           benefit.
+
+           Lastly, do note that when we parse fixed-output content-addressed
+           derivations, we throw out the precomputed output paths and just
+           store the hashes, so there aren't two competing sources of truth an
+           attacker could exploit. */
+        if (drv.type() == DerivationType::InputAddressed && !trusted)
+            throw Error("you are not privileged to build input-addressed derivations");
+
+        /* Make sure that the non-input-addressed derivations that got this far
+           are in fact content-addressed if we don't trust them. */
+        assert(derivationIsCA(drv.type()) || trusted);
+
         auto res = store->buildDerivation(drvPath, drv, buildMode);
         logger->stopWork();
         to << res.status << res.errorMsg;
@@ -688,7 +726,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         auto path = store->parseStorePath(readString(from));
         logger->startWork();
         logger->stopWork();
-        dumpPath(store->printStorePath(path), to);
+        dumpPath(store->toRealPath(path), to);
         break;
     }
 
diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
index 9f95a9726..dc03313f0 100644
--- a/src/libstore/legacy-ssh-store.cc
+++ b/src/libstore/legacy-ssh-store.cc
@@ -209,6 +209,24 @@ struct LegacySSHStore : public Store
         const StorePathSet & references, RepairFlag repair) override
     { unsupported("addTextToStore"); }
 
+private:
+
+    void putBuildSettings(Connection & conn)
+    {
+        conn.to
+            << settings.maxSilentTime
+            << settings.buildTimeout;
+        if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 2)
+            conn.to
+                << settings.maxLogSize;
+        if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 3)
+            conn.to
+                << settings.buildRepeat
+                << settings.enforceDeterminism;
+    }
+
+public:
+
     BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
         BuildMode buildMode) override
     {
@@ -218,16 +236,8 @@ struct LegacySSHStore : public Store
             << cmdBuildDerivation
             << printStorePath(drvPath);
         writeDerivation(conn->to, *this, drv);
-        conn->to
-            << settings.maxSilentTime
-            << settings.buildTimeout;
-        if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 2)
-            conn->to
-                << settings.maxLogSize;
-        if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3)
-            conn->to
-                << settings.buildRepeat
-                << settings.enforceDeterminism;
+
+        putBuildSettings(*conn);
 
         conn->to.flush();
 
@@ -241,6 +251,29 @@ struct LegacySSHStore : public Store
         return status;
     }
 
+    void buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode) override
+    {
+        auto conn(connections->get());
+
+        conn->to << cmdBuildPaths;
+        Strings ss;
+        for (auto & p : drvPaths)
+            ss.push_back(p.to_string(*this));
+        conn->to << ss;
+
+        putBuildSettings(*conn);
+
+        conn->to.flush();
+
+        BuildResult result;
+        result.status = (BuildResult::Status) readInt(conn->from);
+
+        if (!result.success()) {
+            conn->from >> result.errorMsg;
+            throw Error(result.status, result.errorMsg);
+        }
+    }
+
     void ensurePath(const StorePath & path) override
     { unsupported("ensurePath"); }
 
diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index b3576fbeb..13cf8d4ab 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -6,7 +6,7 @@ namespace nix {
 #define WORKER_MAGIC_1 0x6e697863
 #define WORKER_MAGIC_2 0x6478696f
 
-#define PROTOCOL_VERSION 0x117
+#define PROTOCOL_VERSION 0x118
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index 434088da7..9aaa80822 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -246,7 +246,7 @@ struct CmdDevelop : Common, MixEnvironment
         addFlag({
             .longName = "command",
             .shortName = 'c',
-            .description = "command and arguments to be executed insted of an interactive shell",
+            .description = "command and arguments to be executed instead of an interactive shell",
             .labels = {"command", "args"},
             .handler = {[&](std::vector<std::string> ss) {
                 if (ss.empty()) throw UsageError("--command requires at least one argument");
diff --git a/tests/build-hook-ca.nix b/tests/build-hook-ca.nix
new file mode 100644
index 000000000..98db473fc
--- /dev/null
+++ b/tests/build-hook-ca.nix
@@ -0,0 +1,45 @@
+{ busybox }:
+
+with import ./config.nix;
+
+let
+
+  mkDerivation = args:
+    derivation ({
+      inherit system;
+      builder = busybox;
+      args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
+      outputHashMode = "recursive";
+      outputHashAlgo = "sha256";
+    } // removeAttrs args ["builder" "meta"])
+    // { meta = args.meta or {}; };
+
+  input1 = mkDerivation {
+    shell = busybox;
+    name = "build-remote-input-1";
+    buildCommand = "echo FOO > $out";
+    requiredSystemFeatures = ["foo"];
+    outputHash = "sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc=";
+  };
+
+  input2 = mkDerivation {
+    shell = busybox;
+    name = "build-remote-input-2";
+    buildCommand = "echo BAR > $out";
+    requiredSystemFeatures = ["bar"];
+    outputHash = "sha256-XArauVH91AVwP9hBBQNlkX9ccuPpSYx9o0zeIHb6e+Q=";
+  };
+
+in
+
+  mkDerivation {
+    shell = busybox;
+    name = "build-remote";
+    buildCommand =
+      ''
+        read x < ${input1}
+        read y < ${input2}
+        echo "$x $y" > $out
+      '';
+    outputHash = "sha256-3YGhlOfbGUm9hiPn2teXXTT8M1NEpDFvfXkxMaJRld0=";
+  }
diff --git a/tests/build-hook.nix b/tests/build-hook.nix
index 1bd0b759f..eb16676f0 100644
--- a/tests/build-hook.nix
+++ b/tests/build-hook.nix
@@ -26,6 +26,16 @@ let
     requiredSystemFeatures = ["bar"];
   };
 
+  input3 = mkDerivation {
+    shell = busybox;
+    name = "build-remote-input-3";
+    buildCommand = ''
+      read x < ${input2}
+      echo $x BAZ > $out
+    '';
+    requiredSystemFeatures = ["baz"];
+  };
+
 in
 
   mkDerivation {
@@ -34,7 +44,7 @@ in
     buildCommand =
       ''
         read x < ${input1}
-        read y < ${input2}
+        read y < ${input3}
         echo "$x $y" > $out
       '';
   }
diff --git a/tests/build-remote-content-addressed-fixed.sh b/tests/build-remote-content-addressed-fixed.sh
new file mode 100644
index 000000000..1408a19d5
--- /dev/null
+++ b/tests/build-remote-content-addressed-fixed.sh
@@ -0,0 +1,5 @@
+source common.sh
+
+file=build-hook-ca.nix
+
+source build-remote.sh
diff --git a/tests/build-remote-input-addressed.sh b/tests/build-remote-input-addressed.sh
new file mode 100644
index 000000000..b34caa061
--- /dev/null
+++ b/tests/build-remote-input-addressed.sh
@@ -0,0 +1,5 @@
+source common.sh
+
+file=build-hook.nix
+
+source build-remote.sh
diff --git a/tests/build-remote.sh b/tests/build-remote.sh
index 7638f536f..ca6d1de09 100644
--- a/tests/build-remote.sh
+++ b/tests/build-remote.sh
@@ -1,5 +1,3 @@
-source common.sh
-
 if ! canUseSandbox; then exit; fi
 if ! [[ $busybox =~ busybox ]]; then exit; fi
 
@@ -13,24 +11,37 @@ builders=(
   # remote-store URL.
   "ssh://localhost?remote-store=$TEST_ROOT/machine1?system-features=foo - - 1 1 foo"
   "$TEST_ROOT/machine2 - - 1 1 bar"
+  "ssh-ng://localhost?remote-store=$TEST_ROOT/machine3?system-features=baz - - 1 1 baz"
 )
 
 # Note: ssh://localhost bypasses ssh, directly invoking nix-store as a
 # child process. This allows us to test LegacySSHStore::buildDerivation().
 # ssh-ng://... likewise allows us to test RemoteStore::buildDerivation().
-nix build -L -v -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \
+nix build -L -v -f $file -o $TEST_ROOT/result --max-jobs 0 \
   --arg busybox $busybox \
   --store $TEST_ROOT/machine0 \
   --builders "$(join_by '; ' "${builders[@]}")"
 
 outPath=$(readlink -f $TEST_ROOT/result)
 
-grep 'FOO BAR' $TEST_ROOT/machine0/$outPath
+grep 'FOO BAR BAZ' $TEST_ROOT/machine0/$outPath
+
+set -o pipefail
 
 # Ensure that input1 was built on store1 due to the required feature.
-(! nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh)
-nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh
+nix path-info --store $TEST_ROOT/machine1 --all \
+  | grep builder-build-remote-input-1.sh \
+  | grep -v builder-build-remote-input-2.sh \
+  | grep -v builder-build-remote-input-3.sh
 
 # Ensure that input2 was built on store2 due to the required feature.
-(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-2.sh)
-nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-2.sh
+nix path-info --store $TEST_ROOT/machine2 --all \
+  | grep -v builder-build-remote-input-1.sh \
+  | grep builder-build-remote-input-2.sh \
+  | grep -v builder-build-remote-input-3.sh
+
+# Ensure that input3 was built on store3 due to the required feature.
+nix path-info --store $TEST_ROOT/machine3 --all \
+  | grep -v builder-build-remote-input-1.sh \
+  | grep -v builder-build-remote-input-2.sh \
+  | grep builder-build-remote-input-3.sh
diff --git a/tests/local.mk b/tests/local.mk
index aeb0f520f..5d25de019 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -14,7 +14,7 @@ nix_tests = \
   placeholders.sh nix-shell.sh \
   linux-sandbox.sh \
   build-dry.sh \
-  build-remote.sh \
+  build-remote-input-addressed.sh \
   nar-access.sh \
   structured-attrs.sh \
   fetchGit.sh \
@@ -35,6 +35,7 @@ nix_tests = \
   flakes.sh \
   content-addressed.sh
   # parallel.sh
+  # build-remote-content-addressed-fixed.sh \
 
 install-tests += $(foreach x, $(nix_tests), tests/$(x))