hostname-util: introduce machine_spec_valid() and check --machine= value everywhere (#37851)

Replaces #32999

Addresses
https://github.com/systemd/systemd/pull/37741#discussion_r2128482378

src/analyze/analyze.c

@@ -453,8 +453,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_MAN:

src/basic/hostname-util.c

@@ -9,6 +9,7 @@
 #include "os-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "user-util.h"
 
 char* get_default_hostname_raw(void) {
         int r;
@@ -179,6 +180,8 @@ int split_user_at_host(const char *s, char **ret_user, char **ret_host) {
         /* Splits a user@host expression (one of those we accept on --machine= and similar). Returns NULL in
          * each of the two return parameters if that part was left empty. */
 
+        assert(s);
+
         const char *rhs = strchr(s, '@');
         if (rhs) {
                 if (ret_user && rhs > s) {
@@ -192,10 +195,16 @@ int split_user_at_host(const char *s, char **ret_user, char **ret_host) {
                         if (!h)
                                 return -ENOMEM;
                 }
-        } else if (!isempty(s) && ret_host) {
-                h = strdup(s);
-                if (!h)
-                        return -ENOMEM;
+
+        } else {
+                if (isempty(s))
+                        return -EINVAL;
+
+                if (ret_host) {
+                        h = strdup(s);
+                        if (!h)
+                                return -ENOMEM;
+                }
         }
 
         if (ret_user)
@@ -205,3 +214,24 @@ int split_user_at_host(const char *s, char **ret_user, char **ret_host) {
 
         return !!rhs; /* return > 0 if '@' was specified, 0 otherwise */
 }
+
+int machine_spec_valid(const char *s) {
+        _cleanup_free_ char *u = NULL, *h = NULL;
+        int r;
+
+        assert(s);
+
+        r = split_user_at_host(s, &u, &h);
+        if (r == -EINVAL)
+                return false;
+        if (r < 0)
+                return r;
+
+        if (u && !valid_user_group_name(u, VALID_USER_RELAX | VALID_USER_ALLOW_NUMERIC))
+                return false;
+
+        if (h && !hostname_is_valid(h, VALID_HOSTNAME_DOT_HOST))
+                return false;
+
+        return true;
+}

src/basic/hostname-util.h

@@ -39,4 +39,5 @@ static inline bool is_dns_proxy_stub_hostname(const char *hostname) {
 
 int get_pretty_hostname(char **ret);
 
+int machine_spec_valid(const char *s);
 int split_user_at_host(const char *s, char **ret_user, char **ret_host);

src/busctl/busctl.c

@@ -2265,8 +2265,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case 'C':

src/home/homectl.c

@@ -3418,8 +3418,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case 'I':

src/hostname/hostnamectl.c

@@ -33,7 +33,7 @@
 
 static bool arg_ask_password = true;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
-static char *arg_host = NULL;
+static const char *arg_host = NULL;
 static bool arg_transient = false;
 static bool arg_pretty = false;
 static bool arg_static = false;
@@ -791,8 +791,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_TRANSIENT:

src/import/importctl.c

@@ -1111,8 +1111,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_READ_ONLY:

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

@@ -1563,6 +1563,7 @@ int bus_set_address_machine(sd_bus *b, RuntimeScope runtime_scope, const char *m
         _cleanup_free_ char *a = NULL;
 
         assert(b);
+        assert(IN_SET(runtime_scope, RUNTIME_SCOPE_SYSTEM, RUNTIME_SCOPE_USER));
         assert(machine);
 
         _cleanup_free_ char *u = NULL, *h = NULL;
@@ -1648,42 +1649,9 @@ int bus_set_address_machine(sd_bus *b, RuntimeScope runtime_scope, const char *m
         return free_and_replace(b->address, a);
 }
 
-static int user_and_machine_valid(const char *user_and_machine) {
-        const char *h;
-
-        /* Checks if a container specification in the form "user@container" or just "container" is valid.
-         *
-         * If the "@" syntax is used we'll allow either the "user" or the "container" part to be omitted, but
-         * not both. */
-
-        h = strchr(user_and_machine, '@');
-        if (!h)
-                h = user_and_machine;
-        else {
-                _cleanup_free_ char *user = NULL;
-
-                user = strndup(user_and_machine, h - user_and_machine);
-                if (!user)
-                        return -ENOMEM;
-
-                if (!isempty(user) && !valid_user_group_name(user, VALID_USER_RELAX | VALID_USER_ALLOW_NUMERIC))
-                        return false;
-
-                h++;
-
-                if (isempty(h))
-                        return !isempty(user);
-        }
-
-        return hostname_is_valid(h, VALID_HOSTNAME_DOT_HOST);
-}
-
-static int user_and_machine_equivalent(const char *user_and_machine) {
-        _cleanup_free_ char *un = NULL;
-        const char *f;
-
+static int machine_spec_is_current_identity(const char *user_and_machine) {
         /* Returns true if the specified user+machine name are actually equivalent to our own identity and
-         * our own host. If so we can shortcut things.  Why bother? Because that way we don't have to fork
+         * our own host. If so we can shortcut things. Why bother? Because that way we don't have to fork
          * off short-lived worker processes that are then unavailable for authentication and logging in the
          * peer. Moreover joining a namespace requires privileges. If we are in the right namespace anyway,
          * we can avoid permission problems thus. */
@@ -1703,6 +1671,9 @@ static int user_and_machine_equivalent(const char *user_and_machine) {
                 return true;
 
         /* Otherwise, we have to figure out our user id and name, and compare things with that. */
+        _cleanup_free_ char *un = NULL;
+        const char *f;
+
         f = startswith(user_and_machine, FORMAT_UID(uid));
         if (!f) {
                 un = getusername_malloc();
@@ -1724,10 +1695,10 @@ _public_ int sd_bus_open_system_machine(sd_bus **ret, const char *user_and_machi
         assert_return(user_and_machine, -EINVAL);
         assert_return(ret, -EINVAL);
 
-        if (user_and_machine_equivalent(user_and_machine))
+        if (machine_spec_is_current_identity(user_and_machine))
                 return sd_bus_open_system(ret);
 
-        r = user_and_machine_valid(user_and_machine);
+        r = machine_spec_valid(user_and_machine);
         if (r < 0)
                 return r;
         if (r == 0)
@@ -1761,11 +1732,11 @@ _public_ int sd_bus_open_user_machine(sd_bus **ret, const char *user_and_machine
 
         /* Shortcut things if we'd end up on this host and as the same user and have one of the necessary
          * environment variables set already.  */
-        if (user_and_machine_equivalent(user_and_machine) &&
+        if (machine_spec_is_current_identity(user_and_machine) &&
             (secure_getenv("DBUS_SESSION_BUS_ADDRESS") || secure_getenv("XDG_RUNTIME_DIR")))
                 return sd_bus_open_user(ret);
 
-        r = user_and_machine_valid(user_and_machine);
+        r = machine_spec_valid(user_and_machine);
         if (r < 0)
                 return r;
         if (r == 0)

src/locale/localectl.c

@@ -18,6 +18,7 @@
 #include "main-func.h"
 #include "memory-util.h"
 #include "pager.h"
+#include "parse-argument.h"
 #include "polkit-agent.h"
 #include "pretty-print.h"
 #include "runtime-scope.h"
@@ -452,7 +453,7 @@ static int parse_argv(int argc, char *argv[]) {
                 {}
         };
 
-        int c;
+        int r, c;
 
         assert(argc >= 0);
         assert(argv);
@@ -489,8 +490,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case '?':

src/login/loginctl.c

@@ -47,7 +47,7 @@ static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF;
 static const char *arg_kill_whom = NULL;
 static int arg_signal = SIGTERM;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
-static char *arg_host = NULL;
+static const char *arg_host = NULL;
 static bool arg_ask_password = true;
 static unsigned arg_lines = 10;
 static OutputMode arg_output = OUTPUT_SHORT;
@@ -1654,8 +1654,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case '?':

src/mount/mount-tool.c

@@ -279,8 +279,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_DISCOVER:

src/portable/portablectl.c

@@ -21,6 +21,7 @@
 #include "main-func.h"
 #include "os-util.h"
 #include "pager.h"
+#include "parse-argument.h"
 #include "parse-util.h"
 #include "path-util.h"
 #include "polkit-agent.h"
@@ -35,7 +36,7 @@ static bool arg_legend = true;
 static bool arg_ask_password = true;
 static bool arg_quiet = false;
 static const char *arg_profile = "default";
-static const char* arg_copy_mode = NULL;
+static const char *arg_copy_mode = NULL;
 static bool arg_runtime = false;
 static bool arg_reload = true;
 static bool arg_cat = false;
@@ -1318,7 +1319,6 @@ static int help(int argc, char *argv[], void *userdata) {
 }
 
 static int parse_argv(int argc, char *argv[]) {
-        int r;
 
         enum {
                 ARG_VERSION = 0x100,
@@ -1360,15 +1360,12 @@ static int parse_argv(int argc, char *argv[]) {
                 {}
         };
 
+        int r, c;
+
         assert(argc >= 0);
         assert(argv);
 
-        for (;;) {
-                int c;
-
-                c = getopt_long(argc, argv, "hH:M:qp:", options, NULL);
-                if (c < 0)
-                        break;
+        while ((c = getopt_long(argc, argv, "hH:M:qp:", options, NULL)) >= 0)
 
                 switch (c) {
 
@@ -1396,8 +1393,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case 'q':
@@ -1476,7 +1474,6 @@ static int parse_argv(int argc, char *argv[]) {
                 default:
                         assert_not_reached();
                 }
-        }
 
         return 1;
 }

src/run/run.c

@@ -457,8 +457,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_SERVICE_TYPE:
@@ -900,8 +901,9 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) {
                         break;
 
                 case ARG_MACHINE:
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_UNIT:

src/shared/parse-argument.c

@@ -1,7 +1,9 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "alloc-util.h"
+#include "bus-util.h"
 #include "format-table.h"
+#include "hostname-util.h"
 #include "log.h"
 #include "parse-argument.h"
 #include "parse-util.h"
@@ -143,3 +145,23 @@ int parse_signal_argument(const char *s, int *ret) {
         *ret = r;
         return 1; /* work to do */
 }
+
+int parse_machine_argument(const char *s, const char **ret_host, BusTransport *ret_transport) {
+        int r;
+
+        assert(s);
+        assert(ret_host);
+
+        r = machine_spec_valid(s);
+        if (r < 0)
+                return log_error_errno(r, "Failed to validate --machine= argument '%s': %m", s);
+        if (r == 0)
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid --machine= specified: %s", s);
+
+        *ret_host = s;
+
+        if (ret_transport)
+                *ret_transport = BUS_TRANSPORT_MACHINE;
+
+        return 0;
+}

src/shared/parse-argument.h

@@ -8,3 +8,4 @@ int parse_tristate_argument(const char *optname, const char *s, int *ret);
 int parse_json_argument(const char *s, sd_json_format_flags_t *ret);
 int parse_path_argument(const char *path, bool suppress_root, char **arg);
 int parse_signal_argument(const char *s, int *ret);
+int parse_machine_argument(const char *s, const char **ret_host, BusTransport *ret_transport);

src/stdio-bridge/stdio-bridge.c

@@ -14,6 +14,7 @@
 #include "io-util.h"
 #include "log.h"
 #include "main-func.h"
+#include "parse-argument.h"
 #include "time-util.h"
 
 static const char *arg_bus_path = DEFAULT_SYSTEM_BUS_ADDRESS;
@@ -35,9 +36,9 @@ static int help(void) {
 }
 
 static int parse_argv(int argc, char *argv[]) {
+
         enum {
                 ARG_VERSION = 0x100,
-                ARG_MACHINE,
                 ARG_USER,
                 ARG_SYSTEM,
         };
@@ -52,7 +53,7 @@ static int parse_argv(int argc, char *argv[]) {
                 {},
         };
 
-        int c;
+        int r, c;
 
         assert(argc >= 0);
         assert(argv);
@@ -80,8 +81,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_bus_path = optarg;
-                        arg_transport = BUS_TRANSPORT_MACHINE;
+                        r = parse_machine_argument(optarg, &arg_bus_path, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case '?':

src/systemctl/systemctl.c

@@ -788,8 +788,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_RUNTIME:

src/sysupdate/updatectl.c

@@ -36,7 +36,7 @@ static bool arg_reboot = false;
 static bool arg_offline = false;
 static bool arg_now = false;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
-static char *arg_host = NULL;
+static const char *arg_host = NULL;
 
 #define SYSUPDATE_HOST_PATH "/org/freedesktop/sysupdate1/target/host"
 #define SYSUPDATE_TARGET_INTERFACE "org.freedesktop.sysupdate1.Target"

src/test/test-hostname-util.c

@@ -107,7 +107,8 @@ static void test_split_user_at_host_one(const char *s, const char *expected_user
 }
 
 TEST(split_user_at_host) {
-        test_split_user_at_host_one("", NULL, NULL, 0);
+        ASSERT_ERROR(split_user_at_host("", NULL, NULL), EINVAL);
+
         test_split_user_at_host_one("@", NULL, NULL, 1);
         test_split_user_at_host_one("a", NULL, "a", 0);
         test_split_user_at_host_one("a@b", "a", "b", 1);

src/timedate/timedatectl.c

@@ -22,6 +22,7 @@
 #include "log.h"
 #include "main-func.h"
 #include "pager.h"
+#include "parse-argument.h"
 #include "parse-util.h"
 #include "polkit-agent.h"
 #include "pretty-print.h"
@@ -37,7 +38,7 @@
 static PagerFlags arg_pager_flags = 0;
 static bool arg_ask_password = true;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
-static char *arg_host = NULL;
+static const char *arg_host = NULL;
 static bool arg_adjust_system_clock = false;
 static bool arg_monitor = false;
 static char **arg_property = NULL;
@@ -996,8 +997,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case 'M':
-                        arg_transport = BUS_TRANSPORT_MACHINE;
-                        arg_host = optarg;
+                        r = parse_machine_argument(optarg, &arg_host, &arg_transport);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_NO_ASK_PASSWORD: