r23471: Here's a rough patch for expanding domain group membership

in the winbindd_getgrnam() call.  Couple of comments:

* Adds "winbind expand groups" parameter which defines the
  max depth winbindd will expand group members.  The default
  is the current behavior of one level of expansion.
* The entire getrgnam() interface should be async.  I
  haven't done that.
* Refactors the domain users hack in fill_grent_mem() into
  its own function.
(This used to be commit 3d3a8130351753dc5caa2a270d130e2150da6b54)

2 files changed, 347 insertions, 155 deletions

diff --git a/source3/nsswitch/winbindd_ads.c b/source3/nsswitch/winbindd_ads.c
index 5dd7eafcd1..6f1db55c62 100644
--- a/source3/nsswitch/winbindd_ads.c
+++ b/source3/nsswitch/winbindd_ads.c
@@ -1062,7 +1062,11 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain,
 				{
 					sid_copy(&(*sid_mem)[*num_names],
 						 &sid_mem_nocache[i]);
-					(*names)[*num_names] = talloc_move(*names, &names_nocache[i]);
+					(*names)[*num_names] = talloc_asprintf( *names, 
+										"%s%c%s",
+										domains_nocache[i],
+										*lp_winbind_separator(),
+										names_nocache[i] );
 					(*name_types)[*num_names] = name_types_nocache[i];
 					(*num_names)++;
 				}
diff --git a/source3/nsswitch/winbindd_group.c b/source3/nsswitch/winbindd_group.c
index 05aeef2612..4ee9ab55de 100644
--- a/source3/nsswitch/winbindd_group.c
+++ b/source3/nsswitch/winbindd_group.c
@@ -195,7 +195,279 @@ static BOOL fill_grent(struct winbindd_gr *gr, const char *dom_name,
 	return True;
 }
 
