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

diff --git a/source/libads/ldap.c b/source/libads/ldap.c
index ce0341b72c0debc8818ab19295ba7ad75f862c51..f47645c7e7abc2c095eae134d673828693441479 100644 (file)
--- a/source/libads/ldap.c
+++ b/source/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/source/nsswitch/winbindd_ads.c b/source/nsswitch/winbindd_ads.c
index 2fcf02a3164ddc8edbe35e05461b4eef7ce9aa01..46f3660c4969c1266e3e8961eaa4e37b3ac0962b 100644 (file)
--- a/source/nsswitch/winbindd_ads.c
+++ b/source/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);