From 4f910510a1d0b9312593c593e4b28d2c6b92b1e8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Jul 2023 12:32:34 +0200 Subject: [PATCH 1/5] 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 Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit cb59fd43bbf758e4bad774cfc19ef87b157052c2) --- source3/winbindd/winbindd_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c index 1835d0b9d63e..de8cab04707c 100644 --- a/source3/winbindd/winbindd_cache.c +++ b/source3/winbindd/winbindd_cache.c @@ -1508,6 +1508,7 @@ do_query: DEBUG(3, ("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)) { DEBUG(3, ("query_user_list: flushing " "connection cache\n")); -- 2.34.1 From 97da035603559bd4b0ca42f67745dae0b2ee762d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Jul 2023 12:32:34 +0200 Subject: [PATCH 2/5] 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 Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit 4ad5a35a3f67860aa7a1345efcfc92fe40578e31) --- source3/winbindd/winbindd_msrpc.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/winbindd/winbindd_msrpc.c b/source3/winbindd/winbindd_msrpc.c index 3ac13b0e3d11..2926bd65e227 100644 --- a/source3/winbindd/winbindd_msrpc.c +++ b/source3/winbindd/winbindd_msrpc.c @@ -954,16 +954,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; @@ -1033,16 +1030,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; -- 2.34.1 From 04bcb7fa7df01c9168f8a5c31607954eb00bf71f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 16 Feb 2022 14:19:16 +0100 Subject: [PATCH 3/5] 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 Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit 0cb6de4b1d5410f3699172952be81c6eb75c2c86) --- source3/winbindd/winbindd_pam.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index 9805d90fef03..d870188767e7 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -1649,6 +1649,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, -- 2.34.1 From b8d3c7126d530facccf12976e262024b89271ab0 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Jul 2023 13:01:24 +0200 Subject: [PATCH 4/5] 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 Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit b317b10dffd99d1add3ff0b85b958edd9639abc8) --- source3/winbindd/winbindd_pam.c | 42 ++++++++++++--------------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index d870188767e7..9953ffeb94b1 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -1678,6 +1678,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 " @@ -1800,27 +1802,21 @@ 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; - } - if (NT_STATUS_EQUAL(result, NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE)) { /* * Got DCERPC_FAULT_OP_RNG_ERROR for SamLogon @@ -1845,15 +1841,7 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain, break; } - } while ( (attempts < 2) && 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); - } + } while ( (attempts < 3) && retry ); if (!NT_STATUS_IS_OK(result)) { return result; -- 2.34.1 From 5e307c37258fa57f2172a38ca47c2aa62d732f51 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Jul 2023 14:12:03 +0200 Subject: [PATCH 5/5] 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 Reviewed-by: Andreas Schneider Reviewed-by: Volker Lendecke (cherry picked from commit 50e771c12f84f9268c2e9ddeef0965f79f85de3d) --- source3/winbindd/winbindd_pam.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index 9953ffeb94b1..b1acc7efabca 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; @@ -1817,6 +1818,8 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain, continue; } + valid_result = true; + if (NT_STATUS_EQUAL(result, NT_STATUS_RPC_PROCNUM_OUT_OF_RANGE)) { /* * Got DCERPC_FAULT_OP_RNG_ERROR for SamLogon @@ -1843,6 +1846,25 @@ static NTSTATUS winbind_samlogon_retry_loop(struct winbindd_domain *domain, } while ( (attempts < 3) && retry ); + 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)) { return result; } -- 2.34.1