Merge pull request #9547 from hercules-ci/allowed-scheme-without-slash

`allowed-uris`: match whole schemes without slashes

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.

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<bool> traceFunctionCalls{this, false, "trace-function-calls",

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 <algorithm>
 #include <chrono>
@@ -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
+    if (isAllowedURI(uri, evalSettings.allowedUris.get())) return;
-       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 the URI is a path, then check it against allowedPaths as
        well. */

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;

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-._~])";

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);
+}
+
 }

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);
+
 }

tests/unit/libexpr/eval.cc

@@ -0,0 +1,141 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#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

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 "));
+}
+
 }