From: djm <djm@openbsd.org>
Date: Thu, 21 May 2015 06:38:35 +0000 (+0000)
Subject: support arguments to AuthorizedKeysCommand
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=45583e9a9e2e4bf7e003ef361f4142de236ba75b;p=openbsd

support arguments to AuthorizedKeysCommand

bz#2081 loosely based on patch by Sami Hartikainen
feedback and ok markus@
---

diff --git a/usr.bin/ssh/auth2-pubkey.c b/usr.bin/ssh/auth2-pubkey.c
index e7c9658f4c9..9b5ad5540d3 100644
--- a/usr.bin/ssh/auth2-pubkey.c
+++ b/usr.bin/ssh/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.
  *
@@ -62,6 +62,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;
@@ -245,6 +248,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)
 {
@@ -523,144 +808,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/usr.bin/ssh/sshd_config.5 b/usr.bin/ssh/sshd_config.5
index b748e17e782..e2e09aa599e 100644
--- a/usr.bin/ssh/sshd_config.5
+++ b/usr.bin/ssh/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