From 5985b8b5275605ddd5e92e2f0a7a9f494ac6e35d Mon Sep 17 00:00:00 2001 From: regnat 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 | 57 +++++++++++++++++++++++++++++ src/libstore/local-store.hh | 13 +++++++ tests/local.mk | 1 + tests/substitute-with-invalid-ca.sh | 38 +++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 tests/substitute-with-invalid-ca.sh diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 3b99ba39d..efea7bd23 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1168,6 +1168,31 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, throw Error("size mismatch importing path '%s';\n specified: %s\n got: %s", printStorePath(info.path), info.narSize, hashResult.second); + if (info.ca) { + if (auto foHash = std::get_if(&*info.ca)) { + auto actualFoHash = hashCAPath( + foHash->method, + foHash->hash.type, + info.path + ); + if (foHash->hash != actualFoHash.hash) { + throw Error("ca hash mismatch importing path '%s';\n specified: %s\n got: %s", + printStorePath(info.path), + foHash->hash.to_string(Base32, true), + actualFoHash.hash.to_string(Base32, true)); + } + } + if (auto textHash = std::get_if(&*info.ca)) { + auto actualTextHash = hashString(htSHA256, readFile(realPath)); + if (textHash->hash != actualTextHash) { + throw Error("ca hash mismatch importing path '%s';\n specified: %s\n got: %s", + printStorePath(info.path), + textHash->hash.to_string(Base32, true), + actualTextHash.to_string(Base32, true)); + } + } + } + autoGC(); canonicalisePathMetaData(realPath, -1); @@ -1672,4 +1697,36 @@ std::optional LocalStore::queryRealisation( .id = id, .outPath = outputPath, .signatures = signatures}}; }); } + + +FixedOutputHash LocalStore::hashCAPath( + const FileIngestionMethod & method, const HashType & hashType, + const StorePath & path) +{ + return hashCAPath(method, hashType, Store::toRealPath(path), path.hashPart()); +} + +FixedOutputHash LocalStore::hashCAPath( + const FileIngestionMethod & method, + const HashType & hashType, + const Path & path, + const std::string_view pathHash +) +{ + HashModuloSink caSink ( hashType, std::string(pathHash) ); + switch (method) { + case FileIngestionMethod::Recursive: + dumpPath(path, caSink); + break; + case FileIngestionMethod::Flat: + readFile(path, caSink); + break; + } + auto hash = caSink.finish().first; + return FixedOutputHash{ + .method = method, + .hash = hash, + }; +} + } // namespace nix diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 26e034a82..c0b8e0da6 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -283,6 +283,19 @@ private: void createUser(const std::string & userName, uid_t userId) override; + // XXX: Make a generic `Store` method + FixedOutputHash hashCAPath( + const FileIngestionMethod & method, + const HashType & hashType, + const StorePath & path); + + FixedOutputHash hashCAPath( + const FileIngestionMethod & method, + const HashType & hashType, + const Path & path, + const std::string_view pathHash + ); + friend struct LocalDerivationGoal; friend struct PathSubstitutionGoal; friend struct SubstitutionGoal; diff --git a/tests/local.mk b/tests/local.mk index 542be6b7e..59eb4eb0f 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -11,6 +11,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 \ + substitute-with-invalid-ca.sh \ binary-cache-build-remote.sh \ nix-profile.sh repair.sh dump-db.sh case-hack.sh \ check-reqs.sh pass-as-file.sh tarball.sh restricted.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 < $TEST_HOME/good.txt +I’m a good path +EOF + +cat < $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