r15698: An attempt to make the winbind lookup_usergroups() call in security=ads

more scalable:

The most efficient way is to use the "tokenGroups" attribute which gives
the nested group membership. As this attribute can not always be
retrieved when binding with the machine account (the only garanteed way
to get the tokenGroups I could find is when the machine account is a
member of the "Pre Win2k Access" builtin group).

Our current fallback when "tokenGroups" failed is looking for all groups
where the userdn was in the "member" attribute. This behaves not very
well in very large AD domains.

The patch first tries the "memberOf" attribute on the user's dn in that
case and directly retrieves the group's sids by using the LDAP Extended
DN control from the user's object.

The way to pass down the control to the ldap search call is rather
painfull and probably will be rearranged later on.

Successfully tested on win2k sp0, win2k sp4, wink3 sp1 and win2k3 r2.

Guenther
(This used to be commit 7d766b5505e4099ef7dd4e88bb000ebe38d71bd0)

5 files changed, 369 insertions, 38 deletions

diff --git a/source3/include/ads.h b/source3/include/ads.h
index e93d36a5ca..210c6c80f1 100644
--- a/source3/include/ads.h
+++ b/source3/include/ads.h
@@ -107,6 +107,7 @@ typedef void **ADS_MODLIST;
 #define ADS_SERVER_SORT_OID 	"1.2.840.113556.1.4.473"
 #define ADS_PERMIT_MODIFY_OID 	"1.2.840.113556.1.4.1413"
 #define ADS_ASQ_OID		"1.2.840.113556.1.4.1504"
+#define ADS_EXTENDED_DN_OID	"1.2.840.113556.1.4.529"
 
 /* ldap attribute oids (Services for Unix) */
 #define ADS_ATTR_SFU_UIDNUMBER_OID 	"1.2.840.113556.1.6.18.1.310"
@@ -289,3 +290,17 @@ typedef struct {
 #endif
 } smb_krb5_addresses;
 #endif
+
+enum ads_extended_dn_flags {
+	ADS_EXTENDED_DN_HEX_STRING	= 0,
+	ADS_EXTENDED_DN_STRING		= 1 /* not supported on win2k */
+};
+
+/* this is probably not very well suited to pass other controls generically but
+ * is good enough for the extended dn control where it is only used for atm */
+
+typedef struct {
+	const char *control;
+	int val;
+	int critical;
+} ads_control;
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index 4a14527a22..293163c05e 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -445,21 +445,25 @@ static char **ads_pull_strvals(TALLOC_CTX *ctx, const char **in_vals)
  * @param cookie The paged results cookie to be returned on subsequent calls
  * @return status of search
  **/
-ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
-			       int scope, const char *expr,
-			       const char **attrs, void **res, 
-			       int *count, void **cookie)
+ADS_STATUS ads_do_paged_search_args(ADS_STRUCT *ads, const char *bind_path,
+				    int scope, const char *expr,
+				    const char **attrs, void *args, void **res, 
+				    int *count, void **cookie)
 {
 	int rc, i, version;
 	char *utf8_expr, *utf8_path, **search_attrs;
-	LDAPControl PagedResults, NoReferrals, *controls[3], **rcontrols; 
+	LDAPControl PagedResults, NoReferrals, ExtendedDn, *controls[4], **rcontrols;
 	BerElement *cookie_be = NULL;
 	struct berval *cookie_bv= NULL;
+	BerElement *extdn_be = NULL;
+	struct berval *extdn_bv= NULL;
+
 	TALLOC_CTX *ctx;
+	ads_control *external_control = (ads_control *) args;
 
 	*res = NULL;
 
-	if (!(ctx = talloc_init("ads_do_paged_search")))
+	if (!(ctx = talloc_init("ads_do_paged_search_args")))
 		return ADS_ERROR(LDAP_NO_MEMORY);
 
 	/* 0 means the conversion worked but the result was empty 
@@ -509,10 +513,47 @@ ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
 	NoReferrals.ldctl_value.bv_len = 0;
 	NoReferrals.ldctl_value.bv_val = CONST_DISCARD(char *, "");
 
+	if (external_control && strequal(external_control->control, ADS_EXTENDED_DN_OID)) {
+
+		ExtendedDn.ldctl_oid = CONST_DISCARD(char *, external_control->control);
+		ExtendedDn.ldctl_iscritical = (char) external_control->critical;
+
+		/* win2k does not accept a ldctl_value beeing passed in */
+
+		if (external_control->val != 0) {
+
+			if ((extdn_be = ber_alloc_t(LBER_USE_DER)) == NULL ) {
+				rc = LDAP_NO_MEMORY;
+				goto done;
+			}
+
+			if ((ber_printf(extdn_be, "{i}", (ber_int_t) external_control->val)) == -1) {
+				rc = LDAP_NO_MEMORY;
+				goto done;
+			}
+			if ((ber_flatten(extdn_be, &extdn_bv)) == -1) {
+				rc = LDAP_NO_MEMORY;
+				goto done;
+			}
+
+			ExtendedDn.ldctl_value.bv_len = extdn_bv->bv_len;
+			ExtendedDn.ldctl_value.bv_val = extdn_bv->bv_val;
+
+		} else {
+			ExtendedDn.ldctl_value.bv_len = 0;
+			ExtendedDn.ldctl_value.bv_val = CONST_DISCARD(char *, "");
+		}
 
-	controls[0] = &NoReferrals;
-	controls[1] = &PagedResults;
-	controls[2] = NULL;
+		controls[0] = &NoReferrals;
+		controls[1] = &PagedResults;
+		controls[2] = &ExtendedDn;
+		controls[3] = NULL;
+
+	} else {
+		controls[0] = &NoReferrals;
+		controls[1] = &PagedResults;
+		controls[2] = NULL;
+	}
 
 	/* we need to disable referrals as the openldap libs don't
 	   handle them and paged results at the same time.  Using them
@@ -533,7 +574,7 @@ ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
 	ber_bvfree(cookie_bv);
 
 	if (rc) {
-		DEBUG(3,("ads_do_paged_search: ldap_search_with_timeout(%s) -> %s\n", expr,
+		DEBUG(3,("ads_do_paged_search_args: ldap_search_with_timeout(%s) -> %s\n", expr,
 			 ldap_err2string(rc)));
 		goto done;
 	}
@@ -565,12 +606,29 @@ ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
 
 done:
 	talloc_destroy(ctx);
+
+	if (extdn_be) {
+		ber_free(extdn_be, 1);
+	}
+
+	if (extdn_bv) {
+		ber_bvfree(extdn_bv);
+	}
+ 
 	/* if/when we decide to utf8-encode attrs, take out this next line */
 	str_list_free(&search_attrs);
 
 	return ADS_ERROR(rc);
 }
 
+ADS_STATUS ads_do_paged_search(ADS_STRUCT *ads, const char *bind_path,
+			       int scope, const char *expr,
+			       const char **attrs, void **res, 
+			       int *count, void **cookie)
+{
+	return ads_do_paged_search_args(ads, bind_path, scope, expr, attrs, NULL, res, count, cookie);
+}
+
 
 /**
  * Get all results for a search.  This uses ads_do_paged_search() to return 
@@ -583,16 +641,16 @@ done:
  * @param res ** which will contain results - free res* with ads_msgfree()
  * @return status of search
  **/
