ask-password: various modernizations

Primarily clean-up error logging: log either all or no error messages in
the various functions. Mostly this means the actual password querying
calls no longer will log on their own, but the callers have to do so.

Contains various other fixes too, for example ports some code over to
use the clean-up macro.

Should contain no functional changes.

6 files changed, 127 insertions, 144 deletions

diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c
index abfd545c79..bf3fa30f69 100644
--- a/src/ask-password/ask-password.c
+++ b/src/ask-password/ask-password.c
@@ -20,15 +20,15 @@
 ***/
 
 #include <errno.h>
-#include <unistd.h>
 #include <getopt.h>
 #include <stddef.h>
+#include <unistd.h>
 
+#include "ask-password-api.h"
+#include "def.h"
 #include "log.h"
 #include "macro.h"
 #include "strv.h"
-#include "ask-password-api.h"
-#include "def.h"
 
 static const char *arg_icon = NULL;
 static const char *arg_id = NULL;
@@ -154,35 +154,33 @@ int main(int argc, char *argv[]) {
                 timeout = 0;
 
         if (arg_use_tty && isatty(STDIN_FILENO)) {
-                char *password = NULL;
+                _cleanup_free_ char *password = NULL;
 
-                r = ask_password_tty(arg_message, timeout, arg_echo, NULL,
-                                     &password);
-                if (r >= 0) {
-                        puts(password);
-                        free(password);
+                r = ask_password_tty(arg_message, timeout, arg_echo, NULL, &password);
+                if (r < 0) {
+                        log_error_errno(r, "Failed to ask for password on terminal: %m");
+                        goto finish;
                 }
 
+                puts(password);
         } else {
-                char **l;
+                _cleanup_free_ char **l = NULL;
+                char **p;
 
-                r = ask_password_agent(arg_message, arg_icon, arg_id, timeout,
-                                       arg_echo, arg_accept_cached, &l);
-                if (r >= 0) {
-                        char **p;
+                r = ask_password_agent(arg_message, arg_icon, arg_id, timeout, arg_echo, arg_accept_cached, &l);
+                if (r < 0) {
+                        log_error_errno(r, "Failed to ask for password via agent: %m");
+                        goto finish;
+                }
 
-                        STRV_FOREACH(p, l) {
-                                puts(*p);
+                STRV_FOREACH(p, l) {
+                        puts(*p);
 
-                                if (!arg_multiple)
-                                        break;
-                        }
-
-                        strv_free(l);
+                        if (!arg_multiple)
+                                break;
                 }
         }
 
 finish:
-
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c
index 4f0c669ca1..8c0724a0e7 100644
--- a/src/firstboot/firstboot.c
+++ b/src/firstboot/firstboot.c
@@ -477,9 +477,8 @@ static int prompt_root_password(void) {
 
                 r = ask_password_tty(msg2, 0, false, NULL, &b);
                 if (r < 0) {
-                        log_error_errno(r, "Failed to query root password: %m");
                         clear_string(a);
-                        return r;
+                        return log_error_errno(r, "Failed to query root password: %m");
                 }
 
                 if (!streq(a, b)) {
diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c
index b02cdf9a17..314f983b20 100644
--- a/src/shared/ask-password-api.c
+++ b/src/shared/ask-password-api.c
@@ -18,26 +18,28 @@
   You should have received a copy of the GNU Lesser General Public License
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
-#include <stdbool.h>
-#include <termios.h>
-#include <unistd.h>
-#include <poll.h>
-#include <sys/inotify.h>
+
 #include <errno.h>
 #include <fcntl.h>
-#include <sys/socket.h>
-#include <string.h>
-#include <sys/un.h>
+#include <poll.h>
+#include <stdbool.h>
 #include <stddef.h>
+#include <string.h>
+#include <sys/inotify.h>
 #include <sys/signalfd.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <termios.h>
+#include <unistd.h>
 
-#include "util.h"
 #include "formats-util.h"
 #include "mkdir.h"
-#include "strv.h"
 #include "random-util.h"
-#include "terminal-util.h"
 #include "signal-util.h"
+#include "socket-util.h"
+#include "strv.h"
+#include "terminal-util.h"
+#include "util.h"
 #include "ask-password-api.h"
 
 static void backspace_chars(int ttyfd, size_t p) {
@@ -97,10 +99,10 @@ int ask_password_tty(
                         goto finish;
                 }
 
-                loop_write(ttyfd, ANSI_HIGHLIGHT, sizeof(ANSI_HIGHLIGHT)-1, false);
+                loop_write(ttyfd, ANSI_HIGHLIGHT, strlen(ANSI_HIGHLIGHT), false);
                 loop_write(ttyfd, message, strlen(message), false);
                 loop_write(ttyfd, " ", 1, false);
-                loop_write(ttyfd, ANSI_NORMAL, sizeof(ANSI_NORMAL)-1, false);
+                loop_write(ttyfd, ANSI_NORMAL, strlen(ANSI_NORMAL), false);
 
                 new_termios = old_termios;
                 new_termios.c_lflag &= ~(ICANON|ECHO);
@@ -247,52 +249,38 @@ finish:
 }
 
 static int create_socket(char **name) {
-        int fd;
-        union {
-                struct sockaddr sa;
-                struct sockaddr_un un;
-        } sa = {
+        union sockaddr_union sa = {
                 .un.sun_family = AF_UNIX,
         };
-        int one = 1;
-        int r = 0;
+        _cleanup_close_ int fd = -1;
+        static const int one = 1;
         char *c;
+        int r;
 
         assert(name);
 
         fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0);
         if (fd < 0)
-                return log_error_errno(errno, "socket() failed: %m");
+                return -errno;
 
         snprintf(sa.un.sun_path, sizeof(sa.un.sun_path)-1, "/run/systemd/ask-password/sck.%" PRIx64, random_u64());
 
         RUN_WITH_UMASK(0177) {
-                r = bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path));
+                if (bind(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path)) < 0)
+                        return -errno;
         }
 
-        if (r < 0) {
-                r = -errno;
-                log_error_errno(errno, "bind(%s) failed: %m", sa.un.sun_path);
-                goto fail;
-        }
-
-        if (setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0) {
-                r = -errno;
-                log_error_errno(errno, "SO_PASSCRED failed: %m");
-                goto fail;
-        }
+        if (setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0)
+                return -errno;
 
         c = strdup(sa.un.sun_path);
-        if (!c) {
-                r = log_oom();
-                goto fail;
-        }
+        if (!c)
+                return -ENOMEM;
 
         *name = c;
-        return fd;
 
-fail:
-        safe_close(fd);
+        r = fd;
+        fd = -1;
 
         return r;
 }
@@ -323,24 +311,24 @@ int ask_password_agent(
 
         assert(_passphrases);
 
+
         assert_se(sigemptyset(&mask) >= 0);
         assert_se(sigset_add_many(&mask, SIGINT, SIGTERM, -1) >= 0);
         assert_se(sigprocmask(SIG_BLOCK, &mask, &oldmask) >= 0);
 
-        mkdir_p_label("/run/systemd/ask-password", 0755);
+        (void) mkdir_p_label("/run/systemd/ask-password", 0755);
 
         fd = mkostemp_safe(temp, O_WRONLY|O_CLOEXEC);
         if (fd < 0) {
-                r = log_error_errno(errno,
-                                    "Failed to create password file: %m");
+                r = -errno;
                 goto finish;
         }
 
-        fchmod(fd, 0644);
+        (void) fchmod(fd, 0644);
 
         f = fdopen(fd, "w");
         if (!f) {
-                r = log_error_errno(errno, "Failed to allocate FILE: %m");
+                r = -errno;
                 goto finish;
         }
 
@@ -348,7 +336,7 @@ int ask_password_agent(
 
         signal_fd = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
         if (signal_fd < 0) {
-                r = log_error_errno(errno, "signalfd(): %m");
+                r = -errno;
                 goto finish;
         }
 
@@ -381,10 +369,8 @@ int ask_password_agent(
                 fprintf(f, "Id=%s\n", id);
 
         r = fflush_and_check(f);
-        if (r < 0) {
-                log_error_errno(r, "Failed to write query file: %m");
+        if (r < 0)
                 goto finish;
-        }
 
         memcpy(final, temp, sizeof(temp));
 
@@ -393,7 +379,7 @@ int ask_password_agent(
         final[sizeof(final)-9] = 'k';
 
         if (rename(temp, final) < 0) {
-                r = log_error_errno(errno, "Failed to rename query file: %m");
+                r = -errno;
                 goto finish;
         }
 
@@ -419,7 +405,6 @@ int ask_password_agent(
                 t = now(CLOCK_MONOTONIC);
 
                 if (until > 0 && until <= t) {
-                        log_notice("Timed out");
                         r = -ETIME;
                         goto finish;
                 }
@@ -429,12 +414,11 @@ int ask_password_agent(
                         if (errno == EINTR)
                                 continue;
 
-                        r = log_error_errno(errno, "poll() failed: %m");
+                        r = -errno;
                         goto finish;
                 }
 
                 if (k <= 0) {
-                        log_notice("Timed out");
                         r = -ETIME;
                         goto finish;
                 }
@@ -445,7 +429,6 @@ int ask_password_agent(
                 }
 
                 if (pollfd[FD_SOCKET].revents != POLLIN) {
-                        log_error("Unexpected poll() event.");
                         r = -EIO;
                         goto finish;
                 }
@@ -467,14 +450,14 @@ int ask_password_agent(
                             errno == EINTR)
                                 continue;
 
-                        r = log_error_errno(errno, "recvmsg() failed: %m");
+                        r = -errno;
                         goto finish;
                 }
 
                 cmsg_close_all(&msghdr);
 
                 if (n <= 0) {
-                        log_error("Message too short");
+                        log_debug("Message too short");
                         continue;
                 }
 
@@ -482,13 +465,13 @@ int ask_password_agent(
                     control.cmsghdr.cmsg_level != SOL_SOCKET ||
                     control.cmsghdr.cmsg_type != SCM_CREDENTIALS ||
                     control.cmsghdr.cmsg_len != CMSG_LEN(sizeof(struct ucred))) {
-                        log_warning("Received message without credentials. Ignoring.");
+                        log_debug("Received message without credentials. Ignoring.");
                         continue;
                 }
 
                 ucred = (struct ucred*) CMSG_DATA(&control.cmsghdr);
                 if (ucred->uid != 0) {
-                        log_warning("Got request from unprivileged user. Ignoring.");
+                        log_debug("Got request from unprivileged user. Ignoring.");
                         continue;
                 }
 
@@ -508,7 +491,7 @@ int ask_password_agent(
 
                         if (strv_length(l) <= 0) {
                                 strv_free(l);
-                                log_error("Invalid packet");
+                                log_debug("Invalid packet");
                                 continue;
                         }
 
@@ -518,7 +501,7 @@ int ask_password_agent(
                         r = -ECANCELED;
                         goto finish;
                 } else {
-                        log_error("Invalid packet");
+                        log_debug("Invalid packet");
                         continue;
                 }
 
@@ -529,25 +512,32 @@ int ask_password_agent(
 
 finish:
         if (socket_name)
-                unlink(socket_name);
+                (void) unlink(socket_name);
 
-        unlink(temp);
+        (void) unlink(temp);
 
         if (final[0])
-                unlink(final);
+                (void) unlink(final);
 
         assert_se(sigprocmask(SIG_SETMASK, &oldmask, NULL) == 0);
 
         return r;
 }
 
-int ask_password_auto(const char *message, const char *icon, const char *id,
-                      usec_t until, bool accept_cached, char ***_passphrases) {
+int ask_password_auto(
+                const char *message,
+                const char *icon,
+                const char *id,
+                usec_t until,
+                bool accept_cached,
+                char ***_passphrases) {
+
+        int r;
+
         assert(message);
         assert(_passphrases);
 
         if (isatty(STDIN_FILENO)) {
-                int r;
                 char *s = NULL, **l = NULL;
 
                 r = ask_password_tty(message, until, false, NULL, &s);
@@ -556,10 +546,11 @@ int ask_password_auto(const char *message, const char *icon, const char *id,
 
                 r = strv_consume(&l, s);
                 if (r < 0)
-                        return r;
+                        return -ENOMEM;
 
                 *_passphrases = l;
-                return r;
-        } else
-                return ask_password_agent(message, icon, id, until, false, accept_cached, _passphrases);
+                return 0;
+        }
+
+        return ask_password_agent(message, icon, id, until, false, accept_cached, _passphrases);
 }
diff --git a/src/shared/ask-password-api.h b/src/shared/ask-password-api.h
index ccb3be0fca..aeb045fe19 100644
--- a/src/shared/ask-password-api.h
+++ b/src/shared/ask-password-api.h
@@ -26,9 +26,5 @@
 #include "time-util.h"
 
 int ask_password_tty(const char *message, usec_t until, bool echo, const char *flag_file, char **_passphrase);
-
-int ask_password_agent(const char *message, const char *icon, const char *id,
-                       usec_t until, bool echo, bool accept_cached, char ***_passphrases);
-
-int ask_password_auto(const char *message, const char *icon, const char *id,
-                      usec_t until, bool accept_cached, char ***_passphrases);
+int ask_password_agent(const char *message, const char *icon, const char *id, usec_t until, bool echo, bool accept_cached, char ***_passphrases);
+int ask_password_auto(const char *message, const char *icon, const char *id, usec_t until, bool accept_cached, char ***_passphrases);
diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c
index 70466d17e5..29db855c67 100644
--- a/src/shared/spawn-ask-password-agent.c
+++ b/src/shared/spawn-ask-password-agent.c
@@ -19,13 +19,13 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
+#include <signal.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <signal.h>
 
 #include "log.h"
-#include "util.h"
 #include "process-util.h"
+#include "util.h"
 #include "spawn-ask-password-agent.h"
 
 static pid_t agent_pid = 0;
@@ -46,9 +46,9 @@ int ask_password_agent_open(void) {
                        SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH,
                        SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH, "--watch", NULL);
         if (r < 0)
-                log_error_errno(r, "Failed to fork TTY ask password agent: %m");
+                return log_error_errno(r, "Failed to fork TTY ask password agent: %m");
 
-        return r;
+        return 1;
 }
 
 void ask_password_agent_close(void) {
@@ -57,8 +57,8 @@ void ask_password_agent_close(void) {
                 return;
 
         /* Inform agent that we are done */
-        kill(agent_pid, SIGTERM);
-        kill(agent_pid, SIGCONT);
+        (void) kill(agent_pid, SIGTERM);
+        (void) kill(agent_pid, SIGCONT);
         (void) wait_for_terminate(agent_pid, NULL);
         agent_pid = 0;
 }
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index 4630eb94f1..e1e2945c06 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -93,17 +93,16 @@ static int ask_password_plymouth(
 
         r = connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sa.un.sun_path+1));
         if (r < 0)
-                return log_error_errno(errno, "Failed to connect to Plymouth: %m");
+                return -errno;
 
         if (accept_cached) {
                 packet = strdup("c");
                 n = 1;
-        } else if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1),
-                            message, &n) < 0)
+        } else if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0)
                 packet = NULL;
 
         if (!packet)
-                return log_oom();
+                return -ENOMEM;
 
         r = loop_write(fd, packet, n + 1, true);
         if (r < 0)
@@ -305,19 +304,19 @@ static int parse_password(const char *filename, char **wall) {
                         }
 
                 } else {
-                        int tty_fd = -1;
                         _cleanup_free_ char *password = NULL;
+                        int tty_fd = -1;
 
                         if (arg_console) {
                                 tty_fd = acquire_terminal("/dev/console", false, false, false, USEC_INFINITY);
                                 if (tty_fd < 0)
-                                        return tty_fd;
+                                        return log_error_errno(tty_fd, "Failed to acquire /dev/console: %m");
                         }
 
                         r = ask_password_tty(message, not_after, echo, filename, &password);
 
                         if (arg_console) {
-                                safe_close(tty_fd);
+                                tty_fd = safe_close(tty_fd);
                                 release_terminal();
                         }
 
@@ -347,12 +346,9 @@ static int parse_password(const char *filename, char **wall) {
                 sa.un.sun_family = AF_UNIX;
                 strncpy(sa.un.sun_path, socket_name, sizeof(sa.un.sun_path));
 
-                r = sendto(socket_fd, packet, packet_length, MSG_NOSIGNAL, &sa.sa,
-                           offsetof(struct sockaddr_un, sun_path) + strlen(socket_name));
-                if (r < 0) {
-                        log_error_errno(errno, "Failed to send: %m");
-                        return r;
-                }
+                r = sendto(socket_fd, packet, packet_length, MSG_NOSIGNAL, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(socket_name));
+                if (r < 0)
+                        return log_error_errno(errno, "Failed to send: %m");
         }
 
         return 0;
@@ -360,40 +356,43 @@ static int parse_password(const char *filename, char **wall) {
 
 static int wall_tty_block(void) {
         _cleanup_free_ char *p = NULL;
-        int fd, r;
         dev_t devnr;
+        int fd, r;
 
         r = get_ctty_devnr(0, &devnr);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to get controlling TTY: %m");
 
         if (asprintf(&p, "/run/systemd/ask-password-block/%u:%u", major(devnr), minor(devnr)) < 0)
-                return -ENOMEM;
+                return log_oom();
 
         mkdir_parents_label(p, 0700);
         mkfifo(p, 0600);
 
         fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
         if (fd < 0)
-                return -errno;
+                return log_error_errno(errno, "Failed to open %s: %m", p);
 
         return fd;
 }
 
 static bool wall_tty_match(const char *path, void *userdata) {
-        int fd, r;
-        struct stat st;
         _cleanup_free_ char *p = NULL;
+        _cleanup_close_ int fd = -1;
+        struct stat st;
 
         if (!path_is_absolute(path))
                 path = strjoina("/dev/", path);
 
-        r = lstat(path, &st);
-        if (r < 0)
+        if (lstat(path, &st) < 0) {
+                log_debug_errno(errno, "Failed to stat %s: %m", path);
                 return true;
+        }
 
-        if (!S_ISCHR(st.st_mode))
+        if (!S_ISCHR(st.st_mode)) {
+                log_debug("%s is not a character device.", path);
                 return true;
+        }
 
         /* We use named pipes to ensure that wall messages suggesting
          * password entry are not printed over password prompts
@@ -403,16 +402,19 @@ static bool wall_tty_match(const char *path, void *userdata) {
          * advantage that the block will automatically go away if the
          * process dies. */
 
-        if (asprintf(&p, "/run/systemd/ask-password-block/%u:%u", major(st.st_rdev), minor(st.st_rdev)) < 0)
+        if (asprintf(&p, "/run/systemd/ask-password-block/%u:%u", major(st.st_rdev), minor(st.st_rdev)) < 0) {
+                log_oom();
                 return true;
+        }
 
         fd = open(p, O_WRONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
-        if (fd < 0)
-                return true;
+        if (fd < 0) {
+                log_debug_errno(errno, "Failed top open the wall pipe: %m");
+                return 1;
+        }
 
         /* What, we managed to open the pipe? Then this tty is filtered. */
-        safe_close(fd);
-        return false;
+        return 0;
 }
 
 static int show_passwords(void) {
@@ -425,11 +427,10 @@ static int show_passwords(void) {
                 if (errno == ENOENT)
                         return 0;
 
-                log_error_errno(errno, "opendir(/run/systemd/ask-password): %m");
-                return -errno;
+                return log_error_errno(errno, "Failed top open /run/systemd/ask-password: %m");
         }
 
-        while ((de = readdir(d))) {
+        FOREACH_DIRENT_ALL(de, d, return log_error_errno(errno, "Failed to read directory: %m")) {
                 _cleanup_free_ char *p = NULL, *wall = NULL;
                 int q;
 
@@ -454,7 +455,7 @@ static int show_passwords(void) {
                         r = q;
 
                 if (wall)
-                        utmp_wall(wall, NULL, NULL, wall_tty_match, NULL);
+                        (void) utmp_wall(wall, NULL, NULL, wall_tty_match, NULL);
         }
 
         return r;
@@ -474,14 +475,14 @@ static int watch_passwords(void) {
 
         tty_block_fd = wall_tty_block();
 
-        mkdir_p_label("/run/systemd/ask-password", 0755);
+        (void) mkdir_p_label("/run/systemd/ask-password", 0755);
 
         notify = inotify_init1(IN_CLOEXEC);
         if (notify < 0)
-                return -errno;
+                return log_error_errno(errno, "Failed to allocate directory watch: %m");
 
         if (inotify_add_watch(notify, "/run/systemd/ask-password", IN_CLOSE_WRITE|IN_MOVED_TO) < 0)
-                return -errno;
+                return log_error_errno(errno, "Failed to add /run/systemd/ask-password to directory watch: %m");
 
         assert_se(sigemptyset(&mask) >= 0);
         assert_se(sigset_add_many(&mask, SIGINT, SIGTERM, -1) >= 0);
@@ -489,7 +490,7 @@ static int watch_passwords(void) {
 
         signal_fd = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
         if (signal_fd < 0)
-                return -errno;
+                return log_error_errno(errno, "Failed to allocate signal file descriptor: %m");
 
         pollfd[FD_INOTIFY].fd = notify;
         pollfd[FD_INOTIFY].events = POLLIN;
@@ -509,7 +510,7 @@ static int watch_passwords(void) {
                 }
 
                 if (pollfd[FD_INOTIFY].revents != 0)
-                        flush_fd(notify);
+                        (void) flush_fd(notify);
 
                 if (pollfd[FD_SIGNAL].revents != 0)
                         break;
@@ -633,8 +634,6 @@ int main(int argc, char *argv[]) {
                 r = watch_passwords();
         else
                 r = show_passwords();
-        if (r < 0)
-                log_error_errno(r, "Error: %m");
 
 finish:
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;