Skip to content

Commit 295d40b

Browse files
blucajamacku
authored andcommitted
manager: restrict Dump*() to privileged callers or ratelimit
Dump*() methods can take quite some time due to the amount of data to serialize, so they can potentially stall the manager. Make them privileged, as they are debugging tools anyway. Use a new 'dump' capability for polkit, and the 'reload' capability for SELinux, as that's also non-destructive but slow. If the caller is not privileged, allow it but rate limited to 10 calls every 10 minutes. (cherry picked from commit d936595) Resolves: RHEL-35703
1 parent 3f05fc9 commit 295d40b

File tree

10 files changed

+98
-5
lines changed

10 files changed

+98
-5
lines changed

man/org.freedesktop.systemd1.xml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,8 @@ node /org/freedesktop/systemd1 {
13631363
<function>DumpByFileDescriptor()</function>/<function>DumpUnitsMatchingPatternsByFileDescriptor()</function>
13641364
are usually the preferred interface, since it ensures the data can be passed reliably from the service
13651365
manager to the client. Note though that they cannot work when communicating with the service manager
1366-
remotely, as file descriptors are strictly local to a system.</para>
1366+
remotely, as file descriptors are strictly local to a system. All the <function>Dump*()</function>
1367+
methods are rate limited for unprivileged users.</para>
13671368

13681369
<para><function>Reload()</function> may be invoked to reload all unit files.</para>
13691370

@@ -1726,7 +1727,9 @@ node /org/freedesktop/systemd1 {
17261727
<function>UnsetAndSetEnvironment()</function>) require
17271728
<interfacename>org.freedesktop.systemd1.set-environment</interfacename>. <function>Reload()</function>
17281729
and <function>Reexecute()</function> require
1729-
<interfacename>org.freedesktop.systemd1.reload-daemon</interfacename>.
1730+
<interfacename>org.freedesktop.systemd1.reload-daemon</interfacename>. Operations which dump internal
1731+
state require <interfacename>org.freedesktop.systemd1.bypass-dump-ratelimit</interfacename> to avoid
1732+
rate limits.
17301733
</para>
17311734
</refsect2>
17321735
</refsect1>

man/systemd-analyze.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ multi-user.target @47.820s
249249
<para>Without any parameter, this command outputs a (usually very long) human-readable serialization of
250250
the complete service manager state. Optional glob pattern may be specified, causing the output to be
251251
limited to units whose names match one of the patterns. The output format is subject to change without
252-
notice and should not be parsed by applications.</para>
252+
notice and should not be parsed by applications. This command is rate limited for unprivileged users.</para>
253253

254254
<example>
255255
<title>Show the internal state of user manager</title>

src/core/dbus-manager.c

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,17 +1357,47 @@ static int dump_impl(
13571357

13581358
assert(message);
13591359

1360-
/* Anyone can call this method */
1361-
1360+
/* 'status' access is the bare minimum always needed for this, as the policy might straight out
1361+
* forbid a client from querying any information from systemd, regardless of any rate limiting. */
13621362
r = mac_selinux_access_check(message, "status", error);
13631363
if (r < 0)
13641364
return r;
13651365

1366+
/* Rate limit reached? Check if the caller is privileged/allowed by policy to bypass this. We
1367+
* check the rate limit first to avoid the expensive roundtrip to polkit when not needed. */
1368+
if (!ratelimit_below(&m->dump_ratelimit)) {
1369+
/* We need a way for SELinux to constrain the operation when the rate limit is active, even
1370+
* if polkit would allow it, but we cannot easily add new named permissions, so we need to
1371+
* use an existing one. Reload/reexec are also slow but non-destructive/modifying
1372+
* operations, and can cause PID1 to stall. So it seems similar enough in terms of security
1373+
* considerations and impact, and thus use the same access check for dumps which, given the
1374+
* large amount of data to fetch, can stall PID1 for quite some time. */
1375+
r = mac_selinux_access_check(message, "reload", error);
1376+
if (r < 0)
1377+
goto ratelimited;
1378+
1379+
r = bus_verify_bypass_dump_ratelimit_async(m, message, error);
1380+
if (r < 0)
1381+
goto ratelimited;
1382+
if (r == 0)
1383+
/* No authorization for now, but the async polkit stuff will call us again when it
1384+
* has it */
1385+
return 1;
1386+
}
1387+
13661388
r = manager_get_dump_string(m, patterns, &dump);
13671389
if (r < 0)
13681390
return r;
13691391

13701392
return reply(message, dump);
1393+
1394+
ratelimited:
1395+
log_warning("Dump request rejected due to rate limit on unprivileged callers, blocked for %s.",
1396+
FORMAT_TIMESPAN(ratelimit_left(&m->dump_ratelimit), USEC_PER_SEC));
1397+
return sd_bus_error_setf(error,
1398+
SD_BUS_ERROR_LIMITS_EXCEEDED,
1399+
"Dump request rejected due to rate limit on unprivileged callers, blocked for %s.",
1400+
FORMAT_TIMESPAN(ratelimit_left(&m->dump_ratelimit), USEC_PER_SEC));
13711401
}
13721402

13731403
static int reply_dump(sd_bus_message *message, char *dump) {

src/core/dbus.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,9 @@ int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_erro
11771177
int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error) {
11781178
return bus_verify_polkit_async(call, CAP_SYS_ADMIN, "org.freedesktop.systemd1.set-environment", NULL, false, UID_INVALID, &m->polkit_registry, error);
11791179
}
1180+
int bus_verify_bypass_dump_ratelimit_async(Manager *m, sd_bus_message *call, sd_bus_error *error) {
1181+
return bus_verify_polkit_async(call, CAP_SYS_ADMIN, "org.freedesktop.systemd1.bypass-dump-ratelimit", NULL, false, UID_INVALID, &m->polkit_registry, error);
1182+
}
11801183

11811184
uint64_t manager_bus_n_queued_write(Manager *m) {
11821185
uint64_t c = 0;

src/core/dbus.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ int bus_verify_manage_units_async(Manager *m, sd_bus_message *call, sd_bus_error
2727
int bus_verify_manage_unit_files_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
2828
int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
2929
int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
30+
int bus_verify_bypass_dump_ratelimit_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
3031

3132
int bus_forward_agent_released(Manager *m, const char *path);
3233

src/core/manager-serialize.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ int manager_serialize(
164164
(void) serialize_item_format(f, "user-lookup", "%i %i", copy0, copy1);
165165
}
166166

167+
(void) serialize_item_format(f,
168+
"dump-ratelimit",
169+
USEC_FMT " " USEC_FMT " %u %u",
170+
m->dump_ratelimit.begin,
171+
m->dump_ratelimit.interval,
172+
m->dump_ratelimit.num,
173+
m->dump_ratelimit.burst);
174+
167175
bus_track_serialize(m->subscribed, f, "subscribed");
168176

169177
r = dynamic_user_serialize(m, f, fds);
@@ -549,6 +557,21 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
549557
* remains set until all serialized contents are handled. */
550558
if (deserialize_varlink_sockets)
551559
(void) varlink_server_deserialize_one(m->varlink_server, val, fds);
560+
} else if ((val = startswith(l, "dump-ratelimit="))) {
561+
usec_t begin, interval;
562+
unsigned num, burst;
563+
564+
if (sscanf(val, USEC_FMT " " USEC_FMT " %u %u", &begin, &interval, &num, &burst) != 4)
565+
log_notice("Failed to parse dump ratelimit, ignoring: %s", val);
566+
else {
567+
/* If we changed the values across versions, flush the counter */
568+
if (interval != m->dump_ratelimit.interval || burst != m->dump_ratelimit.burst)
569+
m->dump_ratelimit.num = 0;
570+
else
571+
m->dump_ratelimit.num = num;
572+
m->dump_ratelimit.begin = begin;
573+
}
574+
552575
} else {
553576
ManagerTimestamp q;
554577

src/core/manager.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,11 @@ int manager_new(LookupScope scope, ManagerTestRunFlags test_run_flags, Manager *
866866
.test_run_flags = test_run_flags,
867867

868868
.default_oom_policy = OOM_STOP,
869+
870+
.dump_ratelimit = {
871+
.interval = 10 * USEC_PER_MINUTE,
872+
.burst = 10,
873+
},
869874
};
870875

871876
#if ENABLE_EFI

src/core/manager.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,9 @@ struct Manager {
461461
struct restrict_fs_bpf *restrict_fs;
462462

463463
char *default_smack_process_label;
464+
465+
/* Dump*() are slow, so always rate limit them to 10 per 10 minutes */
466+
RateLimit dump_ratelimit;
464467
};
465468

466469
static inline usec_t manager_default_timeout_abort_usec(Manager *m) {

src/core/org.freedesktop.systemd1.policy.in

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,14 @@
7070
</defaults>
7171
</action>
7272

73+
<action id="org.freedesktop.systemd1.bypass-dump-ratelimit">
74+
<description gettext-domain="systemd">Dump the systemd state without rate limits</description>
75+
<message gettext-domain="systemd">Authentication is required to dump the systemd state without rate limits.</message>
76+
<defaults>
77+
<allow_any>auth_admin</allow_any>
78+
<allow_inactive>auth_admin</allow_inactive>
79+
<allow_active>auth_admin_keep</allow_active>
80+
</defaults>
81+
</action>
82+
7383
</policyconfig>

test/units/testsuite-65.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,21 @@ systemd-analyze dot --require systemd-journald.service systemd-logind.service >/
5151
systemd-analyze dot "systemd-*.service" >/dev/null
5252
(! systemd-analyze dot systemd-journald.service systemd-logind.service "*" bbb ccc)
5353
# dump
54+
# this should be rate limited to 10 calls in 10 minutes for unprivileged callers
55+
for _ in {1..10}; do
56+
runas testuser systemd-analyze dump systemd-journald.service >/dev/null
57+
done
58+
(! runas testuser systemd-analyze dump >/dev/null)
59+
# still limited after a reload
60+
systemctl daemon-reload
61+
(! runas testuser systemd-analyze dump >/dev/null)
62+
# and a re-exec
63+
systemctl daemon-reexec
64+
(! runas testuser systemd-analyze dump >/dev/null)
65+
# privileged call, so should not be rate limited
66+
for _ in {1..10}; do
67+
systemd-analyze dump systemd-journald.service >/dev/null
68+
done
5469
systemd-analyze dump >/dev/null
5570
systemd-analyze dump "*" >/dev/null
5671
systemd-analyze dump "*.socket" >/dev/null

0 commit comments

Comments
 (0)