[Patches] Expanded group memberships on boundaries of outgoing trusts (bugs #13299, #13300, #13307)

Stefan Metzmacher metze at samba.org
Tue Feb 27 16:54:57 UTC 2018


Hi,

here are patches, which implement the required group membership
expanding on an AD DC, when we got the users token from a trusted domain.

In order to have tests for this, I've modified
"samba-tool group {add,remove}members" to accept a sid string
instead of a user/group name.

Under the hood, a member is specified as
'<SID=S-1-5-21-222-333-444-555>' and the new fpo_attr module,
creates a foreignSecurityPrincipal object on the fly.

In order to match the Windows behaviour, it's no longer
possible to create foreignSecurityPrincipal objects directly.

While being there I had to fix a bug in the "extended_dn_store"
module, where we allowed linked attributes to deleted objects,
which should only be allowed for non-linked attributes with
DN syntax.

In order to protect administrators from believing, we would
support for "selective authentication" (CROSS_ORIGANIZATION) trusts,
I added code to ignore these in winbindd and the kdc for now.

I have working code to also implement SID-Filtering and also
selective authentication, but the patches need more cleanup,
which is more likely something for the 4.9 release.

With the attached patches (which I'd like to see in 4.8),
we will be in a very good shape and support a lot of trust
setups as AD DC. I'll write up a WHATSNEW section about the
whole trust situation in the next days.

Please review and push:-)

Thanks!
metze
-------------- next part --------------
From 8370f44002fc4ffbc2f7d71a8776de7cfd477580 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 11:06:10 +0100
Subject: [PATCH 01/26] winbindd: disable support for CROSS_ORGANIZATION
 domains

We don't support selective authentication yet,
so we shouldn't silently allow domain wide authentication
for such a trust.

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

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

diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 73e6b76..b19c42f 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -961,6 +961,17 @@ static bool add_trusted_domains_dc(void)
 			trust_flags |= NETR_TRUST_FLAG_IN_FOREST;
 		}
 
+		if (domains[i]->trust_attributes & LSA_TRUST_ATTRIBUTE_CROSS_ORGANIZATION) {
+			/*
+			 * We don't support selective authentication yet.
+			 */
+			DBG_WARNING("Ignoring CROSS_ORGANIZATION trust to "
+				    "domain[%s/%s]\n",
+				    domains[i]->netbios_name,
+				    domains[i]->domain_name);
+			continue;
+		}
+
 		status = add_trusted_domain(domains[i]->netbios_name,
 					    domains[i]->domain_name,
 					    &domains[i]->security_identifier,
-- 
1.9.1


From 51ceaf12a054d7fd429ab21ae2993e86ee1ece2e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 11:10:14 +0100
Subject: [PATCH 02/26] s4:kdc: make use of dsdb_trust_parse_tdo_info() in
 samba_kdc_trust_message2entry()

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/kdc/db-glue.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 69c54b0..5ca2c06 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -57,14 +57,17 @@ enum trust_direction {
 };
 
 static const char *trust_attrs[] = {
+	"securityIdentifier",
+	"flatName",
 	"trustPartner",
+	"trustAttributes",
+	"trustDirection",
+	"trustType",
+	"msDS-TrustForestTrustInfo",
 	"trustAuthIncoming",
 	"trustAuthOutgoing",
 	"whenCreated",
 	"msDS-SupportedEncryptionTypes",
-	"trustAttributes",
-	"trustDirection",
-	"trustType",
 	NULL
 };
 
@@ -1167,7 +1170,6 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 {
 	struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx;
 	const char *our_realm = lpcfg_realm(lp_ctx);
-	const char *dnsdomain = NULL;
 	char *partner_realm = NULL;
 	const char *realm = NULL;
 	const char *krbtgt_realm = NULL;
@@ -1183,7 +1185,7 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 	uint32_t previous_kvno;
 	uint32_t num_keys = 0;
 	enum ndr_err_code ndr_err;
-	int ret, trust_direction_flags;
+	int ret;
 	unsigned int i;
 	struct AuthenticationInformationArray *auth_array;
 	struct timeval tv;
@@ -1191,6 +1193,8 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 	uint32_t *auth_kvno;
 	bool preferr_current = false;
 	uint32_t supported_enctypes = ENC_RC4_HMAC_MD5;
+	struct lsa_TrustDomainInfoInfoEx *tdo = NULL;
+	NTSTATUS status;
 
 	if (dsdb_functional_level(kdc_db_ctx->samdb) >= DS_DOMAIN_FUNCTION_2008) {
 		supported_enctypes = ldb_msg_find_attr_as_uint(msg,
@@ -1198,20 +1202,25 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 					supported_enctypes);
 	}
 
-	trust_direction_flags = ldb_msg_find_attr_as_int(msg, "trustDirection", 0);
-	if (!(trust_direction_flags & direction)) {
+	status = dsdb_trust_parse_tdo_info(mem_ctx, msg, &tdo);
+	if (!NT_STATUS_IS_OK(status)) {
+		krb5_clear_error_message(context);
+		ret = ENOMEM;
+		goto out;
+	}
+
+	if (!(tdo->trust_direction & direction)) {
 		krb5_clear_error_message(context);
 		ret = SDB_ERR_NOENTRY;
 		goto out;
 	}
 
-	dnsdomain = ldb_msg_find_attr_as_string(msg, "trustPartner", NULL);
-	if (dnsdomain == NULL) {
+	if (tdo->domain_name.string == NULL) {
 		krb5_clear_error_message(context);
 		ret = SDB_ERR_NOENTRY;
 		goto out;
 	}
-	partner_realm = strupper_talloc(mem_ctx, dnsdomain);
+	partner_realm = strupper_talloc(mem_ctx, tdo->domain_name.string);
 	if (partner_realm == NULL) {
 		krb5_clear_error_message(context);
 		ret = ENOMEM;
-- 
1.9.1


From 58731cfe67e75b85e872ba959caf0e5e314583fa Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 11:10:14 +0100
Subject: [PATCH 03/26] s4:kdc: only support LSA_TRUST_TYPE_UPLEVEL domains in
 samba_kdc_trust_message2entry()

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/kdc/db-glue.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 5ca2c06..9d633a6 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1215,6 +1215,16 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 		goto out;
 	}
 
+	if (tdo->trust_type != LSA_TRUST_TYPE_UPLEVEL) {
+		/*
+		 * Only UPLEVEL domains support kerberos here,
+		 * as we don't support LSA_TRUST_TYPE_MIT.
+		 */
+		krb5_clear_error_message(context);
+		ret = SDB_ERR_NOENTRY;
+		goto out;
+	}
+
 	if (tdo->domain_name.string == NULL) {
 		krb5_clear_error_message(context);
 		ret = SDB_ERR_NOENTRY;
-- 
1.9.1


From df5c8c128a6609fded01752b039ff50ba4a0996c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 11:06:10 +0100
Subject: [PATCH 04/26] s4:kdc: disable support for CROSS_ORGANIZATION domains

We don't support selective authentication yet,
so we shouldn't silently allow domain wide authentication
for such a trust.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/kdc/db-glue.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 9d633a6..8ccc34c 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1225,6 +1225,15 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 		goto out;
 	}
 
+	if (tdo->trust_attributes & LSA_TRUST_ATTRIBUTE_CROSS_ORGANIZATION) {
+		/*
+		 * We don't support selective authentication yet.
+		 */
+		krb5_clear_error_message(context);
+		ret = SDB_ERR_NOENTRY;
+		goto out;
+	}
+
 	if (tdo->domain_name.string == NULL) {
 		krb5_clear_error_message(context);
 		ret = SDB_ERR_NOENTRY;
-- 
1.9.1


From 33fc351a473e82f854db068d5e6fe7d60409c391 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 25 Feb 2018 00:10:12 +0100
Subject: [PATCH 05/26] tests/dsdb.py: prove the difference between linked and
 non-linked DN references

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/tests/dsdb.py                | 146 +++++++++++++++++++++++++++++-
 selftest/knownfail.d/linked_vs_non_linked |   1 +
 2 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 selftest/knownfail.d/linked_vs_non_linked

diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index fd9919f..231abee 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -23,7 +23,7 @@ from samba.auth import system_session
 from samba.tests import TestCase
 from samba.tests import delete_force
 from samba.ndr import ndr_unpack, ndr_pack
-from samba.dcerpc import drsblobs, security
+from samba.dcerpc import drsblobs, security, misc
 from samba import dsdb
 import ldb
 import samba
@@ -269,6 +269,150 @@ class DsdbTests(TestCase):
                           "LDB_ERR_CONSTRAINT_VIOLATION"
                           % (code, msg))
 
+    def test_linked_vs_non_linked_reference(self):
+        basedn   = self.samdb.get_default_basedn()
+        kept_dn_str   = "cn=reference_kept,cn=Users,%s" % (basedn)
+        removed_dn_str   = "cn=reference_removed,cn=Users,%s" % (basedn)
+        dom_sid = self.samdb.get_domain_sid()
+        none_sid_str = str(dom_sid) + "-4294967294"
+        none_guid_str = "afafafaf-fafa-afaf-fafa-afafafafafaf"
+
+        self.addCleanup(delete_force, self.samdb, kept_dn_str)
+        self.addCleanup(delete_force, self.samdb, removed_dn_str)
+
+        self.samdb.add({
+            "dn": kept_dn_str,
+            "objectClass": "user"})
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=kept_dn_str,
+                                attrs=["objectGUID", "objectSID"])
+        self.assertEqual(len(res), 1)
+        kept_guid = ndr_unpack(misc.GUID, res[0]["objectGUID"][0])
+        kept_sid = ndr_unpack(security.dom_sid, res[0]["objectSid"][0])
+        kept_dn = res[0].dn
+
+        self.samdb.add({
+            "dn": removed_dn_str,
+            "objectClass": "user"})
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=removed_dn_str,
+                                attrs=["objectGUID", "objectSID"])
+        self.assertEqual(len(res), 1)
+        removed_guid = ndr_unpack(misc.GUID, res[0]["objectGUID"][0])
+        removed_sid = ndr_unpack(security.dom_sid, res[0]["objectSid"][0])
+        self.samdb.delete(removed_dn_str)
+
+        #
+        # First try the linked attribute 'manager'
+        # by GUID and SID
+        #
+
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["manager"] = ldb.MessageElement("<SID=%s>" % removed_sid,
+                                           ldb.FLAG_MOD_ADD,
+                                           "manager")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_CONSTRAINT_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_NAME_REFERENCE_INVALID
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["manager"] = ldb.MessageElement("<GUID=%s>" % removed_guid,
+                                           ldb.FLAG_MOD_ADD,
+                                           "manager")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_CONSTRAINT_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_NAME_REFERENCE_INVALID
+            self.assertTrue(werr in msg, msg)
+
+        #
+        # Try the non-linked attribute 'assistant'
+        # by GUID and SID, which should work.
+        #
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<SID=%s>" % removed_sid,
+                                              ldb.FLAG_MOD_ADD,
+                                              "assistant")
+        self.samdb.modify(msg)
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<SID=%s>" % removed_sid,
+                                              ldb.FLAG_MOD_DELETE,
+                                              "assistant")
+        self.samdb.modify(msg)
+
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<GUID=%s>" % removed_guid,
+                                              ldb.FLAG_MOD_ADD,
+                                              "assistant")
+        self.samdb.modify(msg)
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<GUID=%s>" % removed_guid,
+                                              ldb.FLAG_MOD_DELETE,
+                                              "assistant")
+        self.samdb.modify(msg)
+
+        #
+        # Finally ry the non-linked attribute 'assistant'
+        # but with non existing GUID, SID, DN
+        #
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("CN=NoneNone,%s" % (basedn),
+                                              ldb.FLAG_MOD_ADD,
+                                              "assistant")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_CONSTRAINT_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_NAME_REFERENCE_INVALID
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<SID=%s>" % none_sid_str,
+                                              ldb.FLAG_MOD_ADD,
+                                              "assistant")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_CONSTRAINT_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_NAME_REFERENCE_INVALID
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = kept_dn
+        msg["assistant"] = ldb.MessageElement("<GUID=%s>" % none_guid_str,
+                                              ldb.FLAG_MOD_ADD,
+                                              "assistant")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_CONSTRAINT_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_NAME_REFERENCE_INVALID
+            self.assertTrue(werr in msg, msg)
+
+        self.samdb.delete(kept_dn)
+
     def test_normalize_dn_in_domain_full(self):
         domain_dn = self.samdb.domain_dn()
 
