Move access control from FSInputAccessor to FilteringInputAccessor

src/libexpr/eval.cc

@@ -14,6 +14,7 @@
 #include "profiles.hh"
 #include "print.hh"
 #include "fs-input-accessor.hh"
+#include "filtering-input-accessor.hh"
 #include "memory-input-accessor.hh"
 #include "signals.hh"
 #include "gc-small-vector.hh"
@@ -510,17 +511,15 @@ EvalState::EvalState(
     , repair(NoRepair)
     , emptyBindings(0)
     , rootFS(
-        makeFSInputAccessor(
-            CanonPath::root,
         evalSettings.restrictEval || evalSettings.pureEval
-            ? std::optional<std::set<CanonPath>>(std::set<CanonPath>())
+        ? ref<InputAccessor>(AllowListInputAccessor::create(makeFSInputAccessor(CanonPath::root), {},
-            : std::nullopt,
             [](const CanonPath & path) -> RestrictedPathError {
                 auto modeInformation = evalSettings.pureEval
                     ? "in pure evaluation mode (use '--impure' to override)"
                     : "in restricted mode";
                 throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
             }))
+        : makeFSInputAccessor(CanonPath::root))
     , corepkgsFS(makeMemoryInputAccessor())
     , internalFS(makeMemoryInputAccessor())
     , derivationInternal{corepkgsFS->addFile(
@@ -563,7 +562,7 @@ EvalState::EvalState(
     }
 
     /* Allow access to all paths in the search path. */
-    if (rootFS->hasAccessControl())
+    if (rootFS.dynamic_pointer_cast<AllowListInputAccessor>())
         for (auto & i : searchPath.elements)
             resolveSearchPathPath(i.path, true);
 
@@ -583,12 +582,14 @@ EvalState::~EvalState()
 
 void EvalState::allowPath(const Path & path)
 {
-    rootFS->allowPath(CanonPath(path));
+    if (auto rootFS2 = rootFS.dynamic_pointer_cast<AllowListInputAccessor>())
+        rootFS2->allowPath(CanonPath(path));
 }
 
 void EvalState::allowPath(const StorePath & storePath)
 {
-    rootFS->allowPath(CanonPath(store->toRealPath(storePath)));
+    if (auto rootFS2 = rootFS.dynamic_pointer_cast<AllowListInputAccessor>())
+        rootFS2->allowPath(CanonPath(store->toRealPath(storePath)));
 }
 
 void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value & v)
@@ -617,12 +618,14 @@ void EvalState::checkURI(const std::string & uri)
     /* If the URI is a path, then check it against allowedPaths as
        well. */
     if (hasPrefix(uri, "/")) {
-        rootFS->checkAllowed(CanonPath(uri));
+        if (auto rootFS2 = rootFS.dynamic_pointer_cast<AllowListInputAccessor>())
+            rootFS2->checkAccess(CanonPath(uri));
         return;
     }
 
     if (hasPrefix(uri, "file://")) {
-        rootFS->checkAllowed(CanonPath(uri.substr(7)));
+        if (auto rootFS2 = rootFS.dynamic_pointer_cast<AllowListInputAccessor>())
+            rootFS2->checkAccess(CanonPath(uri.substr(7)));
         return;
     }
 

src/libexpr/eval.hh

@@ -30,7 +30,6 @@ class EvalState;
 class StorePath;
 struct SingleDerivedPath;
 enum RepairFlag : bool;
-struct FSInputAccessor;
 struct MemoryInputAccessor;
 
 
@@ -222,7 +221,7 @@ public:
     /**
      * The accessor for the root filesystem.
      */
-    const ref<FSInputAccessor> rootFS;
+    const ref<InputAccessor> rootFS;
 
     /**
      * The in-memory filesystem for <nix/...> paths.

src/libfetchers/filtering-input-accessor.cc

@@ -0,0 +1,83 @@
+#include "filtering-input-accessor.hh"
+
+namespace nix {
+
+std::string FilteringInputAccessor::readFile(const CanonPath & path)
+{
+    checkAccess(path);
+    return next->readFile(prefix + path);
+}
+
+bool FilteringInputAccessor::pathExists(const CanonPath & path)
+{
+    return isAllowed(path) && next->pathExists(prefix + path);
+}
+
+std::optional<InputAccessor::Stat> FilteringInputAccessor::maybeLstat(const CanonPath & path)
+{
+    checkAccess(path);
+    return next->maybeLstat(prefix + path);
+}
+
+InputAccessor::DirEntries FilteringInputAccessor::readDirectory(const CanonPath & path)
+{
+    checkAccess(path);
+    DirEntries entries;
+    for (auto & entry : next->readDirectory(prefix + path)) {
+        if (isAllowed(path + entry.first))
+            entries.insert(std::move(entry));
+    }
+    return entries;
+}
+
+std::string FilteringInputAccessor::readLink(const CanonPath & path)
+{
+    checkAccess(path);
+    return next->readLink(prefix + path);
+}
+
+std::string FilteringInputAccessor::showPath(const CanonPath & path)
+{
+    return next->showPath(prefix + path);
+}
+
+void FilteringInputAccessor::checkAccess(const CanonPath & path)
+{
+    if (!isAllowed(path))
+        throw makeNotAllowedError
+            ? makeNotAllowedError(path)
+            : RestrictedPathError("access to path '%s' is forbidden", showPath(path));
+}
+
+struct AllowListInputAccessorImpl : AllowListInputAccessor
+{
+    std::set<CanonPath> allowedPaths;
+
+    AllowListInputAccessorImpl(
+        ref<InputAccessor> next,
+        std::set<CanonPath> && allowedPaths,
+        MakeNotAllowedError && makeNotAllowedError)
+        : AllowListInputAccessor(SourcePath(next), std::move(makeNotAllowedError))
+        , allowedPaths(std::move(allowedPaths))
+    { }
+
+    bool isAllowed(const CanonPath & path) override
+    {
+        return path.isAllowed(allowedPaths);
+    }
+
+    void allowPath(CanonPath path) override
+    {
+        allowedPaths.insert(std::move(path));
+    }
+};
+
+ref<AllowListInputAccessor> AllowListInputAccessor::create(
+    ref<InputAccessor> next,
+    std::set<CanonPath> && allowedPaths,
+    MakeNotAllowedError && makeNotAllowedError)
+{
+    return make_ref<AllowListInputAccessorImpl>(next, std::move(allowedPaths), std::move(makeNotAllowedError));
+}
+
+}

src/libfetchers/filtering-input-accessor.hh

@@ -0,0 +1,73 @@
+#pragma once
+
+#include "input-accessor.hh"
+
+namespace nix {
+
+/**
+ * A function that should throw an exception of type
+ * `RestrictedPathError` explaining that access to `path` is
+ * forbidden.
+ */
+typedef std::function<RestrictedPathError(const CanonPath & path)> MakeNotAllowedError;
+
+/**
+ * An abstract wrapping `InputAccessor` that performs access
+ * control. Subclasses should override `checkAccess()` to implement an
+ * access control policy.
+ */
+struct FilteringInputAccessor : InputAccessor
+{
+    ref<InputAccessor> next;
+    CanonPath prefix;
+    MakeNotAllowedError makeNotAllowedError;
+
+    FilteringInputAccessor(const SourcePath & src, MakeNotAllowedError && makeNotAllowedError)
+        : next(src.accessor)
+        , prefix(src.path)
+        , makeNotAllowedError(std::move(makeNotAllowedError))
+    { }
+
+    std::string readFile(const CanonPath & path) override;
+
+    bool pathExists(const CanonPath & path) override;
+
+    std::optional<Stat> maybeLstat(const CanonPath & path) override;
+
+    DirEntries readDirectory(const CanonPath & path) override;
+
+    std::string readLink(const CanonPath & path) override;
+
+    std::string showPath(const CanonPath & path) override;
+
+    /**
+     * Call `makeNotAllowedError` to throw a `RestrictedPathError`
+     * exception if `isAllowed()` returns `false` for `path`.
+     */
+    void checkAccess(const CanonPath & path);
+
+    /**
+     * Return `true` iff access to path is allowed.
+     */
+    virtual bool isAllowed(const CanonPath & path) = 0;
+};
+
+/**
+ * A wrapping `InputAccessor` that checks paths against an allow-list.
+ */
+struct AllowListInputAccessor : public FilteringInputAccessor
+{
+    /**
+     * Grant access to the specified path.
+     */
+    virtual void allowPath(CanonPath path) = 0;
+
+    static ref<AllowListInputAccessor> create(
+        ref<InputAccessor> next,
+        std::set<CanonPath> && allowedPaths,
+        MakeNotAllowedError && makeNotAllowedError);
+
+    using FilteringInputAccessor::FilteringInputAccessor;
+};
+
+}

src/libfetchers/fs-input-accessor.cc

@@ -4,19 +4,12 @@
 
 namespace nix {
 
-struct FSInputAccessorImpl : FSInputAccessor, PosixSourceAccessor
+struct FSInputAccessor : InputAccessor, PosixSourceAccessor
 {
     CanonPath root;
-    std::optional<std::set<CanonPath>> allowedPaths;
-    MakeNotAllowedError makeNotAllowedError;
 
-    FSInputAccessorImpl(
+    FSInputAccessor(const CanonPath & root)
-        const CanonPath & root,
-        std::optional<std::set<CanonPath>> && allowedPaths,
-        MakeNotAllowedError && makeNotAllowedError)
         : root(root)
-        , allowedPaths(std::move(allowedPaths))
-        , makeNotAllowedError(std::move(makeNotAllowedError))
     {
         displayPrefix = root.isRoot() ? "" : root.abs();
     }
@@ -27,39 +20,30 @@ struct FSInputAccessorImpl : FSInputAccessor, PosixSourceAccessor
         std::function<void(uint64_t)> sizeCallback) override
     {
         auto absPath = makeAbsPath(path);
-        checkAllowed(absPath);
         PosixSourceAccessor::readFile(absPath, sink, sizeCallback);
     }
 
     bool pathExists(const CanonPath & path) override
     {
-        auto absPath = makeAbsPath(path);
+        return PosixSourceAccessor::pathExists(makeAbsPath(path));
-        return isAllowed(absPath) && PosixSourceAccessor::pathExists(absPath);
     }
 
     std::optional<Stat> maybeLstat(const CanonPath & path) override
     {
-        auto absPath = makeAbsPath(path);
+        return PosixSourceAccessor::maybeLstat(makeAbsPath(path));
-        checkAllowed(absPath);
-        return PosixSourceAccessor::maybeLstat(absPath);
     }
 
     DirEntries readDirectory(const CanonPath & path) override
     {
-        auto absPath = makeAbsPath(path);
-        checkAllowed(absPath);
         DirEntries res;
-        for (auto & entry : PosixSourceAccessor::readDirectory(absPath))
+        for (auto & entry : PosixSourceAccessor::readDirectory(makeAbsPath(path)))
-            if (isAllowed(absPath + entry.first))
             res.emplace(entry);
         return res;
     }
 
     std::string readLink(const CanonPath & path) override
     {
-        auto absPath = makeAbsPath(path);
+        return PosixSourceAccessor::readLink(makeAbsPath(path));
-        checkAllowed(absPath);
-        return PosixSourceAccessor::readLink(absPath);
     }
 
     CanonPath makeAbsPath(const CanonPath & path)
@@ -67,59 +51,22 @@ struct FSInputAccessorImpl : FSInputAccessor, PosixSourceAccessor
         return root + path;
     }
 
-    void checkAllowed(const CanonPath & absPath) override
-    {
-        if (!isAllowed(absPath))
-            throw makeNotAllowedError
-                ? makeNotAllowedError(absPath)
-                : RestrictedPathError("access to path '%s' is forbidden", absPath);
-    }
-
-    bool isAllowed(const CanonPath & absPath)
-    {
-        if (!absPath.isWithin(root))
-            return false;
-
-        if (allowedPaths) {
-            auto p = absPath.removePrefix(root);
-            if (!p.isAllowed(*allowedPaths))
-                return false;
-        }
-
-        return true;
-    }
-
-    void allowPath(CanonPath path) override
-    {
-        if (allowedPaths)
-            allowedPaths->insert(std::move(path));
-    }
-
-    bool hasAccessControl() override
-    {
-        return (bool) allowedPaths;
-    }
-
     std::optional<CanonPath> getPhysicalPath(const CanonPath & path) override
     {
         return makeAbsPath(path);
     }
 };
 
-ref<FSInputAccessor> makeFSInputAccessor(
+ref<InputAccessor> makeFSInputAccessor(const CanonPath & root)
-    const CanonPath & root,
-    std::optional<std::set<CanonPath>> && allowedPaths,
-    MakeNotAllowedError && makeNotAllowedError)
 {
-    return make_ref<FSInputAccessorImpl>(root, std::move(allowedPaths), std::move(makeNotAllowedError));
+    return make_ref<FSInputAccessor>(root);
 }
 
-ref<FSInputAccessor> makeStorePathAccessor(
+ref<InputAccessor> makeStorePathAccessor(
     ref<Store> store,
-    const StorePath & storePath,
+    const StorePath & storePath)
-    MakeNotAllowedError && makeNotAllowedError)
 {
-    return makeFSInputAccessor(CanonPath(store->toRealPath(storePath)), {}, std::move(makeNotAllowedError));
+    return makeFSInputAccessor(CanonPath(store->toRealPath(storePath)));
 }
 
 SourcePath getUnfilteredRootPath(CanonPath path)

src/libfetchers/fs-input-accessor.hh

@@ -7,26 +7,12 @@ namespace nix {
 class StorePath;
 class Store;
 
-struct FSInputAccessor : InputAccessor
+ref<InputAccessor> makeFSInputAccessor(
-{
+    const CanonPath & root);
-    virtual void checkAllowed(const CanonPath & absPath) = 0;
 
-    virtual void allowPath(CanonPath path) = 0;
+ref<InputAccessor> makeStorePathAccessor(
-
-    virtual bool hasAccessControl() = 0;
-};
-
-typedef std::function<RestrictedPathError(const CanonPath & path)> MakeNotAllowedError;
-
-ref<FSInputAccessor> makeFSInputAccessor(
-    const CanonPath & root,
-    std::optional<std::set<CanonPath>> && allowedPaths = {},
-    MakeNotAllowedError && makeNotAllowedError = {});
-
-ref<FSInputAccessor> makeStorePathAccessor(
     ref<Store> store,
-    const StorePath & storePath,
+    const StorePath & storePath);
-    MakeNotAllowedError && makeNotAllowedError = {});
 
 SourcePath getUnfilteredRootPath(CanonPath path);
 

src/libfetchers/git.cc

@@ -9,6 +9,7 @@
 #include "processes.hh"
 #include "git.hh"
 #include "fs-input-accessor.hh"
+#include "filtering-input-accessor.hh"
 #include "mounted-input-accessor.hh"
 #include "git-utils.hh"
 #include "logging.hh"
@@ -639,7 +640,10 @@ struct GitInputScheme : InputScheme
                 repoInfo.workdirInfo.files.insert(submodule.path);
 
         ref<InputAccessor> accessor =
-            makeFSInputAccessor(CanonPath(repoInfo.url), repoInfo.workdirInfo.files, makeNotAllowedError(repoInfo.url));
+            AllowListInputAccessor::create(
+                makeFSInputAccessor(CanonPath(repoInfo.url)),
+                std::move(repoInfo.workdirInfo.files),
+                makeNotAllowedError(repoInfo.url));
 
         /* If the repo has submodules, return a mounted input accessor
            consisting of the accessor for the top-level repo and the