-/* Fill in the group membership field of a NT group given by group_sid */
+/***********************************************************************
+ If "enum users" is set to false, and the group being looked
+ up is the Domain Users SID: S-1-5-domain-513, then for the
+ list of members check if the querying user is in that group,
+ and if so only return that user as the gr_mem array.
+ We can change this to a different parameter than "enum users"
+ if neccessaey, or parameterize the group list we do this for.
+***********************************************************************/
+
+static BOOL fill_grent_mem_domusers( TALLOC_CTX *mem_ctx,
+				     struct winbindd_domain *domain,
+				     struct winbindd_cli_state *state,
+				     DOM_SID *group_sid,
+				     enum lsa_SidType group_name_type,
+				     size_t *num_gr_mem, char **gr_mem, 
+				     size_t *gr_mem_len)
+{
+	DOM_SID querying_user_sid;
+	DOM_SID *pquerying_user_sid = NULL;
+	uint32 num_groups = 0;
+	DOM_SID *user_sids = NULL;
+	BOOL u_in_group = False;
+	NTSTATUS status;
+	int i;
+	unsigned int buf_len = 0;
+	char *buf = NULL;
+
+	DEBUG(10,("fill_grent_mem_domain_users: domain %s\n",
+		  domain->name ));
+
+	if (state) {
+		uid_t ret_uid = (uid_t)-1;
+		if (sys_getpeereid(state->sock, &ret_uid)==0) {
+			/* We know who's asking - look up their SID if
+			   it's one we've mapped before. */
+			status = idmap_uid_to_sid(&querying_user_sid, ret_uid);
+			if (NT_STATUS_IS_OK(status)) {
+				pquerying_user_sid = &querying_user_sid;
+				DEBUG(10,("fill_grent_mem_domain_users: querying uid %u -> %s\n",
+					  (unsigned int)ret_uid,
+					  sid_string_static(pquerying_user_sid) ));
+			}
+		}
+	}
+
+	/* Only look up if it was a winbindd user in this domain. */
+	if (pquerying_user_sid &&
+	    (sid_compare_domain(pquerying_user_sid, &domain->sid) == 0)) {
+		
+		DEBUG(10,("fill_grent_mem_domain_users: querying user = %s\n",
+			  sid_string_static(pquerying_user_sid) ));
+		
+		status = domain->methods->lookup_usergroups(domain,
+							    mem_ctx,
+							    pquerying_user_sid,
+							    &num_groups,
+							    &user_sids);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(1, ("fill_grent_mem_domain_users: lookup_usergroups failed "
+				  "for sid %s in domain %s (error: %s)\n", 
+				  sid_string_static(pquerying_user_sid),
+				  domain->name,
+				  nt_errstr(status)));
+		        return False;			
+		}
+
+		for (i = 0; i < num_groups; i++) {
+			if (sid_equal(group_sid, &user_sids[i])) {
+				/* User is in Domain Users, add their name
+				   as the only group member. */
+				u_in_group = True;
+				break;
+			}
+		}
+	}
+	
+	if (u_in_group) {
+		size_t len = 0;
+		char *domainname = NULL;
+		char *username = NULL;
+		fstring name;
+		enum lsa_SidType type;
+		
+		DEBUG(10,("fill_grent_mem_domain_users: sid %s in 'Domain Users' in domain %s\n",
+			  sid_string_static(pquerying_user_sid), domain->name ));
+		
+		status = domain->methods->sid_to_name(domain, mem_ctx,
+						      pquerying_user_sid,
+						      &domainname,
+						      &username,
+						      &type);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(1, ("could not lookup username for user "
+				  "sid %s in domain %s (error: %s)\n", 
+				  sid_string_static(pquerying_user_sid),
+				  domain->name,
+				  nt_errstr(status)));
+			return False;			
+		}
+		fill_domain_username(name, domain->name, username, True);
+		len = strlen(name);
+		buf_len = len + 1;
+		if (!(buf = (char *)SMB_MALLOC(buf_len))) {
+			DEBUG(1, ("out of memory\n"));
+			return False;			
+		}
+		memcpy(buf, name, buf_len);
+		
+		DEBUG(10,("fill_grent_mem_domain_users: user %s in "
+			  "'Domain Users' in domain %s\n",
+			  name, domain->name ));
+		
+		/* user is the only member */
+		*num_gr_mem = 1;
+	}
+		
+	*gr_mem = buf;
+	*gr_mem_len = buf_len;
+	
+	DEBUG(10, ("fill_grent_mem_domain_users: num_mem = %u, len = %u, mem = %s\n", 
+		   (unsigned int)*num_gr_mem, 
+		   (unsigned int)buf_len, *num_gr_mem ? buf : "NULL")); 
+
+	return True;
+}					 
+
+/***********************************************************************
+ Add names to a list.  Assumes  a canonical version of the string
+ in DOMAIN\user
+***********************************************************************/
+
+static int namecmp( const void *a, const void *b )
+{
+	return StrCaseCmp( * (char * const *) a, * (char * const *) b);	
+}
+
+static NTSTATUS add_names_to_list( TALLOC_CTX *ctx, 
+				   char ***list, uint32 *n_list, 
+				   char **names, uint32 n_names )
+{
+	char **new_list = NULL;	
+	uint32 n_new_list = 0;	
+	int i, j;	
+
+	if ( !names || (n_names == 0) )
+		return NT_STATUS_OK;
+	
+	/* Alloc the maximum size we'll need */
+
+        if ( *list == NULL ) {
+		if ( (new_list = TALLOC_ARRAY( ctx, char *, n_names )) == NULL ) 
+			return NT_STATUS_NO_MEMORY;
+		n_new_list = n_names;		
+	} else {
+		new_list = TALLOC_REALLOC_ARRAY( ctx, *list, char *, 
+						 (*n_list) + n_names );
+		if ( !new_list )
+			return NT_STATUS_NO_MEMORY;
+		n_new_list = (*n_list) + n_names;
+	}
+
+	/* Add all names */
+
+	for ( i=*n_list, j=0; i<n_new_list; i++, j++ ) {
+		new_list[i] = talloc_strdup( new_list, names[j] );
+	}
+
+	/* search for duplicates for sorting and looking for matching
+	   neighbors */
+	
+	qsort( new_list, n_new_list, sizeof(char*), QSORT_CAST namecmp );
+	
+	for ( i=1; i<n_new_list; i++ ) {
+		if ( strcmp( new_list[i-1], new_list[i] ) == 0 ) {			
+			memmove( &new_list[i-1], &new_list[i], 
+				 sizeof(char*)*(n_new_list-i) );
+			n_new_list--;
+		}
+	}
+
+	*list = new_list;
+	*n_list = n_new_list;	
+
+	return NT_STATUS_OK;	
+}
+
+/***********************************************************************
+***********************************************************************/
+
+static NTSTATUS expand_groups( TALLOC_CTX *ctx, 
+			       struct winbindd_domain *d,
+			       DOM_SID *glist, uint32 n_glist,
+			       DOM_SID **new_glist, uint32 *n_new_glist,
+			       char ***members, uint32 *n_members )
+{
+	int i, j;	
+	NTSTATUS status;
+	uint32 num_names = 0;
+	uint32 *name_types = NULL;
+	char **names = NULL;
+	DOM_SID *sid_mem = NULL;
+	TALLOC_CTX *tmp_ctx = NULL;
+	DOM_SID *new_groups = NULL;
+	size_t new_groups_size = 0;	
+	
+	*members = NULL;
+	*n_members = 0;	
+	*new_glist = NULL;
+	*n_new_glist = 0;	
+	
+	for ( i=0; i<n_glist; i++ ) {
+		tmp_ctx = talloc_new( ctx );
+
+		/* Lookup the group membership */
+
+		status = d->methods->lookup_groupmem(d, tmp_ctx, 
+						     &glist[i], &num_names,
+						     &sid_mem, &names, 
+						     &name_types);
+		if ( !NT_STATUS_IS_OK(status) ) 
+			goto out;
+				
+		/* Separate users and groups into two lists */
+
+		for ( j=0; j<num_names; j++ ) {
+
+			/* Users */
+			if ( name_types[j] == SID_NAME_USER ||
+			     name_types[j] == SID_NAME_COMPUTER )
+			{
+				status = add_names_to_list( ctx, members, 
+							    n_members,
+							    names+j, 1 );
+				if ( !NT_STATUS_IS_OK(status) )
+					goto out;
+
+				continue;				
+			} 
+
+			/* Groups */
+			if ( name_types[j] == SID_NAME_DOM_GRP ||
+			     name_types[j] == SID_NAME_ALIAS )
+			{
+				BOOL ret;
+				
+				ret = add_sid_to_array_unique( ctx, 
+							       &sid_mem[j],
+							       &new_groups, 
+							       &new_groups_size );
+				if ( !ret ) {
+					status = NT_STATUS_NO_MEMORY;
+					goto out;
+				}
+
+				continue;				
+			}
+		}
+
+		TALLOC_FREE( tmp_ctx );
+	}
+
+	new_glist = &new_groups;
+	*n_new_glist = (uint32)new_groups_size;	
+	
+ out:
+	TALLOC_FREE( tmp_ctx );
+	
+	return status;	
+}
+
+/***********************************************************************
+ Fill in the group membership field of a NT group given by group_sid
+***********************************************************************/
 
 static BOOL fill_grent_mem(struct winbindd_domain *domain,
 			   struct winbindd_cli_state *state,
@@ -203,25 +475,24 @@ static BOOL fill_grent_mem(struct winbindd_domain *domain,
 			   enum lsa_SidType group_name_type, 
 			   size_t *num_gr_mem, char **gr_mem, size_t *gr_mem_len)
 {
-	DOM_SID *sid_mem = NULL;
 	uint32 num_names = 0;
-	uint32 *name_types = NULL;
 	unsigned int buf_len = 0, buf_ndx = 0, i;
 	char **names = NULL, *buf = NULL;
 	BOOL result = False;
 	TALLOC_CTX *mem_ctx;
-	NTSTATUS status;
 	uint32 group_rid;
-	fstring sid_string;
+	DOM_SID *glist = NULL;
+	DOM_SID *new_glist = NULL;
+	uint32 n_glist, n_new_glist;	
+	int max_depth = lp_winbind_expand_groups();	
 
 	if (!(mem_ctx = talloc_init("fill_grent_mem(%s)", domain->name)))
 		return False;
 
-	/* Initialise group membership information */
-	
-	DEBUG(10, ("group SID %s\n", sid_to_string(sid_string, group_sid)));
+	DEBUG(10, ("group SID %s\n", sid_string_static(group_sid)));
 
 	/* Initialize with no members */
+
 	*num_gr_mem = 0;
 
 	/* HACK ALERT!! This whole routine does not cope with group members
@@ -234,176 +505,91 @@ static BOOL fill_grent_mem(struct winbindd_domain *domain,
 					       gr_mem, gr_mem_len);
 		goto done;
 	}
-	
+
+	/* Verify name type */
+
 	if ( !((group_name_type==SID_NAME_DOM_GRP) ||
-		((group_name_type==SID_NAME_ALIAS) && domain->primary)) )
+	       ((group_name_type==SID_NAME_ALIAS) && domain->primary)) )
 	{
 		DEBUG(1, ("SID %s in domain %s isn't a domain group (%d)\n", 
-			  sid_to_string(sid_string, group_sid), domain->name, 
+			  sid_string_static(group_sid), domain->name, 
 			  group_name_type));
                 goto done;
 	}
 
