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 52bdde1f08ac888e0fb2321e7bd60e62bb82c7a5..ac5da453d64ab02c95150b764e9794fab0aee6e1 100644 (file)
--- 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 96698cb55bc2fd2d2c33627d0e1f3a24e1fa367f..d79843a287fcb64a9e47dd2d127523c82fb0d8e7 100644 (file)
--- 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 b8c70a9630be4df8453e5967173fecfb4e106db9..38b8b7a9c0841639c1317ebedb1958b031ecadbe 100644 (file)
--- 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 a14569cacd7c2fe01b707a6c5976eab2d07e8f18..f9ba74c6320838cc865c580cd77d54dd8377fd22 100644 (file)
--- 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 96f4fef04d60c742b33a202cb58b053ffbfd89d8..55728b7463e2ccd98fc8b2cb650d23687288691a 100644 (file)
--- 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,