From 856bfaeb054729b92992bc392aca2dbcc62e4a8e Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 12 Dec 2022 22:10:18 +0000 Subject: [PATCH] manager: add option to rate limit daemon-reload Reloading is a heavy-weight operation, and currently it is not possible to stop an orchestrator from spamming reload requests. Add configuration options to allow rate-limiting. --- man/kernel-command-line.xml | 2 ++ man/systemd-system.conf.xml | 10 ++++++++++ src/core/dbus-manager.c | 8 ++++++++ src/core/main.c | 33 +++++++++++++++++++++++++++++++++ src/core/manager.h | 3 +++ src/core/system.conf.in | 2 ++ src/core/user.conf.in | 2 ++ test/units/testsuite-59.sh | 19 +++++++++++++++++++ 8 files changed, 79 insertions(+) diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml index 368783d6fee..fcab0a90f40 100644 --- a/man/kernel-command-line.xml +++ b/man/kernel-command-line.xml @@ -72,6 +72,8 @@ systemd.machine_id= systemd.set_credential= systemd.import_credentials= + systemd.reload_limit_interval_sec= + systemd.reload_limit_burst= Parameters understood by the system and service manager to control system behavior. For details, see diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index ac21c31d9aa..6406df13ae6 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -550,6 +550,16 @@ If the value is /, only labels specified with SmackProcessLabel= are assigned and the compile-time default is ignored. + + + ReloadLimitIntervalSec= + ReloadLimitBurst= + + Rate limiting for daemon-reload requests. Default to unset, and any number of daemon-reload + operations can be requested at any time. ReloadLimitIntervalSec= takes a value in seconds + to configure the rate limit window, and ReloadLimitBurst= takes a positive integer to + configure the maximum allowed number of reloads within the configured time window. + diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 5c8a7d410f9..79e96f948c0 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1497,6 +1497,14 @@ static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error * /* Write a log message noting the unit or process who requested the Reload() */ log_reload_caller(message, m); + /* Check the rate limit after the authorization succeeds, to avoid denial-of-service issues. */ + if (!ratelimit_below(&m->reload_ratelimit)) { + log_warning("Reloading request rejected due to rate limit."); + return sd_bus_error_setf(error, + SD_BUS_ERROR_LIMITS_EXCEEDED, + "Reload() request rejected due to rate limit."); + } + /* Instead of sending the reply back right away, we just * remember that we need to and then send it after the reload * is finished. That way the caller knows when the reload diff --git a/src/core/main.c b/src/core/main.c index 9c1de3624cb..804010d7b8d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -173,6 +173,8 @@ static size_t arg_random_seed_size; static int arg_default_oom_score_adjust; static bool arg_default_oom_score_adjust_set; static char *arg_default_smack_process_label; +static usec_t arg_reload_limit_interval_sec; +static unsigned arg_reload_limit_burst; /* A copy of the original environment block */ static char **saved_env = NULL; @@ -483,6 +485,28 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat arg_random_seed = sz > 0 ? p : mfree(p); arg_random_seed_size = sz; + } else if (proc_cmdline_key_streq(key, "systemd.reload_limit_interval_sec")) { + + if (proc_cmdline_value_missing(key, value)) + return 0; + + r = parse_sec(value, &arg_reload_limit_interval_sec); + if (r < 0) { + log_warning_errno(r, "Failed to parse systemd.reload_limit_interval_sec= argument '%s', ignoring: %m", value); + return 0; + } + + } else if (proc_cmdline_key_streq(key, "systemd.reload_limit_burst")) { + + if (proc_cmdline_value_missing(key, value)) + return 0; + + r = safe_atou(value, &arg_reload_limit_burst); + if (r < 0) { + log_warning_errno(r, "Failed to parse systemd.reload_limit_burst= argument '%s', ignoring: %m", value); + return 0; + } + } else if (streq(key, "quiet") && !value) { if (arg_show_status == _SHOW_STATUS_INVALID) @@ -662,6 +686,8 @@ static int parse_config_file(void) { { "Manager", "CtrlAltDelBurstAction", config_parse_emergency_action, 0, &arg_cad_burst_action }, { "Manager", "DefaultOOMPolicy", config_parse_oom_policy, 0, &arg_default_oom_policy }, { "Manager", "DefaultOOMScoreAdjust", config_parse_oom_score_adjust, 0, NULL }, + { "Manager", "ReloadLimitIntervalSec", config_parse_sec, 0, &arg_reload_limit_interval_sec }, + { "Manager", "ReloadLimitBurst", config_parse_unsigned, 0, &arg_reload_limit_burst }, #if ENABLE_SMACK { "Manager", "DefaultSmackProcessLabel", config_parse_string, 0, &arg_default_smack_process_label }, #else @@ -762,6 +788,10 @@ static void set_manager_settings(Manager *m) { m->confirm_spawn = arg_confirm_spawn; m->service_watchdogs = arg_service_watchdogs; m->cad_burst_action = arg_cad_burst_action; + /* Note that we don't do structured initialization here, otherwise it will reset the rate limit + * counter on every daemon-reload. */ + m->reload_ratelimit.interval = arg_reload_limit_interval_sec; + m->reload_ratelimit.burst = arg_reload_limit_burst; manager_set_watchdog(m, WATCHDOG_RUNTIME, arg_runtime_watchdog); manager_set_watchdog(m, WATCHDOG_REBOOT, arg_reboot_watchdog); @@ -2451,6 +2481,9 @@ static void reset_arguments(void) { arg_default_oom_score_adjust_set = false; arg_default_smack_process_label = mfree(arg_default_smack_process_label); + + arg_reload_limit_interval_sec = 0; + arg_reload_limit_burst = 0; } static void determine_default_oom_score_adjust(void) { diff --git a/src/core/manager.h b/src/core/manager.h index 75c16d6e263..4d4b56c3cc0 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -461,6 +461,9 @@ struct Manager { struct restrict_fs_bpf *restrict_fs; char *default_smack_process_label; + + /* Allow users to configure a rate limit for Reload() operations */ + RateLimit reload_ratelimit; }; static inline usec_t manager_default_timeout_abort_usec(Manager *m) { diff --git a/src/core/system.conf.in b/src/core/system.conf.in index 71a5869ec0a..531a7694d98 100644 --- a/src/core/system.conf.in +++ b/src/core/system.conf.in @@ -75,3 +75,5 @@ #DefaultLimitRTTIME= #DefaultOOMPolicy=stop #DefaultSmackProcessLabel= +#ReloadLimitIntervalSec= +#ReloadLimitBurst= diff --git a/src/core/user.conf.in b/src/core/user.conf.in index b69974978ee..b4938942d14 100644 --- a/src/core/user.conf.in +++ b/src/core/user.conf.in @@ -48,3 +48,5 @@ #DefaultLimitRTPRIO= #DefaultLimitRTTIME= #DefaultSmackProcessLabel= +#ReloadLimitIntervalSec= +#ReloadLimitBurst diff --git a/test/units/testsuite-59.sh b/test/units/testsuite-59.sh index 83db0531072..ab6b2216fe1 100755 --- a/test/units/testsuite-59.sh +++ b/test/units/testsuite-59.sh @@ -85,6 +85,25 @@ wait_on_state_or_fail "testservice-abort-restart-59.service" "failed" "30" systemd-analyze log-level info +# Test that rate-limiting daemon-reload works +mkdir -p /run/systemd/system.conf.d/ +cat >/run/systemd/system.conf.d/50-test-59-reload.conf </testok exit 0