From c7b7d74e81bae65ffef38f46dd6abf0b8a9c3d4f Mon Sep 17 00:00:00 2001 From: Xiang Fan Date: Wed, 24 Oct 2018 18:34:04 +0800 Subject: [PATCH] ask-password: check keyring in ask_password_tty and ask_password_agent A race condition happens when calling ask_password_auto() multiple times to unlock several disks on boot and effectively no password caching is utilized. This patch fixes it by polling the cache when waiting for the password. --- src/firstboot/firstboot.c | 12 +- src/shared/ask-password-api.c | 110 +++++++++++++----- src/shared/ask-password-api.h | 2 +- src/test/test-ask-password-api.c | 6 +- .../tty-ask-password-agent.c | 9 +- 5 files changed, 96 insertions(+), 43 deletions(-) diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 6a939aec046..c5deb66edf3 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -531,13 +531,17 @@ static int prompt_root_password(void) { msg2 = strjoina(special_glyph(TRIANGULAR_BULLET), " Please enter new root password again: "); for (;;) { - _cleanup_string_free_erase_ char *a = NULL, *b = NULL; + _cleanup_strv_free_erase_ char **a = NULL, **b = NULL; r = ask_password_tty(-1, msg1, NULL, 0, 0, NULL, &a); if (r < 0) return log_error_errno(r, "Failed to query root password: %m"); + if (strv_length(a) != 1) { + log_warning("Received multiple passwords, where we expected one."); + return -EINVAL; + } - if (isempty(a)) { + if (isempty(*a)) { log_warning("No password entered, skipping."); break; } @@ -546,12 +550,12 @@ static int prompt_root_password(void) { if (r < 0) return log_error_errno(r, "Failed to query root password: %m"); - if (!streq(a, b)) { + if (!streq(*a, *b)) { log_error("Entered passwords did not match, please try again."); continue; } - arg_root_password = TAKE_PTR(a); + arg_root_password = TAKE_PTR(*a); break; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 5f1c34c841d..df5ede77b95 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -27,6 +27,7 @@ #include "fd-util.h" #include "fileio.h" #include "format-util.h" +#include "fs-util.h" #include "io-util.h" #include "log.h" #include "macro.h" @@ -133,6 +134,9 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa (unsigned long) DIV_ROUND_UP(KEYRING_TIMEOUT_USEC, USEC_PER_SEC), 0, 0) < 0) log_debug_errno(errno, "Failed to adjust timeout: %m"); + /* Tell everyone to check the keyring */ + (void) touch("/run/systemd/ask-password"); + log_debug("Added key to keyring as %" PRIi32 ".", serial); return 1; @@ -211,7 +215,7 @@ int ask_password_tty( usec_t until, AskPasswordFlags flags, const char *flag_file, - char **ret) { + char ***ret) { enum { POLL_TTY, @@ -223,6 +227,7 @@ int ask_password_tty( _cleanup_close_ int cttyfd = -1, notify = -1; struct termios old_termios, new_termios; char passphrase[LINE_MAX + 1] = {}, *x; + _cleanup_strv_free_erase_ char **l = NULL; struct pollfd pollfd[_POLL_MAX]; size_t p = 0, codepoint = 0; int r; @@ -235,14 +240,25 @@ int ask_password_tty( if (!message) message = "Password:"; - if (flag_file) { + if (flag_file || ((flags & ASK_PASSWORD_ACCEPT_CACHED) && keyname)) { notify = inotify_init1(IN_CLOEXEC|IN_NONBLOCK); if (notify < 0) return -errno; - + } + if (flag_file) { if (inotify_add_watch(notify, flag_file, IN_ATTRIB /* for the link count */) < 0) return -errno; } + if ((flags & ASK_PASSWORD_ACCEPT_CACHED) && keyname) { + r = ask_password_keyring(keyname, flags, ret); + if (r >= 0) + return 0; + else if (r != -ENOKEY) + return r; + + if (inotify_add_watch(notify, "/run/systemd/ask-password", IN_ATTRIB /* for mtime */) < 0) + return -errno; + } /* If the caller didn't specify a TTY, then use the controlling tty, if we can. */ if (ttyfd < 0) @@ -324,9 +340,17 @@ int ask_password_tty( goto finish; } - if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) + if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0) { (void) flush_fd(notify); + r = ask_password_keyring(keyname, flags, ret); + if (r >= 0) { + r = 0; + goto finish; + } else if (r != -ENOKEY) + goto finish; + } + if (pollfd[POLL_TTY].revents == 0) continue; @@ -436,10 +460,16 @@ int ask_password_tty( goto finish; } - if (keyname) - (void) add_to_keyring_and_log(keyname, flags, STRV_MAKE(x)); + l = strv_new(x, NULL); + if (!l) { + r = -ENOMEM; + goto finish; + } - *ret = x; + if (keyname) + (void) add_to_keyring_and_log(keyname, flags, l); + + *ret = TAKE_PTR(l); r = 0; finish: @@ -495,14 +525,15 @@ int ask_password_agent( enum { FD_SOCKET, FD_SIGNAL, + FD_INOTIFY, _FD_MAX }; - _cleanup_close_ int socket_fd = -1, signal_fd = -1, fd = -1; + _cleanup_close_ int socket_fd = -1, signal_fd = -1, notify = -1, fd = -1; char temp[] = "/run/systemd/ask-password/tmp.XXXXXX"; char final[sizeof(temp)] = ""; _cleanup_free_ char *socket_name = NULL; - _cleanup_strv_free_ char **l = NULL; + _cleanup_strv_free_erase_ char **l = NULL; _cleanup_fclose_ FILE *f = NULL; struct pollfd pollfd[_FD_MAX]; sigset_t mask, oldmask; @@ -519,6 +550,25 @@ int ask_password_agent( (void) mkdir_p_label("/run/systemd/ask-password", 0755); + if ((flags & ASK_PASSWORD_ACCEPT_CACHED) && keyname) { + r = ask_password_keyring(keyname, flags, ret); + if (r >= 0) { + r = 0; + goto finish; + } else if (r != -ENOKEY) + goto finish; + + notify = inotify_init1(IN_CLOEXEC | IN_NONBLOCK); + if (notify < 0) { + r = -errno; + goto finish; + } + if (inotify_add_watch(notify, "/run/systemd/ask-password", IN_ATTRIB /* for mtime */) < 0) { + r = -errno; + goto finish; + } + } + fd = mkostemp_safe(temp); if (fd < 0) { r = fd; @@ -589,6 +639,8 @@ int ask_password_agent( pollfd[FD_SOCKET].events = POLLIN; pollfd[FD_SIGNAL].fd = signal_fd; pollfd[FD_SIGNAL].events = POLLIN; + pollfd[FD_INOTIFY].fd = notify; + pollfd[FD_INOTIFY].events = POLLIN; for (;;) { char passphrase[LINE_MAX+1]; @@ -610,7 +662,7 @@ int ask_password_agent( goto finish; } - k = poll(pollfd, _FD_MAX, until > 0 ? (int) ((until-t)/USEC_PER_MSEC) : -1); + k = poll(pollfd, notify >= 0 ? _FD_MAX : _FD_MAX - 1, until > 0 ? (int) ((until-t)/USEC_PER_MSEC) : -1); if (k < 0) { if (errno == EINTR) continue; @@ -629,6 +681,20 @@ int ask_password_agent( goto finish; } + if (notify >= 0 && pollfd[FD_INOTIFY].revents != 0) { + (void) flush_fd(notify); + + r = ask_password_keyring(keyname, flags, ret); + if (r >= 0) { + r = 0; + goto finish; + } else if (r != -ENOKEY) + goto finish; + } + + if (pollfd[FD_SOCKET].revents == 0) + continue; + if (pollfd[FD_SOCKET].revents != POLLIN) { r = -EIO; goto finish; @@ -736,29 +802,17 @@ int ask_password_auto( assert(ret); - if ((flags & ASK_PASSWORD_ACCEPT_CACHED) && keyname) { + if ((flags & ASK_PASSWORD_ACCEPT_CACHED) && + keyname && + ((flags & ASK_PASSWORD_NO_TTY) || !isatty(STDIN_FILENO)) && + (flags & ASK_PASSWORD_NO_AGENT)) { r = ask_password_keyring(keyname, flags, ret); if (r != -ENOKEY) return r; } - if (!(flags & ASK_PASSWORD_NO_TTY) && isatty(STDIN_FILENO)) { - char *s = NULL, **l = NULL; - - r = ask_password_tty(-1, message, keyname, until, flags, NULL, &s); - if (r < 0) - return r; - - r = strv_push(&l, s); - if (r < 0) { - string_erase(s); - free(s); - return -ENOMEM; - } - - *ret = l; - return 0; - } + if (!(flags & ASK_PASSWORD_NO_TTY) && isatty(STDIN_FILENO)) + return ask_password_tty(-1, message, keyname, until, flags, NULL, ret); if (!(flags & ASK_PASSWORD_NO_AGENT)) return ask_password_agent(message, icon, id, keyname, until, flags, ret); diff --git a/src/shared/ask-password-api.h b/src/shared/ask-password-api.h index 93ca8bff525..2d84ba6b042 100644 --- a/src/shared/ask-password-api.h +++ b/src/shared/ask-password-api.h @@ -15,7 +15,7 @@ typedef enum AskPasswordFlags { ASK_PASSWORD_CONSOLE_COLOR = 1 << 6, /* Use color if /dev/console points to a console that supports color */ } AskPasswordFlags; -int ask_password_tty(int tty_fd, const char *message, const char *keyname, usec_t until, AskPasswordFlags flags, const char *flag_file, char **ret); +int ask_password_tty(int tty_fd, const char *message, const char *keyname, usec_t until, AskPasswordFlags flags, const char *flag_file, char ***ret); int ask_password_agent(const char *message, const char *icon, const char *id, const char *keyname, usec_t until, AskPasswordFlags flag, char ***ret); int ask_password_keyring(const char *keyname, AskPasswordFlags flags, char ***ret); int ask_password_auto(const char *message, const char *icon, const char *id, const char *keyname, usec_t until, AskPasswordFlags flag, char ***ret); diff --git a/src/test/test-ask-password-api.c b/src/test/test-ask-password-api.c index ffd6da80fe2..23b06be19b7 100644 --- a/src/test/test-ask-password-api.c +++ b/src/test/test-ask-password-api.c @@ -3,15 +3,17 @@ #include "alloc-util.h" #include "ask-password-api.h" #include "log.h" +#include "strv.h" static void ask_password(void) { int r; - _cleanup_free_ char *ret; + _cleanup_strv_free_ char **ret = NULL; r = ask_password_tty(-1, "hello?", "da key", 0, 0, NULL, &ret); assert(r >= 0); + assert(strv_length(ret) == 1); - log_info("Got %s", ret); + log_info("Got %s", *ret); } int main(int argc, char **argv) { 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 088abecb7d9..ba2e1d37f0e 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -350,7 +350,6 @@ static int parse_password(const char *filename, char **wall) { if (arg_plymouth) r = ask_password_plymouth(message, not_after, accept_cached ? ASK_PASSWORD_ACCEPT_CACHED : 0, filename, &passwords); else { - char *password = NULL; int tty_fd = -1; if (arg_console) { @@ -368,18 +367,12 @@ static int parse_password(const char *filename, char **wall) { r = ask_password_tty(tty_fd, message, NULL, not_after, (echo ? ASK_PASSWORD_ECHO : 0) | (arg_console ? ASK_PASSWORD_CONSOLE_COLOR : 0), - filename, &password); + filename, &passwords); if (arg_console) { tty_fd = safe_close(tty_fd); release_terminal(); } - - if (r >= 0) - r = strv_push(&passwords, password); - - if (r < 0) - string_free_erase(password); } /* If the query went away, that's OK */