Fix for bug 707, getent group for huge ads groups (>1500 members)

This introduces range retrieval of ADS attributes.

I've rewritten most of Günther's patch, partly to remove code duplication and
partly to get the retrieval of members in one rush, not interrupted by the
lookups for the DN.

Andrew, you told me that you would like to see a check whether the AD sequence
number is the same before and after the retrieval to achieve atomicity. This
would be trivial to add, but I'm not sure that we want this, as this adds two
roundtrips to every membership query. We can not know before the first query
whether we get additional range values, and at that point it's too late to ask
for the USN.

Tested with a group of 4000 members along with lots of small groups.

Volker
(This used to be commit 9d8235bf413f931e40bca0c27a25ed62b4f3d226)

2 files changed, 140 insertions, 29 deletions

diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index ce0341b72c..f47645c7e7 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -1579,27 +1579,26 @@ char *ads_pull_string(ADS_STRUCT *ads,
  * @return Result strings in talloc context
  **/
 char **ads_pull_strings(ADS_STRUCT *ads, 
-		       TALLOC_CTX *mem_ctx, void *msg, const char *field)
+			TALLOC_CTX *mem_ctx, void *msg, const char *field,
+			int *num_values)
 {
 	char **values;
 	char **ret = NULL;
-	int i, n;
+	int i;
 
 	values = ldap_get_values(ads->ld, msg, field);
 	if (!values)
 		return NULL;
 
-	for (i=0;values[i];i++)
-		/* noop */ ;
-	n = i;
+	*num_values = ldap_count_values(values);
 
-	ret = talloc(mem_ctx, sizeof(char *) * (n+1));
+	ret = talloc(mem_ctx, sizeof(char *) * (*num_values+1));
 	if (!ret) {
 		ldap_value_free(values);
 		return NULL;
 	}
 
-	for (i=0;i<n;i++) {
+	for (i=0;i<*num_values;i++) {
 		if (pull_utf8_talloc(mem_ctx, &ret[i], values[i]) == -1) {
 			ldap_value_free(values);
 			return NULL;
@@ -1611,6 +1610,89 @@ char **ads_pull_strings(ADS_STRUCT *ads,
 	return ret;
 }
 
+/**
+ * pull an array of strings from a ADS result 
+ *  (handle large multivalue attributes with range retrieval)
+ * @param ads connection to ads server
+ * @param mem_ctx TALLOC_CTX to use for allocating result string
+ * @param msg Results of search
+ * @param field Attribute to retrieve
+ * @param num_values How many values did we get this time?
+ * @param more_values Are there more values to get?
+ * @return Result strings in talloc context
+ **/
+char **ads_pull_strings_range(ADS_STRUCT *ads, 
+			      TALLOC_CTX *mem_ctx,
+			      void *msg, const char *field,
+			      int *num_values,
+			      BOOL *more_values)
+{
+	char *first_attr, *second_attr;
+	char *expected_range_attrib, *range_attr;
+	BerElement *ptr = NULL;
+	char **result;
+
+	/* Get the first and second attributes. This assumes that the LDAP msg
+	 * contains the requested attributes first and then a possible
+	 * range-augmented attribute. This is the case for the group
+	 * membership query we do against windows AD, but a general
+	 * range-based value retrieval would have to be modified. */
+
+	first_attr = ldap_first_attribute(ads->ld, (LDAPMessage *)msg, &ptr);
+
+	if (first_attr == NULL)
+		return NULL;
+
+	expected_range_attrib = talloc_asprintf(mem_ctx, "%s;Range=", field);
+
+	if (!strequal(first_attr, field) &&
+	    !strnequal(first_attr, expected_range_attrib,
+		       strlen(expected_range_attrib)))
+	{
+		DEBUG(1, ("Expected attribute [%s], got [%s]\n",
+			  field, first_attr));
+		return NULL;
+	}
+
+	second_attr = ldap_next_attribute(ads->ld, (LDAPMessage *)msg, ptr);
+	ber_free(ptr, 0);
+
+	DEBUG(10,("attr: [%s], first_attr: [%s], second_attr: [%s]\n", 
+		  field, first_attr, second_attr));
+
+	if ((second_attr != NULL) &&
+	    (strnequal(second_attr, expected_range_attrib,
+		       strlen(expected_range_attrib)))) {
+
+		/* This is the first in a row of range results. We can not ask
+		 * for the attribute we wanted, as this is empty in the LDAP
+		 * msg, the delivered values are in the second range-augmented
+		 * attribute. */
+		range_attr = second_attr;
+
+	} else {
+
+		/* Upon second and subsequent requests to get attribute
+		 * values, first_attr carries the Range= specifier. */
+		range_attr = first_attr;
+
+	}
+
+	/* We have to ask for more if we have a range specifier in the
+	 * attribute and the attribute does not end in "*". */
+
+	*more_values = ( (strnequal(range_attr, expected_range_attrib,
+				    strlen(expected_range_attrib))) &&
+			 (range_attr[strlen(range_attr)-1] != '*') );
+
+	result = ads_pull_strings(ads, mem_ctx, msg, range_attr, num_values);
+
+	ldap_memfree(first_attr);
+	if (second_attr != NULL)
+		ldap_memfree(second_attr);
+
+	return result;
+}
 
 /**
  * pull a single uint32 from a ADS result
@@ -1960,6 +2042,7 @@ ADS_STATUS ads_workgroup_name(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, const char *
 	int i;
 	void *res;
 	const char *attrs[] = {"servicePrincipalName", NULL};
+	int num_principals;
 
 	(*workgroup) = NULL;
 
@@ -1972,7 +2055,8 @@ ADS_STATUS ads_workgroup_name(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, const char *
 		return rc;
 	}
 
-	principles = ads_pull_strings(ads, mem_ctx, res, "servicePrincipalName");
+	principles = ads_pull_strings(ads, mem_ctx, res,
+				      "servicePrincipalName", &num_principals);
 
 	ads_msgfree(ads, res);
 
diff --git a/source3/nsswitch/winbindd_ads.c b/source3/nsswitch/winbindd_ads.c
index 2fcf02a316..46f3660c49 100644
--- a/source3/nsswitch/winbindd_ads.c
+++ b/source3/nsswitch/winbindd_ads.c
@@ -690,10 +690,11 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain,
 	char *ldap_exp;
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
 	char *sidstr;
-	const char *attrs[] = {"member", NULL};
 	char **members;
 	int i, num_members;
 	fstring sid_string;
+	const char *attr = "member";
+	BOOL more_values;
 
 	DEBUG(10,("ads: lookup_groupmem %s sid=%s\n", domain->name, sid_string_static(group_sid)));
 
@@ -710,33 +711,56 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain,
 
 	/* search for all members of the group */
 	asprintf(&ldap_exp, "(objectSid=%s)",sidstr);
-	rc = ads_search_retry(ads, &res, ldap_exp, attrs);
-	free(ldap_exp);
-	free(sidstr);
 
-	if (!ADS_ERR_OK(rc) || !res) {
-		DEBUG(1,("query_user_list ads_search: %s\n", ads_errstr(rc)));
-		goto done;
-	}
+	members = NULL;
+	num_members = 0;
+	more_values = True;
 
-	count = ads_count_replies(ads, res);
-	if (count == 0) {
-		status = NT_STATUS_OK;
-		goto done;
-	}
+	while (more_values) {
 
-	members = ads_pull_strings(ads, mem_ctx, res, "member");
-	if (!members) {
-		/* no members? ok ... */
-		status = NT_STATUS_OK;
-		goto done;
-	}
+		int num_this_time;
+		char **new_members;
+		const char *attrs[] = {attr, NULL};
+
+		rc = ads_search_retry(ads, &res, ldap_exp, attrs);
 
+		if (!ADS_ERR_OK(rc) || !res) {
+			DEBUG(1,("query_user_list ads_search: %s\n",
+				 ads_errstr(rc)));
+			goto done;
+		}
+
+		count = ads_count_replies(ads, res);
+		if (count == 0)
+			break;
+
+		new_members = ads_pull_strings_range(ads, mem_ctx, res,
+						     "member",
+						     &num_this_time,
+						     &more_values);
+
+		if ((new_members == NULL) || (num_this_time == 0))
+			break;
+
+		members = talloc_realloc(mem_ctx, members,
+					 sizeof(*members) *
+					 (num_members+num_this_time));
+
+		if (members == NULL)
+			break;
+
+		memcpy(members+num_members, new_members,
+		       sizeof(new_members)*num_this_time);
+
+		num_members += num_this_time;
+
+		attr = talloc_asprintf(mem_ctx,
+				       "member;range=%d-*", num_members);
+	}
+		
 	/* 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;
 
 	(*sid_mem) = talloc_zero(mem_ctx, sizeof(**sid_mem) * num_members);
 	(*name_types) = talloc_zero(mem_ctx, sizeof(**name_types) * num_members);
@@ -763,6 +787,9 @@ static NTSTATUS lookup_groupmem(struct winbindd_domain *domain,
 	status = NT_STATUS_OK;
 	DEBUG(3,("ads lookup_groupmem for sid=%s\n", sid_to_string(sid_string, group_sid)));
 done:
+	SAFE_FREE(ldap_exp);
+	SAFE_FREE(sidstr);
+
 	if (res) 
 		ads_msgfree(ads, res);