From 4295c0db5fe29bf4afb4e520d5e4ad71de811c99 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 11:17:22 +0200 Subject: [PATCH 1/6] hostname-util: add new helper split_user_at_host() Let's introduce a common helper for splitting user@host specifications like we use them for --machine=. --- src/basic/hostname-util.c | 33 +++++++++++++++++++++++++++++++++ src/basic/hostname-util.h | 2 ++ src/test/test-hostname-util.c | 27 +++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 673e1de3d42..1372dec0b2f 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -172,3 +172,36 @@ int get_pretty_hostname(char **ret) { *ret = TAKE_PTR(n); return 0; } + +int split_user_at_host(const char *s, char **ret_user, char **ret_host) { + _cleanup_free_ char *u = NULL, *h = NULL; + + /* 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. */ + + const char *rhs = strchr(s, '@'); + if (rhs) { + if (ret_user && rhs > s) { + u = strndup(s, rhs - s); + if (!u) + return -ENOMEM; + } + + if (ret_host && rhs[1] != 0) { + h = strdup(rhs + 1); + if (!h) + return -ENOMEM; + } + } else if (!isempty(s) && ret_host) { + h = strdup(s); + if (!h) + return -ENOMEM; + } + + if (ret_user) + *ret_user = TAKE_PTR(u); + if (ret_host) + *ret_host = TAKE_PTR(h); + + return !!rhs; /* return > 0 if '@' was specified, 0 otherwise */ +} diff --git a/src/basic/hostname-util.h b/src/basic/hostname-util.h index d85391e59fc..4abdfdd76a1 100644 --- a/src/basic/hostname-util.h +++ b/src/basic/hostname-util.h @@ -38,3 +38,5 @@ static inline bool is_dns_proxy_stub_hostname(const char *hostname) { } int get_pretty_hostname(char **ret); + +int split_user_at_host(const char *s, char **ret_user, char **ret_host); diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c index f9f24e0e03c..4abac5ca3a9 100644 --- a/src/test/test-hostname-util.c +++ b/src/test/test-hostname-util.c @@ -89,4 +89,31 @@ TEST(hostname_cleanup) { ASSERT_STREQ(hostname_cleanup(s), "xxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); } +static void test_split_user_at_host_one(const char *s, const char *expected_user, const char *expected_host, int ret) { + _cleanup_free_ char *u = NULL, *h = NULL; + + ASSERT_OK_EQ(split_user_at_host(s, &u, &h), ret); + ASSERT_STREQ(u, expected_user); + ASSERT_STREQ(h, expected_host); + + u = mfree(u); + h = mfree(h); + + ASSERT_OK_EQ(split_user_at_host(s, &u, NULL), ret); + ASSERT_STREQ(u, expected_user); + + ASSERT_OK_EQ(split_user_at_host(s, NULL, &h), ret); + ASSERT_STREQ(h, expected_host); +} + +TEST(split_user_at_host) { + test_split_user_at_host_one("", NULL, NULL, 0); + 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); + test_split_user_at_host_one("@b", NULL, "b", 1); + test_split_user_at_host_one("a@", "a", NULL, 1); + test_split_user_at_host_one("aa@@@bb", "aa", "@@bb", 1); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 020d6c1dcee94826b16788125e092be1bcefb2c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 10:58:54 +0200 Subject: [PATCH 2/6] machined: open up OpenMachinePTY() for unpriv clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The method call already does a PK check, it was just forgotten to allowlist this in the dbus policy. And in the dbus vtable for OpenMachinePTY() call. (It was allowlisted in the per-machine vtable…) Anyway, clean this up. --- man/org.freedesktop.machine1.xml | 1 - src/machine/machined-dbus.c | 2 +- src/machine/org.freedesktop.machine1.conf | 8 ++++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/man/org.freedesktop.machine1.xml b/man/org.freedesktop.machine1.xml index 936f2ad7f27..e2ec4a11475 100644 --- a/man/org.freedesktop.machine1.xml +++ b/man/org.freedesktop.machine1.xml @@ -94,7 +94,6 @@ node /org/freedesktop/machine1 { out s ssh_private_key_path); GetMachineOSRelease(in s name, out a{ss} fields); - @org.freedesktop.systemd1.Privileged("true") OpenMachinePTY(in s name, out h pty, out s pty_path); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 616bea54f82..954f20e4557 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -948,7 +948,7 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_ARGS("s", name), SD_BUS_RESULT("h", pty, "s", pty_path), method_open_machine_pty, - 0), + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("OpenMachineLogin", SD_BUS_ARGS("s", name), SD_BUS_RESULT("h", pty, "s", pty_path), diff --git a/src/machine/org.freedesktop.machine1.conf b/src/machine/org.freedesktop.machine1.conf index bafc1affdb2..c3c8149f9ab 100644 --- a/src/machine/org.freedesktop.machine1.conf +++ b/src/machine/org.freedesktop.machine1.conf @@ -72,6 +72,10 @@ send_interface="org.freedesktop.machine1.Manager" send_member="OpenMachineLogin"/> + + @@ -176,6 +180,10 @@ send_interface="org.freedesktop.machine1.Machine" send_member="OpenLogin"/> + + From 994a42a0967e2f7624462cbb895e71a418bc277b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Jun 2025 18:28:35 +0200 Subject: [PATCH 3/6] run: chop off username from --machine= argument before calling OpenMachinePTY() Let's be compatible with sd-bus' logic to talk to machine, and support the usual user@host syntax. We only want the host part, hence chop if off before passing it to OpenMachinePTY(). Fixes: #32997 --- src/run/run.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 5a9476f13f6..27e09cd2db6 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -36,6 +36,7 @@ #include "format-table.h" #include "format-util.h" #include "fs-util.h" +#include "hostname-util.h" #include "log.h" #include "main-func.h" #include "osc-context.h" @@ -2358,12 +2359,20 @@ static int start_transient_service(sd_bus *bus) { (void) sd_bus_set_allow_interactive_authorization(system_bus, arg_ask_password); - r = bus_call_method(system_bus, - bus_machine_mgr, - "OpenMachinePTY", - &error, - &pty_reply, - "s", arg_host); + /* Chop off a username prefix. We allow this for sd-bus machine connections, hence + * support that here too. */ + _cleanup_free_ char *h = NULL; + r = split_user_at_host(arg_host, /* ret_user= */ NULL, &h); + if (r < 0) + return log_error_errno(r, "Failed to split host specification '%s': %m", arg_host); + + r = bus_call_method( + system_bus, + bus_machine_mgr, + "OpenMachinePTY", + &error, + &pty_reply, + "s", h ?: ".host"); if (r < 0) return log_error_errno(r, "Failed to get machine PTY: %s", bus_error_message(&error, r)); From 137a8ce14d9a93a2949e4b1fdfe8e41e44b8f9a3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 11:27:53 +0200 Subject: [PATCH 4/6] journalctl: politely refuse if non-root usernames are specified for --machine= We currently cannot support that (supporting that would probably require some active component in the machine, or alternatively idmapped mounts or so), hence politely refuse it. See: https://github.com/systemd/systemd/issues/32997#issuecomment-2127700945 --- src/journal/journalctl-util.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/journal/journalctl-util.c b/src/journal/journalctl-util.c index dd6026e9f27..586f702bb4d 100644 --- a/src/journal/journalctl-util.c +++ b/src/journal/journalctl-util.c @@ -5,6 +5,7 @@ #include "alloc-util.h" #include "glob-util.h" +#include "hostname-util.h" #include "id128-util.h" #include "journal-util.h" #include "journalctl.h" @@ -43,9 +44,18 @@ int acquire_journal(sd_journal **ret) { r = sd_journal_open_files_fd(&j, (int[]) { STDIN_FILENO }, 1, arg_journal_additional_open_flags); else if (arg_file) r = sd_journal_open_files(&j, (const char**) arg_file, arg_journal_additional_open_flags); - else if (arg_machine) - r = journal_open_machine(&j, arg_machine, arg_journal_additional_open_flags); - else + else if (arg_machine) { + _cleanup_free_ char *u = NULL, *h = NULL; + + r = split_user_at_host(arg_machine, &u, &h); + if (r < 0) + return log_error_errno(r, "Failed to split machine specification '%s': %m", arg_machine); + + if (!isempty(u) && !streq(u, "root")) + return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Connecting to a machine as non-root is not supported."); + + r = journal_open_machine(&j, h ?: ".host", arg_journal_additional_open_flags); + } else r = sd_journal_open_namespace( &j, arg_namespace, From 2ae32e9d8fc95010ee4b52b3118ea9fbf05d96d6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 11:52:47 +0200 Subject: [PATCH 5/6] sd-bus: port to split_user_at_host() --- src/libsystemd/sd-bus/sd-bus.c | 57 +++++++++++++++++----------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 07bf7003a2e..e8c208cb82e 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1561,14 +1561,18 @@ _public_ int sd_bus_open_system_remote(sd_bus **ret, const char *host) { int bus_set_address_machine(sd_bus *b, RuntimeScope runtime_scope, const char *machine) { _cleanup_free_ char *a = NULL; - const char *rhs; assert(b); assert(machine); - rhs = strchr(machine, '@'); - if (rhs || runtime_scope == RUNTIME_SCOPE_USER) { - _cleanup_free_ char *u = NULL, *eu = NULL, *erhs = NULL; + _cleanup_free_ char *u = NULL, *h = NULL; + int with_at; + with_at = split_user_at_host(machine, &u, &h); + if (with_at < 0) + return with_at; + + if (with_at || runtime_scope == RUNTIME_SCOPE_USER) { + _cleanup_free_ char *eu = NULL, *eh = NULL; /* If there's an "@" in the container specification, we'll connect as a user specified at its * left hand side, which is useful in combination with user=true. This isn't as trivial as it @@ -1578,43 +1582,38 @@ int bus_set_address_machine(sd_bus *b, RuntimeScope runtime_scope, const char *m * into the container and acquire a PAM session there, and then invoke systemd-stdio-bridge * in it, which propagates the bus transport to us. */ - if (rhs) { - if (rhs > machine) - u = strndup(machine, rhs - machine); - else + if (with_at) { + if (!u) { u = getusername_malloc(); /* Empty user name, let's use the local one */ - if (!u) - return -ENOMEM; + if (!u) + return -ENOMEM; + } eu = bus_address_escape(u); if (!eu) return -ENOMEM; - - rhs++; - } else { - /* No "@" specified but we shall connect to the user instance? Then assume root (and - * not a user named identically to the calling one). This means: - * - * --machine=foobar --user → connect to user bus of root user in container "foobar" - * --machine=@foobar --user → connect to user bus of user named like the calling user in container "foobar" - * - * Why? so that behaviour for "--machine=foobar --system" is roughly similar to - * "--machine=foobar --user": both times we unconditionally connect as root user - * regardless what the calling user is. */ - - rhs = machine; } - if (!isempty(rhs)) { - erhs = bus_address_escape(rhs); - if (!erhs) + /* No "@" specified but we shall connect to the user instance? Then assume root (and + * not a user named identically to the calling one). This means: + * + * --machine=foobar --user → connect to user bus of root user in container "foobar" + * --machine=@foobar --user → connect to user bus of user named like the calling user in container "foobar" + * + * Why? so that behaviour for "--machine=foobar --system" is roughly similar to + * "--machine=foobar --user": both times we unconditionally connect as root user + * regardless what the calling user is. */ + + if (h) { + eh = bus_address_escape(h); + if (!eh) return -ENOMEM; } /* systemd-run -M… -PGq --wait -pUser=… -pPAMName=login systemd-stdio-bridge */ a = strjoin("unixexec:path=systemd-run," - "argv1=-M", erhs ?: ".host", "," + "argv1=-M", eh ?: ".host", "," "argv2=-PGq," "argv3=--wait," "argv4=-pUser%3d", eu ?: "root", ",", @@ -1637,7 +1636,7 @@ int bus_set_address_machine(sd_bus *b, RuntimeScope runtime_scope, const char *m /* Just a container name, we can go the simple way, and just join the container, and connect * to the well-known path of the system bus there. */ - e = bus_address_escape(machine); + e = bus_address_escape(h ?: ".host"); if (!e) return -ENOMEM; From 79226d797df012f85dcaf49a839bdaf4de208709 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 11:53:04 +0200 Subject: [PATCH 6/6] sd-bus: treat '@' as equivalent to '@.host' We allow omission of the part before and the part after the @. But so far we didn't allow omitting both. There's no real reason for disallowing that, hence be systematic and allow it. --- src/libsystemd/sd-bus/sd-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index e8c208cb82e..a7fa36bc743 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -1692,7 +1692,7 @@ static int user_and_machine_equivalent(const char *user_and_machine) { /* Omitting the user name means that we shall use the same user name as we run as locally, which * means we'll end up on the same host, let's shortcut */ - if (streq(user_and_machine, "@.host")) + if (STR_IN_SET(user_and_machine, "@.host", "@")) return true; /* Otherwise, if we are root, then we can also allow the ".host" syntax, as that's the user this