Faster winbind against passdb

Ralph Böhme slow at samba.org
Fri Mar 23 18:33:35 UTC 2018


On Fri, Mar 23, 2018 at 08:47:33PM +1300, Andrew Bartlett wrote:
> On Fri, 2018-03-23 at 20:02 +1300, Andrew Bartlett via samba-technical
> wrote:
> > On Fri, 2018-03-23 at 08:01 +0100, Ralph Böhme wrote:
> > > 
> > > do you have an updated patchset on-top of current master?
> > 
> > Yes, the one attached earlier in this thread will be fine just without
> > the travis-ci patch.
> 
> And re-attached here for clarity.

thanks!

Generally looks good, but I'd like to use a similar pattern to what is used in
winbindd_dual_srv.c with reset_cm_connection_on_error()

Can you check the FIXUP commits in the attached patchset? If you agree, feel
free to squash and push with my +1.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 450e46e53a3e676299f1485e4780054feb5349f4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 21 Mar 2018 20:43:10 +1300
Subject: [PATCH 1/5] winbindd: Add a cache of the samr and lsa handles for the
 passdb domain

This domain is very close, in AD DC configurations over a internal ncacn_np pipe
and otherwise in the same process via C linking.  It is however very expensive
to re-create the binding handle per SID->name lookup, so keep a cache.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/winbindd/winbindd_samr.c | 268 +++++++++++++++++++++++----------------
 1 file changed, 160 insertions(+), 108 deletions(-)

diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index aedb77bdee9..b2b4abc31cf 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -40,6 +40,21 @@
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
 
+/*
+ * The other end of this won't go away easily, so we can trust it
+ *
+ * It is either a long-lived process with the same lifetime as
+ * winbindd or a part of this process
+ */
+struct winbind_internal_pipe_private
+{
+	struct rpc_pipe_client *samr_pipe;
+	struct policy_handle samr_domain_hnd;
+	struct rpc_pipe_client *lsa_pipe;
+	struct policy_handle lsa_hnd;
+};
+
+
 NTSTATUS open_internal_samr_conn(TALLOC_CTX *mem_ctx,
 				 struct winbindd_domain *domain,
 				 struct rpc_pipe_client **samr_pipe,
@@ -101,6 +116,70 @@ NTSTATUS open_internal_lsa_conn(TALLOC_CTX *mem_ctx,
 	return status;
 }
 
+
+static NTSTATUS open_cached_internal_pipe_conn(struct winbindd_domain *domain,
+					       struct rpc_pipe_client **samr_pipe,
+					       struct policy_handle *samr_domain_hnd,
+					       struct rpc_pipe_client **lsa_pipe,
+					       struct policy_handle *lsa_hnd)
+{
+	struct winbind_internal_pipe_private *private_data;
+
+	if (domain->private_data == NULL) {
+		TALLOC_CTX *frame = talloc_stackframe();
+		NTSTATUS status;
+
+		private_data = talloc(frame,
+				      struct winbind_internal_pipe_private);
+
+		status = open_internal_samr_conn(private_data,
+						 domain,
+						 &private_data->samr_pipe,
+						 &private_data->samr_domain_hnd);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(frame);
+			return status;
+		}
+
+		status = open_internal_lsa_conn(private_data,
+						&private_data->lsa_pipe,
+						&private_data->lsa_hnd);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(frame);
+			return status;
+		}
+
+		domain->private_data = talloc_steal(domain, private_data);
+
+		TALLOC_FREE(frame);
+
+	} else {
+		private_data =
+			talloc_get_type_abort(domain->private_data,
+					      struct winbind_internal_pipe_private);
+	}
+
+	if (samr_domain_hnd) {
+		*samr_domain_hnd = private_data->samr_domain_hnd;
+	}
+
+	if (samr_pipe) {
+		*samr_pipe = private_data->samr_pipe;
+	}
+
+	if (lsa_hnd) {
+		*lsa_hnd = private_data->lsa_hnd;
+	}
+
+	if (lsa_pipe) {
+		*lsa_pipe = private_data->lsa_pipe;
+	}
+
+	return NT_STATUS_OK;
+}
+
 /*********************************************************************
  SAM specific functions.
 *********************************************************************/
