Markus, ignore the other stuff I sent.. I need to go back to bed and stop
trying to code.. <sigh>
For everone else.. Will this make everyone happy?
This does the follow.
it will always honor AllowUsers.
If there is no Allow/DenyGroups it stated they are not in allowUsers. IF
there are AllowDenyGroups it tries them. And then stated they are not in
either AllowUsers nor AllowGroups
since PErmitRootLogin is not handled in auth.c:allowed_users() I will not
try to add that logic. I still believe it should be true.
Diff against -ccurent BSD tree.
- Ben
Index: auth.c
==================================================================RCS file:
/cvs/src/usr.bin/ssh/auth.c,v
retrieving revision 1.46
diff -u -r1.46 auth.c
--- auth.c 4 Nov 2002 10:07:53 -0000 1.46
+++ auth.c 16 Feb 2003 03:27:42 -0000
@@ -105,15 +105,16 @@
return 0;
}
}
- /* Return false if AllowUsers isn't empty and user isn't listed there
*/
+ /* Return true if person in AllowUsers, otherwise try the group test */
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) {
- log("User %.100s not allowed because not listed in AllowUsers",
+ goto success;
+
+ if (options.num_deny_groups == 0 &&
+ options.num_allow_groups == 0) {
+ log("User %.100s not allowed because not in AllowUsers",
pw->pw_name);
return 0;
}
@@ -136,20 +137,28 @@
return 0;
}
/*
- * Return false if AllowGroups isn't empty and one of user's groups
- * isn't listed there
+ * Return false if AllowGroups isn't empty and one of
+ * user's groups isn't listed there
*/
if (options.num_allow_groups > 0)
if (!ga_match(options.allow_groups,
options.num_allow_groups)) {
ga_free();
- log("User %.100s not allowed because none of user's groups are
listed in AllowGroups",
+ if (options.num_deny_users > 0 ||
+ options.num_allow_users > 0) {
+ log("User %.100s not allowed because not in AllowUsers nor user's
groups in AllowGroups",
+ pw->pw_name);
+ return 0;
+ } else {
+ log("User %.100s not allowed because none of user's groups are
listed in AllowGroups",
pw->pw_name);
- return 0;
+ return 0;
+ }
}
ga_free();
}
/* We found no reason not to let this user try to log on... */
+success:
return 1;
}
No comments either means no one cares or no one has tested. Let me know quickly or I won't push for including it into 3.6. We are getting close to a tree lock people. - Ben On Sat, 15 Feb 2003, Ben Lindstrom wrote:> > Markus, ignore the other stuff I sent.. I need to go back to bed and stop > trying to code.. <sigh> > > For everone else.. Will this make everyone happy? > > This does the follow. > > it will always honor AllowUsers. > > If there is no Allow/DenyGroups it stated they are not in allowUsers. IF > there are AllowDenyGroups it tries them. And then stated they are not in > either AllowUsers nor AllowGroups > > since PErmitRootLogin is not handled in auth.c:allowed_users() I will not > try to add that logic. I still believe it should be true. > > Diff against -ccurent BSD tree. > > - Ben > > Index: auth.c > ==================================================================> RCS file: /cvs/src/usr.bin/ssh/auth.c,v > retrieving revision 1.46 > diff -u -r1.46 auth.c > --- auth.c 4 Nov 2002 10:07:53 -0000 1.46 > +++ auth.c 16 Feb 2003 03:27:42 -0000 > @@ -105,15 +105,16 @@ > return 0; > } > } > - /* Return false if AllowUsers isn't empty and user isn't listed there */ > + /* Return true if person in AllowUsers, otherwise try the group test */ > 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) { > - log("User %.100s not allowed because not listed in AllowUsers", > + goto success; > + > + if (options.num_deny_groups == 0 && > + options.num_allow_groups == 0) { > + log("User %.100s not allowed because not in AllowUsers", > pw->pw_name); > return 0; > } > @@ -136,20 +137,28 @@ > return 0; > } > /* > - * Return false if AllowGroups isn't empty and one of user's groups > - * isn't listed there > + * Return false if AllowGroups isn't empty and one of > + * user's groups isn't listed there > */ > if (options.num_allow_groups > 0) > if (!ga_match(options.allow_groups, > options.num_allow_groups)) { > ga_free(); > - log("User %.100s not allowed because none of user's groups are listed in AllowGroups", > + if (options.num_deny_users > 0 || > + options.num_allow_users > 0) { > + log("User %.100s not allowed because not in AllowUsers nor user's groups in AllowGroups", > + pw->pw_name); > + return 0; > + } else { > + log("User %.100s not allowed because none of user's groups are listed in AllowGroups", > pw->pw_name); > - return 0; > + return 0; > + } > } > ga_free(); > } > /* We found no reason not to let this user try to log on... */ > +success: > return 1; > } > > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > http://www.mindrot.org/mailman/listinfo/openssh-unix-dev >
Just in case anyone wants Ben's patch for OpenSSH's 3.5 release instead
of current, here it is.
diff -uNr openssh-3.5p1/auth.c openssh-3.5p1-allowfix/auth.c
--- openssh-3.5p1/auth.c Sat Sep 21 11:26:53 2002
+++ openssh-3.5p1-allowfix/auth.c Wed Feb 26 14:28:00 2003
@@ -156,15 +156,16 @@
return 0;
}
}
- /* Return false if AllowUsers isn't empty and user isn't listed there
*/
+ /* Return true if person in AllowUsers, otherwise try the group test */
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) {
- log("User %.100s not allowed because not listed in AllowUsers",
+ goto success;
+
+ if (options.num_deny_groups == 0 &&
+ options.num_allow_groups == 0) {
+ log("User %.100s not allowed because not in AllowUsers",
pw->pw_name);
return 0;
}
@@ -194,9 +195,16 @@
if (!ga_match(options.allow_groups,
options.num_allow_groups)) {
ga_free();
- log("User %.100s not allowed because none of user's groups are
listed in AllowGroups",
- pw->pw_name);
- return 0;
+ if (options.num_deny_users > 0 ||
+ options.num_allow_users > 0) {
+ log("User %.100s not allowed because not in AllowUsers nor user's
groups in AllowGroups",
+ pw->pw_name);
+ return 0;
+ } else {
+ log("User %.100s not allow because none of user's groups are
listed in AllowGroups",
+ pw->pw_name);
+ return 0;
+ }
}
ga_free();
}
@@ -219,6 +227,7 @@
#endif /* WITH_AIXAUTHENTICATE */
/* We found no reason not to let this user try to log on... */
+success:
return 1;
}
--
James Dennis
Harvard Law School
"Not everything that counts can be counted,
and not everything that can be counted counts."