Merge pull request #8135 from shawnl/arg_host

sd-bus: rework host handling

man/sd_bus_default.xml

@@ -181,7 +181,8 @@
     <citerefentry project='die-net'><refentrytitle>ssh</refentrytitle><manvolnum>1</manvolnum></citerefentry>.
     <parameter>host</parameter> consists of an optional user name followed by the
     <literal>@</literal> symbol, and the hostname, optionally followed by a
-    <literal>:</literal> and a machine name. If the machine name is given, a connection
+    <literal>:</literal> and a port, optionally followed by a
+    <literal>/</literal> and a machine name. If the machine name is given, a connection
     is created to the system bus in the specified container on the remote machine, and
     otherwise a connection to the system bus on the specified host is created.</para>
 

man/user-system-options.xml

@@ -33,12 +33,13 @@
       <para>Execute the operation remotely. Specify a hostname, or a
       username and hostname separated by <literal>@</literal>, to
       connect to. The hostname may optionally be suffixed by a
-      container name, separated by <literal>:</literal>, which
+      port ssh is listening on, seperated by <literal>:</literal>, and then a
+      container name, separated by <literal>/</literal>, which
       connects directly to a specific container on the specified
       host. This will use SSH to talk to the remote machine manager
       instance. Container names may be enumerated with
       <command>machinectl -H
-      <replaceable>HOST</replaceable></command>.</para>
+      <replaceable>HOST</replaceable></command>. Put IPv6 addresses in brackets.</para>
     </listitem>
   </varlistentry>
 

src/libsystemd/sd-bus/sd-bus.c