@@ -116,8 +195,7 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 	struct wb_acct_info *info = NULL;
 	uint32_t num_info = 0;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_enum_dom_groups\n"));
 
@@ -130,20 +208,24 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
-		goto error;
+		TALLOC_FREE(tmp_ctx);
+		return status;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_enum_dom_groups(tmp_ctx,
 				     samr_pipe,
 				     &dom_pol,
 				     &num_info,
 				     &info);
 	if (!NT_STATUS_IS_OK(status)) {
-		goto error;
+		TALLOC_FREE(tmp_ctx);
+		return status;
 	}
 
 	if (pnum_info) {
@@ -154,10 +236,6 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 		*pinfo = talloc_move(mem_ctx, &info);
 	}
 
-error:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -171,8 +249,7 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 	struct policy_handle dom_pol = { 0 };
 	uint32_t *rids = NULL;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("samr_query_user_list\n"));
 
@@ -181,13 +258,15 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_query_user_list(tmp_ctx,
 				     samr_pipe,
 				     &dom_pol,
@@ -202,10 +281,6 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
-
 	TALLOC_FREE(rids);
 	TALLOC_FREE(tmp_ctx);
 	return status;
@@ -221,8 +296,7 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 	struct netr_DomainTrust *trusts = NULL;
 	uint32_t num_trusts = 0;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("samr: trusted domains\n"));
 
@@ -235,13 +309,15 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_lsa_conn(tmp_ctx, &lsa_pipe, &lsa_policy);
+	status = open_cached_internal_pipe_conn(domain,
+						NULL,
+						NULL,
+						&lsa_pipe,
+						&lsa_policy);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = lsa_pipe->binding_handle;
-
 	status = rpc_trusted_domains(tmp_ctx,
 				     lsa_pipe,
 				     &lsa_policy,
@@ -257,10 +333,6 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&lsa_policy)) {
-		dcerpc_lsa_Close(b, mem_ctx, &lsa_policy, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -284,8 +356,7 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 	uint32_t *name_types = NULL;
 
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_lookup_groupmem\n"));
 
@@ -304,13 +375,15 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_lookup_groupmem(tmp_ctx,
 				     samr_pipe,
 				     &dom_pol,
@@ -340,10 +413,6 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -398,8 +467,7 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 	struct wb_acct_info *info = NULL;
 	uint32_t num_info = 0;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("samr: enum local groups\n"));
 
@@ -412,13 +480,15 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_enum_local_groups(mem_ctx,
 				       samr_pipe,
 				       &dom_pol,
@@ -437,10 +507,6 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -459,8 +525,7 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 	struct dom_sid sid;
 	enum lsa_SidType type;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_name_to_sid\n"));
 
@@ -469,13 +534,15 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_lsa_conn(tmp_ctx, &lsa_pipe, &lsa_policy);
+	status = open_cached_internal_pipe_conn(domain,
+						NULL,
+						NULL,
+						&lsa_pipe,
+						&lsa_policy);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = lsa_pipe->binding_handle;
-
 	status = rpc_name_to_sid(tmp_ctx,
 				 lsa_pipe,
 				 &lsa_policy,
@@ -496,10 +563,6 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&lsa_policy)) {
-		dcerpc_lsa_Close(b, mem_ctx, &lsa_policy, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -518,8 +581,7 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 	char *name = NULL;
 	enum lsa_SidType type;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_sid_to_name\n"));
 
@@ -543,13 +605,15 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_lsa_conn(tmp_ctx, &lsa_pipe, &lsa_policy);
+	status = open_cached_internal_pipe_conn(domain,
+						NULL,
+						NULL,
+						&lsa_pipe,
+						&lsa_policy);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = lsa_pipe->binding_handle;
-
 	status = rpc_sid_to_name(tmp_ctx,
 				 lsa_pipe,
 				 &lsa_policy,
@@ -572,9 +636,6 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&lsa_policy)) {
-		dcerpc_lsa_Close(b, mem_ctx, &lsa_policy, &result);
-	}
 
 	TALLOC_FREE(tmp_ctx);
 	return status;
@@ -595,8 +656,7 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 	char *domain_name = NULL;
 	char **names = NULL;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_rids_to_names for %s\n", domain->name));
 
@@ -616,13 +676,15 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_lsa_conn(tmp_ctx, &lsa_pipe, &lsa_policy);
+	status = open_cached_internal_pipe_conn(domain,
+						NULL,
+						NULL,
+						&lsa_pipe,
+						&lsa_policy);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = lsa_pipe->binding_handle;
-
 	status = rpc_rids_to_names(tmp_ctx,
 				   lsa_pipe,
 				   &lsa_policy,
@@ -650,10 +712,6 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&lsa_policy)) {
-		dcerpc_lsa_Close(b, mem_ctx, &lsa_policy, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -676,7 +734,11 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto error;
 	}
@@ -700,10 +762,6 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 	*lockout_policy = info->info12;
 
 error:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -726,7 +784,11 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto error;
 	}
@@ -750,10 +812,6 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 	*passwd_policy = info->info1;
 
 error:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
-
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
@@ -770,8 +828,7 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 	struct dom_sid *user_grpsids = NULL;
 	uint32_t num_groups = 0;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_lookup_usergroups\n"));
 
@@ -786,13 +843,15 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_lookup_usergroups(tmp_ctx,
 				       samr_pipe,
 				       &dom_pol,
@@ -813,9 +872,6 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
 
 	TALLOC_FREE(tmp_ctx);
 	return status;
@@ -833,8 +889,7 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 	uint32_t num_aliases = 0;
 	uint32_t *alias_rids = NULL;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("sam_lookup_useraliases\n"));
 
@@ -847,13 +902,15 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_lookup_useraliases(tmp_ctx,
 					samr_pipe,
 					&dom_pol,
@@ -874,9 +931,6 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 	}
 
 done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, mem_ctx, &dom_pol, &result);
-	}
 
 	TALLOC_FREE(tmp_ctx);
 	return status;