diff --git a/selftest/knownfail.d/linked_vs_non_linked b/selftest/knownfail.d/linked_vs_non_linked
new file mode 100644
index 0000000..281f4e5
--- /dev/null
+++ b/selftest/knownfail.d/linked_vs_non_linked
@@ -0,0 +1 @@
+^samba.tests.dsdb.*samba.tests.dsdb.DsdbTests.test_linked_vs_non_linked_reference
-- 
1.9.1


From af97bb3a0c1589010b5fe39ea7f1846a8c29cc74 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 13:21:54 +0100
Subject: [PATCH 06/26] dsdb:extended_dn_store: pass the full 'struct
 dsdb_attribute' to extended_store_replace()

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/dsdb/samdb/ldb_modules/extended_dn_store.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index a32ab8d..14c0d5d 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -221,8 +221,9 @@ static int extended_store_replace(struct extended_dn_context *ac,
 				  TALLOC_CTX *callback_mem_ctx,
 				  struct ldb_val *plain_dn,
 				  bool is_delete, 
-				  const char *oid)
+				  const struct dsdb_attribute *schema_attr)
 {
+	const char *oid = schema_attr->syntax->ldap_oid;
 	int ret;
 	struct extended_dn_replace_list *os;
 	static const char *attrs[] = {
@@ -348,7 +349,7 @@ static int extended_dn_add(struct ldb_module *module, struct ldb_request *req)
 		el = &ac->new_req->op.add.message->elements[i];
 		for (j = 0; j < el->num_values; j++) {
 			ret = extended_store_replace(ac, ac->new_req, &el->values[j],
-						     false, schema_attr->syntax->ldap_oid);
+						     false, schema_attr);
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
@@ -439,7 +440,7 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req
 			bool is_delete = (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE);
 
 			ret = extended_store_replace(ac, ac->new_req, &el->values[j],
-						     is_delete, schema_attr->syntax->ldap_oid);
+						     is_delete, schema_attr);
 			if (ret != LDB_SUCCESS) {
 				talloc_free(ac);
 				return ret;
-- 
1.9.1


From 2634d2285852c077e55423729d8e21fe21003773 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 25 Feb 2018 21:45:06 +0100
Subject: [PATCH 07/26] dsdb:extended_dn_store: make sure reject storing
 references to deleted objects in linked attributes

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/linked_vs_non_linked          |  1 -
 source4/dsdb/samdb/ldb_modules/extended_dn_store.c | 75 +++++++++++++++++++++-
 2 files changed, 73 insertions(+), 3 deletions(-)
 delete mode 100644 selftest/knownfail.d/linked_vs_non_linked

diff --git a/selftest/knownfail.d/linked_vs_non_linked b/selftest/knownfail.d/linked_vs_non_linked
deleted file mode 100644
index 281f4e5..0000000
--- a/selftest/knownfail.d/linked_vs_non_linked
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.dsdb.*samba.tests.dsdb.DsdbTests.test_linked_vs_non_linked_reference
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 14c0d5d..9f42db4 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -52,6 +52,8 @@ struct extended_dn_replace_list {
 	struct ldb_val *replace_dn;
 	struct extended_dn_context *ac;
 	struct ldb_request *search_req;
+	bool require_object;
+	bool got_entry;
 };
 
 
@@ -127,6 +129,18 @@ static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
 					LDB_ERR_OPERATIONS_ERROR);
 	}
 	if (ares->error == LDB_ERR_NO_SUCH_OBJECT) {
+		if (os->require_object) {
+			/*
+			 * It's an error if the target doesn't exist,
+			 * unless it's a delete.
+			 */
+			int ret = dsdb_module_werror(os->ac->module,
+						LDB_ERR_CONSTRAINT_VIOLATION,
+						WERR_DS_NAME_REFERENCE_INVALID,
+						"Referenced object not found");
+			return ldb_module_done(os->ac->req, NULL, NULL, ret);
+		}
+
 		/* Don't worry too much about dangling references */
 
 		ldb_reset_err_string(os->ac->ldb);
@@ -177,6 +191,7 @@ static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
 			return ldb_module_done(os->ac->req, NULL, NULL,
 						LDB_ERR_OPERATIONS_ERROR);
 		}
+		os->got_entry = true;
 		break;
 	}
 	case LDB_REPLY_REFERRAL:
@@ -186,7 +201,19 @@ static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
 	case LDB_REPLY_DONE:
 
 		talloc_free(ares);
