From: djm <djm@openbsd.org>
Date: Mon, 4 Mar 2024 02:16:11 +0000 (+0000)
Subject: Separate parsing of string array options from applying them to the
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c2643f71b2531cc2e47e8f74e010c6fc66fd76d5;p=openbsd

Separate parsing of string array options from applying them to the
active configuration. This fixes the config parser from erroneously
rejecting cases like:

AuthenticationMethods password
Match User ivy
  AuthenticationMethods any

bz3657 ok markus@
---

diff --git a/usr.bin/ssh/misc.c b/usr.bin/ssh/misc.c
index 96688133349..7ec927de93e 100644
--- a/usr.bin/ssh/misc.c
+++ b/usr.bin/ssh/misc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.189 2023/10/12 03:36:32 djm Exp $ */
+/* $OpenBSD: misc.c,v 1.190 2024/03/04 02:16:11 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2005-2020 Damien Miller.  All rights reserved.
@@ -2537,6 +2537,19 @@ opt_array_append(const char *file, const int line, const char *directive,
 	opt_array_append2(file, line, directive, array, NULL, lp, s, 0);
 }
 
+void
+opt_array_free2(char **array, int **iarray, u_int l)
+{
+	u_int i;
+
+	if (array == NULL || l == 0)
+		return;
+	for (i = 0; i < l; i++)
+		free(array[i]);
+	free(array);
+	free(iarray);
+}
+
 sshsig_t
 ssh_signal(int signum, sshsig_t handler)
 {
diff --git a/usr.bin/ssh/misc.h b/usr.bin/ssh/misc.h
index b10b503b43c..d460dc7cd62 100644
--- a/usr.bin/ssh/misc.h
+++ b/usr.bin/ssh/misc.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.h,v 1.106 2023/10/11 22:42:26 djm Exp $ */
+/* $OpenBSD: misc.h,v 1.107 2024/03/04 02:16:11 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -208,6 +208,7 @@ void	opt_array_append(const char *file, const int line,
 void	opt_array_append2(const char *file, const int line,
 	    const char *directive, char ***array, int **iarray, u_int *lp,
 	    const char *s, int i);
+void	opt_array_free2(char **array, int **iarray, u_int l);
 
 struct timespec;
 void ptimeout_init(struct timespec *pt);
diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c
index b3c8d626b9b..71ef9056c66 100644
--- a/usr.bin/ssh/readconf.c
+++ b/usr.bin/ssh/readconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.c,v 1.384 2024/01/11 01:45:36 djm Exp $ */
+/* $OpenBSD: readconf.c,v 1.385 2024/03/04 02:16:11 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1009,21 +1009,24 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host,
 {
 	char *str, **charptr, *endofnumber, *keyword, *arg, *arg2, *p;
 	char **cpptr, ***cppptr, fwdarg[256];
-	u_int i, *uintptr, uvalue, max_entries = 0;
+	u_int i, *uintptr, max_entries = 0;
 	int r, oactive, negated, opcode, *intptr, value, value2, cmdline = 0;
-	int remotefwd, dynamicfwd, ca_only = 0;
+	int remotefwd, dynamicfwd, ca_only = 0, found = 0;
 	LogLevel *log_level_ptr;
 	SyslogFacility *log_facility_ptr;
 	long long val64;
 	size_t len;
 	struct Forward fwd;
 	const struct multistate *multistate_ptr;
-	struct allowed_cname *cname;
 	glob_t gl;
 	const char *errstr;
 	char **oav = NULL, **av;
 	int oac = 0, ac;
 	int ret = -1;
+	struct allowed_cname *cnames = NULL;
+	u_int ncnames = 0;
+	char **strs = NULL; /* string array arguments; freed implicitly */
+	u_int nstrs = 0;
 
 	if (activep == NULL) { /* We are processing a command line directive */
 		cmdline = 1;
@@ -1639,14 +1642,13 @@ parse_pubkey_algos:
 	case oPermitRemoteOpen:
 		uintptr = &options->num_permitted_remote_opens;
 		cppptr = &options->permitted_remote_opens;
-		uvalue = *uintptr;	/* modified later */
-		i = 0;
+		found = *uintptr == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			arg2 = xstrdup(arg);
 			/* Allow any/none only in first position */
 			if (strcasecmp(arg, "none") == 0 ||
 			    strcasecmp(arg, "any") == 0) {
-				if (i > 0 || ac > 0) {
+				if (nstrs > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"%s\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword, arg);
@@ -1672,17 +1674,20 @@ parse_pubkey_algos:
 					    lookup_opcode_name(opcode));
 				}
 			}
-			if (*activep && uvalue == 0) {
-				opt_array_append(filename, linenum,
-				    lookup_opcode_name(opcode),
-				    cppptr, uintptr, arg2);
-			}
+			opt_array_append(filename, linenum,
+			    lookup_opcode_name(opcode),
+			    &strs, &nstrs, arg2);
 			free(arg2);
-			i++;
 		}
