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

diff --git a/auth.c b/auth.c
index b6a44021387e9b05c3fb538d3817230e07321c8d..f7c1e7f3b6e21d76a3460cbd6c3c5beacc1d9a47 100644 (file)
--- a/auth.c
+++ b/auth.c
@@ -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 "

diff --git a/match.c b/match.c
index b29a30e91f2c5d7ad264a58763850a4da722b0a1..c15dcd1ef30ff0cd0240bb5396ec14d906fcb802 100644 (file)
--- a/match.c
+++ b/match.c
@@ -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);
 

diff --git a/servconf.c b/servconf.c
index 35abec489f2a662f7331c288d722112523700d4c..a18ebb59746114e4793d8f6c7cf19b86283ba377 100644 (file)
--- a/servconf.c
+++ b/servconf.c
@@ -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++] =