Attachment 2192 Details for Bug 983 – new multiple required authentication methods patch

[patch] new multiple required authentication methods patch

authenticationmethods.diff (text/plain), 16.05 KB, created by Damien Miller on 2012-11-01 11:50:53 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2012-11-01 11:50:53 AEDT

Size: 16.05 KB

patch

obsolete

>diff --git auth.h auth.h
>index a244e3c..ec39bab 100644
>--- auth.h
>+++ auth.h
>@@ -57,6 +57,8 @@ struct Authctxt {
> 	void		*kbdintctxt;
> 	void		*jpake_ctx;
> 	auth_session_t	*as;
>+	char		**auth_methods;	/* modified from server config */
>+	u_int		 num_auth_methods;
> #ifdef KRB5
> 	krb5_context	 krb5_ctx;
> 	krb5_ccache	 krb5_fwd_ccache;
>@@ -130,6 +132,9 @@ void	userauth_finish(Authctxt *, int, char *);
> int	auth_root_allowed(char *);
> 
> char	*auth2_read_banner(void);
>+int	 auth2_methods_valid(const char *, int);
>+int	 auth2_update_methods_lists(Authctxt *, const char *);
>+int	 auth2_setup_methods_lists(Authctxt *);
> 
> void	privsep_challenge_enable(void);
> 
>diff --git auth1.c auth1.c
>index 63fecc1..d568cae 100644
>--- auth1.c
>+++ auth1.c
>@@ -338,6 +338,11 @@ do_authentication(Authctxt *authctxt)
> 		authctxt->pw = fakepw();
> 	}
> 
>+	/* Configuration may have changed as a result of Match */
>+	if (options.num_auth_methods != 0)
>+		fatal("AuthenticationMethods is not supported with SSH "
>+		    "protocol 1");
>+
> 	setproctitle("%s%s", authctxt->valid ? user : "unknown",
> 	    use_privsep ? " [net]" : "");
> 
>diff --git auth2.c auth2.c
>index 21f727e..34906d7 100644
>--- auth2.c
>+++ auth2.c
>@@ -92,8 +92,10 @@ static void input_service_request(int, u_int32_t, void *);
> static void input_userauth_request(int, u_int32_t, void *);
> 
> /* helper */
>-static Authmethod *authmethod_lookup(const char *);
>-static char *authmethods_get(void);
>+static Authmethod *authmethod_lookup(Authctxt *, const char *);
>+static char *authmethods_get(Authctxt *authctxt);
>+static int method_allowed(Authctxt *, const char *);
>+static int list_starts_with(const char *, const char *);
> 
> char *
> auth2_read_banner(void)
>@@ -235,6 +237,8 @@ input_userauth_request(int type, u_int32_t seq, void *ctxt)
> 		if (use_privsep)
> 			mm_inform_authserv(service, style);
> 		userauth_banner();
>+		if (auth2_setup_methods_lists(authctxt) != 0)
>+			packet_disconnect("no authentication methods enabled");
> 	} else if (strcmp(user, authctxt->user) != 0 ||
> 	    strcmp(service, authctxt->service) != 0) {
> 		packet_disconnect("Change of username or service not allowed: "
>@@ -257,7 +261,7 @@ input_userauth_request(int type, u_int32_t seq, void *ctxt)
> 	authctxt->server_caused_failure = 0;
> 
> 	/* try to authenticate user */
>-	m = authmethod_lookup(method);
>+	m = authmethod_lookup(authctxt, method);
> 	if (m != NULL && authctxt->failures < options.max_authtries) {
> 		debug2("input_userauth_request: try method %s", method);
> 		authenticated =	m->userauth(authctxt);
>@@ -273,6 +277,7 @@ void
> userauth_finish(Authctxt *authctxt, int authenticated, char *method)
> {
> 	char *methods;
>+	int partial = 0;
> 
> 	if (!authctxt->valid && authenticated)
> 		fatal("INTERNAL ERROR: authenticated invalid user %s",
>@@ -289,7 +294,13 @@ userauth_finish(Authctxt *authctxt, int authenticated, char *method)
> 	if (authctxt->postponed)
> 		return;
> 
>-	/* XXX todo: check if multiple auth methods are needed */
>+	if (authenticated && options.num_auth_methods != 0) {
>+		if (!auth2_update_methods_lists(authctxt, method)) {
>+			authenticated = 0;
>+			partial = 1;
>+		}
>+	}
>+
> 	if (authenticated == 1) {
> 		/* turn off userauth */
> 		dispatch_set(SSH2_MSG_USERAUTH_REQUEST, &dispatch_protocol_ignore);
>@@ -305,34 +316,57 @@ userauth_finish(Authctxt *authctxt, int authenticated, char *method)
> 			authctxt->failures++;
> 		if (authctxt->failures >= options.max_authtries)
> 			packet_disconnect(AUTH_FAIL_MSG, authctxt->user);
>-		methods = authmethods_get();
>+		methods = authmethods_get(authctxt);
>+		debug3("%s: failure partial=%d next methods=\"%s\"", __func__,
>+		    partial, methods);
> 		packet_start(SSH2_MSG_USERAUTH_FAILURE);
> 		packet_put_cstring(methods);
>-		packet_put_char(0);	/* XXX partial success, unused */
>+		packet_put_char(partial);
> 		packet_send();
> 		packet_write_wait();
> 		xfree(methods);
> 	}
> }
> 
>+/*
>+ * Checks whether method is allowed by at least one AuthenticationMethods
>+ * methods list. Returns 1 if allowed, or no methods lists configured.
>+ * 0 otherwise.
>+ */
>+static int
>+method_allowed(Authctxt *authctxt, const char *method)
>+{
>+	u_int i;
>+
>+	if (options.num_auth_methods == 0)
>+		return 1;
>+	for (i = 0; i < authctxt->num_auth_methods; i++) {
>+		if (list_starts_with(authctxt->auth_methods[i], method))
>+			return 1;
>+	}
>+	return 0;
>+}
>+
> static char *
>-authmethods_get(void)
>+authmethods_get(Authctxt *authctxt)
> {
> 	Buffer b;
> 	char *list;
>-	int i;
>+	u_int i;
> 
> 	buffer_init(&b);
> 	for (i = 0; authmethods[i] != NULL; i++) {
> 		if (strcmp(authmethods[i]->name, "none") == 0)
> 			continue;
>-		if (authmethods[i]->enabled != NULL &&
>-		    *(authmethods[i]->enabled) != 0) {
>-			if (buffer_len(&b) > 0)
>-				buffer_append(&b, ",", 1);
>-			buffer_append(&b, authmethods[i]->name,
>-			    strlen(authmethods[i]->name));
>-		}
>+		if (authmethods[i]->enabled == NULL ||
>+		    *(authmethods[i]->enabled) == 0)
>+			continue;
>+		if (!method_allowed(authctxt, authmethods[i]->name))
>+			continue;
>+		if (buffer_len(&b) > 0)
>+			buffer_append(&b, ",", 1);
>+		buffer_append(&b, authmethods[i]->name,
>+		    strlen(authmethods[i]->name));
> 	}
> 	buffer_append(&b, "\0", 1);
> 	list = xstrdup(buffer_ptr(&b));
>@@ -341,7 +375,7 @@ authmethods_get(void)
> }
> 
> static Authmethod *
>-authmethod_lookup(const char *name)
>+authmethod_lookup(Authctxt *authctxt, const char *name)
> {
> 	int i;
> 
>@@ -349,10 +383,154 @@ authmethod_lookup(const char *name)
> 		for (i = 0; authmethods[i] != NULL; i++)
> 			if (authmethods[i]->enabled != NULL &&
> 			    *(authmethods[i]->enabled) != 0 &&
>-			    strcmp(name, authmethods[i]->name) == 0)
>+			    strcmp(name, authmethods[i]->name) == 0 &&
>+			    method_allowed(authctxt, authmethods[i]->name))
> 				return authmethods[i];
> 	debug2("Unrecognized authentication method name: %s",
> 	    name ? name : "NULL");
> 	return NULL;
> }
> 
>+/*
>+ * Check a comma-separated list of methods for validity. Is need_enable is
>+ * non-zero, then also require that the methods are enabled.
>+ * Returns 0 on success or -1 if the methods list is invalid.
>+ */
>+int
>+auth2_methods_valid(const char *_methods, int need_enable)
>+{
>+	char *methods, *omethods, *method;
>+	u_int i, found;
>+	int ret = -1;
>+
>+	if (*_methods == '\0') {
>+		error("empty authentication method list");
>+		return -1;
>+	}
>+	omethods = methods = xstrdup(_methods);
>+	while ((method = strsep(&methods, ",")) != NULL) {
>+		for (found = i = 0; !found && authmethods[i] != NULL; i++) {
>+			if (strcmp(method, authmethods[i]->name) != 0)
>+				continue;
>+			if (need_enable) {
>+				if (authmethods[i]->enabled == NULL ||
>+				    *(authmethods[i]->enabled) == 0) {
>+					error("Disabled method \"%s\" in "
>+					    "AuthenticationMethods list \"%s\"",
>+					    method, _methods);
>+					goto out;
>+				}
>+			}
>+			found = 1;
>+			break;
>+		}
>+		if (!found) {
>+			error("Unknown authentication method \"%s\" in list",
>+			    method);
>+			goto out;
>+		}
>+	}
>+	ret = 0;
>+ out:
>+	free(omethods);
>+	return ret;
>+}
>+
>+/*
>+ * Prune the AuthenticationMethods supplied in the configuration, removing
>+ * any methods lists that include disabled methods. Note that this might
>+ * leave authctxt->num_auth_methods == 0, even when multiple required auth
>+ * has been requested. For this reason, all tests for whether multiple is
>+ * enabled should consult options.num_auth_methods directly.
>+ */
>+int
>+auth2_setup_methods_lists(Authctxt *authctxt)
>+{
>+	u_int i;
>+
>+	if (options.num_auth_methods == 0)
>+		return 0;
>+	debug3("%s: checking methods", __func__);
>+	authctxt->auth_methods = xcalloc(options.num_auth_methods,
>+	    sizeof(*authctxt->auth_methods));
>+	authctxt->num_auth_methods = 0;
>+	for (i = 0; i < options.num_auth_methods; i++) {
>+		if (auth2_methods_valid(options.auth_methods[i], 1) != 0) {
>+			logit("Authentication methods list \"%s\" contains "
>+			    "disabled method, skipping",
>+			    options.auth_methods[i]);
>+			continue;
>+		}
>+		debug("authentication methods list %d: %s",
>+		    authctxt->num_auth_methods, options.auth_methods[i]);
>+		authctxt->auth_methods[authctxt->num_auth_methods++] =
>+		    xstrdup(options.auth_methods[i]);
>+	}
>+	if (authctxt->num_auth_methods == 0) {
>+		error("No AuthenticationMethods left after eliminating "
>+		    "disabled methods");
>+		return -1;
>+	}
>+	return 0;
>+}
>+
>+static int
>+list_starts_with(const char *methods, const char *method)
>+{
>+	size_t l = strlen(method);
>+
>+	if (strncmp(methods, method, l) != 0)
>+		return 0;
>+	if (methods[l] != ',' && methods[l] != '\0')
>+		return 0;
>+	return 1;
>+}
>+
>+/*
>+ * Remove method from the start of a comma-separated list of methods.
>+ * Returns 0 if the list of methods did not start with that method or 1
>+ * if it did.
>+ */
>+static int
>+remove_method(char **methods, const char *method)
>+{
>+	char *omethods = *methods;
>+	size_t l = strlen(method);
>+
>+	if (!list_starts_with(omethods, method))
>+		return 0;
>+	*methods = xstrdup(omethods + l + (omethods[l] == ',' ? 1 : 0));
>+	free(omethods);
>+	return 1;
>+}
>+
>+/*
>+ * Called after successful authentication. Will remove the successful method
>+ * from the start of each list in which it occurs. If it was the last method
>+ * in any list, then authentication is deemed successful.
>+ * Returns 1 if the method completed any authentication list or 0 otherwise.
>+ */
>+int
>+auth2_update_methods_lists(Authctxt *authctxt, const char *method)
>+{
>+	u_int i, found = 0;
>+
>+	debug3("%s: updating methods list after \"%s\"", __func__, method);
>+	for (i = 0; i < authctxt->num_auth_methods; i++) {
>+		if (!remove_method(&(authctxt->auth_methods[i]), method))
>+			continue;
>+		found = 1;
>+		if (*authctxt->auth_methods[i] == '\0') {
>+			debug2("authentication methods list %d complete", i);
>+			return 1;
>+		}
>+		debug3("authentication methods list %d remaining: \"%s\"",
>+		    i, authctxt->auth_methods[i]);
>+	}
>+	/* This should not happen, but would be bad if it did */
>+	if (!found)
>+		fatal("%s: method not in AuthenticationMethods", __func__);
>+	return 0;
>+}
>+
>+
>diff --git monitor.c monitor.c
>index d8dfe61..749db89 100644
>--- monitor.c
>+++ monitor.c
>@@ -301,6 +301,21 @@ monitor_child_preauth(Authctxt *_authctxt, struct monitor *pmonitor)
> 	while (!authenticated) {
> 		auth_method = "unknown";
> 		authenticated = (monitor_read(pmonitor, mon_dispatch, &ent) == 1);
>+
>+		/* Special handling for multiple required authentications */
>+		if (options.num_auth_methods != 0) {
>+			if (!compat20)
>+				fatal("AuthenticationMethods is not supported"
>+				    "with SSH protocol 1");
>+			if (authenticated &&
>+			    !auth2_update_methods_lists(authctxt,
>+			    auth_method)) {
>+				debug3("%s: method %s: partial", __func__,
>+				    auth_method);
>+				authenticated = 0;
>+			}
>+		}
>+
> 		if (authenticated) {
> 			if (!(ent->flags & MON_AUTHDECIDE))
> 				fatal("%s: unexpected authentication from %d",
>@@ -309,7 +324,6 @@ monitor_child_preauth(Authctxt *_authctxt, struct monitor *pmonitor)
> 			    !auth_root_allowed(auth_method))
> 				authenticated = 0;
> 		}
>-
> 		if (ent->flags & (MON_AUTHDECIDE|MON_ALOG)) {
> 			auth_log(authctxt, authenticated, auth_method,
> 			    compat20 ? " ssh2" : "");
>@@ -687,7 +701,17 @@ mm_answer_pwnamallow(int sock, Buffer *m)
> 	COPY_MATCH_STRING_OPTS();
> #undef M_CP_STROPT
> #undef M_CP_STRARRAYOPT
>-	
>+
>+	/* Create valid auth method lists */
>+	if (compat20 && auth2_setup_methods_lists(authctxt) != 0) {
>+		/*
>+		 * The monitor will continue long enough to let the child
>+		 * run to it's packet_disconnect(), but it must not allow any
>+		 * authentication to succeed.
>+		 */
>+		debug("%s: no valid authentication method lists", __func__);
>+	}
>+
> 	debug3("%s: sending MONITOR_ANS_PWNAM: %d", __func__, allowed);
> 	mm_request_send(sock, MONITOR_ANS_PWNAM, m);
> 
>@@ -819,7 +843,10 @@ mm_answer_bsdauthrespond(int sock, Buffer *m)
> 	debug3("%s: sending authenticated: %d", __func__, authok);
> 	mm_request_send(sock, MONITOR_ANS_BSDAUTHRESPOND, m);
> 
>-	auth_method = "bsdauth";
>+	if (compat20)
>+		auth_method = "keyboard-interactive";
>+	else
>+		auth_method = "bsdauth";
> 
> 	return (authok != 0);
> }
>diff --git servconf.c servconf.c
>index 2d26c3b..0782ada 100644
>--- servconf.c
>+++ servconf.c
>@@ -46,6 +46,8 @@
> #include "groupaccess.h"
> #include "canohost.h"
> #include "packet.h"
>+#include "hostfile.h"
>+#include "auth.h"
> 
> static void add_listen_addr(ServerOptions *, char *, int);
> static void add_one_listen_addr(ServerOptions *, char *, int);
>@@ -305,6 +307,7 @@ typedef enum {
> 	sRevokedKeys, sTrustedUserCAKeys, sAuthorizedPrincipalsFile,
> 	sKexAlgorithms, sIPQoS, sVersionAddendum,
> 	sAuthorizedKeysCommand, sAuthorizedKeysCommandUser,
>+	sAuthenticationMethods,
> 	sDeprecated, sUnsupported
> } ServerOpCodes;
> 
>@@ -420,6 +423,7 @@ static struct {
> 	{ "authorizedkeyscommand", sAuthorizedKeysCommand, SSHCFG_ALL },
> 	{ "authorizedkeyscommanduser", sAuthorizedKeysCommandUser, SSHCFG_ALL },
> 	{ "versionaddendum", sVersionAddendum, SSHCFG_GLOBAL },
>+	{ "authenticationmethods", sAuthenticationMethods, SSHCFG_ALL },
> 	{ NULL, sBadOption, 0 }
> };
> 
>@@ -1477,6 +1481,24 @@ process_server_config_line(ServerOptions *options, char *line,
> 			*charptr = xstrdup(arg);
> 		break;
> 
>+	case sAuthenticationMethods:
>+		if (*activep && options->num_auth_methods == 0) {
>+			while ((arg = strdelim(&cp)) && *arg != '\0') {
>+				if (options->num_auth_methods >=
>+				    MAX_AUTH_METHODS)
>+					fatal("%s line %d: "
>+					    "too many authentication methods.",
>+					    filename, linenum);
>+				if (auth2_methods_valid(arg, 0) != 0)
>+					fatal("%s line %d: invalid "
>+					    "authentication method list.",
>+					    filename, linenum);
>+				options->auth_methods[
>+				    options->num_auth_methods++] = xstrdup(arg);
>+			}
>+		}
>+		return 0;
>+
> 	case sDeprecated:
> 		logit("%s line %d: Deprecated option %s",
> 		    filename, linenum, arg);
>@@ -1903,6 +1925,8 @@ dump_config(ServerOptions *o)
> 	dump_cfg_strarray(sAllowGroups, o->num_allow_groups, o->allow_groups);
> 	dump_cfg_strarray(sDenyGroups, o->num_deny_groups, o->deny_groups);
> 	dump_cfg_strarray(sAcceptEnv, o->num_accept_env, o->accept_env);
>+	dump_cfg_strarray_oneline(sAuthenticationMethods,
>+	    o->num_auth_methods, o->auth_methods);
> 
> 	/* other arguments */
> 	for (i = 0; i < o->num_subsystems; i++)
>diff --git servconf.h servconf.h
>index ab9929f..fa32f6e 100644
>--- servconf.h
>+++ servconf.h
>@@ -28,6 +28,7 @@
> #define MAX_ACCEPT_ENV		256	/* Max # of env vars. */
> #define MAX_MATCH_GROUPS	256	/* Max # of groups for Match. */
> #define MAX_AUTHKEYS_FILES	256	/* Max # of authorized_keys files. */
>+#define MAX_AUTH_METHODS	256	/* Max # of AuthenticationMethods. */
> 
> /* permit_root_login */
> #define	PERMIT_NOT_SET		-1
>@@ -168,6 +169,9 @@ typedef struct {
> 	char   *authorized_keys_command_user;
> 
> 	char   *version_addendum;	/* Appended to SSH banner */
>+
>+	u_int	num_auth_methods;
>+	char   *auth_methods[MAX_AUTH_METHODS];
> }       ServerOptions;
> 
> /* Information about the incoming connection as used by Match */
>@@ -197,6 +201,7 @@ struct connection_info {
> 		M_CP_STRARRAYOPT(allow_groups, num_allow_groups); \
> 		M_CP_STRARRAYOPT(deny_groups, num_deny_groups); \
> 		M_CP_STRARRAYOPT(accept_env, num_accept_env); \
>+		M_CP_STRARRAYOPT(auth_methods, num_auth_methods); \
> 	} while (0)
> 
> struct connection_info *get_connection_info(int, int);
>diff --git sshd.c sshd.c
>index f732a58..ad5bb00 100644
>--- sshd.c
>+++ sshd.c
>@@ -1302,6 +1302,7 @@ main(int ac, char **av)
> 	int remote_port;
> 	char *line;
> 	int config_s[2] = { -1 , -1 };
>+	u_int n;
> 	u_int64_t ibytes, obytes;
> 	mode_t new_umask;
> 	Key *key;
>@@ -1490,6 +1491,26 @@ main(int ac, char **av)
> 	if (options.challenge_response_authentication)
> 		options.kbd_interactive_authentication = 1;
> 
>+	/*
>+	 * Check whether there is any path through configured auth methods.
>+	 * Unfortunately it is not possible to verify this generally before
>+	 * daemonisation in the presence of Match block, but this catches
>+	 * and warns for trivial misconfigurations that could break login.
>+	 */
>+	if (options.num_auth_methods != 0) {
>+		if ((options.protocol & SSH_PROTO_1))
>+			fatal("AuthenticationMethods is not supported with "
>+			    "SSH protocol 1");
>+		for (n = 0; n < options.num_auth_methods; n++) {
>+			if (auth2_methods_valid(options.auth_methods[n],
>+			    1) == 0)
>+				break;
>+		}
>+		if (n >= options.num_auth_methods)
>+			fatal("AuthenticationMethods cannot be satisfied by "
>+			    "enabled authentication methods");
>+	}
>+
> 	/* set default channel AF */
> 	channel_set_af(options.address_family);
>

Actions: View | Diff

Attachments on bug 983: 807 | 941 | 1121 | 1122 | 1123 | 1455 | 1518 | 1521 | 1567 | 1667 | 1768 | 1955 | 1999 | 2079 | 2084 | 2096 | 2138 | 2177 | 2178 | 2192 | 2196