-		if (i == 0)
+		if (nstrs == 0)
 			fatal("%s line %d: missing %s specification",
 			    filename, linenum, lookup_opcode_name(opcode));
+		if (found && *activep) {
+			*cppptr = strs;
+			*uintptr = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
+		}
 		break;
 
 	case oClearAllForwardings:
@@ -1800,12 +1805,14 @@ parse_pubkey_algos:
 		goto parse_int;
 
 	case oSendEnv:
+		/* XXX appends to list; doesn't respect first-match-wins */
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0' || strchr(arg, '=') != NULL) {
 				error("%s line %d: Invalid environment name.",
 				    filename, linenum);
 				goto out;
 			}
+			found = 1;
 			if (!*activep)
 				continue;
 			if (*arg == '-') {
@@ -1817,27 +1824,38 @@ parse_pubkey_algos:
 			    lookup_opcode_name(opcode),
 			    &options->send_env, &options->num_send_env, arg);
 		}
+		if (!found) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
 		break;
 
 	case oSetEnv:
-		value = options->num_setenv;
+		found = options->num_setenv == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (strchr(arg, '=') == NULL) {
 				error("%s line %d: Invalid SetEnv.",
 				    filename, linenum);
 				goto out;
 			}
-			if (!*activep || value != 0)
-				continue;
-			if (lookup_setenv_in_list(arg, options->setenv,
-			    options->num_setenv) != NULL) {
+			if (lookup_setenv_in_list(arg, strs, nstrs) != NULL) {
 				debug2("%s line %d: ignoring duplicate env "
 				    "name \"%.64s\"", filename, linenum, arg);
 				continue;
 			}
 			opt_array_append(filename, linenum,
 			    lookup_opcode_name(opcode),
-			    &options->setenv, &options->num_setenv, arg);
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->setenv = strs;
+			options->num_setenv = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
@@ -2046,52 +2064,46 @@ parse_pubkey_algos:
 		goto parse_flag;
 
 	case oCanonicalDomains:
-		value = options->num_canonical_domains != 0;
-		i = 0;
+		found = options->num_canonical_domains == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
-			if (*arg == '\0') {
-				error("%s line %d: keyword %s empty argument",
-				    filename, linenum, keyword);
-				goto out;
-			}
 			/* Allow "none" only in first position */
 			if (strcasecmp(arg, "none") == 0) {
-				if (i > 0 || ac > 0) {
+				if (nstrs > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"none\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword);
 					goto out;
 				}
 			}
-			i++;
 			if (!valid_domain(arg, 1, &errstr)) {
 				error("%s line %d: %s", filename, linenum,
 				    errstr);
 				goto out;
 			}
-			if (!*activep || value)
-				continue;
-			if (options->num_canonical_domains >=
-			    MAX_CANON_DOMAINS) {
-				error("%s line %d: too many hostname suffixes.",
-				    filename, linenum);
-				goto out;
-			}
-			options->canonical_domains[
-			    options->num_canonical_domains++] = xstrdup(arg);
+			opt_array_append(filename, linenum, keyword,
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->canonical_domains = strs;
+			options->num_canonical_domains = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
 	case oCanonicalizePermittedCNAMEs:
-		value = options->num_permitted_cnames != 0;
-		i = 0;
+		found = options->num_permitted_cnames == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			/*
 			 * Either 'none' (only in first position), '*' for
 			 * everything or 'list:list'
 			 */
 			if (strcasecmp(arg, "none") == 0) {
-				if (i > 0 || ac > 0) {
+				if (ncnames > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"none\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword);
@@ -2112,19 +2124,25 @@ parse_pubkey_algos:
 				*arg2 = '\0';
 				arg2++;
 			}
-			i++;
-			if (!*activep || value)
-				continue;
-			if (options->num_permitted_cnames >=
-			    MAX_CANON_DOMAINS) {
-				error("%s line %d: too many permitted CNAMEs.",
-				    filename, linenum);
-				goto out;
+			cnames = xrecallocarray(cnames, ncnames, ncnames + 1,
+			    sizeof(*cnames));
+			cnames[ncnames].source_list = xstrdup(arg);
+			cnames[ncnames].target_list = xstrdup(arg2);
+			ncnames++;
+		}
+		if (ncnames == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->permitted_cnames = cnames;
+			options->num_permitted_cnames = ncnames;
+		} else {
+			for (i = 0; i < ncnames; i++) {
+				free(cnames[i].source_list);
+				free(cnames[i].target_list);
 			}
-			cname = options->permitted_cnames +
-			    options->num_permitted_cnames++;
-			cname->source_list = xstrdup(arg);
-			cname->target_list = xstrdup(arg2);
+			free(cnames);
 		}
 		break;
 
@@ -2306,12 +2324,11 @@ parse_pubkey_algos:
 		break;
 
 	case oChannelTimeout:
-		uvalue = options->num_channel_timeouts;
-		i = 0;
+		found = options->num_channel_timeouts == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			/* Allow "none" only in first position */
 			if (strcasecmp(arg, "none") == 0) {
-				if (i > 0 || ac > 0) {
+				if (nstrs > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"none\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword);
@@ -2322,11 +2339,18 @@ parse_pubkey_algos:
 				fatal("%s line %d: invalid channel timeout %s",
 				    filename, linenum, arg);
 			}
-			if (!*activep || uvalue != 0)
-				continue;
 			opt_array_append(filename, linenum, keyword,
-			    &options->channel_timeouts,
-			    &options->num_channel_timeouts, arg);
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->channel_timeouts = strs;
+			options->num_channel_timeouts = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
@@ -2358,6 +2382,7 @@ parse_pubkey_algos:
 	/* success */
 	ret = 0;
  out:
+	opt_array_free2(strs, NULL, nstrs);
 	argv_free(oav, oac);
 	return ret;
 }
diff --git a/usr.bin/ssh/readconf.h b/usr.bin/ssh/readconf.h
index b18536ab9c7..9447d5d6e53 100644
--- a/usr.bin/ssh/readconf.h
+++ b/usr.bin/ssh/readconf.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.h,v 1.155 2024/01/11 01:45:36 djm Exp $ */
+/* $OpenBSD: readconf.h,v 1.156 2024/03/04 02:16:11 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -155,12 +155,12 @@ typedef struct {
 	int	proxy_use_fdpass;
 
 	int	num_canonical_domains;
-	char	*canonical_domains[MAX_CANON_DOMAINS];
+	char	**canonical_domains;
 	int	canonicalize_hostname;
 	int	canonicalize_max_dots;
 	int	canonicalize_fallback_local;
 	int	num_permitted_cnames;
-	struct allowed_cname permitted_cnames[MAX_CANON_DOMAINS];
+	struct allowed_cname *permitted_cnames;
 
 	char	*revoked_host_keys;
 
diff --git a/usr.bin/ssh/servconf.c b/usr.bin/ssh/servconf.c
index 7ae6cc0b06c..85fb86d9228 100644
--- a/usr.bin/ssh/servconf.c
+++ b/usr.bin/ssh/servconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.c,v 1.404 2024/02/20 04:10:03 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.405 2024/03/04 02:16:11 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -1247,12 +1247,12 @@ process_server_config_line_depth(ServerOptions *options, char *line,
     struct include_list *includes)
 {
 	char *str, ***chararrayptr, **charptr, *arg, *arg2, *p, *keyword;
-	int cmdline = 0, *intptr, value, value2, n, port, oactive, r, found;
-	int ca_only = 0;
+	int cmdline = 0, *intptr, value, value2, n, port, oactive, r;
+	int ca_only = 0, found = 0;
 	SyslogFacility *log_facility_ptr;
 	LogLevel *log_level_ptr;
 	ServerOpCodes opcode;
-	u_int i, *uintptr, uvalue, flags = 0;
+	u_int i, *uintptr, flags = 0;
 	size_t len;
 	long long val64;
 	const struct multistate *multistate_ptr;
@@ -1262,6 +1262,8 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	char **oav = NULL, **av;
 	int oac = 0, ac;
 	int ret = -1;
+	char **strs = NULL; /* string array arguments; freed implicitly */
+	u_int nstrs = 0;
 
 	/* Strip trailing whitespace. Allow \f (form feed) at EOL only */
 	if ((len = strlen(line)) == 0)
@@ -1718,7 +1720,6 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 
 	case sLogVerbose:
 		found = options->num_log_verbose == 0;
-		i = 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0') {
 				error("%s line %d: keyword %s empty argument",
@@ -1727,19 +1728,25 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 			}
 			/* Allow "none" only in first position */
 			if (strcasecmp(arg, "none") == 0) {
-				if (i > 0 || ac > 0) {
+				if (nstrs > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"none\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword);
 					goto out;
 				}
 			}
-			i++;
-			if (!found || !*activep)
-				continue;
 			opt_array_append(filename, linenum, keyword,
-			    &options->log_verbose, &options->num_log_verbose,
-			    arg);
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->log_verbose = strs;
+			options->num_log_verbose = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
@@ -1765,16 +1772,22 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 		chararrayptr = &options->allow_users;
 		uintptr = &options->num_allow_users;
  parse_allowdenyusers:
+		/* XXX appends to list; doesn't respect first-match-wins */
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0' ||
 			    match_user(NULL, NULL, NULL, arg) == -1)
 				fatal("%s line %d: invalid %s pattern: \"%s\"",
 				    filename, linenum, keyword, arg);
+			found = 1;
 			if (!*activep)
 				continue;
 			opt_array_append(filename, linenum, keyword,
 			    chararrayptr, uintptr, arg);
 		}
+		if (!found) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
 		break;
 
 	case sDenyUsers:
@@ -1785,16 +1798,22 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	case sAllowGroups:
 		chararrayptr = &options->allow_groups;
 		uintptr = &options->num_allow_groups;
+		/* XXX appends to list; doesn't respect first-match-wins */
  parse_allowdenygroups:
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0')
 				fatal("%s line %d: empty %s pattern",
 				    filename, linenum, keyword);
+			found = 1;
 			if (!*activep)
 				continue;
 			opt_array_append(filename, linenum, keyword,
 			    chararrayptr, uintptr, arg);
 		}
+		if (!found) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
 		break;
 
 	case sDenyGroups:
@@ -1978,7 +1997,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	 * AuthorizedKeysFile	/etc/ssh_keys/%u
 	 */
 	case sAuthorizedKeysFile:
-		uvalue = options->num_authkeys_files;
+		found = options->num_authkeys_files == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0') {
 				error("%s line %d: keyword %s empty argument",
@@ -1986,13 +2005,20 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 				goto out;
 			}
 			arg2 = tilde_expand_filename(arg, getuid());
-			if (*activep && uvalue == 0) {
-				opt_array_append(filename, linenum, keyword,
-				    &options->authorized_keys_files,
-				    &options->num_authkeys_files, arg2);
-			}
+			opt_array_append(filename, linenum, keyword,
+			    &strs, &nstrs, arg2);
 			free(arg2);
 		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->authorized_keys_files = strs;
+			options->num_authkeys_files = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
+		}
 		break;
 
 	case sAuthorizedPrincipalsFile:
@@ -2018,34 +2044,47 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 		goto parse_int;
 
 	case sAcceptEnv:
+		/* XXX appends to list; doesn't respect first-match-wins */
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0' || strchr(arg, '=') != NULL)
 				fatal("%s line %d: Invalid environment name.",
 				    filename, linenum);
+			found = 1;
 			if (!*activep)
 				continue;
 			opt_array_append(filename, linenum, keyword,
 			    &options->accept_env, &options->num_accept_env,
 			    arg);
 		}
+		if (!found) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
 		break;
 
 	case sSetEnv:
-		uvalue = options->num_setenv;
+		found = options->num_setenv == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (*arg == '\0' || strchr(arg, '=') == NULL)
 				fatal("%s line %d: Invalid environment.",
 				    filename, linenum);
-			if (!*activep || uvalue != 0)
-				continue;
-			if (lookup_setenv_in_list(arg, options->setenv,
-			    options->num_setenv) != NULL) {
+			if (lookup_setenv_in_list(arg, strs, nstrs) != NULL) {
 				debug2("%s line %d: ignoring duplicate env "
 				    "name \"%.64s\"", filename, linenum, arg);
 				continue;
 			}
 			opt_array_append(filename, linenum, keyword,
-			    &options->setenv, &options->num_setenv, arg);
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->setenv = strs;
+			options->num_setenv = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
@@ -2196,21 +2235,20 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 			uintptr = &options->num_permitted_opens;
 			chararrayptr = &options->permitted_opens;
 		}
-		arg = argv_next(&ac, &av);
-		if (!arg || *arg == '\0')
-			fatal("%s line %d: %s missing argument.",
-			    filename, linenum, keyword);
-		uvalue = *uintptr;	/* modified later */
-		if (strcmp(arg, "any") == 0 || strcmp(arg, "none") == 0) {
-			if (*activep && uvalue == 0) {
-				*uintptr = 1;
-				*chararrayptr = xcalloc(1,
-				    sizeof(**chararrayptr));
-				(*chararrayptr)[0] = xstrdup(arg);
+		found = *uintptr == 0;
+		while ((arg = argv_next(&ac, &av)) != NULL) {
+			if (strcmp(arg, "any") == 0 ||
+			    strcmp(arg, "none") == 0) {
+				if (nstrs != 0) {
+					fatal("%s line %d: %s must appear "
+					    "alone on a %s line.",
+					    filename, linenum, arg, keyword);
+				}
+				opt_array_append(filename, linenum, keyword,
+				    &strs, &nstrs, arg);
+				continue;
 			}
-			break;
-		}
-		for (; arg != NULL && *arg != '\0'; arg = argv_next(&ac, &av)) {
+
 			if (opcode == sPermitListen &&
 			    strchr(arg, ':') == NULL) {
 				/*
@@ -2232,12 +2270,20 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 				fatal("%s line %d: %s bad port number",
 				    filename, linenum, keyword);
 			}
-			if (*activep && uvalue == 0) {
-				opt_array_append(filename, linenum, keyword,
-				    chararrayptr, uintptr, arg2);
-			}
+			opt_array_append(filename, linenum, keyword,
+			    &strs, &nstrs, arg2);
 			free(arg2);
 		}
+		if (nstrs == 0) {
+			fatal("%s line %d: %s missing argument.",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			*chararrayptr = strs;
+			*uintptr = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
+		}
 		break;
 
 	case sForceCommand:
@@ -2362,10 +2408,9 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	case sAuthenticationMethods:
 		found = options->num_auth_methods == 0;
 		value = 0; /* seen "any" pseudo-method */
-		value2 = 0; /* successfully parsed any method */
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			if (strcmp(arg, "any") == 0) {
-				if (options->num_auth_methods > 0) {
+				if (nstrs > 0) {
 					fatal("%s line %d: \"any\" must "
 					    "appear alone in %s",
 					    filename, linenum, keyword);
@@ -2378,17 +2423,19 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 				fatal("%s line %d: invalid %s method list.",
 				    filename, linenum, keyword);
 			}
-			value2 = 1;
-			if (!found || !*activep)
-				continue;
 			opt_array_append(filename, linenum, keyword,
-			    &options->auth_methods,
-			    &options->num_auth_methods, arg);
+			    &strs, &nstrs, arg);
 		}
-		if (value2 == 0) {
+		if (nstrs == 0) {
 			fatal("%s line %d: no %s specified",
 			    filename, linenum, keyword);
 		}
+		if (found && *activep) {
+			options->auth_methods = strs;
+			options->num_auth_methods = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
+		}
 		break;
 
 	case sStreamLocalBindMask:
@@ -2444,12 +2491,11 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 		goto parse_int;
 
 	case sChannelTimeout:
-		uvalue = options->num_channel_timeouts;
-		i = 0;
+		found = options->num_channel_timeouts == 0;
 		while ((arg = argv_next(&ac, &av)) != NULL) {
 			/* Allow "none" only in first position */
 			if (strcasecmp(arg, "none") == 0) {
-				if (i > 0 || ac > 0) {
+				if (nstrs > 0 || ac > 0) {
 					error("%s line %d: keyword %s \"none\" "
 					    "argument must appear alone.",
 					    filename, linenum, keyword);
@@ -2460,11 +2506,18 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 				fatal("%s line %d: invalid channel timeout %s",
 				    filename, linenum, arg);
 			}
-			if (!*activep || uvalue != 0)
-				continue;
 			opt_array_append(filename, linenum, keyword,
-			    &options->channel_timeouts,
-			    &options->num_channel_timeouts, arg);
+			    &strs, &nstrs, arg);
+		}
+		if (nstrs == 0) {
+			fatal("%s line %d: no %s specified",
+			    filename, linenum, keyword);
+		}
+		if (found && *activep) {
+			options->channel_timeouts = strs;
+			options->num_channel_timeouts = nstrs;
+			strs = NULL; /* transferred */
+			nstrs = 0;
 		}
 		break;
 
@@ -2504,6 +2557,7 @@ process_server_config_line_depth(ServerOptions *options, char *line,
 	/* success */
 	ret = 0;
  out:
+	opt_array_free2(strs, NULL, nstrs);
 	argv_free(oav, oac);
 	return ret;
 }