[Patches] Winbindd (on an AD DC) should only use netlogon/lsa against trusted domains (bug #13278)

Stefan Metzmacher metze at samba.org
Thu Feb 22 21:08:59 UTC 2018


Hi,

here're some patches which make sure we only use netlogon/lsa calls
against trusted domains, as that's the only that work reliable
and it provides everything we need in order to act as AD DC.
(In theory it should be everything we for a member and NT4 dc too,
but that's a longer story and there we already have existing users with
expected behaviours).

See https://bugzilla.samba.org/show_bug.cgi?id=13278

This just passed a private autobuild.

Please review and push:-)

Thanks!
metze
-------------- next part --------------
From 569462c6343f1f374ebb68f36cf1bba25d3628bd Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 10:03:23 +0100
Subject: [PATCH 1/7] s3:smb_macros.h: add IS_AD_DC as addition to IS_DC

In the long run we should remove this again (as well as IS_DC).

But for now this makes some code changes in winbindd easier to
follow.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/include/smb_macros.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index f1191ac..06d2474 100644
--- a/source3/include/smb_macros.h
+++ b/source3/include/smb_macros.h
@@ -214,6 +214,7 @@ copy an IP address from one buffer to another
 *****************************************************************************/
 
 #define IS_DC  (lp_server_role()==ROLE_DOMAIN_PDC || lp_server_role()==ROLE_DOMAIN_BDC || lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) 
