diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 6e670efea..6b3c82374 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -260,9 +260,10 @@ void SourceExprCommand::completeInstallable(AddCompletions & completions, std::s evalSettings.pureEval = false; auto state = getEvalState(); - Expr *e = state->parseExprFromFile( - resolveExprPath(state->checkSourcePath(lookupFileArg(*state, *file))) - ); + auto e = + state->parseExprFromFile( + resolveExprPath( + lookupFileArg(*state, *file))); Value root; state->eval(e, root); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7e68e6f9b..23ac349fe 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -509,7 +509,18 @@ EvalState::EvalState( , sOutputSpecified(symbols.create("outputSpecified")) , repair(NoRepair) , emptyBindings(0) - , rootFS(makeFSInputAccessor(CanonPath::root)) + , rootFS( + makeFSInputAccessor( + CanonPath::root, + evalSettings.restrictEval || evalSettings.pureEval + ? std::optional>(std::set()) + : 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); + })) , corepkgsFS(makeMemoryInputAccessor()) , internalFS(makeMemoryInputAccessor()) , derivationInternal{corepkgsFS->addFile( @@ -551,28 +562,10 @@ EvalState::EvalState( searchPath.elements.emplace_back(SearchPath::Elem::parse(i)); } - if (evalSettings.restrictEval || evalSettings.pureEval) { - allowedPaths = PathSet(); - - for (auto & i : searchPath.elements) { - auto r = resolveSearchPathPath(i.path); - if (!r) continue; - - auto path = std::move(*r); - - if (store->isInStore(path)) { - try { - StorePathSet closure; - store->computeFSClosure(store->toStorePath(path).first, closure); - for (auto & path : closure) - allowPath(path); - } catch (InvalidPath &) { - allowPath(path); - } - } else - allowPath(path); - } - } + /* Allow access to all paths in the search path. */ + if (rootFS->hasAccessControl()) + for (auto & i : searchPath.elements) + resolveSearchPathPath(i.path, true); corepkgsFS->addFile( CanonPath("fetchurl.nix"), @@ -590,14 +583,12 @@ EvalState::~EvalState() void EvalState::allowPath(const Path & path) { - if (allowedPaths) - allowedPaths->insert(path); + rootFS->allowPath(CanonPath(path)); } void EvalState::allowPath(const StorePath & storePath) { - if (allowedPaths) - allowedPaths->insert(store->toRealPath(storePath)); + rootFS->allowPath(CanonPath(store->toRealPath(storePath))); } void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value & v) @@ -607,54 +598,6 @@ void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value & mkStorePathString(storePath, v); } -SourcePath EvalState::checkSourcePath(const SourcePath & path_) -{ - // Don't check non-rootFS accessors, they're in a different namespace. - if (path_.accessor != ref(rootFS)) return path_; - - if (!allowedPaths) return path_; - - auto i = resolvedPaths.find(path_.path.abs()); - if (i != resolvedPaths.end()) - return i->second; - - bool found = false; - - /* First canonicalize the path without symlinks, so we make sure an - * attacker can't append ../../... to a path that would be in allowedPaths - * and thus leak symlink targets. - */ - Path abspath = canonPath(path_.path.abs()); - - for (auto & i : *allowedPaths) { - if (isDirOrInDir(abspath, i)) { - found = true; - break; - } - } - - if (!found) { - auto modeInformation = evalSettings.pureEval - ? "in pure eval mode (use '--impure' to override)" - : "in restricted mode"; - throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", abspath, modeInformation); - } - - /* Resolve symlinks. */ - debug("checking access to '%s'", abspath); - SourcePath path = rootPath(CanonPath(canonPath(abspath, true))); - - for (auto & i : *allowedPaths) { - if (isDirOrInDir(path.path.abs(), i)) { - resolvedPaths.insert_or_assign(path_.path.abs(), path); - return path; - } - } - - throw RestrictedPathError("access to canonical path '%1%' is forbidden in restricted mode", path); -} - - void EvalState::checkURI(const std::string & uri) { if (!evalSettings.restrictEval) return; @@ -674,12 +617,12 @@ void EvalState::checkURI(const std::string & uri) /* If the URI is a path, then check it against allowedPaths as well. */ if (hasPrefix(uri, "/")) { - checkSourcePath(rootPath(CanonPath(uri))); + rootFS->checkAllowed(CanonPath(uri)); return; } if (hasPrefix(uri, "file://")) { - checkSourcePath(rootPath(CanonPath(std::string(uri, 7)))); + rootFS->checkAllowed(CanonPath(uri.substr(7))); return; } @@ -1181,10 +1124,8 @@ Value * ExprPath::maybeThunk(EvalState & state, Env & env) } -void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial) +void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial) { - auto path = checkSourcePath(path_); - FileEvalCache::iterator i; if ((i = fileEvalCache.find(path)) != fileEvalCache.end()) { v = i->second; @@ -1205,7 +1146,7 @@ void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial e = j->second; if (!e) - e = parseExprFromFile(checkSourcePath(resolvedPath)); + e = parseExprFromFile(resolvedPath); fileParseCache[resolvedPath] = e; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 9a92992c1..ee7bdda0d 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -217,12 +217,6 @@ public: */ RepairFlag repair; - /** - * The allowed filesystem paths in restricted or pure evaluation - * mode. - */ - std::optional allowedPaths; - Bindings emptyBindings; /** @@ -396,12 +390,6 @@ public: */ void allowAndSetStorePathString(const StorePath & storePath, Value & v); - /** - * Check whether access to a path is allowed and throw an error if - * not. Otherwise return the canonicalised path. - */ - SourcePath checkSourcePath(const SourcePath & path); - void checkURI(const std::string & uri); /** @@ -445,13 +433,15 @@ public: SourcePath findFile(const SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos); /** - * Try to resolve a search path value (not the optional key part) + * Try to resolve a search path value (not the optional key part). * * If the specified search path element is a URI, download it. * * If it is not found, return `std::nullopt` */ - std::optional resolveSearchPathPath(const SearchPath::Path & path); + std::optional resolveSearchPathPath( + const SearchPath::Path & elem, + bool initAccessControl = false); /** * Evaluate an expression to normal form @@ -756,6 +746,13 @@ public: */ [[nodiscard]] StringMap realiseContext(const NixStringContext & context); + /* Call the binary path filter predicate used builtins.path etc. */ + bool callPathFilter( + Value * filterFun, + const SourcePath & path, + std::string_view pathArg, + PosIdx pos); + private: /** diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index f6cf1f689..58fc580fc 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -783,7 +783,7 @@ SourcePath EvalState::findFile(const SearchPath & searchPath, const std::string_ } -std::optional EvalState::resolveSearchPathPath(const SearchPath::Path & value0) +std::optional EvalState::resolveSearchPathPath(const SearchPath::Path & value0, bool initAccessControl) { auto & value = value0.s; auto i = searchPathResolved.find(value); @@ -800,7 +800,6 @@ std::optional EvalState::resolveSearchPathPath(const SearchPath::Pa logWarning({ .msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value) }); - res = std::nullopt; } } @@ -814,6 +813,20 @@ std::optional EvalState::resolveSearchPathPath(const SearchPath::Pa else { auto path = absPath(value); + + /* Allow access to paths in the search path. */ + if (initAccessControl) { + allowPath(path); + if (store->isInStore(path)) { + try { + StorePathSet closure; + store->computeFSClosure(store->toStorePath(path).first, closure); + for (auto & p : closure) + allowPath(p); + } catch (InvalidPath &) { } + } + } + if (pathExists(path)) res = { path }; else { @@ -829,7 +842,7 @@ std::optional EvalState::resolveSearchPathPath(const SearchPath::Pa else debug("failed to resolve search path element '%s'", value); - searchPathResolved[value] = res; + searchPathResolved.emplace(value, res); return res; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index ebf2549e4..0f7706563 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -15,6 +15,7 @@ #include "value-to-json.hh" #include "value-to-xml.hh" #include "primops.hh" +#include "fs-input-accessor.hh" #include #include @@ -90,8 +91,8 @@ StringMap EvalState::realiseContext(const NixStringContext & context) for (auto & [outputName, outputPath] : outputs) { /* Add the output of this derivations to the allowed paths. */ - if (allowedPaths) { - allowPath(outputPath); + if (rootFS->hasAccessControl()) { + allowPath(store->toRealPath(outputPath)); } /* Get all the output paths corresponding to the placeholders we had */ if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { @@ -110,27 +111,19 @@ StringMap EvalState::realiseContext(const NixStringContext & context) return res; } -struct RealisePathFlags { - // Whether to check that the path is allowed in pure eval mode - bool checkForPureEval = true; -}; - -static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, const RealisePathFlags flags = {}) +static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v) { NixStringContext context; auto path = state.coerceToPath(noPos, v, context, "while realising the context of a path"); try { - if (!context.empty()) { + if (!context.empty() && path.accessor == state.rootFS) { auto rewrites = state.realiseContext(context); auto realPath = state.toRealPath(rewriteStrings(path.path.abs(), rewrites), context); return {path.accessor, CanonPath(realPath)}; - } - - return flags.checkForPureEval - ? state.checkSourcePath(path) - : path; + } else + return path; } catch (Error & e) { e.addTrace(state.positions[pos], "while realising the context of path '%s'", path); throw; @@ -1493,7 +1486,7 @@ static void prim_storePath(EvalState & state, const PosIdx pos, Value * * args, })); NixStringContext context; - auto path = state.checkSourcePath(state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to 'builtins.storePath'")).path; + auto path = state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to 'builtins.storePath'").path; /* Resolve symlinks in ‘path’, unless ‘path’ itself is a symlink directly in the store. The latter condition is necessary so e.g. nix-push does the right thing. */ @@ -1535,12 +1528,7 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, { auto & arg = *args[0]; - /* We don’t check the path right now, because we don’t want to - throw if the path isn’t allowed, but just return false (and we - can’t just catch the exception here because we still want to - throw if something in the evaluation of `arg` tries to - access an unauthorized path). */ - auto path = realisePath(state, pos, arg, { .checkForPureEval = false }); + auto path = realisePath(state, pos, arg); /* SourcePath doesn't know about trailing slash. */ auto mustBeDir = arg.type() == nString @@ -1548,14 +1536,9 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, || arg.string_view().ends_with("/.")); try { - auto checked = state.checkSourcePath(path); - auto st = checked.maybeLstat(); + auto st = path.maybeLstat(); auto exists = st && (!mustBeDir || st->type == SourceAccessor::tDirectory); v.mkBool(exists); - } catch (SysError & e) { - /* Don't give away info from errors while canonicalising - ‘path’ in restricted mode. */ - v.mkBool(false); } catch (RestrictedPathError & e) { v.mkBool(false); } @@ -1699,7 +1682,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V auto path = state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument passed to builtins.findFile"); - v.mkPath(state.checkSourcePath(state.findFile(searchPath, path, pos))); + v.mkPath(state.findFile(searchPath, path, pos)); } static RegisterPrimOp primop_findFile(PrimOp { @@ -2178,11 +2161,35 @@ static RegisterPrimOp primop_toFile({ .fun = prim_toFile, }); +bool EvalState::callPathFilter( + Value * filterFun, + const SourcePath & path, + std::string_view pathArg, + PosIdx pos) +{ + auto st = path.lstat(); + + /* Call the filter function. The first argument is the path, the + second is a string indicating the type of the file. */ + Value arg1; + arg1.mkString(pathArg); + + Value arg2; + // assert that type is not "unknown" + arg2.mkString(fileTypeToString(st.type)); + + Value * args []{&arg1, &arg2}; + Value res; + callFunction(*filterFun, 2, args, res, pos); + + return forceBool(res, pos, "while evaluating the return value of the path filter function"); +} + static void addPath( EvalState & state, const PosIdx pos, std::string_view name, - Path path, + SourcePath path, Value * filterFun, FileIngestionMethod method, const std::optional expectedHash, @@ -2190,48 +2197,29 @@ static void addPath( const NixStringContext & context) { try { - // FIXME: handle CA derivation outputs (where path needs to - // be rewritten to the actual output). - auto rewrites = state.realiseContext(context); - path = state.toRealPath(rewriteStrings(path, rewrites), context); - StorePathSet refs; - if (state.store->isInStore(path)) { + if (path.accessor == state.rootFS && state.store->isInStore(path.path.abs())) { + // FIXME: handle CA derivation outputs (where path needs to + // be rewritten to the actual output). + auto rewrites = state.realiseContext(context); + path = {state.rootFS, CanonPath(state.toRealPath(rewriteStrings(path.path.abs(), rewrites), context))}; + try { - auto [storePath, subPath] = state.store->toStorePath(path); + auto [storePath, subPath] = state.store->toStorePath(path.path.abs()); // FIXME: we should scanForReferences on the path before adding it refs = state.store->queryPathInfo(storePath)->references; - path = state.store->toRealPath(storePath) + subPath; + path = {state.rootFS, CanonPath(state.store->toRealPath(storePath) + subPath)}; } catch (Error &) { // FIXME: should be InvalidPathError } } - path = evalSettings.pureEval && expectedHash - ? path - : state.checkSourcePath(state.rootPath(CanonPath(path))).path.abs(); - - PathFilter filter = filterFun ? ([&](const Path & path) { - auto st = lstat(path); - - /* Call the filter function. The first argument is the path, - the second is a string indicating the type of the file. */ - Value arg1; - arg1.mkString(path); - - Value arg2; - arg2.mkString( - S_ISREG(st.st_mode) ? "regular" : - S_ISDIR(st.st_mode) ? "directory" : - S_ISLNK(st.st_mode) ? "symlink" : - "unknown" /* not supported, will fail! */); - - Value * args []{&arg1, &arg2}; - Value res; - state.callFunction(*filterFun, 2, args, res, pos); - - return state.forceBool(res, pos, "while evaluating the return value of the path filter function"); - }) : defaultPathFilter; + std::unique_ptr filter; + if (filterFun) + filter = std::make_unique([&](const Path & p) { + auto p2 = CanonPath(p); + return state.callPathFilter(filterFun, {path.accessor, p2}, p2.abs(), pos); + }); std::optional expectedStorePath; if (expectedHash) @@ -2242,7 +2230,7 @@ static void addPath( }); if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { - auto dstPath = state.rootPath(CanonPath(path)).fetchToStore(state.store, name, method, &filter, state.repair); + auto dstPath = path.fetchToStore(state.store, name, method, filter.get(), state.repair); if (expectedHash && expectedStorePath != dstPath) state.debugThrowLastTrace(Error("store path mismatch in (possibly filtered) path added from '%s'", path)); state.allowAndSetStorePathString(dstPath, v); @@ -2261,7 +2249,8 @@ static void prim_filterSource(EvalState & state, const PosIdx pos, Value * * arg auto path = state.coerceToPath(pos, *args[1], context, "while evaluating the second argument (the path to filter) passed to 'builtins.filterSource'"); state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filterSource"); - addPath(state, pos, path.baseName(), path.path.abs(), args[0], FileIngestionMethod::Recursive, std::nullopt, v, context); + + addPath(state, pos, path.baseName(), path, args[0], FileIngestionMethod::Recursive, std::nullopt, v, context); } static RegisterPrimOp primop_filterSource({ @@ -2356,7 +2345,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value if (name.empty()) name = path->baseName(); - addPath(state, pos, name, path->path.abs(), filterFun, method, expectedHash, v, context); + addPath(state, pos, name, *path, filterFun, method, expectedHash, v, context); } static RegisterPrimOp primop_path({ diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 75ce12a8c..e2986bfe0 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -310,8 +310,11 @@ static void main_nix_build(int argc, char * * argv) else /* If we're in a #! script, interpret filenames relative to the script. */ - exprs.push_back(state->parseExprFromFile(resolveExprPath(state->checkSourcePath(lookupFileArg(*state, - inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i))))); + exprs.push_back( + state->parseExprFromFile( + resolveExprPath( + lookupFileArg(*state, + inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i)))); } } diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index c67409e89..86b9be17d 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -183,7 +183,7 @@ static int main_nix_instantiate(int argc, char * * argv) for (auto & i : files) { Expr * e = fromArgs ? state->parseExprFromString(i, state->rootPath(CanonPath::fromCwd())) - : state->parseExprFromFile(resolveExprPath(state->checkSourcePath(lookupFileArg(*state, i)))); + : state->parseExprFromFile(resolveExprPath(lookupFileArg(*state, i))); processExpr(*state, attrPaths, parseOnly, strict, autoArgs, evalOnly, outputKind, xmlOutputSourceLocation, e); } diff --git a/tests/functional/restricted.sh b/tests/functional/restricted.sh index 197ae7a10..b8deceacc 100644 --- a/tests/functional/restricted.sh +++ b/tests/functional/restricted.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 ') -nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in ' -I src=. +(! nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile ') +nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile ' -I src=. p=$(nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)") cmp $p restricted.sh