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
---------------------------------------------------------------------------
Maybe Matching Threads
Wisdom of the Ancients
