'cycle in netgroup check too greedy'). PR #508 is apparently due to an inconsistency in the way the 4.4BSD netgroup code deals with bad netgroups. When 4.4BSD code encounters a badly formed netgroup entry (e.g. (somehost,-somedomain), which, because of the missing comma between the '-' and 'somedomain,' has only 2 fields instead of 3), it generates an error message and then bails out without doing any more processing on the netgroup containing the bad entry. Conversely, every other *NIX in the world that usees netgroups just tries to parse the entry as best it can and then silently continues on its way. The result is that two bad things happen: 1) we ignore other valid entries within the netgroup containing the bogus entry, which prevents us from interoperating with other systems that don't behave this way, and 2) by printing an error to stderr from inside libc, we hose certain programs, in this case rlogind. In the problem report, Bill Fenner noted that the 'B' from 'Bad' was missing, and that rlogind exited immediately after generating the error. The missing 'B' is apparently not caused by any problem in getnetgrent.c; more likely it's getting swallowed up by rlogind somehow, and the error message itself causes rlogind to become confused. I was able to duplicate this problem and discovered that running a simple test program on my FreeBSD system resulted in a properly formatted (if confusing) error, whereas triggering the error by trying to rlogin to the machine yielded the missing 'B' problem. Anyway, the fixes for this are as follows: - The error message has been reformatted so that it prints out more useful information (e.g. Bad entry (somehost,-somedomain) in netgroup "foo"). We check for NULL entries so that we don't print '(null)' anymore too. :) - Rearranged things in parse_netgrp() so that we make a best guess at what bad entries are supposed to look like and then continue processing instead of bailing out. - Even though the error message has been cleaned up, it's wrapped inside a #ifdef DEBUG. This way we match the behavior of other systems. Since we now handle the error condition better anyway, this error message becomes less important. PR #507 is another case of inconsistency. The code that handles duplicate/circular netgroup entries isn't really 'too greedy; -- it's just too noisy. If you have a netgroup containing duplicate entries, the code actually does the right thing, but it also generates an error message. As with the 'Bad netgroup' message, spewing this out from inside libc can also hose certain programs (like rlogind). Again, no other system generates an error message in this case. The only change here is to hide the error message inside an #ifdef DEBUG. Like the other message, it's largely superfluous since the code handles the condition correctly. Note that PR #510 (+@netgroup host matching in /etc/hosts.equiv) is still being investigated. I haven't been able to duplicate it myself, and I strongly suspect it to be a configuration problem of some kind. However, I'm leaving all three PRs open until I get 510 resolved just for the sake of paranoia.
This commit is contained in:
Bill Paul
parent
689a773d32
commit
dbf973c0c7
|
|
@ -192,7 +192,7 @@ innetgr(group, host, user, dom)
|
|||
char *hst, *usr, *dm;
|
||||
|
||||
/* Sanity check */
|
||||
|
||||
|
||||
if (group == NULL || !strlen(group))
|
||||
return (0);
|
||||
|
||||
|
|
@ -217,6 +217,9 @@ parse_netgrp(group)
|
|||
{
|
||||
register char *spos, *epos;
|
||||
register int len, strpos;
|
||||
#ifdef DEBUG
|
||||
register int fields;
|
||||
#endif
|
||||
char *pos, *gpos;
|
||||
struct netgrp *grp;
|
||||
struct linelist *lp = linehead;
|
||||
|
|
@ -233,7 +236,15 @@ parse_netgrp(group)
|
|||
(lp = read_for_group(group)) == (struct linelist *)0)
|
||||
return (1);
|
||||
if (lp->l_parsed) {
|
||||
#ifdef DEBUG
|
||||
/*
|
||||
* This error message is largely superflous since the
|
||||
* code handles the error condition sucessfully, and
|
||||
* spewing it out from inside libc can actually hose
|
||||
* certain programs.
|
||||
*/
|
||||
fprintf(stderr, "Cycle in netgroup %s\n", lp->l_groupname);
|
||||
#endif
|
||||
return (1);
|
||||
} else
|
||||
lp->l_parsed = 1;
|
||||
|
|
@ -247,8 +258,14 @@ parse_netgrp(group)
|
|||
grouphead.gr = grp;
|
||||
pos++;
|
||||
gpos = strsep(&pos, ")");
|
||||
#ifdef DEBUG
|
||||
fields = 0;
|
||||
#endif
|
||||
for (strpos = 0; strpos < 3; strpos++) {
|
||||
if (spos = strsep(&gpos, ",")) {
|
||||
#ifdef DEBUG
|
||||
fields++;
|
||||
#endif
|
||||
while (*spos == ' ' || *spos == '\t')
|
||||
spos++;
|
||||
if (epos = strpbrk(spos, " \t")) {
|
||||
|
|
@ -262,9 +279,32 @@ parse_netgrp(group)
|
|||
bcopy(spos, grp->ng_str[strpos],
|
||||
len + 1);
|
||||
}
|
||||
} else
|
||||
goto errout;
|
||||
} else {
|
||||
/*
|
||||
* All other systems I've tested
|
||||
* return NULL for empty netgroup
|
||||
* fields. It's up to user programs
|
||||
* to handle the NULLs appropriately.
|
||||
*/
|
||||
grp->ng_str[strpos] = NULL;
|
||||
}
|
||||
}
|
||||
#ifdef DEBUG
|
||||
/*
|
||||
* Note: on other platforms, malformed netgroup
|
||||
* entries are not normally flagged. While we
|
||||
* can catch bad entries and report them, we should
|
||||
* stay silent by default for compatibility's sake.
|
||||
*/
|
||||
if (fields < 3)
|
||||
fprintf(stderr, "Bad entry (%s%s%s%s%s) in netgroup \"%s\"\n",
|
||||
grp->ng_str[NG_HOST] == NULL ? "" : grp->ng_str[NG_HOST],
|
||||
grp->ng_str[NG_USER] == NULL ? "" : ",",
|
||||
grp->ng_str[NG_USER] == NULL ? "" : grp->ng_str[NG_USER],
|
||||
grp->ng_str[NG_DOM] == NULL ? "" : ",",
|
||||
grp->ng_str[NG_DOM] == NULL ? "" : grp->ng_str[NG_DOM],
|
||||
lp->l_groupname);
|
||||
#endif
|
||||
} else {
|
||||
spos = strsep(&pos, ", \t");
|
||||
if (parse_netgrp(spos))
|
||||
|
|
@ -276,10 +316,6 @@ parse_netgrp(group)
|
|||
pos++;
|
||||
}
|
||||
return (0);
|
||||
errout:
|
||||
fprintf(stderr, "Bad netgroup %s at ..%s\n", lp->l_groupname,
|
||||
spos);
|
||||
return (1);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
|||
Loading…
Reference in New Issue