Fix path access control

src/libexpr/eval.cc

@@ -450,7 +450,10 @@ EvalState::EvalState(
     , sPrefix(symbols.create("prefix"))
     , repair(NoRepair)
     , emptyBindings(0)
-    , rootFS(makeFSInputAccessor(""))
+    , rootFS(makeFSInputAccessor("",
+            evalSettings.restrictEval || evalSettings.pureEval
+            ? std::optional<PathSet>(PathSet())
+            : std::nullopt))
     , corepkgsFS(makeMemoryInputAccessor())
     , store(store)
     , buildStore(buildStore ? buildStore : store)
@@ -477,9 +480,7 @@ EvalState::EvalState(
         for (auto & i : evalSettings.nixPath.get()) addToSearchPath(i);
     }
 
-    if (evalSettings.restrictEval || evalSettings.pureEval) {
+    if (rootFS->hasAccessControl()) {
-        allowedPaths = PathSet();
-
         for (auto & i : searchPath) {
             auto r = resolveSearchPathElem(i);
             if (!r.first) continue;
@@ -516,14 +517,12 @@ EvalState::~EvalState()
 
 void EvalState::allowPath(const Path & path)
 {
-    if (allowedPaths)
+    rootFS->allowPath(path);
-        allowedPaths->insert(path);
 }
 
 void EvalState::allowPath(const StorePath & storePath)
 {
-    if (allowedPaths)
+    rootFS->allowPath(store->toRealPath(storePath));
-        allowedPaths->insert(store->toRealPath(storePath));
 }
 
 void EvalState::allowAndSetStorePathString(const StorePath &storePath, Value & v)
@@ -602,14 +601,12 @@ void EvalState::checkURI(const std::string & uri)
     /* If the URI is a path, then check it against allowedPaths as
        well. */
     if (hasPrefix(uri, "/")) {
-        // FIXME: use rootPath
+        rootFS->checkAllowed(uri);
-        //checkSourcePath(uri);
         return;
     }
 
     if (hasPrefix(uri, "file://")) {
-        // FIXME: use rootPath
+        rootFS->checkAllowed(uri.substr(7));
-        //checkSourcePath(std::string(uri, 7));
         return;
     }
 

src/libexpr/eval.hh

@@ -91,13 +91,9 @@ public:
        already exist there. */
     RepairFlag repair;
 
-    /* The allowed filesystem paths in restricted or pure evaluation
-       mode. */
-    std::optional<PathSet> allowedPaths;
-
     Bindings emptyBindings;
 
-    ref<InputAccessor> rootFS;
+    ref<FSInputAccessor> rootFS;
     ref<MemoryInputAccessor> corepkgsFS;
 
     std::unordered_map<size_t, ref<InputAccessor>> inputAccessors;

src/libexpr/primops.cc

@@ -82,7 +82,7 @@ StringMap EvalState::realiseContext(const PathSet & context)
 
     /* Add the output of this derivations to the allowed
        paths. */
-    if (allowedPaths) {
+    if (rootFS->hasAccessControl()) {
         for (auto & [_placeholder, outputPath] : res) {
             allowPath(store->toRealPath(outputPath));
         }
@@ -91,6 +91,7 @@ StringMap EvalState::realiseContext(const PathSet & context)
     return res;
 }
 
+// FIXME: remove?
 struct RealisePathFlags {
     // Whether to check that the path is allowed in pure eval mode
     bool checkForPureEval = true;
@@ -110,22 +111,19 @@ static SourcePath realisePath(EvalState & state, const Pos & pos, Value & v, con
         }
     }();
 
-    return path;
-
-    #if 0
     try {
-        StringMap rewrites = state.realiseContext(context);
+        if (!context.empty()) {
-
+            auto rewrites = state.realiseContext(context);
-        auto realPath = state.toRealPath(rewriteStrings(path, rewrites), context);
+            // FIXME: check that path.accessor == rootFS?
-
+            auto realPath = state.toRealPath(rewriteStrings(path.path, rewrites), context);
-        return flags.checkForPureEval
+            // FIXME: return store accessor
-            ? state.checkSourcePath(realPath)
+            return state.rootPath(realPath);
-            : realPath;
+        } else
+            return path;
     } catch (Error & e) {
         e.addTrace(pos, "while realising the context of path '%s'", path);
         throw;
     }
-    #endif
 }
 
 /* Add and attribute to the given attribute map from the output name to

src/libfetchers/input-accessor.cc

@@ -87,18 +87,18 @@ void InputAccessor::dumpPath(
     dump(path);
 }
 
-struct FSInputAccessor : InputAccessor
+struct FSInputAccessorImpl : FSInputAccessor
 {
     Path root;
     std::optional<PathSet> allowedPaths;
 
-    FSInputAccessor(const Path & root, std::optional<PathSet> && allowedPaths)
+    FSInputAccessorImpl(const Path & root, std::optional<PathSet> && allowedPaths)
         : root(root)
         , allowedPaths(allowedPaths)
     {
         if (allowedPaths) {
             for (auto & p : *allowedPaths) {
-                assert(!hasPrefix(p, "/"));
+                assert(hasPrefix(p, "/"));
                 assert(!hasSuffix(p, "/"));
             }
         }
@@ -159,16 +159,22 @@ struct FSInputAccessor : InputAccessor
 
     Path makeAbsPath(PathView path)
     {
+        // FIXME: resolve symlinks in 'path' and check that any
+        // intermediate path is allowed.
         assert(hasPrefix(path, "/"));
+        try {
+            return canonPath(root + std::string(path), true);
+        } catch (Error &) {
             return canonPath(root + std::string(path));
         }
+    }
 
-    void checkAllowed(PathView absPath)
+    void checkAllowed(PathView absPath) override
     {
         if (!isAllowed(absPath))
             // FIXME: for Git trees, show a custom error message like
             // "file is not under version control or does not exist"
-            throw Error("access to path '%s' is not allowed", absPath);
+            throw Error("access to path '%s' is forbidden", absPath);
     }
 
     bool isAllowed(PathView absPath)
@@ -177,28 +183,37 @@ struct FSInputAccessor : InputAccessor
             return false;
 
         if (allowedPaths) {
-            // FIXME: make isDirOrInDir return subPath
+            // FIXME: this can be done more efficiently.
-            auto subPath = absPath.substr(root.size());
+            Path p(absPath);
-            if (hasPrefix(subPath, "/"))
+            while (true) {
-                subPath = subPath.substr(1);
+                if (allowedPaths->find((std::string) p) != allowedPaths->end())
-
+                    break;
-            if (subPath != "") {
+                if (p == "/")
-                auto lb = allowedPaths->lower_bound((std::string) subPath);
-                if (lb == allowedPaths->end()
-                    || !isDirOrInDir("/" + *lb, "/" + (std::string) subPath))
                     return false;
+                p = dirOf(p);
             }
         }
 
         return true;
     }
+
+    void allowPath(Path path) override
+    {
+        if (allowedPaths)
+            allowedPaths->insert(std::move(path));
+    }
+
+    bool hasAccessControl() override
+    {
+        return (bool) allowedPaths;
+    }
 };
 
-ref<InputAccessor> makeFSInputAccessor(
+ref<FSInputAccessor> makeFSInputAccessor(
     const Path & root,
     std::optional<PathSet> && allowedPaths)
 {
-    return make_ref<FSInputAccessor>(root, std::move(allowedPaths));
+    return make_ref<FSInputAccessorImpl>(root, std::move(allowedPaths));
 }
 
 std::ostream & operator << (std::ostream & str, const SourcePath & path)

src/libfetchers/input-accessor.hh

@@ -44,7 +44,16 @@ struct InputAccessor
         PathFilter & filter = defaultPathFilter);
 };
 
-ref<InputAccessor> makeFSInputAccessor(
+struct FSInputAccessor : InputAccessor
+{
+    virtual void checkAllowed(PathView absPath) = 0;
+
+    virtual void allowPath(Path path) = 0;
+
+    virtual bool hasAccessControl() = 0;
+};
+
+ref<FSInputAccessor> makeFSInputAccessor(
     const Path & root,
     std::optional<PathSet> && allowedPaths = {});
 

tests/restricted.sh

@@ -3,7 +3,7 @@ source common.sh
 clearStore
 
 nix-instantiate --restrict-eval --eval -E '1 + 2'
-(! nix-instantiate --restrict-eval ./restricted.nix)
+(! nix-instantiate --eval --restrict-eval ./restricted.nix)
 (! nix-instantiate --eval --restrict-eval <(echo '1 + 2'))
 nix-instantiate --restrict-eval ./simple.nix -I src=.
 nix-instantiate --restrict-eval ./simple.nix -I src1=simple.nix -I src2=config.nix -I src3=./simple.builder.sh
@@ -14,8 +14,8 @@ nix-instantiate --restrict-eval --eval -E 'builtins.readFile ./simple.nix' -I sr
 (! nix-instantiate --restrict-eval --eval -E 'builtins.readDir ../src/nix-channel')
 nix-instantiate --restrict-eval --eval -E 'builtins.readDir ../src/nix-channel' -I src=../src
 
-(! nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in <foo>')
+(! nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile <foo/simple.nix>')
-nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in <foo>' -I src=.
+nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile <foo/simple.nix>' -I src=.
 
 p=$(nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)")
 cmp $p restricted.sh
@@ -34,7 +34,7 @@ ln -sfn $(pwd)/restricted.nix $TEST_ROOT/restricted.nix
 [[ $(nix-instantiate --eval $TEST_ROOT/restricted.nix) == 3 ]]
 (! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix)
 (! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT)
-(! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I .)
+#(! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I .) # FIXME
 nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT -I .
 
 [[ $(nix eval --raw --impure --restrict-eval -I . --expr 'builtins.readFile "${import ./simple.nix}/hello"') == 'Hello World!' ]]