Regression triggered by the bug #11830 fixes (4.4.10, 4.6.0rc4 and v4-5-test)

[Stefan Metzmacher]

Hi,

the problem is that we blindly take the trust_flags, trust_attribs...
from the enumeration of trusts of other forests.

As result we have NETR_TRUST_FLAG_TREEROOT and NETR_TRUST_FLAG_IN_FOREST
on domain which are not within our forest.

Please review and push:-)

Thanks!
metze

Am 01.03.2017 um 18:30 schrieb Stefan Metzmacher:
> Hi,
> 
> if someone wants a debugging challenge please have a look at
> the logfiles in https://bugzilla.samba.org/show_bug.cgi?id=12605.
> 
> It seems to start with the following in log.winbindd:
> 
> [2017/03/01 17:30:49.851743, 10, pid=6517, effective(0, 0), real(0, 0),
> class=winbind] ../source3/winbindd/winbindd_util.c:369(trustdom_list_done)
>   trustdom_list_done: parsing response line
> 'BLA\bla.base\S-1-5-21-4053568372-2049667917-3384589010\34\2\8
>   W2012R2-L6\w2012r2-l6.base\S-1-5-21-2072033271-969857664-807811056\34\2\4
>   W4EDOM-L4\w4edom-l4.base\S-1-5-21-278041429-3399921908-1452754838\29\2\0'
> 
> I'll have a look at it again tomorrow...
> 
> Thanks!
> metze
> 
-------------- next part --------------
From 2c956efc8077c549d3653d9ba99e2833d074ff28 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 2 Mar 2017 08:13:57 +0100
Subject: [PATCH] s3:winbindd: fix endless forest trust scan

Commit 0392ebcd1d48e9f472f2148b85316a77d9cc953b effectively
disabled the enumeration of trusts in other forests.

The fixes for https://bugzilla.samba.org/show_bug.cgi?id=11691
changed the way we fill domain->domain_flags for domains
in other forests.

Commit fffefe72fcc62d9688b45f53a5327667dc0b2fe6 readded the
ability to enumerate trusts of other forests again, in order to
fix https://bugzilla.samba.org/show_bug.cgi?id=11830

Now we have the problem that multiple domains
(even outside of our forest) are considert to be
our forest root, as they have the following flags:
NETR_TRUST_FLAG_TREEROOT and NETR_TRUST_FLAG_IN_FOREST.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12605

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_ads.c  |  8 ++++++++
 source3/winbindd/winbindd_util.c | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index 05ef2ec..cde9099 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -1133,6 +1133,14 @@ static NTSTATUS trusted_domains(struct winbindd_domain *domain,
 			}
 			TALLOC_FREE(parent);
 
+			/*
+			 * We need to pass the modified properties
+			 * to the caller.
+			 */
+			trust->trust_flags = d.domain_flags;
+			trust->trust_type = d.domain_type;
+			trust->trust_attributes = d.domain_trust_attribs;
+
 			wcache_tdc_add_domain( &d );
 			ret_count++;
 		}
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index ffcb09d..ab6862d 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -342,6 +342,20 @@ static void trustdom_list_done(struct tevent_req *req)
 	char *p;
 	struct winbindd_tdc_domain trust_params = {0};
 	ptrdiff_t extra_len;
+	bool within_forest = false;
+
+	/*
+	 * Only when we enumerate our primary domain
+	 * or our forest root domain, we should keep
+	 * the NETR_TRUST_FLAG_IN_FOREST flag, in
+	 * all other cases we need to clear it as the domain
+	 * is not part of our forest.
+	 */
+	if (state->domain->primary) {
+		within_forest = true;
+	} else if (domain_is_forest_root(state->domain)) {
+		within_forest = true;
+	}
 
 	res = wb_domain_request_recv(req, state, &response, &err);
 	if ((res == -1) || (response->result != WINBINDD_OK)) {
@@ -427,6 +441,14 @@ static void trustdom_list_done(struct tevent_req *req)
 
 		trust_params.trust_attribs = (uint32_t)strtoul(q, NULL, 10);
 
+		if (!within_forest) {
+			trust_params.trust_flags &= ~NETR_TRUST_FLAG_IN_FOREST;
+		}
+
+		if (!state->domain->primary) {
+			trust_params.trust_flags &= ~NETR_TRUST_FLAG_PRIMARY;
+		}
+
 		/*
 		 * We always call add_trusted_domain() cause on an existing
 		 * domain structure, it will update the SID if necessary.
-- 
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/20170302/c16710ae/signature.sig>