From owner-cvs-all@FreeBSD.ORG Sun Dec 14 07:10:38 2003 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 54D2516A4CF; Sun, 14 Dec 2003 07:10:38 -0800 (PST) Received: from hermes.aueb.gr (hermes.aueb.gr [195.251.255.142]) by mx1.FreeBSD.org (Postfix) with ESMTP id 717DC43D39; Sun, 14 Dec 2003 07:10:33 -0800 (PST) (envelope-from dds@aueb.gr) Received: from aueb.gr (faculty02.right.dialup.aueb.gr [195.251.255.246]) by hermes.aueb.gr (8.12.9/8.12.9) with ESMTP id hBEJ6F2a022677; Sun, 14 Dec 2003 21:06:15 +0200 Message-ID: <3FDC7D65.3040406@aueb.gr> Date: Sun, 14 Dec 2003 17:10:29 +0200 From: Diomidis Spinellis User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007 X-Accept-Language: en-us, en, el, de MIME-Version: 1.0 To: Robert Watson References: In-Reply-To: Content-Type: multipart/mixed; boundary="------------010303000607080602010002" cc: Brooks Davis cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: dds@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src UPDATING (initgroups) X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Dec 2003 15:10:38 -0000 This is a multi-part message in MIME format. --------------010303000607080602010002 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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@ --------------010303000607080602010002 Content-Type: text/plain; name="initgroups_users.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="initgroups_users.txt" 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)){ --------------010303000607080602010002--