Date: Thu, 5 Sep 1996 01:06:56 -0400 (EDT) From: Bill Paul <wpaul@skynet.ctr.columbia.edu> To: pst@jnx.com (Paul Traina) Cc: bugs@freebsd.org Subject: Re: proper entry for /etc/groups override? Message-ID: <199609050506.BAA16389@skynet.ctr.columbia.edu> In-Reply-To: <199609050048.RAA12235@base.jnx.com> from "Paul Traina" at Sep 4, 96 05:48:04 pm
next in thread | previous in thread | raw e-mail | index | archive | help
Of all the gin joints in all the towns in all the world, Paul Traina had
to walk into mine and say:
> Bill,
>
> I'm having problems with YP in /etc/group and I'm wondering what I'm doing
> wrong.
You're not doing anything wrong. The code is just busted. I've been
waiting for an excuse to fix this. Thanks for prodding me.
I've got a patch for getgrent.c for you to try at the end of this
message. This should work on both -current and 2.1.x. First, some
discussion.
> With the following program:
>
> ------
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <grp.h>
>
> main(int argc, char **argv)
> {
> char **g;
> struct group *gr;
>
> gr = getgrgid((gid_t) 0);
> if (!gr)
> perror("no gr");
You should bail here if gr == NULL, otherwise you'll SEGV at the
next line.
Also, use warn() or err() rather than perror(); it's more BSDish. :)
> for (g = gr->gr_mem; ; ++g) {
> if (!*g) {
> printf(" -- done\n");
> exit(0);
> }
> printf("%s ", *g);
> }
> }
> ----------
>
> and a YP group entry with:
>
> pst@base$ ypmatch wheel group
> wheel:*:0:root,pst,tli,dennis,dock,pusateri,krishna,shermis
Looks ok.
> and no "wheel" entry in /etc/group, but +::: at the end, I get:
>
> pst@base$ ./foo
> root pst tli dennis dock pusateri krishna shermis -- done
>
> which is correct... we match the wheel group out of YP.
Yep.
> However, I want to specify that getgr* should get wheel group membership
> out of YP, not have it work "by default".
>
> I read the man page, and I think it wants me to put "+wheel" in my group file.
Hm. I have to think about this. Because of the way we use Berkeley DB
databases for the password system, /etc/master.passwd entries must
stick to the proper format, otherwise pwd_mkdb would get very confused.
So you can't have things like + or +:, which are legal to use in
/etc/passwd on other systems.
We don't use databases for groups though. I'm not sure I like this
syntax, but I've endeavoured to make it work right -- this time.
> When I do that, I get:
>
> pst@base$ ./foo
> daemon -- done
>
> This is very bogus. Daemon is certainly not in the wheel group, and the
> other stuff is missing.
Bug in grscan(): the _ypfound flag was being initialized to 0, but
it wasn't being cleared on each iteration of the for(;;) loop (so
it remained set incorrectly during a second pass through the loop
and caused the code to falsely select the next group in /etc/group,
daemon, of which daemon is a member. Fixed.
> With an /etc/group entry for wheel reading:
>
> +wheel:::
>
> I get
>
> pst@base$ ./foo
> -- done
Another bug in grscan(): after filling in the grp struct from YP,
the members were thrown away unconditionally to prepare for local
overriding which never happened (you didn't specify any local members).
In getpwent.c, this is handled using the pw_fields member of the
passwd struct: _pw_breakout_yp() can tell if it needs to preserve
local data for a particular field since pwd_mkdb sets a flag for
each non-empty field. If the breakout function sees that the flag
for a given field is set, it treats the field as being locally
overridden and tosses out the corresponding YP data. (The exception
is for the username: we always want the YP username.)
In getgrent.c, _gr_breakout_yp() has no such nifty mechanism, so we
do things backwards: we fetch the YP data first, fill in the whole
grp struct, then we read the local data and override fields only
if local data for a field exists.
Bleah.
> showing no entries.
>
> with
>
> +wheel:*
>
> the libc function core dumps. :-( Yet another bug.
Yes, though this one isn't strictly due to YP, I don't think.
This entry is actually invalid, and getgrent(3) is within its
rights to ignore it, though it should not blow up because of it.
The problem is that the code does not test for a potential NULL
pointer before diving into the member parsing code. Also fixed.
Note that this one fix is also outside of #ifdef YP since it looks
like it can happen even with YP turned off. If the code encounters
an entry like this, which leads to the NULL pointer condition, it
considers the entry to be bogus (which it is) and skips it.
I _think_ I got this mess behaving correctly now. Please test this
patch for me and let me know how things turn out. I also fixed
what I think was a memory leak in _getypgroup() while I was in
the area.
You should now be able to use the following syntaxes:
+wheel Insert just the wheel group from YP, no overrides.
+wheel::: Ditto.
+wheel:*:: Insert just the wheel group, override password to '*'.
+wheel:::foo Insert just the wheel group, override members to foo.
+ Insert all YP entries, no overrides.
+:: Ditto
+::: Ditto
+:*:: Ditto
You can't do global overrides such as +:*::foo,bar since it really
doesn't make much sense. Note that you also can't override GIDs
for the same reason (though it would be fairly easy to allow it).
-Bill
--
=============================================================================
-Bill Paul (212) 854-6020 | System Manager, Master of Unix-Fu
Work: wpaul@ctr.columbia.edu | Center for Telecommunications Research
Home: wpaul@skynet.ctr.columbia.edu | Columbia University, New York City
=============================================================================
"If you're ever in trouble, go to the CTR. Ask for Bill. He will help you."
=============================================================================
*** getgrent.c.orig Wed Sep 4 23:33:52 1996
--- getgrent.c Thu Sep 5 00:56:24 1996
***************
*** 165,171 ****
_gr_yp_enabled = 0;
while((line = fgetln(_gr_fp, &linelen)) != NULL) {
if(line[0] == '+') {
! if(line[1] && !_gr_yp_enabled) {
_gr_yp_enabled = 1;
} else {
_gr_yp_enabled = -1;
--- 165,171 ----
_gr_yp_enabled = 0;
while((line = fgetln(_gr_fp, &linelen)) != NULL) {
if(line[0] == '+') {
! if(line[1] && line[1] != ':' && !_gr_yp_enabled) {
_gr_yp_enabled = 1;
} else {
_gr_yp_enabled = -1;
***************
*** 218,226 ****
register char *cp, **m;
char *bp;
#ifdef YP
! int _ypfound = 0;
#endif;
for (;;) {
if (!fgets(line, sizeof(line), _gr_fp))
return(0);
bp = line;
--- 218,229 ----
register char *cp, **m;
char *bp;
#ifdef YP
! int _ypfound;
#endif;
for (;;) {
+ #ifdef YP
+ _ypfound = 0;
+ #endif
if (!fgets(line, sizeof(line), _gr_fp))
return(0);
bp = line;
***************
*** 251,261 ****
return(0);
}
} else {
! if (!_getypgroup(&_gr_group, &_gr_group.gr_name[1],
"group.byname"))
continue;
/* We're going to override -- tell the world. */
- members[0] = NULL;
_ypfound++;
}
}
--- 254,270 ----
return(0);
}
} else {
! cp = &_gr_group.gr_name[1];
! if (search && name != NULL)
! if (strcmp(cp, name))
! continue;
! if (!_getypgroup(&_gr_group, cp,
"group.byname"))
continue;
+ if (search && name == NULL)
+ if (gid != _gr_group.gr_gid)
+ continue;
/* We're going to override -- tell the world. */
_ypfound++;
}
}
***************
*** 268,284 ****
continue;
}
}
if ((_gr_group.gr_passwd = strsep(&bp, ":\n")) == NULL)
! break;;
if (!(cp = strsep(&bp, ":\n")))
! continue;
#ifdef YP
if (!_ypfound)
#endif
_gr_group.gr_gid = atoi(cp);
if (search && name == NULL && _gr_group.gr_gid != gid)
continue;
cp = NULL;
for (m = _gr_group.gr_mem = members;; bp++) {
if (m == &members[MAXGRP - 1])
break;
--- 277,323 ----
continue;
}
}
+ #ifdef YP
+ if ((cp = strsep(&bp, ":\n")) == NULL)
+ if (_ypfound)
+ return(1);
+ else
+ break;
+ if (strlen(cp) || !_ypfound)
+ _gr_group.gr_passwd = cp;
+ #else
if ((_gr_group.gr_passwd = strsep(&bp, ":\n")) == NULL)
! break;
! #endif
if (!(cp = strsep(&bp, ":\n")))
! if (_ypfound)
! return(1);
! else
! continue;
#ifdef YP
+ /*
+ * Hurm. Should we be doing this? We allow UIDs to
+ * be overridden -- what about GIDs?
+ */
if (!_ypfound)
#endif
_gr_group.gr_gid = atoi(cp);
if (search && name == NULL && _gr_group.gr_gid != gid)
continue;
cp = NULL;
+ if (bp == NULL) /* !!! Must check for this! */
+ break;
+ #ifdef YP
+ if ((cp = strsep(&bp, ":\n")) == NULL)
+ break;
+
+ if (!strlen(cp) && _ypfound)
+ return(1);
+ else
+ members[0] = NULL;
+ bp = cp;
+ cp = NULL;
+ #endif
for (m = _gr_group.gr_mem = members;; bp++) {
if (m == &members[MAXGRP - 1])
break;
***************
*** 376,383 ****
if(s) *s = '\0';
if(resultlen >= sizeof resultbuf) return 0;
! strcpy(resultbuf, result);
! result = resultbuf;
return(_gr_breakout_yp(gr, resultbuf));
}
--- 415,422 ----
if(s) *s = '\0';
if(resultlen >= sizeof resultbuf) return 0;
! strncpy(resultbuf, result, resultlen);
! free(result);
return(_gr_breakout_yp(gr, resultbuf));
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199609050506.BAA16389>