upstream commit

Validate address ranges for AllowUser/DenyUsers at
configuration load time and refuse to accept bad ones. It was previously
possible to specify invalid CIDR address ranges (e.g. djm@127.1.2.3/55) and
these would always match.

Thanks to Laurence Parry for a detailed bug report. ok markus (for
a previous diff version)

Upstream-ID: 9dfcdd9672b06e65233ea4434c38226680d40bfb
This commit is contained in:
djm@openbsd.org 2016-11-06 05:46:37 +00:00 committed by Damien Miller
parent efb494e81d
commit 010359b326
3 changed files with 38 additions and 13 deletions

22
auth.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: auth.c,v 1.116 2016/08/13 17:47:41 markus Exp $ */
/* $OpenBSD: auth.c,v 1.117 2016/11/06 05:46:37 djm Exp $ */
/*
* Copyright (c) 2000 Markus Friedl. All rights reserved.
*
@ -103,6 +103,7 @@ allowed_user(struct passwd * pw)
struct stat st;
const char *hostname = NULL, *ipaddr = NULL, *passwd = NULL;
u_int i;
int r;
#ifdef USE_SHADOW
struct spwd *spw = NULL;
#endif
@ -192,8 +193,12 @@ allowed_user(struct passwd * pw)
/* Return false if user is listed in DenyUsers */
if (options.num_deny_users > 0) {
for (i = 0; i < options.num_deny_users; i++)
if (match_user(pw->pw_name, hostname, ipaddr,
options.deny_users[i])) {
r = match_user(pw->pw_name, hostname, ipaddr,
options.deny_users[i]);
if (r < 0) {
fatal("Invalid DenyUsers pattern \"%.100s\"",
options.deny_users[i]);
} else if (r != 1) {
logit("User %.100s from %.100s not allowed "
"because listed in DenyUsers",
pw->pw_name, hostname);
@ -202,10 +207,15 @@ allowed_user(struct passwd * pw)
}
/* Return false if AllowUsers isn't empty and user isn't listed there */
if (options.num_allow_users > 0) {
for (i = 0; i < options.num_allow_users; i++)
if (match_user(pw->pw_name, hostname, ipaddr,
options.allow_users[i]))
for (i = 0; i < options.num_allow_users; i++) {
r = match_user(pw->pw_name, hostname, ipaddr,
options.allow_users[i]);
if (r < 0) {
fatal("Invalid AllowUsers pattern \"%.100s\"",
options.allow_users[i]);
} else if (r == 1)
break;
}
/* i < options.num_allow_users iff we break for loop */
if (i >= options.num_allow_users) {
logit("User %.100s from %.100s not allowed because "

21
match.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: match.c,v 1.32 2016/09/21 16:55:42 djm Exp $ */
/* $OpenBSD: match.c,v 1.33 2016/11/06 05:46:37 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -191,11 +191,10 @@ match_host_and_ip(const char *host, const char *ipaddr,
{
int mhost, mip;
/* error in ipaddr match */
if ((mip = addr_match_list(ipaddr, patterns)) == -2)
return -1;
else if (mip == -1) /* negative ip address match */
return 0;
return -1; /* error in ipaddr match */
else if (host == NULL || ipaddr == NULL || mip == -1)
return 0; /* negative ip address match, or testing pattern */
/* negative hostname match */
if ((mhost = match_hostname(host, patterns)) == -1)
@ -207,7 +206,9 @@ match_host_and_ip(const char *host, const char *ipaddr,
}
/*
* match user, user@host_or_ip, user@host_or_ip_list against pattern
* Match user, user@host_or_ip, user@host_or_ip_list against pattern.
* If user, host and ipaddr are all NULL then validate pattern/
* Returns -1 on invalid pattern, 0 on no match, 1 on match.
*/
int
match_user(const char *user, const char *host, const char *ipaddr,
@ -216,6 +217,14 @@ match_user(const char *user, const char *host, const char *ipaddr,
char *p, *pat;
int ret;
/* test mode */
if (user == NULL && host == NULL && ipaddr == NULL) {
if ((p = strchr(pattern, '@')) != NULL &&
match_host_and_ip(NULL, NULL, p + 1) < 0)
return -1;
return 0;
}
if ((p = strchr(pattern,'@')) == NULL)
return match_pattern(user, pattern);

View File

@ -1,5 +1,5 @@
/* $OpenBSD: servconf.c,v 1.298 2016/10/24 01:09:17 dtucker Exp $ */
/* $OpenBSD: servconf.c,v 1.299 2016/11/06 05:46:37 djm Exp $ */
/*
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
* All rights reserved
@ -1366,6 +1366,9 @@ process_server_config_line(ServerOptions *options, char *line,
if (options->num_allow_users >= MAX_ALLOW_USERS)
fatal("%s line %d: too many allow users.",
filename, linenum);
if (match_user(NULL, NULL, NULL, arg) == -1)
fatal("%s line %d: invalid AllowUsers pattern: "
"\"%.100s\"", filename, linenum, arg);
if (!*activep)
continue;
options->allow_users[options->num_allow_users++] =
@ -1378,6 +1381,9 @@ process_server_config_line(ServerOptions *options, char *line,
if (options->num_deny_users >= MAX_DENY_USERS)
fatal("%s line %d: too many deny users.",
filename, linenum);
if (match_user(NULL, NULL, NULL, arg) == -1)
fatal("%s line %d: invalid DenyUsers pattern: "
"\"%.100s\"", filename, linenum, arg);
if (!*activep)
continue;
options->deny_users[options->num_deny_users++] =