diff --git a/doc/manual/rl-next/allowed-uris-can-now-match-whole-schemes.md b/doc/manual/rl-next/allowed-uris-can-now-match-whole-schemes.md new file mode 100644 index 000000000..3cf75a612 --- /dev/null +++ b/doc/manual/rl-next/allowed-uris-can-now-match-whole-schemes.md @@ -0,0 +1,7 @@ +--- +synopsis: Option `allowed-uris` can now match whole schemes in URIs without slashes +prs: 9547 +--- + +If a scheme, such as `github:` is specified in the `allowed-uris` option, all URIs starting with `github:` are allowed. +Previously this only worked for schemes whose URIs used the `://` syntax. diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh index db2971acb..3009a462c 100644 --- a/src/libexpr/eval-settings.hh +++ b/src/libexpr/eval-settings.hh @@ -68,6 +68,11 @@ struct EvalSettings : Config evaluation mode. For example, when set to `https://github.com/NixOS`, builtin functions such as `fetchGit` are allowed to access `https://github.com/NixOS/patchelf.git`. + + Access is granted when + - the URI is equal to the prefix, + - or the URI is a subpath of the prefix, + - or the prefix is a URI scheme ended by a colon `:` and the URI has the same scheme. )"}; Setting traceFunctionCalls{this, false, "trace-function-calls", diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9e494148e..1552e3e92 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -18,6 +18,7 @@ #include "memory-input-accessor.hh" #include "signals.hh" #include "gc-small-vector.hh" +#include "url.hh" #include #include @@ -599,21 +600,45 @@ void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value & mkStorePathString(storePath, v); } +inline static bool isJustSchemePrefix(std::string_view prefix) +{ + return + !prefix.empty() + && prefix[prefix.size() - 1] == ':' + && isValidSchemeName(prefix.substr(0, prefix.size() - 1)); +} + +bool isAllowedURI(std::string_view uri, const Strings & allowedUris) +{ + /* 'uri' should be equal to a prefix, or in a subdirectory of a + prefix. Thus, the prefix https://github.co does not permit + access to https://github.com. */ + for (auto & prefix : allowedUris) { + if (uri == prefix + // Allow access to subdirectories of the prefix. + || (uri.size() > prefix.size() + && prefix.size() > 0 + && hasPrefix(uri, prefix) + && ( + // Allow access to subdirectories of the prefix. + prefix[prefix.size() - 1] == '/' + || uri[prefix.size()] == '/' + + // Allow access to whole schemes + || isJustSchemePrefix(prefix) + ) + )) + return true; + } + + return false; +} + void EvalState::checkURI(const std::string & uri) { if (!evalSettings.restrictEval) return; - /* 'uri' should be equal to a prefix, or in a subdirectory of a - prefix. Thus, the prefix https://github.co does not permit - access to https://github.com. Note: this allows 'http://' and - 'https://' as prefixes for any http/https URI. */ - for (auto & prefix : evalSettings.allowedUris.get()) - if (uri == prefix || - (uri.size() > prefix.size() - && prefix.size() > 0 - && hasPrefix(uri, prefix) - && (prefix[prefix.size() - 1] == '/' || uri[prefix.size()] == '/'))) - return; + if (isAllowedURI(uri, evalSettings.allowedUris.get())) return; /* If the URI is a path, then check it against allowedPaths as well. */ diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index f452dcb9f..24e1befa1 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -832,6 +832,11 @@ std::string showType(const Value & v); */ SourcePath resolveExprPath(SourcePath path); +/** + * Whether a URI is allowed, assuming restrictEval is enabled + */ +bool isAllowedURI(std::string_view uri, const Strings & allowedPaths); + struct InvalidPathError : EvalError { Path path; diff --git a/src/libutil/url-parts.hh b/src/libutil/url-parts.hh index 5c5a30dc2..07bc8d0cd 100644 --- a/src/libutil/url-parts.hh +++ b/src/libutil/url-parts.hh @@ -8,7 +8,7 @@ namespace nix { // URI stuff. const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])"; -const static std::string schemeRegex = "(?:[a-z][a-z0-9+.-]*)"; +const static std::string schemeNameRegex = "(?:[a-z][a-z0-9+.-]*)"; const static std::string ipv6AddressSegmentRegex = "[0-9a-fA-F:]+(?:%\\w+)?"; const static std::string ipv6AddressRegex = "(?:\\[" + ipv6AddressSegmentRegex + "\\]|" + ipv6AddressSegmentRegex + ")"; const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])"; diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 57b64d607..152c06d8e 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -13,7 +13,7 @@ std::regex revRegex(revRegexS, std::regex::ECMAScript); ParsedURL parseURL(const std::string & url) { static std::regex uriRegex( - "((" + schemeRegex + "):" + "((" + schemeNameRegex + "):" + "(?:(?://(" + authorityRegex + ")(" + absPathRegex + "))|(/?" + pathRegex + ")))" + "(?:\\?(" + queryRegex + "))?" + "(?:#(" + queryRegex + "))?", @@ -183,4 +183,12 @@ std::string fixGitURL(const std::string & url) } } +// https://www.rfc-editor.org/rfc/rfc3986#section-3.1 +bool isValidSchemeName(std::string_view s) +{ + static std::regex regex(schemeNameRegex, std::regex::ECMAScript); + + return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default); +} + } diff --git a/src/libutil/url.hh b/src/libutil/url.hh index 833f54678..24806bbff 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -55,4 +55,13 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme); changes absolute paths into file:// URLs. */ std::string fixGitURL(const std::string & url); +/** + * Whether a string is valid as RFC 3986 scheme name. + * Colon `:` is part of the URI; not the scheme name, and therefore rejected. + * See https://www.rfc-editor.org/rfc/rfc3986#section-3.1 + * + * Does not check whether the scheme is understood, as that's context-dependent. + */ +bool isValidSchemeName(std::string_view scheme); + } diff --git a/tests/unit/libexpr/eval.cc b/tests/unit/libexpr/eval.cc new file mode 100644 index 000000000..93d3f658f --- /dev/null +++ b/tests/unit/libexpr/eval.cc @@ -0,0 +1,141 @@ +#include +#include + +#include "eval.hh" +#include "tests/libexpr.hh" + +namespace nix { + +TEST(nix_isAllowedURI, http_example_com) { + Strings allowed; + allowed.push_back("http://example.com"); + + ASSERT_TRUE(isAllowedURI("http://example.com", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed)); + ASSERT_FALSE(isAllowedURI("/", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.co", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.como", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.org", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed)); +} + +TEST(nix_isAllowedURI, http_example_com_foo) { + Strings allowed; + allowed.push_back("http://example.com/foo"); + + ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed)); + ASSERT_FALSE(isAllowedURI("/foo", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.como", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed)); + // Broken? + // ASSERT_TRUE(isAllowedURI("http://example.com/foo?ok=1", allowed)); +} + +TEST(nix_isAllowedURI, http) { + Strings allowed; + allowed.push_back("http://"); + + ASSERT_TRUE(isAllowedURI("http://", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed)); + ASSERT_TRUE(isAllowedURI("http://example.com", allowed)); + ASSERT_FALSE(isAllowedURI("/", allowed)); + ASSERT_FALSE(isAllowedURI("https://", allowed)); + ASSERT_FALSE(isAllowedURI("http:foo", allowed)); +} + +TEST(nix_isAllowedURI, https) { + Strings allowed; + allowed.push_back("https://"); + + ASSERT_TRUE(isAllowedURI("https://example.com", allowed)); + ASSERT_TRUE(isAllowedURI("https://example.com/foo", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com/https:", allowed)); +} + +TEST(nix_isAllowedURI, absolute_path) { + Strings allowed; + allowed.push_back("/var/evil"); // bad idea + + ASSERT_TRUE(isAllowedURI("/var/evil", allowed)); + ASSERT_TRUE(isAllowedURI("/var/evil/", allowed)); + ASSERT_TRUE(isAllowedURI("/var/evil/foo", allowed)); + ASSERT_TRUE(isAllowedURI("/var/evil/foo/", allowed)); + ASSERT_FALSE(isAllowedURI("/", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evi", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed)); +} + +TEST(nix_isAllowedURI, file_url) { + Strings allowed; + allowed.push_back("file:///var/evil"); // bad idea + + ASSERT_TRUE(isAllowedURI("file:///var/evil", allowed)); + ASSERT_TRUE(isAllowedURI("file:///var/evil/", allowed)); + ASSERT_TRUE(isAllowedURI("file:///var/evil/foo", allowed)); + ASSERT_TRUE(isAllowedURI("file:///var/evil/foo/", allowed)); + ASSERT_FALSE(isAllowedURI("/", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evi", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed)); + ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed)); + ASSERT_FALSE(isAllowedURI("http://var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http:///var/evil", allowed)); + ASSERT_FALSE(isAllowedURI("http://var/evil/", allowed)); + ASSERT_FALSE(isAllowedURI("file:///var/evi", allowed)); + ASSERT_FALSE(isAllowedURI("file:///var/evilo", allowed)); + ASSERT_FALSE(isAllowedURI("file:///var/evilo/", allowed)); + ASSERT_FALSE(isAllowedURI("file:///var/evilo/foo", allowed)); + ASSERT_FALSE(isAllowedURI("file:///", allowed)); + ASSERT_FALSE(isAllowedURI("file://", allowed)); +} + +TEST(nix_isAllowedURI, github_all) { + Strings allowed; + allowed.push_back("github:"); + ASSERT_TRUE(isAllowedURI("github:", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed)); + ASSERT_TRUE(isAllowedURI("github://foo/bar", allowed)); + ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed)); + ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed)); + ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed)); + ASSERT_FALSE(isAllowedURI("github", allowed)); +} + +TEST(nix_isAllowedURI, github_org) { + Strings allowed; + allowed.push_back("github:foo"); + ASSERT_FALSE(isAllowedURI("github:", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed)); + ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed)); + ASSERT_FALSE(isAllowedURI("github://foo/bar", allowed)); + ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed)); + ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed)); + ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed)); +} + +TEST(nix_isAllowedURI, non_scheme_colon) { + Strings allowed; + allowed.push_back("https://foo/bar:"); + ASSERT_TRUE(isAllowedURI("https://foo/bar:", allowed)); + ASSERT_TRUE(isAllowedURI("https://foo/bar:/baz", allowed)); + ASSERT_FALSE(isAllowedURI("https://foo/bar:baz", allowed)); +} + +} // namespace nix \ No newline at end of file diff --git a/tests/unit/libutil/url.cc b/tests/unit/libutil/url.cc index a678dad20..7d08f467e 100644 --- a/tests/unit/libutil/url.cc +++ b/tests/unit/libutil/url.cc @@ -344,4 +344,27 @@ namespace nix { ASSERT_EQ(percentDecode(e), s); } +TEST(nix, isValidSchemeName) { + ASSERT_TRUE(isValidSchemeName("http")); + ASSERT_TRUE(isValidSchemeName("https")); + ASSERT_TRUE(isValidSchemeName("file")); + ASSERT_TRUE(isValidSchemeName("file+https")); + ASSERT_TRUE(isValidSchemeName("fi.le")); + ASSERT_TRUE(isValidSchemeName("file-ssh")); + ASSERT_TRUE(isValidSchemeName("file+")); + ASSERT_TRUE(isValidSchemeName("file.")); + ASSERT_TRUE(isValidSchemeName("file1")); + ASSERT_FALSE(isValidSchemeName("file:")); + ASSERT_FALSE(isValidSchemeName("file/")); + ASSERT_FALSE(isValidSchemeName("+file")); + ASSERT_FALSE(isValidSchemeName(".file")); + ASSERT_FALSE(isValidSchemeName("-file")); + ASSERT_FALSE(isValidSchemeName("1file")); + // regex ok? + ASSERT_FALSE(isValidSchemeName("\nhttp")); + ASSERT_FALSE(isValidSchemeName("\nhttp\n")); + ASSERT_FALSE(isValidSchemeName("http\n")); + ASSERT_FALSE(isValidSchemeName("http ")); +} + }