[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Wed Jul 5 13:15:01 UTC 2023


The branch, master has been updated
       via  083fe1c28c6 smbd: call exit_server_cleanly() to avoid panicking
       via  50e771c12f8 s3:winbindd: let winbind_samlogon_retry_loop() fallback to NT_STATUS_NO_LOGON_SERVERS
       via  b317b10dffd s3:winbindd: make use of reset_cm_connection_on_error() in winbind_samlogon_retry_loop()
       via  0cb6de4b1d5 s3:winbindd: let winbind_samlogon_retry_loop() always start with authoritative = 1
       via  4ad5a35a3f6 s3:winbindd: make use of reset_cm_connection_on_error() for winbindd_lookup_{names,sids}()
       via  cb59fd43bbf s3:winbindd: call reset_cm_connection_on_error() in wb_cache_query_user_list()
      from  d2940694c6a ctdb-tests: Run ShellCheck on event-script unit test support scripts

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 083fe1c28c6ec69cbd15d8cc2f7f06b1b630f2bc
Author: Ralph Boehme <slow at samba.org>
Date:   Wed Jul 5 11:33:58 2023 +0200

    smbd: call exit_server_cleanly() to avoid panicking
    
    The parent smdb forwards SIGTERM to its process group in order to kill all
    children like the scavenger. This happens from a function registered via
    atexit() which means the signal forwarding is happening very briefly before the
    main smbd process exits. When exiting the pipe between smbd and scavenger is
    closed which triggers a file event in the scavenger.
    
    However, due to kernel sheduling it is possible that the file descriptor event
    is received before the signal, where we call exit_server() which call
    smb_panic() at the end.
    
    Change the exit to exit_server_cleanly() and just log this event at level 2
    which we already do.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15275
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Wed Jul  5 13:14:08 UTC 2023 on atb-devel-224

commit 50e771c12f84f9268c2e9ddeef0965f79f85de3d
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jul 4 14:12:03 2023 +0200

    s3:winbindd: let winbind_samlogon_retry_loop() fallback to NT_STATUS_NO_LOGON_SERVERS
    
    When we were not able to get a valid response from any DC we should
    report NT_STATUS_NO_LOGON_SERVERS with authoritative = 1.
    
    This matches what windows does. In a chain of transitive
    trusts the ACCESS_DENIED/authoritative=0 is not propagated,
    instead NT_STATUS_NO_LOGON_SERVERS/authoritative=1 is
    passed along the chain if there's no other DC is available.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15413
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit b317b10dffd99d1add3ff0b85b958edd9639abc8
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jul 4 13:01:24 2023 +0200

    s3:winbindd: make use of reset_cm_connection_on_error() in winbind_samlogon_retry_loop()
    
    Note this is more than a simple invalidate_cm_connection() as it may set
    domain->conn.netlogon_force_reauth = true, which is important in order
    to recover from NT_STATUS_RPC_SEC_PKG_ERROR errors.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15413
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 0cb6de4b1d5410f3699172952be81c6eb75c2c86
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Feb 16 14:19:16 2022 +0100

    s3:winbindd: let winbind_samlogon_retry_loop() always start with authoritative = 1
    
    Otherwise we could treat a local problem as non-authoritative.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15413
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 4ad5a35a3f67860aa7a1345efcfc92fe40578e31
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jul 4 12:32:34 2023 +0200

    s3:winbindd: make use of reset_cm_connection_on_error() for winbindd_lookup_{names,sids}()
    
    Note this is more than a simple invalidate_cm_connection() as it may set
    domain->conn.netlogon_force_reauth = true.
    
    This is not strictly needed as the callers call
    reset_cm_connection_on_error() via reconnect_need_retry().
    But it might avoid one roundtrip.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15413
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit cb59fd43bbf758e4bad774cfc19ef87b157052c2
Author: Stefan Metzmacher <metze at samba.org>
Date:   Tue Jul 4 12:32:34 2023 +0200

    s3:winbindd: call reset_cm_connection_on_error() in wb_cache_query_user_list()
    
    This is mostly for consistency, every remote call should call
    reset_cm_connection_on_error(). Note this is more than
    a simple invalidate_cm_connection() as it may set
    domain->conn.netlogon_force_reauth = true.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15413
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/smbd/scavenger.c          |  2 +-
 source3/winbindd/winbindd_cache.c |  1 +
 source3/winbindd/winbindd_msrpc.c | 10 ++----
 source3/winbindd/winbindd_pam.c   | 67 +++++++++++++++++++++++++--------------
 4 files changed, 47 insertions(+), 33 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/scavenger.c b/source3/smbd/scavenger.c
index 897ddf1617c..ce2a58a4ae4 100644
--- a/source3/smbd/scavenger.c
+++ b/source3/smbd/scavenger.c
@@ -104,7 +104,7 @@ static void smbd_scavenger_parent_dead(struct tevent_context *event_ctx,
 		  server_id_str_buf(*state->scavenger_id, &tmp1),
 		  server_id_str_buf(state->parent_id, &tmp2)));
 