-	/* OPTIMIZATION / HACK. */
-	/* If "enum users" is set to false, and the group being looked
-	   up is the Domain Users SID: S-1-5-domain-513, then for the
-	   list of members check if the querying user is in that group,
-	   and if so only return that user as the gr_mem array.
-	   We can change this to a different parameter than "enum users"
-	   if neccessaey, or parameterize the group list we do this for. */
+	/* OPTIMIZATION / HACK. See comment in
+	   fill_grent_mem_domusers() */
 
 	sid_peek_rid( group_sid, &group_rid );
 	if (!lp_winbind_enum_users() && group_rid == DOMAIN_GROUP_RID_USERS) {
-		DOM_SID querying_user_sid;
-		DOM_SID *pquerying_user_sid = NULL;
-		uint32 num_groups = 0;
-		DOM_SID *user_sids = NULL;
-		BOOL u_in_group = False;
-
-		DEBUG(10,("fill_grent_mem: optimized lookup for sid %s domain %s\n",
-			sid_to_string(sid_string, group_sid), domain->name ));
-
-		if (state) {
-			uid_t ret_uid = (uid_t)-1;
-			if (sys_getpeereid(state->sock, &ret_uid)==0) {
-				/* We know who's asking - look up their SID if
-				   it's one we've mapped before. */
-				status = idmap_uid_to_sid(&querying_user_sid, ret_uid);
-				if (NT_STATUS_IS_OK(status)) {
-					pquerying_user_sid = &querying_user_sid;
-					DEBUG(10,("fill_grent_mem: querying uid %u -> %s\n",
-						(unsigned int)ret_uid,
-						sid_to_string(sid_string, pquerying_user_sid) ));
-				}
-			}
-		}
-
-		/* Only look up if it was a winbindd user in this domain. */
-		if (pquerying_user_sid &&
-				(sid_compare_domain(pquerying_user_sid, &domain->sid) == 0)) {
-
-			DEBUG(10,("fill_grent_mem: querying user = %s\n",
-				sid_to_string(sid_string, pquerying_user_sid) ));
-
-			status = domain->methods->lookup_usergroups(domain,
-							mem_ctx,
-							pquerying_user_sid,
-							&num_groups,
-							&user_sids);
-			if (!NT_STATUS_IS_OK(status)) {
-				DEBUG(1, ("fill_grent_mem: lookup_usergroups failed "
-					"for sid %s in domain %s (error: %s)\n", 
-					sid_to_string(sid_string, pquerying_user_sid),
-					domain->name,
-					nt_errstr(status)));
-				goto done;
-			}
+		result = fill_grent_mem_domusers( mem_ctx, domain, state, 
+						  group_sid, group_name_type,
+						  num_gr_mem, gr_mem,
+						  gr_mem_len );
+		goto done;		
+	}
 
-			for (i = 0; i < num_groups; i++) {
-				if (sid_equal(group_sid, &user_sids[i])) {
-					/* User is in Domain Users, add their name
-					   as the only group member. */
-					u_in_group = True;
-					break;
-				}
-			}
-		}
+	/* Real work goes here.  Create a list of group names to
+	   expand startign with the initial one.  Pass that to 
+	   expand_groups() which returns a list of more group names
+	   to expand.  Do this up to the max search depth. */
 
