RepoMirrors
/
openssh
mirror of git://anongit.mindrot.org/openssh.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

upstream: 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@

OpenBSD-Commit-ID: 7f196cba634c2a3dba115f3fac3c4635a2199491
This commit is contained in:
djm@openbsd.org 2024-03-04 02:16:11 +00:00 committed by Damien Miller
parent 6886e1b1f5
commit 65a44a8a4f
No known key found for this signature in database
5 changed files with 215 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
misc.c
View File

@ -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.
@ -2644,6 +2644,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)
{

3
misc.h
View File

@ -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>
@ -210,6 +210,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);

147
readconf.c
View File

@ -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
@ -1032,21 +1032,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;
@ -1662,14 +1665,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);
@ -1695,17 +1697,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:
@ -1823,12 +1828,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 == '-') {
@ -1840,27 +1847,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;
@ -2069,52 +2087,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);
@ -2135,19 +2147,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;
@ -2329,12 +2347,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);
@ -2345,11 +2362,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;
@ -2381,6 +2405,7 @@ parse_pubkey_algos:
/* success */
ret = 0;
out:
opt_array_free2(strs, NULL, nstrs);
argv_free(oav, oac);
return ret;
}

6
readconf.h
View File

@ -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;

166
servconf.c
View File

@ -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
@ -1298,12 +1298,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;
@ -1313,6 +1313,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)
@ -1775,7 +1777,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",
@ -1784,19 +1785,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;
@ -1822,16 +1829,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:
@ -1842,16 +1855,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:
@ -2035,7 +2054,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",
@ -2043,13 +2062,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:
@ -2075,34 +2101,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;
@ -2253,21 +2292,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) {
/*
@ -2289,12 +2327,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:
@ -2419,10 +2465,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);
@ -2435,17 +2480,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:
@ -2505,12 +2552,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);
@ -2521,11 +2567,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;
@ -2565,6 +2618,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;
}