From 39e96f844a467400cda1dd24401b2c8fbe480eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 09:19:46 +0100 Subject: [PATCH 1/3] firstboot: add missing check If we check length of 'a', we must do the same for 'b'. --- src/firstboot/firstboot.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 8001f90d57..901fbf0815 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -550,10 +550,9 @@ static int prompt_root_password(void) { 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 (strv_length(a) != 1) + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Received multiple passwords, where we expected one."); if (isempty(*a)) { log_warning("No password entered, skipping."); @@ -563,6 +562,9 @@ static int prompt_root_password(void) { r = ask_password_tty(-1, msg2, NULL, 0, 0, NULL, &b); if (r < 0) return log_error_errno(r, "Failed to query root password: %m"); + if (strv_length(b) != 1) + return log_error_errno(SYNTHETIC_ERRNO(EIO), + "Received multiple passwords, where we expected one."); if (!streq(*a, *b)) { log_error("Entered passwords did not match, please try again."); From 72c08a471c9ccb65209b72ea539acefe17c33245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 09:32:16 +0100 Subject: [PATCH 2/3] shared/ask-password-api: return "error" when dialogue is cancelled test-ask-password-api would crash if ^D was pressed. If think the callers generally expect a non-empty strv as reply. Let's return an error if we have nothing to return. Also modernize test-ask-password-api a bit. --- src/shared/ask-password-api.c | 12 ++++++++---- src/test/test-ask-password-api.c | 20 +++++++++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 7f6775f4a1..a6c83e181a 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -658,11 +658,15 @@ int ask_password_tty( goto finish; skipped: - if (keyname) - (void) add_to_keyring_and_log(keyname, flags, l); + if (strv_isempty(l)) + r = log_debug_errno(SYNTHETIC_ERRNO(ECANCELED), "Password query was cancelled."); + else { + if (keyname) + (void) add_to_keyring_and_log(keyname, flags, l); - *ret = TAKE_PTR(l); - r = 0; + *ret = TAKE_PTR(l); + r = 0; + } finish: if (ttyfd >= 0 && reset_tty) { diff --git a/src/test/test-ask-password-api.c b/src/test/test-ask-password-api.c index fa91869cf5..13a1064b45 100644 --- a/src/test/test-ask-password-api.c +++ b/src/test/test-ask-password-api.c @@ -1,24 +1,26 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ -#include "alloc-util.h" #include "ask-password-api.h" -#include "log.h" #include "strv.h" +#include "tests.h" -static void ask_password(void) { +static void test_ask_password(void) { int r; _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); + if (r == -ECANCELED) + assert_se(ret == NULL); + else { + assert_se(r >= 0); + assert_se(strv_length(ret) == 1); + log_info("Got \"%s\"", *ret); + } } int main(int argc, char **argv) { - log_parse_environment(); + test_setup_logging(LOG_DEBUG); - ask_password(); + test_ask_password(); return EXIT_SUCCESS; } From 8aaf18e08a2ee95a1d2687fbfdb584548375b5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 6 Feb 2020 09:50:35 +0100 Subject: [PATCH 3/3] shared/ask-password-api: show "(press TAB for no echo)" For #8495: it is arguably useful to not show the length of the password in public spaces. It is possible to press TAB or BS to cancel the asterisks, but this is not very discoverable. Let's make it discoverable by showing a message (in gray). The message is "erased" after the first character is entered. --- src/shared/ask-password-api.c | 25 +++++++++++++++++++++---- src/test/test-ask-password-api.c | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index a6c83e181a..7abe7d1711 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -394,6 +394,10 @@ finish: return r; } +#define NO_ECHO "(no echo) " +#define PRESS_TAB "(press TAB for no echo) " +#define SKIPPED "(skipped)" + int ask_password_tty( int ttyfd, const char *message, @@ -409,7 +413,7 @@ int ask_password_tty( _POLL_MAX, }; - bool reset_tty = false, dirty = false, use_color = false; + bool reset_tty = false, dirty = false, use_color = false, press_tab_visible = false; _cleanup_close_ int cttyfd = -1, notify = -1; struct termios old_termios, new_termios; char passphrase[LINE_MAX + 1] = {}, *x; @@ -465,6 +469,13 @@ int ask_password_tty( (void) loop_write(ttyfd, message, strlen(message), false); (void) loop_write(ttyfd, " ", 1, false); + if (!(flags & ASK_PASSWORD_SILENT)) { + if (use_color) + (void) loop_write(ttyfd, ANSI_GREY, STRLEN(ANSI_GREY), false); + (void) loop_write(ttyfd, PRESS_TAB, strlen(PRESS_TAB), false); + press_tab_visible = true; + } + if (use_color) (void) loop_write(ttyfd, ANSI_NORMAL, STRLEN(ANSI_NORMAL), false); @@ -550,13 +561,19 @@ int ask_password_tty( } + if (press_tab_visible) { + assert(ttyfd >= 0); + backspace_chars(ttyfd, strlen(PRESS_TAB)); + press_tab_visible = false; + } + /* We treat EOF, newline and NUL byte all as valid end markers */ if (n == 0 || c == '\n' || c == 0) break; if (c == 4) { /* C-d also known as EOT */ if (ttyfd >= 0) - (void) loop_write(ttyfd, "(skipped)", 9, false); + (void) loop_write(ttyfd, SKIPPED, strlen(SKIPPED), false); goto skipped; } @@ -606,7 +623,7 @@ int ask_password_tty( * first key (and only as first key), or ... */ if (ttyfd >= 0) - (void) loop_write(ttyfd, "(no echo) ", 10, false); + (void) loop_write(ttyfd, NO_ECHO, strlen(NO_ECHO), false); } else if (ttyfd >= 0) (void) loop_write(ttyfd, "\a", 1, false); @@ -619,7 +636,7 @@ int ask_password_tty( /* ... or by pressing TAB at any time. */ if (ttyfd >= 0) - (void) loop_write(ttyfd, "(no echo) ", 10, false); + (void) loop_write(ttyfd, NO_ECHO, strlen(NO_ECHO), false); } else if (p >= sizeof(passphrase)-1) { diff --git a/src/test/test-ask-password-api.c b/src/test/test-ask-password-api.c index 13a1064b45..098bb35bce 100644 --- a/src/test/test-ask-password-api.c +++ b/src/test/test-ask-password-api.c @@ -8,7 +8,7 @@ static void test_ask_password(void) { int r; _cleanup_strv_free_ char **ret = NULL; - r = ask_password_tty(-1, "hello?", "da key", 0, 0, NULL, &ret); + r = ask_password_tty(-1, "hello?", "da key", 0, ASK_PASSWORD_CONSOLE_COLOR, NULL, &ret); if (r == -ECANCELED) assert_se(ret == NULL); else {