cvs commit: src UPDATING (initgroups)

[Brooks Davis]

On Mon, Dec 15, 2003 at 12:55:16AM +0200, Diomidis Spinellis wrote:
> Brooks Davis wrote:
> [...]
> >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.
> 
> Reinterpreting errno on a case-by-case basis as in
> 
>     if (initgroups(pwd->pw_name, pwd->pw_gid) == -1) {
> 	if (errno == EINVAL)
>            syslog(LOG_ERR, "initgroups(%s,%lu): too many groups", 
> pwd->pw_name, (u_long)pwd->pw_gid);
> 	else
>            syslog(LOG_ERR, "initgroups(%s,%lu): %m", pwd->pw_name,
>                    (u_long)pwd->pw_gid);
> 
> will introduce changes in 34 source code files (many of them contributed 
> and not under our direct control), or result on a non-orthogonal 
> treatment of this problem.  Interpreting the error message through the 
> errno value and the associated manpage is EXACTLY what any competent 
> Unix system administrator should be able and expected to do.

On one hand, a competent admin should be able to do that, on the other,
we've just screwed the user who is probably going to report the wrong
error message to the admin who is going to take a long time to track
this down because they won't be able to replicate the problem easily
(after all, most other accounts on the machine will probably still
work).  My objection is a) we're going the user an unhelpful error and
b) breaking their login.  If b) goes away, I could live with a).

> On the other hand, if non-working accounts cause a significant problem 
> for a number of installations we could add a temporary fix to ignore the 
> error and report the cause just in lib/libutil/login_class.c (which 
> seems to cause the problem).  This could then be removed after a 
> deprecation period (say six months):
> 
>     if (initgroups(pwd->pw_name, pwd->pw_gid) == -1) {
> 	if (errno == EINVAL)
>            syslog(LOG_ERR, "initgroups(%s,%lu): deprecated feature: 
> member of > NGROUPS error ignored", pwd->pw_name, (u_long)pwd->pw_gid);
> 	else {
>            syslog(LOG_ERR, "initgroups(%s,%lu): %m", pwd->pw_name,
>                    (u_long)pwd->pw_gid);
>             login_close(llc);
>             return -1;
>         }

I think we should put this in in stable and probably never remove it.
I'd defintly object if we removed it before 4.11 because we need to ship
at least one release with a warning before breaking things since I don't
think this is a security issue.  If someone can come up with a way not
being a member of a group would be a security issue I'd withdraw that
objection and just suggest that we add a special case syslog to stable
to avoid confusion.

-- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20031214/5b45dd7b/attachment.bin