From 8b8024f1c231c166f5c450905c8fd91d11704ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 20 Feb 2021 17:04:05 +0100 Subject: [PATCH 01/12] shared/conf-parser: allow more than one location of the main config file No functional change as long as only one path is passed. --- src/basic/macro.h | 1 + src/network/netdev/netdev.c | 4 ++-- src/network/networkd-network.c | 2 +- src/resolve/resolved-dnssd.c | 2 +- src/shared/conf-parser.c | 23 ++++++++++++++--------- src/shared/conf-parser.h | 2 +- src/test/test-conf-parser.c | 12 ++++++------ 7 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/basic/macro.h b/src/basic/macro.h index eebd0769dd..f1d5e0894e 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -376,6 +376,7 @@ static inline int __coverity_check_and_return__(int condition) { #define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL })) #define STRV_MAKE_EMPTY ((char*[1]) { NULL }) +#define STRV_MAKE_CONST(...) ((const char* const*) ((const char*[]) { __VA_ARGS__, NULL })) /* Pointers range from NULL to POINTER_MAX */ #define POINTER_MAX ((void*) UINTPTR_MAX) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index b26064297b..36d6c5168a 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -686,7 +686,7 @@ int netdev_load_one(Manager *manager, const char *filename) { dropin_dirname = strjoina(basename(filename), ".d"); r = config_parse_many( - filename, NETWORK_DIRS, dropin_dirname, + STRV_MAKE_CONST(filename), NETWORK_DIRS, dropin_dirname, NETDEV_COMMON_SECTIONS NETDEV_OTHER_SECTIONS, config_item_perf_lookup, network_netdev_gperf_lookup, CONFIG_PARSE_WARN, @@ -729,7 +729,7 @@ int netdev_load_one(Manager *manager, const char *filename) { NETDEV_VTABLE(netdev)->init(netdev); r = config_parse_many( - filename, NETWORK_DIRS, dropin_dirname, + STRV_MAKE_CONST(filename), NETWORK_DIRS, dropin_dirname, NETDEV_VTABLE(netdev)->sections, config_item_perf_lookup, network_netdev_gperf_lookup, CONFIG_PARSE_WARN, diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 4f0dfe469f..e3c138411e 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -446,7 +446,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi }; r = config_parse_many( - filename, NETWORK_DIRS, dropin_dirname, + STRV_MAKE_CONST(filename), NETWORK_DIRS, dropin_dirname, "Match\0" "Link\0" "SR-IOV\0" diff --git a/src/resolve/resolved-dnssd.c b/src/resolve/resolved-dnssd.c index 34c96e18bd..33d695a9ce 100644 --- a/src/resolve/resolved-dnssd.c +++ b/src/resolve/resolved-dnssd.c @@ -88,7 +88,7 @@ static int dnssd_service_load(Manager *manager, const char *filename) { dropin_dirname = strjoina(service->name, ".dnssd.d"); r = config_parse_many( - filename, DNSSD_SERVICE_DIRS, dropin_dirname, + STRV_MAKE_CONST(filename), DNSSD_SERVICE_DIRS, dropin_dirname, "Service\0", config_item_perf_lookup, resolved_dnssd_gperf_lookup, CONFIG_PARSE_WARN, diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index c3d7e536ec..eb468390c2 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -419,11 +419,11 @@ int config_parse( if (ret_mtime) *ret_mtime = mtime; - return 0; + return 1; } static int config_parse_many_files( - const char *conf_file, + const char* const* conf_files, char **files, const char *sections, ConfigItemLookup lookup, @@ -436,20 +436,23 @@ static int config_parse_many_files( char **fn; int r; - if (conf_file) { - r = config_parse(NULL, conf_file, NULL, sections, lookup, table, flags, userdata, &mtime); + /* First read the first found main config file. */ + STRV_FOREACH(fn, (char**) conf_files) { + r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &mtime); if (r < 0) return r; + if (r > 0) + break; } + /* Then read all the drop-ins. */ STRV_FOREACH(fn, files) { usec_t t; r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &t); if (r < 0) return r; - if (t > mtime) /* Find the newest */ - mtime = t; + mtime = MAX(mtime, t); /* Find the newest */ } if (ret_mtime) @@ -476,12 +479,14 @@ int config_parse_many_nulstr( if (r < 0) return r; - return config_parse_many_files(conf_file, files, sections, lookup, table, flags, userdata, ret_mtime); + return config_parse_many_files(STRV_MAKE_CONST(conf_file), + files, sections, lookup, table, flags, userdata, + ret_mtime); } /* Parse each config file in the directories specified as strv. */ int config_parse_many( - const char *conf_file, + const char* const* conf_files, const char* const* conf_file_dirs, const char *dropin_dirname, const char *sections, @@ -505,7 +510,7 @@ int config_parse_many( if (r < 0) return r; - return config_parse_many_files(conf_file, files, sections, lookup, table, flags, userdata, ret_mtime); + return config_parse_many_files(conf_files, files, sections, lookup, table, flags, userdata, ret_mtime); } #define DEFINE_PARSER(type, vartype, conv_func) \ diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 5a7dc7ff59..cd09491112 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -102,7 +102,7 @@ int config_parse_many_nulstr( usec_t *ret_mtime); /* possibly NULL */ int config_parse_many( - const char *conf_file, /* possibly NULL */ + const char* const* conf_files, /* possibly empty */ const char* const* conf_file_dirs, const char *dropin_dirname, const char *sections, /* nulstr */ diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index 04b610c6f1..5da864347e 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -349,27 +349,27 @@ static void test_config_parse(unsigned i, const char *s) { switch (i) { case 0 ... 4: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, "1")); break; case 5 ... 10: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, "1 2 3")); break; case 11: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, "1\\\\ \\\\2")); break; case 12: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, x1000("ABCD"))); break; case 13 ... 14: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, x1000("ABCD") " foobar")); break; @@ -379,7 +379,7 @@ static void test_config_parse(unsigned i, const char *s) { break; case 17: - assert_se(r == 0); + assert_se(r == 1); assert_se(streq(setting1, "2")); break; } From f70e6fb471a4eeed8e2a4a7c608beabbb0aba8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 10:56:04 +0100 Subject: [PATCH 02/12] manager: read ~/.config/systemd/user.conf in user mode This changes the paths we read user manager config from in two ways: - split-usr-root paths are dropped. The user manager is a poster boy for non-early-boot, so reading dropins only from /usr is appropriate. - we look at ~/.config/systemd/user.conf. Users should be allowed to override their own config. As user managers become more and more used, it becomes more important for users to customize their own daemon. By reading from ~/.config, this is possible without privileges. --- man/systemd-system.conf.xml | 24 ++++++++------- src/core/main.c | 60 ++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 075666ac6b..0811e1909e 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -31,7 +31,9 @@ /etc/systemd/system.conf.d/*.conf, /run/systemd/system.conf.d/*.conf, /usr/lib/systemd/system.conf.d/*.conf - /etc/systemd/user.conf, + + ~/.config/systemd/user.conf, + /etc/systemd/user.conf, /etc/systemd/user.conf.d/*.conf, /run/systemd/user.conf.d/*.conf, /usr/lib/systemd/user.conf.d/*.conf @@ -40,16 +42,16 @@ Description - When run as a system instance, systemd interprets the - configuration file system.conf and the files - in system.conf.d directories; when run as a - user instance, systemd interprets the configuration file - user.conf and the files in - user.conf.d directories. These configuration - files contain a few settings controlling basic manager - operations. See - systemd.syntax7 - for a general description of the syntax. + When run as a system instance, systemd interprets the configuration file + system.conf and the files in system.conf.d directories; when + run as a user instance, it interprets the configuration file user.conf (either in + the home directory of the user, or if not found, under /etc/systemd/) and the files + in user.conf.d directories. These configuration files contain a few settings + controlling basic manager operations. + + See + systemd.syntax7 for a + general description of the syntax. diff --git a/src/core/main.c b/src/core/main.c index f29449d691..2237925209 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -163,6 +163,36 @@ static char **saved_env = NULL; static int parse_configuration(const struct rlimit *saved_rlimit_nofile, const struct rlimit *saved_rlimit_memlock); +static int manager_find_user_config_paths(char ***ret_files, char ***ret_dirs) { + _cleanup_free_ char *base = NULL; + _cleanup_strv_free_ char **files = NULL, **dirs = NULL; + int r; + + r = xdg_user_config_dir(&base, "/systemd"); + if (r < 0) + return r; + + r = strv_extendf(&files, "%s/user.conf", base); + if (r < 0) + return r; + + r = strv_extend(&files, PKGSYSCONFDIR "/user.conf"); + if (r < 0) + return r; + + r = strv_consume(&dirs, TAKE_PTR(base)); + if (r < 0) + return r; + + r = strv_extend_strv(&dirs, CONF_PATHS_STRV("systemd"), false); + if (r < 0) + return r; + + *ret_files = TAKE_PTR(files); + *ret_dirs = TAKE_PTR(dirs); + return 0; +} + _noreturn_ static void freeze_or_exit_or_reboot(void) { /* If we are running in a container, let's prefer exiting, after all we can propagate an exit code to @@ -668,26 +698,34 @@ static int parse_config_file(void) { {} }; - const char *fn, *conf_dirs_nulstr; + _cleanup_strv_free_ char **_free_files = NULL, **_free_dirs = NULL; - fn = arg_system ? - PKGSYSCONFDIR "/system.conf" : - PKGSYSCONFDIR "/user.conf"; + const char *const *files, *const *dirs, *suffix; + int r; - conf_dirs_nulstr = arg_system ? - CONF_PATHS_NULSTR("systemd/system.conf.d") : - CONF_PATHS_NULSTR("systemd/user.conf.d"); + if (arg_system) { + files = STRV_MAKE_CONST(PKGSYSCONFDIR "/system.conf"); + dirs = (const char* const*) CONF_PATHS_STRV("systemd"); + suffix = "system.conf.d"; + } else { + r = manager_find_user_config_paths(&_free_files, &_free_dirs); + if (r < 0) + return log_error_errno(r, "Failed to determine config file paths: %m"); + files = (const char* const*) _free_files; + dirs = (const char* const*) _free_dirs; + suffix = "user.conf.d"; + } - (void) config_parse_many_nulstr( - fn, conf_dirs_nulstr, + (void) config_parse_many( + files, dirs, suffix, "Manager\0", config_item_table_lookup, items, CONFIG_PARSE_WARN, NULL, NULL); - /* Traditionally "0" was used to turn off the default unit timeouts. Fix this up so that we used USEC_INFINITY - * like everywhere else. */ + /* Traditionally "0" was used to turn off the default unit timeouts. Fix this up so that we use + * USEC_INFINITY like everywhere else. */ if (arg_default_timeout_start_usec <= 0) arg_default_timeout_start_usec = USEC_INFINITY; if (arg_default_timeout_stop_usec <= 0) From fa256f43e7ad6cafabc0ad89a0aee143670e1cca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 20 Feb 2021 19:25:32 +0100 Subject: [PATCH 03/12] basic/env-util: add putenv_dup() --- src/basic/env-util.c | 15 +++++++++++++++ src/basic/env-util.h | 3 +++ src/test/test-env-util.c | 14 ++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index df24cb935d..7fa598a3b5 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -768,6 +768,21 @@ int set_unset_env(const char *name, const char *value, bool overwrite) { return 0; } +int putenv_dup(const char *assignment, bool override) { + const char *e, *n; + + e = strchr(assignment, '='); + if (!e) + return -EINVAL; + + n = strndupa(assignment, e - assignment); + + /* This is like putenv(), but uses setenv() so that our memory doesn't become part of environ[]. */ + if (setenv(n, e + 1, override) < 0) + return -errno; + return 0; +} + int setenv_systemd_exec_pid(bool update_only) { char str[DECIMAL_STR_MAX(pid_t)]; const char *e; diff --git a/src/basic/env-util.h b/src/basic/env-util.h index 7ba8488af7..1fbe7e4270 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -58,6 +58,9 @@ int getenv_bool_secure(const char *p); /* Like setenv, but calls unsetenv if value == NULL. */ int set_unset_env(const char *name, const char *value, bool overwrite); +/* Like putenv, but duplicates the memory like setenv. */ +int putenv_dup(const char *assignment, bool override); + int setenv_systemd_exec_pid(bool update_only); /* Parses and does sanity checks on an environment variable containing diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index 77b8952f83..1e2e936592 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -361,6 +361,19 @@ static void test_env_assignment_is_valid(void) { assert_se(!env_assignment_is_valid("głąb=printf \"\x1b]0;\x07\"")); } +static void test_putenv_dup(void) { + log_info("/* %s */", __func__); + + assert_se(putenv_dup("A=a1", true) == 0); + assert_se(streq(getenv("A"), "a1")); + assert_se(putenv_dup("A=a1", true) == 0); + assert_se(streq(getenv("A"), "a1")); + assert_se(putenv_dup("A=a2", false) == 0); + assert_se(streq(getenv("A"), "a1")); + assert_se(putenv_dup("A=a2", true) == 0); + assert_se(streq(getenv("A"), "a2")); +} + static void test_setenv_systemd_exec_pid(void) { _cleanup_free_ char *saved = NULL; const char *e; @@ -416,6 +429,7 @@ int main(int argc, char *argv[]) { test_env_name_is_valid(); test_env_value_is_valid(); test_env_assignment_is_valid(); + test_putenv_dup(); test_setenv_systemd_exec_pid(); return 0; From d55ed7de345e2487152a04c8667c1b0852f0c3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 00:06:26 +0100 Subject: [PATCH 04/12] manager: add ManagerEnvironment configuration setting This is useful for various variables that modify process behaviour. This makes it easy to set it for pid1 without touching the kernel command line. Even for the *user manager* this also can be convenient for the unprivileged user, who cannot modify user@.service definition. Variables that could be set like this include $SD_EVENT_PROFILE_DELAYS, $SYSTEMD_FALLBACK_HOSTNAME, $SYSTEMD_MEMPOOL, $SYSTMED_RDRAND, etc. --- man/systemd-system.conf.xml | 23 ++++++++++++++++++----- src/core/main.c | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 0811e1909e..d39928ec23 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -323,11 +323,10 @@ DefaultEnvironment= - Sets manager environment variables passed to - all executed processes. Takes a space-separated list of - variable assignments. See - environ7 - for details about environment variables. + Configures environment variables passed to all executed processes. Takes a + space-separated list of variable assignments. See environ7 for + details about environment variables. Example: @@ -339,6 +338,20 @@ VAR3. + + ManagerEnvironment= + + Takes the same arguments as DefaultEnvironment=, see above. Sets + environment variables just for the manager process itself. These variables are not inherited by + processes spawned by the service manager, use DefaultEnvironment= for that. Note + that these variables are merged into the existing environment block. In particular, in case of the + system manager, this includes variables set by the kernel based on the kernel command line. + + Setting environment variables for the manager process may be useful to modify its behaviour. + See ENVIRONMENT for a descriptions of some + variables understood by systemd. + + DefaultCPUAccounting= DefaultBlockIOAccounting= diff --git a/src/core/main.c b/src/core/main.c index 2237925209..12ac2ba3c6 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -134,6 +134,7 @@ static usec_t arg_kexec_watchdog; static char *arg_early_core_pattern; static char *arg_watchdog_device; static char **arg_default_environment; +static char **arg_manager_environment; static struct rlimit *arg_default_rlimit[_RLIMIT_MAX]; static uint64_t arg_capability_bounding_set; static bool arg_no_new_privs; @@ -670,6 +671,7 @@ static int parse_config_file(void) { { "Manager", "DefaultStartLimitIntervalSec", config_parse_sec, 0, &arg_default_start_limit_interval }, { "Manager", "DefaultStartLimitBurst", config_parse_unsigned, 0, &arg_default_start_limit_burst }, { "Manager", "DefaultEnvironment", config_parse_environ, 0, &arg_default_environment }, + { "Manager", "ManagerEnvironment", config_parse_environ, 0, &arg_manager_environment }, { "Manager", "DefaultLimitCPU", config_parse_rlimit, RLIMIT_CPU, arg_default_rlimit }, { "Manager", "DefaultLimitFSIZE", config_parse_rlimit, RLIMIT_FSIZE, arg_default_rlimit }, { "Manager", "DefaultLimitDATA", config_parse_rlimit, RLIMIT_DATA, arg_default_rlimit }, @@ -2301,6 +2303,19 @@ static void fallback_rlimit_memlock(const struct rlimit *saved_rlimit_memlock) { arg_default_rlimit[RLIMIT_MEMLOCK] = rl; } +static void setenv_manager_environment(void) { + char **p; + int r; + + STRV_FOREACH(p, arg_manager_environment) { + log_debug("Setting '%s' in our own environment.", *p); + + r = putenv_dup(*p, true); + if (r < 0) + log_warning_errno(errno, "Failed to setenv \"%s\", ignoring: %m", *p); + } +} + static void reset_arguments(void) { /* Frees/resets arg_* variables, with a few exceptions commented below. */ @@ -2334,6 +2349,7 @@ static void reset_arguments(void) { arg_watchdog_device = NULL; arg_default_environment = strv_free(arg_default_environment); + arg_manager_environment = strv_free(arg_manager_environment); rlimit_free_all(arg_default_rlimit); arg_capability_bounding_set = CAP_ALL; @@ -2395,6 +2411,9 @@ static int parse_configuration(const struct rlimit *saved_rlimit_nofile, if (arg_show_status == _SHOW_STATUS_INVALID) arg_show_status = SHOW_STATUS_YES; + /* Push variables into the manager environment block */ + setenv_manager_environment(); + return 0; } From aa96ef86a75bd424c47f6267a2bd20558615f6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 11:02:52 +0100 Subject: [PATCH 05/12] man: use ~/.config instead of XDG_CONFIG_HOME e3820eeaf11f3b4614cbdfbc85675bc16a486e21 did that replacement XDG_CONFIG_HOME, in one of two places. Let's use ~/.config everywhere. Quoting https://github.com/systemd/systemd/pull/18704#discussion_r579465254: > I'd really drop XDG_CONFIG_HOME from the docs. It's confusing enough as it > is. Where we don't need the indirections we should not confuse people with > it, in particular as people might then think it's actually a good idea to use > that env var and redirect things. I'd just show the literal path everywhere, > even if we internally use the env var. --- man/systemd.unit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index da350844bd..f72e203295 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -60,7 +60,7 @@ $XDG_RUNTIME_DIR/systemd/user.control/* $XDG_RUNTIME_DIR/systemd/transient/* $XDG_RUNTIME_DIR/systemd/generator.early/* -$XDG_CONFIG_HOME/systemd/user/* +~/.config/systemd/user/* $XDG_CONFIG_DIRS/systemd/user/* /etc/systemd/user/* $XDG_RUNTIME_DIR/systemd/user/* From addddf565b982ba35cf2be290fb1a5e036f2567f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 15:08:07 +0100 Subject: [PATCH 06/12] os-release: add the DEFAULT_HOSTNAME= setting The motivation is that variants of the same distro that share the same compiled rpm want to customize various aspects of the system, in particular the hostname. In some sense the default hostname is part of the identity of the system, so setting it through os-release makes sense. In particular, instead of setting a default value in /etc/hostname, the appropriate default can be baked into the image, leaving /etc/hostname for local overrides only. Why make this a separate field instead of e.g. using NAME from os-release? NAME is already used for other purposes, and it seems likely that people want to set those independently. --- man/os-release.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/man/os-release.xml b/man/os-release.xml index 5a0cfd2887..a6d333f278 100644 --- a/man/os-release.xml +++ b/man/os-release.xml @@ -317,6 +317,23 @@ + + DEFAULT_HOSTNAME= + + A string specifying the hostname if + hostname5 is not + present and no other configuration source specifies the hostname. Must be either a single DNS label + (a string composed of 7-bit ASCII lower-case characters and no spaces or dots, limited to the format + allowed for DNS domain name labels), or a sequence of such labels separated by single dots that forms + a valid DNS FQDN. The total length must be at most 64 characters. + + See + org.freedesktop.hostname15 + for a description of how + systemd-hostnamed.service8 + determines the fallback hostname. + + SYSEXT_LEVEL= From fd03a6a92bc2c4eae2165cbfcbcd93d82cd28ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 16:36:07 +0100 Subject: [PATCH 07/12] sysext: move extension_release_validate() out of os-util.c --- src/shared/extension-release.c | 79 ++++++++++++++++++++++++++++++++++ src/shared/extension-release.h | 12 ++++++ src/shared/meson.build | 2 + src/shared/os-util.c | 71 ------------------------------ src/shared/os-util.h | 5 --- src/sysext/sysext.c | 1 + 6 files changed, 94 insertions(+), 76 deletions(-) create mode 100644 src/shared/extension-release.c create mode 100644 src/shared/extension-release.h diff --git a/src/shared/extension-release.c b/src/shared/extension-release.c new file mode 100644 index 0000000000..5eecf5a323 --- /dev/null +++ b/src/shared/extension-release.c @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "alloc-util.h" +#include "env-util.h" +#include "extension-release.h" +#include "log.h" +#include "os-util.h" +#include "strv.h" + +int extension_release_validate( + const char *name, + const char *host_os_release_id, + const char *host_os_release_version_id, + const char *host_os_release_sysext_level, + char **extension_release) { + + const char *extension_release_id = NULL, *extension_release_sysext_level = NULL; + + assert(name); + assert(!isempty(host_os_release_id)); + + /* Now that we can look into the extension image, let's see if the OS version is compatible */ + if (strv_isempty(extension_release)) { + log_debug("Extension '%s' carries no extension-release data, ignoring extension.", name); + return 0; + } + + extension_release_id = strv_env_pairs_get(extension_release, "ID"); + if (isempty(extension_release_id)) { + log_debug("Extension '%s' does not contain ID in extension-release but requested to match '%s'", + name, strna(host_os_release_id)); + return 0; + } + + if (!streq_ptr(host_os_release_id, extension_release_id)) { + log_debug("Extension '%s' is for OS '%s', but deployed on top of '%s'.", + name, strna(extension_release_id), strna(host_os_release_id)); + return 0; + } + + /* Rolling releases do not typically set VERSION_ID (eg: ArchLinux) */ + if (isempty(host_os_release_version_id) && isempty(host_os_release_sysext_level)) { + log_debug("No version info on the host (rolling release?), but ID in %s matched.", name); + return 1; + } + + /* If the extension has a sysext API level declared, then it must match the host API + * level. Otherwise, compare OS version as a whole */ + extension_release_sysext_level = strv_env_pairs_get(extension_release, "SYSEXT_LEVEL"); + if (!isempty(host_os_release_sysext_level) && !isempty(extension_release_sysext_level)) { + if (!streq_ptr(host_os_release_sysext_level, extension_release_sysext_level)) { + log_debug("Extension '%s' is for sysext API level '%s', but running on sysext API level '%s'", + name, strna(extension_release_sysext_level), strna(host_os_release_sysext_level)); + return 0; + } + } else if (!isempty(host_os_release_version_id)) { + const char *extension_release_version_id; + + extension_release_version_id = strv_env_pairs_get(extension_release, "VERSION_ID"); + if (isempty(extension_release_version_id)) { + log_debug("Extension '%s' does not contain VERSION_ID in extension-release but requested to match '%s'", + name, strna(host_os_release_version_id)); + return 0; + } + + if (!streq_ptr(host_os_release_version_id, extension_release_version_id)) { + log_debug("Extension '%s' is for OS '%s', but deployed on top of '%s'.", + name, strna(extension_release_version_id), strna(host_os_release_version_id)); + return 0; + } + } else if (isempty(host_os_release_version_id) && isempty(host_os_release_sysext_level)) { + /* Rolling releases do not typically set VERSION_ID (eg: ArchLinux) */ + log_debug("No version info on the host (rolling release?), but ID in %s matched.", name); + return 1; + } + + log_debug("Version info of extension '%s' matches host.", name); + return 1; +} diff --git a/src/shared/extension-release.h b/src/shared/extension-release.h new file mode 100644 index 0000000000..5c77010b0e --- /dev/null +++ b/src/shared/extension-release.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +/* Given an image name (for logging purposes), a set of os-release values from the host and a key-value pair + * vector of extension-release variables, check that the distro and (system extension level or distro + * version) match and return 1, and 0 otherwise. */ +int extension_release_validate( + const char *name, + const char *host_os_release_id, + const char *host_os_release_version_id, + const char *host_os_release_sysext_level, + char **extension_release); diff --git a/src/shared/meson.build b/src/shared/meson.build index 6d9a5c5b65..dfa3275b04 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -100,6 +100,8 @@ shared_sources = files(''' exec-util.h exit-status.c exit-status.h + extension-release.c + extension-release.h fdset.c fdset.h fileio-label.c diff --git a/src/shared/os-util.c b/src/shared/os-util.c index 9aed3b5df4..3405c9aeaf 100644 --- a/src/shared/os-util.c +++ b/src/shared/os-util.c @@ -193,74 +193,3 @@ int load_extension_release_pairs(const char *root, const char *extension, char * return load_env_file_pairs(f, p, ret); } - -int extension_release_validate( - const char *name, - const char *host_os_release_id, - const char *host_os_release_version_id, - const char *host_os_release_sysext_level, - char **extension_release) { - - const char *extension_release_id = NULL, *extension_release_sysext_level = NULL; - - assert(name); - assert(!isempty(host_os_release_id)); - - /* Now that we can look into the extension image, let's see if the OS version is compatible */ - if (strv_isempty(extension_release)) { - log_debug("Extension '%s' carries no extension-release data, ignoring extension.", name); - return 0; - } - - extension_release_id = strv_env_pairs_get(extension_release, "ID"); - if (isempty(extension_release_id)) { - log_debug("Extension '%s' does not contain ID in extension-release but requested to match '%s'", - name, strna(host_os_release_id)); - return 0; - } - - if (!streq_ptr(host_os_release_id, extension_release_id)) { - log_debug("Extension '%s' is for OS '%s', but deployed on top of '%s'.", - name, strna(extension_release_id), strna(host_os_release_id)); - return 0; - } - - /* Rolling releases do not typically set VERSION_ID (eg: ArchLinux) */ - if (isempty(host_os_release_version_id) && isempty(host_os_release_sysext_level)) { - log_debug("No version info on the host (rolling release?), but ID in %s matched.", name); - return 1; - } - - /* If the extension has a sysext API level declared, then it must match the host API - * level. Otherwise, compare OS version as a whole */ - extension_release_sysext_level = strv_env_pairs_get(extension_release, "SYSEXT_LEVEL"); - if (!isempty(host_os_release_sysext_level) && !isempty(extension_release_sysext_level)) { - if (!streq_ptr(host_os_release_sysext_level, extension_release_sysext_level)) { - log_debug("Extension '%s' is for sysext API level '%s', but running on sysext API level '%s'", - name, strna(extension_release_sysext_level), strna(host_os_release_sysext_level)); - return 0; - } - } else if (!isempty(host_os_release_version_id)) { - const char *extension_release_version_id; - - extension_release_version_id = strv_env_pairs_get(extension_release, "VERSION_ID"); - if (isempty(extension_release_version_id)) { - log_debug("Extension '%s' does not contain VERSION_ID in extension-release but requested to match '%s'", - name, strna(host_os_release_version_id)); - return 0; - } - - if (!streq_ptr(host_os_release_version_id, extension_release_version_id)) { - log_debug("Extension '%s' is for OS '%s', but deployed on top of '%s'.", - name, strna(extension_release_version_id), strna(host_os_release_version_id)); - return 0; - } - } else if (isempty(host_os_release_version_id) && isempty(host_os_release_sysext_level)) { - /* Rolling releases do not typically set VERSION_ID (eg: ArchLinux) */ - log_debug("No version info on the host (rolling release?), but ID in %s matched.", name); - return 1; - } - - log_debug("Version info of extension '%s' matches host.", name); - return 1; -} diff --git a/src/shared/os-util.h b/src/shared/os-util.h index d4bff5e48e..bdb9e6adea 100644 --- a/src/shared/os-util.h +++ b/src/shared/os-util.h @@ -26,8 +26,3 @@ int parse_os_release(const char *root, ...) _sentinel_; int load_extension_release_pairs(const char *root, const char *extension, char ***ret); int load_os_release_pairs(const char *root, char ***ret); int load_os_release_pairs_with_prefix(const char *root, const char *prefix, char ***ret); - -/* Given an image name (for logging purposes), a set of os-release values from the host - * and a key-value pair vector of extension-release variables, check that the distro and - * (system extension level or distro version) match and return 1, and 0 otherwise. */ -int extension_release_validate(const char *name, const char *host_os_release_id, const char *host_os_release_version_id, const char *host_os_release_sysext_level, char **extension_release); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index cd824887bf..e06108d3a1 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -10,6 +10,7 @@ #include "dissect-image.h" #include "env-util.h" #include "escape.h" +#include "extension-release.h" #include "fd-util.h" #include "fileio.h" #include "format-table.h" From 6ef06723b1de8efc6cfe4bdaee8bc39220bdcb5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 16:51:18 +0100 Subject: [PATCH 08/12] Move os-util.[ch] to basic/ parse_os_release() will be used basic/hostname-util.c later on. --- src/basic/meson.build | 2 ++ src/{shared => basic}/os-util.c | 20 +++++++++++++++++++- src/{shared => basic}/os-util.h | 2 ++ src/machine/image-dbus.c | 1 + src/machine/machined-dbus.c | 1 + src/portable/portablectl.c | 2 +- src/portable/portabled-image-bus.c | 1 + src/shared/discover-image.c | 17 ----------------- src/shared/discover-image.h | 2 -- src/shared/meson.build | 2 -- 10 files changed, 27 insertions(+), 23 deletions(-) rename src/{shared => basic}/os-util.c (93%) rename src/{shared => basic}/os-util.h (96%) diff --git a/src/basic/meson.build b/src/basic/meson.build index 0b0982d543..bacd519a86 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -170,6 +170,8 @@ basic_sources = files(''' nulstr-util.h ordered-set.c ordered-set.h + os-util.c + os-util.h parse-util.c parse-util.h path-lookup.c diff --git a/src/shared/os-util.c b/src/basic/os-util.c similarity index 93% rename from src/shared/os-util.c rename to src/basic/os-util.c index 3405c9aeaf..e6f9f7c511 100644 --- a/src/shared/os-util.c +++ b/src/basic/os-util.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "alloc-util.h" -#include "discover-image.h" #include "env-file.h" #include "env-util.h" #include "fd-util.h" @@ -9,8 +8,27 @@ #include "fs-util.h" #include "macro.h" #include "os-util.h" +#include "path-util.h" #include "string-util.h" #include "strv.h" +#include "utf8.h" + +bool image_name_is_valid(const char *s) { + if (!filename_is_valid(s)) + return false; + + if (string_has_cc(s, NULL)) + return false; + + if (!utf8_is_valid(s)) + return false; + + /* Temporary files for atomically creating new files */ + if (startswith(s, ".#")) + return false; + + return true; +} int path_is_extension_tree(const char *path, const char *extension) { int r; diff --git a/src/shared/os-util.h b/src/basic/os-util.h similarity index 96% rename from src/shared/os-util.h rename to src/basic/os-util.h index bdb9e6adea..f7789c0804 100644 --- a/src/shared/os-util.h +++ b/src/basic/os-util.h @@ -6,6 +6,8 @@ /* The *_extension_release flavours will look for /usr/lib/extension-release/extension-release.NAME * in accordance with the OS extension specification, rather than for /usr/lib/ or /etc/os-release. */ +bool image_name_is_valid(const char *s) _pure_; + int path_is_extension_tree(const char *path, const char *extension); static inline int path_is_os_tree(const char *path) { return path_is_extension_tree(path, NULL); diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index 539b4c802a..309de0b116 100644 --- a/src/machine/image-dbus.c +++ b/src/machine/image-dbus.c @@ -18,6 +18,7 @@ #include "loop-util.h" #include "missing_capability.h" #include "mount-util.h" +#include "os-util.h" #include "process-util.h" #include "raw-clone.h" #include "strv.h" diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 08179eb400..bb6a2bc576 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -24,6 +24,7 @@ #include "machine-pool.h" #include "machined.h" #include "missing_capability.h" +#include "os-util.h" #include "path-util.h" #include "process-util.h" #include "stdio-util.h" diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index 278dae0e1b..af4acc2125 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -12,7 +12,6 @@ #include "bus-wait-for-jobs.h" #include "def.h" #include "dirent-util.h" -#include "discover-image.h" #include "env-file.h" #include "fd-util.h" #include "fileio.h" @@ -20,6 +19,7 @@ #include "fs-util.h" #include "locale-util.h" #include "main-func.h" +#include "os-util.h" #include "pager.h" #include "parse-util.h" #include "path-util.h" diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 018cc9b818..8332332c91 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -17,6 +17,7 @@ #include "fileio.h" #include "io-util.h" #include "missing_capability.h" +#include "os-util.h" #include "portable.h" #include "portabled-bus.h" #include "portabled-image-bus.h" diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index fbce201e29..fb59d1d704 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -1250,23 +1250,6 @@ int image_name_lock(const char *name, int operation, LockFile *ret) { return make_lock_file(p, operation, ret); } -bool image_name_is_valid(const char *s) { - if (!filename_is_valid(s)) - return false; - - if (string_has_cc(s, NULL)) - return false; - - if (!utf8_is_valid(s)) - return false; - - /* Temporary files for atomically creating new files */ - if (startswith(s, ".#")) - return false; - - return true; -} - bool image_in_search_path( ImageClass class, const char *root, diff --git a/src/shared/discover-image.h b/src/shared/discover-image.h index af67d167c4..3726e98d30 100644 --- a/src/shared/discover-image.h +++ b/src/shared/discover-image.h @@ -76,8 +76,6 @@ int image_read_only(Image *i, bool b); const char* image_type_to_string(ImageType t) _const_; ImageType image_type_from_string(const char *s) _pure_; -bool image_name_is_valid(const char *s) _pure_; - int image_path_lock(const char *path, int operation, LockFile *global, LockFile *local); int image_name_lock(const char *name, int operation, LockFile *ret); diff --git a/src/shared/meson.build b/src/shared/meson.build index dfa3275b04..4b27a95cc2 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -198,8 +198,6 @@ shared_sources = files(''' numa-util.h openssl-util.c openssl-util.h - os-util.c - os-util.h output-mode.c output-mode.h pager.c From 209c14705d5ea84aeaa597662af70d65edcc0231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 17:06:18 +0100 Subject: [PATCH 09/12] basic/os-util: make the sentinel implicit --- src/basic/os-util.c | 4 ++-- src/basic/os-util.h | 7 +++++-- src/core/main.c | 3 +-- src/firstboot/firstboot.c | 3 +-- src/hostname/hostnamed.c | 3 +-- src/journal-remote/journal-gatewayd.c | 2 +- src/shared/specifier.c | 2 +- src/sysext/sysext.c | 3 +-- 8 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index e6f9f7c511..3d199f028d 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -136,7 +136,7 @@ static int parse_release_internal(const char *root, const char *extension, va_li return parse_env_filev(f, p, ap); } -int parse_extension_release(const char *root, const char *extension, ...) { +int _parse_extension_release(const char *root, const char *extension, ...) { va_list ap; int r; @@ -147,7 +147,7 @@ int parse_extension_release(const char *root, const char *extension, ...) { return r; } -int parse_os_release(const char *root, ...) { +int _parse_os_release(const char *root, ...) { va_list ap; int r; diff --git a/src/basic/os-util.h b/src/basic/os-util.h index f7789c0804..271b35fa4e 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -23,8 +23,11 @@ static inline int fopen_os_release(const char *root, char **ret_path, FILE **ret return fopen_extension_release(root, NULL, ret_path, ret_file); } -int parse_extension_release(const char *root, const char *extension, ...) _sentinel_; -int parse_os_release(const char *root, ...) _sentinel_; +int _parse_extension_release(const char *root, const char *extension, ...) _sentinel_; +int _parse_os_release(const char *root, ...) _sentinel_; +#define parse_extension_release(root, extension, ...) _parse_extension_release(root, extension, __VA_ARGS__, NULL) +#define parse_os_release(root, ...) _parse_os_release(root, __VA_ARGS__, NULL) + int load_extension_release_pairs(const char *root, const char *extension, char ***ret); int load_os_release_pairs(const char *root, char ***ret); int load_os_release_pairs_with_prefix(const char *root, const char *prefix, char ***ret); diff --git a/src/core/main.c b/src/core/main.c index 12ac2ba3c6..1b47226d73 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1352,8 +1352,7 @@ static int status_welcome(void) { r = parse_os_release(NULL, "PRETTY_NAME", &pretty_name, - "ANSI_COLOR", &ansi_color, - NULL); + "ANSI_COLOR", &ansi_color); if (r < 0) log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r, "Failed to read os-release file, ignoring: %m"); diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 40f0de28c2..c0e88e7915 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -106,8 +106,7 @@ static void print_welcome(void) { r = parse_os_release( arg_root, "PRETTY_NAME", &pretty_name, - "ANSI_COLOR", &ansi_color, - NULL); + "ANSI_COLOR", &ansi_color); if (r < 0) log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r, "Failed to read os-release file, ignoring: %m"); diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 34333a6530..eb06957397 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -159,8 +159,7 @@ static void context_read_os_release(Context *c) { r = parse_os_release(NULL, "PRETTY_NAME", &c->data[PROP_OS_PRETTY_NAME], "CPE_NAME", &c->data[PROP_OS_CPE_NAME], - "HOME_URL", &c->data[PROP_OS_HOME_URL], - NULL); + "HOME_URL", &c->data[PROP_OS_HOME_URL]); if (r < 0 && r != -ENOENT) log_warning_errno(r, "Failed to read os-release file, ignoring: %m"); diff --git a/src/journal-remote/journal-gatewayd.c b/src/journal-remote/journal-gatewayd.c index 4ece66b51b..bd1edb1797 100644 --- a/src/journal-remote/journal-gatewayd.c +++ b/src/journal-remote/journal-gatewayd.c @@ -755,7 +755,7 @@ static int request_handler_machine( if (r < 0) return mhd_respondf(connection, r, MHD_HTTP_INTERNAL_SERVER_ERROR, "Failed to determine disk usage: %m"); - (void) parse_os_release(NULL, "PRETTY_NAME", &os_name, NULL); + (void) parse_os_release(NULL, "PRETTY_NAME", &os_name); (void) get_virtualization(&v); r = asprintf(&json, diff --git a/src/shared/specifier.c b/src/shared/specifier.c index 86731f8cd2..24e8781805 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -203,7 +203,7 @@ static int specifier_os_release_common(const char *field, char **ret) { char *t = NULL; int r; - r = parse_os_release(NULL, field, &t, NULL); + r = parse_os_release(NULL, field, &t); if (r < 0) return r; if (!t) { diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index e06108d3a1..60f4dee353 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -473,8 +473,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) { arg_root, "ID", &host_os_release_id, "VERSION_ID", &host_os_release_version_id, - "SYSEXT_LEVEL", &host_os_release_sysext_level, - NULL); + "SYSEXT_LEVEL", &host_os_release_sysext_level); if (r < 0) return log_error_errno(r, "Failed to acquire 'os-release' data of OS tree '%s': %m", empty_to_root(arg_root)); From 05c6f341b1fc18f59293d115bb4cbee94f8bf3a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 15 Feb 2021 11:50:55 +0100 Subject: [PATCH 10/12] Allow the fallback hostname to be overriden using an environment variable See https://bugzilla.redhat.com/show_bug.cgi?id=1893417 for the back story: the fallback hostname matters a lot in certain environments. Right now the only way to configure the fallback hostname is by recompiling systemd, which is obviously problematic in case when the fallback hostname shall differ between different editions of the same distro that share a single compiled rpm. By making this overridable through an envvar, we're providing an escape hatch without making this a top-level api. Later on a way to set this through os-release is added, but I think the approach with the variable is still useful. It it very convenient for testing, or to override settings only in a particular service, etc. --- docs/ENVIRONMENT.md | 4 ++++ src/basic/hostname-util.c | 21 ++++++++++++++++-- src/basic/hostname-util.h | 1 + src/hostname/hostnamed.c | 40 ++++++++++++++++++++++++++-------- src/resolve/resolved-manager.c | 33 +++++++++++++++------------- src/shared/hostname-setup.c | 7 ++++-- src/test/test-hostname-util.c | 20 +++++++++++++---- 7 files changed, 94 insertions(+), 32 deletions(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 82bb52b344..07a65864bd 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -55,6 +55,10 @@ All tools: * `$SYSTEMD_EFI_OPTIONS` — if set, used instead of the string in the SystemdOptions EFI variable. Analogous to `$SYSTEMD_PROC_CMDLINE`. +* `$SYSTEMD_DEFAULT_HOSTNAME` — override the compiled-in fallback hostname + (relevant in particular for the system manager and `systemd-hostnamed`). + Must be a valid hostname (either a single label or a FQDN). + * `$SYSTEMD_IN_INITRD=[auto|lenient|0|1]` — if set, specifies initrd detection method. Defaults to `auto`. Behavior is defined as follows: `auto`: Checks if `/etc/initrd-release` exists, and a temporary fs is mounted diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index d7aba2c263..5109605e0c 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -3,14 +3,27 @@ #include #include #include +#include #include #include #include "alloc-util.h" #include "hostname-util.h" +#include "os-util.h" #include "string-util.h" #include "strv.h" +char* get_default_hostname(void) { + const char *e = secure_getenv("SYSTEMD_DEFAULT_HOSTNAME"); + if (e) { + if (hostname_is_valid(e, 0)) + return strdup(e); + log_debug("Invalid hostname in $SYSTEMD_DEFAULT_HOSTNAME, ignoring: %s", e); + } + + return strdup(FALLBACK_HOSTNAME); +} + char* gethostname_malloc(void) { struct utsname u; const char *s; @@ -23,7 +36,7 @@ char* gethostname_malloc(void) { s = u.nodename; if (isempty(s) || streq(s, "(none)")) - s = FALLBACK_HOSTNAME; + return get_default_hostname(); return strdup(s); } @@ -31,6 +44,7 @@ char* gethostname_malloc(void) { char* gethostname_short_malloc(void) { struct utsname u; const char *s; + _cleanup_free_ char *f = NULL; /* Like above, but kills the FQDN part if present. */ @@ -38,7 +52,10 @@ char* gethostname_short_malloc(void) { s = u.nodename; if (isempty(s) || streq(s, "(none)") || s[0] == '.') { - s = FALLBACK_HOSTNAME; + s = f = get_default_hostname(); + if (!s) + return NULL; + assert(s[0] != '.'); } diff --git a/src/basic/hostname-util.h b/src/basic/hostname-util.h index 6cff9c1d4c..576ca083e0 100644 --- a/src/basic/hostname-util.h +++ b/src/basic/hostname-util.h @@ -7,6 +7,7 @@ #include "macro.h" #include "strv.h" +char* get_default_hostname(void); char* gethostname_malloc(void); char* gethostname_short_malloc(void); int gethostname_strict(char **ret); diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index eb06957397..9937d159f4 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -322,6 +322,7 @@ static int context_update_kernel_hostname( Context *c, const char *transient_hn) { + _cleanup_free_ char *_hn_free = NULL; const char *hn; HostnameSource hns; int r; @@ -340,7 +341,10 @@ static int context_update_kernel_hostname( /* ... and the ultimate fallback */ } else { - hn = FALLBACK_HOSTNAME; + hn = _hn_free = get_default_hostname(); + if (!hn) + return log_oom(); + hns = HOSTNAME_FALLBACK; } @@ -502,16 +506,20 @@ static int property_get_hostname( void *userdata, sd_bus_error *error) { - _cleanup_free_ char *current = NULL; + _cleanup_free_ char *hn = NULL; int r; - r = gethostname_strict(¤t); - if (r == -ENXIO) - return sd_bus_message_append(reply, "s", FALLBACK_HOSTNAME); - if (r < 0) - return r; + r = gethostname_strict(&hn); + if (r < 0) { + if (r != -ENXIO) + return r; - return sd_bus_message_append(reply, "s", current); + hn = get_default_hostname(); + if (!hn) + return -ENOMEM; + } + + return sd_bus_message_append(reply, "s", hn); } static int property_get_static_hostname( @@ -531,7 +539,21 @@ static int property_get_static_hostname( return sd_bus_message_append(reply, "s", c->data[PROP_STATIC_HOSTNAME]); } -static BUS_DEFINE_PROPERTY_GET_GLOBAL(property_get_fallback_hostname, "s", FALLBACK_HOSTNAME); +static int property_get_fallback_hostname( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + _cleanup_free_ char *hn = get_default_hostname(); + if (!hn) + return log_oom(); + + return sd_bus_message_append(reply, "s", hn); +} static int property_get_hostname_source( sd_bus *bus, diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 2ed9f2d47b..ae46d2dcd8 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -442,20 +442,25 @@ static int determine_hostname(char **full_hostname, char **llmnr_hostname, char return 0; } -static const char *fallback_hostname(void) { +static char* fallback_hostname(void) { - /* Determine the fall back hostname. For exposing this system to the outside world, we cannot have it to be - * "localhost" even if that's the compiled in hostname. In this case, let's revert to "linux" instead. */ + /* Determine the fall back hostname. For exposing this system to the outside world, we cannot have it + * to be "localhost" even if that's the default hostname. In this case, let's revert to "linux" + * instead. */ - if (is_localhost(FALLBACK_HOSTNAME)) - return "linux"; + _cleanup_free_ char *n = get_default_hostname(); + if (!n) + return NULL; - return FALLBACK_HOSTNAME; + if (is_localhost(n)) + return strdup("linux"); + + return TAKE_PTR(n); } static int make_fallback_hostnames(char **full_hostname, char **llmnr_hostname, char **mdns_hostname) { - _cleanup_free_ char *n = NULL, *m = NULL; - char label[DNS_LABEL_MAX], *h; + _cleanup_free_ char *h = NULL, *n = NULL, *m = NULL; + char label[DNS_LABEL_MAX]; const char *p; int r; @@ -463,7 +468,10 @@ static int make_fallback_hostnames(char **full_hostname, char **llmnr_hostname, assert(llmnr_hostname); assert(mdns_hostname); - p = fallback_hostname(); + p = h = fallback_hostname(); + if (!h) + return log_oom(); + r = dns_label_unescape(&p, label, sizeof label, 0); if (r < 0) return log_error_errno(r, "Failed to unescape fallback hostname: %m"); @@ -478,14 +486,9 @@ static int make_fallback_hostnames(char **full_hostname, char **llmnr_hostname, if (r < 0) return log_error_errno(r, "Failed to concatenate mDNS hostname: %m"); - h = strdup(fallback_hostname()); - if (!h) - return log_oom(); - *llmnr_hostname = TAKE_PTR(n); *mdns_hostname = TAKE_PTR(m); - - *full_hostname = h; + *full_hostname = TAKE_PTR(h); return 0; } diff --git a/src/shared/hostname-setup.c b/src/shared/hostname-setup.c index 5cdcdbd93c..0cc1a268bd 100644 --- a/src/shared/hostname-setup.c +++ b/src/shared/hostname-setup.c @@ -194,7 +194,7 @@ int hostname_setup(bool really) { } } - if (isempty(hn)) { + if (!hn) { /* Don't override the hostname if it is already set and not explicitly configured */ char buf[HOST_NAME_MAX + 1] = {}; @@ -206,7 +206,10 @@ int hostname_setup(bool really) { if (enoent) log_info("No hostname configured, using fallback hostname."); - hn = FALLBACK_HOSTNAME; + hn = b = get_default_hostname(); + if (!hn) + return log_oom(); + source = HOSTNAME_FALLBACK; } diff --git a/src/test/test-hostname-util.c b/src/test/test-hostname-util.c index 24c8ed9e3b..6d62958d67 100644 --- a/src/test/test-hostname-util.c +++ b/src/test/test-hostname-util.c @@ -10,6 +10,8 @@ #include "tmpfile-util.h" static void test_hostname_is_valid(void) { + log_info("/* %s */", __func__); + assert_se(hostname_is_valid("foobar", 0)); assert_se(hostname_is_valid("foobar.com", 0)); assert_se(!hostname_is_valid("foobar.com.", 0)); @@ -49,6 +51,8 @@ static void test_hostname_is_valid(void) { static void test_hostname_cleanup(void) { char *s; + log_info("/* %s */", __func__); + s = strdupa("foobar"); assert_se(streq(hostname_cleanup(s), "foobar")); s = strdupa("foobar.com"); @@ -94,6 +98,8 @@ static void test_hostname_cleanup(void) { static void test_hostname_malloc(void) { _cleanup_free_ char *h = NULL, *l = NULL; + log_info("/* %s */", __func__); + assert_se(h = gethostname_malloc()); log_info("hostname_malloc: \"%s\"", h); @@ -101,21 +107,27 @@ static void test_hostname_malloc(void) { log_info("hostname_short_malloc: \"%s\"", l); } -static void test_fallback_hostname(void) { +static void test_default_hostname(void) { + log_info("/* %s */", __func__); + if (!hostname_is_valid(FALLBACK_HOSTNAME, 0)) { log_error("Configured fallback hostname \"%s\" is not valid.", FALLBACK_HOSTNAME); exit(EXIT_FAILURE); } + + _cleanup_free_ char *n = get_default_hostname(); + assert_se(n); + log_info("get_default_hostname: \"%s\"", n); + assert_se(hostname_is_valid(n, 0)); } int main(int argc, char *argv[]) { - test_setup_logging(LOG_INFO); + test_setup_logging(LOG_DEBUG); test_hostname_is_valid(); test_hostname_cleanup(); test_hostname_malloc(); - - test_fallback_hostname(); + test_default_hostname(); return 0; } From e7637751c4d3dbdaf9e5e13801d912389c8b7f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 19 Feb 2021 17:48:20 +0100 Subject: [PATCH 11/12] Use the DEFAULT_HOSTNAME field from os-release This provides a fairly comprehensible fix for https://bugzilla.redhat.com/show_bug.cgi?id=1893417. This adds yet-another level of configuration: - /etc/hostname - transient hostname - $SYSTEMD_DEFAULT_HOSTNAME - DEFAULT_HOSTNAME is os-release - -Dfallback-hostname= - "linux" It's a lot of layers, but each has it's own justification. --- src/basic/hostname-util.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/basic/hostname-util.c b/src/basic/hostname-util.c index 5109605e0c..29d69910d2 100644 --- a/src/basic/hostname-util.c +++ b/src/basic/hostname-util.c @@ -14,6 +14,8 @@ #include "strv.h" char* get_default_hostname(void) { + int r; + const char *e = secure_getenv("SYSTEMD_DEFAULT_HOSTNAME"); if (e) { if (hostname_is_valid(e, 0)) @@ -21,6 +23,16 @@ char* get_default_hostname(void) { log_debug("Invalid hostname in $SYSTEMD_DEFAULT_HOSTNAME, ignoring: %s", e); } + _cleanup_free_ char *f = NULL; + r = parse_os_release(NULL, "DEFAULT_HOSTNAME", &f); + if (r < 0) + log_debug_errno(r, "Failed to parse os-release, ignoring: %m"); + else if (f) { + if (hostname_is_valid(f, 0)) + return TAKE_PTR(f); + log_debug("Invalid hostname in os-release, ignoring: %s", f); + } + return strdup(FALLBACK_HOSTNAME); } From 8770c8135bb2ddd7926a227c41d25cba805eee3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 20 Feb 2021 18:54:24 +0100 Subject: [PATCH 12/12] hostnamed: rename FallbackHostname to DefaultHostname This follows the addition of DEFAULT_HOSTNAME= in os-release. The distinction between the value from os-release or the env var and the compile-time setting is not made in the api: HostnameSource is "default" is all cases. I think that this level of detail is not needed, because the users of this mostly care whether the hostname was set by user configuration or not. --- NEWS | 4 ++-- man/org.freedesktop.hostname1.xml | 16 +++++++++------- src/hostname/hostnamed.c | 14 +++++++------- src/shared/hostname-setup.c | 14 +++++++------- src/shared/hostname-setup.h | 2 +- 5 files changed, 26 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index 8fc5ce73ff..b917262495 100644 --- a/NEWS +++ b/NEWS @@ -184,8 +184,8 @@ CHANGES WITH 248: ignored. The goal is to honour configuration as specified by the user. - * systemd-hostnamed now exports the fallback hostname and the source of - the configured hostname ("static", "transient", or "fallback") as + * systemd-hostnamed now exports the default hostname and the source of + the configured hostname ("static", "transient", or "default") as D-Bus properties. * systemd-hostnamed now exports the HardwareVendor and HardwareModel diff --git a/man/org.freedesktop.hostname1.xml b/man/org.freedesktop.hostname1.xml index 5b5f4fb3a7..3a5088eded 100644 --- a/man/org.freedesktop.hostname1.xml +++ b/man/org.freedesktop.hostname1.xml @@ -63,7 +63,7 @@ node /org/freedesktop/hostname1 { readonly s StaticHostname = '...'; readonly s PrettyHostname = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") - readonly s FallbackHostname = '...'; + readonly s DefaultHostname = '...'; readonly s HostnameSource = '...'; readonly s IconName = '...'; readonly s Chassis = '...'; @@ -124,7 +124,7 @@ node /org/freedesktop/hostname1 { - + @@ -183,13 +183,15 @@ node /org/freedesktop/hostname1 { set this setting will be the empty string. Applications should then find a suitable fallback, such as the dynamic hostname. - The FallbackHostname property exposes the fallback hostname (configured at - compilation time). + The DefaultHostname property exposes the default hostname (configured through + os-release5, or a + fallback set at compilation time). The HostnameSource property exposes the origin of the currently configured hostname. One of static (set from /etc/hostname), transient (a non-permanent hostname from an external source), - fallback (the compiled-in fallback value). + default (the value from os-release or the the compiled-in + fallback). The IconName property exposes the icon name following the XDG icon naming spec. If not set, information such as the chassis type (see below) is used to find a @@ -332,8 +334,8 @@ node /org/freedesktop/hostname1 { Limit the hostname to 63 chars, which is the length of a DNS label. If after stripping special chars the empty string is the result, you can pass this - as-is to systemd-hostnamed in which case it will automatically use - &FALLBACK_HOSTNAME;. + as-is to systemd-hostnamed in which case it will automatically use a suitable + fallback. Uppercase charaacters should be replaced with their lowercase equivalents. diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 9937d159f4..2ece8b2fbd 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -345,7 +345,7 @@ static int context_update_kernel_hostname( if (!hn) return log_oom(); - hns = HOSTNAME_FALLBACK; + hns = HOSTNAME_DEFAULT; } r = sethostname_idempotent(hn); @@ -539,7 +539,7 @@ static int property_get_static_hostname( return sd_bus_message_append(reply, "s", c->data[PROP_STATIC_HOSTNAME]); } -static int property_get_fallback_hostname( +static int property_get_default_hostname( sd_bus *bus, const char *path, const char *interface, @@ -581,16 +581,16 @@ static int property_get_hostname_source( else { /* If the hostname was not set by us, try to figure out where it came from. If we set - * it to the fallback hostname, the file will tell us. We compare the string because + * it to the default hostname, the file will tell us. We compare the string because * it is possible that the hostname was set by an older version that had a different * fallback, in the initramfs or before we reexecuted. */ - r = read_one_line_file("/run/systemd/fallback-hostname", &fallback); + r = read_one_line_file("/run/systemd/default-hostname", &fallback); if (r < 0 && r != -ENOENT) - log_warning_errno(r, "Failed to read /run/systemd/fallback-hostname, ignoring: %m"); + log_warning_errno(r, "Failed to read /run/systemd/default-hostname, ignoring: %m"); if (streq_ptr(fallback, hostname)) - c->hostname_source = HOSTNAME_FALLBACK; + c->hostname_source = HOSTNAME_DEFAULT; else c->hostname_source = HOSTNAME_TRANSIENT; } @@ -988,7 +988,7 @@ static const sd_bus_vtable hostname_vtable[] = { SD_BUS_PROPERTY("Hostname", "s", property_get_hostname, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("StaticHostname", "s", property_get_static_hostname, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("PrettyHostname", "s", property_get_machine_info_field, offsetof(Context, data) + sizeof(char*) * PROP_PRETTY_HOSTNAME, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("FallbackHostname", "s", property_get_fallback_hostname, 0, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("DefaultHostname", "s", property_get_default_hostname, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("HostnameSource", "s", property_get_hostname_source, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("IconName", "s", property_get_icon_name, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Chassis", "s", property_get_chassis, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/shared/hostname-setup.c b/src/shared/hostname-setup.c index 0cc1a268bd..511aa7d031 100644 --- a/src/shared/hostname-setup.c +++ b/src/shared/hostname-setup.c @@ -152,13 +152,13 @@ void hostname_update_source_hint(const char *hostname, HostnameSource source) { * notice if somebody sets the hostname directly (not going through hostnamed). */ - if (source == HOSTNAME_FALLBACK) { - r = write_string_file("/run/systemd/fallback-hostname", hostname, + if (source == HOSTNAME_DEFAULT) { + r = write_string_file("/run/systemd/default-hostname", hostname, WRITE_STRING_FILE_CREATE | WRITE_STRING_FILE_ATOMIC); if (r < 0) - log_warning_errno(r, "Failed to create \"/run/systemd/fallback-hostname\": %m"); + log_warning_errno(r, "Failed to create \"/run/systemd/default-hostname\": %m"); } else - unlink_or_warn("/run/systemd/fallback-hostname"); + unlink_or_warn("/run/systemd/default-hostname"); } int hostname_setup(bool really) { @@ -204,13 +204,13 @@ int hostname_setup(bool really) { } if (enoent) - log_info("No hostname configured, using fallback hostname."); + log_info("No hostname configured, using default hostname."); hn = b = get_default_hostname(); if (!hn) return log_oom(); - source = HOSTNAME_FALLBACK; + source = HOSTNAME_DEFAULT; } @@ -233,7 +233,7 @@ int hostname_setup(bool really) { static const char* const hostname_source_table[] = { [HOSTNAME_STATIC] = "static", [HOSTNAME_TRANSIENT] = "transient", - [HOSTNAME_FALLBACK] = "fallback", + [HOSTNAME_DEFAULT] = "default", }; DEFINE_STRING_TABLE_LOOKUP(hostname_source, HostnameSource); diff --git a/src/shared/hostname-setup.h b/src/shared/hostname-setup.h index 1fa0b6f333..5ac7241a59 100644 --- a/src/shared/hostname-setup.h +++ b/src/shared/hostname-setup.h @@ -7,7 +7,7 @@ typedef enum HostnameSource { HOSTNAME_STATIC, /* from /etc/hostname */ HOSTNAME_TRANSIENT, /* a transient hostname set through systemd, hostnamed, the container manager, or otherwise */ - HOSTNAME_FALLBACK, /* the compiled-in fallback was used */ + HOSTNAME_DEFAULT, /* the os-release default or the compiled-in fallback were used */ _HOSTNAME_INVALID = -EINVAL, } HostnameSource;