diff --git a/src/libutil-tests/meson.build b/src/libutil-tests/meson.build index 019bdb6d2a3..24cbf18e549 100644 --- a/src/libutil-tests/meson.build +++ b/src/libutil-tests/meson.build @@ -33,6 +33,9 @@ deps_private += rapidcheck gtest = dependency('gtest', main : true) deps_private += gtest +gmock = dependency('gmock') +deps_private += gmock + configdata = configuration_data() configdata.set_quoted('PACKAGE_VERSION', meson.project_version()) @@ -72,6 +75,7 @@ sources = files( 'position.cc', 'processes.cc', 'sort.cc', + 'source-accessor.cc', 'spawn.cc', 'strings.cc', 'suggestions.cc', diff --git a/src/libutil-tests/source-accessor.cc b/src/libutil-tests/source-accessor.cc new file mode 100644 index 00000000000..50ae870c88c --- /dev/null +++ b/src/libutil-tests/source-accessor.cc @@ -0,0 +1,138 @@ +#include "nix/util/fs-sink.hh" +#include "nix/util/file-system.hh" +#include "nix/util/processes.hh" + +#include +#include +#include + +namespace nix { + +MATCHER_P2(HasContents, path, expected, "") +{ + auto stat = arg->maybeLstat(path); + if (!stat) { + *result_listener << arg->showPath(path) << " does not exist"; + return false; + } + if (stat->type != SourceAccessor::tRegular) { + *result_listener << arg->showPath(path) << " is not a regular file"; + return false; + } + auto actual = arg->readFile(path); + if (actual != expected) { + *result_listener << arg->showPath(path) << " has contents " << ::testing::PrintToString(actual); + return false; + } + return true; +} + +MATCHER_P2(HasSymlink, path, target, "") +{ + auto stat = arg->maybeLstat(path); + if (!stat) { + *result_listener << arg->showPath(path) << " does not exist"; + return false; + } + if (stat->type != SourceAccessor::tSymlink) { + *result_listener << arg->showPath(path) << " is not a symlink"; + return false; + } + auto actual = arg->readLink(path); + if (actual != target) { + *result_listener << arg->showPath(path) << " points to " << ::testing::PrintToString(actual); + return false; + } + return true; +} + +MATCHER_P2(HasDirectory, path, dirents, "") +{ + auto stat = arg->maybeLstat(path); + if (!stat) { + *result_listener << arg->showPath(path) << " does not exist"; + return false; + } + if (stat->type != SourceAccessor::tDirectory) { + *result_listener << arg->showPath(path) << " is not a directory"; + return false; + } + auto actual = arg->readDirectory(path); + std::set actualKeys, expectedKeys(dirents.begin(), dirents.end()); + for (auto & [k, _] : actual) + actualKeys.insert(k); + if (actualKeys != expectedKeys) { + *result_listener << arg->showPath(path) << " has entries " << ::testing::PrintToString(actualKeys); + return false; + } + return true; +} + +class FSSourceAccessorTest : public ::testing::Test +{ +protected: + std::filesystem::path tmpDir; + std::unique_ptr delTmpDir; + + void SetUp() override + { + tmpDir = nix::createTempDir(); + delTmpDir = std::make_unique(tmpDir, true); + } + + void TearDown() override + { + delTmpDir.release(); + } +}; + +TEST_F(FSSourceAccessorTest, works) +{ + { + RestoreSink sink(false); + sink.dstPath = tmpDir; + sink.dirFd = openDirectory(tmpDir); + sink.createDirectory(CanonPath("subdir")); + sink.createRegularFile(CanonPath("file1"), [](CreateRegularFileSink & crf) { crf("content1"); }); + sink.createRegularFile(CanonPath("subdir/file2"), [](CreateRegularFileSink & crf) { crf("content2"); }); + sink.createSymlink(CanonPath("rootlink"), "target"); + sink.createDirectory(CanonPath("a")); + sink.createSymlink(CanonPath("a/dirlink"), "../subdir"); + } + + EXPECT_THAT(makeFSSourceAccessor(tmpDir / "file1"), HasContents(CanonPath::root, "content1")); + EXPECT_THAT(makeFSSourceAccessor(tmpDir / "rootlink"), HasSymlink(CanonPath::root, "target")); + EXPECT_THAT( + makeFSSourceAccessor(tmpDir), + HasDirectory(CanonPath::root, std::set{"file1", "subdir", "rootlink", "a"})); + EXPECT_THAT(makeFSSourceAccessor(tmpDir / "subdir"), HasDirectory(CanonPath::root, std::set{"file2"})); + + { + auto accessor = makeFSSourceAccessor(tmpDir); + EXPECT_THAT(accessor, HasContents(CanonPath("file1"), "content1")); + EXPECT_THAT(accessor, HasContents(CanonPath("subdir/file2"), "content2")); + + EXPECT_TRUE(accessor->pathExists(CanonPath("file1"))); + EXPECT_FALSE(accessor->pathExists(CanonPath("nonexistent"))); + + EXPECT_THROW(accessor->readFile(CanonPath("a/dirlink/file2")), SymlinkNotAllowed); + EXPECT_THROW(accessor->maybeLstat(CanonPath("a/dirlink/file2")), SymlinkNotAllowed); + EXPECT_THROW(accessor->readDirectory(CanonPath("a/dirlink")), SymlinkNotAllowed); + EXPECT_THROW(accessor->pathExists(CanonPath("a/dirlink/file2")), SymlinkNotAllowed); + } + + { + auto accessor = makeFSSourceAccessor(tmpDir / "nonexistent"); + EXPECT_FALSE(accessor->maybeLstat(CanonPath::root)); + EXPECT_THROW(accessor->readFile(CanonPath::root), SystemError); + } + + { + auto accessor = makeFSSourceAccessor(tmpDir, true); + EXPECT_EQ(accessor->getLastModified(), 0); + accessor->maybeLstat(CanonPath("file1")); + EXPECT_GT(accessor->getLastModified(), 0); + } +} + +} // namespace nix diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 0291d682729..09767899830 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -103,7 +103,8 @@ void SourceAccessor::dumpPath(const CanonPath & path, Sink & sink, PathFilter & time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter) { - auto path2 = PosixSourceAccessor::createAtRoot(path, /*trackLastModified=*/true); + SourcePath path2 = { + makeFSSourceAccessor(std::filesystem::path{}, /*trackLastModified=*/true), CanonPath(absPath(path))}; path2.dumpPath(sink, filter); return path2.accessor->getLastModified().value(); } diff --git a/src/libutil/include/nix/util/file-descriptor.hh b/src/libutil/include/nix/util/file-descriptor.hh index d049845883c..441ec4d4fc9 100644 --- a/src/libutil/include/nix/util/file-descriptor.hh +++ b/src/libutil/include/nix/util/file-descriptor.hh @@ -2,7 +2,6 @@ ///@file #include "nix/util/canon-path.hh" -#include "nix/util/types.hh" #include "nix/util/error.hh" #ifdef _WIN32 @@ -236,18 +235,6 @@ std::wstring handleToFileName(Descriptor handle); #ifndef _WIN32 namespace unix { -struct SymlinkNotAllowed : public Error -{ - CanonPath path; - - SymlinkNotAllowed(CanonPath path) - /* Can't provide better error message, since the parent directory is only known to the caller. */ - : Error("relative path '%s' points to a symlink, which is not allowed", path.rel()) - , path(std::move(path)) - { - } -}; - /** * Safe(r) function to open \param path file relative to \param dirFd, while * disallowing escaping from a directory and resolving any symlinks in the diff --git a/src/libutil/include/nix/util/posix-source-accessor.hh b/src/libutil/include/nix/util/posix-source-accessor.hh index 29561a3daaf..37b8d9189cd 100644 --- a/src/libutil/include/nix/util/posix-source-accessor.hh +++ b/src/libutil/include/nix/util/posix-source-accessor.hh @@ -43,36 +43,6 @@ public: std::optional getPhysicalPath(const CanonPath & path) override; - /** - * Create a `PosixSourceAccessor` and `SourcePath` corresponding to - * some native path. - * - * @param Whether the accessor should return a non-null getLastModified. - * When true the accessor must be used only by a single thread. - * - * The `PosixSourceAccessor` is rooted as far up the tree as - * possible, (e.g. on Windows it could scoped to a drive like - * `C:\`). This allows more `..` parent accessing to work. - * - * @note When `path` is trusted user input, canonicalize it using - * `std::filesystem::canonical`, `makeParentCanonical`, `std::filesystem::weakly_canonical`, etc, - * as appropriate for the use case. At least weak canonicalization is - * required for the `SourcePath` to do anything useful at the location it - * points to. - * - * @note A canonicalizing behavior is not built in `createAtRoot` so that - * callers do not accidentally introduce symlink-related security vulnerabilities. - * Furthermore, `createAtRoot` does not know whether the file pointed to by - * `path` should be resolved if it is itself a symlink. In other words, - * `createAtRoot` can not decide between aforementioned `canonical`, `makeParentCanonical`, etc. for its callers. - * - * See - * [`std::filesystem::path::root_path`](https://en.cppreference.com/w/cpp/filesystem/path/root_path) - * and - * [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path). - */ - static SourcePath createAtRoot(const std::filesystem::path & path, bool trackLastModified = false); - std::optional getLastModified() override { return trackLastModified ? std::optional{mtime} : std::nullopt; diff --git a/src/libutil/include/nix/util/source-accessor.hh b/src/libutil/include/nix/util/source-accessor.hh index 1006895b33c..59af81569c7 100644 --- a/src/libutil/include/nix/util/source-accessor.hh +++ b/src/libutil/include/nix/util/source-accessor.hh @@ -222,6 +222,24 @@ ref makeEmptySourceAccessor(); */ MakeError(RestrictedPathError, Error); +struct SymlinkNotAllowed : public Error +{ + CanonPath path; + + SymlinkNotAllowed(CanonPath path) + : Error("relative path '%s' points to a symlink, which is not allowed", path.rel()) + , path(std::move(path)) + { + } + + template + SymlinkNotAllowed(CanonPath path, const std::string & fs, Args &&... args) + : Error(fs, std::forward(args)...) + , path(std::move(path)) + { + } +}; + /** * Return an accessor for the root filesystem. */ @@ -233,7 +251,7 @@ ref getFSSourceAccessor(); * elements, and that absolute symlinks are resolved relative to * `root`. */ -ref makeFSSourceAccessor(std::filesystem::path root); +ref makeFSSourceAccessor(std::filesystem::path root, bool trackLastModified = false); /** * Construct an accessor that presents a "union" view of a vector of diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index abbab45db21..2c41c775e27 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -1,7 +1,5 @@ #include "nix/util/posix-source-accessor.hh" -#include "nix/util/source-path.hh" #include "nix/util/signals.hh" -#include "nix/util/sync.hh" #include @@ -20,15 +18,6 @@ PosixSourceAccessor::PosixSourceAccessor() { } -SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path, bool trackLastModified) -{ - std::filesystem::path path2 = absPath(path); - return { - make_ref(path2.root_path(), trackLastModified), - CanonPath{path2.relative_path().string()}, - }; -} - std::filesystem::path PosixSourceAccessor::makeAbsPath(const CanonPath & path) { return root.empty() ? (std::filesystem::path{path.abs()}) @@ -208,7 +197,7 @@ void PosixSourceAccessor::assertNoSymlinks(CanonPath path) while (!path.isRoot()) { auto st = cachedLstat(path); if (st && S_ISLNK(st->st_mode)) - throw Error("path '%s' is a symlink", showPath(path)); + throw SymlinkNotAllowed(path, "path '%s' is a symlink", showPath(path)); path.pop(); } } @@ -219,8 +208,8 @@ ref getFSSourceAccessor() return rootFS; } -ref makeFSSourceAccessor(std::filesystem::path root) +ref makeFSSourceAccessor(std::filesystem::path root, bool trackLastModified) { - return make_ref(std::move(root)); + return make_ref(std::move(root), trackLastModified); } } // namespace nix diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc index d90342ff0ec..bdb8054eb5a 100644 --- a/src/libutil/unix/file-descriptor.cc +++ b/src/libutil/unix/file-descriptor.cc @@ -3,6 +3,7 @@ #include "nix/util/signals.hh" #include "nix/util/finally.hh" #include "nix/util/serialise.hh" +#include "nix/util/source-accessor.hh" #include #include @@ -301,10 +302,10 @@ openFileEnsureBeneathNoSymlinksIterative(Descriptor dirFd, const CanonPath & pat if (errno == ENOTDIR) /* Path component might be a symlink. */ { struct ::stat st; if (::fstatat(getParentFd(), component.c_str(), &st, AT_SYMLINK_NOFOLLOW) == 0 && S_ISLNK(st.st_mode)) - throw unix::SymlinkNotAllowed(path2); + throw SymlinkNotAllowed(path2); errno = ENOTDIR; /* Restore the errno. */ } else if (errno == ELOOP) { - throw unix::SymlinkNotAllowed(path2); + throw SymlinkNotAllowed(path2); } return INVALID_DESCRIPTOR; @@ -315,7 +316,7 @@ openFileEnsureBeneathNoSymlinksIterative(Descriptor dirFd, const CanonPath & pat auto res = ::openat(getParentFd(), std::string(path.baseName().value()).c_str(), flags | O_NOFOLLOW, mode); if (res < 0 && errno == ELOOP) - throw unix::SymlinkNotAllowed(path); + throw SymlinkNotAllowed(path); return res; } @@ -328,7 +329,7 @@ Descriptor unix::openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPa dirFd, path.rel_c_str(), flags, static_cast(mode), RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS); if (maybeFd) { if (*maybeFd < 0 && errno == ELOOP) - throw unix::SymlinkNotAllowed(path); + throw SymlinkNotAllowed(path); return *maybeFd; } #endif diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index e87f4954607..0c6a1f74bc3 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -38,7 +38,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand if (!namePart) namePart = baseNameOf(path); - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(path)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(path)); auto storePath = dryRun ? store->computeStorePath(*namePart, sourcePath, caMethod, hashAlgo, {}).first : store->addToStoreSlow(*namePart, sourcePath, caMethod, hashAlgo, {}).path; diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 2945c672c2c..eeb9a81bc14 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -85,9 +85,7 @@ struct CmdHashBase : Command return std::make_unique(hashAlgo); }; - auto makeSourcePath = [&]() -> SourcePath { - return PosixSourceAccessor::createAtRoot(makeParentCanonical(path)); - }; + auto makeSourcePath = [&]() -> SourcePath { return makeFSSourceAccessor(makeParentCanonical(path)); }; Hash h{HashAlgorithm::SHA256}; // throwaway def to appease C++ switch (mode) { diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index a2c0aaf3ff8..0f657cd6439 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -191,7 +191,7 @@ static void opAdd(Strings opFlags, Strings opArgs) throw UsageError("unknown flag"); for (auto & i : opArgs) { - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(i)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(i)); cout << fmt("%s\n", store->printStorePath(store->addToStore(std::string(baseNameOf(i)), sourcePath))); } } @@ -215,7 +215,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) opArgs.pop_front(); for (auto & i : opArgs) { - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(i)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(i)); std::cout << fmt( "%s\n", store->printStorePath(store->addToStoreSlow(baseNameOf(i), sourcePath, method, hashAlgo).path)); } diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 93e9f0f9541..f87b5def889 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -256,7 +256,7 @@ hashPath(char * algo, int base32, char * path) PPCODE: try { Hash h = hashPath( - PosixSourceAccessor::createAtRoot(path), + SourcePath{getFSSourceAccessor(), CanonPath(absPath(Path(path)))}, FileIngestionMethod::NixArchive, parseHashAlgo(algo)).first; auto s = h.to_string(base32 ? HashFormat::Nix32 : HashFormat::Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); @@ -336,7 +336,7 @@ StoreWrapper::addToStore(char * srcPath, int recursive, char * algo) auto method = recursive ? ContentAddressMethod::Raw::NixArchive : ContentAddressMethod::Raw::Flat; auto path = THIS->store->addToStore( std::string(baseNameOf(srcPath)), - PosixSourceAccessor::createAtRoot(srcPath), + SourcePath{getFSSourceAccessor(), CanonPath(absPath(Path(srcPath)))}, method, parseHashAlgo(algo)); XPUSHs(sv_2mortal(newSVpv(THIS->store->printStorePath(path).c_str(), 0))); } catch (Error & e) {