From: Michael Vogt Date: Fri, 20 Mar 2026 12:49:50 +0000 (+0100) Subject: logind: extend verify_shutdown_creds() to take `sd_varlink *link` X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=db426d147d0ea8082bd8c69628aba63d3ddd635e;p=thirdparty%2Fsystemd.git logind: extend verify_shutdown_creds() to take `sd_varlink *link` To properly support a shutdown interface with varlink we need the functionality of verify_shutdown_creds(). This is currently dbus only. There are some options: 1. refactor and abstract so that verify_shutdown_creds() is agnostic 2. provide a equivalent function with varlink 3. allow to call it with either a dbus or a varlink message. The most elegant of course is (1) but it makes reviewing harder and has a higher regression risk. It will also be more code. Doing (2) has the risk of drift, i.e. we will need to keep the two functions in sync and not forget about it ever. So this commit opts for (3): allowing either dbus or varlink. This is quite ugly, however the big advantage is that its very simple to review as the dbus/varlink branches mirror each other. And there is no risk of drift the dbus/varlink options are close to each other. It unlikely we get a third bus, so it will most likely stay this way. It is still ugly though so I can understand if this is undesired and I can look into (1) if its too ugly. With this function avaialble logind-varlink.c is now updated to use it. --- diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 52bdde1f08a..ac5da453d64 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2306,7 +2306,7 @@ static int method_do_shutdown_or_sleep( } else if (!a) assert_se(a = handle_action_lookup(action)); - r = manager_verify_shutdown_creds(m, message, a, flags, error); + r = manager_verify_shutdown_creds(m, message, /* link= */ NULL, a, flags, error); if (r != 0) return r; @@ -2660,7 +2660,7 @@ static int method_schedule_shutdown(sd_bus_message *message, void *userdata, sd_ assert_se(a = handle_action_lookup(handle)); assert(a->polkit_action); - r = manager_verify_shutdown_creds(m, message, a, 0, error); + r = manager_verify_shutdown_creds(m, message, /* link= */ NULL, a, 0, error); if (r != 0) return r; diff --git a/src/login/logind-shutdown.c b/src/login/logind-shutdown.c index 96698cb55bc..d79843a287f 100644 --- a/src/login/logind-shutdown.c +++ b/src/login/logind-shutdown.c @@ -4,8 +4,8 @@ #include "sd-bus.h" #include "sd-event.h" +#include "sd-varlink.h" -#include "alloc-util.h" #include "bus-common-errors.h" #include "bus-polkit.h" #include "fs-util.h" @@ -40,11 +40,11 @@ int manager_have_multiple_sessions( int manager_verify_shutdown_creds( Manager *m, sd_bus_message *message, + sd_varlink *link, const HandleActionData *a, uint64_t flags, sd_bus_error *error) { - _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; bool multiple_sessions, blocked, interactive; _unused_ bool error_or_denial = false; Inhibitor *offending = NULL; @@ -53,15 +53,24 @@ int manager_verify_shutdown_creds( assert(m); assert(a); - assert(message); + assert(!!message != !!link); /* exactly one transport */ + assert(!link || !error); /* varlink doesn't use sd_bus_error */ - r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID, &creds); - if (r < 0) - return r; + if (message) { + _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; - r = sd_bus_creds_get_euid(creds, &uid); - if (r < 0) - return r; + r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID, &creds); + if (r < 0) + return r; + + r = sd_bus_creds_get_euid(creds, &uid); + if (r < 0) + return r; + } else { + r = sd_varlink_get_peer_uid(link, &uid); + if (r < 0) + return r; + } r = manager_have_multiple_sessions(m, uid); if (r < 0) @@ -72,14 +81,25 @@ int manager_verify_shutdown_creds( interactive = flags & SD_LOGIND_INTERACTIVE; if (multiple_sessions) { - r = bus_verify_polkit_async_full( - message, - a->polkit_action_multiple_sessions, - /* details= */ NULL, - /* good_user= */ UID_INVALID, - interactive ? POLKIT_ALLOW_INTERACTIVE : 0, - &m->polkit_registry, - error); + if (message) + r = bus_verify_polkit_async_full( + message, + a->polkit_action_multiple_sessions, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + interactive ? POLKIT_ALLOW_INTERACTIVE : 0, + &m->polkit_registry, + error); + else + r = varlink_verify_polkit_async_full( + link, + m->bus, + a->polkit_action_multiple_sessions, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + interactive ? POLKIT_ALLOW_INTERACTIVE : 0, + &m->polkit_registry); + if (r < 0) { /* If we get -EBUSY, it means a polkit decision was made, but not for * this action in particular. Assuming we are blocked on inhibitors, @@ -102,11 +122,16 @@ int manager_verify_shutdown_creds( if (!FLAGS_SET(flags, SD_LOGIND_SKIP_INHIBITORS) && (offending->mode != INHIBIT_BLOCK_WEAK || (uid == 0 && FLAGS_SET(flags, SD_LOGIND_ROOT_CHECK_INHIBITORS)))) { + if (link) + return sd_varlink_errorbo( + link, + "io.systemd.Shutdown.BlockedByInhibitor", + SD_JSON_BUILD_PAIR_STRING("who", offending->who), + SD_JSON_BUILD_PAIR_STRING("why", offending->why)); if (error) return sd_bus_error_set(error, BUS_ERROR_BLOCKED_BY_INHIBITOR_LOCK, "Operation denied due to active block inhibitor"); - else - return -EACCES; + return -EACCES; } /* We want to always ask here, even for root, to only allow bypassing if explicitly allowed @@ -117,14 +142,25 @@ int manager_verify_shutdown_creds( if (interactive) polkit_flags |= POLKIT_ALLOW_INTERACTIVE; - r = bus_verify_polkit_async_full( - message, - a->polkit_action_ignore_inhibit, - /* details= */ NULL, - /* good_user= */ UID_INVALID, - polkit_flags, - &m->polkit_registry, - error); + if (message) + r = bus_verify_polkit_async_full( + message, + a->polkit_action_ignore_inhibit, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + polkit_flags, + &m->polkit_registry, + error); + else + r = varlink_verify_polkit_async_full( + link, + m->bus, + a->polkit_action_ignore_inhibit, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + polkit_flags, + &m->polkit_registry); + if (r < 0) return r; if (r == 0) @@ -132,14 +168,25 @@ int manager_verify_shutdown_creds( } if (!multiple_sessions && !blocked) { - r = bus_verify_polkit_async_full( - message, - a->polkit_action, - /* details= */ NULL, - /* good_user= */ UID_INVALID, - interactive ? POLKIT_ALLOW_INTERACTIVE : 0, - &m->polkit_registry, - error); + if (message) + r = bus_verify_polkit_async_full( + message, + a->polkit_action, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + interactive ? POLKIT_ALLOW_INTERACTIVE : 0, + &m->polkit_registry, + error); + else + r = varlink_verify_polkit_async_full( + link, + m->bus, + a->polkit_action, + /* details= */ NULL, + /* good_user= */ UID_INVALID, + interactive ? POLKIT_ALLOW_INTERACTIVE : 0, + &m->polkit_registry); + if (r < 0) return r; if (r == 0) diff --git a/src/login/logind-shutdown.h b/src/login/logind-shutdown.h index b8c70a9630b..38b8b7a9c08 100644 --- a/src/login/logind-shutdown.h +++ b/src/login/logind-shutdown.h @@ -6,5 +6,10 @@ #define SHUTDOWN_SCHEDULE_FILE "/run/systemd/shutdown/scheduled" int manager_have_multiple_sessions(Manager *m, uid_t uid); -int manager_verify_shutdown_creds(Manager *m, sd_bus_message *message, const HandleActionData *a, uint64_t flags, sd_bus_error *error); + +/* manager_verify_shutdown_creds() takes *either* a "message" or "link" depending on if it is used + * to validate a D-Bus or Varlink shutdown request. When varlink is used the sd_bus_error *error + * must be NULL */ +int manager_verify_shutdown_creds(Manager *m, sd_bus_message *message, sd_varlink *link, const HandleActionData *a, uint64_t flags, sd_bus_error *error); + void manager_reset_scheduled_shutdown(Manager *m); diff --git a/src/login/logind-varlink.c b/src/login/logind-varlink.c index a14569cacd7..f9ba74c6320 100644 --- a/src/login/logind-varlink.c +++ b/src/login/logind-varlink.c @@ -364,11 +364,11 @@ static int setup_wall_message_timer(Manager *m, sd_varlink *link) { static int manager_do_shutdown_action(sd_varlink *link, sd_json_variant *parameters, HandleAction action) { Manager *m = ASSERT_PTR(sd_varlink_get_userdata(link)); int skip_inhibitors = -1; - uid_t uid; int r; static const sd_json_dispatch_field dispatch_table[] = { { "skipInhibitors", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_tristate, 0, 0 }, + VARLINK_DISPATCH_POLKIT_FIELD, {} }; @@ -381,23 +381,10 @@ static int manager_do_shutdown_action(sd_varlink *link, sd_json_variant *paramet const HandleActionData *a = handle_action_lookup(action); assert(a); - /* TODO: provide full polkit support (matching the D-Bus verify_shutdown_creds() with - * multiple-sessions and inhibitor-override checks). This requires some refactor. */ - r = varlink_check_privileged_peer(link); - if (r < 0) - return r; - - r = sd_varlink_get_peer_uid(link, &uid); - if (r < 0) + r = manager_verify_shutdown_creds(m, /* message= */ NULL, link, a, flags, /* error= */ NULL); + if (r != 0) return r; - /* Check for active inhibitors, mirroring the D-Bus verify_shutdown_creds() logic, - * we need this to ensure we handle the strong INHIBIT_BLOCK - * TODO: drop once we do the verify_shutdown_creds() */ - if (manager_is_inhibited(m, a->inhibit_what, NULL, /* flags= */ 0, uid, /* ret_offending= */ NULL) && - !FLAGS_SET(flags, SD_LOGIND_SKIP_INHIBITORS)) - return sd_varlink_error(link, "io.systemd.Shutdown.BlockedByInhibitor", /* parameters= */ NULL); - if (m->delayed_action) return sd_varlink_error(link, "io.systemd.Shutdown.AlreadyInProgress", /* parameters= */ NULL); diff --git a/src/shared/varlink-io.systemd.Shutdown.c b/src/shared/varlink-io.systemd.Shutdown.c index 96f4fef04d6..55728b7463e 100644 --- a/src/shared/varlink-io.systemd.Shutdown.c +++ b/src/shared/varlink-io.systemd.Shutdown.c @@ -30,7 +30,12 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_DEFINE_INPUT(skipInhibitors, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE)); static SD_VARLINK_DEFINE_ERROR(AlreadyInProgress); -static SD_VARLINK_DEFINE_ERROR(BlockedByInhibitor); +static SD_VARLINK_DEFINE_ERROR( + BlockedByInhibitor, + SD_VARLINK_FIELD_COMMENT("Who is holding the inhibitor"), + SD_VARLINK_DEFINE_FIELD(who, SD_VARLINK_STRING, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("Why the inhibitor is held"), + SD_VARLINK_DEFINE_FIELD(why, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); SD_VARLINK_DEFINE_INTERFACE( io_systemd_Shutdown,