[Patches] improve wb_looup{name,sid,sids}()

Stefan Metzmacher metze at samba.org
Sat Mar 11 00:20:12 UTC 2017


Hi,

here're some improvements to wb_looup{name,sid,sids}()

We avoid the bogus fallback to the forest root domain
as the DC of our domain already does all the work for us.

And with this patches we only do one round trip to our dc
for the following:

bin/wbinfo
--lookup-sids=S-1-5-21-278041429-3399921908-1452754838-500,S-1-5-21-2930975464-1937418634-1288008815-500,S-1-5-21-1368093
395-3821428921-3924672915-500,S-1-5-21-167342819-981449877-2130266853-500,S-1-5-21-313966788-4060240134-2249344781-500

S-1-5-21-278041429-3399921908-1452754838-500 -> W4EDOM-L4\Administrator 1
S-1-5-21-2930975464-1937418634-1288008815-500 -> W2012R2-L4\Administrator 1
S-1-5-21-1368093395-3821428921-3924672915-500 -> S1-W2012-L4\Administrator 1
S-1-5-21-167342819-981449877-2130266853-500 -> S2-W2012-L4\Administrator 1
S-1-5-21-313966788-4060240134-2249344781-500 -> S4XDOM\Administrator 1

(we're member of S2-W2012-L4.S1-W2012-L4.W2012R2-L4.BASE)

We have one forest with
W2012R2-L4.BASE
S1-W2012-L4.W2012R2-L4.BASE
and
S2-W2012-L4.S1-W2012-L4.W2012R2-L4.BASE

And a forest trust to W4EDOM-L4.BASE
And a forest trust to S4XDOM.BASE (samba-4.6.0)

As a note to remember

winbindd on the member does this:

       lsa_LookupSids3: struct lsa_LookupSids3
          in: struct lsa_LookupSids3
              sids                     : *
                  sids: struct lsa_SidArray
                      num_sids                 : 0x00000005 (5)
                      sids                     : *
                          sids: ARRAY(5)
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-278041429-3399921908-1452754838-500
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-2930975464-1937418634-1288008815-500
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-1368093395-3821428921-3924672915-500
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-167342819-981449877-2130266853-500
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-313966788-4060240134-2249344781-500
              names                    : *
                  names: struct lsa_TransNameArray2
                      count                    : 0x00000000 (0)
                      names                    : NULL
              level                    : LSA_LOOKUP_NAMES_ALL (1)
              count                    : *
                  count                    : 0x00000000 (0)
              lookup_options           :
LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
              client_revision          : LSA_CLIENT_REVISION_2 (2)


And the dc for S4XDOM.BASE gets this from the W2012R2-L4.BASE dc:

          in: struct lsa_LookupSids3
              sids                     : *
                  sids: struct lsa_SidArray
                      num_sids                 : 0x00000001 (1)
                      sids                     : *
                          sids: ARRAY(1)
                              sids: struct lsa_SidPtr
                                  sid                      : *
                                      sid                      :
S-1-5-21-313966788-4060240134-2249344781-500
              names                    : *
                  names: struct lsa_TransNameArray2
                      count                    : 0x00000000 (0)
                      names                    : NULL
              level                    :
LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
              count                    : *
                  count                    : 0x00000000 (0)
              lookup_options           :
LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
              client_revision          : LSA_CLIENT_REVISION_2 (2)

Please review and push:-)

Thanks!
metze

-------------- next part --------------
From cd43e418338ac196877d63b77b193eb4e81a0920 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 10 Mar 2017 15:23:36 +0100
Subject: [PATCH 1/4] winbindd: remove bogus fallback to the forest root in
 wb_lookupname*()

It's the job of the domain controller in our domain
to traverse the trust chain.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/wb_lookupname.c | 41 ----------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/source3/winbindd/wb_lookupname.c b/source3/winbindd/wb_lookupname.c
index 62b2e47..1dd6b68 100644
--- a/source3/winbindd/wb_lookupname.c
+++ b/source3/winbindd/wb_lookupname.c
@@ -32,7 +32,6 @@ struct wb_lookupname_state {
 };
 
 static void wb_lookupname_done(struct tevent_req *subreq);
-static void wb_lookupname_root_done(struct tevent_req *subreq);
 
 struct tevent_req *wb_lookupname_send(TALLOC_CTX *mem_ctx,
 				      struct tevent_context *ev,
@@ -86,46 +85,6 @@ static void wb_lookupname_done(struct tevent_req *subreq)
 		subreq, struct tevent_req);
 	struct wb_lookupname_state *state = tevent_req_data(
 		req, struct wb_lookupname_state);
