Merge pull request #19806 from poettering/ask-pw-asterisk

systemd-ask-password: make pw echo fully configurable

man/systemd-ask-password.xml

@@ -157,12 +157,23 @@
       </varlistentry>
 
       <varlistentry>
-        <term><option>--echo</option></term>
+        <term><option>--echo=yes|no|masked</option></term>
 
-        <listitem><para>Echo the user input instead of masking it.
+        <listitem><para>Controls whether to echo user input. Takes a boolean or the special string
-        This is useful when using
+        <literal>masked</literal>, the default being the latter. If enabled the typed characters are echoed
-        <filename>systemd-ask-password</filename> to query for
+        literally, which is useful for prompting for usernames and other non-protected data. If disabled the
-        usernames. </para></listitem>
+        typed characters are not echoed in any form, the user will not get feedback on their input. If set to
+        <literal>masked</literal>, an asterisk (<literal>*</literal>) is echoed for each character
+        typed. In this mode, if the user hits the tabulator key (<literal>↹</literal>), echo is turned
+        off. (Alternatively, if the user hits the backspace key (<literal>⌫</literal>) while no data has
+        been entered otherwise, echo is turned off, too).</para></listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term><option>--echo</option></term>
+        <term><option>-e</option></term>
+
+        <listitem><para>Equivalent to <option>--echo=yes</option>, see above.</para></listitem>
       </varlistentry>
 
       <varlistentry>
@@ -171,7 +182,7 @@
         <listitem><para>Controls whether or not to prefix the query with a
         lock and key emoji (🔐), if the TTY settings permit this. The default
         is <literal>auto</literal>, which defaults to <literal>yes</literal>,
-        unless <option>--echo</option> is given.</para></listitem>
+        unless <option>--echo=yes</option> is given.</para></listitem>
       </varlistentry>
 
       <varlistentry>

src/ask-password/ask-password.c

