diff --git a/src/shared/bus-polkit.c b/src/shared/bus-polkit.c index e3e327198c..b7f7c2b47a 100644 --- a/src/shared/bus-polkit.c +++ b/src/shared/bus-polkit.c @@ -167,18 +167,45 @@ int bus_test_polkit( #if ENABLE_POLKIT -typedef struct AsyncPolkitQuery { +typedef struct AsyncPolkitQueryAction { char *action; char **details; - sd_bus_message *request, *reply; + LIST_FIELDS(struct AsyncPolkitQueryAction, authorized); +} AsyncPolkitQueryAction; + +static AsyncPolkitQueryAction *async_polkit_query_action_free(AsyncPolkitQueryAction *a) { + if (!a) + return NULL; + + free(a->action); + strv_free(a->details); + + return mfree(a); +} + +DEFINE_TRIVIAL_CLEANUP_FUNC(AsyncPolkitQueryAction*, async_polkit_query_action_free); + +typedef struct AsyncPolkitQuery { + unsigned n_ref; + + AsyncPolkitQueryAction *action; + + sd_bus_message *request; sd_bus_slot *slot; Hashmap *registry; sd_event_source *defer_event_source; + + LIST_HEAD(AsyncPolkitQueryAction, authorized_actions); + AsyncPolkitQueryAction *denied_action; + AsyncPolkitQueryAction *error_action; + sd_bus_error error; } AsyncPolkitQuery; static AsyncPolkitQuery *async_polkit_query_free(AsyncPolkitQuery *q) { + AsyncPolkitQueryAction *a; + if (!q) return NULL; @@ -188,16 +215,27 @@ static AsyncPolkitQuery *async_polkit_query_free(AsyncPolkitQuery *q) { hashmap_remove(q->registry, q->request); sd_bus_message_unref(q->request); - sd_bus_message_unref(q->reply); - free(q->action); - strv_free(q->details); + async_polkit_query_action_free(q->action); sd_event_source_disable_unref(q->defer_event_source); + while ((a = q->authorized_actions)) { + LIST_REMOVE(authorized, q->authorized_actions, a); + async_polkit_query_action_free(a); + } + + async_polkit_query_action_free(q->denied_action); + async_polkit_query_action_free(q->error_action); + + sd_bus_error_free(&q->error); + return mfree(q); } +DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(AsyncPolkitQuery, async_polkit_query, async_polkit_query_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(AsyncPolkitQuery*, async_polkit_query_unref); + static int async_polkit_defer(sd_event_source *s, void *userdata) { AsyncPolkitQuery *q = ASSERT_PTR(userdata); @@ -206,21 +244,73 @@ static int async_polkit_defer(sd_event_source *s, void *userdata) { /* This is called as idle event source after we processed the async polkit reply, hopefully after the * method call we re-enqueued has been properly processed. */ - async_polkit_query_free(q); + async_polkit_query_unref(q); return 0; } -static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) { - AsyncPolkitQuery *q = ASSERT_PTR(userdata); +static int async_polkit_read_reply(sd_bus_message *reply, AsyncPolkitQuery *q) { + _cleanup_(async_polkit_query_action_freep) AsyncPolkitQueryAction *a = NULL; + int authorized, challenge, r; + + assert(reply); + assert(q); + + assert(q->action); + a = TAKE_PTR(q->action); + + if (sd_bus_message_is_method_error(reply, NULL)) { + const sd_bus_error *e; + + e = sd_bus_message_get_error(reply); + + /* Save error from polkit reply, so it can be returned when the same authorization is + * attempted for second time */ + if (!bus_error_is_unknown_service(e)) { + q->error_action = TAKE_PTR(a); + return sd_bus_error_copy(&q->error, e); + } + + /* Treat no PK available as access denied */ + q->denied_action = TAKE_PTR(a); + + return 0; + } + + r = sd_bus_message_enter_container(reply, 'r', "bba{ss}"); + if (r >= 0) + r = sd_bus_message_read(reply, "bb", &authorized, &challenge); + if (r < 0) + return r; + + /* It's currently expected that processing of a DBus message shall be interrupted on the first + * auth. error */ + assert(!q->denied_action); + assert(!q->error_action); + assert(!sd_bus_error_is_set(&q->error)); + + if (authorized) + LIST_PREPEND(authorized, q->authorized_actions, TAKE_PTR(a)); + else if (challenge) { + q->error_action = TAKE_PTR(a); + return sd_bus_error_set(&q->error, SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED, "Interactive authentication required."); + } else + q->denied_action = TAKE_PTR(a); + + return 0; +} + +static int async_polkit_process_reply(sd_bus_message *reply, AsyncPolkitQuery *q) { int r; assert(reply); + assert(q); assert(q->slot); q->slot = sd_bus_slot_unref(q->slot); - assert(!q->reply); - q->reply = sd_bus_message_ref(reply); + r = async_polkit_read_reply(reply, q); + if (r < 0) + return r; /* Now, let's dispatch the original message a second time be re-enqueing. This will then traverse the * whole message processing again, and thus re-validating and re-retrieving the "userdata" field @@ -229,92 +319,156 @@ static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_e * We install an idle event loop event to clean-up the PolicyKit request data when we are idle again, * i.e. after the second time the message is processed is complete. */ - assert(!q->defer_event_source); - r = sd_event_add_defer(sd_bus_get_event(sd_bus_message_get_bus(reply)), &q->defer_event_source, async_polkit_defer, q); - if (r < 0) - goto fail; + if (!q->defer_event_source) { + r = sd_event_add_defer( + sd_bus_get_event(sd_bus_message_get_bus(reply)), + &q->defer_event_source, + async_polkit_defer, + q); + if (r < 0) + return r; - r = sd_event_source_set_priority(q->defer_event_source, SD_EVENT_PRIORITY_IDLE); - if (r < 0) - goto fail; + r = sd_event_source_set_priority(q->defer_event_source, SD_EVENT_PRIORITY_IDLE); + if (r < 0) + return r; + } r = sd_event_source_set_enabled(q->defer_event_source, SD_EVENT_ONESHOT); if (r < 0) - goto fail; + return r; r = sd_bus_message_rewind(q->request, true); if (r < 0) - goto fail; + return r; r = sd_bus_enqueue_for_read(sd_bus_message_get_bus(q->request), q->request); if (r < 0) - goto fail; + return r; return 1; +} -fail: - log_debug_errno(r, "Processing asynchronous PolicyKit reply failed, ignoring: %m"); - (void) sd_bus_reply_method_errno(q->request, r, NULL); - async_polkit_query_free(q); +static int async_polkit_callback(sd_bus_message *reply, void *userdata, sd_bus_error *error) { + AsyncPolkitQuery *q = ASSERT_PTR(userdata); + int r; + + assert(reply); + + r = async_polkit_process_reply(reply, q); + if (r < 0) { + log_debug_errno(r, "Processing asynchronous PolicyKit reply failed, ignoring: %m"); + (void) sd_bus_reply_method_errno(q->request, r, NULL); + async_polkit_query_unref(q); + } return r; } -static int process_polkit_response( +_pure_ static int async_polkit_query_check_action( AsyncPolkitQuery *q, - sd_bus_message *call, const char *action, const char **details, - Hashmap **registry, sd_bus_error *ret_error) { - int authorized, challenge, r; - assert(q); - assert(call); assert(action); - assert(registry); assert(ret_error); - assert(q->action); - assert(q->reply); + LIST_FOREACH(authorized, a, q->authorized_actions) + if (streq(a->action, action) && strv_equal(a->details, (char**) details)) + return 1; - /* If the operation we want to authenticate changed between the first and the second time, - * let's not use this authentication, it might be out of date as the object and context we - * operate on might have changed. */ - if (!streq(q->action, action) || !strv_equal(q->details, (char**) details)) - return -ESTALE; + if (q->error_action && streq(q->error_action->action, action)) + return sd_bus_error_copy(ret_error, &q->error); - if (sd_bus_message_is_method_error(q->reply, NULL)) { - const sd_bus_error *e; + if (q->denied_action && streq(q->denied_action->action, action)) + return -EACCES; - e = sd_bus_message_get_error(q->reply); - - /* Treat no PK available as access denied */ - if (bus_error_is_unknown_service(e)) - return -EACCES; - - /* Copy error from polkit reply */ - sd_bus_error_copy(ret_error, e); - return -sd_bus_error_get_errno(e); - } - - r = sd_bus_message_enter_container(q->reply, 'r', "bba{ss}"); - if (r >= 0) - r = sd_bus_message_read(q->reply, "bb", &authorized, &challenge); - if (r < 0) - return r; - - if (authorized) - return 1; - - if (challenge) - return sd_bus_error_set(ret_error, SD_BUS_ERROR_INTERACTIVE_AUTHORIZATION_REQUIRED, "Interactive authentication required."); - - return -EACCES; + return 0; } #endif +/* bus_verify_polkit_async() handles verification of D-Bus calls with polkit. Because the polkit API + * is asynchronous, the whole thing is a bit complex and requires some support in the code that uses + * it. It relies on sd-bus's support for interrupting the processing of a message. + * + * Requirements: + * + * * bus_verify_polkit_async() must be called before any changes to internal state. + * * If bus_verify_polkit_async() has made a new polkit query (signaled by return value 0), + * processing of the message should be interrupted. This is done by returning 1--which sd-bus + * handles specially--and is usually accompanied by a comment. (The message will be queued for + * processing again later when a reply from polkit is received.) + * * The code needs to keep a hashmap, here called registry, in which bus_verify_polkit_async() + * stores active queries. This hashmap's lifetime must be larger than the method handler's; + * e.g., it can be a member of some "manager" object or a global variable. + * + * Return value: + * + * * 0 - a new polkit call has been made, which means the processing of the message should be + * interrupted; + * * 1 - the action has been allowed; + * * -EACCES - the action has been denied; + * * < 0 - an unspecified error. + * + * A step-by-step description of how it works: + * + * 1. A D-Bus method handler calls bus_verify_polkit_async(), passing it the D-Bus message being + * processed and the polkit action to verify. + * 2. bus_verify_polkit_async() checks the registry for an existing query object associated with the + * message. Let's assume this is the first call, so it finds nothing. + * 3. A new AsyncPolkitQuery object is created and an async. D-Bus call to polkit is made. The + * function then returns 0. The method handler returns 1 to tell sd-bus that the processing of + * the message has been interrupted. + * 4. (Later) A reply from polkit is received and async_polkit_callback() is called. + * 5. async_polkit_callback() reads the reply and stores its result in the passed query. + * 6. async_polkit_callback() enqueues the original message again. + * 7. (Later) The same D-Bus method handler is called for the same message. It calls + * bus_verify_polkit_async() again. + * 8. bus_verify_polkit_async() checks the registry for an existing query object associated with the + * message. It finds one and returns the result for the action. + * 9. The method handler continues processing of the message. If there's another action that needs + * to be verified: + * 10. bus_verify_polkit_async() is called again for the new action. The registry already contains a + * query for the message, but the new action hasn't been seen yet, hence steps 4-8 are repeated. + * 11. (In the method handler again.) bus_verify_polkit_async() returns query results for both + * actions and the processing continues as in step 9. + * + * Memory handling: + * + * async_polkit_callback() registers a deferred call of async_polkit_defer() for the query, which + * causes the query to be removed from the registry and freed. Deferred events are run with idle + * priority, so this will happen after processing of the D-Bus message, when the query is no longer + * needed. + * + * Schematically: + * + * (m - D-Bus message, a - polkit action, q - polkit query) + * + * -> foo_method(m) + * -> bus_verify_polkit_async(m, a) + * -> async_polkit_query_ref(q) + * -> bus_call_method_async(q) + * <- bus_verify_polkit_async(m, a) = 0 + * <- foo_method(m) = 1 + * ... + * -> async_polkit_callback(q) + * -> sd_event_add_defer(async_polkit_defer, q) + * -> sd_bus_enqueue_for_read(m) + * <- async_polkit_callback(q) + * ... + * -> foo_method(m) + * -> bus_verify_polkit_async(m, a) + * <- bus_verify_polkit_async(m, a) = 1/-EACCES/error + * ... + * // possibly another call to bus_verify_polkit_async with action a2 + * <- foo_method(m) + * ... + * -> async_polkit_defer(q) + * -> async_polkit_query_unref(q) + * <- async_polkit_defer(q) + */ + int bus_verify_polkit_async( sd_bus_message *call, int capability, @@ -336,11 +490,16 @@ int bus_verify_polkit_async( return r; #if ENABLE_POLKIT - AsyncPolkitQuery *q = hashmap_get(*registry, call); - /* This is the second invocation of this function, and there's already a response from - * polkit, let's process it */ - if (q) - return process_polkit_response(q, call, action, details, registry, ret_error); + _cleanup_(async_polkit_query_unrefp) AsyncPolkitQuery *q = NULL; + + q = async_polkit_query_ref(hashmap_get(*registry, call)); + /* This is a repeated invocation of this function, hence let's check if we've already got + * a response from polkit for this action */ + if (q) { + r = async_polkit_query_check_action(q, action, details, ret_error); + if (r != 0) + return r; + } #endif r = sd_bus_query_sender_privilege(call, capability); @@ -366,39 +525,42 @@ int bus_verify_polkit_async( if (r < 0) return r; - q = new(AsyncPolkitQuery, 1); - if (!q) + if (!q) { + q = new(AsyncPolkitQuery, 1); + if (!q) + return -ENOMEM; + + *q = (AsyncPolkitQuery) { + .n_ref = 1, + .request = sd_bus_message_ref(call), + }; + } + + assert(!q->action); + q->action = new(AsyncPolkitQueryAction, 1); + if (!q->action) return -ENOMEM; - *q = (AsyncPolkitQuery) { - .request = sd_bus_message_ref(call), + *q->action = (AsyncPolkitQueryAction) { + .action = strdup(action), + .details = strv_copy((char**) details), }; - - q->action = strdup(action); - if (!q->action) { - async_polkit_query_free(q); + if (!q->action->action || !q->action->details) return -ENOMEM; - } - q->details = strv_copy((char**) details); - if (!q->details) { - async_polkit_query_free(q); - return -ENOMEM; - } + if (!q->registry) { + r = hashmap_put(*registry, call, q); + if (r < 0) + return r; - r = hashmap_put(*registry, call, q); - if (r < 0) { - async_polkit_query_free(q); - return r; + q->registry = *registry; } - q->registry = *registry; - r = sd_bus_call_async(call->bus, &q->slot, pk, async_polkit_callback, q, 0); - if (r < 0) { - async_polkit_query_free(q); + if (r < 0) return r; - } + + TAKE_PTR(q); return 0; #endif @@ -408,7 +570,7 @@ int bus_verify_polkit_async( Hashmap *bus_verify_polkit_async_registry_free(Hashmap *registry) { #if ENABLE_POLKIT - return hashmap_free_with_destructor(registry, async_polkit_query_free); + return hashmap_free_with_destructor(registry, async_polkit_query_unref); #else assert(hashmap_isempty(registry)); return hashmap_free(registry);