@@ -890,8 +944,7 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 	struct policy_handle dom_pol = { 0 };
 	uint32_t seq = DOM_SEQUENCE_NONE;
 	TALLOC_CTX *tmp_ctx;
-	NTSTATUS status, result;
-	struct dcerpc_binding_handle *b = NULL;
+	NTSTATUS status;
 
 	DEBUG(3,("samr: sequence number\n"));
 
@@ -904,13 +957,15 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	status = open_internal_samr_conn(tmp_ctx, domain, &samr_pipe, &dom_pol);
+	status = open_cached_internal_pipe_conn(domain,
+						&samr_pipe,
+						&dom_pol,
+						NULL,
+						NULL);
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
 
-	b = samr_pipe->binding_handle;
-
 	status = rpc_sequence_number(tmp_ctx,
 				     samr_pipe,
 				     &dom_pol,
@@ -923,11 +978,8 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 	if (pseq) {
 		*pseq = seq;
 	}
-done:
-	if (b && is_valid_policy_hnd(&dom_pol)) {
-		dcerpc_samr_Close(b, tmp_ctx, &dom_pol, &result);
-	}
 
+done:
 	TALLOC_FREE(tmp_ctx);
 	return status;
 }
-- 
2.13.6


From 27a4213444b3d4043e5772f8fe7d88dd7a540e34 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 23 Mar 2018 18:33:48 +0100
Subject: [PATCH 2/5] FIXUP: winbindd: Add a cache of the samr and lsa handles
 for the passdb domain