@@ -45,7 +45,9 @@ static int help(void) {
                "                      Credential name for LoadCredential=/SetCredential=\n"
                "                      credentials\n"
                "     --timeout=SEC    Timeout in seconds\n"
-               "     --echo           Do not mask input (useful for usernames)\n"
+               "     --echo=yes|no|masked\n"
+               "                      Control whether to show password while typing (echo)\n"
+               "  -e --echo           Equivalent to --echo=yes\n"
                "     --emoji=yes|no|auto\n"
                "                      Show a lock and key emoji\n"
                "     --no-tty         Ask question via agent even on TTY\n"
@@ -66,7 +68,6 @@ static int parse_argv(int argc, char *argv[]) {
         enum {
                 ARG_ICON = 0x100,
                 ARG_TIMEOUT,
-                ARG_ECHO,
                 ARG_EMOJI,
                 ARG_NO_TTY,
                 ARG_ACCEPT_CACHED,
@@ -83,7 +84,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "version",       no_argument,       NULL, ARG_VERSION       },
                 { "icon",          required_argument, NULL, ARG_ICON          },
                 { "timeout",       required_argument, NULL, ARG_TIMEOUT       },
-                { "echo",          no_argument,       NULL, ARG_ECHO          },
+                { "echo",          optional_argument, NULL, 'e'               },
                 { "emoji",         required_argument, NULL, ARG_EMOJI         },
                 { "no-tty",        no_argument,       NULL, ARG_NO_TTY        },
                 { "accept-cached", no_argument,       NULL, ARG_ACCEPT_CACHED },
@@ -96,12 +97,14 @@ static int parse_argv(int argc, char *argv[]) {
         };
 
         const char *emoji = NULL;
-        int c;
+        int c, r;
 
         assert(argc >= 0);
         assert(argv);
 
-        while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0)
+        /* Note the asymmetry: the long option --echo= allows an optional argument, the short option does
+         * not. */
+        while ((c = getopt_long(argc, argv, "+he", options, NULL)) >= 0)
 
                 switch (c) {
 
@@ -116,14 +119,30 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_TIMEOUT:
-                        if (parse_sec(optarg, &arg_timeout) < 0)
+                        r = parse_sec(optarg, &arg_timeout);
-                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                        if (r < 0)
-                                                       "Failed to parse --timeout parameter %s",
+                                return log_error_errno(r, "Failed to parse --timeout= parameter: %s", optarg);
-                                                       optarg);
+
                         break;
 
-                case ARG_ECHO:
+                case 'e':
-                        arg_flags |= ASK_PASSWORD_ECHO;
+                        if (!optarg) {
+                                /* Short option -e is used, or no argument to long option --echo= */
+                                arg_flags |= ASK_PASSWORD_ECHO;
+                                arg_flags &= ~ASK_PASSWORD_SILENT;
+                        } else if (isempty(optarg) || streq(optarg, "masked"))
+                                /* Empty argument or explicit string "masked" for default behaviour. */
+                                arg_flags &= ~(ASK_PASSWORD_ECHO|ASK_PASSWORD_SILENT);
+                        else {
+                                bool b;
+
+                                r = parse_boolean_argument("--echo=", optarg, &b);
+                                if (r < 0)
+                                        return r;
+
+                                SET_FLAG(arg_flags, ASK_PASSWORD_ECHO, b);
+                                SET_FLAG(arg_flags, ASK_PASSWORD_SILENT, !b);
+                        }
                         break;
 
                 case ARG_EMOJI:
@@ -168,12 +187,12 @@ static int parse_argv(int argc, char *argv[]) {
         if (isempty(emoji) || streq(emoji, "auto"))
                 SET_FLAG(arg_flags, ASK_PASSWORD_HIDE_EMOJI, FLAGS_SET(arg_flags, ASK_PASSWORD_ECHO));
         else {
-                int r;
                 bool b;
 
                 r = parse_boolean_argument("--emoji=", emoji, &b);
                 if (r < 0)
                          return r;
+
                 SET_FLAG(arg_flags, ASK_PASSWORD_HIDE_EMOJI, !b);
         }
 
@@ -181,6 +200,14 @@ static int parse_argv(int argc, char *argv[]) {
                 arg_message = strv_join(argv + optind, " ");
                 if (!arg_message)
                         return log_oom();
+        } else if (FLAGS_SET(arg_flags, ASK_PASSWORD_ECHO)) {
+                /* By default ask_password_auto() will query with the string "Password: ", which is not right
+                 * when full echo is on, since then it's unlikely a password. Let's hence default to a less
+                 * confusing string in that case. */
+
+                arg_message = strdup("Input:");
+                if (!arg_message)
+                        return log_oom();
         }
 
         return 1;

src/test/test-capability.c

@@ -159,9 +159,15 @@ static void test_drop_privileges_fail(void) {
 }
 
 static void test_drop_privileges(void) {
+        fork_test(test_drop_privileges_fail);
+
+        if (have_effective_cap(CAP_NET_RAW) == 0) /* The remaining two tests only work if we have CAP_NET_RAW
+                                                   * in the first place. If we are run in some restricted
+                                                   * container environment we might not. */
+                return;
+
         fork_test(test_drop_privileges_keep_net_raw);
         fork_test(test_drop_privileges_dontkeep_net_raw);
-        fork_test(test_drop_privileges_fail);
 }
 
 static void test_have_effective_cap(void) {

src/test/test-exec-util.c

@@ -22,7 +22,7 @@
 #include "tests.h"
 
 static int here = 0, here2 = 0, here3 = 0;
-void *ignore_stdout_args[] = {&here, &here2, &here3};
+static void *ignore_stdout_args[] = { &here, &here2, &here3 };
 
 /* noop handlers, just check that arguments are passed correctly */
 static int ignore_stdout_func(int fd, void *arg) {

src/test/test-execute.c

@@ -924,8 +924,8 @@ int main(int argc, char *argv[]) {
         can_unshare = have_namespaces();
 
         /* It is needed otherwise cgroup creation fails */
-        if (getuid() != 0)
+        if (geteuid() != 0 || have_effective_cap(CAP_SYS_ADMIN) <= 0)
-                return log_tests_skipped("not root");
+                return log_tests_skipped("not privileged");
 
         r = enter_cgroup_subroot(NULL);
         if (r == -ENOMEDIUM)

src/test/test-mount-util.c

@@ -4,6 +4,7 @@
 #include <sys/statvfs.h>
 
 #include "alloc-util.h"
+#include "capability-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "mount-util.h"
@@ -75,8 +76,8 @@ static void test_bind_remount_recursive(void) {
         _cleanup_free_ char *subdir = NULL;
         const char *p;
 
-        if (geteuid() != 0) {
+        if (geteuid() != 0 || have_effective_cap(CAP_SYS_ADMIN) <= 0) {
-                (void) log_tests_skipped("not running as root");
+                (void) log_tests_skipped("not running privileged");
                 return;
         }
 
@@ -128,8 +129,8 @@ static void test_bind_remount_recursive(void) {
 static void test_bind_remount_one(void) {
         pid_t pid;
 
-        if (geteuid() != 0) {
+        if (geteuid() != 0 || have_effective_cap(CAP_SYS_ADMIN) <= 0) {
-                (void) log_tests_skipped("not running as root");
+                (void) log_tests_skipped("not running privileged");
                 return;
         }
 

src/test/test-seccomp.c

@@ -15,6 +15,7 @@
 #endif
 
 #include "alloc-util.h"
+#include "capability-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "macro.h"
@@ -41,6 +42,10 @@
 #  define SECCOMP_RESTRICT_ADDRESS_FAMILIES_BROKEN 0
 #endif
 
+static bool have_seccomp_privs(void) {
+        return geteuid() == 0 && have_effective_cap(CAP_SYS_ADMIN) > 0; /* If we are root but CAP_SYS_ADMIN we can't do caps (unless we also do NNP) */
+}
+
 static void test_parse_syscall_and_errno(void) {
         _cleanup_free_ char *n = NULL;
         int e;
@@ -168,8 +173,8 @@ static void test_filter_sets(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -303,8 +308,8 @@ static void test_restrict_namespace(void) {
                 log_notice("Seccomp not available, skipping remaining tests in %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping remaining tests in %s", __func__);
+                log_notice("Not privileged, skipping remaining tests in %s", __func__);
                 return;
         }
 
@@ -373,8 +378,8 @@ static void test_protect_sysctl(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -426,8 +431,8 @@ static void test_protect_syslog(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -468,8 +473,8 @@ static void test_restrict_address_families(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -557,8 +562,8 @@ static void test_restrict_realtime(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -604,8 +609,8 @@ static void test_memory_deny_write_execute_mmap(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 #if HAVE_VALGRIND_VALGRIND_H
@@ -674,8 +679,8 @@ static void test_memory_deny_write_execute_shmat(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 #if HAVE_VALGRIND_VALGRIND_H
@@ -739,8 +744,8 @@ static void test_restrict_archs(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -779,8 +784,8 @@ static void test_load_syscall_filter_set_raw(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -877,8 +882,8 @@ static void test_lock_personality(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }
 
@@ -941,8 +946,8 @@ static void test_restrict_suid_sgid(void) {
                 log_notice("Seccomp not available, skipping %s", __func__);
                 return;
         }
-        if (geteuid() != 0) {
+        if (!have_seccomp_privs()) {
-                log_notice("Not root, skipping %s", __func__);
+                log_notice("Not privileged, skipping %s", __func__);
                 return;
         }