-	struct winbindd_domain *root_domain;
-	NTSTATUS status, result;
-
-	status = dcerpc_wbint_LookupName_recv(subreq, state, &result);
-	TALLOC_FREE(subreq);
-	if (tevent_req_nterror(req, status)) {
-		return;
-	}
-	if (NT_STATUS_IS_OK(result)) {
-		tevent_req_done(req);
-		return;
-	}
-
-	/*
-	 * "our" DC did not find it, lets retry with the forest root
-	 * domain
-	 */
-
-	root_domain = find_root_domain();
-	if (root_domain == NULL) {
-		tevent_req_nterror(req, result);
-		return;
-	}
-
-	subreq = dcerpc_wbint_LookupName_send(
-		state, state->ev, dom_child_handle(root_domain),
-		state->dom_name,
-		state->name, state->flags, &state->type, &state->sid);
-	if (tevent_req_nomem(subreq, req)) {
-		return;
-	}
-	tevent_req_set_callback(subreq, wb_lookupname_root_done, req);
-}
-
-static void wb_lookupname_root_done(struct tevent_req *subreq)
-{
-	struct tevent_req *req = tevent_req_callback_data(
-		subreq, struct tevent_req);
-	struct wb_lookupname_state *state = tevent_req_data(
-		req, struct wb_lookupname_state);
 	NTSTATUS status, result;
 
 	status = dcerpc_wbint_LookupName_recv(subreq, state, &result);
-- 
1.9.1


From 1c514c86c952b2f8036b00374b40919b40fd57bd Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 10 Mar 2017 15:23:36 +0100
Subject: [PATCH 2/4] winbindd: remove bogus fallback to the forest root in
 wb_lookupsid*()

It's the job of the domain controller in our domain
to traverse the trust chain.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/wb_lookupsid.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/source3/winbindd/wb_lookupsid.c b/source3/winbindd/wb_lookupsid.c
index 7ff5c1e..8873ebb 100644
--- a/source3/winbindd/wb_lookupsid.c
+++ b/source3/winbindd/wb_lookupsid.c
@@ -71,36 +71,15 @@ static void wb_lookupsid_done(struct tevent_req *subreq)
 		subreq, struct tevent_req);
 	struct wb_lookupsid_state *state = tevent_req_data(
 		req, struct wb_lookupsid_state);
-	struct winbindd_domain *forest_root;
 	NTSTATUS status, result;
 
 	status = dcerpc_wbint_LookupSid_recv(subreq, state, &result);
 	TALLOC_FREE(subreq);
-	if (tevent_req_nterror(req, status)) {
+	if (any_nt_status_not_ok(status, result, &status)) {
+		tevent_req_nterror(req, status);
 		return;
 	}
-	if (NT_STATUS_IS_OK(result)) {
-		tevent_req_done(req);
-		return;
-	}
-
-	/*
-	 * Let's try the forest root
-	 */
-	forest_root = find_root_domain();
-	if ((forest_root == NULL) || (forest_root == state->lookup_domain)) {
-		tevent_req_nterror(req, result);
-		return;
-	}
-	state->lookup_domain = forest_root;
-
-	subreq = dcerpc_wbint_LookupSid_send(
-		state, state->ev, dom_child_handle(state->lookup_domain),
-		&state->sid, &state->type, &state->domname, &state->name);
-	if (tevent_req_nomem(subreq, req)) {
-		return;
-	}
-	tevent_req_set_callback(subreq, wb_lookupsid_done, req);
+	tevent_req_done(req);
 }
 
 NTSTATUS wb_lookupsid_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
-- 
1.9.1


From a537ba47a7cabdfbc9267a5c4484d902157b1f9f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 10 Mar 2017 15:23:36 +0100
Subject: [PATCH 3/4] winbindd: remove unused find_root_domain()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_proto.h |  1 -
 source3/winbindd/winbindd_util.c  | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index c5d934e..ede2c3e 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -470,7 +470,6 @@ struct winbindd_domain *find_domain_from_name(const char *domain_name);
 struct winbindd_domain *find_domain_from_sid_noinit(const struct dom_sid *sid);
 struct winbindd_domain *find_domain_from_sid(const struct dom_sid *sid);
 struct winbindd_domain *find_our_domain(void);