- struct declaration { on same line

- rename struct winbind_internal_pipe_private to winbind_internal_pipes

- long lines

- use talloc_zero

- simplify if else, use talloc_move

- rename variable private_data to internal_pipes
---
 source3/winbindd/winbindd_samr.c | 53 ++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index b2b4abc31cf..da54d697909 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -46,8 +46,7 @@
  * It is either a long-lived process with the same lifetime as
  * winbindd or a part of this process
  */
-struct winbind_internal_pipe_private
-{
+struct winbind_internal_pipes {
 	struct rpc_pipe_client *samr_pipe;
 	struct policy_handle samr_domain_hnd;
 	struct rpc_pipe_client *lsa_pipe;
@@ -117,64 +116,64 @@ NTSTATUS open_internal_lsa_conn(TALLOC_CTX *mem_ctx,
 }
 
 
-static NTSTATUS open_cached_internal_pipe_conn(struct winbindd_domain *domain,
-					       struct rpc_pipe_client **samr_pipe,
-					       struct policy_handle *samr_domain_hnd,
-					       struct rpc_pipe_client **lsa_pipe,
-					       struct policy_handle *lsa_hnd)
+static NTSTATUS open_cached_internal_pipe_conn(
+	struct winbindd_domain *domain,
+	struct rpc_pipe_client **samr_pipe,
+	struct policy_handle *samr_domain_hnd,
+	struct rpc_pipe_client **lsa_pipe,
+	struct policy_handle *lsa_hnd)
 {
-	struct winbind_internal_pipe_private *private_data;
+	struct winbind_internal_pipes *internal_pipes = NULL;
 
 	if (domain->private_data == NULL) {
 		TALLOC_CTX *frame = talloc_stackframe();
 		NTSTATUS status;
 
-		private_data = talloc(frame,
-				      struct winbind_internal_pipe_private);
-
-		status = open_internal_samr_conn(private_data,
-						 domain,
-						 &private_data->samr_pipe,
-						 &private_data->samr_domain_hnd);
+		internal_pipes = talloc_zero(frame,
+					     struct winbind_internal_pipes);
 
+		status = open_internal_samr_conn(
+			internal_pipes,
+			domain,
+			&internal_pipes->samr_pipe,
+			&internal_pipes->samr_domain_hnd);
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(frame);
 			return status;
 		}
 
-		status = open_internal_lsa_conn(private_data,
-						&private_data->lsa_pipe,
-						&private_data->lsa_hnd);
+		status = open_internal_lsa_conn(internal_pipes,
+						&internal_pipes->lsa_pipe,
+						&internal_pipes->lsa_hnd);
 
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(frame);
 			return status;
 		}
 
-		domain->private_data = talloc_steal(domain, private_data);
+		domain->private_data = talloc_move(domain, &internal_pipes);
 
 		TALLOC_FREE(frame);
 
-	} else {
-		private_data =
-			talloc_get_type_abort(domain->private_data,
-					      struct winbind_internal_pipe_private);
 	}
 
+	internal_pipes = talloc_get_type_abort(
+		domain->private_data, struct winbind_internal_pipes);
+
 	if (samr_domain_hnd) {
-		*samr_domain_hnd = private_data->samr_domain_hnd;
+		*samr_domain_hnd = internal_pipes->samr_domain_hnd;
 	}
 
 	if (samr_pipe) {
-		*samr_pipe = private_data->samr_pipe;
+		*samr_pipe = internal_pipes->samr_pipe;
 	}
 
 	if (lsa_hnd) {
-		*lsa_hnd = private_data->lsa_hnd;
+		*lsa_hnd = internal_pipes->lsa_hnd;
 	}
 
 	if (lsa_pipe) {
-		*lsa_pipe = private_data->lsa_pipe;
+		*lsa_pipe = internal_pipes->lsa_pipe;
 	}
 
 	return NT_STATUS_OK;
-- 
2.13.6


From 356eb0ad668d8437aba2fa8d50f2f7e20deb53ff Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 21 Mar 2018 20:44:31 +1300
Subject: [PATCH 3/5] winbindd: Do re-connect if the RPC call fails in the
 passdb case

This is very, very unlikely but possible as in the AD case the RPC server is in
another process that may eventually be able to restart.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/winbindd/winbindd_samr.c | 307 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 307 insertions(+)

diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index da54d697909..5d5bc064387 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -28,6 +28,7 @@
 #include "winbindd_rpc.h"
 #include "lib/util_unixsids.h"
 #include "rpc_client/rpc_client.h"
+#include "rpc_client/cli_pipe.h"
 #include "../librpc/gen_ndr/ndr_samr_c.h"
 #include "rpc_client/cli_samr.h"
 #include "../librpc/gen_ndr/ndr_lsa_c.h"
@@ -222,6 +223,29 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 				     &dom_pol,
 				     &num_info,
 				     &info);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(tmp_ctx);
+			return status;
+		}
+
+		status = rpc_enum_dom_groups(tmp_ctx,
+					     samr_pipe,
+					     &dom_pol,
+					     &num_info,
+					     &info);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		TALLOC_FREE(tmp_ctx);
 		return status;
