[PATCH] winbindd: support foreign domains when NTLM is restricted

Uri Simchoni uri at samba.org
Tue Feb 9 22:57:18 UTC 2016


Hi,

Attached is a patch to fix 
https://bugzilla.samba.org/show_bug.cgi?id=11691 - winbindd doesn't 
connect to a trusted domain if the DC of that domain refuses NTLM 
authentication. Review appreciated.

The basic idea is to pre-mark the domain as AD domain based on trust 
properties, thus allowing the initial connection to be attempted using 
Kerberos.

The patch is marked V2 because I circulated a similar patch a few days 
ago - basically it's a bit cleaner and possibly more correct in a couple 
of places (re-scanning root domain).

In the previous message I expressed my desire to further clean up the 
code in this area, namely that foreign domain properties will be 
determined ONLY based on trust properties, not by contacting the domain, 
and possibly some other cleanups - this will wait for another time I'm 
afraid, and also this limited change is safer for backporting.

Thanks,
Uri.

-------------- next part --------------
From 70f9328a799a374a057db5651b9b17a26feca759 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 10 Feb 2016 00:26:45 +0200
Subject: [PATCH v2 1/3] winbindd: introduce add_trusted_domain_from_tdc()

This is purely a refactoring patch -
Add a routine that adds a winbindd domain object based on
domain trust cache entry. add_trusted_domain() becomes
a wrapper for this new routine.

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

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/winbindd/winbindd_util.c | 72 +++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 8139daa..307ae6d 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -34,6 +34,10 @@
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
 
+static struct winbindd_domain *
+add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
+			    struct winbindd_methods *methods);
+
 extern struct winbindd_methods cache_methods;
 
 /**
@@ -119,14 +123,40 @@ static bool is_in_internal_domain(const struct dom_sid *sid)
    If the domain already exists in the list,
    return it and don't re-initialize.  */
 