-		
+
+		if (!os->got_entry && os->require_object) {
+			/*
+			 * It's an error if the target doesn't exist,
+			 * unless it's a delete.
+			 */
+			int ret = dsdb_module_werror(os->ac->module,
+						 LDB_ERR_CONSTRAINT_VIOLATION,
+						 WERR_DS_NAME_REFERENCE_INVALID,
+						 "Referenced object not found");
+			return ldb_module_done(os->ac->req, NULL, NULL, ret);
+		}
+
 		/* Run the next search */
 
 		if (os->next) {
@@ -231,6 +258,8 @@ static int extended_store_replace(struct extended_dn_context *ac,
 		"objectGUID",
 		NULL
 	};
+	uint32_t ctrl_flags = 0;
+	bool is_untrusted = ldb_req_is_untrusted(ac->req);
 
 	os = talloc_zero(ac, struct extended_dn_replace_list);
 	if (!os) {
@@ -276,9 +305,51 @@ static int extended_store_replace(struct extended_dn_context *ac,
 		return ret;
 	}
 
+	/*
+	 * By default we require the presence of the target.
+	 */
+	os->require_object = true;
+
+	if (schema_attr->linkID == 0) {
+		/*
+		 * None linked attributes allow references
+		 * to deleted objects.
+		 */
+		ctrl_flags |= DSDB_SEARCH_SHOW_RECYCLED;
+	}
+
+	if (is_delete) {
+		/*
+		 * On delete want to be able to
+		 * find a deleted object, but
+		 * it's not a problem if they doesn't
+		 * exist.
+		 */
+		ctrl_flags |= DSDB_SEARCH_SHOW_RECYCLED;
+		os->require_object = false;
+	}
+
+	if (!is_untrusted) {
+		struct ldb_control *ctrl = NULL;
+
+		/*
+		 * During provision or dbcheck we may not find
+		 * an object.
+		 */
+
+		ctrl = ldb_request_get_control(ac->req, LDB_CONTROL_RELAX_OID);
+		if (ctrl != NULL) {
+			os->require_object = false;
+		}
+		ctrl = ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK);
+		if (ctrl != NULL) {
+			os->require_object = false;
+		}
+	}
+
 	ret = dsdb_request_add_controls(os->search_req,
 					DSDB_FLAG_AS_SYSTEM |
-					DSDB_SEARCH_SHOW_RECYCLED |
+					ctrl_flags |
 					DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT);
 	if (ret != LDB_SUCCESS) {
 		talloc_free(os);
-- 
1.9.1


From 3dea1ea7d5c6c8bc95440944a7daa8336f520098 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 23:24:59 +0100
Subject: [PATCH 08/26] dsdb:unique_object_sids: remove
 unique_object_sids_init() and get the domain_sid during the request

samdb_domain_sid() already has an effective cache, there's no need to
cache it again.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 .../ldb_modules/tests/test_unique_object_sids.c    |   3 -
 .../dsdb/samdb/ldb_modules/unique_object_sids.c    | 124 ++++++++++-----------
 2 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c b/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c
index dfc6d49..d087601 100644
--- a/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c
@@ -146,9 +146,6 @@ static int setup(void **state)
 	rc = ldb_connect(test_ctx->ldb, test_ctx->dbpath, 0, NULL);
 	assert_int_equal(rc, LDB_SUCCESS);
 
-	rc = unique_object_sids_init(test_ctx->module);
-	assert_int_equal(rc, LDB_SUCCESS);
-
 	*state = test_ctx;
 
 	last_request = NULL;
diff --git a/source4/dsdb/samdb/ldb_modules/unique_object_sids.c b/source4/dsdb/samdb/ldb_modules/unique_object_sids.c
index f8427bc..9b77df7 100644
--- a/source4/dsdb/samdb/ldb_modules/unique_object_sids.c
+++ b/source4/dsdb/samdb/ldb_modules/unique_object_sids.c
@@ -36,43 +36,84 @@
 #include "libcli/security/dom_sid.h"
 #include "dsdb/samdb/ldb_modules/util.h"
 
-struct private_data {
-	const struct dom_sid *domain_sid;
-};
-
-
 /*
  * Does the add request contain a local objectSID
  */
 static bool message_contains_local_objectSID(
 	struct ldb_module *module,
-	const struct ldb_message *msg)
+	const struct ldb_message *msg,
+	struct ldb_request *req)
 {
-	struct dom_sid *objectSID	= NULL;
+	struct ldb_context *ldb = NULL;
+	struct ldb_message_element *el = NULL;
+	struct ldb_control *provision = NULL;
+	struct dom_sid *objectSID = NULL;
+	struct dom_sid objectDOMAIN = {0, };
+	const struct dom_sid *domain_sid = NULL;
+	TALLOC_CTX *frame = NULL;
+	bool match;
+
+	el = ldb_msg_find_element(msg, "objectSID");
+	if (el == NULL) {
+		/*
+		 * Without an objectSID, there's nothing to do.
+		 */
+		return false;
+	}
 
-	struct private_data *data =
-		talloc_get_type(
-			ldb_module_get_private(module),
-			struct private_data);
+	provision = ldb_request_get_control(req, LDB_CONTROL_PROVISION_OID);
+	if (provision != NULL) {
+		/*
+		 * During provision we may not be able do
+		 * find our own domain sid.
+		 */
+		return false;
+	}
 
-	TALLOC_CTX *frame = talloc_stackframe();
+	ldb = ldb_module_get_ctx(module);
+	frame = talloc_stackframe();
 
 	objectSID = samdb_result_dom_sid(frame, msg, "objectSID");
 	if (objectSID == NULL) {
+		/*
+		 * Something went wrong, we better don't allow
+		 * non-unique sids.
+		 */
 		TALLOC_FREE(frame);
-		return false;
+		return true;
 	}
 
-	/*
-	 * data->domain_sid can be NULL but dom_sid_in_domain handles this
-	 * case correctly. See unique_object_sids_init for more details.
-	 */
-	if (!dom_sid_in_domain(data->domain_sid, objectSID)) {
+	/* S-1-5-21-222222-333333-444444-555 */
+	objectDOMAIN = *objectSID;
+	if (objectDOMAIN.num_auths == 5) {
+		sid_split_rid(&objectDOMAIN, NULL);
+	}
+	if (objectDOMAIN.num_auths != 4) {
+		/*
+		 * It's not a domain or principal in a domain
+		 */
 		TALLOC_FREE(frame);
 		return false;
 	}
+
+	domain_sid = samdb_domain_sid(ldb);
+	if (domain_sid == NULL) {
+		/*
+		 * Something went wrong, we better don't allow
+		 * non-unique sids.
+		 */
+		TALLOC_FREE(frame);
+		return true;
+	}
+
+	match = dom_sid_equal(domain_sid, &objectDOMAIN);
+	if (match) {
+		TALLOC_FREE(frame);
+		return true;
+	}
+
 	TALLOC_FREE(frame);
-	return true;
+	return false;
 }
 
 static int flag_objectSID(
@@ -112,7 +153,7 @@ static int unique_object_sids_add(
 	struct ldb_context *ldb		= NULL;
 	int rc;
 
-	if (!message_contains_local_objectSID(module, msg)) {
+	if (!message_contains_local_objectSID(module, msg, req)) {
 		/*
 		 * Request does not contain a local objectSID so chain the
 		 * next module
@@ -158,7 +199,7 @@ static int unique_object_sids_modify(
 	struct ldb_context *ldb		= NULL;
 	int rc;
 
-	if (!message_contains_local_objectSID(module, msg)) {
+	if (!message_contains_local_objectSID(module, msg, req)) {
 		/*
 		 * Request does not contain a local objectSID so chain the
 		 * next module
@@ -208,49 +249,8 @@ static int unique_object_sids_modify(
 	return ldb_next_request(module, new_req);
 }
 
-/* init */
-static int unique_object_sids_init(
-	struct ldb_module *module)
-{
-	struct ldb_context *ldb = ldb_module_get_ctx(module);
-	struct private_data *data = NULL;
-	int ret;
-
-	ret = ldb_next_init(module);
-
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
-	data = talloc_zero(module, struct private_data);
-	if (!data) {
-		return ldb_module_oom(module);
-	}
-
-	data->domain_sid = samdb_domain_sid(ldb);
-	if (data->domain_sid == NULL) {
-		/*
-		 * Unable to determine the domainSID, this normally occurs
-		 * when provisioning. As there is no easy way to detect
-		 * that we are provisioning.  We currently just log this as a
-		 * warning.
-		 */
-		ldb_debug(
-			ldb,
-			LDB_DEBUG_WARNING,
-			"Unable to determine the DomainSID, "
-			"can not enforce uniqueness constraint on local "
-			"domainSIDs\n");
-	}
-
-	ldb_module_set_private(module, data);
-
-	return LDB_SUCCESS;
-}
-
 static const struct ldb_module_ops ldb_unique_object_sids_module_ops = {
 	.name		   = "unique_object_sids",
-	.init_context	   = unique_object_sids_init,
 	.add               = unique_object_sids_add,
 	.modify            = unique_object_sids_modify,
 };
-- 
1.9.1


From 8d61c772ebfdb96fac87781ae6d64368f7095dc2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 23 Feb 2018 16:04:57 +0100
Subject: [PATCH 09/26] provision: use the provision control when adding
 foreignSecurityPrincipals

The next commits will require this.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/provision/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 558587c..1a5f021 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1507,7 +1507,7 @@ def fill_samdb(samdb, lp, names, logger, policyguid,
         setup_add_ldif(samdb, setup_path("provision_well_known_sec_princ.ldif"), {
             "CONFIGDN": names.configdn,
             "WELLKNOWNPRINCIPALS_DESCRIPTOR": protected1wd_descr,
-            })
+            }, controls=["relax:0", "provision:0"])
 
     if fill == FILL_FULL or fill == FILL_SUBDOMAIN:
         setup_modify_ldif(samdb,
@@ -1522,7 +1522,7 @@ def fill_samdb(samdb, lp, names, logger, policyguid,
             "DOMAINSID": str(names.domainsid),
             "ADMINPASS_B64": b64encode(adminpass.encode('utf-16-le')),
             "KRBTGTPASS_B64": b64encode(krbtgtpass.encode('utf-16-le'))
-            })
+            }, controls=["relax:0", "provision:0"])
 
         logger.info("Setting up self join")
         setup_self_join(samdb, admin_session_info, names=names, fill=fill,
-- 
1.9.1


From 8bec253c13d808693106ac1033a81086bb6d63f4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 22:51:19 +0100
Subject: [PATCH 10/26] tests/dsdb.py: verify that foreignSecurityPrincipal
 objects require the provision control

Windows rejects creating foreignSecurityPrincipal objects directly.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/tests/dsdb.py                | 47 ++++++++++++++++++++++++++++---
 selftest/knownfail.d/duplicate_objectSIDs |  1 +
 2 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 selftest/knownfail.d/duplicate_objectSIDs

diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index 231abee..7a4ee29 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -25,6 +25,7 @@ from samba.tests import delete_force
 from samba.ndr import ndr_unpack, ndr_pack
 from samba.dcerpc import drsblobs, security, misc
 from samba import dsdb
+from samba import werror
 import ldb
 import samba
 import uuid
@@ -214,19 +215,57 @@ class DsdbTests(TestCase):
             c = "9"
         else:
             c = "0"
-        sid     = str(dom_sid)[:-1] + c + "-1000"
+        sid_str = str(dom_sid)[:-1] + c + "-1000"
+        sid     = ndr_pack(security.dom_sid(sid_str))
         basedn  = self.samdb.get_default_basedn()
-        dn      = "CN=%s,CN=ForeignSecurityPrincipals,%s" % (sid, basedn)
+        dn      = "CN=%s,CN=ForeignSecurityPrincipals,%s" % (sid_str, basedn)
+
+        #
+        # First without control
+        #
+
+        try:
+            self.samdb.add({
+                "dn": dn,
+                "objectClass": "foreignSecurityPrincipal"})
+            self.fail("No exception should get ERR_OBJECT_CLASS_VIOLATION")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_OBJECT_CLASS_VIOLATION, str(e))
+            werr = "%08X" % werror.WERR_DS_MISSING_REQUIRED_ATT
+            self.assertTrue(werr in msg, msg)
+
+        try:
+            self.samdb.add({
+                "dn": dn,
+                "objectClass": "foreignSecurityPrincipal",
+                "objectSid": sid})
+            self.fail("No exception should get ERR_UNWILLING_TO_PERFORM")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_UNWILLING_TO_PERFORM, str(e))
+            werr = "%08X" % werror.WERR_DS_ILLEGAL_MOD_OPERATION
+            self.assertTrue(werr in msg, msg)
+
+        #
+        # We need to use the provision control
+        # in order to add foreignSecurityPrincipal
+        # objects
+        #
+
+        controls = ["provision:0"]
         self.samdb.add({
             "dn": dn,
-            "objectClass": "foreignSecurityPrincipal"})
+            "objectClass": "foreignSecurityPrincipal"},
+            controls=controls)
 
         self.samdb.delete(dn)
 
         try:
             self.samdb.add({
                 "dn": dn,
-                "objectClass": "foreignSecurityPrincipal"})
+                "objectClass": "foreignSecurityPrincipal"},
+                controls=controls)
         except ldb.LdbError as e:
             (code, msg) = e.args
             self.fail("Got unexpected exception %d - %s "
diff --git a/selftest/knownfail.d/duplicate_objectSIDs b/selftest/knownfail.d/duplicate_objectSIDs
new file mode 100644
index 0000000..6e6ddbe
--- /dev/null
+++ b/selftest/knownfail.d/duplicate_objectSIDs
@@ -0,0 +1 @@
+^samba.tests.dsdb.*samba.tests.dsdb.DsdbTests.test_duplicate_objectSIDs_allowed_on_foreign_security_principals
-- 
1.9.1


From 1104214b569782fffd1cdd6cb04ae03f17b571b9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 22 Feb 2018 22:51:46 +0100
Subject: [PATCH 11/26] dsdb:samldb: require as_system or provision control to
 create foreignSecurityPrincipal objects

Windows rejects creating foreignSecurityPrincipal objects directly.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/duplicate_objectSIDs |  1 -
 source4/dsdb/samdb/ldb_modules/samldb.c   | 46 ++++++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 5 deletions(-)
 delete mode 100644 selftest/knownfail.d/duplicate_objectSIDs

diff --git a/selftest/knownfail.d/duplicate_objectSIDs b/selftest/knownfail.d/duplicate_objectSIDs
deleted file mode 100644
index 6e6ddbe..0000000
--- a/selftest/knownfail.d/duplicate_objectSIDs
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.dsdb.*samba.tests.dsdb.DsdbTests.test_duplicate_objectSIDs_allowed_on_foreign_security_principals
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 3e429e1..108235a 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1249,14 +1249,52 @@ static int samldb_fill_object(struct samldb_ctx *ac)
 
 static int samldb_fill_foreignSecurityPrincipal_object(struct samldb_ctx *ac)
 {
-	struct ldb_context *ldb;
-	const struct ldb_val *rdn_value;
-	struct dom_sid *sid;
+	struct ldb_context *ldb = NULL;
+	const struct ldb_val *rdn_value = NULL;
+	struct ldb_message_element *sid_el = NULL;
+	struct dom_sid *sid = NULL;
+	struct ldb_control *as_system = NULL;
+	struct ldb_control *provision = NULL;
+	bool allowed = false;
 	int ret;
 
 	ldb = ldb_module_get_ctx(ac->module);
 
-	sid = samdb_result_dom_sid(ac->msg, ac->msg, "objectSid");
+	as_system = ldb_request_get_control(ac->req, LDB_CONTROL_AS_SYSTEM_OID);
+	if (as_system != NULL) {
+		allowed = true;
+	}
+
+	provision = ldb_request_get_control(ac->req, LDB_CONTROL_PROVISION_OID);
+	if (provision != NULL) {
+		allowed = true;
+	}
+
+	sid_el = ldb_msg_find_element(ac->msg, "objectSid");
+
+	if (!allowed && sid_el == NULL) {
+		return dsdb_module_werror(ac->module,
+				LDB_ERR_OBJECT_CLASS_VIOLATION,
+				WERR_DS_MISSING_REQUIRED_ATT,
+				"objectSid missing on foreignSecurityPrincipal");
+	}
+
+	if (!allowed) {
+		return dsdb_module_werror(ac->module,
+				LDB_ERR_UNWILLING_TO_PERFORM,
+				WERR_DS_ILLEGAL_MOD_OPERATION,
+				"foreignSecurityPrincipal object not allowed");
+	}
+
+	if (sid_el != NULL) {
+		sid = samdb_result_dom_sid(ac->msg, ac->msg, "objectSid");
+		if (sid == NULL) {
+			ldb_set_errstring(ldb,
+					  "samldb: invalid objectSid!");
+			return LDB_ERR_CONSTRAINT_VIOLATION;
+		}
+	}
+
 	if (sid == NULL) {
 		rdn_value = ldb_dn_get_rdn_val(ac->msg->dn);
 		if (rdn_value == NULL) {
-- 
1.9.1


From d27bde2800cb6827b976a536cfd446150ca23488 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Sun, 25 Feb 2018 00:10:12 +0100
Subject: [PATCH 12/26] tests/dsdb.py: test creation of
 foreignSecurityPrincipal via 'member: <SID=...>'

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/tests/dsdb.py                         | 84 ++++++++++++++++++++++
 .../knownfail.d/foreignSecurityPrincipal_member    |  1 +
 2 files changed, 85 insertions(+)
 create mode 100644 selftest/knownfail.d/foreignSecurityPrincipal_member

diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index 7a4ee29..52e9b00 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -274,6 +274,90 @@ class DsdbTests(TestCase):
         # cleanup
         self.samdb.delete(dn)
 
+    def test_foreignSecurityPrincipal_member(self):
+
+        dom_sid = self.samdb.get_domain_sid()
+        lsid_str = str(dom_sid) + "-4294967294"
+        lsid     = ndr_pack(security.dom_sid(lsid_str))
+        bsid_str = "S-1-5-32-4294967294"
+        bsid     = ndr_pack(security.dom_sid(bsid_str))
+        fsid_str = "S-1-5-4294967294"
+        fsid     = ndr_pack(security.dom_sid(fsid_str))
+        basedn   = self.samdb.get_default_basedn()
+        cn       = "dsdb_testg_01"
+        dn_str   = "cn=%s,cn=Users,%s" % (cn, basedn)
+        dn = ldb.Dn(self.samdb, dn_str)
+
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=basedn,
+                                expression="(objectSid=%s)" % lsid_str,
+                                attrs=[])
+        self.assertEqual(len(res), 0)
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=basedn,
+                                expression="(objectSid=%s)" % bsid_str,
+                                attrs=[])
+        self.assertEqual(len(res), 0)
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=basedn,
+                                expression="(objectSid=%s)" % fsid_str,
+                                attrs=[])
+        self.assertEqual(len(res), 0)
+
+        self.addCleanup(delete_force, self.samdb, dn_str)
+
+        self.samdb.add({
+            "dn": dn_str,
+            "objectClass": "group"})
+
+        msg = ldb.Message()
+        msg.dn = dn
+        msg["member"] = ldb.MessageElement("<SID=%s>" % lsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           "member")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_UNWILLING_TO_PERFORM")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_UNWILLING_TO_PERFORM, str(e))
+            werr = "%08X" % werror.WERR_DS_INVALID_GROUP_TYPE
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = dn
+        msg["member"] = ldb.MessageElement("<SID=%s>" % bsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           "member")
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get LDB_ERR_NO_SUCH_OBJECT")
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, ldb.ERR_NO_SUCH_OBJECT, str(e))
+            werr = "%08X" % werror.WERR_NO_SUCH_MEMBER
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = dn
+        msg["member"] = ldb.MessageElement("<SID=%s>" % fsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           "member")
+        self.samdb.modify(msg)
+
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=basedn,
+                                expression="(objectSid=%s)" % fsid_str,
+                                attrs=[])
+        self.assertEqual(len(res), 1)
+        self.samdb.delete(res[0].dn)
+        self.samdb.delete(dn)
+        res = self.samdb.search(scope=ldb.SCOPE_SUBTREE,
+                                base=basedn,
+                                expression="(objectSid=%s)" % fsid_str,
+                                attrs=[])
+        self.assertEqual(len(res), 0)
+
     #
     # Duplicate objectSID's should not be permitted for sids in the local
     # domain. The test sequence is add an object, delete it, then attempt to
