Attachment 2191 Details for Bug 1663 – Revised patch, against -current as of 20121005

[patch] Revised patch, against -current as of 20121005

authorized_keys_command.diff (text/plain), 15.36 KB, created by Damien Miller on 2012-10-05 15:56:19 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2012-10-05 15:56:19 AEST

Size: 15.36 KB

patch

obsolete

>diff --git auth.c auth.c
>index 01c6a5d..4177c5a 100644
>--- auth.c
>+++ auth.c
>@@ -319,41 +319,42 @@ check_key_in_hostfiles(struct passwd *pw, Key *key, const char *host,
> 	return host_status;
> }
> 
>-
> /*
>- * Check a given file for security. This is defined as all components
>+ * Check a given path for security. This is defined as all components
>  * of the path to the file must be owned by either the owner of
>  * of the file or root and no directories must be group or world writable.
>  *
>  * XXX Should any specific check be done for sym links ?
>  *
>- * Takes an open file descriptor, the file name, a uid and and
>+ * Takes an the file name, its stat information (preferably from fstat() to
>+ * avoid races), the uid of the expected owner, their home directory and an
>  * error buffer plus max size as arguments.
>  *
>  * Returns 0 on success and -1 on failure
>  */
>-static int
>-secure_filename(FILE *f, const char *file, struct passwd *pw,
>-    char *err, size_t errlen)
>+int
>+auth_secure_path(const char *name, struct stat *stp, const char *pw_dir,
>+    uid_t uid, char *err, size_t errlen)
> {
>-	uid_t uid = pw->pw_uid;
> 	char buf[MAXPATHLEN], homedir[MAXPATHLEN];
> 	char *cp;
> 	int comparehome = 0;
> 	struct stat st;
> 
>-	if (realpath(file, buf) == NULL) {
>-		snprintf(err, errlen, "realpath %s failed: %s", file,
>+	if (realpath(name, buf) == NULL) {
>+		snprintf(err, errlen, "realpath %s failed: %s", name,
> 		    strerror(errno));
> 		return -1;
> 	}
>-	if (realpath(pw->pw_dir, homedir) != NULL)
>+	if (pw_dir != NULL && realpath(pw_dir, homedir) != NULL)
> 		comparehome = 1;
> 
>-	/* check the open file to avoid races */
>-	if (fstat(fileno(f), &st) < 0 ||
>-	    (st.st_uid != 0 && st.st_uid != uid) ||
>-	    (st.st_mode & 022) != 0) {
>+	if (!S_ISREG(stp->st_mode)) {
>+		snprintf(err, errlen, "%s is not a regular file", buf);
>+		return -1;
>+	}
>+	if ((stp->st_uid != 0 && stp->st_uid != uid) ||
>+	    (stp->st_mode & 022) != 0) {
> 		snprintf(err, errlen, "bad ownership or modes for file %s",
> 		    buf);
> 		return -1;
>@@ -389,6 +390,31 @@ secure_filename(FILE *f, const char *file, struct passwd *pw,
> 	return 0;
> }
> 
>+/*
>+ * Version of secure_path() that accepts an open file descriptor to
>+ * avoid races.
>+ *
>+ * Returns 0 on success and -1 on failure
>+ */
>+static int
>+secure_filename(FILE *f, const char *file, struct passwd *pw,
>+    char *err, size_t errlen)
>+{
>+	uid_t uid = pw->pw_uid;
>+	char buf[MAXPATHLEN], homedir[MAXPATHLEN];
>+	char *cp;
>+	int comparehome = 0;
>+	struct stat st;
>+
>+	/* check the open file to avoid races */
>+	if (fstat(fileno(f), &st) < 0) {
>+		snprintf(err, errlen, "cannot stat file %s: %s",
>+		    buf, strerror(errno));
>+		return -1;
>+	}
>+	return auth_secure_path(file, &st, pw->pw_dir, pw->pw_uid, err, errlen);
>+}
>+
> static FILE *
> auth_openfile(const char *file, struct passwd *pw, int strict_modes,
>     int log_missing, char *file_type)
>diff --git auth.h auth.h
>index 5d63c57..ef014c4 100644
>--- auth.h
>+++ auth.h
>@@ -111,6 +111,10 @@ int	 auth_rhosts_rsa_key_allowed(struct passwd *, char *, char *, Key *);
> int	 hostbased_key_allowed(struct passwd *, const char *, char *, Key *);
> int	 user_key_allowed(struct passwd *, Key *);
> 
>+struct stat;
>+int	 auth_secure_path(const char *, struct stat *, const char *, uid_t,
>+    char *, size_t);
>+
> #ifdef KRB5
> int	auth_krb5(Authctxt *authctxt, krb5_data *auth, char **client, krb5_data *);
> int	auth_krb5_tgt(Authctxt *authctxt, krb5_data *tgt);
>diff --git auth2-pubkey.c auth2-pubkey.c
>index b871bb5..fbd1a9a 100644
>--- auth2-pubkey.c
>+++ auth2-pubkey.c
>@@ -26,9 +26,13 @@
> 
> #include <sys/types.h>
> #include <sys/stat.h>
>+#include <sys/wait.h>
> 
>+#include <errno.h>
> #include <fcntl.h>
>+#include <paths.h>
> #include <pwd.h>
>+#include <signal.h>
> #include <stdio.h>
> #include <stdarg.h>
> #include <string.h>
>@@ -239,7 +243,7 @@ match_principals_file(char *file, struct passwd *pw, struct KeyCert *cert)
> 			if (strcmp(cp, cert->principals[i]) == 0) {
> 				debug3("matched principal \"%.100s\" "
> 				    "from file \"%s\" on line %lu",
>-			    	    cert->principals[i], file, linenum);
>+				    cert->principals[i], file, linenum);
> 				if (auth_parse_options(pw, line_opts,
> 				    file, linenum) != 1)
> 					continue;
>@@ -252,31 +256,22 @@ match_principals_file(char *file, struct passwd *pw, struct KeyCert *cert)
> 	fclose(f);
> 	restore_uid();
> 	return 0;
>-}	
>+}
> 
>-/* return 1 if user allows given key */
>+/*
>+ * Checks whether key is allowed in authorized_keys-format file,
>+ * returns 1 if the key is allowed or 0 otherwise.
>+ */
> static int
>-user_key_allowed2(struct passwd *pw, Key *key, char *file)
>+check_authkeys_file(FILE *f, char *file, Key* key, struct passwd *pw)
> {
> 	char line[SSH_MAX_PUBKEY_BYTES];
> 	const char *reason;
> 	int found_key = 0;
>-	FILE *f;
> 	u_long linenum = 0;
> 	Key *found;
> 	char *fp;
> 
>-	/* Temporarily use the user's uid. */
>-	temporarily_use_uid(pw);
>-
>-	debug("trying public key file %s", file);
>-	f = auth_openkeyfile(file, pw, options.strict_modes);
>-
>-	if (!f) {
>-		restore_uid();
>-		return 0;
>-	}
>-
> 	found_key = 0;
> 	found = key_new(key_is_cert(key) ? KEY_UNSPEC : key->type);
> 
>@@ -369,8 +364,6 @@ user_key_allowed2(struct passwd *pw, Key *key, char *file)
> 			break;
> 		}
> 	}
>-	restore_uid();
>-	fclose(f);
> 	key_free(found);
> 	if (!found_key)
> 		debug2("key not found");
>@@ -432,7 +425,172 @@ user_cert_trusted_ca(struct passwd *pw, Key *key)
> 	return ret;
> }
> 
>-/* check whether given key is in .ssh/authorized_keys* */
>+/*
>+ * Checks whether key is allowed in file.
>+ * returns 1 if the key is allowed or 0 otherwise.
>+ */
>+static int
>+user_key_allowed2(struct passwd *pw, Key *key, char *file)
>+{
>+	FILE *f;
>+	int found_key = 0;
>+
>+	/* Temporarily use the user's uid. */
>+	temporarily_use_uid(pw);
>+
>+	debug("trying public key file %s", file);
>+	if ((f = auth_openkeyfile(file, pw, options.strict_modes)) != NULL) {
>+		found_key = check_authkeys_file(f, file, key, pw);
>+		fclose(f);
>+	}
>+
>+	restore_uid();
>+	return found_key;
>+}
>+
>+/*
>+ * Checks whether key is allowed in output of command.
>+ * returns 1 if the key is allowed or 0 otherwise.
>+ */
>+static int
>+user_key_command_allowed2(struct passwd *user_pw, Key *key)
>+{
>+	FILE *f;
>+	int ok, found_key = 0;
>+	struct passwd *pw;
>+	struct stat st;
>+	int status, devnull, p[2], i;
>+	pid_t pid;
>+	char errmsg[512];
>+
>+	if (options.authorized_keys_command == NULL ||
>+	    options.authorized_keys_command[0] != '/')
>+		return 0;
>+
>+	/* If no user specified to run commands the default to target user */
>+	if (options.authorized_keys_command_user == NULL)
>+		pw = user_pw;
>+	else {
>+		pw = getpwnam(options.authorized_keys_command_user);
>+		if (pw == NULL) {
>+			error("AuthorizedKeyCommandUser \"%s\" not found: %s",
>+			    options.authorized_keys_command, strerror(errno));
>+			return 0;
>+		}
>+	}
>+
>+	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));
>+		goto out;
>+	}
>+	if (auth_secure_path(options.authorized_keys_command, &st, NULL, 0,
>+	    errmsg, sizeof(errmsg)) != 0) {
>+		error("Unsafe AuthorizedKeysCommand: %s", errmsg);
>+		goto out;
>+	}
>+
>+	/* open the pipe and read the keys */
>+	if (pipe(p) != 0) {
>+		error("%s: pipe: %s", __func__, strerror(errno));
>+		goto out;
>+	}
>+
>+	debug3("Running AuthorizedKeysCommand: \"%s\" as \"%s\"",
>+	    options.authorized_keys_command, pw->pw_name);
>+
>+	/*
>+	 * 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", __func__, strerror(errno));
>+		close(p[0]);
>+		close(p[1]);
>+		return 0;
>+	case 0: /* child */
>+		for (i = 0; i < NSIG; i++)
>+			signal(i, SIG_DFL);
>+
>+		/* 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);
>+		}
>+
>+		close(p[0]);
>+		if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
>+			error("%s: open %s: %s", __func__, _PATH_DEVNULL,
>+			    strerror(errno));
>+			_exit(1);
>+		}
>+		if (dup2(devnull, STDIN_FILENO) == -1 ||
>+		    dup2(p[1], STDOUT_FILENO) == -1 ||
>+		    dup2(devnull, STDERR_FILENO) == -1) {
>+			error("%s: dup2: %s", __func__, strerror(errno));
>+			_exit(1);
>+		}
>+		closefrom(STDERR_FILENO + 1);
>+
>+		execl(options.authorized_keys_command,
>+		    options.authorized_keys_command, pw->pw_name, NULL);
>+
>+		error("AuthorizedKeysCommand %s exec failed: %s",
>+		    options.authorized_keys_command, strerror(errno));
>+		_exit(127);
>+	default: /* parent */
>+		break;
>+	}
>+
>+	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 */
>+		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));
>+		goto out;
>+	}
>+	found_key = ok;
>+ out:
>+	restore_uid();
>+	return found_key;
>+}
>+
>+/*
>+ * Check whether key authenticates and authorises the user.
>+ */
> int
> user_key_allowed(struct passwd *pw, Key *key)
> {
>@@ -448,6 +606,10 @@ user_key_allowed(struct passwd *pw, Key *key)
> 	if (success)
> 		return success;
> 
>+	success = user_key_command_allowed2(pw, key);
>+	if (success > 0)
>+		return success;
>+
> 	for (i = 0; !success && i < options.num_authkeys_files; i++) {
> 		file = expand_authorized_keys(
> 		    options.authorized_keys_files[i], pw);
>diff --git servconf.c servconf.c
>index a9398b4..0539a2b 100644
>--- servconf.c
>+++ servconf.c
>@@ -128,6 +128,8 @@ initialize_server_options(ServerOptions *options)
> 	options->num_permitted_opens = -1;
> 	options->adm_forced_command = NULL;
> 	options->chroot_directory = NULL;
>+	options->authorized_keys_command = NULL;
>+	options->authorized_keys_command_user = NULL;
> 	options->zero_knowledge_password_authentication = -1;
> 	options->revoked_keys_file = NULL;
> 	options->trusted_user_ca_keys = NULL;
>@@ -302,6 +304,7 @@ typedef enum {
> 	sZeroKnowledgePasswordAuthentication, sHostCertificate,
> 	sRevokedKeys, sTrustedUserCAKeys, sAuthorizedPrincipalsFile,
> 	sKexAlgorithms, sIPQoS, sVersionAddendum,
>+	sAuthorizedKeysCommand, sAuthorizedKeysCommandUser,
> 	sDeprecated, sUnsupported
> } ServerOpCodes;
> 
>@@ -414,6 +417,8 @@ static struct {
> 	{ "authorizedprincipalsfile", sAuthorizedPrincipalsFile, SSHCFG_ALL },
> 	{ "kexalgorithms", sKexAlgorithms, SSHCFG_GLOBAL },
> 	{ "ipqos", sIPQoS, SSHCFG_ALL },
>+	{ "authorizedkeyscommand", sAuthorizedKeysCommand, SSHCFG_ALL },
>+	{ "authorizedkeyscommanduser", sAuthorizedKeysCommandUser, SSHCFG_ALL },
> 	{ "versionaddendum", sVersionAddendum, SSHCFG_GLOBAL },
> 	{ NULL, sBadOption, 0 }
> };
>@@ -1453,6 +1458,26 @@ process_server_config_line(ServerOptions *options, char *line,
> 		}
> 		return 0;
> 
>+	case sAuthorizedKeysCommand:
>+		len = strspn(cp, WHITESPACE);
>+		if (*activep && options->authorized_keys_command == NULL) {
>+			options->authorized_keys_command = xstrdup(cp + len);
>+			if (*options->authorized_keys_command != '/') {
>+				fatal("%.200s line %d: AuthorizedKeysCommand "
>+				    "must be an absolute path",
>+				    filename, linenum);
>+			}
>+		}
>+		return 0;
>+
>+	case sAuthorizedKeysCommandUser:
>+		charptr = &options->authorized_keys_command_user;
>+
>+		arg = strdelim(&cp);
>+		if (*activep && *charptr == NULL)
>+			*charptr = xstrdup(arg);
>+		break;
>+
> 	case sDeprecated:
> 		logit("%s line %d: Deprecated option %s",
> 		    filename, linenum, arg);
>@@ -1603,6 +1628,8 @@ copy_set_server_options(ServerOptions *dst, ServerOptions *src, int preauth)
> 	M_CP_INTOPT(hostbased_uses_name_from_packet_only);
> 	M_CP_INTOPT(kbd_interactive_authentication);
> 	M_CP_INTOPT(zero_knowledge_password_authentication);
>+	M_CP_STROPT(authorized_keys_command);
>+	M_CP_STROPT(authorized_keys_command_user);
> 	M_CP_INTOPT(permit_root_login);
> 	M_CP_INTOPT(permit_empty_passwd);
> 
>@@ -1858,6 +1885,8 @@ dump_config(ServerOptions *o)
> 	dump_cfg_string(sAuthorizedPrincipalsFile,
> 	    o->authorized_principals_file);
> 	dump_cfg_string(sVersionAddendum, o->version_addendum);
>+	dump_cfg_string(sAuthorizedKeysCommand, o->authorized_keys_command);
>+	dump_cfg_string(sAuthorizedKeysCommandUser, o->authorized_keys_command_user);
> 
> 	/* string arguments requiring a lookup */
> 	dump_cfg_string(sLogLevel, log_level_name(o->log_level));
>diff --git servconf.h servconf.h
>index 7e8f0ec..e5b872b 100644
>--- servconf.h
>+++ servconf.h
>@@ -164,6 +164,8 @@ typedef struct {
> 	char   *revoked_keys_file;
> 	char   *trusted_user_ca_keys;
> 	char   *authorized_principals_file;
>+	char   *authorized_keys_command;
>+	char   *authorized_keys_command_user;
> 
> 	char   *version_addendum;	/* Appended to SSH banner */
> }       ServerOptions;
>diff --git sshd.c sshd.c
>index 51ccb82..1fd39ca 100644
>--- sshd.c
>+++ sshd.c
>@@ -336,9 +336,20 @@ main_sigchld_handler(int sig)
> static void
> grace_alarm_handler(int sig)
> {
>+	pid_t pgid;
>+
> 	if (use_privsep && pmonitor != NULL && pmonitor->m_pid > 0)
> 		kill(pmonitor->m_pid, SIGALRM);
> 
>+	/*
>+	 * Try to kill any processes that we have spawned, E.g. authorized
>+	 * keys command helpers.
>+	 */
>+	if ((pgid = getpgid(0)) == getpid()) {
>+		signal(SIGTERM, SIG_IGN);
>+		killpg(pgid, SIGTERM);
>+	}
>+
> 	/* Log error and exit. */
> 	sigdie("Timeout before authentication for %s", get_remote_ipaddr());
> }
>diff --git sshd_config sshd_config
>index 1c600a8..997150a 100644
>--- sshd_config
>+++ sshd_config
>@@ -49,6 +49,9 @@ AuthorizedKeysFile	.ssh/authorized_keys
> 
> #AuthorizedPrincipalsFile none
> 
>+#AuthorizedKeysCommand none
>+#AuthorizedKeysCommandRunAs nobody
>+
> # For this to work you will also need host keys in /etc/ssh/ssh_known_hosts
> #RhostsRSAAuthentication no
> # similar for protocol version 2
>diff --git sshd_config.5 sshd_config.5
>index 18f6a2f..ee6f6b9 100644
>--- sshd_config.5
>+++ sshd_config.5
>@@ -151,6 +151,20 @@ See
> in
> .Xr ssh_config 5
> for more information on patterns.
>+.It Cm AuthorizedKeysCommand
>+Specifies a program to be used for lookup of the user's public keys.
>+The program will be invoked with a single argument of the username
>+being authenticated, and 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
>+and authorize the user then public key authentication continues using the usual
>+.Cm AuthorizedKeysFile
>+files.
>+By default, no AuthorizedKeysCommand is run.
>+.It Cm AuthorizedKeysCommandUser
>+Specifies the user under whose account the AuthorizedKeysCommand is run.
>+The default is the user being authenticated.
> .It Cm AuthorizedKeysFile
> Specifies the file that contains the public keys that can be used
> for user authentication.
>@@ -713,6 +727,8 @@ Available keywords are
> .Cm AllowTcpForwarding ,
> .Cm AllowUsers ,
> .Cm AuthorizedKeysFile ,
>+.Cm AuthorizedKeysCommand ,
>+.Cm AuthorizedKeysCommandUser ,
> .Cm AuthorizedPrincipalsFile ,
> .Cm Banner ,
> .Cm ChrootDirectory ,

Actions: View | Diff

Attachments on bug 1663: 1703 | 1765 | 1804 | 1811 | 1850 | 1895 | 1896 | 1915 | 1920 | 2067 | 2083 | 2191