userdbctl: actually implement option parsing stop after --chain

The basic idea is that --chain should stop option parsing. But
previously this didn't work, so --chain could be specified anywhere
in the command line. To maintain with compatibility with that,
allow --chain to be specified anywhere until the first positional
arg or option in the command string. This allows options to be passed
in the expected fashion:
  userdbctl --chain ssh-authorized-keys user cmd --opt1 --opt2
  userdbctl --chain ssh-authorized-keys user -- cmd --opt1 --opt2
but also allows the invocations which worked previously:
  userdbctl ssh-authorized-keys user --chain cmd
  userdbctl ssh-authorized-keys user cmd --chain

Fixes 8072a7e6a9eaf2de120797dd16c5e0baea606219.

The error messages are extended a bit. "binary path" is misleading:
we support all kinds of executables, not only compiled programs.

diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c
index 60a6ff051a3b832b627368a6a9ab58e269e4b836..35a17c50217da55e34b2b1d4fcf944e6ab221c35 100644 (file)
--- a/src/userdb/userdbctl.c
+++ b/src/userdb/userdbctl.c
@@ -1148,11 +1148,13 @@ static int verb_ssh_authorized_keys(int argc, char *argv[], uintptr_t _data, voi
                 /* Make similar restrictions on the chain command as OpenSSH itself makes on the primary command. */
                 if (!path_is_absolute(argv[2]))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                               "Chain invocation of ssh-authorized-keys commands requires an absolute binary path argument.");
+                                               "Chain invocation of ssh-authorized-keys commands requires an absolute program path (got '%s').",
+                                               argv[2]);
 
                 if (!path_is_normalized(argv[2]))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                               "Chain invocation of ssh-authorized-keys commands requires an normalized binary path argument.");
+                                               "Chain invocation of ssh-authorized-keys commands requires a normalized program path (got '%s').",
+                                               argv[2]);
 
                 chain_invocation = argv + 2;
         } else {
@@ -1628,9 +1630,10 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) {
                 arg_services = l;
         }
 
-        OptionParser opts = { argc, argv };
+        OptionParser opts = { argc, argv, OPTION_PARSER_RETURN_POSITIONAL_ARGS };
+        _cleanup_strv_free_ char **args = NULL;
 
-        FOREACH_OPTION_OR_RETURN(c, &opts)
+        FOREACH_OPTION_OR_RETURN(c, &opts) {
                 switch (c) {
 
                 OPTION_COMMON_HELP:
@@ -1733,6 +1736,12 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) {
                         arg_chain = true;
                         break;
 
+                OPTION_POSITIONAL:
+                        r = strv_extend(&args, opts.arg);
+                        if (r < 0)
+                                return log_oom();
+                        break;
+
                 OPTION_LONG("uid-min", "ID", "Filter by minimum UID/GID (default 0)"):
                         r = parse_uid(opts.arg, &arg_uid_min);
                         if (r < 0)
@@ -1811,6 +1820,15 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) {
                 }
                 }
 
+                /* When --chain was seen, stop parsing switches after the second positional argument:
+                 * [OPTS0…, VERB, OPTS1…, USERNAME, OPTS2…, COMMAND, OPTS3…]
+                 * We shall parse OPTS0, OPTS1, OPTS2, but OPTS3 are for COMMAND.
+                 * --chain can be anywhere in OPTS0, OPTS1, OPTS2, or first in OPTS3.
+                 */
+                if (arg_chain && strv_length(args) >= 3)
+                        opts.state = OPTION_PARSER_DONE;
+        }
+
         if (arg_uid_min > arg_uid_max)
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                        "Minimum UID/GID " UID_FMT " is above maximum UID/GID " UID_FMT ", refusing.",
@@ -1823,12 +1841,16 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) {
         if (arg_from_file)
                 arg_boundaries = false;
 
-        *remaining_args = option_parser_get_args(&opts);
+        /* We gathered some positional args in 'args' ourselves. Append the remaining ones. */
+        if (strv_extend_strv(&args, option_parser_get_args(&opts), /* filter_duplicates= */ false) < 0)
+                return log_oom();
+
+        *remaining_args = TAKE_PTR(args);
         return 1;
 }
 
 static int run(int argc, char *argv[]) {
-        char **args = NULL;
+        _cleanup_strv_free_ char **args = NULL;
         int r;
 
         log_setup();

diff --git a/test/units/TEST-46-HOMED.sh b/test/units/TEST-46-HOMED.sh
index 4b81799ef3deada02a966d26a4d59b06a653cee4..5afa42d73968e602d07145311501fa4e627a2f38 100755 (executable)
--- a/test/units/TEST-46-HOMED.sh
+++ b/test/units/TEST-46-HOMED.sh
@@ -586,6 +586,15 @@ EOF
     (! userdbctl ssh-authorized-keys dropin-user --chain '')
     (! SYSTEMD_LOG_LEVEL=debug userdbctl ssh-authorized-keys dropin-user --chain /usr/bin/false)
 
+    # Check that invocations with --chain work as expected
+    userdbctl ssh-authorized-keys --chain dropin-user /bin/echo --asdf | grep -e --asdf
+    userdbctl ssh-authorized-keys dropin-user --chain /bin/echo --asdf | grep -e --asdf
+    userdbctl ssh-authorized-keys dropin-user /bin/echo --chain --asdf | grep -e --asdf
+    userdbctl ssh-authorized-keys --chain dropin-user -- /bin/echo --asdf | grep -e --asdf
+    userdbctl ssh-authorized-keys --chain -- dropin-user /bin/echo --asdf | grep -e --asdf
+    userdbctl --chain -- ssh-authorized-keys dropin-user /bin/echo --asdf | grep -e --asdf
+    (! userdbctl --chain -- ssh-authorized-keys dropin-user -- /bin/echo --asdf)
+
     (! userdbctl '')
     for opt in json multiplexer output synthesize with-dropin with-nss with-varlink; do
         (! userdbctl "--$opt=''")