2 files changed, 378 insertions, 108 deletions

diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index f96e843c2..553144c7b 100644
--- a/auth2-pubkey.c
+++ b/auth2-pubkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-pubkey.c,v 1.49 2015/05/04 06:10:48 djm Exp $ */
+/* $OpenBSD: auth2-pubkey.c,v 1.50 2015/05/21 06:38:35 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -65,6 +65,9 @@
 #include "monitor_wrap.h"
 #include "authfile.h"
 #include "match.h"
+#include "ssherr.h"
+#include "channels.h" /* XXX for session.h */
+#include "session.h" /* XXX for child_set_env(); refactor? */
 
 /* import */
 extern ServerOptions options;
@@ -248,6 +251,288 @@ pubkey_auth_info(Authctxt *authctxt, const Key *key, const char *fmt, ...)
         free(extra);
 }
 
+/*
+ * Splits 's' into an argument vector. Handles quoted string and basic
+ * escape characters (\\, \", \'). Caller must free the argument vector
+ * and its members.
+ */
+static int
+split_argv(const char *s, int *argcp, char ***argvp)
+{
+        int r = SSH_ERR_INTERNAL_ERROR;
+        int argc = 0, quote, i, j;
+        char *arg, **argv = xcalloc(1, sizeof(*argv));
+
+        *argvp = NULL;
+        *argcp = 0;
+
+        for (i = 0; s[i] != '\0'; i++) {
+                /* Skip leading whitespace */
+                if (s[i] == ' ' || s[i] == '\t')
+                        continue;
+
+                /* Start of a token */
+                quote = 0;
+                if (s[i] == '\\' &&
+                    (s[i + 1] == '\'' || s[i + 1] == '\"' || s[i + 1] == '\\'))
+                        i++;
+                else if (s[i] == '\'' || s[i] == '"')
+                        quote = s[i++];
+
+                argv = xreallocarray(argv, (argc + 2), sizeof(*argv));
+                arg = argv[argc++] = xcalloc(1, strlen(s + i) + 1);
+                argv[argc] = NULL;
+
+                /* Copy the token in, removing escapes */
+                for (j = 0; s[i] != '\0'; i++) {
+                        if (s[i] == '\\') {
+                                if (s[i + 1] == '\'' ||
+                                    s[i + 1] == '\"' ||
+                                    s[i + 1] == '\\') {
+                                        i++; /* Skip '\' */
+                                        arg[j++] = s[i];
+                                } else {
+                                        /* Unrecognised escape */
+                                        arg[j++] = s[i];
+                                }
+                        } else if (quote == 0 && (s[i] == ' ' || s[i] == '\t'))
+                                break; /* done */
+                        else if (quote != 0 && s[i] == quote)
+                                break; /* done */
+                        else
+                                arg[j++] = s[i];
+                }
+                if (s[i] == '\0') {
+                        if (quote != 0) {
+                                /* Ran out of string looking for close quote */
+                                r = SSH_ERR_INVALID_FORMAT;
+                                goto out;
+                        }
+                        break;
+                }
+        }
+        /* Success */
+        *argcp = argc;
+        *argvp = argv;
+        argc = 0;
+        argv = NULL;
+        r = 0;
+ out:
+        if (argc != 0 && argv != NULL) {
+                for (i = 0; i < argc; i++)
+                        free(argv[i]);
+                free(argv);
+        }
+        return r;
+}
+
+/*
+ * Reassemble an argument vector into a string, quoting and escaping as
+ * necessary. Caller must free returned string.
+ */
+static char *
+assemble_argv(int argc, char **argv)
+{
+        int i, j, ws, r;
+        char c, *ret;
+        struct sshbuf *buf, *arg;
+
+        if ((buf = sshbuf_new()) == NULL || (arg = sshbuf_new()) == NULL)
+                fatal("%s: sshbuf_new failed", __func__);
+
+        for (i = 0; i < argc; i++) {
+                ws = 0;
+                sshbuf_reset(arg);
+                for (j = 0; argv[i][j] != '\0'; j++) {
+                        r = 0;
+                        c = argv[i][j];
+                        switch (c) {
+                        case ' ':
+                        case '\t':
+                                ws = 1;
+                                r = sshbuf_put_u8(arg, c);
+                                break;
+                        case '\\':
+                        case '\'':
+                        case '"':
+                                if ((r = sshbuf_put_u8(arg, '\\')) != 0)
+                                        break;
+                                /* FALLTHROUGH */
+                        default:
+                                r = sshbuf_put_u8(arg, c);
+                                break;
+                        }
+                        if (r != 0)
+                                fatal("%s: sshbuf_put_u8: %s",
+                                    __func__, ssh_err(r));
+                }
+                if ((i != 0 && (r = sshbuf_put_u8(buf, ' ')) != 0) ||
+                    (ws != 0 && (r = sshbuf_put_u8(buf, '"')) != 0) ||
+                    (r = sshbuf_putb(buf, arg)) != 0 ||
+                    (ws != 0 && (r = sshbuf_put_u8(buf, '"')) != 0))
+                        fatal("%s: buffer error: %s", __func__, ssh_err(r));
+        }
+        if ((ret = malloc(sshbuf_len(buf) + 1)) == NULL)
+                fatal("%s: malloc failed", __func__);
+        memcpy(ret, sshbuf_ptr(buf), sshbuf_len(buf));
+        ret[sshbuf_len(buf)] = '\0';
+        sshbuf_free(buf);
+        sshbuf_free(arg);
+        return ret;
+}
+
+/*
+ * Runs command in a subprocess. Returns pid on success and a FILE* to the
+ * subprocess' stdout or 0 on failure.
+ * NB. "command" is only used for logging.
+ */
+static pid_t
+subprocess(const char *tag, struct passwd *pw, const char *command,
+    int ac, char **av, FILE **child)
+{
+        FILE *f;
+        struct stat st;
+        int devnull, p[2], i;
+        pid_t pid;
+        char *cp, errmsg[512];
+        u_int envsize;
+        char **child_env;
+
+        *child = NULL;
+
+        debug3("%s: %s command \"%s\" running as %s", __func__,
+            tag, command, pw->pw_name);
+
+        /* Verify the path exists and is safe-ish to execute */
+        if (*av[0] != '/') {
+                error("%s path is not absolute", tag);
+                return 0;
+        }
+        temporarily_use_uid(pw);
+        if (stat(av[0], &st) < 0) {
+                error("Could not stat %s \"%s\": %s", tag,
+                    av[0], strerror(errno));
+                restore_uid();
+                return 0;
+        }
+        if (auth_secure_path(av[0], &st, NULL, 0,
+            errmsg, sizeof(errmsg)) != 0) {
+                error("Unsafe %s \"%s\": %s", tag, av[0], errmsg);
+                restore_uid();
+                return 0;
+        }
+
+        /*
+         * Run the command; stderr is left in place, stdout is the
+         * authorized_keys output.
+         */
+        if (pipe(p) != 0) {
+                error("%s: pipe: %s", tag, strerror(errno));
+                restore_uid();
+                return 0;
+        }
+
+        /*
+         * Don't want to call this in the child, where it can fatal() and
+         * run cleanup_exit() code.
+         */
+        restore_uid();
+
+        switch ((pid = fork())) {
+        case -1: /* error */
+                error("%s: fork: %s", tag, strerror(errno));
+                close(p[0]);
+                close(p[1]);
+                return 0;
+        case 0: /* child */
+                /* Prepare a minimal environment for the child. */
+                envsize = 5;
+                child_env = xcalloc(sizeof(*child_env), envsize);
+                child_set_env(&child_env, &envsize, "PATH", _PATH_STDPATH);
+                child_set_env(&child_env, &envsize, "USER", pw->pw_name);
+                child_set_env(&child_env, &envsize, "LOGNAME", pw->pw_name);
+                child_set_env(&child_env, &envsize, "HOME", pw->pw_dir);
+                if ((cp = getenv("LANG")) != NULL)
+                        child_set_env(&child_env, &envsize, "LANG", cp);
+
+                for (i = 0; i < NSIG; i++)
+                        signal(i, SIG_DFL);
+
+                if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
+                        error("%s: open %s: %s", tag, _PATH_DEVNULL,
+                            strerror(errno));
+                        _exit(1);
+                }
+                /* Keep stderr around a while longer to catch errors */
+                if (dup2(devnull, STDIN_FILENO) == -1 ||
+                    dup2(p[1], STDOUT_FILENO) == -1) {
+                        error("%s: dup2: %s", tag, strerror(errno));
+                        _exit(1);
+                }
+                closefrom(STDERR_FILENO + 1);
+
+                /* Don't use permanently_set_uid() here to avoid fatal() */
+                if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) {
+                        error("%s: setresgid %u: %s", tag, (u_int)pw->pw_gid,
+                            strerror(errno));
+                        _exit(1);
+                }
+                if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0) {
+                        error("%s: setresuid %u: %s", tag, (u_int)pw->pw_uid,
+                            strerror(errno));
+                        _exit(1);
+                }
+                /* stdin is pointed to /dev/null at this point */
+                if (dup2(STDIN_FILENO, STDERR_FILENO) == -1) {
+                        error("%s: dup2: %s", tag, strerror(errno));
+                        _exit(1);
+                }
+
+                execve(av[0], av, child_env);
+                error("%s exec \"%s\": %s", tag, command, strerror(errno));
+                _exit(127);
+        default: /* parent */
+                break;
+        }
+
+        close(p[1]);
+        if ((f = fdopen(p[0], "r")) == NULL) {
+                error("%s: fdopen: %s", tag, strerror(errno));
+                close(p[0]);
+                /* Don't leave zombie child */
+                kill(pid, SIGTERM);
+                while (waitpid(pid, NULL, 0) == -1 && errno == EINTR)
+                        ;
+                return 0;
+        }
+        /* Success */
+        debug3("%s: %s pid %ld", __func__, tag, (long)pid);
+        *child = f;
+        return pid;
+}
+
+/* Returns 0 if pid exited cleanly, non-zero otherwise */
+static int
+exited_cleanly(pid_t pid, const char *tag, const char *cmd)
+{
+        int status;
+
+        while (waitpid(pid, &status, 0) == -1) {
+                if (errno != EINTR) {
+                        error("%s: waitpid: %s", tag, strerror(errno));
+                        return -1;
+                }
+        }
+        if (WIFSIGNALED(status)) {
+                error("%s %s exited on signal %d", tag, cmd, WTERMSIG(status));
+                return -1;
+        } else if (WEXITSTATUS(status) != 0) {
+                error("%s %s failed, status %d", tag, cmd, WEXITSTATUS(status));
+                return -1;
+        }
+        return 0;
+}
+
 static int
 match_principals_option(const char *principal_list, struct sshkey_cert *cert)
 {
@@ -526,144 +811,117 @@ user_key_allowed2(struct passwd *pw, Key *key, char *file)
 static int
 user_key_command_allowed2(struct passwd *user_pw, Key *key)
 {
-        FILE *f;
-        int ok, found_key = 0;
+        FILE *f = NULL;
+        int r, ok, found_key = 0;
         struct passwd *pw;
-        struct stat st;
-        int status, devnull, p[2], i;
+        int i, uid_swapped = 0, ac = 0;
         pid_t pid;
-        char *username, errmsg[512];
+        char *username = NULL, *key_fp = NULL, *keytext = NULL;
+        char *tmp, *command = NULL, **av = NULL;
+        void (*osigchld)(int);
 
-        if (options.authorized_keys_command == NULL ||
-            options.authorized_keys_command[0] != '/')
+        if (options.authorized_keys_command == NULL)
                 return 0;
-
         if (options.authorized_keys_command_user == NULL) {
                 error("No user for AuthorizedKeysCommand specified, skipping");
                 return 0;
         }
 
+        /*
+         * NB. all returns later this function should go via "out" to
+         * ensure the original SIGCHLD handler is restored properly.
+         */
+        osigchld = signal(SIGCHLD, SIG_DFL);
+
+        /* Prepare and verify the user for the command */
         username = percent_expand(options.authorized_keys_command_user,
             "u", user_pw->pw_name, (char *)NULL);
         pw = getpwnam(username);
         if (pw == NULL) {
                 error("AuthorizedKeysCommandUser \"%s\" not found: %s",
                     username, strerror(errno));
-                free(username);
-                return 0;
+                goto out;
         }
-        free(username);
-
-        temporarily_use_uid(pw);
 
-        if (stat(options.authorized_keys_command, &st) < 0) {
-                error("Could not stat AuthorizedKeysCommand \"%s\": %s",
-                    options.authorized_keys_command, strerror(errno));
+        /* Prepare AuthorizedKeysCommand */
+        if ((key_fp = sshkey_fingerprint(key, options.fingerprint_hash,
+            SSH_FP_DEFAULT)) == NULL) {
+                error("%s: sshkey_fingerprint failed", __func__);
                 goto out;
         }
-        if (auth_secure_path(options.authorized_keys_command, &st, NULL, 0,
-            errmsg, sizeof(errmsg)) != 0) {
-                error("Unsafe AuthorizedKeysCommand: %s", errmsg);
+        if ((r = sshkey_to_base64(key, &keytext)) != 0) {
+                error("%s: sshkey_to_base64 failed: %s", __func__, ssh_err(r));
                 goto out;
         }
 
-        if (pipe(p) != 0) {
-                error("%s: pipe: %s", __func__, strerror(errno));
+        /* Turn the command into an argument vector */
+        if (split_argv(options.authorized_keys_command, &ac, &av) != 0) {
+                error("AuthorizedKeysCommand \"%s\" contains invalid quotes",
+                    command);
                 goto out;
         }
-
-        debug3("Running AuthorizedKeysCommand: \"%s %s\" as \"%s\"",
-            options.authorized_keys_command, user_pw->pw_name, pw->pw_name);
+        if (ac == 0) {
+                error("AuthorizedKeysCommand \"%s\" yielded no arguments",
+                    command);
+                goto out;
+        }
+        for (i = 1; i < ac; i++) {
+                tmp = percent_expand(av[i],
+                    "u", user_pw->pw_name,
+                    "h", user_pw->pw_dir,
+                    "t", sshkey_ssh_name(key),
+                    "f", key_fp,
+                    "k", keytext,
+                    (char *)NULL);
+                if (tmp == NULL)
+                        fatal("%s: percent_expand failed", __func__);
+                free(av[i]);
+                av[i] = tmp;
+        }
+        /* Prepare a printable command for logs, etc. */
+        command = assemble_argv(ac, av);
 
         /*
-         * Don't want to call this in the child, where it can fatal() and
-         * run cleanup_exit() code.
+         * If AuthorizedKeysCommand was run without arguments
+         * then fall back to the old behaviour of passing the
+         * target username as a single argument.
          */
-        restore_uid();
-
-        switch ((pid = fork())) {
-        case -1: /* error */
-                error("%s: fork: %s", __func__, strerror(errno));
-                close(p[0]);
-                close(p[1]);
-                return 0;
-        case 0: /* child */
-                for (i = 0; i < NSIG; i++)
-                        signal(i, SIG_DFL);
-
-                if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
-                        error("%s: open %s: %s", __func__, _PATH_DEVNULL,
-                            strerror(errno));
-                        _exit(1);
-                }
-                /* Keep stderr around a while longer to catch errors */
-                if (dup2(devnull, STDIN_FILENO) == -1 ||
-                    dup2(p[1], STDOUT_FILENO) == -1) {
-                        error("%s: dup2: %s", __func__, strerror(errno));
-                        _exit(1);
-                }
-                closefrom(STDERR_FILENO + 1);
-
-                /* Don't use permanently_set_uid() here to avoid fatal() */
-                if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) {
-                        error("setresgid %u: %s", (u_int)pw->pw_gid,
-                            strerror(errno));
-                        _exit(1);
-                }
-                if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0) {
-                        error("setresuid %u: %s", (u_int)pw->pw_uid,
-                            strerror(errno));
-                        _exit(1);
-                }
-                /* stdin is pointed to /dev/null at this point */
-                if (dup2(STDIN_FILENO, STDERR_FILENO) == -1) {
-                        error("%s: dup2: %s", __func__, strerror(errno));
-                        _exit(1);
-                }
-
-                execl(options.authorized_keys_command,
-                    options.authorized_keys_command, user_pw->pw_name, NULL);
-
-                error("AuthorizedKeysCommand %s exec failed: %s",
-                    options.authorized_keys_command, strerror(errno));
-                _exit(127);
-        default: /* parent */
-                break;
+        if (ac == 1) {
+                av = xreallocarray(av, ac + 2, sizeof(*av));
+                av[1] = xstrdup(user_pw->pw_name);
+                av[2] = NULL;
+                /* Fix up command too, since it is used in log messages */
+                free(command);
+                xasprintf(&command, "%s %s", av[0], av[1]);
         }
 
+        if ((pid = subprocess("AuthorizedKeysCommand", pw, command,
+            ac, av, &f)) == 0)
+                goto out;
+
+        uid_swapped = 1;
         temporarily_use_uid(pw);
 
-        close(p[1]);
-        if ((f = fdopen(p[0], "r")) == NULL) {
-                error("%s: fdopen: %s", __func__, strerror(errno));
-                close(p[0]);
-                /* Don't leave zombie child */
-                kill(pid, SIGTERM);
-                while (waitpid(pid, NULL, 0) == -1 && errno == EINTR)
-                        ;
-                goto out;
-        }
         ok = check_authkeys_file(f, options.authorized_keys_command, key, pw);
-        fclose(f);
 
-        while (waitpid(pid, &status, 0) == -1) {
-                if (errno != EINTR) {
-                        error("%s: waitpid: %s", __func__, strerror(errno));
-                        goto out;
-                }
-        }
-        if (WIFSIGNALED(status)) {
-                error("AuthorizedKeysCommand %s exited on signal %d",
-                    options.authorized_keys_command, WTERMSIG(status));
-                goto out;
-        } else if (WEXITSTATUS(status) != 0) {
-                error("AuthorizedKeysCommand %s returned status %d",
-                    options.authorized_keys_command, WEXITSTATUS(status));
+        if (exited_cleanly(pid, "AuthorizedKeysCommand", command) != 0)
                 goto out;
-        }
+
+        /* Read completed successfully */
         found_key = ok;
  out:
-        restore_uid();
+        if (f != NULL)
+                fclose(f);
+        signal(SIGCHLD, osigchld);
+        for (i = 0; i < ac; i++)
+                free(av[i]);
+        free(av);
+        if (uid_swapped)
+                restore_uid();
+        free(command);
+        free(username);
+        free(key_fp);
+        free(keytext);
         return found_key;
 }
 
diff --git a/sshd_config.5 b/sshd_config.5
index 562dad356..e40ecedef 100644
--- a/sshd_config.5
+++ b/sshd_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: sshd_config.5,v 1.200 2015/04/29 03:48:56 dtucker Exp $
-.Dd $Mdocdate: April 29 2015 $
+.\" $OpenBSD: sshd_config.5,v 1.201 2015/05/21 06:38:35 djm Exp $
+.Dd $Mdocdate: May 21 2015 $
 .Dt SSHD_CONFIG 5
 .Os
 .Sh NAME
@@ -234,9 +234,21 @@ The default is not to require multiple authentication; successful completion
 of a single authentication method is sufficient.
 .It Cm AuthorizedKeysCommand
 Specifies a program to be used to look up the user's public keys.
-The program must be owned by root and not writable by group or others.
-It will be invoked with a single argument of the username
-being authenticated, and should produce on standard output zero or
+The program must be owned by root, not writable by group or others and
+specified by an absolute path.
+.Pp
+Arguments to
+.Cm AuthorizedKeysCommand
+may be provided using the following tokens, which will be expanded
+at runtime: %% is replaced by a literal '%', %u is replaced by the
+username being authenticated, %h is replaced by the home directory
+of the user being authenticated, %t is replaced with the key type
+offered for authentication, %f is replaced with the fingerprint of
+the key, and %k is replaced with the key being offered for authentication.
+If no arguments are specified then the username of the target user
+will be supplied.
+.Pp
+The program should produce on standard output zero or
 more lines of authorized_keys output (see AUTHORIZED_KEYS in
 .Xr sshd 8 ) .
 If a key supplied by AuthorizedKeysCommand does not successfully authenticate