From 68a3d9153883b90c99ea2aec20075146ce58beaa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 12:04:11 +0100 Subject: [PATCH 01/13] sd-bus: use SOCK_CLOEXEC on one more socket --- src/libsystemd/sd-bus/bus-container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index f09d5e7fd1..b11ebb3f65 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -49,7 +49,7 @@ int bus_container_connect_socket(sd_bus *b) { bus_socket_setup(b); - if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, pair) < 0) + if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, pair) < 0) return -errno; r = namespace_fork("(sd-buscntrns)", "(sd-buscntr)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG, From c4dd2d75756a95302bf9e4c56346e61a61e03b66 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 12:04:21 +0100 Subject: [PATCH 02/13] machine: drop really old kdbus left-over The "x-machine-kernel" dbus address has been removed a long time ago, hence don't generate it either. --- src/machine/machine-dbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index bb67beb665..4646571668 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -487,7 +487,7 @@ static int container_bus_new(Machine *m, sd_bus_error *error, sd_bus **ret) { if (r < 0) return r; - if (asprintf(&address, "x-machine-kernel:pid=%1$" PID_PRI ";x-machine-unix:pid=%1$" PID_PRI, m->leader) < 0) + if (asprintf(&address, "x-machine-unix:pid=%" PID_PRI, m->leader) < 0) return -ENOMEM; bus->address = address; From 1feb8eee2de38549da5b7e54c0dbd09436dc1d06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 16:25:12 +0100 Subject: [PATCH 03/13] logs-show: drop redundant validation of machine name The immediately following container_get_leader() call validate the name anyway, no need to twice exactly the same way twice immediately after each other. --- src/shared/logs-show.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index bf574d32a5..597ec00ffb 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1523,9 +1523,6 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { assert(machine); assert(boot_id); - if (!machine_name_is_valid(machine)) - return -EINVAL; - r = container_get_leader(machine, &pid); if (r < 0) return r; From 9e815cf2c2d8463e58c6609edb6a33f9bb62e7b5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 16:26:04 +0100 Subject: [PATCH 04/13] hostname-util: explain what 'LDH' is --- src/basic/hostname-util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 5c3157fdf2..82cd37b154 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -89,6 +89,8 @@ int gethostname_strict(char **ret) { } bool valid_ldh_char(char c) { + /* "LDH" → "Letters, digits, hyphens", as per RFC 5890, Section 2.3.1 */ + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || From 52ef5dd7989d886945e18ab50de31bf5a21ba158 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 16:40:45 +0100 Subject: [PATCH 05/13] hostname-util: flagsify hostname_is_valid(), drop machine_name_is_valid() Let's clean up hostname_is_valid() a bit: let's turn the second boolean argument into a more explanatory flags field, and add a flag that accepts the special name ".host" as valid. This is useful for the container logic, where the special hostname ".host" refers to the "root container", i.e. the host system itself, and can be specified at various places. let's also get rid of machine_name_is_valid(). It was just an alias, which is confusing and even more so now that we have the flags param. --- src/basic/hostname-util.c | 39 ++++++------- src/basic/hostname-util.h | 10 +++- src/basic/util.c | 2 +- src/core/hostname-setup.c | 2 +- src/firstboot/firstboot.c | 4 +- src/hostname/hostnamectl.c | 2 +- src/hostname/hostnamed.c | 4 +- src/import/export.c | 4 +- src/import/import-fs.c | 2 +- src/import/import-raw.c | 2 +- src/import/import-tar.c | 2 +- src/import/import.c | 4 +- src/import/importd.c | 8 +-- src/import/pull-raw.c | 2 +- src/import/pull-tar.c | 2 +- src/import/pull.c | 4 +- src/journal/sd-journal.c | 2 +- src/libsystemd-network/sd-dhcp-client.c | 2 +- src/libsystemd-network/sd-dhcp6-client.c | 2 +- src/libsystemd/sd-bus/sd-bus.c | 8 +-- src/libsystemd/sd-login/sd-login.c | 6 +- src/machine/machinectl.c | 16 +++--- src/machine/machined-dbus.c | 2 +- src/machine/machined.c | 2 +- src/network/generator/network-generator.c | 2 +- src/network/networkd-network.c | 2 +- src/nspawn/nspawn-oci.c | 2 +- src/nspawn/nspawn-settings.c | 2 +- src/nspawn/nspawn.c | 6 +- src/test/test-hostname-util.c | 68 +++++++++++------------ 30 files changed, 107 insertions(+), 108 deletions(-) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 82cd37b154..e1e3d1b061 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -98,28 +98,24 @@ bool valid_ldh_char(char c) { c == '-'; } -/** - * Check if s looks like a valid hostname or FQDN. This does not do - * full DNS validation, but only checks if the name is composed of - * allowed characters and the length is not above the maximum allowed - * by Linux (c.f. dns_name_is_valid()). Trailing dot is allowed if - * allow_trailing_dot is true and at least two components are present - * in the name. Note that due to the restricted charset and length - * this call is substantially more conservative than - * dns_name_is_valid(). - */ -bool hostname_is_valid(const char *s, bool allow_trailing_dot) { +bool hostname_is_valid(const char *s, ValidHostnameFlags flags) { unsigned n_dots = 0; const char *p; bool dot, hyphen; + /* Check if s looks like a valid hostname or FQDN. This does not do full DNS validation, but only + * checks if the name is composed of allowed characters and the length is not above the maximum + * allowed by Linux (c.f. dns_name_is_valid()). A trailing dot is allowed if + * VALID_HOSTNAME_TRAILING_DOT flag is set and at least two components are present in the name. Note + * that due to the restricted charset and length this call is substantially more conservative than + * dns_name_is_valid(). Doesn't accept empty hostnames, hostnames with leading dots, and hostnames + * with multiple dots in a sequence. Doesn't allow hyphens at the beginning or end of label. */ + if (isempty(s)) return false; - /* Doesn't accept empty hostnames, hostnames with - * leading dots, and hostnames with multiple dots in a - * sequence. Also ensures that the length stays below - * HOST_NAME_MAX. */ + if (streq(s, ".host")) /* Used by the container logic to denote the "root container" */ + return FLAGS_SET(flags, VALID_HOSTNAME_DOT_HOST); for (p = s, dot = hyphen = true; *p; p++) if (*p == '.') { @@ -145,14 +141,13 @@ bool hostname_is_valid(const char *s, bool allow_trailing_dot) { hyphen = false; } - if (dot && (n_dots < 2 || !allow_trailing_dot)) + if (dot && (n_dots < 2 || !FLAGS_SET(flags, VALID_HOSTNAME_TRAILING_DOT))) return false; if (hyphen) return false; - if (p-s > HOST_NAME_MAX) /* Note that HOST_NAME_MAX is 64 on - * Linux, but DNS allows domain names - * up to 255 characters */ + if (p-s > HOST_NAME_MAX) /* Note that HOST_NAME_MAX is 64 on Linux, but DNS allows domain names up to + * 255 characters */ return false; return true; @@ -243,7 +238,7 @@ int shorten_overlong(const char *s, char **ret) { if (!h) return -ENOMEM; - if (hostname_is_valid(h, false)) { + if (hostname_is_valid(h, 0)) { *ret = h; return 0; } @@ -254,7 +249,7 @@ int shorten_overlong(const char *s, char **ret) { strshorten(h, HOST_NAME_MAX); - if (!hostname_is_valid(h, false)) { + if (!hostname_is_valid(h, 0)) { free(h); return -EDOM; } @@ -287,7 +282,7 @@ int read_etc_hostname_stream(FILE *f, char **ret) { hostname_cleanup(p); /* normalize the hostname */ - if (!hostname_is_valid(p, true)) /* check that the hostname we return is valid */ + if (!hostname_is_valid(p, VALID_HOSTNAME_TRAILING_DOT)) /* check that the hostname we return is valid */ return -EBADMSG; copy = strdup(p); diff --git a/src/basic/hostname-util.h b/src/basic/hostname-util.h index 34819c2953..fe417297ee 100644 --- a/src/basic/hostname-util.h +++ b/src/basic/hostname-util.h @@ -14,10 +14,14 @@ char* gethostname_short_malloc(void); int gethostname_strict(char **ret); bool valid_ldh_char(char c) _const_; -bool hostname_is_valid(const char *s, bool allow_trailing_dot) _pure_; -char* hostname_cleanup(char *s); -#define machine_name_is_valid(s) hostname_is_valid(s, false) +typedef enum ValidHostnameFlags { + VALID_HOSTNAME_TRAILING_DOT = 1 << 0, /* Accept trailing dot on multi-label names */ + VALID_HOSTNAME_DOT_HOST = 1 << 1, /* Accept ".host" as valid hostname */ +} ValidHostnameFlags; + +bool hostname_is_valid(const char *s, ValidHostnameFlags flags) _pure_; +char* hostname_cleanup(char *s); bool is_localhost(const char *hostname); diff --git a/src/basic/util.c b/src/basic/util.c index b080ce4e96..7c708eb3be 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -165,7 +165,7 @@ int container_get_leader(const char *machine, pid_t *pid) { return 0; } - if (!machine_name_is_valid(machine)) + if (!hostname_is_valid(machine, 0)) return -EINVAL; p = strjoina("/run/systemd/machines/", machine); diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c index 867ea19905..4e20e764a3 100644 --- a/src/core/hostname-setup.c +++ b/src/core/hostname-setup.c @@ -24,7 +24,7 @@ int hostname_setup(void) { if (r < 0) log_warning_errno(r, "Failed to retrieve system hostname from kernel command line, ignoring: %m"); else if (r > 0) { - if (hostname_is_valid(b, true)) + if (hostname_is_valid(b, VALID_HOSTNAME_TRAILING_DOT)) hn = b; else { log_warning("Hostname specified on kernel command line is invalid, ignoring: %s", b); diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 742b43f9fc..48baae3f89 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -493,7 +493,7 @@ static int prompt_hostname(void) { break; } - if (!hostname_is_valid(h, true)) { + if (!hostname_is_valid(h, VALID_HOSTNAME_TRAILING_DOT)) { log_error("Specified hostname invalid."); continue; } @@ -1135,7 +1135,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_HOSTNAME: - if (!hostname_is_valid(optarg, true)) + if (!hostname_is_valid(optarg, VALID_HOSTNAME_TRAILING_DOT)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Host name %s is not valid.", optarg); diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 0d39e9176f..de6eb9ea2f 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -248,7 +248,7 @@ static int set_hostname(int argc, char **argv, void *userdata) { /* If the passed hostname is already valid, then assume the user doesn't know anything about pretty * hostnames, so let's unset the pretty hostname, and just set the passed hostname as static/dynamic * hostname. */ - if (arg_static && hostname_is_valid(hostname, true)) + if (arg_static && hostname_is_valid(hostname, VALID_HOSTNAME_TRAILING_DOT)) p = ""; /* No pretty hostname (as it is redundant), just a static one */ else p = hostname; /* Use the passed name as pretty hostname */ diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index a1794bdab1..a0dc25c834 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -596,7 +596,7 @@ static int method_set_hostname(sd_bus_message *m, void *userdata, sd_bus_error * if (isempty(name)) name = FALLBACK_HOSTNAME; - if (!hostname_is_valid(name, false)) + if (!hostname_is_valid(name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid hostname '%s'", name); assert_se(uname(&u) >= 0); @@ -650,7 +650,7 @@ static int method_set_static_hostname(sd_bus_message *m, void *userdata, sd_bus_ if (streq_ptr(name, c->data[PROP_STATIC_HOSTNAME])) return sd_bus_reply_method_return(m, NULL); - if (!isempty(name) && !hostname_is_valid(name, false)) + if (!isempty(name) && !hostname_is_valid(name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid static hostname '%s'", name); r = bus_verify_polkit_async( diff --git a/src/import/export.c b/src/import/export.c index 83990df64c..b8507330ac 100644 --- a/src/import/export.c +++ b/src/import/export.c @@ -65,7 +65,7 @@ static int export_tar(int argc, char *argv[], void *userdata) { _cleanup_close_ int open_fd = -1; int r, fd; - if (machine_name_is_valid(argv[1])) { + if (hostname_is_valid(argv[1], 0)) { r = image_find(IMAGE_MACHINE, argv[1], &image); if (r == -ENOENT) return log_error_errno(r, "Machine image %s not found.", argv[1]); @@ -141,7 +141,7 @@ static int export_raw(int argc, char *argv[], void *userdata) { _cleanup_close_ int open_fd = -1; int r, fd; - if (machine_name_is_valid(argv[1])) { + if (hostname_is_valid(argv[1], 0)) { r = image_find(IMAGE_MACHINE, argv[1], &image); if (r == -ENOENT) return log_error_errno(r, "Machine image %s not found.", argv[1]); diff --git a/src/import/import-fs.c b/src/import/import-fs.c index 3b43ea112d..a22eef8255 100644 --- a/src/import/import-fs.c +++ b/src/import/import-fs.c @@ -126,7 +126,7 @@ static int import_fs(int argc, char *argv[], void *userdata) { local = empty_or_dash_to_null(local); if (local) { - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local image name '%s' is not valid.", local); diff --git a/src/import/import-raw.c b/src/import/import-raw.c index 9f5c13ba16..c0869ffcf9 100644 --- a/src/import/import-raw.c +++ b/src/import/import-raw.c @@ -393,7 +393,7 @@ int raw_import_start(RawImport *i, int fd, const char *local, bool force_local, assert(fd >= 0); assert(local); - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return -EINVAL; if (i->input_fd >= 0) diff --git a/src/import/import-tar.c b/src/import/import-tar.c index 9f68d45eac..d68ce91613 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -329,7 +329,7 @@ int tar_import_start(TarImport *i, int fd, const char *local, bool force_local, assert(fd >= 0); assert(local); - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return -EINVAL; if (i->input_fd >= 0) diff --git a/src/import/import.c b/src/import/import.c index eade0f0ec8..9ea8e7f16d 100644 --- a/src/import/import.c +++ b/src/import/import.c @@ -64,7 +64,7 @@ static int import_tar(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local image name '%s' is not valid.", local); @@ -159,7 +159,7 @@ static int import_raw(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local image name '%s' is not valid.", local); diff --git a/src/import/importd.c b/src/import/importd.c index 63f80e0e38..b5cff4aca8 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -717,7 +717,7 @@ static int method_import_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ if (!S_ISREG(st.st_mode) && !S_ISFIFO(st.st_mode)) return -EINVAL; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); @@ -787,7 +787,7 @@ static int method_import_fs(sd_bus_message *msg, void *userdata, sd_bus_error *e if (r < 0) return r; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); @@ -852,7 +852,7 @@ static int method_export_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_ if (r < 0) return r; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); @@ -932,7 +932,7 @@ static int method_pull_tar_or_raw(sd_bus_message *msg, void *userdata, sd_bus_er if (isempty(local)) local = NULL; - else if (!machine_name_is_valid(local)) + else if (!hostname_is_valid(local, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Local name %s is invalid", local); diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 7956ef0395..4a2bab3d07 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -651,7 +651,7 @@ int raw_pull_start( if (!http_url_is_valid(url)) return -EINVAL; - if (local && !machine_name_is_valid(local)) + if (local && !hostname_is_valid(local, 0)) return -EINVAL; if (i->raw_job) diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 72e5b8be27..bbbd0f57a0 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -481,7 +481,7 @@ int tar_pull_start( if (!http_url_is_valid(url)) return -EINVAL; - if (local && !machine_name_is_valid(local)) + if (local && !hostname_is_valid(local, 0)) return -EINVAL; if (i->tar_job) diff --git a/src/import/pull.c b/src/import/pull.c index 9aff377b93..a4cec9448e 100644 --- a/src/import/pull.c +++ b/src/import/pull.c @@ -72,7 +72,7 @@ static int pull_tar(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local image name '%s' is not valid.", local); @@ -158,7 +158,7 @@ static int pull_raw(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local image name '%s' is not valid.", local); diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 946f74d535..e94fb4dafb 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -1981,7 +1981,7 @@ _public_ int sd_journal_open_container(sd_journal **ret, const char *machine, in assert_return(machine, -EINVAL); assert_return(ret, -EINVAL); assert_return((flags & ~OPEN_CONTAINER_ALLOWED_FLAGS) == 0, -EINVAL); - assert_return(machine_name_is_valid(machine), -EINVAL); + assert_return(hostname_is_valid(machine, 0), -EINVAL); p = strjoina("/run/systemd/machines/", machine); r = parse_env_file(NULL, p, diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index f47a542483..252a01add6 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -540,7 +540,7 @@ int sd_dhcp_client_set_hostname( /* Make sure hostnames qualify as DNS and as Linux hostnames */ if (hostname && - !(hostname_is_valid(hostname, false) && dns_name_is_valid(hostname) > 0)) + !(hostname_is_valid(hostname, 0) && dns_name_is_valid(hostname) > 0)) return -EINVAL; return free_and_strdup(&client->hostname, hostname); diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 97fd5fd4fb..e068e0dc91 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -407,7 +407,7 @@ int sd_dhcp6_client_set_fqdn( /* Make sure FQDN qualifies as DNS and as Linux hostname */ if (fqdn && - !(hostname_is_valid(fqdn, false) && dns_name_is_valid(fqdn) > 0)) + !(hostname_is_valid(fqdn, 0) && dns_name_is_valid(fqdn) > 0)) return -EINVAL; return free_and_strdup(&client->fqdn, fqdn); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index b8d4dc8d95..da7827015a 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -973,7 +973,7 @@ static int parse_container_unix_address(sd_bus *b, const char **p, char **guid) return -EINVAL; if (machine) { - if (!streq(machine, ".host") && !machine_name_is_valid(machine)) + if (!hostname_is_valid(machine, VALID_HOSTNAME_DOT_HOST)) return -EINVAL; free_and_replace(b->machine, machine); @@ -1448,7 +1448,7 @@ int bus_set_address_system_remote(sd_bus *b, const char *host) { } if (!in_charset(p, "0123456789") || *p == '\0') { - if (!machine_name_is_valid(p) || got_forward_slash) + if (!hostname_is_valid(p, 0) || got_forward_slash) return -EINVAL; m = TAKE_PTR(p); @@ -1463,7 +1463,7 @@ int bus_set_address_system_remote(sd_bus *b, const char *host) { 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)) + if (!in_charset(m, "0123456789") && hostname_is_valid(m, 0)) c = strjoina(",argv", p ? "7" : "5", "=--machine=", m); } @@ -1538,7 +1538,7 @@ _public_ int sd_bus_open_system_machine(sd_bus **ret, const char *machine) { assert_return(machine, -EINVAL); assert_return(ret, -EINVAL); - assert_return(streq(machine, ".host") || machine_name_is_valid(machine), -EINVAL); + assert_return(hostname_is_valid(machine, VALID_HOSTNAME_DOT_HOST), -EINVAL); r = sd_bus_new(&b); if (r < 0) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 1fc379512f..d15dafbd95 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -847,7 +847,7 @@ _public_ int sd_get_machine_names(char ***machines) { /* Filter out the unit: symlinks */ for (a = b = l; *a; a++) { - if (startswith(*a, "unit:") || !machine_name_is_valid(*a)) + if (startswith(*a, "unit:") || !hostname_is_valid(*a, 0)) free(*a); else { *b = *a; @@ -877,7 +877,7 @@ _public_ int sd_machine_get_class(const char *machine, char **class) { if (!c) return -ENOMEM; } else { - if (!machine_name_is_valid(machine)) + if (!hostname_is_valid(machine, 0)) return -EINVAL; p = strjoina("/run/systemd/machines/", machine); @@ -899,7 +899,7 @@ _public_ int sd_machine_get_ifindices(const char *machine, int **ret_ifindices) const char *p; int r; - assert_return(machine_name_is_valid(machine), -EINVAL); + assert_return(hostname_is_valid(machine, 0), -EINVAL); p = strjoina("/run/systemd/machines/", machine); r = parse_env_file(NULL, p, "NETIF", &netif_line); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 4a3279d264..3c839455b6 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1561,7 +1561,7 @@ static int make_service_name(const char *name, char **ret) { assert(name); assert(ret); - if (!machine_name_is_valid(name)) + if (!hostname_is_valid(name, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid machine name %s.", name); @@ -1881,7 +1881,7 @@ static int import_tar(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local name %s is not a suitable machine name.", local); @@ -1941,7 +1941,7 @@ static int import_raw(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local name %s is not a suitable machine name.", local); @@ -1995,7 +1995,7 @@ static int import_fs(int argc, char *argv[], void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Need either path or local name."); - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local name %s is not a suitable machine name.", local); @@ -2048,7 +2048,7 @@ static int export_tar(int argc, char *argv[], void *userdata) { assert(bus); local = argv[1]; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Machine name %s is not valid.", local); @@ -2090,7 +2090,7 @@ static int export_raw(int argc, char *argv[], void *userdata) { assert(bus); local = argv[1]; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Machine name %s is not valid.", local); @@ -2155,7 +2155,7 @@ static int pull_tar(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local name %s is not a suitable machine name.", local); @@ -2211,7 +2211,7 @@ static int pull_raw(int argc, char *argv[], void *userdata) { local = ll; - if (!machine_name_is_valid(local)) + if (!hostname_is_valid(local, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Local name %s is not a suitable machine name.", local); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 494813e334..501db643f8 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -239,7 +239,7 @@ static int method_create_or_register_machine(Manager *manager, sd_bus_message *m r = sd_bus_message_read(message, "s", &name); if (r < 0) return r; - if (!machine_name_is_valid(name)) + if (!hostname_is_valid(name, 0)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid machine name"); r = sd_bus_message_read_array(message, 'y', &v, &n); diff --git a/src/machine/machined.c b/src/machine/machined.c index 4d8891c4fc..6a5bf391e6 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -166,7 +166,7 @@ static int manager_enumerate_machines(Manager *m) { if (startswith(de->d_name, "unit:")) continue; - if (!machine_name_is_valid(de->d_name)) + if (!hostname_is_valid(de->d_name, 0)) continue; k = manager_add_machine(m, de->d_name, &machine); diff --git a/src/network/generator/network-generator.c b/src/network/generator/network-generator.c index 2fa21a067a..f9b51d8b7b 100644 --- a/src/network/generator/network-generator.c +++ b/src/network/generator/network-generator.c @@ -601,7 +601,7 @@ static int parse_cmdline_ip_address(Context *context, int family, const char *va if (p != value) { hostname = strndupa(value, p - value); - if (!hostname_is_valid(hostname, false)) + if (!hostname_is_valid(hostname, 0)) return -EINVAL; } diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 7ad8d087f4..73c3788e27 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -922,7 +922,7 @@ int config_parse_hostname( if (r < 0) return r; - if (!hostname_is_valid(hn, false)) { + if (!hostname_is_valid(hn, 0)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Hostname is not valid, ignoring assignment: %s", rvalue); return 0; diff --git a/src/nspawn/nspawn-oci.c b/src/nspawn/nspawn-oci.c index ca708be755..ccbae616ca 100644 --- a/src/nspawn/nspawn-oci.c +++ b/src/nspawn/nspawn-oci.c @@ -462,7 +462,7 @@ static int oci_hostname(const char *name, JsonVariant *v, JsonDispatchFlags flag assert_se(n = json_variant_string(v)); - if (!hostname_is_valid(n, false)) + if (!hostname_is_valid(n, 0)) return json_log(v, flags, SYNTHETIC_ERRNO(EINVAL), "Hostname string is not a valid hostname: %s", n); diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index 92bb5120ab..b43d61468e 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -701,7 +701,7 @@ int config_parse_hostname( assert(rvalue); assert(s); - if (!hostname_is_valid(rvalue, false)) { + if (!hostname_is_valid(rvalue, 0)) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid hostname, ignoring: %s", rvalue); return 0; } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 5e3d11be36..cfbc8f11bf 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -985,7 +985,7 @@ static int parse_argv(int argc, char *argv[]) { if (isempty(optarg)) arg_machine = mfree(arg_machine); else { - if (!machine_name_is_valid(optarg)) + if (!hostname_is_valid(optarg, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid machine name: %s", optarg); @@ -999,7 +999,7 @@ static int parse_argv(int argc, char *argv[]) { if (isempty(optarg)) arg_hostname = mfree(arg_hostname); else { - if (!hostname_is_valid(optarg, false)) + if (!hostname_is_valid(optarg, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid hostname: %s", optarg); @@ -2984,7 +2984,7 @@ static int determine_names(void) { return log_oom(); hostname_cleanup(arg_machine); - if (!machine_name_is_valid(arg_machine)) + if (!hostname_is_valid(arg_machine, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to determine machine name automatically, please use -M."); if (arg_ephemeral) { diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c index 73839b3115..c7a63bd047 100644 --- a/src/test/test-hostname-util.c +++ b/src/test/test-hostname-util.c @@ -10,40 +10,40 @@ #include "util.h" static void test_hostname_is_valid(void) { - assert_se(hostname_is_valid("foobar", false)); - assert_se(hostname_is_valid("foobar.com", false)); - assert_se(!hostname_is_valid("foobar.com.", false)); - assert_se(hostname_is_valid("fooBAR", false)); - assert_se(hostname_is_valid("fooBAR.com", false)); - assert_se(!hostname_is_valid("fooBAR.", false)); - assert_se(!hostname_is_valid("fooBAR.com.", false)); - assert_se(!hostname_is_valid("fööbar", false)); - assert_se(!hostname_is_valid("", false)); - assert_se(!hostname_is_valid(".", false)); - assert_se(!hostname_is_valid("..", false)); - assert_se(!hostname_is_valid("foobar.", false)); - assert_se(!hostname_is_valid(".foobar", false)); - assert_se(!hostname_is_valid("foo..bar", false)); - assert_se(!hostname_is_valid("foo.bar..", false)); - assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", false)); - assert_se(!hostname_is_valid("au-xph5-rvgrdsb5hcxc-47et3a5vvkrc-server-wyoz4elpdpe3.openstack.local", false)); + assert_se(hostname_is_valid("foobar", 0)); + assert_se(hostname_is_valid("foobar.com", 0)); + assert_se(!hostname_is_valid("foobar.com.", 0)); + assert_se(hostname_is_valid("fooBAR", 0)); + assert_se(hostname_is_valid("fooBAR.com", 0)); + assert_se(!hostname_is_valid("fooBAR.", 0)); + assert_se(!hostname_is_valid("fooBAR.com.", 0)); + assert_se(!hostname_is_valid("fööbar", 0)); + assert_se(!hostname_is_valid("", 0)); + assert_se(!hostname_is_valid(".", 0)); + assert_se(!hostname_is_valid("..", 0)); + assert_se(!hostname_is_valid("foobar.", 0)); + assert_se(!hostname_is_valid(".foobar", 0)); + assert_se(!hostname_is_valid("foo..bar", 0)); + assert_se(!hostname_is_valid("foo.bar..", 0)); + assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", 0)); + assert_se(!hostname_is_valid("au-xph5-rvgrdsb5hcxc-47et3a5vvkrc-server-wyoz4elpdpe3.openstack.local", 0)); - assert_se(hostname_is_valid("foobar", true)); - assert_se(hostname_is_valid("foobar.com", true)); - assert_se(hostname_is_valid("foobar.com.", true)); - assert_se(hostname_is_valid("fooBAR", true)); - assert_se(hostname_is_valid("fooBAR.com", true)); - assert_se(!hostname_is_valid("fooBAR.", true)); - assert_se(hostname_is_valid("fooBAR.com.", true)); - assert_se(!hostname_is_valid("fööbar", true)); - assert_se(!hostname_is_valid("", true)); - assert_se(!hostname_is_valid(".", true)); - assert_se(!hostname_is_valid("..", true)); - assert_se(!hostname_is_valid("foobar.", true)); - assert_se(!hostname_is_valid(".foobar", true)); - assert_se(!hostname_is_valid("foo..bar", true)); - assert_se(!hostname_is_valid("foo.bar..", true)); - assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", true)); + assert_se(hostname_is_valid("foobar", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(hostname_is_valid("foobar.com", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(hostname_is_valid("foobar.com.", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(hostname_is_valid("fooBAR", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(hostname_is_valid("fooBAR.com", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("fooBAR.", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(hostname_is_valid("fooBAR.com.", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("fööbar", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid(".", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("..", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("foobar.", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid(".foobar", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("foo..bar", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("foo.bar..", VALID_HOSTNAME_TRAILING_DOT)); + assert_se(!hostname_is_valid("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", VALID_HOSTNAME_TRAILING_DOT)); } static void test_hostname_cleanup(void) { @@ -151,7 +151,7 @@ static void test_hostname_malloc(void) { } static void test_fallback_hostname(void) { - if (!hostname_is_valid(FALLBACK_HOSTNAME, false)) { + if (!hostname_is_valid(FALLBACK_HOSTNAME, 0)) { log_error("Configured fallback hostname \"%s\" is not valid.", FALLBACK_HOSTNAME); exit(EXIT_FAILURE); } From d4e98094655a057293e032325be092140f779bc6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 16:43:39 +0100 Subject: [PATCH 06/13] hostname-setup: clarify that failures reading /etc/hostname are ignored --- src/core/hostname-setup.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c index 4e20e764a3..6ccaa479de 100644 --- a/src/core/hostname-setup.c +++ b/src/core/hostname-setup.c @@ -34,12 +34,11 @@ int hostname_setup(void) { if (!hn) { r = read_etc_hostname(NULL, &b); - if (r < 0) { - if (r == -ENOENT) - enoent = true; - else - log_warning_errno(r, "Failed to read configured hostname: %m"); - } else + if (r == -ENOENT) + enoent = true; + else if (r < 0) + log_warning_errno(r, "Failed to read configured hostname, ignoring: %m"); + else hn = b; } From 79485fc27a5b18ac8957960eaccbc8a960e1bd64 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2020 16:44:04 +0100 Subject: [PATCH 07/13] firstboot: clean-up the copied hostname, not argv[] directly, as that's ugly --- src/firstboot/firstboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 48baae3f89..afead11b42 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -1139,11 +1139,11 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Host name %s is not valid.", optarg); - hostname_cleanup(optarg); r = free_and_strdup(&arg_hostname, optarg); if (r < 0) return log_oom(); + hostname_cleanup(arg_hostname); break; case ARG_MACHINE_ID: From f8ecc2c00df7bd810557f3056ec12f6a0730812d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 13:16:39 +0100 Subject: [PATCH 08/13] sd-bus: make credential acquisition more graceful MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far when asked for augmented bus credentials and the process was already gone we'd fail fatally. Let's make this graceful instead, and never allow augmenting fail due to PID having vanished — unless the augmenting is the explicit and only purpose of the requested operation. This should be safe as clients have to explicitly query the acquired creds anyway and handle if they couldn't be acquired. Moreover we already handle permission problems gracefully, thus clients must be ready to deal with missing creds. This is useful to make selinux authorization work for short-lived client proceses. PReviously we'd augment creds to have more info to log about (the selinux decision would not be based on augmented data however, because that'd be unsafe), and would fail if we couldn't get it. Now, we'll try to acquire the data, but if we cannot acquire it, we'll still do the selinux check, except that logging will be more limited. --- src/libsystemd/sd-bus/bus-control.c | 4 ++-- src/libsystemd/sd-bus/bus-convenience.c | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index a63937e1ce..3ee22c9638 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -713,7 +713,7 @@ _public_ int sd_bus_get_name_creds( } r = bus_creds_add_more(c, mask, pid, 0); - if (r < 0) + if (r < 0 && r != -ESRCH) /* Return the error, but ignore ESRCH which just means the process is already gone */ return r; } @@ -788,7 +788,7 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r } r = bus_creds_add_more(c, mask, pid, 0); - if (r < 0) + if (r < 0 && r != -ESRCH) /* If the process vanished, then don't complain, just return what we got */ return r; *ret = TAKE_PTR(c); diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index 4bf228b436..ea2fd8935b 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -603,8 +603,9 @@ _public_ int sd_bus_set_property( return r; } -_public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_bus_creds **creds) { +_public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_bus_creds **ret) { sd_bus_creds *c; + int r; assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); @@ -618,7 +619,7 @@ _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_b /* All data we need? */ if (c && (mask & ~c->mask) == 0) { - *creds = sd_bus_creds_ref(c); + *ret = sd_bus_creds_ref(c); return 0; } @@ -629,15 +630,22 @@ _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_b if (call->sender) /* There's a sender, but the creds are missing. */ - return sd_bus_get_name_creds(call->bus, call->sender, mask, creds); + return sd_bus_get_name_creds(call->bus, call->sender, mask, ret); else /* There's no sender. For direct connections * the credentials of the AF_UNIX peer matter, * which may be queried via sd_bus_get_owner_creds(). */ - return sd_bus_get_owner_creds(call->bus, mask, creds); + return sd_bus_get_owner_creds(call->bus, mask, ret); } - return bus_creds_extend_by_pid(c, mask, creds); + r = bus_creds_extend_by_pid(c, mask, ret); + if (r == -ESRCH) { + /* Process doesn't exist anymore? propagate the few things we have */ + *ret = sd_bus_creds_ref(c); + return 0; + } + + return r; } _public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) { From 1ca37419b13b836d7fb2b9815d5efb6dccc62134 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 13:20:28 +0100 Subject: [PATCH 09/13] sd-bus: 'ret' parameter to sd_bus_query_sender_creds() is not optional, check for it --- src/libsystemd/sd-bus/bus-convenience.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index ea2fd8935b..0314642f3e 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -611,6 +611,7 @@ _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_b assert_return(call->sealed, -EPERM); assert_return(call->bus, -EINVAL); assert_return(!bus_pid_changed(call->bus), -ECHILD); + assert_return(ret, -EINVAL); if (!BUS_IS_OPEN(call->bus->state)) return -ENOTCONN; From 1b630835dff5e13046dfd266629f8ff244dc7fb0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 13:21:58 +0100 Subject: [PATCH 10/13] sd-bus: add API for connecting to a specific user's user bus of a specific container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is unfortunately harder to implement than it sounds. The user's bus is bound a to the user's lifecycle after all (i.e. only exists as long as the user has at least one PAM session), and the path dynamically (at least theoretically, in practice it's going to be the same always) generated via $XDG_RUNTIME_DIR in /run/. To fix this properly, we'll thus go through PAM before connecting to a user bus. Which is hard since we cannot just link against libpam in the container, since the container might have been compiled entirely differently. So our way out is to use systemd-run from outside, which invokes a transient unit that does PAM from outside, doing so via D-Bus. Inside the transient unit we then invoke systemd-stdio-bridge which forwards D-Bus from the user bus to us. The systemd-stdio-bridge makes up the PAM session and thus we can sure tht the bus exists at least as long as the bus connection is kept. Or so say this differently: if you use "systemctl -M lennart@foobar" now, the bus connection works like this: 1. sd-bus on the host forks off: systemd-run -M foobar -PGq --wait -pUser=lennart -pPAMName=login systemd-stdio-bridge 2. systemd-run gets a connection to the "foobar" container's system bus, and invokes the "systemd-stdio-bridge" binary as transient service inside a PAM session for the user "lennart" 3. The systemd-stdio-bridge then proxies our D-Bus traffic to the user bus. sd-bus (on host) → systemd-run (on host) → systemd-stdio-bridge (in container) Complicated? Well, to some point yes, but otoh it's actually nice in various other ways, primarily as it makes the -H and -M codepaths more alike. In the -H case (i.e. connect to remote host via SSH) a very similar three steps are used. The only difference is that instead of "systemd-run" the "ssh" binary is used to invoke the stdio bridge in a PAM session of some other system. Thus we get similar implementation and isolation for similar operations. Fixes: #14580 --- src/busctl/busctl.c | 2 +- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-bus/bus-internal.h | 2 +- src/libsystemd/sd-bus/sd-bus.c | 213 +++++++++++++++++++++++++-- src/shared/bus-util.c | 15 +- src/stdio-bridge/stdio-bridge.c | 2 +- src/systemctl/systemctl.c | 2 +- src/systemd/sd-bus.h | 1 + 8 files changed, 217 insertions(+), 22 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 06a15ddd80..6f805e95a0 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -121,7 +121,7 @@ static int acquire_bus(bool set_monitor, sd_bus **ret) { break; case BUS_TRANSPORT_MACHINE: - r = bus_set_address_system_machine(bus, arg_host); + r = bus_set_address_machine(bus, arg_user, arg_host); break; default: diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index b03bcd952f..9e9e8fd372 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -739,6 +739,8 @@ global: LIBSYSTEMD_248 { global: + sd_bus_open_user_machine; + sd_event_source_set_ratelimit; sd_event_source_get_ratelimit; sd_event_source_is_ratelimited; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 233a228315..82fa97fc5d 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -401,7 +401,7 @@ void bus_close_io_fds(sd_bus *b); int bus_set_address_system(sd_bus *bus); int bus_set_address_user(sd_bus *bus); int bus_set_address_system_remote(sd_bus *b, const char *host); -int bus_set_address_system_machine(sd_bus *b, const char *machine); +int bus_set_address_machine(sd_bus *b, bool user, const char *machine); int bus_maybe_reply_error(sd_bus_message *m, int r, sd_bus_error *error); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index da7827015a..e9bc19d96a 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -41,6 +41,7 @@ #include "process-util.h" #include "string-util.h" #include "strv.h" +#include "user-util.h" #define log_debug_bus_message(m) \ do { \ @@ -1514,44 +1515,228 @@ _public_ int sd_bus_open_system_remote(sd_bus **ret, const char *host) { return 0; } -int bus_set_address_system_machine(sd_bus *b, const char *machine) { - _cleanup_free_ char *e = NULL; +int bus_set_address_machine(sd_bus *b, bool user, const char *machine) { + const char *rhs; char *a; assert(b); assert(machine); - e = bus_address_escape(machine); - if (!e) - return -ENOMEM; + rhs = strchr(machine, '@'); + if (rhs || user) { + _cleanup_free_ char *u = NULL, *eu = NULL, *erhs = NULL; - a = strjoin("x-machine-unix:machine=", e); - if (!a) - return -ENOMEM; + /* 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 + * might sound: it's not sufficient to enter the container and connect to some socket there, + * since the --user socket path depends on $XDG_RUNTIME_DIR which is set via PAM. Thus, to be + * able to connect, we need to have a PAM session. Our way out? We use systemd-run to get + * 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 + u = getusername_malloc(); /* Empty user name, let's use the local one */ + 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) + return -ENOMEM; + } + + /* systemd-run -M… -PGq --wait -pUser=… -pPAMName=login systemd-stdio-bridge */ + + a = strjoin("unixexec:path=systemd-run," + "argv1=-M", erhs ?: ".host", "," + "argv2=-PGq," + "argv3=--wait," + "argv4=-pUser%3d", eu ?: "root", ",", + "argv5=-pPAMName%3dlogin," + "argv6=systemd-stdio-bridge"); + if (!a) + return -ENOMEM; + + if (user) { + char *k; + + /* Ideally we'd use the "--user" switch to systemd-stdio-bridge here, but it's only + * available in recent systemd versions. Using the "-p" switch with the explicit path + * is a working alternative, and is compatible with older versions, hence that's what + * we use here. */ + + k = strjoin(a, ",argv7=-punix:path%3d%24%7bXDG_RUNTIME_DIR%7d/bus"); + if (!k) + return -ENOMEM; + + free_and_replace(a, k); + } + } else { + _cleanup_free_ char *e = NULL; + + /* 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); + if (!e) + return -ENOMEM; + + a = strjoin("x-machine-unix:machine=", e); + if (!a) + return -ENOMEM; + } return free_and_replace(b->address, a); } -_public_ int sd_bus_open_system_machine(sd_bus **ret, const char *machine) { +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)) + 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; + + /* 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 + * 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. */ + + assert(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")) + return true; + + /* Otherwise, if we are root, then we can also allow the ".host" syntax, as that's the user this + * would connect to. */ + if (geteuid() == 0 && STR_IN_SET(user_and_machine, ".host", "root@.host")) + return true; + + /* Otherwise, we have to figure our user name, and compare things with that. */ + un = getusername_malloc(); + if (!un) + return -ENOMEM; + + f = startswith(user_and_machine, un); + if (!f) + return false; + + return STR_IN_SET(f, "@", "@.host"); +} + +_public_ int sd_bus_open_system_machine(sd_bus **ret, const char *user_and_machine) { _cleanup_(bus_freep) sd_bus *b = NULL; int r; - assert_return(machine, -EINVAL); + assert_return(user_and_machine, -EINVAL); assert_return(ret, -EINVAL); - assert_return(hostname_is_valid(machine, VALID_HOSTNAME_DOT_HOST), -EINVAL); + + if (user_and_machine_equivalent(user_and_machine)) + return sd_bus_open_system(ret); + + r = user_and_machine_valid(user_and_machine); + if (r < 0) + return r; + + assert_return(r > 0, -EINVAL); r = sd_bus_new(&b); if (r < 0) return r; - r = bus_set_address_system_machine(b, machine); + r = bus_set_address_machine(b, false, user_and_machine); if (r < 0) return r; b->bus_client = true; - b->trusted = false; b->is_system = true; - b->is_local = false; + + r = sd_bus_start(b); + if (r < 0) + return r; + + *ret = TAKE_PTR(b); + return 0; +} + +_public_ int sd_bus_open_user_machine(sd_bus **ret, const char *user_and_machine) { + _cleanup_(bus_freep) sd_bus *b = NULL; + int r; + + assert_return(user_and_machine, -EINVAL); + assert_return(ret, -EINVAL); + + /* Shortcut things if we'd end up on this host and as the same user. */ + if (user_and_machine_equivalent(user_and_machine)) + return sd_bus_open_user(ret); + + r = user_and_machine_valid(user_and_machine); + if (r < 0) + return r; + + assert_return(r > 0, -EINVAL); + + r = sd_bus_new(&b); + if (r < 0) + return r; + + r = bus_set_address_machine(b, true, user_and_machine); + if (r < 0) + return r; + + b->bus_client = true; + b->trusted = true; r = sd_bus_start(b); if (r < 0) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index fbda218b3b..58211ebd03 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -249,7 +249,12 @@ int bus_connect_user_systemd(sd_bus **_bus) { return 0; } -int bus_connect_transport(BusTransport transport, const char *host, bool user, sd_bus **ret) { +int bus_connect_transport( + BusTransport transport, + const char *host, + bool user, + sd_bus **ret) { + _cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL; int r; @@ -258,7 +263,7 @@ int bus_connect_transport(BusTransport transport, const char *host, bool user, s assert(ret); assert_return((transport == BUS_TRANSPORT_LOCAL) == !host, -EINVAL); - assert_return(transport == BUS_TRANSPORT_LOCAL || !user, -EOPNOTSUPP); + assert_return(transport != BUS_TRANSPORT_REMOTE || !user, -EOPNOTSUPP); switch (transport) { @@ -279,7 +284,10 @@ int bus_connect_transport(BusTransport transport, const char *host, bool user, s break; case BUS_TRANSPORT_MACHINE: - r = sd_bus_open_system_machine(&bus, host); + if (user) + r = sd_bus_open_user_machine(&bus, host); + else + r = sd_bus_open_system_machine(&bus, host); break; default: @@ -293,7 +301,6 @@ int bus_connect_transport(BusTransport transport, const char *host, bool user, s return r; *ret = TAKE_PTR(bus); - return 0; } diff --git a/src/stdio-bridge/stdio-bridge.c b/src/stdio-bridge/stdio-bridge.c index 81d50717b2..1b7c3feaea 100644 --- a/src/stdio-bridge/stdio-bridge.c +++ b/src/stdio-bridge/stdio-bridge.c @@ -121,7 +121,7 @@ static int run(int argc, char *argv[]) { return log_error_errno(r, "Failed to allocate bus: %m"); if (arg_transport == BUS_TRANSPORT_MACHINE) - r = bus_set_address_system_machine(a, arg_bus_path); + r = bus_set_address_machine(a, false, arg_bus_path); else r = sd_bus_set_address(a, arg_bus_path); if (r < 0) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d002d933ae..9a934badce 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -877,7 +877,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { assert_not_reached("Unhandled option"); } - if (arg_transport != BUS_TRANSPORT_LOCAL && arg_scope != UNIT_FILE_SYSTEM) + if (arg_transport == BUS_TRANSPORT_REMOTE && arg_scope != UNIT_FILE_SYSTEM) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot access user instance remotely."); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 87fbcf366e..c51df2908d 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -135,6 +135,7 @@ int sd_bus_open(sd_bus **ret); int sd_bus_open_with_description(sd_bus **ret, const char *description); int sd_bus_open_user(sd_bus **ret); int sd_bus_open_user_with_description(sd_bus **ret, const char *description); +int sd_bus_open_user_machine(sd_bus **ret, const char *machine); int sd_bus_open_system(sd_bus **ret); int sd_bus_open_system_with_description(sd_bus **ret, const char *description); int sd_bus_open_system_remote(sd_bus **ret, const char *host); From ba4a31b7a61cc730b7cbc10948d4f1d6d78d93c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 13:23:00 +0100 Subject: [PATCH 11/13] man: document new ability to connect to user of container --- man/sd_bus_default.xml | 22 ++++++++++++++++++++-- man/user-system-options.xml | 10 ++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/man/sd_bus_default.xml b/man/sd_bus_default.xml index 4ae26414eb..768a386160 100644 --- a/man/sd_bus_default.xml +++ b/man/sd_bus_default.xml @@ -24,6 +24,7 @@ sd_bus_open_with_description sd_bus_open_user sd_bus_open_user_with_description + sd_bus_open_user_machine sd_bus_open_system sd_bus_open_system_with_description sd_bus_open_system_remote @@ -73,6 +74,12 @@ const char *description + + int sd_bus_open_user_machine + sd_bus **bus + const char *machine + + int sd_bus_open_system sd_bus **bus @@ -187,14 +194,24 @@ work for the root user on the remote machine. sd_bus_open_system_machine() connects to the system bus in the specified - machine, where machine is the name of a local - container. See + machine, where machine is the name of a local container, + possibly prefixed by a user name and a separating @. If the container name is + specified as the special string .host the connection is made to the local system. This + is useful to connect to the local system bus as specific user, e.g. foobar@.host to + connect to the local system bus as local user foobar. If the @ + syntax is used either the left-hand side or the right-hand side may be ommited (but not both) in which + case the local user name or .host is implied. If the @ syntax is + not used the connection is always made as root user. See sd_bus_set_address3 for a description of the address syntax, and machinectl1 for more information about the "machine" concept. Note that connections into local containers are only available to privileged processes at this time. + sd_bus_open_user_machine() is similar to + sd_bus_open_system_machine(), but connects to the user bus of the root user, or if + the @ syntax is used, of the specified user. + These calls allocate a bus connection object and initiate the connection to a well-known bus of some form. An alternative to using these high-level calls is to create an unconnected bus @@ -210,6 +227,7 @@ Reference ownership The functions sd_bus_open(), sd_bus_open_user(), + sd_bus_open_user_machine(), sd_bus_open_system(), sd_bus_open_system_remote(), and sd_bus_open_system_machine() return a new diff --git a/man/user-system-options.xml b/man/user-system-options.xml index 728118e60c..e2c19b2982 100644 --- a/man/user-system-options.xml +++ b/man/user-system-options.xml @@ -45,8 +45,14 @@ - Execute operation on a local container. Specify a - container name to connect to. + Execute operation on a local container. Specify a container name to connect to, optionally + prefixed by a user name to connect as and a separating @ character. If the special + string .host is used in place of the container name, a connection to the local + system is made (which is useful to connect to a specific user's user bus: --user + --machine=lennart@.host). If the @ syntax is not used, the connection is + made as root user. If the @ syntax is used either the left hand side or the right hand + side may be ommitted (but not both) in which case the local user name and .host are + implied. From cedfd142dea105b37223ad24d784494bfae83dc5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 13:23:31 +0100 Subject: [PATCH 12/13] stdio-bridge: add support for --system and --user So far, the bridge always acted as if "--system" was used, i.e. would unconditionally connect to the system bus. Let's add "--user" too, to connect to the users session bus. This is mostly for completeness' sake. I wanted to use this when making sd-bus's ability to connect to other user's D-Bus busses work, but it didn't exist so far. In the interest of keeping things compatible the implementation in sd-bus will not use the new "--user" switch, and instead manually construct the right bus path via "--path=", but we still should add the proper switches, as preparation for a brighter future, one day. --- src/stdio-bridge/stdio-bridge.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/stdio-bridge/stdio-bridge.c b/src/stdio-bridge/stdio-bridge.c index 1b7c3feaea..5d8b514874 100644 --- a/src/stdio-bridge/stdio-bridge.c +++ b/src/stdio-bridge/stdio-bridge.c @@ -23,6 +23,7 @@ static const char *arg_bus_path = DEFAULT_BUS_PATH; static BusTransport arg_transport = BUS_TRANSPORT_LOCAL; +static bool arg_user = false; static int help(void) { @@ -30,8 +31,10 @@ 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" - " -M --machine=MACHINE Name of machine to connect to\n", + " -p --bus-path=PATH Path to the bus address (default: %s)\n" + " --system Connect to system bus\n" + " --user Connect to user bus\n" + " -M --machine=CONTAINER Name of local container to connect to\n", program_invocation_short_name, DEFAULT_BUS_PATH); return 0; @@ -42,12 +45,16 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_VERSION = 0x100, ARG_MACHINE, + ARG_USER, + ARG_SYSTEM, }; static const struct option options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, ARG_VERSION }, { "bus-path", required_argument, NULL, 'p' }, + { "user", no_argument, NULL, ARG_USER }, + { "system", no_argument, NULL, ARG_SYSTEM }, { "machine", required_argument, NULL, 'M' }, {}, }; @@ -67,6 +74,14 @@ static int parse_argv(int argc, char *argv[]) { case ARG_VERSION: return version(); + case ARG_USER: + arg_user = true; + break; + + case ARG_SYSTEM: + arg_user = false; + break; + case 'p': arg_bus_path = optarg; break; @@ -121,7 +136,7 @@ static int run(int argc, char *argv[]) { return log_error_errno(r, "Failed to allocate bus: %m"); if (arg_transport == BUS_TRANSPORT_MACHINE) - r = bus_set_address_machine(a, false, arg_bus_path); + r = bus_set_address_machine(a, arg_user, arg_bus_path); else r = sd_bus_set_address(a, arg_bus_path); if (r < 0) From 1ecb46724cae151606bc825f0e39f14d4dfe1a0e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 14 Dec 2020 16:36:00 +0100 Subject: [PATCH 13/13] bus-util: improve logging when we can't connect to the bus Previously, we'd already have explicit logging for the case where $XDG_RUNTIME_DIR is not set. Let's also add some explicit logging for the EPERM/ACCESS case. Let's also in both cases suggest the --machine=@.host syntax. And while we are at it, let's remove side-effects from the macro. By checking for both the EPERM/EACCES case and the $XDG_RUNTIME_DIR case we will now catch both the cases where people use "su" to issue a "systemctl --user" operation, and those where they (more correctly, but still not good enough) call "su -". Fixes: #17901 --- src/shared/bus-util.h | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index a02d82a52e..27dd6c19c0 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -9,6 +9,7 @@ #include "sd-bus.h" #include "sd-event.h" +#include "errno-util.h" #include "macro.h" #include "string-util.h" #include "time-util.h" @@ -39,13 +40,21 @@ int bus_connect_transport(BusTransport transport, const char *host, bool user, s int bus_connect_transport_systemd(BusTransport transport, const char *host, bool user, sd_bus **bus); #define bus_log_address_error(r) \ - log_error_errno(r, \ - r == -ENOMEDIUM ? "Failed to set bus address: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined" : \ - "Failed to set bus address: %m") + ({ \ + int _k = (r); \ + log_error_errno(_k, \ + _k == -ENOMEDIUM ? "Failed to set bus address: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined (consider using --machine=@.host --user to connect to bus of other user)" : \ + "Failed to set bus address: %m"); \ + }) + #define bus_log_connect_error(r) \ - log_error_errno(r, \ - r == -ENOMEDIUM ? "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined" : \ - "Failed to connect to bus: %m") + ({ \ + int _k = (r); \ + log_error_errno(_k, \ + _k == -ENOMEDIUM ? "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined (consider using --machine=@.host --user to connect to bus of other user)" : \ + ERRNO_IS_PRIVILEGE(_k) ? "Failed to connect to bus: Operation not permitted (consider using --machine=@.host --user to connect to bus of other user)" : \ + "Failed to connect to bus: %m"); \ + }) #define bus_log_parse_error(r) \ log_error_errno(r, "Failed to parse bus message: %m")