@@ -271,6 +295,28 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 				     &dom_pol,
 				     &domain->sid,
 				     &rids);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_query_user_list(tmp_ctx,
+					     samr_pipe,
+					     &dom_pol,
+					     &domain->sid,
+					     &rids);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -322,6 +368,27 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 				     &lsa_policy,
 				     &num_trusts,
 				     &trusts);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(lsa_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							NULL,
+							NULL,
+							&lsa_pipe,
+							&lsa_policy);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_trusted_domains(tmp_ctx,
+				     lsa_pipe,
+				     &lsa_policy,
+				     &num_trusts,
+				     &trusts);
+	}
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -395,6 +462,34 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 				     &names,
 				     &name_types);
 
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_lookup_groupmem(tmp_ctx,
+					     samr_pipe,
+					     &dom_pol,
+					     domain->name,
+					     &domain->sid,
+					     group_sid,
+					     type,
+					     &num_names,
+					     &sid_mem,
+					     &names,
+					     &name_types);
+	}
+
 	if (pnum_names) {
 		*pnum_names = num_names;
 	}
@@ -493,6 +588,28 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 				       &dom_pol,
 				       &num_info,
 				       &info);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_enum_local_groups(mem_ctx,
+					       samr_pipe,
+					       &dom_pol,
+					       &num_info,
+					       &info);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -550,6 +667,30 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 				 flags,
 				 &sid,
 				 &type);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(lsa_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							NULL,
+							NULL,
+							&lsa_pipe,
+							&lsa_policy);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_name_to_sid(tmp_ctx,
+					 lsa_pipe,
+					 &lsa_policy,
+					 domain_name,
+					 name,
+					 flags,
+					 &sid,
+					 &type);
+	}
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -622,6 +763,31 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 				 &name,
 				 &type);
 
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(lsa_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							NULL,
+							NULL,
+							&lsa_pipe,
+							&lsa_policy);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_sid_to_name(tmp_ctx,
+					 lsa_pipe,
+					 &lsa_policy,
+					 domain,
+					 sid,
+					 &domain_name,
+					 &name,
+					 &type);
+	}
+
 	if (ptype) {
 		*ptype = type;
 	}
@@ -694,6 +860,32 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 				   &domain_name,
 				   &names,
 				   &types);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(lsa_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							NULL,
+							NULL,
+							&lsa_pipe,
+							&lsa_policy);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_rids_to_names(tmp_ctx,
+					   lsa_pipe,
+					   &lsa_policy,
+					   domain,
+					   domain_sid,
+					   rids,
+					   num_rids,
+					   &domain_name,
+					   &names,
+					   &types);
+	}
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -750,6 +942,28 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 					     DomainLockoutInformation,
 					     &info,
 					     &result);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto error;
+		}
+
+		status = dcerpc_samr_QueryDomainInfo(b,
+						     mem_ctx,
+						     &dom_pol,
+						     DomainLockoutInformation,
+						     &info,
+						     &result);
+	}
 	if (!NT_STATUS_IS_OK(status)) {
 		goto error;
 	}
@@ -800,6 +1014,29 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 					     DomainPasswordInformation,
 					     &info,
 					     &result);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto error;
+		}
+
+		status = dcerpc_samr_QueryDomainInfo(b,
+						     mem_ctx,
+						     &dom_pol,
+						     DomainPasswordInformation,
+						     &info,
+						     &result);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto error;
 	}
@@ -858,6 +1095,30 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 				       user_sid,
 				       &num_groups,
 				       &user_grpsids);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_lookup_usergroups(tmp_ctx,
+					       samr_pipe,
+					       &dom_pol,
+					       &domain->sid,
+					       user_sid,
+					       &num_groups,
+					       &user_grpsids);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -917,6 +1178,30 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 					sids,
 					&num_aliases,
 					&alias_rids);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_lookup_useraliases(tmp_ctx,
+						samr_pipe,
+						&dom_pol,
+						num_sids,
+						sids,
+						&num_aliases,
+						&alias_rids);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -970,6 +1255,28 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 				     &dom_pol,
 				     domain->name,
 				     &seq);
