richardphibel / rpms / systemd

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