diff options
author | djm@openbsd.org <djm@openbsd.org> | 2016-11-06 05:46:37 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2016-11-06 16:48:29 +1100 |
commit | 010359b32659f455fddd2bd85fd7cc4d7a3b994a (patch) | |
tree | 3e7256e7255cac73e3ab1e9e3bde697a66b60865 | |
parent | efb494e81d1317209256b38b49f4280897c61e69 (diff) |
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
-rw-r--r-- | auth.c | 22 | ||||
-rw-r--r-- | match.c | 21 | ||||
-rw-r--r-- | servconf.c | 8 |
3 files changed, 38 insertions, 13 deletions
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: auth.c,v 1.116 2016/08/13 17:47:41 markus Exp $ */ | 1 | /* $OpenBSD: auth.c,v 1.117 2016/11/06 05:46:37 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. | 3 | * Copyright (c) 2000 Markus Friedl. All rights reserved. |
4 | * | 4 | * |
@@ -103,6 +103,7 @@ allowed_user(struct passwd * pw) | |||
103 | struct stat st; | 103 | struct stat st; |
104 | const char *hostname = NULL, *ipaddr = NULL, *passwd = NULL; | 104 | const char *hostname = NULL, *ipaddr = NULL, *passwd = NULL; |
105 | u_int i; | 105 | u_int i; |
106 | int r; | ||
106 | #ifdef USE_SHADOW | 107 | #ifdef USE_SHADOW |
107 | struct spwd *spw = NULL; | 108 | struct spwd *spw = NULL; |
108 | #endif | 109 | #endif |
@@ -192,8 +193,12 @@ allowed_user(struct passwd * pw) | |||
192 | /* Return false if user is listed in DenyUsers */ | 193 | /* Return false if user is listed in DenyUsers */ |
193 | if (options.num_deny_users > 0) { | 194 | if (options.num_deny_users > 0) { |
194 | for (i = 0; i < options.num_deny_users; i++) | 195 | for (i = 0; i < options.num_deny_users; i++) |
195 | if (match_user(pw->pw_name, hostname, ipaddr, | 196 | r = match_user(pw->pw_name, hostname, ipaddr, |
196 | options.deny_users[i])) { | 197 | options.deny_users[i]); |
198 | if (r < 0) { | ||
199 | fatal("Invalid DenyUsers pattern \"%.100s\"", | ||
200 | options.deny_users[i]); | ||
201 | } else if (r != 1) { | ||
197 | logit("User %.100s from %.100s not allowed " | 202 | logit("User %.100s from %.100s not allowed " |
198 | "because listed in DenyUsers", | 203 | "because listed in DenyUsers", |
199 | pw->pw_name, hostname); | 204 | pw->pw_name, hostname); |
@@ -202,10 +207,15 @@ allowed_user(struct passwd * pw) | |||
202 | } | 207 | } |
203 | /* Return false if AllowUsers isn't empty and user isn't listed there */ | 208 | /* Return false if AllowUsers isn't empty and user isn't listed there */ |
204 | if (options.num_allow_users > 0) { | 209 | if (options.num_allow_users > 0) { |
205 | for (i = 0; i < options.num_allow_users; i++) | 210 | for (i = 0; i < options.num_allow_users; i++) { |
206 | if (match_user(pw->pw_name, hostname, ipaddr, | 211 | r = match_user(pw->pw_name, hostname, ipaddr, |
207 | options.allow_users[i])) | 212 | options.allow_users[i]); |
213 | if (r < 0) { | ||
214 | fatal("Invalid AllowUsers pattern \"%.100s\"", | ||
215 | options.allow_users[i]); | ||
216 | } else if (r == 1) | ||
208 | break; | 217 | break; |
218 | } | ||
209 | /* i < options.num_allow_users iff we break for loop */ | 219 | /* i < options.num_allow_users iff we break for loop */ |
210 | if (i >= options.num_allow_users) { | 220 | if (i >= options.num_allow_users) { |
211 | logit("User %.100s from %.100s not allowed because " | 221 | logit("User %.100s from %.100s not allowed because " |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: match.c,v 1.32 2016/09/21 16:55:42 djm Exp $ */ | 1 | /* $OpenBSD: match.c,v 1.33 2016/11/06 05:46:37 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> | 3 | * Author: Tatu Ylonen <ylo@cs.hut.fi> |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * 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, | |||
191 | { | 191 | { |
192 | int mhost, mip; | 192 | int mhost, mip; |
193 | 193 | ||
194 | /* error in ipaddr match */ | ||
195 | if ((mip = addr_match_list(ipaddr, patterns)) == -2) | 194 | if ((mip = addr_match_list(ipaddr, patterns)) == -2) |
196 | return -1; | 195 | return -1; /* error in ipaddr match */ |
197 | else if (mip == -1) /* negative ip address match */ | 196 | else if (host == NULL || ipaddr == NULL || mip == -1) |
198 | return 0; | 197 | return 0; /* negative ip address match, or testing pattern */ |
199 | 198 | ||
200 | /* negative hostname match */ | 199 | /* negative hostname match */ |
201 | if ((mhost = match_hostname(host, patterns)) == -1) | 200 | if ((mhost = match_hostname(host, patterns)) == -1) |
@@ -207,7 +206,9 @@ match_host_and_ip(const char *host, const char *ipaddr, | |||
207 | } | 206 | } |
208 | 207 | ||
209 | /* | 208 | /* |
210 | * match user, user@host_or_ip, user@host_or_ip_list against pattern | 209 | * Match user, user@host_or_ip, user@host_or_ip_list against pattern. |
210 | * If user, host and ipaddr are all NULL then validate pattern/ | ||
211 | * Returns -1 on invalid pattern, 0 on no match, 1 on match. | ||
211 | */ | 212 | */ |
212 | int | 213 | int |
213 | match_user(const char *user, const char *host, const char *ipaddr, | 214 | 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, | |||
216 | char *p, *pat; | 217 | char *p, *pat; |
217 | int ret; | 218 | int ret; |
218 | 219 | ||
220 | /* test mode */ | ||
221 | if (user == NULL && host == NULL && ipaddr == NULL) { | ||
222 | if ((p = strchr(pattern, '@')) != NULL && | ||
223 | match_host_and_ip(NULL, NULL, p + 1) < 0) | ||
224 | return -1; | ||
225 | return 0; | ||
226 | } | ||
227 | |||
219 | if ((p = strchr(pattern,'@')) == NULL) | 228 | if ((p = strchr(pattern,'@')) == NULL) |
220 | return match_pattern(user, pattern); | 229 | return match_pattern(user, pattern); |
221 | 230 | ||
diff --git a/servconf.c b/servconf.c index 35abec489..a18ebb597 100644 --- a/servconf.c +++ b/servconf.c | |||
@@ -1,5 +1,5 @@ | |||
1 | 1 | ||
2 | /* $OpenBSD: servconf.c,v 1.298 2016/10/24 01:09:17 dtucker Exp $ */ | 2 | /* $OpenBSD: servconf.c,v 1.299 2016/11/06 05:46:37 djm Exp $ */ |
3 | /* | 3 | /* |
4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland | 4 | * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland |
5 | * All rights reserved | 5 | * All rights reserved |
@@ -1366,6 +1366,9 @@ process_server_config_line(ServerOptions *options, char *line, | |||
1366 | if (options->num_allow_users >= MAX_ALLOW_USERS) | 1366 | if (options->num_allow_users >= MAX_ALLOW_USERS) |
1367 | fatal("%s line %d: too many allow users.", | 1367 | fatal("%s line %d: too many allow users.", |
1368 | filename, linenum); | 1368 | filename, linenum); |
1369 | if (match_user(NULL, NULL, NULL, arg) == -1) | ||
1370 | fatal("%s line %d: invalid AllowUsers pattern: " | ||
1371 | "\"%.100s\"", filename, linenum, arg); | ||
1369 | if (!*activep) | 1372 | if (!*activep) |
1370 | continue; | 1373 | continue; |
1371 | options->allow_users[options->num_allow_users++] = | 1374 | options->allow_users[options->num_allow_users++] = |
@@ -1378,6 +1381,9 @@ process_server_config_line(ServerOptions *options, char *line, | |||
1378 | if (options->num_deny_users >= MAX_DENY_USERS) | 1381 | if (options->num_deny_users >= MAX_DENY_USERS) |
1379 | fatal("%s line %d: too many deny users.", | 1382 | fatal("%s line %d: too many deny users.", |
1380 | filename, linenum); | 1383 | filename, linenum); |
1384 | if (match_user(NULL, NULL, NULL, arg) == -1) | ||
1385 | fatal("%s line %d: invalid DenyUsers pattern: " | ||
1386 | "\"%.100s\"", filename, linenum, arg); | ||
1381 | if (!*activep) | 1387 | if (!*activep) |
1382 | continue; | 1388 | continue; |
1383 | options->deny_users[options->num_deny_users++] = | 1389 | options->deny_users[options->num_deny_users++] = |