+	/*
+	 * Very unlikely, but before checking the result,
+	 * if we disconnected try again
+	 */
+	if (!rpccli_is_connected(samr_pipe)) {
+		TALLOC_FREE(domain->private_data);
+		status = open_cached_internal_pipe_conn(domain,
+							&samr_pipe,
+							&dom_pol,
+							NULL,
+							NULL);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+
+		status = rpc_sequence_number(tmp_ctx,
+					     samr_pipe,
+					     &dom_pol,
+					     domain->name,
+					     &seq);
+	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
-- 
2.13.6


From dbb6b39e15864eddbcd2decf3586e649145795f9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 23 Mar 2018 18:24:59 +0100
Subject: [PATCH 4/5] FIXUP: winbindd: Do re-connect if the RPC call fails in
 the passdb case

- add helper function reset_connection_on_error() similar to
  winbindd_dual_srv.c reset_cm_connection_on_error()

- use it in the RPC requests with a goto again retry logic
---
 source3/winbindd/winbindd_samr.c | 369 ++++++++++-----------------------------
 1 file changed, 94 insertions(+), 275 deletions(-)

diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index 5d5bc064387..1c24ababc2d 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -180,6 +180,30 @@ static NTSTATUS open_cached_internal_pipe_conn(
 	return NT_STATUS_OK;
 }
 
+static bool reset_connection_on_error(struct winbindd_domain *domain,
+				      struct rpc_pipe_client *p,
+				      NTSTATUS status)
+{
+	struct winbind_internal_pipes *internal_pipes = NULL;
+
+	internal_pipes = talloc_get_type_abort(
+		domain->private_data, struct winbind_internal_pipes);
+
+	if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT) ||
+	    NT_STATUS_EQUAL(status, NT_STATUS_IO_DEVICE_ERROR))
+	{
+		TALLOC_FREE(internal_pipes);
+		return true;
+	}
+
+	if (!rpccli_is_connected(p)) {
+		TALLOC_FREE(internal_pipes);
+		return true;
+	}
+
+	return false;
+}
+
 /*********************************************************************
  SAM specific functions.
 *********************************************************************/
@@ -196,6 +220,7 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 	uint32_t num_info = 0;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_enum_dom_groups\n"));
 
@@ -208,6 +233,7 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -223,27 +249,10 @@ static NTSTATUS sam_enum_dom_groups(struct winbindd_domain *domain,
 				     &dom_pol,
 				     &num_info,
 				     &info);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			TALLOC_FREE(tmp_ctx);
-			return status;
-		}
 
-		status = rpc_enum_dom_groups(tmp_ctx,
-					     samr_pipe,
-					     &dom_pol,
-					     &num_info,
-					     &info);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -273,6 +282,7 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 	uint32_t *rids = NULL;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("samr_query_user_list\n"));
 
@@ -281,6 +291,7 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -295,26 +306,9 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 				     &dom_pol,
 				     &domain->sid,
 				     &rids);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
-
-		status = rpc_query_user_list(tmp_ctx,
-					     samr_pipe,
-					     &dom_pol,
-					     &domain->sid,
-					     &rids);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -342,6 +336,7 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 	uint32_t num_trusts = 0;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("samr: trusted domains\n"));
 
@@ -354,6 +349,7 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						NULL,
 						NULL,
