diff --git a/doc/manual/rl-next/port-in-store-uris.md b/doc/manual/rl-next/port-in-store-uris.md new file mode 100644 index 000000000..8291c0fd1 --- /dev/null +++ b/doc/manual/rl-next/port-in-store-uris.md @@ -0,0 +1,13 @@ +--- +synopsis: "Add support for user@address:port syntax in store URIs" +prs: [3425] +issues: [7044] +--- + +It's now possible to specify the port used for the SSH stores directly in the store URL in accordance with [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986). Previously the only way to specify custom ports was via `ssh_config` or `NIX_SSHOPTS` environment variable, because Nix incorrectly passed the port number together with the host name to the SSH executable. This has now been fixed. + +This change affects [store references](@docroot@/store/types/index.md#store-url-format) passed via the `--store` and similar flags in CLI as well as in the configuration for [remote builders](@docroot@/command-ref/conf-file.md#conf-builders). For example, the following store URIs now work: + +- `ssh://127.0.0.1:2222` +- `ssh://[b573:6a48:e224:840b:6007:6275:f8f7:ebf3]:22` +- `ssh-ng://[b573:6a48:e224:840b:6007:6275:f8f7:ebf3]:22` diff --git a/src/libfetchers/git-lfs-fetch.cc b/src/libfetchers/git-lfs-fetch.cc index 97f10f0c6..1337c5b83 100644 --- a/src/libfetchers/git-lfs-fetch.cc +++ b/src/libfetchers/git-lfs-fetch.cc @@ -44,16 +44,19 @@ static void downloadToSink( static std::string getLfsApiToken(const ParsedURL & url) { + assert(url.authority.has_value()); + + // FIXME: Not entirely correct. auto [status, output] = runProgram( RunOptions{ .program = "ssh", - .args = {*url.authority, "git-lfs-authenticate", url.path, "download"}, + .args = {url.authority->to_string(), "git-lfs-authenticate", url.path, "download"}, }); if (output.empty()) throw Error( "git-lfs-authenticate: no output (cmd: ssh %s git-lfs-authenticate %s download)", - url.authority.value_or(""), + url.authority.value_or(ParsedURL::Authority{}).to_string(), url.path); auto queryResp = nlohmann::json::parse(output); diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 9f8344edf..e5635ee75 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -15,7 +15,7 @@ struct PathInputScheme : InputScheme if (url.scheme != "path") return {}; - if (url.authority && *url.authority != "") + if (url.authority && url.authority->host.size()) throw Error("path URL '%s' should not have an authority ('%s')", url, *url.authority); Input input{settings}; diff --git a/src/libflake/flakeref.cc b/src/libflake/flakeref.cc index 9a75a2259..5b1c3e8b2 100644 --- a/src/libflake/flakeref.cc +++ b/src/libflake/flakeref.cc @@ -142,7 +142,7 @@ std::pair parsePathFlakeRefWithFragment( if (pathExists(flakeRoot + "/.git")) { auto parsedURL = ParsedURL{ .scheme = "git+file", - .authority = "", + .authority = ParsedURL::Authority{}, .path = flakeRoot, .query = query, .fragment = fragment, @@ -172,7 +172,7 @@ std::pair parsePathFlakeRefWithFragment( return fromParsedURL( fetchSettings, - {.scheme = "path", .authority = "", .path = path, .query = query, .fragment = fragment}, + {.scheme = "path", .authority = ParsedURL::Authority{}, .path = path, .query = query, .fragment = fragment}, isFlake); } @@ -192,7 +192,7 @@ parseFlakeIdRef(const fetchers::Settings & fetchSettings, const std::string & ur if (std::regex_match(url, match, flakeRegex)) { auto parsedURL = ParsedURL{ .scheme = "flake", - .authority = "", + .authority = ParsedURL::Authority{}, .path = match[1], }; diff --git a/src/libstore-tests/machines.cc b/src/libstore-tests/machines.cc index 72562e6fc..e4186372d 100644 --- a/src/libstore-tests/machines.cc +++ b/src/libstore-tests/machines.cc @@ -39,6 +39,14 @@ TEST(machines, getMachinesUriOnly) EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, SizeIs(0))); } +TEST(machines, getMachinesUriWithPort) +{ + auto actual = Machine::parseConfig({"TEST_ARCH-TEST_OS"}, "nix@scratchy.labs.cs.uu.nl:2222"); + ASSERT_THAT(actual, SizeIs(1)); + EXPECT_THAT( + actual[0], Field(&Machine::storeUri, Eq(StoreReference::parse("ssh://nix@scratchy.labs.cs.uu.nl:2222")))); +} + TEST(machines, getMachinesDefaults) { auto actual = Machine::parseConfig({"TEST_ARCH-TEST_OS"}, "nix@scratchy.labs.cs.uu.nl - - - - - - -"); diff --git a/src/libstore/common-ssh-store-config.cc b/src/libstore/common-ssh-store-config.cc index 0e3a126ec..12f187b4c 100644 --- a/src/libstore/common-ssh-store-config.cc +++ b/src/libstore/common-ssh-store-config.cc @@ -5,33 +5,22 @@ namespace nix { -static std::string extractConnStr(std::string_view scheme, std::string_view _connStr) +CommonSSHStoreConfig::CommonSSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) + : CommonSSHStoreConfig(scheme, ParsedURL::Authority::parse(authority), params) { - if (_connStr.empty()) - throw UsageError("`%s` store requires a valid SSH host as the authority part in Store URI", scheme); - - std::string connStr{_connStr}; - - std::smatch result; - static std::regex v6AddrRegex("^((.*)@)?\\[(.*)\\]$"); - - if (std::regex_match(connStr, result, v6AddrRegex)) { - connStr = result[1].matched ? result.str(1) + result.str(3) : result.str(3); - } - - return connStr; } -CommonSSHStoreConfig::CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params) +CommonSSHStoreConfig::CommonSSHStoreConfig( + std::string_view scheme, const ParsedURL::Authority & authority, const Params & params) : StoreConfig(params) - , host(extractConnStr(scheme, host)) + , authority(authority) { } SSHMaster CommonSSHStoreConfig::createSSHMaster(bool useMaster, Descriptor logFD) const { return { - host, + authority, sshKey.get(), sshPublicHostKey.get(), useMaster, diff --git a/src/libstore/include/nix/store/common-ssh-store-config.hh b/src/libstore/include/nix/store/common-ssh-store-config.hh index 9e6a24b74..bbd81835d 100644 --- a/src/libstore/include/nix/store/common-ssh-store-config.hh +++ b/src/libstore/include/nix/store/common-ssh-store-config.hh @@ -2,6 +2,7 @@ ///@file #include "nix/store/store-api.hh" +#include "nix/util/url.hh" namespace nix { @@ -11,7 +12,8 @@ struct CommonSSHStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params); + CommonSSHStoreConfig(std::string_view scheme, const ParsedURL::Authority & authority, const Params & params); + CommonSSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params); const Setting sshKey{ this, "", "ssh-key", "Path to the SSH private key used to authenticate to the remote machine."}; @@ -32,23 +34,9 @@ struct CommonSSHStoreConfig : virtual StoreConfig )"}; /** - * The `parseURL` function supports both IPv6 URIs as defined in - * RFC2732, but also pure addresses. The latter one is needed here to - * connect to a remote store via SSH (it's possible to do e.g. `ssh root@::1`). - * - * When initialized, the following adjustments are made: - * - * - If the URL looks like `root@[::1]` (which is allowed by the URL parser and probably - * needed to pass further flags), it - * will be transformed into `root@::1` for SSH (same for `[::1]` -> `::1`). - * - * - If the URL looks like `root@::1` it will be left as-is. - * - * - In any other case, the string will be left as-is. - * - * Will throw an error if `connStr` is empty too. + * Authority representing the SSH host to connect to. */ - std::string host; + ParsedURL::Authority authority; /** * Small wrapper around `SSHMaster::SSHMaster` that gets most diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 310aca80d..0014a6638 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -291,7 +291,7 @@ public: Only the first element is required. To leave a field at its default, set it to `-`. - 1. The URI of the remote store in the format `ssh://[username@]hostname`. + 1. The URI of the remote store in the format `ssh://[username@]hostname[:port]`. > **Example** > diff --git a/src/libstore/include/nix/store/ssh.hh b/src/libstore/include/nix/store/ssh.hh index 998312ddf..6eb38acef 100644 --- a/src/libstore/include/nix/store/ssh.hh +++ b/src/libstore/include/nix/store/ssh.hh @@ -2,6 +2,7 @@ ///@file #include "nix/util/sync.hh" +#include "nix/util/url.hh" #include "nix/util/processes.hh" #include "nix/util/file-system.hh" @@ -11,7 +12,8 @@ class SSHMaster { private: - const std::string host; + ParsedURL::Authority authority; + std::string hostnameAndUser; bool fakeSSH; const std::string keyFile; /** @@ -43,7 +45,7 @@ private: public: SSHMaster( - std::string_view host, + const ParsedURL::Authority & authority, std::string_view keyFile, std::string_view sshPublicHostKey, bool useMaster, diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 09bea1ca3..075702f93 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -18,7 +18,7 @@ namespace nix { LegacySSHStoreConfig::LegacySSHStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) : StoreConfig(params) - , CommonSSHStoreConfig(scheme, authority, params) + , CommonSSHStoreConfig(scheme, ParsedURL::Authority::parse(authority), params) { } @@ -71,7 +71,7 @@ ref LegacySSHStore::openConnection() TeeSource tee(conn->from, saved); try { conn->remoteVersion = - ServeProto::BasicClientConnection::handshake(conn->to, tee, SERVE_PROTOCOL_VERSION, config->host); + ServeProto::BasicClientConnection::handshake(conn->to, tee, SERVE_PROTOCOL_VERSION, config->authority.host); } catch (SerialisationError & e) { // in.close(): Don't let the remote block on us not writing. conn->sshConn->in.close(); @@ -79,9 +79,10 @@ ref LegacySSHStore::openConnection() NullSink nullSink; tee.drainInto(nullSink); } - throw Error("'nix-store --serve' protocol mismatch from '%s', got '%s'", config->host, chomp(saved.s)); + throw Error( + "'nix-store --serve' protocol mismatch from '%s', got '%s'", config->authority.host, chomp(saved.s)); } catch (EndOfFile & e) { - throw Error("cannot connect to '%1%'", config->host); + throw Error("cannot connect to '%1%'", config->authority.host); } return conn; @@ -89,7 +90,7 @@ ref LegacySSHStore::openConnection() std::string LegacySSHStore::getUri() { - return *Config::uriSchemes().begin() + "://" + config->host; + return *Config::uriSchemes().begin() + "://" + config->authority.to_string(); } std::map LegacySSHStore::queryPathInfosUncached(const StorePathSet & paths) @@ -99,7 +100,10 @@ std::map LegacySSHStore::queryPathInfosUncached /* No longer support missing NAR hash */ assert(GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4); - debug("querying remote host '%s' for info on '%s'", config->host, concatStringsSep(", ", printStorePathSet(paths))); + debug( + "querying remote host '%s' for info on '%s'", + config->authority.host, + concatStringsSep(", ", printStorePathSet(paths))); auto infos = conn->queryPathInfos(*this, paths); @@ -136,7 +140,7 @@ void LegacySSHStore::queryPathInfoUncached( void LegacySSHStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) { - debug("adding path '%s' to remote host '%s'", printStorePath(info.path), config->host); + debug("adding path '%s' to remote host '%s'", printStorePath(info.path), config->authority.host); auto conn(connections->get()); @@ -157,7 +161,8 @@ void LegacySSHStore::addToStore(const ValidPathInfo & info, Source & source, Rep conn->to.flush(); if (readInt(conn->from) != 1) - throw Error("failed to add path '%s' to remote host '%s'", printStorePath(info.path), config->host); + throw Error( + "failed to add path '%s' to remote host '%s'", printStorePath(info.path), config->authority.host); } else { diff --git a/src/libstore/legacy-ssh-store.md b/src/libstore/legacy-ssh-store.md index 043acebd6..c33fc8992 100644 --- a/src/libstore/legacy-ssh-store.md +++ b/src/libstore/legacy-ssh-store.md @@ -1,6 +1,6 @@ R"( -**Store URL format**: `ssh://[username@]hostname` +**Store URL format**: `ssh://[username@]hostname[:port]` This store type allows limited access to a remote store on another machine via SSH. diff --git a/src/libstore/ssh-store.md b/src/libstore/ssh-store.md index 881537e71..26e0d6e39 100644 --- a/src/libstore/ssh-store.md +++ b/src/libstore/ssh-store.md @@ -1,6 +1,6 @@ R"( -**Store URL format**: `ssh-ng://[username@]hostname` +**Store URL format**: `ssh-ng://[username@]hostname[:port]` Experimental store type that allows full access to a Nix store on a remote machine. diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 474b3622a..8ed72c643 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -18,24 +18,62 @@ static std::string parsePublicHostKey(std::string_view host, std::string_view ss } } +class InvalidSSHAuthority : public Error +{ +public: + InvalidSSHAuthority(const ParsedURL::Authority & authority, std::string_view reason) + : Error("invalid SSH authority: '%s': %s", authority.to_string(), reason) + { + } +}; + +/** + * Checks if the hostname/username are valid for use with ssh. + * + * @todo Enforce this better. Probably this needs to reimplement the same logic as in + * https://github.com/openssh/openssh-portable/blob/6ebd472c391a73574abe02771712d407c48e130d/ssh.c#L648-L681 + */ +static void checkValidAuthority(const ParsedURL::Authority & authority) +{ + if (const auto & user = authority.user) { + if (user->empty()) + throw InvalidSSHAuthority(authority, "user name must not be empty"); + if (user->starts_with("-")) + throw InvalidSSHAuthority(authority, fmt("user name '%s' must not start with '-'", *user)); + } + + { + std::string_view host = authority.host; + if (host.empty()) + throw InvalidSSHAuthority(authority, "host name must not be empty"); + if (host.starts_with("-")) + throw InvalidSSHAuthority(authority, fmt("host name '%s' must not start with '-'", host)); + } +} + SSHMaster::SSHMaster( - std::string_view host, + const ParsedURL::Authority & authority, std::string_view keyFile, std::string_view sshPublicHostKey, bool useMaster, bool compress, Descriptor logFD) - : host(host) - , fakeSSH(host == "localhost") + : authority(authority) + , hostnameAndUser([authority]() { + std::ostringstream oss; + if (authority.user) + oss << *authority.user << "@"; + oss << authority.host; + return std::move(oss).str(); + }()) + , fakeSSH(authority.host == "localhost") , keyFile(keyFile) - , sshPublicHostKey(parsePublicHostKey(host, sshPublicHostKey)) + , sshPublicHostKey(parsePublicHostKey(authority.host, sshPublicHostKey)) , useMaster(useMaster && !fakeSSH) , compress(compress) , logFD(logFD) { - if (host == "" || hasPrefix(host, "-")) - throw Error("invalid SSH host name '%s'", host); - + checkValidAuthority(authority); auto state(state_.lock()); state->tmpDir = std::make_unique(createTempDir("", "nix", 0700)); } @@ -59,14 +97,15 @@ void SSHMaster::addCommonSSHOpts(Strings & args) args.insert(args.end(), {"-i", keyFile}); if (!sshPublicHostKey.empty()) { std::filesystem::path fileName = state->tmpDir->path() / "host-key"; - auto p = host.rfind("@"); - std::string thost = p != std::string::npos ? std::string(host, p + 1) : host; - writeFile(fileName.string(), thost + " " + sshPublicHostKey + "\n"); + writeFile(fileName.string(), authority.host + " " + sshPublicHostKey + "\n"); args.insert(args.end(), {"-oUserKnownHostsFile=" + fileName.string()}); } if (compress) args.push_back("-C"); + if (authority.port) + args.push_back(fmt("-p%d", *authority.port)); + // We use this to make ssh signal back to us that the connection is established. // It really does run locally; see createSSHEnv which sets up SHELL to make // it launch more reliably. The local command runs synchronously, so presumably @@ -77,7 +116,7 @@ void SSHMaster::addCommonSSHOpts(Strings & args) bool SSHMaster::isMasterRunning() { - Strings args = {"-O", "check", host}; + Strings args = {"-O", "check", hostnameAndUser}; addCommonSSHOpts(args); auto res = runProgram(RunOptions{.program = "ssh", .args = args, .mergeStderrToStdout = true}); @@ -142,7 +181,7 @@ std::unique_ptr SSHMaster::startCommand(Strings && comman Strings args; if (!fakeSSH) { - args = {"ssh", host.c_str(), "-x"}; + args = {"ssh", hostnameAndUser.c_str(), "-x"}; addCommonSSHOpts(args); if (socketPath != "") args.insert(args.end(), {"-S", socketPath}); @@ -175,7 +214,7 @@ std::unique_ptr SSHMaster::startCommand(Strings && comman if (reply != "started") { printTalkative("SSH stdout first line: %s", reply); - throw Error("failed to start SSH connection to '%s'", host); + throw Error("failed to start SSH connection to '%s'", authority.host); } } @@ -220,7 +259,7 @@ Path SSHMaster::startMaster() if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) throw SysError("duping over stdout"); - Strings args = {"ssh", host.c_str(), "-M", "-N", "-S", state->socketPath}; + Strings args = {"ssh", hostnameAndUser.c_str(), "-M", "-N", "-S", state->socketPath}; if (verbosity >= lvlChatty) args.push_back("-v"); addCommonSSHOpts(args); @@ -241,7 +280,7 @@ Path SSHMaster::startMaster() if (reply != "started") { printTalkative("SSH master stdout first line: %s", reply); - throw Error("failed to start SSH master connection to '%s'", host); + throw Error("failed to start SSH master connection to '%s'", authority.host); } return state->socketPath; diff --git a/src/libstore/store-reference.cc b/src/libstore/store-reference.cc index 99edefeba..13feeae3e 100644 --- a/src/libstore/store-reference.cc +++ b/src/libstore/store-reference.cc @@ -48,7 +48,7 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen auto parsedUri = parseURL(uri); params.insert(parsedUri.query.begin(), parsedUri.query.end()); - auto baseURI = parsedUri.authority.value_or("") + parsedUri.path; + auto baseURI = parsedUri.authority.value_or(ParsedURL::Authority{}).to_string() + parsedUri.path; return { .variant = diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 5e9b81f46..fb27689de 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -7,20 +7,8 @@ namespace nix { /* ----------- tests for url.hh --------------------------------------------------*/ -std::string print_map(StringMap m) -{ - StringMap::iterator it; - std::string s = "{ "; - for (it = m.begin(); it != m.end(); ++it) { - s += "{ "; - s += it->first; - s += " = "; - s += it->second; - s += " } "; - } - s += "}"; - return s; -} +using Authority = ParsedURL::Authority; +using HostType = Authority::HostType; TEST(parseURL, parsesSimpleHttpUrl) { @@ -29,13 +17,14 @@ TEST(parseURL, parsesSimpleHttpUrl) ParsedURL expected{ .scheme = "http", - .authority = "www.example.org", + .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"}, .path = "/file.tar.gz", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsesSimpleHttpsUrl) @@ -45,13 +34,14 @@ TEST(parseURL, parsesSimpleHttpsUrl) ParsedURL expected{ .scheme = "https", - .authority = "www.example.org", + .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"}, .path = "/file.tar.gz", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsesSimpleHttpUrlWithQueryAndFragment) @@ -61,13 +51,14 @@ TEST(parseURL, parsesSimpleHttpUrlWithQueryAndFragment) ParsedURL expected{ .scheme = "https", - .authority = "www.example.org", + .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"}, .path = "/file.tar.gz", .query = (StringMap) {{"download", "fast"}, {"when", "now"}}, .fragment = "hello", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsesSimpleHttpUrlWithComplexFragment) @@ -77,7 +68,7 @@ TEST(parseURL, parsesSimpleHttpUrlWithComplexFragment) ParsedURL expected{ .scheme = "http", - .authority = "www.example.org", + .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"}, .path = "/file.tar.gz", .query = (StringMap) {{"field", "value"}}, .fragment = "?foo=bar#", @@ -93,13 +84,14 @@ TEST(parseURL, parsesFilePlusHttpsUrl) ParsedURL expected{ .scheme = "file+https", - .authority = "www.example.org", + .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"}, .path = "/video.mp4", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, rejectsAuthorityInUrlsWithFileTransportation) @@ -115,13 +107,14 @@ TEST(parseURL, parseIPv4Address) ParsedURL expected{ .scheme = "http", - .authority = "127.0.0.1:8080", + .authority = Authority{.hostType = HostType::IPv4, .host = "127.0.0.1", .port = 8080}, .path = "/file.tar.gz", .query = (StringMap) {{"download", "fast"}, {"when", "now"}}, .fragment = "hello", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parseScopedRFC6874IPv6Address) @@ -131,13 +124,14 @@ TEST(parseURL, parseScopedRFC6874IPv6Address) ParsedURL expected{ .scheme = "http", - .authority = "[fe80::818c:da4d:8975:415c\%enp0s25]:8080", + .authority = Authority{.hostType = HostType::IPv6, .host = "fe80::818c:da4d:8975:415c\%enp0s25", .port = 8080}, .path = "", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parseIPv6Address) @@ -147,13 +141,19 @@ TEST(parseURL, parseIPv6Address) ParsedURL expected{ .scheme = "http", - .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .authority = + Authority{ + .hostType = HostType::IPv6, + .host = "2a02:8071:8192:c100:311d:192d:81ac:11ea", + .port = 8080, + }, .path = "", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parseEmptyQueryParams) @@ -170,13 +170,21 @@ TEST(parseURL, parseUserPassword) ParsedURL expected{ .scheme = "http", - .authority = "user:pass@www.example.org:8080", + .authority = + Authority{ + .hostType = HostType::Name, + .host = "www.example.org", + .user = "user", + .password = "pass", + .port = 8080, + }, .path = "/file.tar.gz", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parseFileURLWithQueryAndFragment) @@ -186,13 +194,14 @@ TEST(parseURL, parseFileURLWithQueryAndFragment) ParsedURL expected{ .scheme = "file", - .authority = "", + .authority = Authority{}, .path = "/none/of//your/business", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsedUrlsIsEqualToItself) @@ -210,25 +219,28 @@ TEST(parseURL, parseFTPUrl) ParsedURL expected{ .scheme = "ftp", - .authority = "ftp.nixos.org", + .authority = Authority{.hostType = HostType::Name, .host = "ftp.nixos.org"}, .path = "/downloads/nixos.iso", .query = (StringMap) {}, .fragment = "", }; ASSERT_EQ(parsed, expected); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsesAnythingInUriFormat) { auto s = "whatever://github.com/NixOS/nixpkgs.git"; auto parsed = parseURL(s); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, parsesAnythingInUriFormatWithoutDoubleSlash) { auto s = "whatever:github.com/NixOS/nixpkgs.git"; auto parsed = parseURL(s); + ASSERT_EQ(s, parsed.to_string()); } TEST(parseURL, emptyStringIsInvalidURL) diff --git a/src/libutil/include/nix/util/url.hh b/src/libutil/include/nix/util/url.hh index 1c51ab797..0a6194b19 100644 --- a/src/libutil/include/nix/util/url.hh +++ b/src/libutil/include/nix/util/url.hh @@ -5,10 +5,76 @@ namespace nix { +/** + * Represents a parsed RFC3986 URL. + * + * @note All fields are already percent decoded. + */ struct ParsedURL { + /** + * Parsed representation of a URL authority. + * + * It consists of user information, hostname and an optional port number. + * Note that passwords in the userinfo are not yet supported and are ignored. + * + * @todo Maybe support passwords in userinfo part of the url for auth. + */ + struct Authority + { + enum class HostType { + Name, //< Registered name (can be empty) + IPv4, + IPv6, + IPvFuture + }; + + static Authority parse(std::string_view encodedAuthority); + bool operator==(const Authority & other) const = default; + std::string to_string() const; + friend std::ostream & operator<<(std::ostream & os, const Authority & self); + + /** + * Type of the host subcomponent, as specified by rfc3986 3.2.2. Host. + */ + HostType hostType = HostType::Name; + + /** + * Host subcomponent. Either a registered name or IPv{4,6,Future} literal addresses. + * + * IPv6 enclosing brackets are already stripped. Percent encoded characters + * in the hostname are decoded. + */ + std::string host; + + /** Percent-decoded user part of the userinfo. */ + std::optional user; + + /** + * Password subcomponent of the authority (if specified). + * + * @warning As per the rfc3986, the password syntax is deprecated, + * but it's necessary to make the parse -> to_string roundtrip. + * We don't use it anywhere (at least intentionally). + * @todo Warn about unused password subcomponent. + */ + std::optional password; + + /** Port subcomponent (if specified). Default value is determined by the scheme. */ + std::optional port; + }; + std::string scheme; - std::optional authority; + /** + * Optional parsed authority component of the URL. + * + * IMPORTANT: An empty authority (i.e. one with an empty host string) and + * a missing authority (std::nullopt) are drastically different cases. This + * is especially important for "file:///path/to/file" URLs defined by RFC8089. + * The presence of the authority is indicated by `//` following the : + * part of the URL. + */ + std::optional authority; std::string path; StringMap query; std::string fragment; diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 2f9c7736a..134d313ed 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -34,6 +34,81 @@ static std::string percentEncodeSpaces(std::string_view url) return replaceStrings(std::string(url), " ", percentEncode(" ")); } +ParsedURL::Authority ParsedURL::Authority::parse(std::string_view encodedAuthority) +{ + auto parsed = boost::urls::parse_authority(encodedAuthority); + if (!parsed) + throw BadURL("invalid URL authority: '%s': %s", encodedAuthority, parsed.error().message()); + + auto hostType = [&]() { + switch (parsed->host_type()) { + case boost::urls::host_type::ipv4: + return HostType::IPv4; + case boost::urls::host_type::ipv6: + return HostType::IPv6; + case boost::urls::host_type::ipvfuture: + return HostType::IPvFuture; + case boost::urls::host_type::none: + case boost::urls::host_type::name: + return HostType::Name; + } + unreachable(); + }(); + + auto port = [&]() -> std::optional { + if (!parsed->has_port()) + return std::nullopt; + /* If the port number is non-zero and representable. */ + if (auto portNumber = parsed->port_number()) + return portNumber; + throw BadURL("port '%s' is invalid", parsed->port()); + }(); + + return { + .hostType = hostType, + .host = parsed->host_address(), + .user = parsed->has_userinfo() ? parsed->user() : std::optional{}, + .password = parsed->has_password() ? parsed->password() : std::optional{}, + .port = port, + }; +} + +std::ostream & operator<<(std::ostream & os, const ParsedURL::Authority & self) +{ + if (self.user) { + os << percentEncode(*self.user); + if (self.password) + os << ":" << percentEncode(*self.password); + os << "@"; + } + + using HostType = ParsedURL::Authority::HostType; + switch (self.hostType) { + case HostType::Name: + os << percentEncode(self.host); + break; + case HostType::IPv4: + os << self.host; + break; + case HostType::IPv6: + case HostType::IPvFuture: + /* Reencode percent sign for RFC4007 ScopeId literals. */ + os << "[" << percentEncode(self.host, ":") << "]"; + } + + if (self.port) + os << ":" << *self.port; + + return os; +} + +std::string ParsedURL::Authority::to_string() const +{ + std::ostringstream oss; + oss << *this; + return std::move(oss).str(); +} + ParsedURL parseURL(const std::string & url) try { /* Drop the shevron suffix used for the flakerefs. Shevron character is reserved and @@ -47,14 +122,21 @@ try { throw BadURL("'%s' doesn't have a scheme", url); auto scheme = urlView.scheme(); - auto authority = [&]() -> std::optional { + auto authority = [&]() -> std::optional { if (urlView.has_authority()) - return percentDecode(urlView.authority().buffer()); + return ParsedURL::Authority::parse(urlView.authority().buffer()); return std::nullopt; }(); + /* 3.2.2. Host (RFC3986): + * If the URI scheme defines a default for host, then that default + * applies when the host subcomponent is undefined or when the + * registered name is empty (zero length). For example, the "file" URI + * scheme is defined so that no authority, an empty host, and + * "localhost" all mean the end-user's machine, whereas the "http" + * scheme considers a missing authority or empty host invalid. */ auto transportIsFile = parseUrlScheme(scheme).transport == "file"; - if (authority && *authority != "" && transportIsFile) + if (authority && authority->host.size() && transportIsFile) throw BadURL("file:// URL '%s' has unexpected authority '%s'", url, *authority); auto path = urlView.path(); /* Does pct-decoding */ @@ -135,7 +217,7 @@ std::string encodeQuery(const StringMap & ss) std::string ParsedURL::to_string() const { - return scheme + ":" + (authority ? "//" + *authority : "") + percentEncode(path, allowedInPath) + return scheme + ":" + (authority ? "//" + authority->to_string() : "") + percentEncode(path, allowedInPath) + (query.empty() ? "" : "?" + encodeQuery(query)) + (fragment.empty() ? "" : "#" + percentEncode(fragment)); } @@ -177,7 +259,7 @@ std::string fixGitURL(const std::string & url) if (hasPrefix(url, "file:")) return url; if (url.find("://") == std::string::npos) { - return (ParsedURL{.scheme = "file", .authority = "", .path = url}).to_string(); + return (ParsedURL{.scheme = "file", .authority = ParsedURL::Authority{}, .path = url}).to_string(); } return url; } diff --git a/tests/nixos/remote-builds.nix b/tests/nixos/remote-builds.nix index fbfff9a7d..3bfb651bd 100644 --- a/tests/nixos/remote-builds.nix +++ b/tests/nixos/remote-builds.nix @@ -18,6 +18,9 @@ let services.openssh.enable = true; virtualisation.writableStore = true; nix.settings.sandbox = true; + services.openssh.ports = [ + 22 + ] ++ lib.optional supportsCustomPort 2222; # Regression test for use of PID namespaces when /proc has # filesystems mounted on top of it @@ -42,6 +45,7 @@ let supportsBadShell = lib.versionAtLeast config.nodes.client.nix.package.version "2.25pre"; + supportsCustomPort = lib.versionAtLeast config.nodes.client.nix.package.version "2.31.0pre20250806"; in { @@ -74,7 +78,7 @@ in nix.distributedBuilds = true; nix.buildMachines = [ { - hostName = "builder1"; + hostName = "builder1" + (lib.optionalString supportsCustomPort ":2222"); sshUser = "root"; sshKey = "/root/.ssh/id_ed25519"; system = "i686-linux";