Merge pull request #3425 from mkg20001/pr

Add user@address:port support

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,

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<Path> 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

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**
              >

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,

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::Connection> 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::Connection> 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::Connection> LegacySSHStore::openConnection()
 
 std::string LegacySSHStore::getUri()
 {
-    return *Config::uriSchemes().begin() + "://" + config->host;
+    return *Config::uriSchemes().begin() + "://" + config->authority.to_string();
 }
 
 std::map<StorePath, UnkeyedValidPathInfo> LegacySSHStore::queryPathInfosUncached(const StorePathSet & paths)
@@ -99,7 +100,10 @@ std::map<StorePath, UnkeyedValidPathInfo> 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 {
 

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.

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.

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<AutoDelete>(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::Connection> 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::Connection> 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;

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 =