Date: Sun, 14 Dec 2003 13:36:25 -0800 From: Brooks Davis <brooks@one-eyed-alien.net> To: Diomidis Spinellis <dds@aueb.gr> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src UPDATING (initgroups) Message-ID: <20031214213624.GA4077@Odin.AC.HMC.Edu> In-Reply-To: <3FDC7D65.3040406@aueb.gr> References: <Pine.NEB.3.96L.1031213210011.58711D-100000@fledge.watson.org> <3FDC7D65.3040406@aueb.gr>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Sun, Dec 14, 2003 at 05:10:29PM +0200, Diomidis Spinellis wrote:
> 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.
Error handling IS a problem. With a fairly standard configuration,
accounts with >16 groups can not log in at least under STABLE.
> 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.
I don't think a syslog message mentioning "invalid argument" is
sufficent in STABLE. We've turned accounts with a minor problem that
few people noticed into accounts that can't login. I don't think it's
reasionable to force admins to back trace from "invalid argument" to
EINVAL to a non-standard meaning listed in the function call manpage,
espeicaly since we could emit a useful error instead.
> 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.
This is insufficent. It would not have caught the case we saw at work
because the user got the extra groups from NIS.
-- Brooks
--
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQE/3NfXXY6L6fI4GtQRAkA2AJ487A8AN1FMZ8hSTWHkuYW+N77RhgCdEP+P
S6k9RbxzIKsvhSFdl09Dm9k=
=nxiU
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031214213624.GA4077>