diff --git a/selftest/knownfail.d/foreignSecurityPrincipal_member b/selftest/knownfail.d/foreignSecurityPrincipal_member
new file mode 100644
index 0000000..673e98c
--- /dev/null
+++ b/selftest/knownfail.d/foreignSecurityPrincipal_member
@@ -0,0 +1 @@
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_member
-- 
1.9.1


From d614a19f56a22cdf41c21c95d913e118283dff9d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 31 Jan 2018 18:00:24 +0100
Subject: [PATCH 13/26] dsdb:ldb_modules: add fpo_attrs module that creates
 foreignSecurityPrincipal objects if needed

For now we only handle the most important attribute 'member',
see [MS-SAMR] 3.1.1.8.9 member.

In future we should handle all of them, see
[MS-ADTS] 3.1.1.5.2.3 Special Classes and Attributes:

* FPO-enabled attributes: member, msDS-MembersForAzRole, msDS-NeverRevealGroup,
  msDS-NonMembers, msDS-RevealOnDemandGroup, msDS-ServiceAccount.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 .../knownfail.d/foreignSecurityPrincipal_member    |   1 -
 source4/dsdb/samdb/ldb_modules/fpo_attrs.c         | 348 +++++++++++++++++++++
 source4/dsdb/samdb/ldb_modules/samba_dsdb.c        |   1 +
 .../dsdb/samdb/ldb_modules/wscript_build_server    |   8 +
 4 files changed, 357 insertions(+), 1 deletion(-)
 delete mode 100644 selftest/knownfail.d/foreignSecurityPrincipal_member
 create mode 100644 source4/dsdb/samdb/ldb_modules/fpo_attrs.c

diff --git a/selftest/knownfail.d/foreignSecurityPrincipal_member b/selftest/knownfail.d/foreignSecurityPrincipal_member
deleted file mode 100644
index 673e98c..0000000
--- a/selftest/knownfail.d/foreignSecurityPrincipal_member
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_member
diff --git a/source4/dsdb/samdb/ldb_modules/fpo_attrs.c b/source4/dsdb/samdb/ldb_modules/fpo_attrs.c
new file mode 100644
index 0000000..2b34651
--- /dev/null
+++ b/source4/dsdb/samdb/ldb_modules/fpo_attrs.c
@@ -0,0 +1,348 @@
+/*
+   FPO-attributes ldb module
+
+   Copyright (C) Stefan Metzmacher 2018
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "includes.h"
+#include "ldb_module.h"
+#include "dsdb/samdb/samdb.h"
+#include "dsdb/samdb/ldb_modules/util.h"
+#include "libcli/security/security.h"
+#include "ldb_wrap.h"
+
+struct fpo_attrs_ctx {
+	struct ldb_module *module;
+	struct ldb_request *req;
+
+	const struct ldb_message *msg;
+
+	/*
+	 * Used by the FPO-enabled attribute validation.
+	 */
+	struct dsdb_trust_routing_table *routing_table;
+};
+
+static struct fpo_attrs_ctx *fpo_attrs_ctx_init(struct ldb_module *module,
+						struct ldb_request *req)
+{
+	struct fpo_attrs_ctx *ac;
+
+	ac = talloc_zero(req, struct fpo_attrs_ctx);
+	if (ac == NULL) {
+		ldb_module_oom(module);
+		return NULL;
+	}
+
+	ac->module = module;
+	ac->req = req;
+
+	switch (req->operation) {
+	case LDB_ADD:
+		ac->msg = req->op.add.message;
+		break;
+	case LDB_MODIFY:
+		ac->msg = req->op.mod.message;
+		break;
+	default:
+		TALLOC_FREE(ac);
+		ldb_module_operr(module);
+		return NULL;
+	}
+
+	return ac;
+}
+
+static int fpo_attrs_foreign_member(struct fpo_attrs_ctx *ac, struct ldb_dn *member_dn)
+{
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+	struct dom_sid member_sid = { 0, };
+	struct dom_sid member_domain = { 0, };
+	char *fsid = NULL;
+	struct ldb_message *fmsg = NULL;
+	const struct dom_sid *domain_sid = NULL;
+	const struct lsa_TrustDomainInfoInfoEx *tdo = NULL;
+	uint32_t trust_attributes = 0;
+	NTSTATUS status;
+	bool match;
+	bool ok;
+	int ret;
+
+	/*
+	 * member DN doesn't exist yet
+	 *
+	 * Check if a foreign SID is specified,
+	 * which would trigger the creation
+	 * of a foreignSecurityPrincipal.
+	 */
+	status = dsdb_get_extended_dn_sid(member_dn,
+					  &member_sid,
+					  "SID");
+	if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+		/*
+		 * No SID specified
+		 */
+		return dsdb_module_werror(ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified member dn doesn't exist");
+	}
+	if (!NT_STATUS_IS_OK(status)) {
+		return ldb_module_operr(ac->module);
+	}
+	if (ldb_dn_get_extended_comp_num(member_dn) != 1) {
+		return dsdb_module_werror(ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified extended component other than SID");
+		return ldb_module_operr(ac->module);
+	}
+	if (ldb_dn_get_comp_num(member_dn) != 0) {
+		return dsdb_module_werror(ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified more the SID");
+	}
+
+	member_domain = member_sid;
+	sid_split_rid(&member_domain, NULL);
+
+	match = dom_sid_equal(&global_sid_Builtin, &member_domain);
+	if (match) {
+		/*
+		 * Non existing BUILTIN sid
+		 */
+		return dsdb_module_werror(ac->module,
+				LDB_ERR_NO_SUCH_OBJECT,
+				WERR_NO_SUCH_MEMBER,
+				"specified member sid doesn't exist in BUILTIN");
+	}
+
+	domain_sid = samdb_domain_sid(ldb);
+	if (domain_sid == NULL) {
+		return ldb_module_operr(ac->module);
+	}
+	match = dom_sid_equal(domain_sid, &member_domain);
+	if (match) {
+		/*
+		 * Non existing SID in our domain.
+		 */
+		return dsdb_module_werror(ac->module,
+				LDB_ERR_UNWILLING_TO_PERFORM,
+				WERR_DS_INVALID_GROUP_TYPE,
+				"specified member sid doesn't exist in domain");
+	}
+
+	if (ac->routing_table == NULL) {
+		status = dsdb_trust_routing_table_load(ldb, ac,
+						       &ac->routing_table);
+		if (!NT_STATUS_IS_OK(status)) {
+			return ldb_module_operr(ac->module);
+		}
+	}
+
+	tdo = dsdb_trust_domain_by_sid(ac->routing_table, &member_domain, NULL);
+	if (tdo != NULL) {
+		trust_attributes = tdo->trust_attributes;
+	}
+
+	if (trust_attributes & LSA_TRUST_ATTRIBUTE_WITHIN_FOREST) {
+		return dsdb_module_werror(ac->module,
+				LDB_ERR_UNWILLING_TO_PERFORM,
+				WERR_DS_INVALID_GROUP_TYPE,
+				"specified member sid doesn't exist in forest");
+	}
+
+	fsid = dom_sid_string(ac, &member_sid);
+	if (fsid == NULL) {
+		return ldb_module_oom(ac->module);
+	}
+
+	fmsg = ldb_msg_new(ac);
+	if (fmsg == NULL) {
+		return ldb_module_oom(ac->module);
+	}
+
+	fmsg->dn = ldb_dn_copy(fmsg, ldb_get_default_basedn(ldb));
+	if (fmsg->dn == NULL) {
+		return ldb_module_oom(ac->module);
+	}
+
+	ok = ldb_dn_add_child_fmt(fmsg->dn,
+				  "CN=%s,CN=ForeignSecurityPrincipals",
+				  fsid);
+	if (!ok) {
+		return ldb_module_oom(ac->module);
+	}
+
+	ret = ldb_msg_add_string(fmsg, "objectClass", "foreignSecurityPrincipal");
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	ret = dsdb_module_add(ac->module, fmsg,
+			      DSDB_FLAG_AS_SYSTEM |
+			      DSDB_FLAG_NEXT_MODULE,
+			      ac->req);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	return LDB_SUCCESS;
+}
+
+static int fpo_attrs_check(struct fpo_attrs_ctx *ac)
+{
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+	unsigned int i;
+
+	/*
+	 * TODO: handle all FPO-Attributes.
+	 */
+
+	for (i = 0; i < ac->msg->num_elements; i++) {
+		struct ldb_message_element *el = NULL;
+		unsigned int j;
+
+		if (ldb_attr_cmp(ac->msg->elements[i].name, "member") != 0) {
+			continue;
+		}
+
+		el = &ac->msg->elements[i];
+		for (j = 0; j < el->num_values; j++) {
+			struct ldb_result *group_res = NULL;
+			const char *group_attrs[] = { NULL };
+			struct ldb_dn *member_dn = NULL;
+			int ret;
+
+			if (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE) {
+				/*
+				 * We don't need to create an
+				 * foreignSecurityPrincipal on delete
+				 */
+				continue;
+			}
+
+			member_dn = ldb_dn_from_ldb_val(ac, ldb,
+							&el->values[j]);
+			if (!ldb_dn_validate(member_dn)) {
+				return ldb_operr(ldb);
+			}
+
+			ret = dsdb_module_search_dn(ac->module, ac, &group_res,
+						    member_dn, group_attrs,
+						    DSDB_FLAG_NEXT_MODULE, ac->req);
+			if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+				/*
+				 * member DN doesn't exist yet
+				 *
+				 * Check if a foreign SID is specified,
+				 * which would trigger the creation
+				 * of a foreignSecurityPrincipal.
+				 */
+				ret = fpo_attrs_foreign_member(ac, member_dn);
+			}
+			if (ret != LDB_SUCCESS) {
+				return ret;
+			}
+		}
+	}
+
+	return LDB_SUCCESS;
+}
+
+/* add */
+static int fpo_attrs_add(struct ldb_module *module, struct ldb_request *req)
+{
+	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct fpo_attrs_ctx *ac = NULL;
+	struct ldb_message_element *el = NULL;
+	int ret;
+
+	ldb_debug(ldb, LDB_DEBUG_TRACE, "fpo_attrs_add\n");
+
+	/* do not manipulate our control entries */
+	if (ldb_dn_is_special(req->op.add.message->dn)) {
+		return ldb_next_request(module, req);
+	}
+
+	el = ldb_msg_find_element(req->op.add.message, "member");
+	if (el == NULL) {
+		return ldb_next_request(module, req);
+	}
+
+	ac = fpo_attrs_ctx_init(module, req);
+	if (ac == NULL) {
+		return ldb_operr(ldb);
+	}
+
+	ret = fpo_attrs_check(ac);
+	TALLOC_FREE(ac);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	/* nothing matched, go on */
+	return ldb_next_request(module, req);
+}
+
+/* modify */
+static int fpo_attrs_modify(struct ldb_module *module, struct ldb_request *req)
+{
+	struct ldb_context *ldb = ldb_module_get_ctx(module);
+	struct fpo_attrs_ctx *ac = NULL;
+	struct ldb_message_element *el = NULL;
+	int ret;
+
+	ldb_debug(ldb, LDB_DEBUG_TRACE, "fpo_attrs_modify\n");
+
+	if (ldb_dn_is_special(req->op.mod.message->dn)) {
+		/* do not manipulate our control entries */
+		return ldb_next_request(module, req);
+	}
+
+	/* make sure that "objectSid" is not specified */
+	el = ldb_msg_find_element(req->op.mod.message, "member");
+	if (el == NULL) {
+		return ldb_next_request(module, req);
+	}
+
+	ac = fpo_attrs_ctx_init(module, req);
+	if (ac == NULL) {
+		return ldb_operr(ldb);
+	}
+
+	ret = fpo_attrs_check(ac);
+	TALLOC_FREE(ac);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	/* nothing matched, go on */
+	return ldb_next_request(module, req);
+}
+
+static const struct ldb_module_ops ldb_fpo_attrs_module_ops = {
+	.name          = "fpo_attrs",
+	.add           = fpo_attrs_add,
+	.modify        = fpo_attrs_modify,
+};
+
+int ldb_fpo_attrs_module_init(const char *version)
+{
+	LDB_MODULE_CHECK_VERSION(version);
+	return ldb_register_module(&ldb_fpo_attrs_module_ops);
+}
diff --git a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
index e0acb4e..402cba3 100644
--- a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
+++ b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c
@@ -286,6 +286,7 @@ static int samba_dsdb_init(struct ldb_module *module)
 					     "anr",
 					     "server_sort",
 					     "asq",
+					     "fpo_attrs",
 					     "extended_dn_store",
 					     NULL };
 	/* extended_dn_in or extended_dn_in_openldap goes here */
diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build_server b/source4/dsdb/samdb/ldb_modules/wscript_build_server
index 368260a..7fc5085 100644
--- a/source4/dsdb/samdb/ldb_modules/wscript_build_server
+++ b/source4/dsdb/samdb/ldb_modules/wscript_build_server
@@ -69,6 +69,14 @@ bld.SAMBA_MODULE('ldb_samldb',
 	deps='talloc samdb DSDB_MODULE_HELPERS DSDB_MODULE_HELPER_RIDALLOC'
 	)
 
+bld.SAMBA_MODULE('ldb_fpo_attrs',
+	source='fpo_attrs.c',
+	subsystem='ldb',
+	init_function='ldb_fpo_attrs_module_init',
+	module_init_name='ldb_init_module',
+	internal_module=False,
+	deps='talloc samdb DSDB_MODULE_HELPERS'
+	)
 
 bld.SAMBA_MODULE('ldb_samba3sam',
 	source='samba3sam.c',
-- 
1.9.1


From e36b21a55147ec3353996f20dcdcea95f78cf424 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 14:19:39 +0100
Subject: [PATCH 14/26] selftest/Samba4: use DOMAIN/REALM from the dcvars
 instead of using hardcoded values

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/target/Samba4.pm | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index fd4e4b6..b537d74 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1049,8 +1049,8 @@ rpc_server:tcpip = no
 	my $ret = $self->provision($prefix,
 				   "member server",
 				   $hostname,
-				   "SAMBADOMAIN",
-				   "samba.example.com",
+				   $dcvars->{DOMAIN},
+				   $dcvars->{REALM},
 				   "2008",
 				   "locMEMpass3",
 				   $dcvars->{SERVER_IP},
@@ -1125,8 +1125,8 @@ sub provision_rpc_proxy($$$)
 	my $ret = $self->provision($prefix,
 				   "member server",
 				   "localrpcproxy",
-				   "SAMBADOMAIN",
-				   "samba.example.com",
+				   $dcvars->{DOMAIN},
+				   $dcvars->{REALM},
 				   "2008",
 				   "locRPCproxypass4",
 				   $dcvars->{SERVER_IP},
@@ -1209,8 +1209,8 @@ sub provision_promoted_dc($$$)
 	# We do this so that we don't run the provision.  That's the job of 'samba-tool domain dcpromo'.
 	my $ctx = $self->provision_raw_prepare($prefix, "domain controller",
 					       "promotedvdc",
-					       "SAMBADOMAIN",
-					       "samba.example.com",
+					       $dcvars->{DOMAIN},
+					       $dcvars->{REALM},
 					       "2008",
 					       $dcvars->{PASSWORD},
 					       $dcvars->{SERVER_IP},
@@ -1651,8 +1651,8 @@ sub provision_rodc($$$)
 	# We do this so that we don't run the provision.  That's the job of 'net join RODC'.
 	my $ctx = $self->provision_raw_prepare($prefix, "domain controller",
 					       "rodc",
-					       "SAMBADOMAIN",
-					       "samba.example.com",
+					       $dcvars->{DOMAIN},
+					       $dcvars->{REALM},
 					       "2008",
 					       $dcvars->{PASSWORD},
 					       $dcvars->{SERVER_IP},
-- 
1.9.1


From 1a4925ce5dd9cf0b45902fb011ab1510312ee893 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 14:56:27 +0100
Subject: [PATCH 15/26] selftest: generate a ramdon domain sid during provision
 and export as SAMSID/[TRUST_]DOMSID

This will be useful for future tests.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/selftest.pl      |  3 +++
 selftest/target/Samba.pm  |  6 ++++++
 selftest/target/Samba3.pm | 21 +++++++++++++++++++++
 selftest/target/Samba4.pm | 27 ++++++++++++++++++++++++---
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index 12452bc..42c1e62 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -804,6 +804,7 @@ my @exported_envvars = (
 	# domain stuff
 	"DOMAIN",
 	"REALM",
+	"DOMSID",
 
 	# stuff related to a trusted domain
 	"TRUST_SERVER",
@@ -814,6 +815,7 @@ my @exported_envvars = (
 	"TRUST_PASSWORD",
 	"TRUST_DOMAIN",
 	"TRUST_REALM",
+	"TRUST_DOMSID",
 
 	# domain controller stuff
 	"DC_SERVER",
@@ -868,6 +870,7 @@ my @exported_envvars = (
 	"SERVER_IPV6",
 	"NETBIOSNAME",
 	"NETBIOSALIAS",
+	"SAMSID",
 
 	# user stuff
 	"USERNAME",
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index f25507f..b0482d3 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -443,4 +443,10 @@ sub cleanup_child($$)
     return $childpid;
 }
 
+sub random_domain_sid()
+{
+	my $domain_sid = "S-1-5-21-". int(rand(4294967295)) . "-" . int(rand(4294967295)) . "-" . int(rand(4294967295));
+	return $domain_sid;
+}
+
 1;
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index e6c95fa..e80491d 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -225,6 +225,7 @@ sub setup_nt4_dc
 	       return undef;
 	}
 
+	$vars->{DOMSID} = $vars->{SAMSID};
 	$vars->{DC_SERVER} = $vars->{SERVER};
 	$vars->{DC_SERVER_IP} = $vars->{SERVER_IP};
 	$vars->{DC_SERVER_IPV6} = $vars->{SERVER_IPV6};
@@ -273,6 +274,7 @@ sub setup_nt4_dc_schannel
 	       return undef;
 	}
 
+	$vars->{DOMSID} = $vars->{SAMSID};
 	$vars->{DC_SERVER} = $vars->{SERVER};
 	$vars->{DC_SERVER_IP} = $vars->{SERVER_IP};
 	$vars->{DC_SERVER_IPV6} = $vars->{SERVER_IPV6};
@@ -347,6 +349,7 @@ sub setup_nt4_member
 	       return undef;
 	}
 
+	$ret->{DOMSID} = $nt4_dc_vars->{DOMSID};
 	$ret->{DC_SERVER} = $nt4_dc_vars->{SERVER};
 	$ret->{DC_SERVER_IP} = $nt4_dc_vars->{SERVER_IP};
 	$ret->{DC_SERVER_IPV6} = $nt4_dc_vars->{SERVER_IPV6};
@@ -429,6 +432,7 @@ sub setup_ad_member
 	close(USERMAP);
 	$ret->{DOMAIN} = $dcvars->{DOMAIN};
 	$ret->{REALM} = $dcvars->{REALM};
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 
 	my $ctx;
 	$ctx = {};
@@ -521,6 +525,7 @@ sub setup_ad_member_rfc2307
 	close(USERMAP);
 	$ret->{DOMAIN} = $dcvars->{DOMAIN};
 	$ret->{REALM} = $dcvars->{REALM};
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 
 	my $ctx;
 	my $prefix_abs = abs_path($prefix);
@@ -606,6 +611,7 @@ sub setup_ad_member_idmap_rid
 	close(USERMAP);
 	$ret->{DOMAIN} = $dcvars->{DOMAIN};
 	$ret->{REALM} = $dcvars->{REALM};
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 
 	my $ctx;
 	my $prefix_abs = abs_path($prefix);
@@ -692,6 +698,7 @@ sub setup_ad_member_idmap_ad
 	close(USERMAP);
 	$ret->{DOMAIN} = $dcvars->{DOMAIN};
 	$ret->{REALM} = $dcvars->{REALM};
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 
 	my $ctx;
 	my $prefix_abs = abs_path($prefix);
@@ -1029,6 +1036,8 @@ $ret->{USERNAME} = KTEST\\Administrator
 #This is the secrets.tdb created by 'net ads join' from Samba3 to a
 #Samba4 DC with the same parameters as are being used here.  The
 #domain SID is S-1-5-21-1071277805-689288055-3486227160
+	$ret->{SAMSID} = "S-1-5-21-1911091480-1468226576-2729736297";
+	$ret->{DOMSID} = "S-1-5-21-1071277805-689288055-3486227160";
 
 	system("cp $self->{srcdir}/source3/selftest/ktest-secrets.tdb $prefix/private/secrets.tdb");
 	chmod 0600, "$prefix/private/secrets.tdb";
@@ -1364,6 +1373,7 @@ sub provision($$$$$$$$$)
 	## setup the various environment variables we need
 	##
 
+	my $samsid = Samba::random_domain_sid();
 	my $swiface = Samba::get_interface($server);
 	my %ret = ();
 	my %createuser_env = ();
@@ -2133,6 +2143,16 @@ sub provision($$$$$$$$$)
 	";
 	close(CONF);
 
+	my $net = Samba::bindir_path($self, "net");
+	my $cmd = "";
+	$cmd .= "SMB_CONF_PATH=\"$conffile\" ";
+	$cmd .= "$net setlocalsid $samsid";
+
+	if (system($cmd) != 0) {
+	    warn("Join failed\n$cmd");
+	    return undef;
+	}
+
 	unless (open(DFQCONF, ">$dfqconffile")) {
 	        warn("Unable to open $dfqconffile");
 		return undef;
@@ -2256,6 +2276,7 @@ force_user:x:$gid_force_user:
 	$ret{USERNAME} = $unix_name;
 	$ret{USERID} = $unix_uid;
 	$ret{DOMAIN} = $domain;
+	$ret{SAMSID} = $samsid;
 	$ret{NETBIOSNAME} = $server;
 	$ret{PASSWORD} = $password;
 	$ret{PIDDIR} = $piddir;
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index b537d74..608265c 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -372,6 +372,7 @@ sub setup_trust($$$$$)
 	$localenv->{TRUST_PASSWORD} = $remoteenv->{PASSWORD};
 	$localenv->{TRUST_DOMAIN} = $remoteenv->{DOMAIN};
 	$localenv->{TRUST_REALM} = $remoteenv->{REALM};
+	$localenv->{TRUST_DOMSID} = $remoteenv->{DOMSID};
 
 	my $samba_tool =  Samba::bindir_path($self, "samba-tool");
 	# setup the trust
@@ -401,10 +402,10 @@ sub setup_trust($$$$$)
 	return $localenv
 }
 
-sub provision_raw_prepare($$$$$$$$$$$)
+sub provision_raw_prepare($$$$$$$$$$$$)
 {
 	my ($self, $prefix, $server_role, $hostname,
-	    $domain, $realm, $functional_level,
+	    $domain, $realm, $samsid, $functional_level,
 	    $password, $kdc_ipv4, $kdc_ipv6) = @_;
 	my $ctx;
 	my $netbiosname = uc($hostname);
@@ -448,6 +449,7 @@ sub provision_raw_prepare($$$$$$$$$$$)
 	$ctx->{domain} = $domain;
 	$ctx->{realm} = uc($realm);
 	$ctx->{dnsname} = lc($realm);
+	$ctx->{samsid} = $samsid;
 
 	$ctx->{functional_level} = $functional_level;
 
@@ -543,6 +545,9 @@ sub provision_raw_prepare($$$$$$$$$$$)
 	push (@provision_options, "--quiet");
 	push (@provision_options, "--domain=$ctx->{domain}");
 	push (@provision_options, "--realm=$ctx->{realm}");
+	if (defined($ctx->{samsid})) {
+		push (@provision_options, "--domain-sid=$ctx->{samsid}");
+	}
 	push (@provision_options, "--adminpass=$ctx->{password}");
 	push (@provision_options, "--krbtgtpass=krbtgt$ctx->{password}");
 	push (@provision_options, "--machinepass=machine$ctx->{password}");
@@ -722,6 +727,7 @@ nogroup:x:65534:nobody
 		DOMAIN => $ctx->{domain},
 		USERNAME => $ctx->{username},
 		REALM => $ctx->{realm},
+		SAMSID => $ctx->{samsid},
 		PASSWORD => $ctx->{password},
 		LDAPDIR => $ctx->{ldapdir},
 		LDAP_INSTANCE => $ctx->{ldap_instance},
@@ -757,6 +763,10 @@ nogroup:x:65534:nobody
 		$ret->{RESOLV_WRAPPER_HOSTS} = $ctx->{dns_host_file};
 	}
 
+	if ($ctx->{server_role} eq "domain controller") {
+		$ret->{DOMSID} = $ret->{SAMSID};
+	}
+
 	return $ret;
 }
 
@@ -872,9 +882,13 @@ sub provision($$$$$$$$$$)
 	    $password, $kdc_ipv4, $kdc_ipv6, $extra_smbconf_options, $extra_smbconf_shares,
 	    $extra_provision_options) = @_;
 
+	my $samsid = Samba::random_domain_sid();
+
 	my $ctx = $self->provision_raw_prepare($prefix, $server_role,
 					       $hostname,
-					       $domain, $realm, $functional_level,
+					       $domain, $realm,
+					       $samsid,
+					       $functional_level,
 					       $password, $kdc_ipv4, $kdc_ipv6);
 
 	if (defined($extra_provision_options)) {
@@ -1086,6 +1100,7 @@ rpc_server:tcpip = no
 	$ret->{MEMBER_USERNAME} = $ret->{USERNAME};
 	$ret->{MEMBER_PASSWORD} = $ret->{PASSWORD};
 
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 	$ret->{DC_SERVER} = $dcvars->{DC_SERVER};
 	$ret->{DC_SERVER_IP} = $dcvars->{DC_SERVER_IP};
 	$ret->{DC_SERVER_IPV6} = $dcvars->{DC_SERVER_IPV6};
@@ -1191,6 +1206,7 @@ sub provision_rpc_proxy($$$)
 	$ret->{RPC_PROXY_USERNAME} = $ret->{USERNAME};
 	$ret->{RPC_PROXY_PASSWORD} = $ret->{PASSWORD};
 
+	$ret->{DOMSID} = $dcvars->{DOMSID};
 	$ret->{DC_SERVER} = $dcvars->{DC_SERVER};
 	$ret->{DC_SERVER_IP} = $dcvars->{DC_SERVER_IP};
 	$ret->{DC_SERVER_IPV6} = $dcvars->{DC_SERVER_IPV6};
@@ -1211,6 +1227,7 @@ sub provision_promoted_dc($$$)
 					       "promotedvdc",
 					       $dcvars->{DOMAIN},
 					       $dcvars->{REALM},
+					       $dcvars->{SAMSID},
 					       "2008",
 					       $dcvars->{PASSWORD},
 					       $dcvars->{SERVER_IP},
@@ -1306,6 +1323,7 @@ sub provision_vampire_dc($$$)
 					       $name,
 					       $dcvars->{DOMAIN},
 					       $dcvars->{REALM},
+					       $dcvars->{DOMSID},
 					       $fl,
 					       $dcvars->{PASSWORD},
 					       $dcvars->{SERVER_IP},
@@ -1382,10 +1400,12 @@ sub provision_subdom_dc($$$)
 	print "PROVISIONING SUBDOMAIN DC...\n";
 
 	# We do this so that we don't run the provision.  That's the job of 'net vampire'.
+	my $samsid = undef; # TODO pass the domain sid all the way down
 	my $ctx = $self->provision_raw_prepare($prefix, "domain controller",
 					       "localsubdc",
 					       "SAMBASUBDOM",
 					       "sub.samba.example.com",
+					       $samsid,
 					       "2008",
 					       $dcvars->{PASSWORD},
 					       undef);
@@ -1653,6 +1673,7 @@ sub provision_rodc($$$)
 					       "rodc",
 					       $dcvars->{DOMAIN},
 					       $dcvars->{REALM},
+					       $dcvars->{DOMSID},
 					       "2008",
 					       $dcvars->{PASSWORD},
 					       $dcvars->{SERVER_IP},
-- 
1.9.1


From 76ebe7be848294752e264b187341e70a85cfbd06 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 17:04:00 +0100
Subject: [PATCH 16/26] samba-tool: allow sid strings for 'group
 {add,remove}members'

This makes it possible to add foreign SIDS as group members.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 python/samba/samdb.py | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index 89014a5..348bd21 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -32,6 +32,7 @@ from samba.ndr import ndr_unpack, ndr_pack
 from samba.dcerpc import drsblobs, misc
 from samba.common import normalise_int32
 from samba.compat import text_type
+from samba.dcerpc import security
 
 __docformat__ = "restructuredText"
 
@@ -270,25 +271,40 @@ changetype: modify
             for member in members:
                 filter = ('(&(sAMAccountName=%s)(|(objectclass=user)'
                           '(objectclass=group)))' % ldb.binary_encode(member))
+                foreign_msg = None
+                try:
+                    membersid = security.dom_sid(member)
+                except TypeError as e:
+                    membersid = None
+
+                if membersid is not None:
+                    filter = '(objectSid=%s)' % str(membersid)
+                    dn_str = "<SID=%s>" % str(membersid)
+                    foreign_msg = ldb.Message()
+                    foreign_msg.dn = ldb.Dn(self, dn_str)
+
                 targetmember = self.search(base=self.domain_dn(),
                                            scope=ldb.SCOPE_SUBTREE,
                                            expression="%s" % filter,
                                            attrs=[])
 
+                if len(targetmember) == 0 and foreign_msg is not None:
+                    targetmember = [foreign_msg]
                 if len(targetmember) != 1:
                     raise Exception('Unable to find "%s". Operation cancelled.' % member)
+                targetmember_dn = targetmember[0].dn.extended_str(1)
 
-                if add_members_operation is True and (targetgroup[0].get('member') is None or str(targetmember[0].dn) not in targetgroup[0]['member']):
+                if add_members_operation is True and (targetgroup[0].get('member') is None or str(targetmember_dn) not in targetgroup[0]['member']):
                     modified = True
                     addtargettogroup += """add: member
 member: %s
-""" % (str(targetmember[0].dn))
+""" % (str(targetmember_dn))
 
-                elif add_members_operation is False and (targetgroup[0].get('member') is not None and str(targetmember[0].dn) in targetgroup[0]['member']):
+                elif add_members_operation is False and (targetgroup[0].get('member') is not None and targetmember_dn in targetgroup[0]['member']):
                     modified = True
                     addtargettogroup += """delete: member
 member: %s
-""" % (str(targetmember[0].dn))
+""" % (str(targetmember_dn))
 
             if modified is True:
                 self.modify_ldif(addtargettogroup)
-- 
1.9.1


From 7ada33b3965187b46c3e302a26a01c84fab37ff1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 17:05:49 +0100
Subject: [PATCH 17/26] selftest/Samba4: create add ${TRUST_DOMSID}-513 to a
 local group

This will allow testing expanding groups on the trust boundary.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/target/Samba4.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 608265c..842cd1e 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -399,6 +399,20 @@ sub setup_trust($$$$$)
 		return undef;
 	}
 
+	my $groupname = "g_$localenv->{TRUST_DOMAIN}";
+	my $groupadd = $cmd_env;
+	$groupadd .= " $samba_tool group add '$groupname' --group-scope=Domain $cmd_config";
+	unless (system($groupadd) == 0) {
+		warn("Failed to create group \n$groupadd");
+		return undef;
+	}
+	my $groupmem = $cmd_env;
+	$groupmem .= " $samba_tool group addmembers '$groupname' '$localenv->{TRUST_DOMSID}-513' $cmd_config";
+	unless (system($groupmem) == 0) {
+		warn("Failed to add group member \n$groupmem");
+		return undef;
+	}
+
 	return $localenv
 }
 
-- 
1.9.1


From 716f4f17fa75cabb7cf110b599aadd9fefc0ce54 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 17:46:55 +0100
Subject: [PATCH 18/26] testprogs/blackbox: add test_trust_token.sh

This demonstrates, which SID we expect in a token of
an user of a trusted domain.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 testprogs/blackbox/test_trust_token.sh | 93 ++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100755 testprogs/blackbox/test_trust_token.sh

diff --git a/testprogs/blackbox/test_trust_token.sh b/testprogs/blackbox/test_trust_token.sh
new file mode 100755
index 0000000..2dd4e37
--- /dev/null
+++ b/testprogs/blackbox/test_trust_token.sh
@@ -0,0 +1,93 @@
+#!/bin/bash
+# Copyright (C) 2017 Stefan Metzmacher <metze at samba.org>
+
+if [ $# -lt 12 ]; then
+cat <<EOF
+Usage: $# test_trust_token.sh SERVER USERNAME PASSWORD REALM DOMAIN DOMSID TRUST_USERNAME TRUST_PASSWORD TRUST_REALM TRUST_DOMAIN TRUST_DOMSID TYPE
+EOF
+exit 1;
+fi
+
+SERVER=$1
+shift 1
+USERNAME=$1
+PASSWORD=$2
+REALM=$3
+DOMAIN=$4
+DOMSID=$5
+shift 5
+TRUST_USERNAME=$1
+TRUST_PASSWORD=$2
+TRUST_REALM=$3
+TRUST_DOMAIN=$4
+TRUST_DOMSID=$5
+shift 5
+TYPE=$1
+shift 1
+failed=0
+
+samba4bindir="$BINDIR"
+
+ldbsearch="$samba4bindir/ldbsearch"
+
+. `dirname $0`/subunit.sh
+. `dirname $0`/common_test_fns.inc
+
+test_token()
+{
+	auth_args="${1}"
+	auth_sid="${2-}"
+
+	out=$($VALGRIND $ldbsearch -H ldap://$SERVER.$REALM -U$TRUST_REALM\\$TRUST_USERNAME%$TRUST_PASSWORD -b '' -s base -k ${auth_args} tokenGroups 2>&1)
+	ret=$?
+	test x"$ret" = x"0" || {
+		echo "$out"
+		return 1
+	}
+
+	trust_sids=$(echo "$out" | grep '^tokenGroups' | grep "${TRUST_DOMSID}-" | wc -l)
+	test "$trust_sids" -ge "2" || {
+		echo "$out"
+		echo "Less than 2 sids from $TRUST_DOMAIN $TRUST_DOMSID"
+		return 1
+	}
+
+	domain_sids=$(echo "$out" | grep '^tokenGroups' | grep "${DOMSID}-" | wc -l)
+	test "$domain_sids" -ge "1" || {
+		echo "$out"
+		echo "Less than 1 sid from $DOMAIN $DOMSID"
+		return 1
+	}
+
+	builtin_sids=$(echo "$out" | grep '^tokenGroups' | grep "S-1-5-32-" | wc -l)
+	test "$builtin_sids" -ge "1" || {
+		echo "$out"
+		echo "Less than 1 sid from BUILTIN S-1-5-32"
+		return 1
+	}
+
+	#
+	# The following should always be present
+	#
+	# SID_WORLD(S-1-1-0)
+	# SID_NT_NETWORK(S-1-5-2)
+	# SID_NT_AUTHENTICATED_USERS(S-1-5-11)
+	#
+	required_sids="S-1-1-0 S-1-5-2 S-1-5-11 ${auth_sid}"
+	for sid in $required_sids; do
+		found=$(echo "$out" | grep "^tokenGroups: ${sid}$" | wc -l)
+		test x"$found" = x"1" || {
+			echo "$out"
+			echo "SID: ${sid} not found"
+			return 1
+		}
+	done
+
+	return 0
+}
+
+testit "Test token with kerberos" test_token "yes" "" || failed=`expr $failed + 1`
+# Check that SID_NT_NTLM_AUTHENTICATION(S-1-5-64-10) is added for NTLMSSP
+testit "Test token with NTLMSSP" test_token "no" "S-1-5-64-10" || failed=`expr $failed + 1`
+
+exit $failed
-- 
1.9.1


From c96204d3556302f5afef0ebb624ba843450687de Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 26 Feb 2018 17:46:55 +0100
Subject: [PATCH 19/26] s4:selftest: run samba4.blackbox.trust_token against
 fl2003dc and fl2008r2dc

This fails currently as we don't expand groups on the trust boundary.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/expand_trust_token | 2 ++
 source4/selftest/tests.py               | 2 ++
 2 files changed, 4 insertions(+)
 create mode 100644 selftest/knownfail.d/expand_trust_token

diff --git a/selftest/knownfail.d/expand_trust_token b/selftest/knownfail.d/expand_trust_token
new file mode 100644
index 0000000..79d8448
--- /dev/null
+++ b/selftest/knownfail.d/expand_trust_token
@@ -0,0 +1,2 @@
+^samba4.blackbox.trust_token.Test.token.with.kerberos
+^samba4.blackbox.trust_token.Test.token.with.NTLMSSP
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 4e397a8..d15c464 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -413,6 +413,8 @@ plantestsuite("samba4.blackbox.trust_ntlm", "nt4_member:local", [os.path.join(bb
 
 plantestsuite("samba4.blackbox.trust_utils(fl2008r2dc:local)", "fl2008r2dc:local", [os.path.join(bbdir, "test_trust_utils.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$TRUST_SERVER', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$PREFIX', "forest"])
 plantestsuite("samba4.blackbox.trust_utils(fl2003dc:local)", "fl2003dc:local", [os.path.join(bbdir, "test_trust_utils.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$TRUST_SERVER', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$PREFIX', "external"])
+plantestsuite("samba4.blackbox.trust_token", "fl2008r2dc", [os.path.join(bbdir, "test_trust_token.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$DOMSID', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$TRUST_DOMSID', 'forest'])
+plantestsuite("samba4.blackbox.trust_token", "fl2003dc", [os.path.join(bbdir, "test_trust_token.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', '$DOMSID', '$TRUST_USERNAME', '$TRUST_PASSWORD', '$TRUST_REALM', '$TRUST_DOMAIN', '$TRUST_DOMSID', 'external'])
 plantestsuite("samba4.blackbox.ktpass(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(bbdir, "test_ktpass.sh"), '$PREFIX/ad_dc_ntvfs'])
 plantestsuite("samba4.blackbox.password_settings(ad_dc_ntvfs:local)", "ad_dc_ntvfs:local", [os.path.join(bbdir, "test_password_settings.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$REALM', '$DOMAIN', "$PREFIX/ad_dc_ntvfs"])
 plantestsuite("samba4.blackbox.cifsdd(ad_dc_ntvfs)", "ad_dc_ntvfs", [os.path.join(samba4srcdir, "client/tests/test_cifsdd.sh"), '$SERVER', '$USERNAME', '$PASSWORD', "$DOMAIN"])
-- 
1.9.1


From b9a320af772a9c10c8ae7ceaf59e41fe72aec0c7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 2 Feb 2018 04:08:47 +0100
Subject: [PATCH 20/26] s4:auth: split out a authsam_domain_group_filter()
 function

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/sam.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index 49e34ba..bb64bd9 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -286,6 +286,41 @@ _PUBLIC_ NTSTATUS authsam_account_ok(TALLOC_CTX *mem_ctx,
 	return NT_STATUS_OK;
 }
 
+static NTSTATUS authsam_domain_group_filter(TALLOC_CTX *mem_ctx,
+					    char **_filter)
+{
+	char *filter = NULL;
+
+	*_filter = NULL;
+
+	filter = talloc_strdup(mem_ctx, "(&(objectClass=group)");
+	if (filter == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	/*
+	 * Skip all builtin groups, they're added later.
+	 */
+	filter = talloc_asprintf_append_buffer(filter,
+				"(!(groupType:1.2.840.113556.1.4.803:=%u))",
+				GROUP_TYPE_BUILTIN_LOCAL_GROUP);
+	if (filter == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	/*
+	 * Only include security groups.
+	 */
+	filter = talloc_asprintf_append_buffer(filter,
+				"(groupType:1.2.840.113556.1.4.803:=%u))",
+				GROUP_TYPE_SECURITY_ENABLED);
+	if (filter == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	*_filter = filter;
+	return NT_STATUS_OK;
+}
+
 _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx,
 					   struct ldb_context *sam_ctx,
 					   const char *netbios_name,
@@ -300,7 +335,8 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx,
 	NTSTATUS status;
 	struct auth_user_info_dc *user_info_dc;
 	struct auth_user_info *info;
-	const char *str, *filter;
+	const char *str = NULL;
+	char *filter = NULL;
 	/* SIDs for the account and his primary group */
 	struct dom_sid *account_sid;
 	const char *primary_group_string;
@@ -346,13 +382,15 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx,
 	sids[PRIMARY_GROUP_SID_INDEX] = *domain_sid;
 	sid_append_rid(&sids[PRIMARY_GROUP_SID_INDEX], ldb_msg_find_attr_as_uint(msg, "primaryGroupID", ~0));
 
-	/* Filter out builtin groups from this token.  We will search
+	/*
+	 * Filter out builtin groups from this token. We will search
 	 * for builtin groups later, and not include them in the PAC
-	 * on SamLogon validation info */
-	filter = talloc_asprintf(tmp_ctx, "(&(objectClass=group)(!(groupType:1.2.840.113556.1.4.803:=%u))(groupType:1.2.840.113556.1.4.803:=%u))", GROUP_TYPE_BUILTIN_LOCAL_GROUP, GROUP_TYPE_SECURITY_ENABLED);
-	if (filter == NULL) {
+	 * or SamLogon validation info.
+	 */
+	status = authsam_domain_group_filter(tmp_ctx, &filter);
+	if (!NT_STATUS_IS_OK(status)) {
 		TALLOC_FREE(user_info_dc);
-		return NT_STATUS_NO_MEMORY;
+		return status;
 	}
 
 	primary_group_string = dom_sid_string(tmp_ctx, &sids[PRIMARY_GROUP_SID_INDEX]);
-- 
1.9.1


From d8ec4989c2ddc5789a1c85fa242f47fbdf081f25 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 23:12:36 +0100
Subject: [PATCH 21/26] s4:auth: add authsam_update_user_info_dc() that
 implements SID expanding for the local domain

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/auth.h |  3 +++
 source4/auth/sam.c  | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/source4/auth/auth.h b/source4/auth/auth.h
index f88489b..51895c9 100644
--- a/source4/auth/auth.h
+++ b/source4/auth/auth.h
@@ -136,6 +136,9 @@ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, struct ldb_context *sam_
 					   struct ldb_message *msg,
 					   DATA_BLOB user_sess_key, DATA_BLOB lm_sess_key,
 				  struct auth_user_info_dc **_user_info_dc);
+NTSTATUS authsam_update_user_info_dc(TALLOC_CTX *mem_ctx,
+			struct ldb_context *sam_ctx,
+			struct auth_user_info_dc *user_info_dc);
 NTSTATUS auth_system_session_info(TALLOC_CTX *parent_ctx,
 					   struct loadparm_context *lp_ctx,
 					   struct auth_session_info **_session_info) ;
diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index bb64bd9..b5c9d5e 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -589,6 +589,68 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx,
 	return NT_STATUS_OK;
 }
 
+_PUBLIC_ NTSTATUS authsam_update_user_info_dc(TALLOC_CTX *mem_ctx,
+			struct ldb_context *sam_ctx,
+			struct auth_user_info_dc *user_info_dc)
+{
+	char *filter = NULL;
+	NTSTATUS status;
+	uint32_t i;
+	uint32_t n = 0;
+
+	/*
+	 * This function exists to expand group memberships
+	 * in the local domain (forest), as the token
+	 * may come from a different domain.
+	 */
+
+	/*
+	 * Filter out builtin groups from this token. We will search
+	 * for builtin groups later.
+	 */
+	status = authsam_domain_group_filter(mem_ctx, &filter);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(user_info_dc);
+		return status;
+	}
+
+	/*
+	 * We loop only over the existing number of
+	 * sids.
+	 */
+	n = user_info_dc->num_sids;
+	for (i = 0; i < n; i++) {
+		struct dom_sid *sid = &user_info_dc->sids[i];
+		char sid_buf[DOM_SID_STR_BUFLEN] = {0,};
+		char dn_str[DOM_SID_STR_BUFLEN*2] = {0,};
+		DATA_BLOB dn_blob = data_blob_null;
+		int len;
+
+		len = dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
+		if (len+1 > sizeof(sid_buf)) {
+			return NT_STATUS_INVALID_SID;
+		}
+		snprintf(dn_str, sizeof(dn_str) - 1, "<SID=%s>", sid_buf);
+		dn_blob = data_blob_string_const(dn_str);
+
+		/*
+		 * We already have the SID in the token, so set
+		 * 'only childs' flag to true and add all
+		 * groups which match the filter.
+		 */
+		status = dsdb_expand_nested_groups(sam_ctx, &dn_blob,
+						   true, filter,
+						   user_info_dc,
+						   &user_info_dc->sids,
+						   &user_info_dc->num_sids);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+	}
+
+	return NT_STATUS_OK;
+}
+
 NTSTATUS sam_get_results_principal(struct ldb_context *sam_ctx,
 				   TALLOC_CTX *mem_ctx, const char *principal,
 				   const char **attrs,
-- 
1.9.1


From 745d663690a22107f76383014b31c66ebc288466 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 9 Jan 2018 09:23:26 +0100
Subject: [PATCH 22/26] s4:auth_winbind: only call
 authsam_logon_success_accounting() for local users

There's no need to do a crack_name_to_nt4_name(), as the authentication
already provides the nt4 domain and account names.

This should only happen on an RODC, that we use the winbind auth module
for local users. So we should make sure we only try to reset
the badPwdCount for users of our own domain.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/auth/ntlm/auth_winbind.c | 57 ++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c
index 2e88563..c627df7 100644
--- a/source4/auth/ntlm/auth_winbind.c
+++ b/source4/auth/ntlm/auth_winbind.c
@@ -200,10 +200,10 @@ static void winbind_check_password_done(struct tevent_req *subreq)
 		struct winbind_check_password_state);
 	struct auth_method_context *ctx = state->ctx;
 	const struct auth_usersupplied_info *user_info = state->user_info;
-	const char *account_name = user_info->mapped.account_name;
 	struct ldb_dn *domain_dn = NULL;
+	const char *nt4_domain = NULL;
+	const char *nt4_account = NULL;
 	struct ldb_message *msg = NULL;
-	const char *p = NULL;
 	NTSTATUS status;
 
 	status = dcerpc_winbind_SamLogon_r_recv(subreq, state);
@@ -224,32 +224,31 @@ static void winbind_check_password_done(struct tevent_req *subreq)
 		return;
 	}
 
-	/*
-	 * At best, reset the badPwdCount to 0 if the account exists.
-	 * This means that lockouts happen at a badPwdCount earlier than
-	 * normal, but makes it more fault tolerant.
-	 */
-	p = strchr_m(account_name, '@');
-	if (p != NULL) {
-		const char *nt4_domain = NULL;
-		const char *nt4_account = NULL;
-
-		status = crack_name_to_nt4_name(state,
-						ctx->auth_ctx->sam_ctx,
-						DRSUAPI_DS_NAME_FORMAT_USER_PRINCIPAL,
-						account_name,
-						&nt4_domain, &nt4_account);
-		if (NT_STATUS_IS_OK(status) &&
-		    lpcfg_is_mydomain(ctx->auth_ctx->lp_ctx, nt4_domain))
-		{
-			account_name = nt4_account;
-		}
+	status = make_user_info_dc_netlogon_validation(state,
+						      user_info->client.account_name,
+						      state->req.in.validation_level,
+						      &state->req.out.validation,
+						      true, /* This user was authenticated */
+						      &state->user_info_dc);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
+	nt4_domain = state->user_info_dc->info->domain_name;
+	nt4_account = state->user_info_dc->info->account_name;
+
+	if (lpcfg_is_mydomain(ctx->auth_ctx->lp_ctx, nt4_domain)) {
+		domain_dn = ldb_get_default_basedn(ctx->auth_ctx->sam_ctx);
 	}
 
-	domain_dn = ldb_get_default_basedn(ctx->auth_ctx->sam_ctx);
 	if (domain_dn != NULL) {
+		/*
+		 * At best, reset the badPwdCount to 0 if the account exists.
+		 * This means that lockouts happen at a badPwdCount earlier than
+		 * normal, but makes it more fault tolerant.
+		 */
 		status = authsam_search_account(state, ctx->auth_ctx->sam_ctx,
-						account_name, domain_dn, &msg);
+						nt4_account, domain_dn, &msg);
 		if (NT_STATUS_IS_OK(status)) {
 			authsam_logon_success_accounting(
 				ctx->auth_ctx->sam_ctx, msg,
@@ -259,16 +258,6 @@ static void winbind_check_password_done(struct tevent_req *subreq)
 		}
 	}
 
-	status = make_user_info_dc_netlogon_validation(state,
-						      user_info->client.account_name,
-						      state->req.in.validation_level,
-						      &state->req.out.validation,
-						      true, /* This user was authenticated */
-						      &state->user_info_dc);
-	if (tevent_req_nterror(req, status)) {
-		return;
-	}
-
 	tevent_req_done(req);
 }
 
-- 
1.9.1


From 85875f50830963532ef124fe77d588bb1771c007 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 2 Feb 2018 12:37:51 +0100
Subject: [PATCH 23/26] s4:auth_winbind: make sure we expand group memberships
 of the local domain

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/expand_trust_token |  1 -
 source4/auth/ntlm/auth_winbind.c        | 12 ++++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail.d/expand_trust_token b/selftest/knownfail.d/expand_trust_token
index 79d8448..c0d44d7 100644
--- a/selftest/knownfail.d/expand_trust_token
+++ b/selftest/knownfail.d/expand_trust_token
@@ -1,2 +1 @@
 ^samba4.blackbox.trust_token.Test.token.with.kerberos
-^samba4.blackbox.trust_token.Test.token.with.NTLMSSP
diff --git a/source4/auth/ntlm/auth_winbind.c b/source4/auth/ntlm/auth_winbind.c
index c627df7..a3efde8 100644
--- a/source4/auth/ntlm/auth_winbind.c
+++ b/source4/auth/ntlm/auth_winbind.c
@@ -258,6 +258,18 @@ static void winbind_check_password_done(struct tevent_req *subreq)
 		}
 	}
 
+	/*
+	 * We need to expand group memberships within our local domain,
+	 * as the token might be generated by a trusted domain, unless we're
+	 * an RODC.
+	 */
+	status = authsam_update_user_info_dc(state->user_info_dc,
+					     ctx->auth_ctx->sam_ctx,
+					     state->user_info_dc);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+
 	tevent_req_done(req);
 }
 
-- 
1.9.1


From 5d75dd765c84717144899e83712b5ed0a14338c2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 11:44:21 +0100
Subject: [PATCH 24/26] s4:kdc: remember is_krbtgt, is_rodc and is_trust
 samba_kdc_entry

This can later be used for sid filtering and similar things.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/kdc/db-glue.c   | 6 +++++-
 source4/kdc/samba_kdc.h | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 8ccc34c..c2dd236 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -828,6 +828,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
 		goto out;
 	}
 
+	p->is_rodc = is_rodc;
 	p->kdc_db_ctx = kdc_db_ctx;
 	p->realm_dn = talloc_reference(p, realm_dn);
 	if (!p->realm_dn) {
@@ -874,6 +875,8 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
 	 */
 
 	if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) {
+		p->is_krbtgt = true;
+
 		if (flags & (SDB_F_CANON)) {
 			/*
 			 * When requested to do so, ensure that the
@@ -1272,12 +1275,13 @@ static krb5_error_code samba_kdc_trust_message2entry(krb5_context context,
 		goto out;
 	}
 
-	p = talloc(mem_ctx, struct samba_kdc_entry);
+	p = talloc_zero(mem_ctx, struct samba_kdc_entry);
 	if (!p) {
 		ret = ENOMEM;
 		goto out;
 	}
 
+	p->is_trust = true;
 	p->kdc_db_ctx = kdc_db_ctx;
 	p->realm_dn = realm_dn;
 
diff --git a/source4/kdc/samba_kdc.h b/source4/kdc/samba_kdc.h
index b76cc31..e228a82 100644
--- a/source4/kdc/samba_kdc.h
+++ b/source4/kdc/samba_kdc.h
@@ -54,6 +54,9 @@ struct samba_kdc_entry {
 	struct samba_kdc_db_context *kdc_db_ctx;
 	struct ldb_message *msg;
 	struct ldb_dn *realm_dn;
+	bool is_krbtgt;
+	bool is_rodc;
+	bool is_trust;
 	void *entry_ex;
 };
 
-- 
1.9.1


From 4c3c02f1e46ae76a701ca60b1a5aab4c6b5ee754 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 18:40:58 +0100
Subject: [PATCH 25/26] s4:kdc: pass krbtgt and server to
 samba_kdc_update_pac_blob()

This will be used for SID expanding and filtering.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/kdc/mit_samba.c  | 12 +++++++++++-
 source4/kdc/pac-glue.c   |  2 ++
 source4/kdc/pac-glue.h   |  2 ++
 source4/kdc/wdc-samba4.c |  1 +
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c
index 1cd6750..414e67c 100644
--- a/source4/kdc/mit_samba.c
+++ b/source4/kdc/mit_samba.c
@@ -481,7 +481,8 @@ krb5_error_code mit_samba_reget_pac(struct mit_samba_context *ctx,
 	DATA_BLOB *upn_blob = NULL;
 	DATA_BLOB *deleg_blob = NULL;
 	struct samba_kdc_entry *client_skdc_entry = NULL;
-	struct samba_kdc_entry *krbtgt_skdc_entry;
+	struct samba_kdc_entry *krbtgt_skdc_entry = NULL;
+	struct samba_kdc_entry *server_skdc_entry = NULL;
 	bool is_in_db = false;
 	bool is_untrusted = false;
 	size_t num_types = 0;
@@ -509,6 +510,13 @@ krb5_error_code mit_samba_reget_pac(struct mit_samba_context *ctx,
 		}
 	}
 
+	if (server == NULL) {
+		return EINVAL;
+	}
+	server_skdc_entry =
+		talloc_get_type_abort(server->e_data,
+				      struct samba_kdc_entry);
+
 	if (krbtgt == NULL) {
 		return EINVAL;
 	}
@@ -567,6 +575,8 @@ krb5_error_code mit_samba_reget_pac(struct mit_samba_context *ctx,
 
 		nt_status = samba_kdc_update_pac_blob(tmp_ctx,
 						      context,
+						      krbtgt_skdc_entry,
+						      server_skdc_entry,
 						      *pac,
 						      pac_blob,
 						      pac_srv_sig,
diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
index 1a862e2..9b5f309 100644
--- a/source4/kdc/pac-glue.c
+++ b/source4/kdc/pac-glue.c
@@ -747,6 +747,8 @@ NTSTATUS samba_kdc_get_pac_blob(TALLOC_CTX *mem_ctx,
 
 NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
 				   krb5_context context,
+				   struct samba_kdc_entry *krbtgt,
+				   struct samba_kdc_entry *server,
 				   const krb5_pac pac, DATA_BLOB *pac_blob,
 				   struct PAC_SIGNATURE_DATA *pac_srv_sig,
 				   struct PAC_SIGNATURE_DATA *pac_kdc_sig)
diff --git a/source4/kdc/pac-glue.h b/source4/kdc/pac-glue.h
index 92a6bc7..2eb7fd3 100644
--- a/source4/kdc/pac-glue.h
+++ b/source4/kdc/pac-glue.h
@@ -51,6 +51,8 @@ NTSTATUS samba_kdc_get_pac_blob(TALLOC_CTX *mem_ctx,
 
 NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
 				   krb5_context context,
+				   struct samba_kdc_entry *krbtgt,
+				   struct samba_kdc_entry *server,
 				   const krb5_pac pac, DATA_BLOB *pac_blob,
 				   struct PAC_SIGNATURE_DATA *pac_srv_sig,
 				   struct PAC_SIGNATURE_DATA *pac_kdc_sig);
diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c
index b90578c..a7d8de1 100644
--- a/source4/kdc/wdc-samba4.c
+++ b/source4/kdc/wdc-samba4.c
@@ -186,6 +186,7 @@ static krb5_error_code samba_wdc_reget_pac(void *priv, krb5_context context,
 		}
 
 		nt_status = samba_kdc_update_pac_blob(mem_ctx, context,
+						      krbtgt_skdc_entry, p,
 						      *pac, pac_blob,
 						      pac_srv_sig, pac_kdc_sig);
 		if (!NT_STATUS_IS_OK(nt_status)) {
-- 
1.9.1


From f42cde2fbdf42689e58828a4c0a9e127abe4421c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 1 Feb 2018 18:40:58 +0100
Subject: [PATCH 26/26] s4:kdc: make sure we expand group memberships of the
 local domain

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/expand_trust_token |  1 -
 source4/kdc/pac-glue.c                  | 11 +++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
 delete mode 100644 selftest/knownfail.d/expand_trust_token

diff --git a/selftest/knownfail.d/expand_trust_token b/selftest/knownfail.d/expand_trust_token
deleted file mode 100644
index c0d44d7..0000000
--- a/selftest/knownfail.d/expand_trust_token
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.blackbox.trust_token.Test.token.with.kerberos
diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
index 9b5f309..126001c 100644
--- a/source4/kdc/pac-glue.c
+++ b/source4/kdc/pac-glue.c
@@ -763,6 +763,17 @@ NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_UNSUCCESSFUL;
 	}
 
+	/*
+	 * We need to expand group memberships within our local domain,
+	 * as the token might be generated by a trusted domain.
+	 */
+	nt_status = authsam_update_user_info_dc(mem_ctx,
+						krbtgt->kdc_db_ctx->samdb,
+						user_info_dc);
+	if (!NT_STATUS_IS_OK(nt_status)) {
+		return nt_status;
+	}
+
 	nt_status = samba_get_logon_info_pac_blob(mem_ctx, 
 						  user_info_dc, pac_blob);
 
-- 
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/20180227/79d886a7/signature.sig>


More information about the samba-technical mailing list