From 16f017fa6e3417edb4bba5db13a623f7289a3abc Mon Sep 17 00:00:00 2001 From: Ivan Shapovalov Date: Mon, 22 Jun 2015 19:04:38 +0300 Subject: systemctl: legacy tools shall never be interactive Fixes (the main concern of) issue #213. --- src/systemctl/systemctl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 74d7fc2b3b..16a8eef308 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -100,7 +100,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; @@ -6340,6 +6340,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) { -- cgit v1.2.3-54-g00ecf From 4c315c2c7c859df318f70e7aa59697a6781c92aa Mon Sep 17 00:00:00 2001 From: Ivan Shapovalov Date: Wed, 9 Sep 2015 16:01:10 +0300 Subject: systemctl: minor: use IN_SET() instead of repeated equality checks. --- src/systemctl/systemctl.c | 73 +++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 31 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 16a8eef308..22455b5475 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1299,10 +1299,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) { @@ -2898,10 +2899,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); @@ -3022,26 +3024,29 @@ 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)) return r; @@ -3559,12 +3564,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[] = " "; @@ -4467,7 +4474,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; @@ -5134,9 +5143,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 @@ -5636,10 +5646,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) -- cgit v1.2.3-54-g00ecf From a9085ea35628fa0bfeb3b48fb53f7b823081ad09 Mon Sep 17 00:00:00 2001 From: Ivan Shapovalov Date: Wed, 9 Sep 2015 16:04:35 +0300 Subject: systemctl: fix logind bus call error handling in halt_main() Handle -EOPNOTSUPP and -EINPROGRESS like in start_special(). --- src/systemctl/systemctl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 22455b5475..bde3607e21 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3048,8 +3048,12 @@ static int start_special(sd_bus *bus, char **args) { 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); @@ -7378,6 +7382,10 @@ static int halt_main(sd_bus *bus) { 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."); -- cgit v1.2.3-54-g00ecf From 2ac3930f5b5fb06c7bd959c2ea8667554fceccfe Mon Sep 17 00:00:00 2001 From: Ivan Shapovalov Date: Wed, 9 Sep 2015 16:06:49 +0300 Subject: systemctl: relax permission checks in halt_main() Thus we allow (non-interactive) polkit auth to kick in for legacy commands (halt, poweroff, reboot, telinit) as well. Fixes (another aspect of) issue #213. --- src/systemctl/systemctl.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index bde3607e21..9822c70a0d 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -7371,14 +7371,19 @@ 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; @@ -7387,9 +7392,6 @@ static int halt_main(sd_bus *bus) { return r; /* on all other errors, try low-level operation */ } - - log_error("Must be root."); - return -EPERM; } if (arg_when > 0) { @@ -7398,6 +7400,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; @@ -7473,6 +7477,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."); -- cgit v1.2.3-54-g00ecf From 58158dc765b4ee7089ef9fd2405c48497acf5435 Mon Sep 17 00:00:00 2001 From: Ivan Shapovalov Date: Thu, 10 Sep 2015 00:41:46 +0300 Subject: systemctl: improve clarity of error messages in the logind path --- src/systemctl/systemctl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'src/systemctl') diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9822c70a0d..20ded4dc0a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2763,7 +2763,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) @@ -2775,22 +2775,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: @@ -2834,7 +2839,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 -- cgit v1.2.3-54-g00ecf