login: handle -EALREADY from bus_verify_polkit_async_full()

Commit 536c18e5c3 ("bus-polkit: shortcut auth. after first denial")
added logic to async_polkit_query_check_action() that returns
-EALREADY when a failure or denial decision was made for a previous
action.

This has the consequence that root is able to ignore inhibitors and
shutdown etc. even when polkit explicitly denies it. This is because
when systemctl's verb_start_special() calls logind_reboot(), unless
the call succeeds or returns one of -EACCES, -EOPNOTSUPP, or
-EINPROGRESS, a fallback path is taken to attempt the action without
going through logind. Hence, since logind_reboot() started returning
-EALREADY in some cases, the fallback path was taken, and the shutdown
was performed anyways.

For example:

 root@ubuntu:/# cat /etc/polkit-1/rules.d/10-systemd-logind-no-skip-inhibitors.rules
 // Never allow strong inhibitors to be ignored.
 polkit.addRule(function(action, subject) {
     if ((action.id == "org.freedesktop.login1.power-off-ignore-inhibit" ||
          action.id == "org.freedesktop.login1.reboot-ignore-inhibit" ||
          action.id == "org.freedesktop.login1.halt-ignore-inhibit" ||
          action.id == "org.freedesktop.login1.suspend-ignore-inhibit" ||
          action.id == "org.freedesktop.login1.hibernate-ignore-inhibit")) {

         return polkit.Result.NO;
     }
 });
 root@ubuntu:/# systemctl reboot -i
 Call to Reboot failed: Operation already in progress

..but the reboot continues anyways due to the fallback.

To fix this, add logic in systemd-logind's verify_shutdown_creds() to
handle -EALREADY from bus_verify_polkit_async_full(): if we receive
-EALREADY when checking authorization for <action>-multiple-sessions,
and we are blocked on inhibitors, continue on to get the decision for
<action>-ignore-inhibit directly.

While here, add similar logic to method_inhibit(), which may need to
verify multiple polkit actions in a single call.

Fixes 536c18e5c3
This commit is contained in:
Nick Rosbrook
2025-03-21 14:38:20 -04:00
parent 94a5969b31
commit 4b1e7a5822

View File

@@ -2172,7 +2172,7 @@ static int verify_shutdown_creds(
sd_bus_error *error) {
_cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL;
bool multiple_sessions, blocked, interactive;
bool multiple_sessions, blocked, interactive, error_or_denial = false;
Inhibitor *offending = NULL;
uid_t uid;
int r;
@@ -2206,8 +2206,15 @@ static int verify_shutdown_creds(
interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
&m->polkit_registry,
error);
if (r < 0)
return r;
if (r < 0) {
/* If we get -EALREADY, it means a polkit decision was made, but not for
* this action in particular. Assuming we are blocked on inhibitors,
* ignore that error and allow the decision to be revealed below. */
if (blocked && r == -EALREADY)
error_or_denial = true;
else
return r;
}
if (r == 0)
return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
}
@@ -2261,6 +2268,13 @@ static int verify_shutdown_creds(
return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
}
/* If error_or_denial was set above, it means that a polkit denial or
* error was deferred for a future call to bus_verify_polkit_async_full()
* to catch. In any case, it also means that the payload guarded by
* these polkit calls should never be executed, and hence we should
* never reach this point. */
assert(!error_or_denial);
return 0;
}
@@ -3754,6 +3768,7 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error
InhibitMode mm;
InhibitWhat w;
uid_t uid;
bool error_or_denial = false;
int r;
assert(message);
@@ -3801,12 +3816,26 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error
/* details= */ NULL,
&m->polkit_registry,
error);
if (r < 0)
return r;
if (r < 0) {
/* If we get -EALREADY, it means a polkit decision was made, but not for
* this action in particular. Assuming there are more actions requested,
* ignore that error and allow the decision to be revealed later. */
if ((~v & w) && r == -EALREADY)
error_or_denial = true;
else
return r;
}
if (r == 0)
return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
}
/* If error_or_denial was set above, it means that a polkit denial or
* error was deferred for a future call to bus_verify_polkit_async()
* to catch. In any case, it also means that the payload guarded by
* these polkit calls should never be executed, and hence we should
* never reach this point. */
assert(!error_or_denial);
r = sd_bus_query_sender_creds(message, SD_BUS_CREDS_EUID|SD_BUS_CREDS_PID|SD_BUS_CREDS_PIDFD, &creds);
if (r < 0)
return r;