Re: REPOST: Finding group members - fix to winbindd_ads.c
Ken Cross wrote: Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you get *all* the members of a group, whether primary or supplemental. The same call to an AD using LDAP just returns supplemental members. My patch causes the call to either an NT domain or AD to return the same thing. There was some discussion about making it a separate call, but I think that'd be a mistake. It seems like they should be consistent. Would like to add my two cents to the discussion, too :) I think there are good reasons for the habit of not including primary group members in /etc/group, which would also apply to winbind getgrnam/getgrgid calls. Many (if not most) invocations of getgrnam/getgrgid are for GID - name conversions, where the list of group members is just unnecessary load. Which can be reduced by omitting primary group members. It's a pity there are no widespread standard Unix calls for these conversions without querying group members, but that's a boundary condition which winbind can hardly get around. Every ls -l gathers group member lists just to ignore them, for example. Furthermore, some platforms do have an implicit limitation of group sizes, by providing a default fixed result buffer size, which most callers of getgrnam/getgrgid (better: their reentrant counterparts getgrnam_r/getgrgid_r) adhere to. Omitting primary members makes hitting such limits less likely. Cheers! Michael
RE: REPOST: Finding group members - fix to winbindd_ads.c
Michael: I don't disagree with anything you said. However, we currently get different results from a getgrnam/getgrgid depending on whether you net rpc join or net ads join. That, IMHO, is a Bad Thing. Thanks, Ken -Original Message- From: Michael Steffens [mailto:[EMAIL PROTECTED]] Sent: Monday, February 10, 2003 7:20 AM To: Ken Cross Cc: 'Multiple recipients of list SAMBA-TECHNICAL' Subject: Re: REPOST: Finding group members - fix to winbindd_ads.c Ken Cross wrote: Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you get *all* the members of a group, whether primary or supplemental. The same call to an AD using LDAP just returns supplemental members. My patch causes the call to either an NT domain or AD to return the same thing. There was some discussion about making it a separate call, but I think that'd be a mistake. It seems like they should be consistent. Would like to add my two cents to the discussion, too :) I think there are good reasons for the habit of not including primary group members in /etc/group, which would also apply to winbind getgrnam/getgrgid calls. Many (if not most) invocations of getgrnam/getgrgid are for GID - name conversions, where the list of group members is just unnecessary load. Which can be reduced by omitting primary group members. It's a pity there are no widespread standard Unix calls for these conversions without querying group members, but that's a boundary condition which winbind can hardly get around. Every ls -l gathers group member lists just to ignore them, for example. Furthermore, some platforms do have an implicit limitation of group sizes, by providing a default fixed result buffer size, which most callers of getgrnam/getgrgid (better: their reentrant counterparts getgrnam_r/getgrgid_r) adhere to. Omitting primary members makes hitting such limits less likely. Cheers! Michael
Re: REPOST: Finding group members - fix to winbindd_ads.c
Hello Ken, Ken Cross wrote: I don't disagree with anything you said. However, we currently get different results from a getgrnam/getgrgid depending on whether you net rpc join or net ads join. That, IMHO, is a Bad Thing. Agreed 100%, too! :) Wrote this because from previous postings I had the impressions that most people are voting for unifying to inclusion of primary group members, who also had their valid points. But I think this way is not necessarily a good idea. Unifying the behaviour *is* a good idea, for sure. :) Cheers! Michael -Original Message- From: Michael Steffens [mailto:[EMAIL PROTECTED]] Sent: Monday, February 10, 2003 7:20 AM To: Ken Cross Cc: 'Multiple recipients of list SAMBA-TECHNICAL' Subject: Re: REPOST: Finding group members - fix to winbindd_ads.c Ken Cross wrote: Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you get *all* the members of a group, whether primary or supplemental. The same call to an AD using LDAP just returns supplemental members. My patch causes the call to either an NT domain or AD to return the same thing. There was some discussion about making it a separate call, but I think that'd be a mistake. It seems like they should be consistent. Would like to add my two cents to the discussion, too :) I think there are good reasons for the habit of not including primary group members in /etc/group, which would also apply to winbind getgrnam/getgrgid calls. Many (if not most) invocations of getgrnam/getgrgid are for GID - name conversions, where the list of group members is just unnecessary load. Which can be reduced by omitting primary group members. It's a pity there are no widespread standard Unix calls for these conversions without querying group members, but that's a boundary condition which winbind can hardly get around. Every ls -l gathers group member lists just to ignore them, for example. Furthermore, some platforms do have an implicit limitation of group sizes, by providing a default fixed result buffer size, which most callers of getgrnam/getgrgid (better: their reentrant counterparts getgrnam_r/getgrgid_r) adhere to. Omitting primary members makes hitting such limits less likely. Cheers! Michael
REPOST: Finding group members - fix to winbindd_ads.c
Samba-folk:
A couple of weeks ago I posted a patch to winbindd_ads.c to change the
behavior of WINBINDD_GETGRNAM and WINBINDD_GETGRGID using LDAP.
Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you
get *all* the members of a group, whether primary or supplemental.
The same call to an AD using LDAP just returns supplemental members.
My patch causes the call to either an NT domain or AD to return the same
thing.
There was some discussion about making it a separate call, but I think
that'd be a mistake. It seems like they should be consistent.
Andrew said to repost a patch if it seems to have been forgotten, so...
Ken Cross
Network Storage Solutions
Index: winbindd_ads.c
===
RCS file: /cvsroot/samba/source/nsswitch/winbindd_ads.c,v
retrieving revision 1.43.2.7
diff -p -u -r1.43.2.7 winbindd_ads.c
--- winbindd_ads.c 20 Dec 2002 20:21:27 - 1.43.2.7
+++ winbindd_ads.c 8 Feb 2003 13:57:03 -
@@ -73,7 +73,7 @@ static ADS_STRUCT *ads_cached_connection
ads_destroy(ads);
/* if we get ECONNREFUSED then it might be a NT4
- server, fall back to MSRPC */
+ server, fall back to MSRPC */
if (status.error_type == ADS_ERROR_SYSTEM
status.err.rc == ECONNREFUSED) {
DEBUG(1,(Trying MSRPC methods\n));
@@ -531,13 +531,17 @@ static NTSTATUS lookup_groupmem(struct w
ADS_STATUS rc;
int count;
void *res=NULL;
+ void *res2=NULL;
ADS_STRUCT *ads = NULL;
char *exp;
NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
char *sidstr;
const char *attrs[] = {member, NULL};
+ const char *attrs2[] = {userPrincipalName, sAMAccountName,
+ objectSid, sAMAccountType, NULL};
char **members;
- int i, num_members;
+ char **primaries;
+ int i, num_members=0, num_primaries=0;
*num_names = 0;
@@ -554,33 +558,79 @@ static NTSTATUS lookup_groupmem(struct w
free(sidstr);
if (!ADS_ERR_OK(rc)) {
- DEBUG(1,(query_user_list ads_search: %s\n, ads_errstr(rc)));
+ DEBUG(1,(lookup_groupmem ads_search: %s\n, ads_errstr(rc)));
goto done;
}
count = ads_count_replies(ads, res);
- if (count == 0) {
- status = NT_STATUS_OK;
- goto done;
+ if (count 0) {
+ if(members = ads_pull_strings(ads, mem_ctx, res, member)) {
+ for (i=0;members[i];i++) /* noop */ ;
+ num_members = i;
+ }
}
+
+ if (res) ads_msgfree(ads, res);
+ res = NULL;
+
+ /*
+* We must also find all users with primaryGroupID == group_rid
+*/
+ asprintf(exp, ((primaryGroupID=%d)(objectClass=user)), group_rid);
+ rc = ads_search_retry(ads, res, exp, attrs2);
+ free(exp);
- members = ads_pull_strings(ads, mem_ctx, res, member);
- if (!members) {
- /* no members? ok ... */
- status = NT_STATUS_OK;
+ if (!ADS_ERR_OK(rc)) {
+ DEBUG(1,(lookup_groupmem ads_search: %s\n, ads_errstr(rc)));
goto done;
}
- /* now we need to turn a list of members into rids, names and name types
- the problem is that the members are in the form of distinguised names
- */
- for (i=0;members[i];i++) /* noop */ ;
- num_members = i;
+ num_primaries = ads_count_replies(ads, res);
+
+ (*rid_mem) = talloc_zero(mem_ctx, sizeof(uint32) * (num_members +
+num_primaries));
+ (*name_types) = talloc_zero(mem_ctx, sizeof(uint32) * (num_members +
+num_primaries));
+ (*names) = talloc_zero(mem_ctx, sizeof(char *) * (num_members +
+num_primaries));
+
+ /*
+* Store users whose primary groups match this group.
+* We have conveniently retrieved all the information we need.
+*/
+ for (i=0; inum_primaries; i++) {
+ uint32 atype, rid;
+ DOM_SID sid;
+ char *pName;
+
+ if (i==0)
+ res2 = ads_first_entry(ads, res);
+ else
+ res2 = ads_next_entry(ads, res2);
+
+ if (!res2) {
+ DEBUG(0,(lookup_groupmem mismatch between count and entries??));
+ break;
+ }
+
+ pName = ads_pull_username(ads, mem_ctx, res2);
+
+ if (!pName ||
+ !ads_pull_uint32(ads, res2, sAMAccountType, atype) ||
+ !ads_pull_sid(ads, res2, objectSid, sid) ||
+ !sid_peek_rid(sid, rid)) {
+ DEBUG(3,(lookup_groupmem failed to retrieve user data\n));
+ continue;
+ }
- (*rid_mem) = talloc_zero(mem_ctx, sizeof(uint32) * num_members);
Re: REPOST: Finding group members - fix to winbindd_ads.c
On Sat, Feb 08, 2003 at 09:10:41AM -0500, Ken Cross wrote: Samba-folk: A couple of weeks ago I posted a patch to winbindd_ads.c to change the behavior of WINBINDD_GETGRNAM and WINBINDD_GETGRGID using LDAP. Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you get *all* the members of a group, whether primary or supplemental. The same call to an AD using LDAP just returns supplemental members. My patch causes the call to either an NT domain or AD to return the same thing. There was some discussion about making it a separate call, but I think that'd be a mistake. It seems like they should be consistent. Andrew said to repost a patch if it seems to have been forgotten, so... It's *not* been forgotten :-). Honest ! It's in my inbox, where all the patches due for review (by me at least) live :-). I work on these things on a LIFO basis so sometimes things stay there for a little while :-), but it's *not* been forgotten :-). Jeremy.
RE: REPOST: Finding group members - fix to winbindd_ads.c
No problem, Jeremy -- I understand completely. Thanks for the response. Ken -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] Sent: Saturday, February 08, 2003 1:00 PM To: Ken Cross Cc: 'Multiple recipients of list SAMBA-TECHNICAL' Subject: Re: REPOST: Finding group members - fix to winbindd_ads.c On Sat, Feb 08, 2003 at 09:10:41AM -0500, Ken Cross wrote: Samba-folk: A couple of weeks ago I posted a patch to winbindd_ads.c to change the behavior of WINBINDD_GETGRNAM and WINBINDD_GETGRGID using LDAP. Currently, if you do WINBINDD_GETGRNAM to an NT domain using RPC, you get *all* the members of a group, whether primary or supplemental. The same call to an AD using LDAP just returns supplemental members. My patch causes the call to either an NT domain or AD to return the same thing. There was some discussion about making it a separate call, but I think that'd be a mistake. It seems like they should be consistent. Andrew said to repost a patch if it seems to have been forgotten, so... It's *not* been forgotten :-). Honest ! It's in my inbox, where all the patches due for review (by me at least) live :-). I work on these things on a LIFO basis so sometimes things stay there for a little while :-), but it's *not* been forgotten :-). Jeremy.
