From 3dbd83b9a196b8c58eda9b6033442bf2020a9f60 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Thu, 27 May 2021 13:25:25 +0200
Subject: [PATCH] Check the CA hash when importing stuff in the local store
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When adding a path to the local store (via `LocalStore::addToStore`),
ensure that the `ca` field of the provided `ValidPathInfo` does indeed
correspond to the content of the path.
Otherwise any untrusted user (or any binary cache) can add arbitrary
content-addressed paths to the store (as content-addressed paths don’t
need a signature).
---
 src/libstore/local-store.cc         | 50 +++++++++++++++++++++++++++++
 src/libstore/local-store.hh         | 10 ++++--
 tests/local.mk                      |  1 +
 tests/substitute-with-invalid-ca.sh | 38 ++++++++++++++++++++++
 4 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 tests/substitute-with-invalid-ca.sh

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index f5092151a..5b62d79ee 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1029,6 +1029,40 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
                 throw Error("size mismatch importing path '%s';\n  wanted: %s\n  got:   %s",
                     info.path, info.narSize, hashResult.second);
 
+            if (!info.ca.empty()) {
+                auto ca = info.ca;
+                if (hasPrefix(ca, "fixed:")) {
+                    bool recursive = ca.compare(6, 2, "r:") == 0;
+                    Hash expectedHash(std::string(ca, recursive ? 8 : 6));
+                    if (info.references.empty()) {
+                        auto actualFoHash = hashCAPath(
+                                recursive,
+                                expectedHash.type,
+                                info.path
+                                );
+                        if (ca != actualFoHash) {
+                            throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
+                                    info.path,
+                                    ca,
+                                    actualFoHash);
+                        }
+                    } else {
+                        throw Error("path '%s' claims to be content-addressed, but has references. This isn’t allowed",
+                                info.path);
+                    }
+
+                } else if (hasPrefix(ca, "text:")) {
+                    Hash textHash(std::string(ca, 5));
+                    auto actualTextHash = hashString(htSHA256, readFile(realPath));
+                    if (textHash != actualTextHash) {
+                        throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
+                                info.path,
+                                textHash.to_string(Base32, true),
+                                actualTextHash.to_string(Base32, true));
+                    }
+                }
+            }
+
             autoGC();
 
             canonicalisePathMetaData(realPath, -1);
@@ -1450,4 +1484,20 @@ void LocalStore::createUser(const std::string & userName, uid_t userId)
 }
 
 
+std::string LocalStore::hashCAPath(
+    bool recursive,
+    const HashType & hashType,
+    const Path & path
+)
+{
+    HashSink caSink(hashType);
+    if (recursive) {
+        dumpPath(path, caSink);
+    } else {
+        readFile(path, caSink);
+    }
+    auto hash = caSink.finish().first;
+    return makeFixedOutputCA(recursive, hash);
+}
+
 }
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 379a06af8..0185d0ebf 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -295,8 +295,14 @@ private:
 
     void createUser(const std::string & userName, uid_t userId) override;
 
-    friend class DerivationGoal;
-    friend class SubstitutionGoal;
+    std::string hashCAPath(
+        bool recursive,
+        const HashType & hashType,
+        const Path & path
+    );
+
+    friend struct DerivationGoal;
+    friend struct SubstitutionGoal;
 };
 
 
diff --git a/tests/local.mk b/tests/local.mk
index 187f96ea2..4c087ae39 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -12,6 +12,7 @@ nix_tests = \
   timeout.sh secure-drv-outputs.sh nix-channel.sh \
   multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \
   binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \
+  substitute-with-invalid-ca.sh \
   check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \
   placeholders.sh nix-shell.sh \
   linux-sandbox.sh \
diff --git a/tests/substitute-with-invalid-ca.sh b/tests/substitute-with-invalid-ca.sh
new file mode 100644
index 000000000..4d0b01e0f
--- /dev/null
+++ b/tests/substitute-with-invalid-ca.sh
@@ -0,0 +1,38 @@
+source common.sh
+
+BINARY_CACHE=file://$cacheDir
+
+getHash() {
+    basename "$1" | cut -d '-' -f 1
+}
+getRemoteNarInfo () {
+    echo "$cacheDir/$(getHash "$1").narinfo"
+}
+
+cat <<EOF > $TEST_HOME/good.txt
+I’m a good path
+EOF
+
+cat <<EOF > $TEST_HOME/bad.txt
+I’m a bad path
+EOF
+
+good=$(nix-store --add $TEST_HOME/good.txt)
+bad=$(nix-store --add $TEST_HOME/bad.txt)
+nix copy --to "$BINARY_CACHE" "$good"
+nix copy --to "$BINARY_CACHE" "$bad"
+nix-collect-garbage >/dev/null 2>&1
+
+# Falsifying the narinfo file for '$good'
+goodPathNarInfo=$(getRemoteNarInfo "$good")
+badPathNarInfo=$(getRemoteNarInfo "$bad")
+for fieldName in URL FileHash FileSize NarHash NarSize; do
+    sed -i "/^$fieldName/d" "$goodPathNarInfo"
+    grep -E "^$fieldName" "$badPathNarInfo" >> "$goodPathNarInfo"
+done
+
+# Copying back '$good' from the binary cache. This should fail as it is
+# corrupted
+if nix copy --from "$BINARY_CACHE" "$good"; then
+    fail "Importing a path with a wrong CA field should fail"
+fi