From 80edaab12fd3b6987f132996d9eb8613cd100312 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 22 Sep 2025 23:04:58 +0200 Subject: [PATCH] Mount allowed paths on `storeFS` with pure eval No `AllowListSourceAccessor` for pure eval --- not needed anymore! --- src/libexpr/eval.cc | 98 +++++++++++++++++++++------------- tests/functional/restricted.sh | 2 +- 2 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 6cf902e35..190c79212 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -213,35 +213,57 @@ EvalState::EvalState( , settings{settings} , symbols(StaticEvalSymbols::staticSymbolTable()) , repair(NoRepair) - , storeFS(makeMountedSourceAccessor({ - {CanonPath::root, makeEmptySourceAccessor()}, - /* In the pure eval case, we can simply require - valid paths. However, in the *impure* eval - case this gets in the way of the union - mechanism, because an invalid access in the - upper layer will *not* be caught by the union - source accessor, but instead abort the entire - lookup. + , storeFS([&] { + auto accessor = makeMountedSourceAccessor({{CanonPath::root, makeEmptySourceAccessor()}}); - This happens when the store dir in the - ambient file system has a path (e.g. because - another Nix store there), but the relocated - store does not. + /* In the pure eval case, we can simply require + valid paths. However, in the *impure* eval + case this gets in the way of the union + mechanism, because an invalid access in the + upper layer will *not* be caught by the union + source accessor, but instead abort the entire + lookup. - TODO make the various source accessors doing - access control all throw the same type of - exception, and make union source accessor - catch it, so we don't need to do this hack. - */ - {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)}, - })) - , rootFS([&] { - auto accessor = [&]() -> decltype(rootFS) { - /* In pure eval mode, we provide a filesystem that only - contains the Nix store. */ - if (settings.pureEval) - return storeFS; + This happens when the store dir in the + ambient file system has a path (e.g. because + another Nix store there), but the relocated + store does not. + TODO make the various source accessors doing + access control all throw the same type of + exception, and make union source accessor + catch it, so we don't need to do this hack. + */ + if (settings.pureEval) { + /* This is just an overkill way to make sure other store + paths get this error, and not the "doesn't exist" error + that the mounted source accessor would do on its own. */ + accessor->mount( + CanonPath::root, + AllowListSourceAccessor::create( + getFSSourceAccessor(), + {}, + {CanonPath::root, CanonPath(store->storeDir)}, + [&](const CanonPath & path) -> RestrictedPathError { + throw RestrictedPathError( + "access to absolute path '%1%' is forbidden in pure evaluation mode (use '--impure' to override)", + CanonPath(store->storeDir) / path); + })); + /* We don't want to list store paths */ + accessor->mount(CanonPath(store->storeDir), makeEmptySourceAccessor()); + } else { + accessor->mount(CanonPath(store->storeDir), store->getFSAccessor(false)); + } + + return accessor; + }()) + , rootFS([&] -> decltype(rootFS) { + /* In pure eval mode, we provide a filesystem that only + contains the Nix store. */ + if (settings.pureEval) + return storeFS; + + auto makeImpureAccessor = [&]() -> decltype(rootFS) { /* If we have a chroot store and pure eval is not enabled, use a union accessor to make the chroot store available at its logical location while still having the underlying @@ -253,18 +275,16 @@ EvalState::EvalState( return makeUnionSourceAccessor({getFSSourceAccessor(), storeFS}); return getFSSourceAccessor(); - }(); + }; /* Apply access control if needed. */ - if (settings.restrictEval || settings.pureEval) - accessor = AllowListSourceAccessor::create( - accessor, {}, {}, [&settings](const CanonPath & path) -> RestrictedPathError { - auto modeInformation = settings.pureEval ? "in pure evaluation mode (use '--impure' to override)" - : "in restricted mode"; - throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation); + if (settings.restrictEval) + return AllowListSourceAccessor::create( + makeImpureAccessor(), {}, {}, [](const CanonPath & path) -> RestrictedPathError { + throw RestrictedPathError("access to absolute path '%1%' is forbidden in restricted mode", path); }); - return accessor; + return makeImpureAccessor(); }()) , corepkgsFS(make_ref()) , internalFS(make_ref()) @@ -351,13 +371,19 @@ void EvalState::allowPathLegacy(const Path & path) void EvalState::allowPath(const StorePath & storePath) { - if (auto rootFS2 = rootFS.dynamic_pointer_cast()) + if (settings.pureEval) { + storeFS->mount(CanonPath(store->printStorePath(storePath)), ref{store->getFSAccessor(storePath)}); + } + if (settings.restrictEval) { + auto rootFS2 = rootFS.dynamic_pointer_cast(); + assert(rootFS2); rootFS2->allowPrefix(CanonPath(store->printStorePath(storePath))); + } } void EvalState::allowClosure(const StorePath & storePath) { - if (!rootFS.dynamic_pointer_cast()) + if (!settings.pureEval && !settings.restrictEval) return; StorePathSet closure; diff --git a/tests/functional/restricted.sh b/tests/functional/restricted.sh index 00ee4ddc8..ec76ea3cf 100755 --- a/tests/functional/restricted.sh +++ b/tests/functional/restricted.sh @@ -65,7 +65,7 @@ traverseDir="${_NIX_TEST_SOURCE_DIR}/restricted-traverse-me" ln -sfn "${_NIX_TEST_SOURCE_DIR}/restricted-secret" "${_NIX_TEST_SOURCE_DIR}/restricted-innocent" mkdir -p "$traverseDir" goUp="..$(echo "$traverseDir" | sed -e 's,[^/]\+,..,g')" -output="$(nix eval --raw --restrict-eval -I "$traverseDir" \ +output="$(nix eval --raw --impure --restrict-eval -I "$traverseDir" \ --expr "builtins.readFile \"$traverseDir/$goUp${_NIX_TEST_SOURCE_DIR}/restricted-innocent\"" \ 2>&1 || :)" echo "$output" | grep "is forbidden"