-	exit_server("smbd_scavenger_parent_dead");
+	exit_server_cleanly("smbd_scavenger_parent_dead");
 }
 
 static void scavenger_sig_term_handler(struct tevent_context *ev,
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 7cb3e60943b..d23b3415825 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1508,6 +1508,7 @@ do_query:
 			DBG_NOTICE("query_user_list: returned 0x%08x, "
 				  "retrying\n", NT_STATUS_V(status));
 		}
+		reset_cm_connection_on_error(domain, NULL, status);
 		if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL)) {
 			DBG_NOTICE("query_user_list: flushing "
 				  "connection cache\n");
diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c
index 3233846ca3f..a7bd9be4377 100644
--- a/source3/winbindd/winbindd_msrpc.c
+++ b/source3/winbindd/winbindd_msrpc.c
@@ -1008,16 +1008,13 @@ NTSTATUS winbindd_lookup_sids(TALLOC_CTX *mem_ctx,
 	/* And restore our original timeout. */
 	dcerpc_binding_handle_set_timeout(b, orig_timeout);
 
-	if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) ||
-	    NT_STATUS_EQUAL(status, NT_STATUS_RPC_SEC_PKG_ERROR) ||
-	    NT_STATUS_EQUAL(status, NT_STATUS_NETWORK_ACCESS_DENIED)) {
+	if (reset_cm_connection_on_error(domain, b, status)) {
 		/*
 		 * This can happen if the schannel key is not
 		 * valid anymore, we need to invalidate the
 		 * all connections to the dc and reestablish
 		 * a netlogon connection first.
 		 */
-		invalidate_cm_connection(domain);
 		domain->can_do_ncacn_ip_tcp = domain->active_directory;
 		if (!retried) {
 			retried = true;
@@ -1087,16 +1084,13 @@ static NTSTATUS winbindd_lookup_names(TALLOC_CTX *mem_ctx,
 	/* And restore our original timeout. */
 	dcerpc_binding_handle_set_timeout(b, orig_timeout);
 
-	if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) ||
-	    NT_STATUS_EQUAL(status, NT_STATUS_RPC_SEC_PKG_ERROR) ||
-	    NT_STATUS_EQUAL(status, NT_STATUS_NETWORK_ACCESS_DENIED)) {
+	if (reset_cm_connection_on_error(domain, b, status)) {
 		/*
 		 * This can happen if the schannel key is not
 		 * valid anymore, we need to invalidate the
 		 * all connections to the dc and reestablish
 		 * a netlogon connection first.
 		 */
-		invalidate_cm_connection(domain);
 		if (!retried) {
 			retried = true;
 			goto connect;
diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 7225757d12e..ec643878e4c 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -1637,6 +1637,7 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain,
 	int attempts = 0;
 	int netr_attempts = 0;
 	bool retry = false;
+	bool valid_result = false;
 	NTSTATUS result;
 	enum netr_LogonInfoClass logon_type_i;
 	enum netr_LogonInfoClass logon_type_n;
@@ -1649,6 +1650,15 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain,
 		struct rpc_pipe_client *netlogon_pipe;
 		struct netlogon_creds_cli_context *netlogon_creds_ctx = NULL;
 
+		/*
+		 * We should always reset authoritative to 1
+		 * before calling a server again.
+		 *
+		 * Otherwise we could treat a local problem as
+		 * non-authoritative.
+		 */
+		*authoritative = 1;
+
 		retry = false;
 
 		result = cm_connect_netlogon_secure(domain, &netlogon_pipe,
@@ -1669,6 +1679,8 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain,
 				 "(error: %s, attempts: %d)\n",
 				  nt_errstr(result), netr_attempts));
 
+			reset_cm_connection_on_error(domain, NULL, result);
+
 			/* After the first retry always close the connection */
 			if (netr_attempts > 0) {
 				DEBUG(3, ("This is again a problem for this "
@@ -1791,26 +1803,22 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain,
 		   might not yet have noticed that the DC has killed
 		   our connection. */
 
-		if (!rpccli_is_connected(netlogon_pipe)) {
-			retry = true;
+		retry = reset_cm_connection_on_error(domain,
+						     netlogon_pipe->binding_handle,
+						     result);
+		if (retry) {
+			DBG_PREFIX(attempts > 1 ? DBGLVL_NOTICE : DBGLVL_INFO, (
+				   "This is problem %d for this "
+				   "particular call,"
+				   "DOMAIN[%s] DC[%s] - %s\n",
+				   attempts,
+				   domain->name,
+				   domain->dcname,
+				   nt_errstr(result)));
 			continue;
 		}
 
-		/* if we get access denied, a possible cause was that we had
-		   an open connection to the DC, but someone changed our
-		   machine account password out from underneath us using 'net
-		   rpc changetrustpw' */
-
-		if ( NT_STATUS_EQUAL(result, NT_STATUS_ACCESS_DENIED) ) {
-			DEBUG(1,("winbind_samlogon_retry_loop: sam_logon returned "
-				 "ACCESS_DENIED.  Maybe the DC has Restrict "
-				 "NTLM set or the trust account "
-				"password was changed and we didn't know it. "
-				 "Killing connections to domain %s\n",
-				domainname));
-			invalidate_cm_connection(domain);
-			retry = true;
-		}
+		valid_result = true;
 
 		if (NT_STATUS_EQUAL(result, NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE)) {
 			/*
@@ -1836,14 +1844,25 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain,
 			break;
 		}
 
-	} while ( (attempts < 2) && retry );
+	} while ( (attempts < 3) && retry );
 
-	if (NT_STATUS_EQUAL(result, NT_STATUS_IO_TIMEOUT)) {
-		DEBUG(3,("winbind_samlogon_retry_loop: sam_network_logon(ex) "
-				"returned NT_STATUS_IO_TIMEOUT after the retry. "
-				"Killing connections to domain %s\n",
-			domainname));
-		invalidate_cm_connection(domain);
+	if (!valid_result) {
+		/*
+		 * This matches what windows does. In a chain of transitive
+		 * trusts the ACCESS_DENIED/authoritative=0 is not propagated
+		 * instead of NT_STATUS_NO_LOGON_SERVERS/authoritative=1 is
+		 * passed along the chain if there's no other DC is available.
+		 */
+		DBG_WARNING("Mapping %s/authoritative=%u to "
+			    "NT_STATUS_NO_LOGON_SERVERS/authoritative=1 for"
+			    "USERNAME[%s] USERDOMAIN[%s] REMOTE-DOMAIN[%s] \n",
+			    nt_errstr(result),
+			    *authoritative,
+			    username,
+			    domainname,
+			    domain->name);
+		*authoritative = 1;
+		return NT_STATUS_NO_LOGON_SERVERS;
 	}
 
 	if (!NT_STATUS_IS_OK(result)) {


-- 
Samba Shared Repository



More information about the samba-cvs mailing list