@@ -368,27 +364,12 @@ static NTSTATUS sam_trusted_domains(struct winbindd_domain *domain,
 				     &lsa_policy,
 				     &num_trusts,
 				     &trusts);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(lsa_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							NULL,
-							NULL,
-							&lsa_pipe,
-							&lsa_policy);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_trusted_domains(tmp_ctx,
-				     lsa_pipe,
-				     &lsa_policy,
-				     &num_trusts,
-				     &trusts);
+	if (!retry && reset_connection_on_error(domain, lsa_pipe, status)) {
+		retry = true;
+		goto again;
 	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -423,6 +404,7 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_lookup_groupmem\n"));
 
@@ -441,6 +423,7 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -462,32 +445,9 @@ static NTSTATUS sam_lookup_groupmem(struct winbindd_domain *domain,
 				     &names,
 				     &name_types);
 
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
-
-		status = rpc_lookup_groupmem(tmp_ctx,
-					     samr_pipe,
-					     &dom_pol,
-					     domain->name,
-					     &domain->sid,
-					     group_sid,
-					     type,
-					     &num_names,
-					     &sid_mem,
-					     &names,
-					     &name_types);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (pnum_names) {
@@ -562,6 +522,7 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 	uint32_t num_info = 0;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("samr: enum local groups\n"));
 
@@ -574,6 +535,7 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -587,27 +549,11 @@ static NTSTATUS sam_enum_local_groups(struct winbindd_domain *domain,
 				       samr_pipe,
 				       &dom_pol,
 				       &num_info,
-				       &info);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_enum_local_groups(mem_ctx,
-					       samr_pipe,
-					       &dom_pol,
-					       &num_info,
-					       &info);
+				       &info);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -642,6 +588,7 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 	enum lsa_SidType type;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_name_to_sid\n"));
 
@@ -650,6 +597,7 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						NULL,
 						NULL,
