cvs commit: src UPDATING (initgroups)

[Diomidis Spinellis]

Robert Watson wrote:
> On Sat, 13 Dec 2003, Brooks Davis wrote:
>>A similar change is needed in 4.x or the change should be backed there. 
>>I think we should back it out (in stable) until the various users of
>>initgroups are fixed to output something useful and, preferably, not
>>exit when this happens.  As it is, we turned an rarely hit edge case
>>that was somewhat difficult to debug into a fatal error that that is
>>about equally difficult to debug and breaks the account in question. 
> 
> Sigh.  I agree.  I didn't realize the MFC had happened, but sure enough,
> it's change 1.3.8.2.  I have somewhat mixed feelings: I feel strongly we
> should generate an error and fail closed, but I also agree that the
> transition period was too short (we should have a warning period on the
> -STABLE branch), and that we need to do something about error handling.
> I'm surprised more people haven't bumped into it, but I guess the MFC was
> only a couple of days ago.

I have checked all instances in our code where initgroups(3) is called. 
  Appart from a single case, where the error value is silently 
propagated upward (openpam_borrow_cred), in all other cases the returned 
value is either checked and appropriately reported or ignored (see 
attached file).  So, error handling is not a problem.

The problem reported in current@ was probably through a call to 
setusercontext(3).  The call should have generated a syslog message of 
the form:
"initgroups(kjwolf, 1000): invalid argument"
EINVAL is now appropriately documented in setgroups(2):
"The number specified in the ngroups argument is larger than the NGROUPS 
limit."
so again, the error should be visible in the hosts syslog.

Given that this type of error was silently ignored in the past (with 
group memberships more than NGROUPS being silently ignored), I agree 
that we might want to help users check their systems.  The following 
script will check a typical group(5) file and report cases where 
setgroups would overflow.

#!/bin/sh
awk -F'[:,]' '
{ for (i = 4; i <= NF; i++) if (length($i)) g[$i]++; }
END { for (u in g) if (g[u] > '`sysctl -n kern.ngroups`' - 2) print "Too 
many group memberships for user " u }
' /etc/group

I suggest we add it in the corresponding UPDATING entry/entries.

Diomidis - dds@
-------------- next part --------------
silent_fail	lib/libpam/libpam/openpam_borrow_cred.c:	if (initgroups(pwd->pw_name, pwd->pw_gid) == -1 ||
UNKNOWN-ERROR(per_RFC)+exit	usr.sbin/inetd/builtins.c:		if (initgroups(pw->pw_name, pw->pw_gid) == -1)

ignore	contrib/lukemftpd/src/ftpd.c:	(void) initgroups(pw->pw_name, pw->pw_gid);
ignore	contrib/opie/opieftpd.c:  initgroups(pw->pw_name, pw->pw_gid);
ignore	contrib/opie/opielogin.c:  initgroups(name, thisuser.pw_gid);
ignore	crypto/heimdal/appl/ftp/ftpd/ftpd.c:    initgroups(pw->pw_name, pw->pw_gid);
ignore	crypto/kerberosIV/appl/bsd/login.c:	initgroups(username, pwd->pw_gid);
ignore	crypto/kerberosIV/appl/bsd/rshd.c:    initgroups(pwd->pw_name, pwd->pw_gid);
ignore	crypto/kerberosIV/appl/ftp/ftpd/ftpd.c:    initgroups(pw->pw_name, pw->pw_gid);
ignore	libexec/ftpd/ftpd.c:	(void) initgroups(pw->pw_name, pw->pw_gid);
ignore	libexec/rexecd/rexecd.c:	initgroups(pwd->pw_name, pwd->pw_gid);
ignore	libexec/rshd/rshd.c:	initgroups(pwd->pw_name, pwd->pw_gid);
ignore	libexec/uucpd/uucpd.c:		initgroups(pw->pw_name, pw->pw_gid);
ignore	usr.bin/calendar/calendar.c:			(void)initgroups(pw->pw_name, pw->pw_gid);
ignore	usr.bin/usr.bin/calendar/calendar.c:			(void)initgroups(pw->pw_name, pw->pw_gid);
ignore	usr.sbin/cron/cron/do_command.c:			initgroups(usernm, e->gid);
ignore	usr.sbin/cron/cron/popen.c:				initgroups(usernm, e->gid);
ignore	usr.sbin/inetd/inetd.c:					(void) initgroups(pwd->pw_name,
perror+exit	crypto/openssh/session.c:		if (initgroups(pw->pw_name, pw->pw_gid) < 0) {
print+exit	contrib/opie/opiesu.c:  if (initgroups(user, thisuser.pw_gid)) {
print+exit	contrib/sendmail/src/deliver.c:					if (initgroups(DefUser, DefGid) == -1 &&
print+exit	contrib/sendmail/src/deliver.c:					if (initgroups(user,
print+exit	contrib/sendmail/src/deliver.c:			if (initgroups(RealUserName, RealGid) == -1 && suidwarn)
print+exit	crypto/openssh/uidswap.c:		if (initgroups(pw->pw_name, pw->pw_gid) < 0)
print+exit	libexec/atrun/atrun.c:	if (initgroups(pentry->pw_name,pentry->pw_gid))
print+exit	libexec/atrun/atrun.c:	if (initgroups(pentry->pw_name,pentry->pw_gid))
print+fail	usr.bin/su/su.c:	if (initgroups(user, pwd->pw_gid))
print+ignore?	contrib/sendmail/src/recipient.c:			if (initgroups(user, gid) == -1)
print+panic	contrib/bind/bin/named/ns_main.c:		if (getuid() == 0 && initgroups(user_name, group_id) < 0)
syslog+fail	crypto/kerberosIV/appl/kauth/kauthd.c:	 initgroups(passwd->pw_name, passwd->pw_gid) ||
syslog+fail	lib/libutil/login_class.c:	if (initgroups(pwd->pw_name, pwd->pw_gid) == -1) {
syslog+fail	usr.sbin/lpr/lpd/printjob.c:			fail = initgroups(daemon_uname, daemon_defgid);
warn	crypto/heimdal/appl/login/login.c:    if(initgroups(pwd->pw_name, pwd->pw_gid)){