daandemeyer / rpms / systemd

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