Patrick Gosling
2005-Jan-20 10:27 UTC
AllowUsers - proposal for useful variations on the theme
A short while ago, I looked at using the AllowUsers configuration option in openssh (v3.8p1 , but I believe this to be unchanged in 3.9p1) to restrict access such that only specific remote machines could access specific local accounts. I swiftly discovered that a) specifying wildcarded IP numbers to try to allow a useful IP range was pointless: if I specified AllowUsers joe at 192.0.34.* then an attacker who controlled their own DNS could trivially arrange to connect from 192.0.34.example.com (the openssh code only checks (and complains) if the reverse DNS lookup returns a purely numeric "hostname"). b) specifying hostnames was less useful than I might have hoped, since the code took only the canonical name obtained by DNS lookup of the connecting host's IP address and matched that against the (potentially wildcarded) host components of the AllowUsers entries. The problem here is that it will commonly be the case that the hostname that a user would like to specify that they'll connect from might be one provided by a "dynamic dns" provider, but the hostname that sshd will regard as theirs will be some random (regularly changing) thing generated by their ISP. I'll note here that I don't believe that AllowUsers (even if designed right) would provide robust security, but that it could be a useful component of a more complex security policy. So I'm aware that depending on the security and robustness of a service like dyndns.org as part of a local security policy is imperfect; but I also think that it provides non-trivial benefit in practice. The conclusion of this was that, having decided that modifying the specification/behaviour of the AllowUsers option to fix these observed difficulties was not acceptable (since it might break existing configurations), I've written a small patch to 3.8p1 that adds two new configuration options: AllowUsersFixedname which accepts the same arguments as AllowUsers, but which does not treat the HOST component of a USER at HOST entry as wildcarded. Instead it takes the IP number of the connecting host, and tests if it matches any of the IP numbers returned by a DNS lookup on the HOST component of the USER at HOST entry. AllowUsersIpaddr which accepts the same arguments as AllowUsers, allows wildcards, but which only does the IP matching test, not the hostname matching test. I would very much welcome feedback on the viability of this; it's clearly to my benefit to try to convince people that this would be a useful feature to incorporate in openssh, since I've had unpleasant experience in the past of having to reverse engineer local bodges into new releases at high speed after security issues have been publicised 8-) . Relevant diff against 3.9p1 appended below (although it should be noted that I've only tested this in anger against 3.8p1 ; however the changelog suggests that this is not an area where there's substantial change between 3.8 and 3.9 ...) Patrick Gosling, Department of Engineering, University of Cambridge. --------------------------------------------------------------------------- diff -r -U 8 openssh-3.9p1.orig/auth.c openssh-3.9p1.jpmg/auth.c --- openssh-3.9p1.orig/auth.c 2004-08-12 13:40:25.000000000 +0100 +++ openssh-3.9p1.jpmg/auth.c 2005-01-20 10:11:24.689070494 +0000 @@ -69,17 +69,17 @@ * Otherwise true is returned. */ int allowed_user(struct passwd * pw) { struct stat st; const char *hostname = NULL, *ipaddr = NULL, *passwd = NULL; char *shell; - int i; + int i, allowed; #ifdef USE_SHADOW struct spwd *spw = NULL; #endif /* Shouldn't be called if pw is NULL, but better safe than sorry... */ if (!pw || !pw->pw_name) return 0; @@ -138,44 +138,85 @@ } if (S_ISREG(st.st_mode) == 0 || (st.st_mode & (S_IXOTH|S_IXUSR|S_IXGRP)) == 0) { logit("User %.100s not allowed because shell %.100s is not executable", pw->pw_name, shell); return 0; } - if (options.num_deny_users > 0 || options.num_allow_users > 0) { + if (options.num_deny_users > 0 || options.num_allow_users > 0 || + options.num_allow_users_fixedname > 0 || + options.num_allow_users_ipaddr > 0 ) { hostname = get_canonical_hostname(options.use_dns); ipaddr = get_remote_ipaddr(); } /* 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])) { logit("User %.100s not allowed because listed in DenyUsers", pw->pw_name); return 0; } } - /* Return false if AllowUsers isn't empty and user isn't listed there */ + /* Check all of AllowUsers, AllowUsersIpaddr and AllowUsersFixedname + If none of them set, allow. + If any set and user at host matches against any, allow. + Else return 0; */ + + if ((options.num_allow_users > 0) || + (options.num_allow_users_fixedname > 0) || + (options.num_allow_users_ipaddr > 0)) { + allowed = 0; + } else { + allowed = 1; + } + 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])) break; /* i < options.num_allow_users iff we break for loop */ - if (i >= options.num_allow_users) { - logit("User %.100s not allowed because not listed in AllowUsers", - pw->pw_name); - return 0; + if (i < options.num_allow_users) { + allowed = 1; + } + } + + if (options.num_allow_users_fixedname > 0) { + for (i = 0; i < options.num_allow_users_fixedname; i++) + if (match_user_fixedname(pw->pw_name, hostname, + options.allow_users[i])) + break; + /* i < options.num_allow_users_fixedname iff we break for loop */ + if (i < options.num_allow_users_fixedname) { + allowed = 1; } } + + if (options.num_allow_users_ipaddr > 0) { + for (i = 0; i < options.num_allow_users_ipaddr; i++) + if (match_user(pw->pw_name, NULL, ipaddr, + options.allow_users_ipaddr[i])) + break; + /* i < options.num_allow_users_ipaddr iff we break for loop */ + if (i < options.num_allow_users_ipaddr) { + allowed = 1; + } + } + + if (allowed == 0) { + logit("User %.100s not allowed because not listed in AllowUsers, AllowUsersFixedname or AllowUsersIpaddr", + pw->pw_name); + return 0; + } + if (options.num_deny_groups > 0 || options.num_allow_groups > 0) { /* Get the user's group access list (primary and supplementary) */ if (ga_init(pw->pw_name, pw->pw_gid) == 0) { logit("User %.100s not allowed because not in any group", pw->pw_name); return 0; } diff -r -U 8 openssh-3.9p1.orig/match.c openssh-3.9p1.jpmg/match.c --- openssh-3.9p1.orig/match.c 2002-03-05 01:42:43.000000000 +0000 +++ openssh-3.9p1.jpmg/match.c 2005-01-20 10:11:24.690070417 +0000 @@ -43,16 +43,19 @@ /* * Returns true if the given string matches the pattern (which may contain ? * and * as wildcards), and zero if it does not match. */ int match_pattern(const char *s, const char *pattern) { + if (s == NULL) + return 0; + for (;;) { /* If at end of pattern, accept if also at end of string. */ if (!*pattern) return !*s; if (*pattern == '*') { /* Skip the asterisk. */ pattern++; @@ -217,16 +220,58 @@ if ((ret = match_pattern(user, pat)) == 1) ret = match_host_and_ip(host, ipaddr, p); xfree(pat); return ret; } +int +match_user_fixedname(const char *user, const char *ipaddr, + const char *pattern) +{ + char *p, *pat; + int ret; + struct addrinfo hints, *ai, *aitop, ntop[NI_MAXHOST]; + + if ((p = strchr(pattern, '@')) == NULL) + return match_pattern(user, pattern); + + pat = xstrdup(pattern); + p = strchr(pat, '@'); + *p++ = '\0'; + + if ((ret = match_pattern(user, pat)) == 1) { + memset(&hints, 0, sizeof(hints)); + hints.ai_socktype = SOCK_DGRAM; /* dummy */ + if (getaddrinfo(p, NULL, &hints, &aitop) != 0) { + logit("checking getaddrinfo for %.700s failed - " + "check AllowUsersFixedname entry!", p); + ret = 0; + } else { + ret = 0; + for (ai = aitop; ai; ai = ai->ai_next) { + if (getnameinfo(ai->ai_addr, ai->ai_addrlen, + ntop, sizeof(ntop), NULL, 0, + NI_NUMERICHOST) == 0 && + (strcmp(ipaddr, ntop) == 0)) { + ret = 1; + break; + } + } + } + freeaddrinfo(aitop); + } + + xfree(pat); + + return ret; +} + /* * Returns first item from client-list that is also supported by server-list, * caller must xfree() returned string. */ #define MAX_PROP 40 #define SEP "," char * match_list(const char *client, const char *server, u_int *next) diff -r -U 8 openssh-3.9p1.orig/match.h openssh-3.9p1.jpmg/match.h --- openssh-3.9p1.orig/match.h 2002-03-05 01:42:43.000000000 +0000 +++ openssh-3.9p1.jpmg/match.h 2005-01-20 10:11:24.691070340 +0000 @@ -14,11 +14,12 @@ #ifndef MATCH_H #define MATCH_H int match_pattern(const char *, const char *); int match_pattern_list(const char *, const char *, u_int, int); int match_hostname(const char *, const char *, u_int); int match_host_and_ip(const char *, const char *, const char *); int match_user(const char *, const char *, const char *, const char *); +int match_user_fixedname(const char *, const char *, const char *); char *match_list(const char *, const char *, u_int *); #endif diff -r -U 8 openssh-3.9p1.orig/servconf.c openssh-3.9p1.jpmg/servconf.c --- openssh-3.9p1.orig/servconf.c 2004-08-13 12:30:24.000000000 +0100 +++ openssh-3.9p1.jpmg/servconf.c 2005-01-20 10:11:54.385791373 +0000 @@ -78,16 +78,17 @@ options->kbd_interactive_authentication = -1; options->challenge_response_authentication = -1; options->permit_empty_passwd = -1; options->permit_user_env = -1; options->use_login = -1; options->compression = -1; options->allow_tcp_forwarding = -1; options->num_allow_users = 0; + options->num_allow_users_fixedname = 0; options->num_deny_users = 0; options->num_allow_groups = 0; options->num_deny_groups = 0; options->ciphers = NULL; options->macs = NULL; options->protocol = SSH_PROTO_UNKNOWN; options->gateway_ports = -1; options->num_subsystems = 0; @@ -258,17 +259,17 @@ sKerberosAuthentication, sKerberosOrLocalPasswd, sKerberosTicketCleanup, sKerberosGetAFSToken, sKerberosTgtPassing, sChallengeResponseAuthentication, sPasswordAuthentication, sKbdInteractiveAuthentication, sListenAddress, sPrintMotd, sPrintLastLog, sIgnoreRhosts, sX11Forwarding, sX11DisplayOffset, sX11UseLocalhost, sStrictModes, sEmptyPasswd, sTCPKeepAlive, sPermitUserEnvironment, sUseLogin, sAllowTcpForwarding, sCompression, - sAllowUsers, sDenyUsers, sAllowGroups, sDenyGroups, + sAllowUsers, sAllowUsersFixedname, sAllowUsersIpaddr, sDenyUsers, sAllowGroups, sDenyGroups, sIgnoreUserKnownHosts, sCiphers, sMacs, sProtocol, sPidFile, sGatewayPorts, sPubkeyAuthentication, sXAuthLocation, sSubsystem, sMaxStartups, sMaxAuthTries, sBanner, sUseDNS, sHostbasedAuthentication, sHostbasedUsesNameFromPacketOnly, sClientAliveInterval, sClientAliveCountMax, sAuthorizedKeysFile, sAuthorizedKeysFile2, sGssAuthentication, sGssCleanupCreds, sAcceptEnv, sUsePrivilegeSeparation, @@ -347,16 +348,18 @@ { "permitemptypasswords", sEmptyPasswd }, { "permituserenvironment", sPermitUserEnvironment }, { "uselogin", sUseLogin }, { "compression", sCompression }, { "tcpkeepalive", sTCPKeepAlive }, { "keepalive", sTCPKeepAlive }, /* obsolete alias */ { "allowtcpforwarding", sAllowTcpForwarding }, { "allowusers", sAllowUsers }, + { "allowusersfixedname", sAllowUsersFixedname }, + { "allowusersipaddr", sAllowUsersIpaddr }, { "denyusers", sDenyUsers }, { "allowgroups", sAllowGroups }, { "denygroups", sDenyGroups }, { "ciphers", sCiphers }, { "macs", sMacs }, { "protocol", sProtocol }, { "gatewayports", sGatewayPorts }, { "subsystem", sSubsystem }, @@ -761,16 +764,36 @@ if (options->num_allow_users >= MAX_ALLOW_USERS) fatal("%s line %d: too many allow users.", filename, linenum); options->allow_users[options->num_allow_users++] xstrdup(arg); } break; + case sAllowUsersFixedname: + while ((arg = strdelim(&cp)) && *arg != '\0') { + if (options->num_allow_users_fixedname >= MAX_ALLOW_USERS) + fatal("%s line %d: too many allow users (fixedname).", + filename, linenum); + options->allow_users_fixedname[options->num_allow_users_fixedname++] + xstrdup(arg); + } + break; + + case sAllowUsersIpaddr: + while ((arg = strdelim(&cp)) && *arg != '\0') { + if (options->num_allow_users_ipaddr >= MAX_ALLOW_USERS) + fatal("%s line %d: too many allow users.", + filename, linenum); + options->allow_users_ipaddr[options->num_allow_users_ipaddr++] + xstrdup(arg); + } + break; + case sDenyUsers: while ((arg = strdelim(&cp)) && *arg != '\0') { if (options->num_deny_users >= MAX_DENY_USERS) fatal( "%s line %d: too many deny users.", filename, linenum); options->deny_users[options->num_deny_users++] xstrdup(arg); } diff -r -U 8 openssh-3.9p1.orig/servconf.h openssh-3.9p1.jpmg/servconf.h --- openssh-3.9p1.orig/servconf.h 2004-06-25 04:33:20.000000000 +0100 +++ openssh-3.9p1.jpmg/servconf.h 2005-01-20 10:11:24.696069956 +0000 @@ -95,16 +95,20 @@ int permit_empty_passwd; /* If false, do not permit empty * passwords. */ int permit_user_env; /* If true, read ~/.ssh/environment */ int use_login; /* If true, login(1) is used */ int compression; /* If true, compression is allowed */ int allow_tcp_forwarding; u_int num_allow_users; char *allow_users[MAX_ALLOW_USERS]; + u_int num_allow_users_fixedname; + char *allow_users_fixedname[MAX_ALLOW_USERS]; + u_int num_allow_users_ipaddr; + char *allow_users_ipaddr[MAX_ALLOW_USERS]; u_int num_deny_users; char *deny_users[MAX_DENY_USERS]; u_int num_allow_groups; char *allow_groups[MAX_ALLOW_GROUPS]; u_int num_deny_groups; char *deny_groups[MAX_DENY_GROUPS]; u_int num_subsystems; diff -r -U 8 openssh-3.9p1.orig/sshd.8 openssh-3.9p1.jpmg/sshd.8 --- openssh-3.9p1.orig/sshd.8 2004-05-02 13:15:08.000000000 +0100 +++ openssh-3.9p1.jpmg/sshd.8 2005-01-20 10:11:24.697069880 +0000 @@ -330,16 +330,18 @@ .Cm RhostsRSAAuthentication , .Cm HostbasedAuthentication and using a .Cm from="pattern-list" option in a key file. Configuration options that require DNS include using a USER at HOST pattern in .Cm AllowUsers +, +.Cm AllowUsersFixedname or .Cm DenyUsers . .El .Sh CONFIGURATION FILE .Nm reads configuration data from .Pa /etc/ssh/sshd_config (or the file specified with diff -r -U 8 openssh-3.9p1.orig/sshd_config.5 openssh-3.9p1.jpmg/sshd_config.5 --- openssh-3.9p1.orig/sshd_config.5 2004-06-30 13:39:34.000000000 +0100 +++ openssh-3.9p1.jpmg/sshd_config.5 2005-01-20 10:13:10.850922736 +0000 @@ -112,16 +112,48 @@ .Ql \&? can be used as wildcards in the patterns. Only user names are valid; a numerical user ID is not recognized. By default, login is allowed for all users. If the pattern takes the form USER at HOST then USER and HOST are separately checked, restricting logins to particular users from particular hosts. +.Pp +The HOST component is tested for a match against +the IP number and the hostname of the connecting host. It is +critically important to note that many wildcarded representations of +IP numbers can be trivially matched by an attacker with control of +their DNS (eg if the allowed HOST was specified as +.Ql 192.0.34.* +then an attacker controlling the zone example.com would merely need to +create an entry in the DNS for a host called +.Ql 192.0.34.example.com +to get past this restriction). The AllowUsersIpaddr option avoids this +problem. +.Pp +A further restriction on this option is that only the canonical name +of the connecting host is tested for a match against the HOST +component; if the allowed hostname is an cname alias of the connecting +host, connection will not be successful. The AllowUsersFixedname +option avoids this problem. +.Pp +.It Cm AllowUsersFixedname +This keyword can be followed by a list of user name patterns as for +AllowUsers. The behaviour is the same, excepting that the HOST component +of a pattern may not be wildcarded, and that the check made is whether +the IP address from which the connection is being made is one of the +DNS registered IP addresses of the specified HOST. +.Pp +.It Cm AllowUsersIpaddr +This keyword can be followed by a list of user name patterns as for +AllowUsers. The behaviour is the same, excepting that the HOST component +of a pattern, which may be wildcarded, is only tested for a match against +the IP address from which the connection is being made. +.Pp .It Cm AuthorizedKeysFile Specifies the file that contains the public keys that can be used for user authentication. .Cm AuthorizedKeysFile may contain tokens of the form %T which are substituted during connection set-up. The following tokens are defined: %% is replaced by a literal '%', %h is replaced by the home directory of the user being authenticated and ---------------------------------------------------------------------------