]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
manager: restrict Dump*() to privileged callers or ratelimit 27438/head
authorLuca Boccassi <bluca@debian.org>
Thu, 27 Apr 2023 22:23:30 +0000 (23:23 +0100)
committerLuca Boccassi <bluca@debian.org>
Fri, 19 May 2023 14:18:23 +0000 (15:18 +0100)
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.

man/org.freedesktop.systemd1.xml
man/systemd-analyze.xml
src/core/dbus-manager.c
src/core/dbus.c
src/core/dbus.h
src/core/manager-serialize.c
src/core/manager.c
src/core/manager.h
src/core/org.freedesktop.systemd1.policy.in
test/units/testsuite-65.sh

index 38fd7098aa2e6375964cca50921b551c1505f94b..0835481f37d807f6ec95fb6434bd54b809e34814 100644 (file)
@@ -1403,7 +1403,8 @@ node /org/freedesktop/systemd1 {
       <function>DumpByFileDescriptor()</function>/<function>DumpUnitsMatchingPatternsByFileDescriptor()</function>
       are usually the preferred interface, since it ensures the data can be passed reliably from the service
       manager to the client. Note though that they cannot work when communicating with the service manager
-      remotely, as file descriptors are strictly local to a system.</para>
+      remotely, as file descriptors are strictly local to a system. All the <function>Dump*()</function>
+      methods are rate limited for unprivileged users.</para>
 
       <para><function>Reload()</function> may be invoked to reload all unit files.</para>
 
@@ -1778,7 +1779,9 @@ node /org/freedesktop/systemd1 {
       <function>UnsetAndSetEnvironment()</function>) require
       <interfacename>org.freedesktop.systemd1.set-environment</interfacename>. <function>Reload()</function>
       and <function>Reexecute()</function> require
-      <interfacename>org.freedesktop.systemd1.reload-daemon</interfacename>.
+      <interfacename>org.freedesktop.systemd1.reload-daemon</interfacename>. Operations which dump internal
+      state require <interfacename>org.freedesktop.systemd1.bypass-dump-ratelimit</interfacename> to avoid
+      rate limits.
       </para>
     </refsect2>
   </refsect1>
index 7176e3c046850f7c7f05fb5af285dd726243cb88..e0eba1cf64401af087dde2659909814239cbc231 100644 (file)
@@ -278,7 +278,7 @@ multi-user.target @47.820s
       <para>Without any parameter, this command outputs a (usually very long) human-readable serialization of
       the complete service manager state. Optional glob pattern may be specified, causing the output to be
       limited to units whose names match one of the patterns. The output format is subject to change without
-      notice and should not be parsed by applications.</para>
+      notice and should not be parsed by applications. This command is rate limited for unprivileged users.</para>
 
       <example>
         <title>Show the internal state of user manager</title>
index b18aa1d4ff2c61f4f5a2020cb8a5dfe7fe67354a..d6b2dcb11452e5a194ad2494b4ba6aeb6e1c0872 100644 (file)
@@ -1413,17 +1413,47 @@ static int dump_impl(
 
         assert(message);
 
-        /* Anyone can call this method */
-
+        /* 'status' access is the bare minimum always needed for this, as the policy might straight out
+         * forbid a client from querying any information from systemd, regardless of any rate limiting. */
         r = mac_selinux_access_check(message, "status", error);
         if (r < 0)
                 return r;
 
+        /* Rate limit reached? Check if the caller is privileged/allowed by policy to bypass this. We
+         * check the rate limit first to avoid the expensive roundtrip to polkit when not needed. */
+        if (!ratelimit_below(&m->dump_ratelimit)) {
+                /* We need a way for SELinux to constrain the operation when the rate limit is active, even
+                 * if polkit would allow it, but we cannot easily add new named permissions, so we need to
+                 * use an existing one. Reload/reexec are also slow but non-destructive/modifying
+                 * operations, and can cause PID1 to stall. So it seems similar enough in terms of security
+                 * considerations and impact, and thus use the same access check for dumps which, given the
+                 * large amount of data to fetch, can stall PID1 for quite some time. */
+                r = mac_selinux_access_check(message, "reload", error);
+                if (r < 0)
+                        goto ratelimited;
+
+                r = bus_verify_bypass_dump_ratelimit_async(m, message, error);
+                if (r < 0)
+                        goto ratelimited;
+                if (r == 0)
+                        /* No authorization for now, but the async polkit stuff will call us again when it
+                         * has it */
+                        return 1;
+        }
+
         r = manager_get_dump_string(m, patterns, &dump);
         if (r < 0)
                 return r;
 
         return reply(message, dump);
+
+ratelimited:
+        log_warning("Dump request rejected due to rate limit on unprivileged callers, blocked for %s.",
+                    FORMAT_TIMESPAN(ratelimit_left(&m->dump_ratelimit), USEC_PER_SEC));
+        return sd_bus_error_setf(error,
+                                 SD_BUS_ERROR_LIMITS_EXCEEDED,
+                                 "Dump request rejected due to rate limit on unprivileged callers, blocked for %s.",
+                                 FORMAT_TIMESPAN(ratelimit_left(&m->dump_ratelimit), USEC_PER_SEC));
 }
 
 static int reply_dump(sd_bus_message *message, char *dump) {
index 7277696196e050bf85d86deefbf654f4dd7eecb0..3fef44e6687575adf9a26d616017c216a7c4c99b 100644 (file)
@@ -1203,6 +1203,9 @@ int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_erro
 int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error) {
         return bus_verify_polkit_async(call, CAP_SYS_ADMIN, "org.freedesktop.systemd1.set-environment", NULL, false, UID_INVALID, &m->polkit_registry, error);
 }
+int bus_verify_bypass_dump_ratelimit_async(Manager *m, sd_bus_message *call, sd_bus_error *error) {
+        return bus_verify_polkit_async(call, CAP_SYS_ADMIN, "org.freedesktop.systemd1.bypass-dump-ratelimit", NULL, false, UID_INVALID, &m->polkit_registry, error);
+}
 
 uint64_t manager_bus_n_queued_write(Manager *m) {
         uint64_t c = 0;
index 369d9f56a289fe422e4115f0dde93dbb53ac41a1..50e7bb400e92bb7b599f360e1267b335fd60eab6 100644 (file)
@@ -27,6 +27,7 @@ int bus_verify_manage_units_async(Manager *m, sd_bus_message *call, sd_bus_error
 int bus_verify_manage_unit_files_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
 int bus_verify_reload_daemon_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
 int bus_verify_set_environment_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
+int bus_verify_bypass_dump_ratelimit_async(Manager *m, sd_bus_message *call, sd_bus_error *error);
 
 int bus_forward_agent_released(Manager *m, const char *path);
 
index 4570f06b73111fef038c862ec9be592fb160f762..c2b6fc1db6abe0013642721c20a31fd22c5ec46b 100644 (file)
@@ -165,6 +165,14 @@ int manager_serialize(
                 (void) serialize_item_format(f, "user-lookup", "%i %i", copy0, copy1);
         }
 
+        (void) serialize_item_format(f,
+                                     "dump-ratelimit",
+                                     USEC_FMT " " USEC_FMT " %u %u",
+                                     m->dump_ratelimit.begin,
+                                     m->dump_ratelimit.interval,
+                                     m->dump_ratelimit.num,
+                                     m->dump_ratelimit.burst);
+
         bus_track_serialize(m->subscribed, f, "subscribed");
 
         r = dynamic_user_serialize(m, f, fds);
@@ -550,6 +558,21 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                          * remains set until all serialized contents are handled. */
                         if (deserialize_varlink_sockets)
                                 (void) varlink_server_deserialize_one(m->varlink_server, val, fds);
+                } else if ((val = startswith(l, "dump-ratelimit="))) {
+                        usec_t begin, interval;
+                        unsigned num, burst;
+
+                        if (sscanf(val, USEC_FMT " " USEC_FMT " %u %u", &begin, &interval, &num, &burst) != 4)
+                                log_notice("Failed to parse dump ratelimit, ignoring: %s", val);
+                        else {
+                                /* If we changed the values across versions, flush the counter */
+                                if (interval != m->dump_ratelimit.interval || burst != m->dump_ratelimit.burst)
+                                        m->dump_ratelimit.num = 0;
+                                else
+                                        m->dump_ratelimit.num = num;
+                                m->dump_ratelimit.begin = begin;
+                        }
+
                 } else {
                         ManagerTimestamp q;
 
index 36881de479038a997ea2c352aaf597d6872f6f08..f8d469ebf64da8bfce565bf87e0f547936da70b5 100644 (file)
@@ -913,6 +913,11 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags,
 
                 .default_memory_pressure_watch = CGROUP_PRESSURE_WATCH_AUTO,
                 .default_memory_pressure_threshold_usec = USEC_INFINITY,
+
+                .dump_ratelimit = {
+                        .interval = 10 * USEC_PER_MINUTE,
+                        .burst = 10,
+                },
         };
 
 #if ENABLE_EFI
index 486eda11b604be3e9f2cfd285883e0ae8f9c3b26..08dcee3ce761c630f4e2fb443c86d2f52df2976c 100644 (file)
@@ -471,6 +471,8 @@ struct Manager {
 
         /* Allow users to configure a rate limit for Reload() operations */
         RateLimit reload_ratelimit;
+        /* Dump*() are slow, so always rate limit them to 10 per 10 minutes */
+        RateLimit dump_ratelimit;
 
         sd_event_source *memory_pressure_event_source;
 };
index 74adeadf38c2c124a0da660be54813cb14276a64..9e9a20f66f67777dd08386b86fb25810fa0210dd 100644 (file)
                 </defaults>
         </action>
 
+        <action id="org.freedesktop.systemd1.bypass-dump-ratelimit">
+                <description gettext-domain="systemd">Dump the systemd state without rate limits</description>
+                <message gettext-domain="systemd">Authentication is required to dump the systemd state without rate limits.</message>
+                <defaults>
+                        <allow_any>auth_admin</allow_any>
+                        <allow_inactive>auth_admin</allow_inactive>
+                        <allow_active>auth_admin_keep</allow_active>
+                </defaults>
+        </action>
+
 </policyconfig>
index 05673e7cc73f43f986a360a4017dbb2ba56cc9d0..c6a81b4b4eb2c54f514f95ed82033282956ebce3 100755 (executable)
@@ -53,6 +53,21 @@ systemd-analyze dot --require systemd-journald.service systemd-logind.service >/
 systemd-analyze dot "systemd-*.service" >/dev/null
 (! systemd-analyze dot systemd-journald.service systemd-logind.service "*" bbb ccc)
 # dump
+# this should be rate limited to 10 calls in 10 minutes for unprivileged callers
+for _ in {1..10}; do
+    runas testuser systemd-analyze dump systemd-journald.service >/dev/null
+done
+(! runas testuser systemd-analyze dump >/dev/null)
+# still limited after a reload
+systemctl daemon-reload
+(! runas testuser systemd-analyze dump >/dev/null)
+# and a re-exec
+systemctl daemon-reexec
+(! runas testuser systemd-analyze dump >/dev/null)
+# privileged call, so should not be rate limited
+for _ in {1..10}; do
+    systemd-analyze dump systemd-journald.service >/dev/null
+done
 systemd-analyze dump >/dev/null
 systemd-analyze dump "*" >/dev/null
 systemd-analyze dump "*.socket" >/dev/null