daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
b9a53a
From 2ec3c78b1d1ba907cd888aac3cdc3a86c03cda90 Mon Sep 17 00:00:00 2001
4b8c80
From: Jan Synacek <jsynacek@redhat.com>
4b8c80
Date: Fri, 31 Jan 2020 15:17:25 +0100
4b8c80
Subject: [PATCH] polkit: when authorizing via PK let's re-resolve
4b8c80
 callback/userdata instead of caching it
4b8c80
4b8c80
Previously, when doing an async PK query we'd store the original
4b8c80
callback/userdata pair and call it again after the PK request is
4b8c80
complete. This is problematic, since PK queries might be slow and in the
4b8c80
meantime the userdata might be released and re-acquired. Let's avoid
4b8c80
this by always traversing through the message handlers so that we always
4b8c80
re-resolve the callback and userdata pair and thus can be sure it's
4b8c80
up-to-date and properly valid.
4b8c80
4b8c80
Resolves: CVE-2020-1712
4b8c80
---
4b8c80
 src/shared/bus-util.c | 74 +++++++++++++++++++++++++++++--------------
4b8c80
 1 file changed, 50 insertions(+), 24 deletions(-)
4b8c80
4b8c80
diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c
4b8c80
index 2d908eb45c..5ed68429be 100644
4b8c80
--- a/src/shared/bus-util.c
4b8c80
+++ b/src/shared/bus-util.c
4b8c80
@@ -319,10 +319,10 @@ int bus_test_polkit(
4b8c80
 
4b8c80
 typedef struct AsyncPolkitQuery {
4b8c80
         sd_bus_message *request, *reply;
4b8c80
-        sd_bus_message_handler_t callback;
4b8c80
-        void *userdata;
4b8c80
         sd_bus_slot *slot;
4b8c80
+
4b8c80
         Hashmap *registry;
4b8c80
+        sd_event_source *defer_event_source;
4b8c80
 } AsyncPolkitQuery;
4b8c80
 
4b8c80
 static void async_polkit_query_free(AsyncPolkitQuery *q) {
4b8c80
@@ -338,9 +338,22 @@ static void async_polkit_query_free(AsyncPolkitQuery *q) {
4b8c80
         sd_bus_message_unref(q->request);
4b8c80
         sd_bus_message_unref(q->reply);
4b8c80
 
4b8c80
+        sd_event_source_disable_unref(q->defer_event_source);
4b8c80
         free(q);
4b8c80
 }
4b8c80
 
4b8c80
+static int async_polkit_defer(sd_event_source *s, void *userdata) {
4b8c80
+        AsyncPolkitQuery *q = userdata;
4b8c80
+
4b8c80
+        assert(s);
4b8c80
+
4b8c80
+        /* This is called as idle event source after we processed the async polkit reply, hopefully after the
4b8c80
+         * method call we re-enqueued has been properly processed. */
4b8c80
+
4b8c80
+        async_polkit_query_free(q);
4b8c80
+        return 0;
4b8c80
+}
4b8c80
+
4b8c80
 static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) {
4b8c80
         _cleanup_(sd_bus_error_free) sd_bus_error error_buffer = SD_BUS_ERROR_NULL;
4b8c80
         AsyncPolkitQuery *q = userdata;
4b8c80
@@ -349,19 +362,45 @@ static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_e
4b8c80
         assert(reply);
4b8c80
         assert(q);
4b8c80
 
4b8c80
+        assert(q->slot);
4b8c80
         q->slot = sd_bus_slot_unref(q->slot);
4b8c80
+
4b8c80
+        assert(!q->reply);
4b8c80
         q->reply = sd_bus_message_ref(reply);
4b8c80
 
4b8c80
+        /* Now, let's dispatch the original message a second time be re-enqueing. This will then traverse the
4b8c80
+         * whole message processing again, and thus re-validating and re-retrieving the "userdata" field
4b8c80
+         * again.
4b8c80
+         *
4b8c80
+         * We install an idle event loop event to clean-up the PolicyKit request data when we are idle again,
4b8c80
+         * i.e. after the second time the message is processed is complete. */
4b8c80
+
4b8c80
+        assert(!q->defer_event_source);
4b8c80
+        r = sd_event_add_defer(sd_bus_get_event(sd_bus_message_get_bus(reply)), &q->defer_event_source, async_polkit_defer, q);
4b8c80
+        if (r < 0)
4b8c80
+                goto fail;
4b8c80
+
4b8c80
+        r = sd_event_source_set_priority(q->defer_event_source, SD_EVENT_PRIORITY_IDLE);
4b8c80
+        if (r < 0)
4b8c80
+                goto fail;
4b8c80
+
4b8c80
+        r = sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_ONESHOT);
4b8c80
+        if (r < 0)
4b8c80
+                goto fail;
4b8c80
+
4b8c80
         r = sd_bus_message_rewind(q->request, true);
4b8c80
-        if (r < 0) {
4b8c80
-                r = sd_bus_reply_method_errno(q->request, r, NULL);
4b8c80
-                goto finish;
4b8c80
-        }
4b8c80
+        if (r < 0)
4b8c80
+                goto fail;
4b8c80
 
4b8c80
-        r = q->callback(q->request, q->userdata, &error_buffer);
4b8c80
-        r = bus_maybe_reply_error(q->request, r, &error_buffer);
4b8c80
+        r = sd_bus_enqueue_for_read(sd_bus_message_get_bus(q->request), q->request);
4b8c80
+        if (r < 0)
4b8c80
+                goto fail;
4b8c80
 
4b8c80
-finish:
4b8c80
+        return 1;
4b8c80
+
4b8c80
+fail:
4b8c80
+        log_debug_errno(r, "Processing asynchronous PolicyKit reply failed, ignoring: %m");
4b8c80
+        (void) sd_bus_reply_method_errno(q->request, r, NULL);
4b8c80
         async_polkit_query_free(q);
4b8c80
 
4b8c80
         return r;
4b8c80
@@ -382,11 +421,9 @@ int bus_verify_polkit_async(
4b8c80
 #if ENABLE_POLKIT
4b8c80
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *pk = NULL;
4b8c80
         AsyncPolkitQuery *q;
4b8c80
-        const char *sender, **k, **v;
4b8c80
-        sd_bus_message_handler_t callback;
4b8c80
-        void *userdata;
4b8c80
         int c;
4b8c80
 #endif
4b8c80
+        const char *sender, **k, **v;
4b8c80
         int r;
4b8c80
 
4b8c80
         assert(call);
4b8c80
@@ -444,20 +481,11 @@ int bus_verify_polkit_async(
4b8c80
         else if (r > 0)
4b8c80
                 return 1;
4b8c80
 
4b8c80
-#if ENABLE_POLKIT
4b8c80
-        if (sd_bus_get_current_message(call->bus) != call)
4b8c80
-                return -EINVAL;
4b8c80
-
4b8c80
-        callback = sd_bus_get_current_handler(call->bus);
4b8c80
-        if (!callback)
4b8c80
-                return -EINVAL;
4b8c80
-
4b8c80
-        userdata = sd_bus_get_current_userdata(call->bus);
4b8c80
-
4b8c80
         sender = sd_bus_message_get_sender(call);
4b8c80
         if (!sender)
4b8c80
                 return -EBADMSG;
4b8c80
 
4b8c80
+#if ENABLE_POLKIT
4b8c80
         c = sd_bus_message_get_allow_interactive_authorization(call);
4b8c80
         if (c < 0)
4b8c80
                 return c;
4b8c80
@@ -509,8 +537,6 @@ int bus_verify_polkit_async(
4b8c80
                 return -ENOMEM;
4b8c80
 
4b8c80
         q->request = sd_bus_message_ref(call);
4b8c80
-        q->callback = callback;
4b8c80
-        q->userdata = userdata;
4b8c80
 
4b8c80
         r = hashmap_put(*registry, call, q);
4b8c80
         if (r < 0) {