From 8c1e088ac9ca2722122774d6839fae3ccf6b48fc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:25:17 +0100 Subject: [PATCH 1/6] nspawn: drop two entirely redundant lines --- src/nspawn/nspawn-settings.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index edc0f663bb..fc9e9fc54f 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -285,9 +285,6 @@ int config_parse_capability( } } - if (u == 0) - return 0; - *result |= u; return 0; } From a1dfd585c4a3a737b0c4b61e82148bbdfd201733 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:21:15 +0100 Subject: [PATCH 2/6] nspawn: add helper settings_network_configured() The new helper returns whether the settings file had *any* networking setting configured at all. We already have a similar helper settings_private_network() which returns a similar result. The difference is that the new helper will return true when the private network was explicitly turned off, while the old one will only return true if configured and enabled. We'll reuse the helper a 2nd time later on, but even without it it makes things a bit more readable. --- src/nspawn/nspawn-settings.c | 21 +++++++++++++++++++++ src/nspawn/nspawn-settings.h | 2 ++ src/nspawn/nspawn.c | 10 +--------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index fc9e9fc54f..c63b8da23a 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -170,6 +170,8 @@ Settings* settings_free(Settings *s) { bool settings_private_network(Settings *s) { assert(s); + /* Determines whether we shall open up our own private network */ + return s->private_network > 0 || s->network_veth > 0 || @@ -190,6 +192,25 @@ bool settings_network_veth(Settings *s) { s->network_zone; } +bool settings_network_configured(Settings *s) { + assert(s); + + /* Determines whether any network configuration setting was used. (i.e. in contrast to + * settings_private_network() above this might also indicate if private networking was explicitly + * turned off.) */ + + return + s->private_network >= 0 || + s->network_veth >= 0 || + s->network_bridge || + s->network_zone || + s->network_interfaces || + s->network_macvlan || + s->network_ipvlan || + s->network_veth_extra || + s->network_namespace_path; +} + int settings_allocate_properties(Settings *s) { _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; int r; diff --git a/src/nspawn/nspawn-settings.h b/src/nspawn/nspawn-settings.h index 1b3ace5f8f..797e383401 100644 --- a/src/nspawn/nspawn-settings.h +++ b/src/nspawn/nspawn-settings.h @@ -242,6 +242,8 @@ Settings* settings_free(Settings *s); bool settings_network_veth(Settings *s); bool settings_private_network(Settings *s); +bool settings_network_configured(Settings *s); + int settings_allocate_properties(Settings *s); DEFINE_TRIVIAL_CLEANUP_FUNC(Settings*, settings_free); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 1f327b0952..f8f9e72421 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4407,15 +4407,7 @@ static int merge_settings(Settings *settings, const char *path) { } if ((arg_settings_mask & SETTING_NETWORK) == 0 && - (settings->private_network >= 0 || - settings->network_veth >= 0 || - settings->network_bridge || - settings->network_zone || - settings->network_interfaces || - settings->network_macvlan || - settings->network_ipvlan || - settings->network_veth_extra || - settings->network_namespace_path)) { + settings_network_configured(settings)) { if (!arg_settings_trusted) log_warning("Ignoring network settings, file %s is not trusted.", path); From d3689b94353d5f29b9d99d80bf6d888bd4eb70cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:23:36 +0100 Subject: [PATCH 3/6] nspawn: use three boolean fields from settings file when actually set Let's turn these three fields into tristates, so that we can distinguish whether they are not configured at all from explicitly turned off. Let#s then use this to ensure that we only copy the settings fields into our execution environment if they are actually configured. We already do this for some of the boolean settings, this adds it for the missing ones. The goal here is to ensure that an empty settings file used in --settings=override mode (i.e. the default mode used in the systemd-nspawn@.service unit) is truly a NOP. --- src/nspawn/nspawn-gperf.gperf | 6 +++--- src/nspawn/nspawn-settings.c | 4 ++++ src/nspawn/nspawn-settings.h | 6 +++--- src/nspawn/nspawn.c | 9 ++++++--- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/nspawn/nspawn-gperf.gperf b/src/nspawn/nspawn-gperf.gperf index 4af00c8d95..d25bef7468 100644 --- a/src/nspawn/nspawn-gperf.gperf +++ b/src/nspawn/nspawn-gperf.gperf @@ -20,7 +20,7 @@ struct ConfigPerfItem; %includes %% Exec.Boot, config_parse_boot, 0, 0 -Exec.Ephemeral, config_parse_bool, 0, offsetof(Settings, ephemeral) +Exec.Ephemeral, config_parse_tristate, 0, offsetof(Settings, ephemeral) Exec.ProcessTwo, config_parse_pid2, 0, 0 Exec.Parameters, config_parse_strv, 0, offsetof(Settings, parameters) Exec.Environment, config_parse_strv, 0, offsetof(Settings, environment) @@ -34,7 +34,7 @@ Exec.MachineID, config_parse_id128, 0, of Exec.WorkingDirectory, config_parse_path, 0, offsetof(Settings, working_directory) Exec.PivotRoot, config_parse_pivot_root, 0, 0 Exec.PrivateUsers, config_parse_private_users, 0, 0 -Exec.NotifyReady, config_parse_bool, 0, offsetof(Settings, notify_ready) +Exec.NotifyReady, config_parse_tristate, 0, offsetof(Settings, notify_ready) Exec.SystemCallFilter, config_parse_syscall_filter, 0, 0, Exec.LimitCPU, config_parse_rlimit, RLIMIT_CPU, offsetof(Settings, rlimit) Exec.LimitFSIZE, config_parse_rlimit, RLIMIT_FSIZE, offsetof(Settings, rlimit) @@ -59,7 +59,7 @@ Exec.CPUAffinity, config_parse_cpu_affinity, 0, 0 Exec.ResolvConf, config_parse_resolv_conf, 0, offsetof(Settings, resolv_conf) Exec.LinkJournal, config_parse_link_journal, 0, 0 Exec.Timezone, config_parse_timezone, 0, offsetof(Settings, timezone) -Exec.SuppressSync, config_parse_bool, 0, offsetof(Settings, suppress_sync) +Exec.SuppressSync, config_parse_tristate, 0, offsetof(Settings, suppress_sync) Files.ReadOnly, config_parse_tristate, 0, offsetof(Settings, read_only) Files.Volatile, config_parse_volatile_mode, 0, offsetof(Settings, volatile_mode) Files.Bind, config_parse_bind, 0, 0 diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c index c63b8da23a..1f58bf3ed4 100644 --- a/src/nspawn/nspawn-settings.c +++ b/src/nspawn/nspawn-settings.c @@ -27,6 +27,7 @@ Settings *settings_new(void) { *s = (Settings) { .start_mode = _START_MODE_INVALID, + .ephemeral = -1, .personality = PERSONALITY_INVALID, .resolv_conf = _RESOLV_CONF_MODE_INVALID, @@ -57,6 +58,9 @@ Settings *settings_new(void) { .clone_ns_flags = ULONG_MAX, .use_cgns = -1, + + .notify_ready = -1, + .suppress_sync = -1, }; return s; diff --git a/src/nspawn/nspawn-settings.h b/src/nspawn/nspawn-settings.h index 797e383401..59397ca54b 100644 --- a/src/nspawn/nspawn-settings.h +++ b/src/nspawn/nspawn-settings.h @@ -162,7 +162,7 @@ typedef struct OciHook { typedef struct Settings { /* [Exec] */ StartMode start_mode; - bool ephemeral; + int ephemeral; char **parameters; char **environment; char *user; @@ -177,7 +177,7 @@ typedef struct Settings { char *pivot_root_old; UserNamespaceMode userns_mode; uid_t uid_shift, uid_range; - bool notify_ready; + int notify_ready; char **syscall_allow_list; char **syscall_deny_list; struct rlimit *rlimit[_RLIMIT_MAX]; @@ -190,7 +190,7 @@ typedef struct Settings { LinkJournal link_journal; bool link_journal_try; TimezoneMode timezone; - bool suppress_sync; + int suppress_sync; /* [Files] */ int read_only; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f8f9e72421..25075d2b46 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4284,7 +4284,8 @@ static int merge_settings(Settings *settings, const char *path) { strv_free_and_replace(arg_parameters, settings->parameters); } - if ((arg_settings_mask & SETTING_EPHEMERAL) == 0) + if ((arg_settings_mask & SETTING_EPHEMERAL) == 0 && + settings->ephemeral >= 0) arg_ephemeral = settings->ephemeral; if ((arg_settings_mask & SETTING_DIRECTORY) == 0 && @@ -4454,7 +4455,8 @@ static int merge_settings(Settings *settings, const char *path) { if ((arg_settings_mask & SETTING_BIND_USER) == 0) strv_free_and_replace(arg_bind_user, settings->bind_user); - if ((arg_settings_mask & SETTING_NOTIFY_READY) == 0) + if ((arg_settings_mask & SETTING_NOTIFY_READY) == 0 && + settings->notify_ready >= 0) arg_notify_ready = settings->notify_ready; if ((arg_settings_mask & SETTING_SYSCALL_FILTER) == 0) { @@ -4577,7 +4579,8 @@ static int merge_settings(Settings *settings, const char *path) { arg_console_mode = settings->console_mode; } - if ((arg_settings_mask & SETTING_SUPPRESS_SYNC) == 0) + if ((arg_settings_mask & SETTING_SUPPRESS_SYNC) == 0 && + settings->suppress_sync >= 0) arg_suppress_sync = settings->suppress_sync; /* The following properties can only be set through the OCI settings logic, not from the command line, hence we From 0cc3c9f997d0ea39f0309e74117a033635b2291f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:25:42 +0100 Subject: [PATCH 4/6] nspawn: copy BindUser= setting from settings only if set Let's only pick this up from the settings if actually set. As in the previous commit this makes sure that an empty settings file in --settings=override mode is really a NOP. --- src/nspawn/nspawn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 25075d2b46..3209b50417 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4452,7 +4452,8 @@ static int merge_settings(Settings *settings, const char *path) { } } - if ((arg_settings_mask & SETTING_BIND_USER) == 0) + if ((arg_settings_mask & SETTING_BIND_USER) == 0 && + !strv_isempty(settings->bind_user)) strv_free_and_replace(arg_bind_user, settings->bind_user); if ((arg_settings_mask & SETTING_NOTIFY_READY) == 0 && From 2d09ea44fcd7c13658bf2e706b4ecd6aba35bfbf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:26:53 +0100 Subject: [PATCH 5/6] nspawn: only copy syscall filters from settings if actually configured As in the previous commit, let's not copy settings that aren#t configured, so that --settings=override with an empty .nspawn file is truly a NOP. --- src/nspawn/nspawn.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 3209b50417..9adc166aa9 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4462,19 +4462,23 @@ static int merge_settings(Settings *settings, const char *path) { if ((arg_settings_mask & SETTING_SYSCALL_FILTER) == 0) { - if (!arg_settings_trusted && !strv_isempty(settings->syscall_allow_list)) - log_warning("Ignoring SystemCallFilter= settings, file %s is not trusted.", path); - else { - strv_free_and_replace(arg_syscall_allow_list, settings->syscall_allow_list); - strv_free_and_replace(arg_syscall_deny_list, settings->syscall_deny_list); + if (!strv_isempty(settings->syscall_allow_list) || !strv_isempty(settings->syscall_deny_list)) { + if (!arg_settings_trusted && !strv_isempty(settings->syscall_allow_list)) + log_warning("Ignoring SystemCallFilter= settings, file %s is not trusted.", path); + else { + strv_free_and_replace(arg_syscall_allow_list, settings->syscall_allow_list); + strv_free_and_replace(arg_syscall_deny_list, settings->syscall_deny_list); + } } #if HAVE_SECCOMP - if (!arg_settings_trusted && settings->seccomp) - log_warning("Ignoring SECCOMP filter, file %s is not trusted.", path); - else { - seccomp_release(arg_seccomp); - arg_seccomp = TAKE_PTR(settings->seccomp); + if (settings->seccomp) { + if (!arg_settings_trusted) + log_warning("Ignoring SECCOMP filter, file %s is not trusted.", path); + else { + seccomp_release(arg_seccomp); + arg_seccomp = TAKE_PTR(settings->seccomp); + } } #endif } From 9baa294c12e6d3e191081260092b877bf6bd3275 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 18:27:40 +0100 Subject: [PATCH 6/6] nspawn: don't muck with caps if no network setting is used in settings file Our goal here (as in the previous commits) is to ensure that a settings file loaded in --settings=override mode is truly a NOP. Previously this was not the case as we'd drop CAP_NET_ADMIN from the caps if the settings file didn't enable networking. With this change we'll drop it only if explicitly turned off in the settings file, and otherwise let the built-in defaults and cmdline params reign supreme as documented. Fixes: #20055 --- src/nspawn/nspawn.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 9adc166aa9..66daeb4f64 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4337,7 +4337,8 @@ static int merge_settings(Settings *settings, const char *path) { plus = settings->capability; minus = settings->drop_capability; - if ((arg_settings_mask & SETTING_NETWORK) == 0) { + if ((arg_settings_mask & SETTING_NETWORK) == 0 && + settings_network_configured(settings)) { if (settings_private_network(settings)) plus |= UINT64_C(1) << CAP_NET_ADMIN; else