-static struct winbindd_domain *add_trusted_domain(const char *domain_name, const char *alt_name,
-						  struct winbindd_methods *methods,
-						  const struct dom_sid *sid)
+static struct winbindd_domain *
+add_trusted_domain(const char *domain_name, const char *alt_name,
+		   struct winbindd_methods *methods, const struct dom_sid *sid)
+{
+	struct winbindd_tdc_domain tdc;
+
+	ZERO_STRUCT(tdc);
+
+	tdc.domain_name = domain_name;
+	tdc.dns_name = alt_name;
+	if (sid) {
+		sid_copy(&tdc.sid, sid);
+	}
+
+	return add_trusted_domain_from_tdc(&tdc, methods);
+}
+
+/* Add a trusted domain out of a trusted domain cache
+   entry
+*/
+static struct winbindd_domain *
+add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
+			    struct winbindd_methods *methods)
 {
 	struct winbindd_domain *domain;
 	const char *alternative_name = NULL;
 	const char **ignored_domains, **dom;
 	int role = lp_server_role();
+	const char *domain_name = tdc->domain_name;
+	const struct dom_sid *sid = &tdc->sid;
+
+	if (is_null_sid(sid)) {
+		sid = NULL;
+	}
 
 	ignored_domains = lp_parm_string_list(-1, "winbind", "ignore domains", NULL);
 	for (dom=ignored_domains; dom && *dom; dom++) {
@@ -138,8 +168,8 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
 
 	/* use alt_name if available to allow DNS lookups */
 
-	if (alt_name && *alt_name) {
-		alternative_name = alt_name;
+	if (tdc->dns_name && *tdc->dns_name) {
+		alternative_name = tdc->dns_name;
 	}
 
 	/* We can't call domain_list() as this function is called from
@@ -151,8 +181,7 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
 			break;
 		}
 
-		if (alternative_name && *alternative_name)
-		{
+		if (alternative_name) {
 			if (strequal(alternative_name, domain->name) ||
 			    strequal(alternative_name, domain->alt_name))
 			{
@@ -160,12 +189,7 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
 			}
 		}
 
-		if (sid)
-		{
-			if (is_null_sid(sid)) {
-				continue;
-			}
-
+		if (sid != NULL) {
 			if (dom_sid_equal(sid, &domain->sid)) {
 				break;
 			}
@@ -219,11 +243,11 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
 	domain->internal = is_internal_domain(sid);
 	domain->sequence_number = DOM_SEQUENCE_NONE;
 	domain->last_seq_check = 0;
-	domain->initialized = False;
+	domain->initialized = false;
 	domain->online = is_internal_domain(sid);
 	domain->check_online_timeout = 0;
 	domain->dc_probe_pid = (pid_t)-1;
-	if (sid) {
+	if (sid != NULL) {
 		sid_copy(&domain->sid, sid);
 	}
 
@@ -252,9 +276,9 @@ static struct winbindd_domain *add_trusted_domain(const char *domain_name, const
 
 	setup_domain_child(domain);
 
-	DEBUG(2,("Added domain %s %s %s\n",
-		 domain->name, domain->alt_name,
-		 &domain->sid?sid_string_dbg(&domain->sid):""));
+	DEBUG(2,
+	      ("Added domain %s %s %s\n", domain->name, domain->alt_name,
+	       !is_null_sid(&domain->sid) ? sid_string_dbg(&domain->sid) : ""));
 
 	return domain;
 }
@@ -438,10 +462,8 @@ static void rescan_forest_root_trusts( void )
 		d = find_domain_from_name_noinit( dom_list[i].domain_name );
 
 		if ( !d ) {
-			d = add_trusted_domain( dom_list[i].domain_name,
-						dom_list[i].dns_name,
-						&cache_methods,
-						&dom_list[i].sid );
+			d = add_trusted_domain_from_tdc(&dom_list[i],
+							&cache_methods);
 		}
 
 		if (d == NULL) {
@@ -507,10 +529,8 @@ static void rescan_forest_trusts( void )
 			   about it */
 
 			if ( !d ) {
-				d = add_trusted_domain( dom_list[i].domain_name,
-							dom_list[i].dns_name,
-							&cache_methods,
-							&dom_list[i].sid );
+				d = add_trusted_domain_from_tdc(&dom_list[i],
+								&cache_methods);
 			}
 
 			if (d == NULL) {
-- 
2.5.0


From e3596024427fda6a02e1e5a5058455e45630cc88 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 10 Feb 2016 00:32:23 +0200
Subject: [PATCH v2 2/3] winbindd: initialize foreign domain as AD based on
 trust

Based on trust parameters, initialize the active_directory
member of domain object to true.

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

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/winbindd/winbindd_util.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 307ae6d..cb8d80f 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -250,6 +250,9 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
 	if (sid != NULL) {
 		sid_copy(&domain->sid, sid);
 	}
+	domain->domain_flags = tdc->trust_flags;
+	domain->domain_type = tdc->trust_type;
+	domain->domain_trust_attribs = tdc->trust_attribs;
 
 	/* Is this our primary domain ? */
 	if (strequal(domain_name, get_global_sam_name()) &&
@@ -267,6 +270,10 @@ add_trusted_domain_from_tdc(const struct winbindd_tdc_domain *tdc,
 		if (lp_security() == SEC_ADS) {
 			domain->active_directory = true;
 		}
+	} else if (!domain->internal) {
+		if (domain->domain_type == LSA_TRUST_TYPE_UPLEVEL) {
+			domain->active_directory = true;
+		}
 	}
 
 	/* Link to domain list */
-- 
2.5.0


From fa4b826b9d010f993ad6183cf9a1a63a2cf23ca1 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 10 Feb 2016 00:38:11 +0200
Subject: [PATCH v2 3/3] winbindd: return trust parameters when listing trusts

When asking a child domain process to list trusts on that domain,
return (along with trust domain names and SID) the trust properties -
flags, type, and attributes.

Use those attributes to initialize domain object.

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

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/winbindd/winbindd_misc.c | 11 +++---
 source3/winbindd/winbindd_util.c | 74 +++++++++++++++++++++++++++++-----------
 2 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c
index 29831aa..d32a71e 100644
--- a/source3/winbindd/winbindd_misc.c
+++ b/source3/winbindd/winbindd_misc.c
@@ -181,11 +181,12 @@ enum winbindd_result winbindd_dual_list_trusted_domains(struct winbindd_domain *
 		}
 
 		extra_data = talloc_asprintf_append_buffer(
-			extra_data, "%s\\%s\\%s\n",
-			trusts.array[i].netbios_name,
-			trusts.array[i].dns_name,
-			sid_string_talloc(state->mem_ctx,
-					  trusts.array[i].sid));
+		    extra_data, "%s\\%s\\%s\\%u\\%u\\%u\n",
+		    trusts.array[i].netbios_name, trusts.array[i].dns_name,
+		    sid_string_talloc(state->mem_ctx, trusts.array[i].sid),
+		    trusts.array[i].trust_flags,
+		    (uint32_t)trusts.array[i].trust_type,
+		    trusts.array[i].trust_attributes);
 	}
 
 	/* add our primary domain */
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index cb8d80f..dfc5ea3 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -343,24 +343,37 @@ static void trustdom_list_done(struct tevent_req *req)
 	struct winbindd_response *response;
 	int res, err;
 	char *p;
+	struct winbindd_tdc_domain trust_params = {0};
+	ptrdiff_t extra_len;
 
 	res = wb_domain_request_recv(req, state, &response, &err);
 	if ((res == -1) || (response->result != WINBINDD_OK)) {
-		DEBUG(1, ("Could not receive trustdoms\n"));
+		DBG_WARNING("Could not receive trustdoms\n");
 		TALLOC_FREE(state);
 		return;
 	}
 
+	if (response->length < sizeof(struct winbindd_response)) {
+		DBG_ERR("ill-formed trustdom response - short length\n");
+		TALLOC_FREE(state);
+		return;
+	}
+
+	extra_len = response->length - sizeof(struct winbindd_response);
+
 	p = (char *)response->extra_data.data;
 
-	while ((p != NULL) && (*p != '\0')) {
+	while ((p - (char *)response->extra_data.data) < extra_len) {
 		char *q, *sidstr, *alt_name;
-		struct dom_sid sid;
-		char *alternate_name = NULL;
+
+		DBG_DEBUG("parsing response line '%s'\n", p);
+
+		ZERO_STRUCT(trust_params);
+		trust_params.domain_name = p;
 
 		alt_name = strchr(p, '\\');
 		if (alt_name == NULL) {
-			DEBUG(0, ("Got invalid trustdom response\n"));
+			DBG_ERR("Got invalid trustdom response\n");
 			break;
 		}
 
@@ -369,26 +382,52 @@ static void trustdom_list_done(struct tevent_req *req)
 
 		sidstr = strchr(alt_name, '\\');
 		if (sidstr == NULL) {
-			DEBUG(0, ("Got invalid trustdom response\n"));
+			DBG_ERR("Got invalid trustdom response\n");
 			break;
 		}
 
 		*sidstr = '\0';
 		sidstr += 1;
 
-		q = strchr(sidstr, '\n');
-		if (q != NULL)
-			*q = '\0';
+		/* use the real alt_name if we have one, else pass in NULL */
+		if (!strequal(alt_name, "(null)")) {
+			trust_params.dns_name = alt_name;
+		}
+
+		q = strtok(sidstr, "\\");
+		if (q == NULL) {
+			DBG_ERR("Got invalid trustdom response\n");
+			break;
+		}
 
-		if (!string_to_sid(&sid, sidstr)) {
+		if (!string_to_sid(&trust_params.sid, sidstr)) {
 			DEBUG(0, ("Got invalid trustdom response\n"));
 			break;
 		}
 
-		/* use the real alt_name if we have one, else pass in NULL */
+		q = strtok(NULL, "\\");
+		if (q == NULL) {
+			DBG_ERR("Got invalid trustdom response\n");
+			break;
+		}
+
+		trust_params.trust_flags = (uint32_t)strtoul(q, NULL, 10);
+
+		q = strtok(NULL, "\\");
+		if (q == NULL) {
+			DBG_ERR("Got invalid trustdom response\n");
+			break;
+		}
+
+		trust_params.trust_type = (uint32_t)strtoul(q, NULL, 10);
+
+		q = strtok(NULL, "\n");
+		if (q == NULL) {
+			DBG_ERR("Got invalid trustdom response\n");
+			break;
+		}
 
-		if ( !strequal( alt_name, "(null)" ) )
-			alternate_name = alt_name;
+		trust_params.trust_attribs = (uint32_t)strtoul(q, NULL, 10);
 
 		/*
 		 * We always call add_trusted_domain() cause on an existing
@@ -396,13 +435,10 @@ static void trustdom_list_done(struct tevent_req *req)
 		 * This is important because we need the SID for sibling
 		 * domains.
 		 */
-		(void)add_trusted_domain(p, alternate_name,
-					    &cache_methods,
-					    &sid);
+		(void)add_trusted_domain_from_tdc(&trust_params,
+						  &cache_methods);
 
-		p=q;
-		if (p != NULL)
-			p += 1;
+		p = q + strlen(q) + 1;
 	}
 
 	/*
-- 
2.5.0



More information about the samba-technical mailing list