-ADS_STATUS ads_do_search_all(ADS_STRUCT *ads, const char *bind_path,
-			     int scope, const char *expr,
-			     const char **attrs, void **res)
+ADS_STATUS ads_do_search_all_args(ADS_STRUCT *ads, const char *bind_path,
+				  int scope, const char *expr,
+				  const char **attrs, void *args, void **res)
 {
 	void *cookie = NULL;
 	int count = 0;
 	ADS_STATUS status;
 
 	*res = NULL;
-	status = ads_do_paged_search(ads, bind_path, scope, expr, attrs, res,
+	status = ads_do_paged_search_args(ads, bind_path, scope, expr, attrs, args, res,
 				     &count, &cookie);
 
 	if (!ADS_ERR_OK(status)) 
@@ -604,8 +662,8 @@ ADS_STATUS ads_do_search_all(ADS_STRUCT *ads, const char *bind_path,
 		ADS_STATUS status2;
 		LDAPMessage *msg, *next;
 
-		status2 = ads_do_paged_search(ads, bind_path, scope, expr, 
-					      attrs, &res2, &count, &cookie);
+		status2 = ads_do_paged_search_args(ads, bind_path, scope, expr, 
+					      attrs, args, &res2, &count, &cookie);
 
 		if (!ADS_ERR_OK(status2)) break;
 
@@ -626,6 +684,13 @@ ADS_STATUS ads_do_search_all(ADS_STRUCT *ads, const char *bind_path,
 	return status;
 }
 
+ADS_STATUS ads_do_search_all(ADS_STRUCT *ads, const char *bind_path,
+			     int scope, const char *expr,
+			     const char **attrs, void **res)
+{
+	return ads_do_search_all_args(ads, bind_path, scope, expr, attrs, NULL, res);
+}
+
 /**
  * Run a function on all results for a search.  Uses ads_do_paged_search() and
  *  runs the function as each page is returned, using ads_process_results()
@@ -2580,4 +2645,127 @@ ADS_STATUS ads_upn_suffixes(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, char **suffixe
 	return status;
 }
 
+/**
+ * pull a DOM_SID from an extended dn string
+ * @param mem_ctx TALLOC_CTX 
+ * @param flags string type of extended_dn
+ * @param sid pointer to a DOM_SID
+ * @return boolean inidicating success
+ **/
+BOOL ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, 
+				  const char *dn, 
+				  enum ads_extended_dn_flags flags, 
+				  DOM_SID *sid)
+{
+	char *p, *q;
+
+	if (!dn) {
+		return False;
+	}
+
+	/* 
+	 * ADS_EXTENDED_DN_HEX_STRING:
+	 * <GUID=238e1963cb390f4bb032ba0105525a29>;<SID=010500000000000515000000bb68c8fd6b61b427572eb04556040000>;CN=gd,OU=berlin,OU=suse,DC=ber,DC=suse,DC=de
+	 *
+	 * ADS_EXTENDED_DN_STRING (only with w2k3):
+	<GUID=63198e23-39cb-4b0f-b032-ba0105525a29>;<SID=S-1-5-21-4257769659-666132843-1169174103-1110>;CN=gd,OU=berlin,OU=suse,DC=ber,DC=suse,DC=de
+	 */
+
+	p = strchr(dn, ';');
+	if (!p) {
+		return False;
+	}
+
+	if (strncmp(p, ";<SID=", strlen(";<SID=")) != 0) {
+		return False;
+	}
+
+	p += strlen(";<SID=");
+
+	q = strchr(p, '>');
+	if (!q) {
+		return False;
+	}
+	
+	*q = '\0';
+
+	DEBUG(100,("ads_get_sid_from_extended_dn: sid string is %s\n", p));
+
+	switch (flags) {
+	
+	case ADS_EXTENDED_DN_STRING:
+		if (!string_to_sid(sid, p)) {
+			return False;
+		}
+		break;
+	case ADS_EXTENDED_DN_HEX_STRING: {
+		pstring buf;
+		size_t buf_len;
+
+		buf_len = strhex_to_str(buf, strlen(p), p);
+		if (buf_len == 0) {
+			return False;
+		}
+
+		if (!sid_parse(buf, buf_len, sid)) {
+			DEBUG(10,("failed to parse sid\n"));
+			return False;
+		}
+		break;
+		}
+	default:
+		DEBUG(10,("unknown extended dn format\n"));
+		return False;
+	}
+
+	return True;
+}
+
+/**
+ * pull an array of DOM_SIDs from a ADS result
+ * @param ads connection to ads server
+ * @param mem_ctx TALLOC_CTX for allocating sid array
+ * @param msg Results of search
+ * @param field Attribute to retrieve
+ * @param flags string type of extended_dn
+ * @param sids pointer to sid array to allocate
+ * @return the count of SIDs pulled
+ **/
+int ads_pull_sids_from_extendeddn(ADS_STRUCT *ads, 
+				  TALLOC_CTX *mem_ctx, 
+				  void *msg, 
+				  const char *field,
+				  enum ads_extended_dn_flags flags,
+				  DOM_SID **sids)
+{
+	int i;
+	size_t dn_count;
+	char **dn_strings;
+
+	if ((dn_strings = ads_pull_strings(ads, mem_ctx, msg, field, 
+					   &dn_count)) == NULL) {
+		return 0;
+	}
+
+	(*sids) = TALLOC_ZERO_ARRAY(mem_ctx, DOM_SID, dn_count + 1);
+	if (!(*sids)) {
+		TALLOC_FREE(dn_strings);
+		return 0;
+	}
+
+	for (i=0; i<dn_count; i++) {
+
+		if (!ads_get_sid_from_extended_dn(mem_ctx, dn_strings[i], 
+						  flags, &(*sids)[i])) {
+			TALLOC_FREE(*sids);
+			TALLOC_FREE(dn_strings);
+			return 0;
+		}
+	}
+
+	TALLOC_FREE(dn_strings);
+
+	return dn_count;
+}
+
 #endif
diff --git a/source3/libads/ldap_utils.c b/source3/libads/ldap_utils.c
index 58f1c20ad2..fe0c659b04 100644
--- a/source3/libads/ldap_utils.c
+++ b/source3/libads/ldap_utils.c
@@ -27,9 +27,9 @@
   a wrapper around ldap_search_s that retries depending on the error code
   this is supposed to catch dropped connections and auto-reconnect
 */
-ADS_STATUS ads_do_search_retry(ADS_STRUCT *ads, const char *bind_path, int scope, 
-			       const char *expr,
-			       const char **attrs, void **res)
+static ADS_STATUS ads_do_search_retry_internal(ADS_STRUCT *ads, const char *bind_path, int scope, 
+					       const char *expr,
+					       const char **attrs, void *args, void **res)
 {
 	ADS_STATUS status = ADS_SUCCESS;
 	int count = 3;
@@ -49,7 +49,7 @@ ADS_STATUS ads_do_search_retry(ADS_STRUCT *ads, const char *bind_path, int scope
 	}
 
 	*res = NULL;
-	status = ads_do_search_all(ads, bp, scope, expr, attrs, res);
+	status = ads_do_search_all_args(ads, bp, scope, expr, attrs, args, res);
 	if (ADS_ERR_OK(status)) {
 		DEBUG(5,("Search for %s gave %d replies\n",
 			 expr, ads_count_replies(ads, *res)));
@@ -82,7 +82,7 @@ ADS_STATUS ads_do_search_retry(ADS_STRUCT *ads, const char *bind_path, int scope
 		}
 
 		*res = NULL;
-		status = ads_do_search_all(ads, bp, scope, expr, attrs, res);
+		status = ads_do_search_all_args(ads, bp, scope, expr, attrs, args, res);
 		if (ADS_ERR_OK(status)) {
 			DEBUG(5,("Search for %s gave %d replies\n",
 				 expr, ads_count_replies(ads, *res)));
@@ -99,6 +99,20 @@ ADS_STATUS ads_do_search_retry(ADS_STRUCT *ads, const char *bind_path, int scope
 	return status;
 }
 
+ADS_STATUS ads_do_search_retry(ADS_STRUCT *ads, const char *bind_path, int scope, 
+			       const char *expr,
+			       const char **attrs, void **res)
+{
+	return ads_do_search_retry_internal(ads, bind_path, scope, expr, attrs, NULL, res);
+}
+
+ADS_STATUS ads_do_search_retry_args(ADS_STRUCT *ads, const char *bind_path, int scope, 
+				    const char *expr,
+				    const char **attrs, void *args, void **res)
+{
+	return ads_do_search_retry_internal(ads, bind_path, scope, expr, attrs, args, res);
+}
+
 
 ADS_STATUS ads_search_retry(ADS_STRUCT *ads, void **res, 
 			    const char *expr, 
@@ -116,6 +130,21 @@ ADS_STATUS ads_search_retry_dn(ADS_STRUCT *ads, void **res,
 				   "(objectclass=*)", attrs, res);
 }
 
+ADS_STATUS ads_search_retry_extended_dn(ADS_STRUCT *ads, void **res, 
+					const char *dn, 
+					const char **attrs,
+					enum ads_extended_dn_flags flags)
+{
+	ads_control args;
+
+	args.control = ADS_EXTENDED_DN_OID;
+	args.val = flags;
+	args.critical = True;
+
+	return ads_do_search_retry_args(ads, dn, LDAP_SCOPE_BASE,
+					"(objectclass=*)", attrs, &args, res);
+}
+
 ADS_STATUS ads_search_retry_sid(ADS_STRUCT *ads, void **res, 
 				const DOM_SID *sid,
 				const char **attrs)
diff --git a/source3/nsswitch/winbindd_ads.c b/source3/nsswitch/winbindd_ads.c
index 3dc26f4cfa..8259fd7cd3 100644
--- a/source3/nsswitch/winbindd_ads.c
+++ b/source3/nsswitch/winbindd_ads.c
@@ -542,11 +542,11 @@ done:
 
 /* Lookup groups a user is a member of - alternate method, for when
    tokenGroups are not available. */
-static NTSTATUS lookup_usergroups_alt(struct winbindd_domain *domain,
-				      TALLOC_CTX *mem_ctx,
-				      const char *user_dn, 
-				      DOM_SID *primary_group,
-				      size_t *p_num_groups, DOM_SID **user_sids)
+static NTSTATUS lookup_usergroups_member(struct winbindd_domain *domain,
+					 TALLOC_CTX *mem_ctx,
+					 const char *user_dn, 
+					 DOM_SID *primary_group,
+					 size_t *p_num_groups, DOM_SID **user_sids)
 {
 	ADS_STATUS rc;
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
@@ -559,7 +559,7 @@ static NTSTATUS lookup_usergroups_alt(struct winbindd_domain *domain,
 	char *escaped_dn;
 	size_t num_groups = 0;
 
-	DEBUG(3,("ads: lookup_usergroups_alt\n"));
+	DEBUG(3,("ads: lookup_usergroups_member\n"));
 
 	ads = ads_cached_connection(domain);
 
@@ -573,9 +573,6 @@ static NTSTATUS lookup_usergroups_alt(struct winbindd_domain *domain,
 		goto done;
 	}
 
-	/* buggy server, no tokenGroups.  Instead lookup what groups this user
-	   is a member of by DN search on member*/
-
 	if (!(ldap_exp = talloc_asprintf(mem_ctx, "(&(member=%s)(objectCategory=group))", escaped_dn))) {
 		DEBUG(1,("lookup_usergroups(dn=%s) asprintf failed!\n", user_dn));
 		SAFE_FREE(escaped_dn);
@@ -624,7 +621,7 @@ static NTSTATUS lookup_usergroups_alt(struct winbindd_domain *domain,
 	*p_num_groups = num_groups;
 	status = (user_sids != NULL) ? NT_STATUS_OK : NT_STATUS_NO_MEMORY;
 
-	DEBUG(3,("ads lookup_usergroups (alt) for dn=%s\n", user_dn));
+	DEBUG(3,("ads lookup_usergroups (member) succeeded for dn=%s\n", user_dn));
 done:
 	if (res) 
 		ads_msgfree(ads, res);
@@ -632,6 +629,89 @@ done:
 	return status;
 }
 
+/* Lookup groups a user is a member of - alternate method, for when
+   tokenGroups are not available. */
+static NTSTATUS lookup_usergroups_memberof(struct winbindd_domain *domain,
+					   TALLOC_CTX *mem_ctx,
+					   const char *user_dn, 
+					   DOM_SID *primary_group,
+					   size_t *p_num_groups, DOM_SID **user_sids)
+{
+	ADS_STATUS rc;
+	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+	int count;
+	void *res = NULL;
+	ADS_STRUCT *ads;
+	const char *attrs[] = {"memberOf", NULL};
+	size_t num_groups = 0;
+	DOM_SID *group_sids = NULL;
+	int i;
+
+	DEBUG(3,("ads: lookup_usergroups_memberof\n"));
+
+	ads = ads_cached_connection(domain);
+
+	if (!ads) {
+		domain->last_status = NT_STATUS_SERVER_DISABLED;
+		goto done;
+	}
+
+	rc = ads_search_retry_extended_dn(ads, &res, user_dn, attrs, 
+					  ADS_EXTENDED_DN_HEX_STRING);
+	
+	if (!ADS_ERR_OK(rc) || !res) {
+		DEBUG(1,("lookup_usergroups_memberof ads_search member=%s: %s\n", 
+			user_dn, ads_errstr(rc)));
+		return ads_ntstatus(rc);
+	}
+	
+	count = ads_count_replies(ads, res);
+
+	if (count == 0) {
+		status = NT_STATUS_NO_SUCH_USER;
+		goto done;
+	}
+	
+	*user_sids = NULL;
+	num_groups = 0;
+
+	/* always add the primary group to the sid array */
+	add_sid_to_array(mem_ctx, primary_group, user_sids, &num_groups);
+
+	count = ads_pull_sids_from_extendeddn(ads, mem_ctx, res, "memberOf", 
+					      ADS_EXTENDED_DN_HEX_STRING, 
+					      &group_sids);
+	if (count == 0) {
+		DEBUG(1,("No memberOf for this user?!?\n"));
+		status = NT_STATUS_NO_MEMORY;
+		goto done;
+	}
+
+	for (i=0; i<count; i++) {
+
+		/* ignore Builtin groups from ADS - Guenther */
+		if (sid_check_is_in_builtin(&group_sids[i])) {
+			continue;
+		}
+		       
+		add_sid_to_array(mem_ctx, &group_sids[i], user_sids,
+				 &num_groups);
+	
+	}
+
+	*p_num_groups = num_groups;
+	status = (user_sids != NULL) ? NT_STATUS_OK : NT_STATUS_NO_MEMORY;
+
+	DEBUG(3,("ads lookup_usergroups (memberof) succeeded for dn=%s\n", user_dn));
+done:
+	TALLOC_FREE(group_sids);
+	if (res) 
+		ads_msgfree(ads, res);
+
+	return status;
+}
+
+
 /* Lookup groups a user is a member of. */
 static NTSTATUS lookup_usergroups(struct winbindd_domain *domain,
 				  TALLOC_CTX *mem_ctx,
@@ -717,13 +797,29 @@ static NTSTATUS lookup_usergroups(struct winbindd_domain *domain,
 	/* there must always be at least one group in the token, 
 	   unless we are talking to a buggy Win2k server */
 
+	/* actually this only happens when the machine account has no read
+	 * permissions on the tokenGroup attribute - gd */
+
 	if (count == 0) {
 
-		status = lookup_usergroups_alt(domain, mem_ctx, user_dn, 
-					       &primary_group,
-					       &num_groups, user_sids);
-		*p_num_groups = (uint32)num_groups;
-		return status;
+		/* no tokenGroups */
+		
+		/* lookup what groups this user is a member of by DN search on
+		 * "memberOf" */
+
+		status = lookup_usergroups_memberof(domain, mem_ctx, user_dn,
+						    &primary_group,
+						    p_num_groups, user_sids);
+		if (NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+
+		/* lookup what groups this user is a member of by DN search on
+		 * "member" */
+
+		return lookup_usergroups_member(domain, mem_ctx, user_dn, 
+						&primary_group,
+						p_num_groups, user_sids);
 	}
 
 	*user_sids = NULL;
@@ -745,7 +841,7 @@ static NTSTATUS lookup_usergroups(struct winbindd_domain *domain,
 	*p_num_groups = (uint32)num_groups;
 	status = (user_sids != NULL) ? NT_STATUS_OK : NT_STATUS_NO_MEMORY;
 
-	DEBUG(3,("ads lookup_usergroups for sid=%s\n",
+	DEBUG(3,("ads lookup_usergroups (tokenGroups) succeeded for sid=%s\n",
 		 sid_to_string(sid_string, sid)));
 done:
 	return status;
diff --git a/source3/nsswitch/winbindd_util.c b/source3/nsswitch/winbindd_util.c
index 82fd1e128b..4e5075d89d 100644
--- a/source3/nsswitch/winbindd_util.c
+++ b/source3/nsswitch/winbindd_util.c
@@ -1247,6 +1247,7 @@ NTSTATUS lookup_usergroups_cached(struct winbindd_domain *domain,
 	
 	*user_sids = NULL;
 	num_groups = 0;
+	*p_num_groups = 0;
 
 	info3 = netsamlogon_cache_get(mem_ctx, user_sid);
 
@@ -1275,6 +1276,8 @@ NTSTATUS lookup_usergroups_cached(struct winbindd_domain *domain,
 	SAFE_FREE(info3);
 	*p_num_groups = num_groups;
 	status = (user_sids != NULL) ? NT_STATUS_OK : NT_STATUS_NO_MEMORY;
-		
+	
+	DEBUG(3,(": lookup_usergroups_cached succeeded\n"));
+
 	return status;
 }