-		if (u_in_group) {
-			size_t len = 0;
-			char *domainname = NULL;
-			char *username = NULL;
-			fstring name;
-			enum lsa_SidType type;
+	if ( (glist = TALLOC_ARRAY(mem_ctx, DOM_SID, 1 )) == NULL ) {
+		result = False;
+		DEBUG(0,("fill_grent_mem: talloc failure!\n"));
+		goto done;
+	}
+	sid_copy( &glist[0], group_sid );	
+	n_glist = 1;	
 
-			DEBUG(10,("fill_grent_mem: sid %s in 'Domain Users' in domain %s\n",
-				sid_to_string(sid_string, pquerying_user_sid), domain->name ));
-
-			status = domain->methods->sid_to_name(domain, mem_ctx,
-								pquerying_user_sid,
-								&domainname,
-								&username,
-								&type);
-			if (!NT_STATUS_IS_OK(status)) {
-				DEBUG(1, ("could not lookup username for user "
-					"sid %s in domain %s (error: %s)\n", 
-					sid_to_string(sid_string, pquerying_user_sid),
-					domain->name,
-					nt_errstr(status)));
-				goto done;
-			}
-			fill_domain_username(name, domain->name, username, True);
-			len = strlen(name);
-			buf_len = len + 1;
-			if (!(buf = (char *)SMB_MALLOC(buf_len))) {
-				DEBUG(1, ("out of memory\n"));
-				goto done;
-			}
-			memcpy(buf, name, buf_len);
+	for ( i=0; i<max_depth && glist; i++ ) {
+		size_t n_members = 0;
+		char **members = NULL;
+		NTSTATUS nt_status;		
 
-			DEBUG(10,("fill_grent_mem: user %s in 'Domain Users' in domain %s\n",
-				name, domain->name ));
+		nt_status = expand_groups( mem_ctx, domain,
+					   glist, n_glist, 
+					   &new_glist, &n_new_glist,
+					   &members, &n_members);
+		if ( !NT_STATUS_IS_OK(nt_status) ) {
+			result = False;			
+			goto done;
+		}		
+		
+		/* Add new group members to list */
 
-			/* user is the only member */
-			*num_gr_mem = 1;
+		nt_status = add_names_to_list( mem_ctx, &names, &num_names, 
+					       members, n_members );
+		if ( !NT_STATUS_IS_OK(nt_status) ) {
+			result = False;			
+			goto done;
 		}
 		
-		*gr_mem = buf;
-		*gr_mem_len = buf_len;
+		TALLOC_FREE( members );		
 
-		DEBUG(10, ("num_mem = %u, len = %u, mem = %s\n", (unsigned int)*num_gr_mem, 
-		   (unsigned int)buf_len, *num_gr_mem ? buf : "NULL")); 
-		result = True;
-		goto done;
-	}
-
-	/* Lookup group members */
-	status = domain->methods->lookup_groupmem(domain, mem_ctx, group_sid, &num_names, 
-						  &sid_mem, &names, &name_types);
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(1, ("could not lookup membership for group sid %s in domain %s (error: %s)\n", 
-			  sid_to_string(sid_string, group_sid), domain->name, nt_errstr(status)));
-		goto done;
-	}
+		/* If we have no more groups to expand, break out
+		   early */
 