@@ -1357,38 +1357,90 @@ _public_ int sd_bus_open_user(sd_bus **ret) {
 
 int bus_set_address_system_remote(sd_bus *b, const char *host) {
         _cleanup_free_ char *e = NULL;
-        char *m = NULL, *c = NULL, *a;
+        char *m = NULL, *c = NULL, *a, *rbracket = NULL, *p = NULL;
 
         assert(b);
         assert(host);
 
-        /* Let's see if we shall enter some container */
-        m = strchr(host, ':');
-        if (m) {
-                m++;
+        /* Skip ":"s in ipv6 addresses */
+        if (*host == '[') {
+                char *t;
 
-                /* Let's make sure this is not a port of some kind,
-                 * and is a valid machine name. */
-                if (!in_charset(m, DIGITS) && machine_name_is_valid(m)) {
-                        char *t;
+                rbracket = strchr(host, ']');
+                if (!rbracket)
+                        return -EINVAL;
+                t = strndupa(host + 1, rbracket - host - 1);
+                e = bus_address_escape(t);
+                if (!e)
+                        return -ENOMEM;
+        } else if ((a = strchr(host, '@'))) {
+                if (*(a + 1) == '[') {
+                        _cleanup_free_ char *t = NULL;
 
-                        /* Cut out the host part */
-                        t = strndupa(host, m - host - 1);
+                        rbracket = strchr(a + 1, ']');
+                        if (!rbracket)
+                                return -EINVAL;
+                        t = new0(char, strlen(host));
+                        if (!t)
+                                return -ENOMEM;
+                        strncat(t, host, a - host + 1);
+                        strncat(t, a + 2, rbracket - a - 2);
                         e = bus_address_escape(t);
                         if (!e)
                                 return -ENOMEM;
+                } else if (*(a + 1) == '\0' || strchr(a + 1, '@'))
+                        return -EINVAL;
+        }
 
-                        c = strjoina(",argv5=--machine=", m);
+        /* Let's see if a port was given */
+        m = strchr(rbracket ? rbracket + 1 : host, ':');
+        if (m) {
+                char *t;
+                bool got_forward_slash = false;
+
+                p = m + 1;
+
+                t = strchr(p, '/');
+                if (t) {
+                        p = strndupa(p, t - p);
+                        got_forward_slash = true;
+                }
+
+                if (!in_charset(p, "0123456789") || *p == '\0') {
+                        if (!machine_name_is_valid(p) || got_forward_slash)
+                                return -EINVAL;
+                        else {
+                                m = p;
+                                p = NULL;
+                                goto interpret_port_as_machine_old_syntax;
+                        }
                 }
         }
 
+        /* Let's see if a machine was given */
+        m = strchr(rbracket ? rbracket + 1 : host, '/');
+        if (m) {
+                m++;
+interpret_port_as_machine_old_syntax:
+                /* Let's make sure this is not a port of some kind,
+                 * and is a valid machine name. */
+                if (!in_charset(m, "0123456789") && machine_name_is_valid(m))
+                        c = strjoina(",argv", p ? "7" : "5", "=--machine=", m);
+        }
+
         if (!e) {
-                e = bus_address_escape(host);
+                char *t;
+
+                t = strndupa(host, strcspn(host, ":/"));
+
+                e = bus_address_escape(t);
                 if (!e)
                         return -ENOMEM;
         }
 
-        a = strjoin("unixexec:path=ssh,argv1=-xT,argv2=--,argv3=", e, ",argv4=systemd-stdio-bridge", c);
+        a = strjoin("unixexec:path=ssh,argv1=-xT", p ? ",argv2=-p,argv3=" : "", strempty(p),
+                                ",argv", p ? "4" : "2", "=--,argv", p ? "5" : "3", "=", e,
+                                ",argv", p ? "6" : "4", "=systemd-stdio-bridge", c);
         if (!a)
                 return -ENOMEM;
 

src/libsystemd/sd-bus/test-bus-address.c

@@ -0,0 +1,69 @@
+#include "sd-bus.h"
+
+#include "bus-internal.h"
+#include "log.h"
+#include "string-util.h"
+#include "strv.h"
+
+static void test_one_address(sd_bus *b,
+                             const char *host,
+                             int result, const char *expected) {
+        int r;
+
+        r = bus_set_address_system_remote(b, host);
+        log_info("\"%s\" → %d, \"%s\"", host, r, strna(r >= 0 ? b->address : NULL));
+        if (result < 0 || expected) {
+                assert(r == result);
+                if (r >= 0)
+                        assert_se(streq(b->address, expected));
+        }
+}
+
+static void test_bus_set_address_system_remote(char **args) {
+        _cleanup_(sd_bus_unrefp) sd_bus *b = NULL;
+
+        assert_se(sd_bus_new(&b) >= 0);
+        if (!strv_isempty(args)) {
+                char **a;
+                STRV_FOREACH(a, args)
+                        test_one_address(b, *a, 0, NULL);
+                return;
+        };
+
+        test_one_address(b, "host",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=--,argv3=host,argv4=systemd-stdio-bridge");
+        test_one_address(b, "host:123",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=-p,argv3=123,argv4=--,argv5=host,argv6=systemd-stdio-bridge");
+        test_one_address(b, "host:123:123",
+                         -EINVAL, NULL);
+                test_one_address(b, "host:",
+                                 -EINVAL, NULL);
+        test_one_address(b, "user@host",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=--,argv3=user%40host,argv4=systemd-stdio-bridge");
+         test_one_address(b, "user@host@host",
+                                 -EINVAL, NULL);
+        test_one_address(b, "[::1]",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=--,argv3=%3a%3a1,argv4=systemd-stdio-bridge");
+        test_one_address(b, "user@[::1]",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=--,argv3=user%40%3a%3a1,argv4=systemd-stdio-bridge");
+        test_one_address(b, "user@[::1]:99",
+                         0, "unixexec:path=ssh,argv1=-xT,argv2=-p,argv3=99,argv4=--,argv5=user%40%3a%3a1,argv6=systemd-stdio-bridge");
+        test_one_address(b, "user@[::1]:",
+                         -EINVAL, NULL);
+        test_one_address(b, "user@[::1:",
+                         -EINVAL, NULL);
+        test_one_address(b, "user@",
+                         -EINVAL, NULL);
+        test_one_address(b, "user@@",
+                         -EINVAL, NULL);
+}
+
+int main(int argc, char *argv[]) {
+        log_set_max_level(LOG_INFO);
+        log_parse_environment();
+        log_open();
+
+        test_bus_set_address_system_remote(argv + 1);
+
+        return 0;
+}

src/stdio-bridge/stdio-bridge.c

@@ -10,6 +10,7 @@
 #include "sd-bus.h"
 #include "sd-daemon.h"
 
+#include "alloc-util.h"
 #include "bus-internal.h"
 #include "bus-util.h"
 #include "build.h"
@@ -18,7 +19,8 @@
 
 #define DEFAULT_BUS_PATH "unix:path=/run/dbus/system_bus_socket"
 
-const char *arg_bus_path = DEFAULT_BUS_PATH;
+static const char *arg_bus_path = DEFAULT_BUS_PATH;
+static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
 
 static int help(void) {
 
@@ -26,7 +28,8 @@ static int help(void) {
                "STDIO or socket-activatable proxy to a given DBus endpoint.\n\n"
                "  -h --help              Show this help\n"
                "     --version           Show package version\n"
-               "  -p --bus-path=PATH     Path to the kernel bus (default: %s)\n",
+               "  -p --bus-path=PATH     Path to the kernel bus (default: %s)\n"
+               "  -M --machine=MACHINE   Name of machine to connect to\n",
                program_invocation_short_name, DEFAULT_BUS_PATH);
 
         return 0;
@@ -36,12 +39,14 @@ static int parse_argv(int argc, char *argv[]) {
 
         enum {
                 ARG_VERSION = 0x100,
+                ARG_MACHINE,
         };
 
         static const struct option options[] = {
                 { "help",            no_argument,       NULL, 'h'         },
                 { "version",         no_argument,       NULL, ARG_VERSION },
                 { "bus-path",        required_argument, NULL, 'p'         },
+                { "machine",         required_argument, NULL, 'M'         },
                 {},
         };
 
@@ -66,8 +71,15 @@ static int parse_argv(int argc, char *argv[]) {
 
                 case 'p':
                         arg_bus_path = optarg;
+
                         break;
 
+                case 'M':
+                        arg_bus_path = optarg;
+
+                        arg_transport = BUS_TRANSPORT_MACHINE;
+
+                        break;
                 default:
                         log_error("Unknown option code %c", c);
                         return -EINVAL;
@@ -113,7 +125,10 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
-        r = sd_bus_set_address(a, arg_bus_path);
+        if (arg_transport == BUS_TRANSPORT_MACHINE)
+                r = bus_set_address_system_machine(a, arg_bus_path);
+        else
+                r = sd_bus_set_address(a, arg_bus_path);
         if (r < 0) {
                 log_error_errno(r, "Failed to set address to connect to: %m");
                 goto finish;

src/test/meson.build

@@ -782,6 +782,10 @@ tests += [
 ############################################################
 
 tests += [
+        [['src/libsystemd/sd-bus/test-bus-address.c'],
+         [],
+         [threads]],
+
         [['src/libsystemd/sd-bus/test-bus-marshal.c'],
          [],
          [threads,