Merge pull request #1227 from intelfx/systemctl-legacy-tools-polkit

systemctl: fix various aspects of polkit authorization in legacy tools.

1 files changed, 76 insertions, 43 deletions

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index e265d0fd13..d21ba9a566 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -101,7 +101,7 @@ static bool arg_quiet = false;
 static bool arg_full = false;
 static bool arg_recursive = false;
 static int arg_force = 0;
-static bool arg_ask_password = true;
+static bool arg_ask_password = false;
 static bool arg_runtime = false;
 static UnitFilePresetMode arg_preset_mode = UNIT_FILE_PRESET_FULL;
 static char **arg_wall = NULL;
@@ -1300,10 +1300,11 @@ static void output_unit_file_list(const UnitFileList *units, unsigned c) {
                 const char *on, *off;
                 const char *id;
 
-                if (u->state == UNIT_FILE_MASKED ||
-                    u->state == UNIT_FILE_MASKED_RUNTIME ||
-                    u->state == UNIT_FILE_DISABLED ||
-                    u->state == UNIT_FILE_INVALID) {
+                if (IN_SET(u->state,
+                           UNIT_FILE_MASKED,
+                           UNIT_FILE_MASKED_RUNTIME,
+                           UNIT_FILE_DISABLED,
+                           UNIT_FILE_INVALID)) {
                         on  = ansi_highlight_red();
                         off = ansi_highlight_off();
                 } else if (u->state == UNIT_FILE_ENABLED) {
@@ -2765,7 +2766,7 @@ static int start_unit(sd_bus *bus, char **args) {
 static int reboot_with_logind(sd_bus *bus, enum action a) {
 #ifdef HAVE_LOGIND
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
-        const char *method;
+        const char *method, *description;
         int r;
 
         if (!bus)
@@ -2777,22 +2778,27 @@ static int reboot_with_logind(sd_bus *bus, enum action a) {
 
         case ACTION_REBOOT:
                 method = "Reboot";
+                description = "reboot system";
                 break;
 
         case ACTION_POWEROFF:
                 method = "PowerOff";
+                description = "power off system";
                 break;
 
         case ACTION_SUSPEND:
                 method = "Suspend";
+                description = "suspend system";
                 break;
 
         case ACTION_HIBERNATE:
                 method = "Hibernate";
+                description = "hibernate system";
                 break;
 
         case ACTION_HYBRID_SLEEP:
                 method = "HybridSleep";
+                description = "put system into hybrid sleep";
                 break;
 
         default:
@@ -2836,7 +2842,7 @@ static int reboot_with_logind(sd_bus *bus, enum action a) {
                         NULL,
                         "b", arg_ask_password);
         if (r < 0)
-                log_error("Failed to execute operation: %s", bus_error_message(&error, r));
+                log_error("Failed to %s via logind: %s", description, bus_error_message(&error, r));
 
         return r;
 #else
@@ -2901,10 +2907,11 @@ static int check_inhibitors(sd_bus *bus, enum action a) {
                         return log_error_errno(ERANGE, "Bad PID %"PRIu32": %m", pid);
 
                 if (!strv_contains(sv,
-                                  a == ACTION_HALT ||
-                                  a == ACTION_POWEROFF ||
-                                  a == ACTION_REBOOT ||
-                                  a == ACTION_KEXEC ? "shutdown" : "sleep"))
+                                   IN_SET(a,
+                                          ACTION_HALT,
+                                          ACTION_POWEROFF,
+                                          ACTION_REBOOT,
+                                          ACTION_KEXEC) ? "shutdown" : "sleep"))
                         continue;
 
                 get_process_comm(pid, &comm);
@@ -3025,29 +3032,36 @@ static int start_special(sd_bus *bus, char **args) {
         }
 
         if (arg_force >= 2 &&
-            (a == ACTION_HALT ||
-             a == ACTION_POWEROFF ||
-             a == ACTION_REBOOT))
+            IN_SET(a,
+                   ACTION_HALT,
+                   ACTION_POWEROFF,
+                   ACTION_REBOOT))
                 return halt_now(a);
 
         if (arg_force >= 1 &&
-            (a == ACTION_HALT ||
-             a == ACTION_POWEROFF ||
-             a == ACTION_REBOOT ||
-             a == ACTION_KEXEC ||
-             a == ACTION_EXIT))
+            IN_SET(a,
+                   ACTION_HALT,
+                   ACTION_POWEROFF,
+                   ACTION_REBOOT,
+                   ACTION_KEXEC,
+                   ACTION_EXIT))
                 return daemon_reload(bus, args);
 
         /* first try logind, to allow authentication with polkit */
         if (geteuid() != 0 &&
-            (a == ACTION_POWEROFF ||
-             a == ACTION_REBOOT ||
-             a == ACTION_SUSPEND ||
-             a == ACTION_HIBERNATE ||
-             a == ACTION_HYBRID_SLEEP)) {
+            IN_SET(a,
+                   ACTION_POWEROFF,
+                   ACTION_REBOOT,
+                   ACTION_SUSPEND,
+                   ACTION_HIBERNATE,
+                   ACTION_HYBRID_SLEEP)) {
                 r = reboot_with_logind(bus, a);
-                if (r >= 0 || IN_SET(r, -EOPNOTSUPP, -EINPROGRESS))
+                if (r >= 0)
                         return r;
+                if (IN_SET(r, -EOPNOTSUPP, -EINPROGRESS))
+                        /* requested operation is not supported or already in progress */
+                        return r;
+                /* on all other errors, try low-level operation */
         }
 
         r = start_unit(bus, args);
@@ -3573,12 +3587,14 @@ static void print_status_info(
 
         if (i->control_group &&
             (i->main_pid > 0 || i->control_pid > 0 ||
-             ((arg_transport != BUS_TRANSPORT_LOCAL && arg_transport != BUS_TRANSPORT_MACHINE) || cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, i->control_group) == 0))) {
+             (!IN_SET(arg_transport, BUS_TRANSPORT_LOCAL, BUS_TRANSPORT_MACHINE) || cg_is_empty_recursive(SYSTEMD_CGROUP_CONTROLLER, i->control_group) == 0))) {
                 unsigned c;
 
                 printf("   CGroup: %s\n", i->control_group);
 
-                if (arg_transport == BUS_TRANSPORT_LOCAL || arg_transport == BUS_TRANSPORT_MACHINE) {
+                if (IN_SET(arg_transport,
+                           BUS_TRANSPORT_LOCAL,
+                           BUS_TRANSPORT_MACHINE)) {
                         unsigned k = 0;
                         pid_t extra[2];
                         static const char prefix[] = "           ";
@@ -4487,7 +4503,9 @@ static int show_system_status(sd_bus *bus) {
                format_timestamp_relative(since1, sizeof(since1), mi.timestamp));
 
         printf("   CGroup: %s\n", mi.control_group ?: "/");
-        if (arg_transport == BUS_TRANSPORT_LOCAL || arg_transport == BUS_TRANSPORT_MACHINE) {
+        if (IN_SET(arg_transport,
+                   BUS_TRANSPORT_LOCAL,
+                   BUS_TRANSPORT_MACHINE)) {
                 static const char prefix[] = "           ";
                 unsigned c;
 
@@ -5154,9 +5172,10 @@ static int enable_sysv_units(const char *verb, char **args) {
         if (arg_scope != UNIT_FILE_SYSTEM)
                 return 0;
 
-        if (!streq(verb, "enable") &&
-            !streq(verb, "disable") &&
-            !streq(verb, "is-enabled"))
+        if (!STR_IN_SET(verb,
+                        "enable",
+                        "disable",
+                        "is-enabled"))
                 return 0;
 
         /* Processes all SysV units, and reshuffles the array so that
@@ -5656,10 +5675,11 @@ static int unit_is_enabled(sd_bus *bus, char **args) {
                         if (state < 0)
                                 return log_error_errno(state, "Failed to get unit file state for %s: %m", *name);
 
-                        if (state == UNIT_FILE_ENABLED ||
-                            state == UNIT_FILE_ENABLED_RUNTIME ||
-                            state == UNIT_FILE_STATIC ||
-                            state == UNIT_FILE_INDIRECT)
+                        if (IN_SET(state,
+                                   UNIT_FILE_ENABLED,
+                                   UNIT_FILE_ENABLED_RUNTIME,
+                                   UNIT_FILE_STATIC,
+                                   UNIT_FILE_INDIRECT))
                                 enabled = true;
 
                         if (!arg_quiet)
@@ -6360,6 +6380,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
         assert(argc >= 0);
         assert(argv);
 
+        /* we default to allowing interactive authorization only in systemctl (not in the legacy commands) */
+        arg_ask_password = true;
+
         while ((c = getopt_long(argc, argv, "ht:p:alqfs:H:M:n:o:ir", options, NULL)) >= 0)
 
                 switch (c) {
@@ -7373,21 +7396,27 @@ static int halt_main(sd_bus *bus) {
                 return r;
 
         if (geteuid() != 0) {
+                if (arg_when > 0 ||
+                    arg_dry ||
+                    arg_force > 0) {
+                        log_error("Must be root.");
+                        return -EPERM;
+                }
+
                 /* Try logind if we are a normal user and no special
                  * mode applies. Maybe PolicyKit allows us to shutdown
                  * the machine. */
-
-                if (arg_when <= 0 &&
-                    arg_force <= 0 &&
-                    (arg_action == ACTION_POWEROFF ||
-                     arg_action == ACTION_REBOOT)) {
+                if (IN_SET(arg_action,
+                           ACTION_POWEROFF,
+                           ACTION_REBOOT)) {
                         r = reboot_with_logind(bus, arg_action);
                         if (r >= 0)
                                 return r;
+                        if (IN_SET(r, -EOPNOTSUPP, -EINPROGRESS))
+                                /* requested operation is not supported or already in progress */
+                                return r;
+                        /* on all other errors, try low-level operation */
                 }
-
-                log_error("Must be root.");
-                return -EPERM;
         }
 
         if (arg_when > 0) {
@@ -7396,6 +7425,8 @@ static int halt_main(sd_bus *bus) {
                 _cleanup_free_ char *m = NULL;
                 const char *action;
 
+                assert(geteuid() == 0);
+
                 if (avoid_bus()) {
                         log_error("Unable to perform operation without bus connection.");
                         return -ENOSYS;
@@ -7471,6 +7502,8 @@ static int halt_main(sd_bus *bus) {
         if (!arg_dry && !arg_force)
                 return start_with_fallback(bus);
 
+        assert(geteuid() == 0);
+
         if (!arg_no_wtmp) {
                 if (sd_booted() > 0)
                         log_debug("Not writing utmp record, assuming that systemd-update-utmp is used.");