-struct winbindd_domain *find_root_domain(void);
 struct winbindd_domain *find_lookup_domain_from_sid(const struct dom_sid *sid);
 struct winbindd_domain *find_lookup_domain_from_name(const char *domain_name);
 bool parse_domain_user(const char *domuser, fstring domain, fstring user);
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index ab6862d..1a38dde 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -1005,17 +1005,6 @@ struct winbindd_domain *find_our_domain(void)
 	return NULL;
 }
 
-struct winbindd_domain *find_root_domain(void)
-{
-	struct winbindd_domain *ours = find_our_domain();
-
-	if (ours->forest_name == NULL) {
-		return NULL;
-	}
-
-	return find_domain_from_name( ours->forest_name );
-}
-
 /* Find the appropriate domain to lookup a name or SID */
 
 struct winbindd_domain *find_lookup_domain_from_sid(const struct dom_sid *sid)
-- 
1.9.1


From 11dd34ca51205548a5337d942d78ac867c80d667 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 10 Mar 2017 16:53:53 +0100
Subject: [PATCH 4/4] winbindd: avoid multiple wbint_LookupSids/lsa_LookupSids
 calls to the same domain

find_lookup_domain_from_sid() returns the same domain for all non local
sids on a domain member. We should not chunk one wb_lookupsids_send/recv
into multiple wbint_LookupSids_send/recv to the same 'lookup' domain,
just because the requested SIDs don't all belong to the same domain.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/wb_lookupsids.c | 48 ++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index c395f54..3f48ad7 100644
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -25,7 +25,6 @@
 #include "passdb/machine_sid.h"
 
 struct wb_lookupsids_domain {
-	struct dom_sid sid;
 	struct winbindd_domain *domain;
 
 	/*
@@ -194,7 +193,12 @@ static bool wb_lookupsids_next(struct tevent_req *req,
 
 		d = &state->domains[state->domains_done];
 
-		if (sid_check_is_our_sam(&d->sid)) {
+		if (d->domain->internal) {
+			/*
+			 * This is only our local SAM,
+			 * see wb_lookupsids_bulk() and
+			 * wb_lookupsids_get_domain().
+			 */
 			state->rids.num_rids = d->sids.num_sids;
 			state->rids.rids = talloc_array(state, uint32_t,
 							state->rids.num_rids);
@@ -207,7 +211,7 @@ static bool wb_lookupsids_next(struct tevent_req *req,
 			}
 			subreq = dcerpc_wbint_LookupRids_send(
 				state, state->ev, dom_child_handle(d->domain),
-				&d->sid, &state->rids, &state->domain_name,
+				&d->domain->sid, &state->rids, &state->domain_name,
 				&state->rid_names);
 			if (tevent_req_nomem(subreq, req)) {
 				return false;
@@ -322,14 +326,42 @@ static struct wb_lookupsids_domain *wb_lookupsids_get_domain(
 	domains = *pdomains;
 	num_domains = talloc_array_length(domains);
 
+	wb_domain = find_lookup_domain_from_sid(sid);
+	if (wb_domain == NULL) {
+		return NULL;
+	}
+
 	for (i=0; i<num_domains; i++) {
-		if (dom_sid_compare_domain(sid, &domains[i].sid) == 0) {
+		if (domains[i].domain != wb_domain) {
+			continue;
+		}
+
+		if (!domains[i].domain->internal) {
+			/*
+			 * If it's not our local sam,
+			 * we can re-use the domain without
+			 * checking the sid.
+			 *
+			 * Note the wb_lookupsids_bulk() above
+			 * already catched special SIDs,
+			 * e.g. the unix and builtin domains.
+			 */
 			return &domains[i];
 		}
-	}
 
-	wb_domain = find_lookup_domain_from_sid(sid);
-	if (wb_domain == NULL) {
+		if (dom_sid_compare_domain(sid, &domains[i].domain->sid) == 0) {
+			/*
+			 * If it's out local sam we can also use it.
+			 */
+			return &domains[i];
+		}
+
+		/*
+		 * I'm not sure if this can be triggered,
+		 * as wb_lookupsids_bulk() should also catch this,
+		 * but we need to make sure that we don't use
+		 * wbint_LookupRids() without a SID match.
+		 */
 		return NULL;
 	}
 
@@ -341,8 +373,6 @@ static struct wb_lookupsids_domain *wb_lookupsids_get_domain(
 	*pdomains = domains;
 
 	domain = &domains[num_domains];
-	sid_copy(&domain->sid, sid);
-	sid_split_rid(&domain->sid, NULL);
 	domain->domain = wb_domain;
 
 	domain->sids.sids = talloc_array(domains, struct lsa_SidPtr, num_sids);
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170311/57a4931c/signature.sig>


More information about the samba-technical mailing list