@@ -667,30 +615,12 @@ static NTSTATUS sam_name_to_sid(struct winbindd_domain *domain,
 				 flags,
 				 &sid,
 				 &type);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(lsa_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							NULL,
-							NULL,
-							&lsa_pipe,
-							&lsa_policy);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_name_to_sid(tmp_ctx,
-					 lsa_pipe,
-					 &lsa_policy,
-					 domain_name,
-					 name,
-					 flags,
-					 &sid,
-					 &type);
+	if (!retry && reset_connection_on_error(domain, lsa_pipe, status)) {
+		retry = true;
+		goto again;
 	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -722,6 +652,7 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 	enum lsa_SidType type;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_sid_to_name\n"));
 
@@ -745,6 +676,7 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						NULL,
 						NULL,
@@ -763,29 +695,9 @@ static NTSTATUS sam_sid_to_name(struct winbindd_domain *domain,
 				 &name,
 				 &type);
 
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(lsa_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							NULL,
-							NULL,
-							&lsa_pipe,
-							&lsa_policy);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
-
-		status = rpc_sid_to_name(tmp_ctx,
-					 lsa_pipe,
-					 &lsa_policy,
-					 domain,
-					 sid,
-					 &domain_name,
-					 &name,
-					 &type);
+	if (!retry && reset_connection_on_error(domain, lsa_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (ptype) {
@@ -822,6 +734,7 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 	char **names = NULL;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_rids_to_names for %s\n", domain->name));
 
@@ -841,6 +754,7 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						NULL,
 						NULL,
@@ -860,32 +774,12 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain,
 				   &domain_name,
 				   &names,
 				   &types);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(lsa_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							NULL,
-							NULL,
-							&lsa_pipe,
-							&lsa_policy);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_rids_to_names(tmp_ctx,
-					   lsa_pipe,
-					   &lsa_policy,
-					   domain,
-					   domain_sid,
-					   rids,
-					   num_rids,
-					   &domain_name,
-					   &names,
-					   &types);
+	if (!retry && reset_connection_on_error(domain, lsa_pipe, status)) {
+		retry = true;
+		goto again;
 	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
@@ -917,6 +811,7 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status, result;
 	struct dcerpc_binding_handle *b = NULL;
+	bool retry = false;
 
 	DEBUG(3,("sam_lockout_policy\n"));
 
@@ -925,6 +820,7 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -942,28 +838,12 @@ static NTSTATUS sam_lockout_policy(struct winbindd_domain *domain,
 					     DomainLockoutInformation,
 					     &info,
 					     &result);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto error;
-		}
 
-		status = dcerpc_samr_QueryDomainInfo(b,
-						     mem_ctx,
-						     &dom_pol,
-						     DomainLockoutInformation,
-						     &info,
-						     &result);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
+
 	if (!NT_STATUS_IS_OK(status)) {
 		goto error;
 	}
@@ -989,6 +869,7 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status, result;
 	struct dcerpc_binding_handle *b = NULL;
+	bool retry = false;
 
 	DEBUG(3,("sam_password_policy\n"));
 
@@ -997,6 +878,7 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -1014,27 +896,10 @@ static NTSTATUS sam_password_policy(struct winbindd_domain *domain,
 					     DomainPasswordInformation,
 					     &info,
 					     &result);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto error;
-		}
 
-		status = dcerpc_samr_QueryDomainInfo(b,
-						     mem_ctx,
-						     &dom_pol,
-						     DomainPasswordInformation,
-						     &info,
-						     &result);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -1065,6 +930,7 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 	uint32_t num_groups = 0;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_lookup_usergroups\n"));
 
@@ -1079,6 +945,7 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -1095,28 +962,10 @@ static NTSTATUS sam_lookup_usergroups(struct winbindd_domain *domain,
 				       user_sid,
 				       &num_groups,
 				       &user_grpsids);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_lookup_usergroups(tmp_ctx,
-					       samr_pipe,
-					       &dom_pol,
-					       &domain->sid,
-					       user_sid,
-					       &num_groups,
-					       &user_grpsids);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -1150,6 +999,7 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 	uint32_t *alias_rids = NULL;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("sam_lookup_useraliases\n"));
 
@@ -1162,6 +1012,7 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -1178,28 +1029,10 @@ static NTSTATUS sam_lookup_useraliases(struct winbindd_domain *domain,
 					sids,
 					&num_aliases,
 					&alias_rids);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_lookup_useraliases(tmp_ctx,
-						samr_pipe,
-						&dom_pol,
-						num_sids,
-						sids,
-						&num_aliases,
-						&alias_rids);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
@@ -1229,6 +1062,7 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 	uint32_t seq = DOM_SEQUENCE_NONE;
 	TALLOC_CTX *tmp_ctx;
 	NTSTATUS status;
+	bool retry = false;
 
 	DEBUG(3,("samr: sequence number\n"));
 
@@ -1241,6 +1075,7 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+again:
 	status = open_cached_internal_pipe_conn(domain,
 						&samr_pipe,
 						&dom_pol,
@@ -1255,26 +1090,10 @@ static NTSTATUS sam_sequence_number(struct winbindd_domain *domain,
 				     &dom_pol,
 				     domain->name,
 				     &seq);
-	/*
-	 * Very unlikely, but before checking the result,
-	 * if we disconnected try again
-	 */
-	if (!rpccli_is_connected(samr_pipe)) {
-		TALLOC_FREE(domain->private_data);
-		status = open_cached_internal_pipe_conn(domain,
-							&samr_pipe,
-							&dom_pol,
-							NULL,
-							NULL);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
 
-		status = rpc_sequence_number(tmp_ctx,
-					     samr_pipe,
-					     &dom_pol,
-					     domain->name,
-					     &seq);
+	if (!retry && reset_connection_on_error(domain, samr_pipe, status)) {
+		retry = true;
+		goto again;
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
-- 
2.13.6


From b71e9185c80da83d692120c2d8033e9e12af1c22 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 21 Mar 2018 21:23:13 +1300
Subject: [PATCH 5/5] winbindd: Use talloc_zero_array for consistency with
 other winbindd_domain allocators

The other allocator for this structure uses talloc_zero()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/winbindd/wb_seqnums.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/winbindd/wb_seqnums.c b/source3/winbindd/wb_seqnums.c
index 2a4cdc930e8..3c9af01e40f 100644
--- a/source3/winbindd/wb_seqnums.c
+++ b/source3/winbindd/wb_seqnums.c
@@ -56,8 +56,8 @@ struct tevent_req *wb_seqnums_send(TALLOC_CTX *mem_ctx,
 
 	state->subreqs = talloc_array(state, struct tevent_req *,
 				      state->num_domains);
-	state->domains = talloc_array(state, struct winbindd_domain *,
-				      state->num_domains);
+	state->domains = talloc_zero_array(state, struct winbindd_domain *,
+					   state->num_domains);
 	state->stati = talloc_array(state, NTSTATUS, state->num_domains);
 	state->seqnums = talloc_array(state, uint32_t, state->num_domains);
 
-- 
2.13.6



More information about the samba-technical mailing list