-	DEBUG(10, ("looked up %d names\n", num_names));
+		if ( !&new_glist )
+			break;
 
-	if (DEBUGLEVEL >= 10) {
-		for (i = 0; i < num_names; i++)
-			DEBUG(10, ("\t%20s %s %d\n", names[i],
-				   sid_string_static(&sid_mem[i]),
-				   name_types[i]));
+		/* One more round */
+		TALLOC_FREE(glist);
+		glist = new_glist;
 	}
-
-	/* Add members to list */
+	TALLOC_FREE( glist );	
+	 
+	DEBUG(10, ("looked up %d names\n", num_names));
 
  again:
+	/* Add members to list */
 
 	for (i = 0; i < num_names; i++) {
-		char *the_name;
-		fstring name;
 		int len;
 			
-		the_name = names[i];
+		DEBUG(10, ("processing name %s\n", names[i]));
 
-		DEBUG(10, ("processing name %s\n", the_name));
-
-		/* FIXME: need to cope with groups within groups.  These
-                   occur in Universal groups on a Windows 2000 native mode
-                   server. */
-
-		/* make sure to allow machine accounts */
-
-		if (name_types[i] != SID_NAME_USER && name_types[i] != SID_NAME_COMPUTER) {
-			DEBUG(3, ("name %s isn't a domain user (%s)\n", the_name, sid_type_lookup(name_types[i])));
-			continue;
-		}
-
-		/* Append domain name */
-
-		fill_domain_username(name, domain->name, the_name, True);
-
-		len = strlen(name);
+		len = strlen(names[i]);
 		
 		/* Add to list or calculate buffer length */
 
@@ -412,8 +598,8 @@ static BOOL fill_grent_mem(struct winbindd_domain *domain,
 			(*num_gr_mem)++;
 			DEBUG(10, ("buf_len + %d = %d\n", len + 1, buf_len));
 		} else {
-			DEBUG(10, ("appending %s at ndx %d\n", name, buf_ndx));
-			safe_strcpy(&buf[buf_ndx], name, len);
+			DEBUG(10, ("appending %s at ndx %d\n", names[i], buf_ndx));
+			safe_strcpy(&buf[buf_ndx], names[i], len);
 			buf_ndx += len;
 			buf[buf_ndx] = ',';
 			buf_ndx++;
@@ -432,6 +618,8 @@ static BOOL fill_grent_mem(struct winbindd_domain *domain,
 		goto again;
 	}
 
+	/* Now we're done */
+
 	if (buf && buf_ndx > 0) {
 		buf[buf_ndx - 1] = '\0';
 	}
@@ -530,7 +718,7 @@ void winbindd_getgrnam(struct winbindd_cli_state *state)
         
 	winbindd_lookupname_async( state->mem_ctx, domain->name, name_group,
 				   getgrnam_recv, WINBINDD_GETGRNAM, state );
-	}
+}
 
 struct getgrsid_state {
 	struct winbindd_cli_state *state;
@@ -606,7 +794,7 @@ static void getgrsid_lookupsid_recv( void *private_data, BOOL success,
 		return;
 	}
 
-	/* eitehr it's a domain group, a domain local group, or a
+	/* either it's a domain group, a domain local group, or a
 	   local group in an internal domain */
 
  	if ( !( (name_type==SID_NAME_DOM_GRP) ||