+#define IS_AD_DC  (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC)
 
 /*
  * If you add any entries to KERBEROS_VERIFY defines, please modify the below expressions
-- 
1.9.1


From c43381f153497b9cdca12942ef43ee90d04396d3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 10:40:19 +0100
Subject: [PATCH 2/7] winbind: force the usage of schannel in cm_connect_lsa()
 as AD DC

This makes sure we only talk to direct trusts.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_cm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 4229647..8b1cf8c 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -2970,6 +2970,13 @@ retry:
 
 	TALLOC_FREE(conn->lsa_pipe);
 
+	if (IS_AD_DC) {
+		/*
+		 * Make sure we only use schannel as AD DC.
+		 */
+		goto schannel;
+	}
+
 	result = get_trust_credentials(domain, talloc_tos(), false, &creds);
 	if (!NT_STATUS_IS_OK(result)) {
 		DEBUG(10, ("cm_connect_lsa: No user available for "
@@ -3083,6 +3090,13 @@ retry:
 		goto done;
 	}
 
+	if (IS_AD_DC) {
+		/*
+		 * Make sure we only use schannel as AD DC.
+		 */
+		goto done;
+	}
+
 	DEBUG(10,("cm_connect_lsa: rpccli_lsa_open_policy failed, trying "
 		  "anonymous\n"));
 
-- 
1.9.1


From 03f8d9dee46b04b0e1c8d5b4c699abd0805e3001 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 10:40:19 +0100
Subject: [PATCH 3/7] winbind: let cm_connect_netlogon_transport() only work
 against direct trust as AD DC

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_cm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 8b1cf8c..f5f0976 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -3205,6 +3205,17 @@ static NTSTATUS cm_connect_netlogon_transport(struct winbindd_domain *domain,
 
 	*cli = NULL;
 
+	if (IS_AD_DC) {
+		if (domain->secure_channel_type == SEC_CHAN_NULL) {
+			/*
+			 * Make sure we don't even try to
+			 * connect to a foreign domain
+			 * without a direct outbound trust.
+			 */
+			return NT_STATUS_NO_TRUST_LSA_SECRET;
+		}
+	}
+
 	result = init_dc_connection_rpc(domain, domain->rodc);
 	if (!NT_STATUS_IS_OK(result)) {
 		return result;
-- 
1.9.1


From 0fc2edae284c2c4998e214d0b9b827bdfdee572b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 10:33:48 +0100
Subject: [PATCH 4/7] winbind: make sure we don't contact trusted domains via
 SAMR as AD DC

This is not needed for the normal operation of an AD DC.

Administrators should just use other tools instead of
wbinfo to list and query users and groups.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_cm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index f5f0976..4ea447f 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -2650,6 +2650,20 @@ NTSTATUS cm_connect_sam(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx,
 		}
 	}
 
+	if (IS_AD_DC) {
+		/*
+		 * In theory we should not use SAMR within
+		 * winbindd at all, but that's a larger task to
+		 * remove this and avoid breaking existing
+		 * setups.
+		 *
+		 * At least as AD DC we have the restriction
+		 * to avoid SAMR against trusted domains,
+		 * as there're no existing setups.
+		 */
+		return NT_STATUS_REQUEST_NOT_ACCEPTED;
+	}
+
 retry:
 	status = init_dc_connection_rpc(domain, need_rw_dc);
 	if (!NT_STATUS_IS_OK(status)) {
-- 
1.9.1


From f0ef70783a7b1abc114b62ac17f93149c9146a45 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 2 Feb 2018 16:55:01 +0100
Subject: [PATCH 5/7] winbind: make sure we don't contact trusted domains via
 LDAP as AD DC

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/idmap_ad.c     | 11 +++++++++++
 source3/winbindd/winbindd_ads.c | 23 +++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/source3/winbindd/idmap_ad.c b/source3/winbindd/idmap_ad.c
index 315a944..1530410 100644
--- a/source3/winbindd/idmap_ad.c
+++ b/source3/winbindd/idmap_ad.c
@@ -532,6 +532,17 @@ static NTSTATUS idmap_ad_get_context(struct idmap_domain *dom,
 	struct idmap_ad_context *ctx = NULL;
 	NTSTATUS status;
 
+	if (IS_AD_DC) {
+		/*
+		 * Make sure we never try to use LDAP against
+		 * a trusted domain as AD_DC.
+		 *
+		 * This shouldn't be called currently,
+		 * but you never know what happens in future.
+		 */
+		return NT_STATUS_REQUEST_NOT_ACCEPTED;
+	}
+
 	if (dom->private_data != NULL) {
 		*pctx = talloc_get_type_abort(dom->private_data,
 					      struct idmap_ad_context);
diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index c330b92..725fa4f 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -159,6 +159,14 @@ ADS_STATUS ads_idmap_cached_connection(ADS_STRUCT **adsp, const char *dom_name)
 	struct winbindd_domain *wb_dom;
 	ADS_STATUS status;
 
+	if (IS_AD_DC) {
+		/*
+		 * Make sure we never try to use LDAP against
+		 * a trusted domain as AD DC.
+		 */
+		return ADS_ERROR_NT(NT_STATUS_REQUEST_NOT_ACCEPTED);
+	}
+
 	ads_cached_connection_reuse(adsp);
 	if (*adsp != NULL) {
 		return ADS_SUCCESS;
@@ -231,6 +239,14 @@ static ADS_STRUCT *ads_cached_connection(struct winbindd_domain *domain)
 	ADS_STATUS status;
 	char *password, *realm;
 
+	if (IS_AD_DC) {
+		/*
+		 * Make sure we never try to use LDAP against
+		 * a trusted domain as AD DC.
+		 */
+		return NULL;
+	}
+
 	DEBUG(10,("ads_cached_connection\n"));
 	ads_cached_connection_reuse((ADS_STRUCT **)&domain->private_data);
 
@@ -1309,6 +1325,13 @@ static NTSTATUS sequence_number(struct winbindd_domain *domain, uint32_t *seq)
 		return NT_STATUS_OK;
 	}
 
+	if (IS_AD_DC) {
+		DEBUG(10,("sequence: Avoid LDAP connection for domain %s\n",
+			  domain->name));
+		*seq = time(NULL);
+		return NT_STATUS_OK;
+	}
+
 	*seq = DOM_SEQUENCE_NONE;
 
 	ads = ads_cached_connection(domain);
-- 
1.9.1


From cb0f09e948f723e637e0b9aebf2c6c5b42c41d78 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 11:24:38 +0100
Subject: [PATCH 6/7] winbind: set_dc_type_and_flags() is not needed on a DC

On a DC we load the trusts in the parent in add_trusted_domains_dc()
from our local configuration. There's no need to find out the trust details
via network calls.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_cm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index 4ea447f..c33a6e9 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -2190,6 +2190,15 @@ static bool set_dc_type_and_flags_trustinfo( struct winbindd_domain *domain )
 	TALLOC_CTX *mem_ctx = NULL;
 	struct dcerpc_binding_handle *b;
 
+	if (IS_DC) {
+		/*
+		 * On a DC we loaded all trusts
+		 * from configuration and never learn
+		 * new domains.
+		 */
+		return true;
+	}
+
 	DEBUG(5, ("set_dc_type_and_flags_trustinfo: domain %s\n", domain->name ));
 
 	/* Our primary domain doesn't need to worry about trust flags.
@@ -2584,6 +2593,15 @@ done:
 
 static void set_dc_type_and_flags( struct winbindd_domain *domain )
 {
+	if (IS_DC) {
+		/*
+		 * On a DC we loaded all trusts
+		 * from configuration and never learn
+		 * new domains.
+		 */
+		return;
+	}
+
 	/* we always have to contact our primary domain */
 
 	if ( domain->primary || domain->internal) {
-- 
1.9.1


From bf38b279ec72e1c09e78b5532b2704dd05ce1f4e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 10:19:58 +0100
Subject: [PATCH 7/7] winbind: don't try to do an authenticated SMB connection
 as AD DC

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/winbindd_cm.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/source3/winbindd/winbindd_cm.c b/source3/winbindd/winbindd_cm.c
index c33a6e9..c5c2114 100644
--- a/source3/winbindd/winbindd_cm.c
+++ b/source3/winbindd/winbindd_cm.c
@@ -999,6 +999,28 @@ static NTSTATUS cm_prepare_connection(struct winbindd_domain *domain,
 
 	enum smb_signing_setting smb_sign_client_connections = lp_client_ipc_signing();
 
+	if (IS_AD_DC) {
+		if (domain->secure_channel_type == SEC_CHAN_NULL) {
+			/*
+			 * Make sure we don't even try to
+			 * connect to a foreign domain
+			 * without a direct outbound trust.
+			 */
+			return NT_STATUS_NO_TRUST_LSA_SECRET;
+		}
+
+		/*
+		 * As AD DC we only use netlogon and lsa
+		 * using schannel over an anonymous transport
+		 * (ncacn_ip_tcp or ncacn_np).
+		 *
+		 * In most cases we won't use the SMB connection
+		 * anyway, but at least use an anonymous connection
+		 * in order to avoid getting problems with kerberos.
+		 */
+		smb_sign_client_connections = SMB_SIGNING_OFF;
+	}
+
 	if (smb_sign_client_connections == SMB_SIGNING_DEFAULT) {
 		/*
 		 * If we are connecting to our own AD domain, require
@@ -1011,8 +1033,7 @@ static NTSTATUS cm_prepare_connection(struct winbindd_domain *domain,
 		 * AD domain in our forest
 		 * then require smb signing to disrupt MITM attacks
 		 */
-		} else if ((lp_security() == SEC_ADS ||
-			    lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC)
+		} else if ((lp_security() == SEC_ADS)
 			   && domain->active_directory
 			   && (domain->domain_trust_attribs
 			       & LSA_TRUST_ATTRIBUTE_WITHIN_FOREST)) {
@@ -1071,6 +1092,19 @@ static NTSTATUS cm_prepare_connection(struct winbindd_domain *domain,
 		try_ipc_auth = true;
 	}
 
+	if (IS_AD_DC) {
+		/*
+		 * As AD DC we only use netlogon and lsa
+		 * using schannel over an anonymous transport
+		 * (ncacn_ip_tcp or ncacn_np).
+		 *
+		 * In most cases we won't use the SMB connection
+		 * anyway, but at least use an anonymous connection
+		 * in order to avoid getting problems with kerberos.
+		 */
+		try_ipc_auth = false;
+	}
+
 	if (try_ipc_auth) {
 		result = get_trust_credentials(domain, talloc_tos(), false, &creds);
 		if (!NT_STATUS_IS_OK(result)) {
-- 
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/20180222/c1bee23d/signature.sig>


More information about the samba-technical mailing list