Bug 2038 - winbindd_getgroups() may fail if not all domain groups have a gidNumber assigned
Summary: winbindd_getgroups() may fail if not all domain groups have a gidNumber assigned
Status: CLOSED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: winbind (show other bugs)
Version: 3.0.8
Hardware: All All
: P3 major
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-11-15 06:58 UTC by Johann Hanne
Modified: 2005-08-24 10:20 UTC (History)
0 users

See Also:


Attachments
Proposed patch (652 bytes, patch)
2004-11-15 07:00 UTC, Johann Hanne
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Johann Hanne 2004-11-15 06:58:56 UTC
This happens with Samba 3.0.8 configured as a domain member server (security = 
ADS), the domain being in w2k native mode.

The following code appears twice in the winbindd_getgroups() function in 
nsswitch/winbindd_group.c.
--
if (gid_list == NULL)
  goto done;
--
It's within the two loops which iterate through the list of group memberships. 
This has the following consequence: If the loop is executed for the first time 
(examining the first group membership) and if THIS (i.e. the first) group has 
NO gidNumber assigned (i.e. add_gids_from_group_sid() fails), then the function 
fails completely ("goto done;"!), even if some other group (i.e. a non-first 
group) has a gidNumber assigned.

The code should either be removed completely (I see no reason why the function 
should fail, even if the user has NO unix group membership, this is valid 
scenario?!), or it should be at least moved to somewhere after the loops, so it 
only fails if no group has a gidNumber assigned (in fact a posix user must have 
a group memberships - the primary group; however, this doesn't need to be 
checked here, because getpwent() fails already if there is no primary group 
membership).
Comment 1 Johann Hanne 2004-11-15 07:00:24 UTC
Created attachment 772 [details]
Proposed patch
Comment 2 Jeremy Allison 2004-11-15 16:01:59 UTC
Looks good - applied. Moved this check to just before the return.
Jeremy.
Comment 3 Gerald (Jerry) Carter 2005-02-09 09:10:39 UTC
This is fixed in 3.0.11 (by either guenther or volker) I think.
Comment 4 Gerald (Jerry) Carter 2005-08-24 10:20:48 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.