winbind cache issue for NDR entries

[Jeremy Allison]

On Fri, Jan 29, 2021 at 07:39:40PM +0530, Shilpa K via samba-technical wrote:
>Hello,
>
>We had a customer report that the users were not able to login for about
>30minutes and the problem cleared itself in almost about 30minutes. They
>are using Samba as a member server in a domain which has 2 way trust with
>another domain (say ABC.COM). Upon investigation, we found that there was a
>problem with trusted domain DCs for a very short duration as per the event
>log on the DC of the primary domain. This problem seems to have been
>cleared away after a short duration. Around the same time, a user belonging
>to a trusted domain mapped Samba share and encountered a problem. At this
>time, looks like NDR cache entry for trusted domain group "Domain Users"
>was added in winbindd_cache.tdb to indicate that there was a lookup problem
>and the status NT_STATUS_TRUSTED_DOMAIN_FAILURE was stored as part of this
>entry. Once the issue with trusted domain DC was cleared and the domain was
>back online, when users tried to login, PAM_AUTH was successful for the
>users but getpwnam failed while looking up SID for "Domain Users". This
>failure was returned from the entry in the winbindd_cache.tdb as
>wcache_fetch_ndr() succeeded for this entry. Due to this, users belonging
>to the trusted domain were not able to login. Once the cache was expired,
>getpwnam succeeded for trusted domain users and the shares could be mapped.
>In order to resolve this issue, should we not refresh the sequence number
>when the domain goes online? Btw, we are using "winbind cache time = 1800".

Yep, looks like we should add a call to force a refresh of the
sequence number in the cache here:

source3/winbindd/winbindd_cm.c:set_domain_online()

  538 
  539         domain->online = True;
  540 

Add a force_refresh_domain_sequence_number(domain) call above.

Here is a (raw, untested) patch that implements this.

Any chance you can test this for me ?

Jeremy.
-------------- next part --------------
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 201bb34b98a..c8c10aed094 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -517,6 +517,41 @@ static bool store_cache_seqnum( struct winbindd_domain *domain )
 				   domain->last_seq_check);
 }
 
+void force_refresh_domain_sequence_number(struct winbindd_domain *domain)
+{
+	NTSTATUS status;
+
+	if (winbindd_can_contact_domain(domain)) {
+		status = domain->backend->sequence_number(domain,
+						  &domain->sequence_number);
+	} else {
+		/* just use the current time */
+		status = NT_STATUS_OK;
+		domain->sequence_number = time(NULL);
+	}
+
+	/* the above call could have set our domain->backend to NULL when
+	 * coming from offline to online mode, make sure to reinitialize the
+	 * backend - Guenther */
+	get_cache(domain);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("failed with %s\n", nt_errstr(status));
+		domain->sequence_number = DOM_SEQUENCE_NONE;
+	}
+
+	domain->last_status = status;
+	domain->last_seq_check = time(NULL);
+
+	/* save the new sequence number in the cache */
+	store_cache_seqnum(domain);
+
+	DBG_DEBUG("%s seq number is now %"PRIu32"\n",
+		   domain->name, domain->sequence_number);
+
+	return;
+}
+
 /*
   refresh the domain sequence number on timeout.
 */
@@ -561,39 +596,11 @@ static void refresh_sequence_number(struct winbindd_domain *domain)
 		goto done;
 	}
 
-	/* important! make sure that we know if this is a native 
-	   mode domain or not.  And that we can contact it. */
-
-	if ( winbindd_can_contact_domain( domain ) ) {		
-		status = domain->backend->sequence_number(domain, 
-							  &domain->sequence_number);
-	} else {
-		/* just use the current time */
-		status = NT_STATUS_OK;
-		domain->sequence_number = time(NULL);
-	}
-
-
-	/* the above call could have set our domain->backend to NULL when
-	 * coming from offline to online mode, make sure to reinitialize the
-	 * backend - Guenther */
-	get_cache( domain );
-
-	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(10,("refresh_sequence_number: failed with %s\n", nt_errstr(status)));
-		domain->sequence_number = DOM_SEQUENCE_NONE;
-	}
-
-	domain->last_status = status;
-	domain->last_seq_check = time(NULL);
-
-	/* save the new sequence number in the cache */
-	store_cache_seqnum( domain );
+	force_refresh_domain_sequence_number(domain);
 
 done:
-	DEBUG(10, ("refresh_sequence_number: %s seq number is now %d\n", 
-		   domain->name, domain->sequence_number));
-
+	DBG_DEBUG("%s seq number is now %"PRIu32"\n",
+		   domain->name, domain->sequence_number);
 	return;
 }
 
diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 2029e4e8315..b1fe4102253 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -537,6 +537,7 @@ static void set_domain_online(struct winbindd_domain *domain)
 			     MSG_WINBIND_FAILED_TO_GO_ONLINE, NULL);
 
 	domain->online = True;
+	force_refresh_domain_sequence_number(domain);
 
 	/* Send a message to the parent that the domain is online. */
 	if (parent_pid > 1 && !domain->internal) {
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 8923bb3124f..dc2dd130be8 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -53,6 +53,7 @@ NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
 
 /* The following definitions come from winbindd/winbindd_cache.c  */
 
+void force_refresh_domain_sequence_number(struct winbindd_domain *domain);
 NTSTATUS wb_cache_query_user_list(struct winbindd_domain *domain,
 				  TALLOC_CTX *mem_ctx,
 				  uint32_t **prids);