From e43bb655feaa23977322d68278c88ac075eb2c41 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sun, 10 Dec 2023 13:58:35 -0800 Subject: [PATCH] libstore/daemon.cc: note trust model difference in readDerivation()s Below the comment added by this commit is a much longer comment followed by a trust check, both of which have confused me on at least two occasions. I figured it out once, forgot it, then had to ask @Ericson2314 to explain it, at which point I understood it again. I think this might confuse other people too, or maybe I will just forget it a third time. So let's add a comment. Farther down in the function is the following check: ``` if (!(drvType.isCA() || trusted)) throw Error("you are not privileged to build input-addressed derivations"); ``` This seems really strange at first. A key property of Nix is that you can compute the outpath of a derivation using the derivation (and its references-closure) without trusting anybody! The missing insight is that at this point in the code the builder doesn't necessarily have the references-closure of the derivation being built, and therefore needs to trust that the derivation's outPath is honest. It's incredibly easy to overlook this, because the only difference between these two cases is which of these identically-named functions we used: - `readDerivation(Source,Store)` - `Store::readDerivation()` These functions have different trust models (except in the special case where the first function is used on the local store). We should call the reader's attention to this fact. Co-authored-by: Cole Helbling --- src/libstore/daemon.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 530b1a178..a112d6d31 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -574,6 +574,15 @@ static void performOp(TunnelLogger * logger, ref store, case WorkerProto::Op::BuildDerivation: { auto drvPath = store->parseStorePath(readString(from)); BasicDerivation drv; + /* + * Note: unlike wopEnsurePath, this operation reads a + * derivation-to-be-realized from the client with + * readDerivation(Source,Store) rather than reading it from + * the local store with Store::readDerivation(). Since the + * derivation-to-be-realized is not registered in the store + * it cannot be trusted that its outPath was calculated + * correctly. + */ readDerivation(from, *store, drv, Derivation::nameFromPath(drvPath)); BuildMode buildMode = (BuildMode) readInt(from); logger->startWork();