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

Stefan Metzmacher metze at samba.org
Tue Mar 6 23:07:27 UTC 2018


Hi,

> Yes, it is noisy but there is still some signal. Attached is a graph
> of the median time (which I should make the default), and also the
> maximum.
> 
> In all cases, the times for adding links in a transaction go up a
> little bit.
> 
> I am not objecting to the patches, though. It is only a little change
> and we can always look at optimising later.

Thanks Douglas!

Here's the current patchset.

It passed autobuild also with this on top:

--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -641,6 +641,11 @@ static int extended_dn_add(struct ldb_module
*module, struct ldb_request *req)
        int ret;
        unsigned int i, j;

+       if (ldb_request_get_control(req,
DSDB_CONTROL_REPLICATED_UPDATE_OID)) {
+               smb_panic(__location__);
+               return LDB_ERR_LOOP_DETECT;
+       }
+
        if (ldb_dn_is_special(req->op.add.message->dn)) {
                /* do not manipulate our control entries */
                return ldb_next_request(module, req);
@@ -724,6 +729,11 @@ static int extended_dn_modify(struct ldb_module
*module, struct ldb_request *req
        struct ldb_control *fix_links_control = NULL;
        int ret;

+       if (ldb_request_get_control(req,
DSDB_CONTROL_REPLICATED_UPDATE_OID)) {
+               smb_panic(__location__);
+               return LDB_ERR_LOOP_DETECT;
+       }
+
        if (ldb_dn_is_special(req->op.mod.message->dn)) {
                /* do not manipulate our control entries */
                return ldb_next_request(module, req);


Andrew, do you have any further objections?
Or can Ralph just continue with his review?

Thanks!
metze


-------------- next part --------------
From f38fd6f5b583ed88bae2e7ea1e40c2fda3ec7b3a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 08:04:38 +0100
Subject: [PATCH 01/28] drsuapi.idl: add DN/fpo-enabled attributes as
 DRSUAPI_ATTID_* values

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 librpc/idl/drsuapi.idl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/librpc/idl/drsuapi.idl b/librpc/idl/drsuapi.idl
index 51ef567..cd90500 100644
--- a/librpc/idl/drsuapi.idl
+++ b/librpc/idl/drsuapi.idl
@@ -460,6 +460,7 @@ interface drsuapi
 		DRSUAPI_ATTID_ou				= 0x0000000b,
 		DRSUAPI_ATTID_description			= 0x0000000d,
 		DRSUAPI_ATTID_member				= 0x0000001f,
+		DRSUAPI_ATTID_distinguishedName			= 0x00000031,
 		DRSUAPI_ATTID_instanceType			= 0x00020001,
 		DRSUAPI_ATTID_whenCreated			= 0x00020002,
 		DRSUAPI_ATTID_possSuperiors			= 0x00020008,
@@ -549,8 +550,13 @@ interface drsuapi
 		DRSUAPI_ATTID_transportAddressAttribute		= 0x0009037f,
 		DRSUAPI_ATTID_msDS_Behavior_Version		= 0x000905b3,
 		DRSUAPI_ATTID_msDS_KeyVersionNumber		= 0x000906f6,
+		DRSUAPI_ATTID_msDS_NonMembers			= 0x00090701,
+		DRSUAPI_ATTID_msDS_MembersForAzRole		= 0x0009070e,
 		DRSUAPI_ATTID_msDS_HasDomainNCs			= 0x0009071c,
 		DRSUAPI_ATTID_msDS_hasMasterNCs			= 0x0009072c,
+		DRSUAPI_ATTID_msDS_NeverRevealGroup		= 0x00090786,
+		DRSUAPI_ATTID_msDS_RevealOnDemandGroup		= 0x00090788,
+		DRSUAPI_ATTID_msDS_HostServiceAccount		= 0x00090808,
 		DRSUAPI_ATTID_isRecycled			= 0x0009080a,
 
 		DRSUAPI_ATTID_INVALID				= 0xFFFFFFFF
-- 
1.9.1


From 998186c070dfeca0fee71e237b8db7eacbeb3d11 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 08:04:58 +0100
Subject: [PATCH 02/28] dsdb:extended_dn_store: ignore
 DRSUAPI_ATTID_distinguishedName attributes

We have several tests which already test that, we can avoid doing
searches at all in that case.

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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index a32ab8d..8829de0 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -331,6 +331,11 @@ static int extended_dn_add(struct ldb_module *module, struct ldb_request *req)
 			continue;
 		}
 
+		if (schema_attr->attributeID_id == DRSUAPI_ATTID_distinguishedName) {
+			/* distinguishedName values are ignored */
+			continue;
+		}
+
 		/* Before we setup a procedure to modify the incoming message, we must copy it */
 		if (!ac->new_req) {
 			struct ldb_message *msg = ldb_msg_copy(ac, req->op.add.message);
@@ -413,6 +418,11 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req
 			continue;
 		}
 
+		if (schema_attr->attributeID_id == DRSUAPI_ATTID_distinguishedName) {
+			/* distinguishedName values are ignored */
+			continue;
+		}
+
 		/* Before we setup a procedure to modify the incoming message, we must copy it */
 		if (!ac->new_req) {
 			struct ldb_message *msg = ldb_msg_copy(ac, req->op.mod.message);
-- 
1.9.1


From e517d147cf0cf80f6de0ecbd52ba27462f246abe Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 10:31:21 +0100
Subject: [PATCH 03/28] dsdb:extended_dn_store: we need to pass down our
 altered request down on NO_SUCH_OBJECT

It's quite likely that there're more than one attribute and we may
already altered values.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 8829de0..47bce53 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -143,7 +143,7 @@ static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
 			/* Otherwise, we are done - let's run the
 			 * request now we have swapped the DNs for the
 			 * full versions */
-			return ldb_next_request(os->ac->module, os->ac->req);
+			return ldb_next_request(os->ac->module, os->ac->new_req);
 		}
 	}
 	if (ares->error != LDB_SUCCESS) {
-- 
1.9.1


From bb50ea2ed9494a2698bdb7025788107a5254f51e 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 04/28] 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 47bce53..119b217 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[] = {
@@ -353,7 +354,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;
 			}
@@ -449,7 +450,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 3bd16008c9e6b96b54fdb96c2276c89ee5aaa7df Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 08:03:24 +0100
Subject: [PATCH 05/28] dsdb:extended_dn_store: We need to ignore self
 references on add operation

We have several schema related tests, which already prove
that for the defaultObjectCategory attribute.

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 | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 119b217..7f26091 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -219,6 +219,7 @@ static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
 
 static int extended_store_replace(struct extended_dn_context *ac,
 				  TALLOC_CTX *callback_mem_ctx,
+				  struct ldb_dn *self_dn,
 				  struct ldb_val *plain_dn,
 				  bool is_delete, 
 				  const struct dsdb_attribute *schema_attr)
@@ -250,6 +251,19 @@ static int extended_store_replace(struct extended_dn_context *ac,
 		return LDB_ERR_INVALID_DN_SYNTAX;
 	}
 
+	if (self_dn != NULL) {
+		ret = ldb_dn_compare(self_dn, os->dsdb_dn->dn);
+		if (ret == 0) {
+			/*
+			 * If this is a reference to the object
+			 * itself during an 'add', we won't
+			 * be able to find the object.
+			 */
+			talloc_free(os);
+			return LDB_SUCCESS;
+		}
+	}
+
 	if (is_delete && !ldb_dn_has_extended(os->dsdb_dn->dn)) {
 		/* NO need to figure this DN out, this element is
 		 * going to be deleted anyway, and becuase it's not
@@ -353,7 +367,9 @@ static int extended_dn_add(struct ldb_module *module, struct ldb_request *req)
 		/* Re-calculate el */
 		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],
+			ret = extended_store_replace(ac, ac->new_req,
+						     req->op.add.message->dn,
+						     &el->values[j],
 						     false, schema_attr);
 			if (ret != LDB_SUCCESS) {
 				return ret;
@@ -449,7 +465,9 @@ static int extended_dn_modify(struct ldb_module *module, struct ldb_request *req
 			 * input of an extended DN */
 			bool is_delete = (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE);
 
-			ret = extended_store_replace(ac, ac->new_req, &el->values[j],
+			ret = extended_store_replace(ac, ac->new_req,
+						     NULL, /* self_dn to be ignored */
+						     &el->values[j],
 						     is_delete, schema_attr);
 			if (ret != LDB_SUCCESS) {
 				talloc_free(ac);
-- 
1.9.1


From 65aa4b4f9c3e56c32b8b0fe0b52ee008c04902a3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 10:31:21 +0100
Subject: [PATCH 06/28] dsdb:extended_dn_store: rename extended_replace_dn to
 extended_replace_callback

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 7f26091..203ed8f 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -117,7 +117,7 @@ static int extended_final_callback(struct ldb_request *req, struct ldb_reply *ar
 	return ret;
 }
 
-static int extended_replace_dn(struct ldb_request *req, struct ldb_reply *ares)
+static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *ares)
 {
 	struct extended_dn_replace_list *os = talloc_get_type(req->context, 
 							   struct extended_dn_replace_list);
@@ -282,7 +282,7 @@ static int extended_store_replace(struct extended_dn_context *ac,
 	 * processing */
 	ret = ldb_build_search_req(&os->search_req,
 				   ac->ldb, os, os->dsdb_dn->dn, LDB_SCOPE_BASE, NULL, 
-				   attrs, NULL, os, extended_replace_dn,
+				   attrs, NULL, os, extended_replace_callback,
 				   ac->req);
 	LDB_REQ_SET_LOCATION(os->search_req);
 	if (ret != LDB_SUCCESS) {
-- 
1.9.1


From 66e78304aba4be190af56eeea6d599c74df0a1ea Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 10:31:21 +0100
Subject: [PATCH 07/28] dsdb:extended_dn_store: split out a
 extended_replace_dn() function

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 | 54 ++++++++++++++--------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 203ed8f..beb64df 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -86,6 +86,37 @@ static struct extended_dn_context *extended_dn_context_init(struct ldb_module *m
 	return ac;
 }
 
+static int extended_replace_dn(struct extended_dn_replace_list *os,
+			       struct ldb_dn *dn)
+{
+	struct dsdb_dn *dsdb_dn = NULL;
+	const char *str = NULL;
+
+	/*
+	 * Rebuild with the string or binary 'extra part' the
+	 * DN may have had as a prefix
+	 */
+	dsdb_dn = dsdb_dn_construct(os, dn,
+				    os->dsdb_dn->extra_part,
+				    os->dsdb_dn->oid);
+	if (dsdb_dn == NULL) {
+		return ldb_module_operr(os->ac->module);
+	}
+
+	str = dsdb_dn_get_extended_linearized(os->mem_ctx,
+					      dsdb_dn, 1);
+	if (str == NULL) {
+		return ldb_module_operr(os->ac->module);
+	}
+
+	/*
+	 * Replace the DN with the extended version of the DN
+	 * (ie, add SID and GUID)
+	 */
+	*os->replace_dn = data_blob_string_const(str);
+	return LDB_SUCCESS;
+}
+
 /* An extra layer of indirection because LDB does not allow the original request to be altered */
 
 static int extended_final_callback(struct ldb_request *req, struct ldb_reply *ares)
@@ -158,24 +189,11 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 		/* This *must* be the right DN, as this is a base
 		 * search.  We can't check, as it could be an extended
 		 * DN, so a module below will resolve it */
-		struct ldb_dn *dn = ares->message->dn;
-		
-		/* Rebuild with the string or binary 'extra part' the
-		 * DN may have had as a prefix */
-		struct dsdb_dn *dsdb_dn = dsdb_dn_construct(ares, dn, 
-							    os->dsdb_dn->extra_part,
-							    os->dsdb_dn->oid);
-		if (dsdb_dn) {
-			/* Replace the DN with the extended version of the DN
-			 * (ie, add SID and GUID) */
-			*os->replace_dn = data_blob_string_const(
-				dsdb_dn_get_extended_linearized(os->mem_ctx, 
-								dsdb_dn, 1));
-			talloc_free(dsdb_dn);
-		}
-		if (os->replace_dn->data == NULL) {
-			return ldb_module_done(os->ac->req, NULL, NULL,
-						LDB_ERR_OPERATIONS_ERROR);
+		int ret;
+
+		ret = extended_replace_dn(os, ares->message->dn);
+		if (ret != LDB_SUCCESS) {
+			return ldb_module_done(os->ac->req, NULL, NULL, ret);
 		}
 		break;
 	}
-- 
1.9.1


From 490f976fd1d093d1ca613c1592eb3880e61d23a1 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 08/28] 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                | 147 +++++++++++++++++++++++++++++-
 selftest/knownfail.d/linked_vs_non_linked |   1 +
 2 files changed, 147 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..afbc2e3 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -23,8 +23,9 @@ 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
+from samba import werror
 import ldb
 import samba
 import uuid
@@ -269,6 +270,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 2c3ac8b2e4731d5e6e9807f957f7d3421bfdc66f 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 09/28] 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 | 136 ++++++++++++++++++++-
 2 files changed, 134 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 beb64df..7ec90e2 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -52,6 +52,9 @@ struct extended_dn_replace_list {
 	struct ldb_val *replace_dn;
 	struct extended_dn_context *ac;
 	struct ldb_request *search_req;
+	bool fpo_enabled;
+	bool require_object;
+	bool got_entry;
 };
 
 
@@ -114,6 +117,7 @@ static int extended_replace_dn(struct extended_dn_replace_list *os,
 	 * (ie, add SID and GUID)
 	 */
 	*os->replace_dn = data_blob_string_const(str);
+	os->got_entry = true;
 	return LDB_SUCCESS;
 }
 
@@ -158,6 +162,43 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 					LDB_ERR_OPERATIONS_ERROR);
 	}
 	if (ares->error == LDB_ERR_NO_SUCH_OBJECT) {
+		if (os->got_entry) {
+			/* This is in internal error... */
+			int ret = ldb_module_operr(os->ac->module);
+			return ldb_module_done(os->ac->req, NULL, NULL, ret);
+		}
+
+		if (os->require_object && os->fpo_enabled) {
+			int ret;
+
+			/*
+			 * It's an error if the target doesn't exist,
+			 * unless it's a delete.
+			 *
+			 * Note FPO-enabled attributes generate
+			 * a different error.
+			 */
+			ret = dsdb_module_werror(os->ac->module,
+						 LDB_ERR_NO_SUCH_OBJECT,
+						 WERR_NO_SUCH_USER,
+						"specified dn doesn't exist");
+
+			return ldb_module_done(os->ac->req, NULL, NULL,
+					       ret);
+		}
+
+		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);
+		}
+
 		/* Don't worry too much about dangling references */
 
 		ldb_reset_err_string(os->ac->ldb);
@@ -195,6 +236,7 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 		if (ret != LDB_SUCCESS) {
 			return ldb_module_done(os->ac->req, NULL, NULL, ret);
 		}
+		/* os->got_entry is true at this point */
 		break;
 	}
 	case LDB_REPLY_REFERRAL:
@@ -204,7 +246,38 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 	case LDB_REPLY_DONE:
 
 		talloc_free(ares);
-		
+
+		if (!os->got_entry && os->require_object && os->fpo_enabled) {
+			int ret;
+
+			/*
+			 * It's an error if the target doesn't exist,
+			 * unless it's a delete.
+			 *
+			 * Note FPO-enabled attributes generate
+			 * a different error.
+			 */
+			ret = dsdb_module_werror(os->ac->module,
+						 LDB_ERR_NO_SUCH_OBJECT,
+						 WERR_NO_SUCH_USER,
+						"specified dn doesn't exist");
+
+			return ldb_module_done(os->ac->req, NULL, NULL,
+					       ret);
+		}
+
+		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) {
@@ -250,6 +323,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) {
@@ -308,9 +383,66 @@ 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;
+
+	/*
+	 * Handle FPO-enabled attributes cause a different
+	 * error.
+	 */
+	switch (schema_attr->attributeID_id) {
+	case DRSUAPI_ATTID_member:
+	case DRSUAPI_ATTID_msDS_NonMembers:
+	case DRSUAPI_ATTID_msDS_MembersForAzRole:
+	case DRSUAPI_ATTID_msDS_NeverRevealGroup:
+	case DRSUAPI_ATTID_msDS_RevealOnDemandGroup:
+	case DRSUAPI_ATTID_msDS_HostServiceAccount:
+		os->fpo_enabled = true;
+		break;
+	}
+
+	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 f4683fcb94d5099c031f3beb55b364eb59bc0098 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 10/28] 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 24c2d55..f36f277 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1509,7 +1509,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,
@@ -1524,7 +1524,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 1c6afc429c1eeb30de8c2304b3fdcc1704e78858 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 11/28] 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                | 46 ++++++++++++++++++++++++++++---
 selftest/knownfail.d/duplicate_objectSIDs |  1 +
 2 files changed, 43 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 afbc2e3..7a4ee29 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -215,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 d668836a853f850f04f9f2347282116484c50169 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 12/28] 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 0cd055a00f896aecc597c06bbb8fd72c418135f0 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 13/28] tests/dsdb.py: test creation of
 foreignSecurityPrincipal via 'attr: <SID=...>'

[MS-ADTS] 3.1.1.5.2.3 Special Classes and Attributes claims:

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

'msDS-NonMembers' always generates NOT_SUPPORTED.

'msDS-ServiceAccount' is not defined in any schema
(only msDS-HostServiceAccount).

'msDS-HostServiceAccount' is not an FPO-enabled attribute
and behaves as the 'manager' attribute.

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

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

diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py
index 7a4ee29..b3cf697 100644
--- a/python/samba/tests/dsdb.py
+++ b/python/samba/tests/dsdb.py
@@ -274,6 +274,232 @@ class DsdbTests(TestCase):
         # cleanup
         self.samdb.delete(dn)
 
+    def _test_foreignSecurityPrincipal(self, obj_class, fpo_attr):
+
+        dom_sid = self.samdb.get_domain_sid()
+        lsid_str = str(dom_sid) + "-4294967294"
+        bsid_str = "S-1-5-32-4294967294"
+        fsid_str = "S-1-5-4294967294"
+        basedn   = self.samdb.get_default_basedn()
+        cn       = "dsdb_test_fpo"
+        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": obj_class})
+
+        msg = ldb.Message()
+        msg.dn = dn
+        msg[fpo_attr] = ldb.MessageElement("<SID=%s>" % lsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        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[fpo_attr] = ldb.MessageElement("<SID=%s>" % bsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        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[fpo_attr] = ldb.MessageElement("<SID=%s>" % fsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        try:
+            self.samdb.modify(msg)
+        except ldb.LdbError as e:
+            self.fail("Should have not raised an exception")
+
+        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)
+
+    def test_foreignSecurityPrincipal_member(self):
+        return self._test_foreignSecurityPrincipal(
+                "group", "member")
+
+    def test_foreignSecurityPrincipal_MembersForAzRole(self):
+        return self._test_foreignSecurityPrincipal(
+                "msDS-AzRole", "msDS-MembersForAzRole")
+
+    def test_foreignSecurityPrincipal_NeverRevealGroup(self):
+        return self._test_foreignSecurityPrincipal(
+                "computer", "msDS-NeverRevealGroup")
+
+    def test_foreignSecurityPrincipal_RevealOnDemandGroup(self):
+        return self._test_foreignSecurityPrincipal(
+                "computer", "msDS-RevealOnDemandGroup")
+
+    def _test_fail_foreignSecurityPrincipal(self, obj_class, fpo_attr,
+                                            msg_exp, lerr_exp, werr_exp,
+                                            allow_reference=True):
+
+        dom_sid = self.samdb.get_domain_sid()
+        lsid_str = str(dom_sid) + "-4294967294"
+        bsid_str = "S-1-5-32-4294967294"
+        fsid_str = "S-1-5-4294967294"
+        basedn   = self.samdb.get_default_basedn()
+        cn1       = "dsdb_test_fpo1"
+        dn1_str   = "cn=%s,cn=Users,%s" % (cn1, basedn)
+        dn1 = ldb.Dn(self.samdb, dn1_str)
+        cn2       = "dsdb_test_fpo2"
+        dn2_str   = "cn=%s,cn=Users,%s" % (cn2, basedn)
+        dn2 = ldb.Dn(self.samdb, dn2_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, dn1_str)
+        self.addCleanup(delete_force, self.samdb, dn2_str)
+
+        self.samdb.add({
+            "dn": dn1_str,
+            "objectClass": obj_class})
+
+        self.samdb.add({
+            "dn": dn2_str,
+            "objectClass": obj_class})
+
+        msg = ldb.Message()
+        msg.dn = dn1
+        msg[fpo_attr] = ldb.MessageElement("<SID=%s>" % lsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get %s" % msg_exp)
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, lerr_exp, str(e))
+            werr = "%08X" % werr_exp
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = dn1
+        msg[fpo_attr] = ldb.MessageElement("<SID=%s>" % bsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get %s" % msg_exp)
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, lerr_exp, str(e))
+            werr = "%08X" % werr_exp
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = dn1
+        msg[fpo_attr] = ldb.MessageElement("<SID=%s>" % fsid_str,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        try:
+            self.samdb.modify(msg)
+            self.fail("No exception should get %s" % msg)
+        except ldb.LdbError as e:
+            (code, msg) = e.args
+            self.assertEqual(code, lerr_exp, str(e))
+            werr = "%08X" % werr_exp
+            self.assertTrue(werr in msg, msg)
+
+        msg = ldb.Message()
+        msg.dn = dn1
+        msg[fpo_attr] = ldb.MessageElement("%s" % dn2,
+                                           ldb.FLAG_MOD_ADD,
+                                           fpo_attr)
+        try:
+            self.samdb.modify(msg)
+            if not allow_reference:
+                sel.fail("No exception should get %s" % msg_exp)
+        except ldb.LdbError as e:
+            if allow_reference:
+                self.fail("Should have not raised an exception: %s" % e)
+            (code, msg) = e.args
+            self.assertEqual(code, lerr_exp, str(e))
+            werr = "%08X" % werr_exp
+            self.assertTrue(werr in msg, msg)
+
+        self.samdb.delete(dn2)
+        self.samdb.delete(dn1)
+
+    def test_foreignSecurityPrincipal_NonMembers(self):
+        return self._test_fail_foreignSecurityPrincipal(
+                "group", "msDS-NonMembers",
+                "LDB_ERR_UNWILLING_TO_PERFORM/WERR_NOT_SUPPORTED",
+                ldb.ERR_UNWILLING_TO_PERFORM, werror.WERR_NOT_SUPPORTED,
+                allow_reference=False)
+
+    def test_foreignSecurityPrincipal_HostServiceAccount(self):
+        return self._test_fail_foreignSecurityPrincipal(
+                "computer", "msDS-HostServiceAccount",
+                "LDB_ERR_CONSTRAINT_VIOLATION/WERR_DS_NAME_REFERENCE_INVALID",
+                ldb.ERR_CONSTRAINT_VIOLATION,
+                werror.WERR_DS_NAME_REFERENCE_INVALID)
+
+    def test_foreignSecurityPrincipal_manager(self):
+        return self._test_fail_foreignSecurityPrincipal(
+                "user", "manager",
+                "LDB_ERR_CONSTRAINT_VIOLATION/WERR_DS_NAME_REFERENCE_INVALID",
+                ldb.ERR_CONSTRAINT_VIOLATION,
+                werror.WERR_DS_NAME_REFERENCE_INVALID)
+
     #
     # 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 b/selftest/knownfail.d/foreignSecurityPrincipal
new file mode 100644
index 0000000..90df089
--- /dev/null
+++ b/selftest/knownfail.d/foreignSecurityPrincipal
@@ -0,0 +1,5 @@
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_member
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_MembersForAzRole
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_NeverRevealGroup
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_RevealOnDemandGroup
+^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_NonMembers
-- 
1.9.1


From e42e02128b421646dbe1ea375bb66f3e1c99e428 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 14/28] dsdb:extended_dn_store: add support for FPO
 (foreignSecurityPrincipal) enabled attributes

This implements the handling for FPO-enabled attributes, 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.

Note there's no msDS-ServiceAccount in any schema (only
msDS-HostServiceAccount and that's not an FPO-enabled attribute
at least not in W2008R2)

msDS-NonMembers always generates NOT_SUPPORTED against W2008R2.

See also [MS-SAMR] 3.1.1.8.9 member.

We now create foreignSeurityPrincipal objects on the fly (as needed).

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 selftest/knownfail.d/foreignSecurityPrincipal      |   5 -
 source4/dsdb/samdb/ldb_modules/extended_dn_store.c | 238 ++++++++++++++++++---
 2 files changed, 206 insertions(+), 37 deletions(-)
 delete mode 100644 selftest/knownfail.d/foreignSecurityPrincipal

diff --git a/selftest/knownfail.d/foreignSecurityPrincipal b/selftest/knownfail.d/foreignSecurityPrincipal
deleted file mode 100644
index 90df089..0000000
--- a/selftest/knownfail.d/foreignSecurityPrincipal
+++ /dev/null
@@ -1,5 +0,0 @@
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_member
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_MembersForAzRole
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_NeverRevealGroup
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_RevealOnDemandGroup
-^samba.tests.dsdb.*.samba.tests.dsdb.DsdbTests.test_foreignSecurityPrincipal_NonMembers
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
index 7ec90e2..a37b55c 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_store.c
@@ -67,6 +67,11 @@ struct extended_dn_context {
 
 	struct extended_dn_replace_list *ops;
 	struct extended_dn_replace_list *cur;
+
+	/*
+	 * Used by the FPO-enabled attribute validation.
+	 */
+	struct dsdb_trust_routing_table *routing_table;
 };
 
 
@@ -121,6 +126,171 @@ static int extended_replace_dn(struct extended_dn_replace_list *os,
 	return LDB_SUCCESS;
 }
 
+static int extended_dn_handle_fpo_attr(struct extended_dn_replace_list *os)
+{
+	struct dom_sid target_sid = { 0, };
+	struct dom_sid target_domain = { 0, };
+	struct ldb_message *fmsg = NULL;
+	char *fsid = NULL;
+	const struct dom_sid *domain_sid = NULL;
+	struct ldb_dn *domain_dn = NULL;
+	const struct lsa_TrustDomainInfoInfoEx *tdo = NULL;
+	uint32_t trust_attributes = 0;
+	const char *no_attrs[] = { NULL, };
+	struct ldb_result *res = NULL;
+	NTSTATUS status;
+	bool match;
+	bool ok;
+	int ret;
+
+	/*
+	 * 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(os->dsdb_dn->dn,
+					  &target_sid,
+					  "SID");
+	if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+		/*
+		 * No SID specified
+		 */
+		return dsdb_module_werror(os->ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified dn doesn't exist");
+	}
+	if (!NT_STATUS_IS_OK(status)) {
+		return ldb_module_operr(os->ac->module);
+	}
+	if (ldb_dn_get_extended_comp_num(os->dsdb_dn->dn) != 1) {
+		return dsdb_module_werror(os->ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified extended component other than SID");
+	}
+	if (ldb_dn_get_comp_num(os->dsdb_dn->dn) != 0) {
+		return dsdb_module_werror(os->ac->module,
+					  LDB_ERR_NO_SUCH_OBJECT,
+					  WERR_NO_SUCH_USER,
+					  "specified more the SID");
+	}
+
+	target_domain = target_sid;
+	sid_split_rid(&target_domain, NULL);
+
+	match = dom_sid_equal(&global_sid_Builtin, &target_domain);
+	if (match) {
+		/*
+		 * Non existing BUILTIN sid
+		 */
+		return dsdb_module_werror(os->ac->module,
+				LDB_ERR_NO_SUCH_OBJECT,
+				WERR_NO_SUCH_MEMBER,
+				"specified sid doesn't exist in BUILTIN");
+	}
+
+	domain_sid = samdb_domain_sid(os->ac->ldb);
+	if (domain_sid == NULL) {
+		return ldb_module_operr(os->ac->module);
+	}
+	match = dom_sid_equal(domain_sid, &target_domain);
+	if (match) {
+		/*
+		 * Non existing SID in our domain.
+		 */
+		return dsdb_module_werror(os->ac->module,
+				LDB_ERR_UNWILLING_TO_PERFORM,
+				WERR_DS_INVALID_GROUP_TYPE,
+				"specified sid doesn't exist in domain");
+	}
+
+	if (os->ac->routing_table == NULL) {
+		status = dsdb_trust_routing_table_load(os->ac->ldb, os->ac,
+						       &os->ac->routing_table);
+		if (!NT_STATUS_IS_OK(status)) {
+			return ldb_module_operr(os->ac->module);
+		}
+	}
+
+	tdo = dsdb_trust_domain_by_sid(os->ac->routing_table,
+				       &target_domain, NULL);
+	if (tdo != NULL) {
+		trust_attributes = tdo->trust_attributes;
+	}
+
+	if (trust_attributes & LSA_TRUST_ATTRIBUTE_WITHIN_FOREST) {
+		return dsdb_module_werror(os->ac->module,
+				LDB_ERR_UNWILLING_TO_PERFORM,
+				WERR_DS_INVALID_GROUP_TYPE,
+				"specified sid doesn't exist in forest");
+	}
+
+	fmsg = ldb_msg_new(os);
+	if (fmsg == NULL) {
+		return ldb_module_oom(os->ac->module);
+	}
+
+	fsid = dom_sid_string(fmsg, &target_sid);
+	if (fsid == NULL) {
+		return ldb_module_oom(os->ac->module);
+	}
+
+	domain_dn = ldb_get_default_basedn(os->ac->ldb);
+	if (domain_dn == NULL) {
+		return ldb_module_operr(os->ac->module);
+	}
+
+	fmsg->dn = ldb_dn_copy(fmsg, domain_dn);
+	if (fmsg->dn == NULL) {
+		return ldb_module_oom(os->ac->module);
+	}
+
+	ok = ldb_dn_add_child_fmt(fmsg->dn,
+				  "CN=%s,CN=ForeignSecurityPrincipals",
+				  fsid);
+	if (!ok) {
+		return ldb_module_oom(os->ac->module);
+	}
+
+	ret = ldb_msg_add_string(fmsg, "objectClass", "foreignSecurityPrincipal");
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	ret = dsdb_module_add(os->ac->module, fmsg,
+			      DSDB_FLAG_AS_SYSTEM |
+			      DSDB_FLAG_NEXT_MODULE,
+			      os->ac->req);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	ret = dsdb_module_search_dn(os->ac->module, fmsg, &res,
+				    fmsg->dn, no_attrs,
+				    DSDB_FLAG_AS_SYSTEM |
+				    DSDB_FLAG_NEXT_MODULE |
+				    DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT,
+				    os->ac->req);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	/*
+	 * dsdb_module_search_dn() garantees exactly one result message
+	 * on success.
+	 */
+	ret = extended_replace_dn(os, res->msgs[0]->dn);
+	TALLOC_FREE(fmsg);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	return LDB_SUCCESS;
+}
+
 /* An extra layer of indirection because LDB does not allow the original request to be altered */
 
 static int extended_final_callback(struct ldb_request *req, struct ldb_reply *ares)
@@ -171,20 +341,12 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 		if (os->require_object && os->fpo_enabled) {
 			int ret;
 
-			/*
-			 * It's an error if the target doesn't exist,
-			 * unless it's a delete.
-			 *
-			 * Note FPO-enabled attributes generate
-			 * a different error.
-			 */
-			ret = dsdb_module_werror(os->ac->module,
-						 LDB_ERR_NO_SUCH_OBJECT,
-						 WERR_NO_SUCH_USER,
-						"specified dn doesn't exist");
-
-			return ldb_module_done(os->ac->req, NULL, NULL,
-					       ret);
+			ret = extended_dn_handle_fpo_attr(os);
+			if (ret != LDB_SUCCESS) {
+				return ldb_module_done(os->ac->req, NULL, NULL,
+						       ret);
+			}
+			/* os->got_entry is true at this point... */
 		}
 
 		if (!os->got_entry && os->require_object) {
@@ -250,20 +412,12 @@ static int extended_replace_callback(struct ldb_request *req, struct ldb_reply *
 		if (!os->got_entry && os->require_object && os->fpo_enabled) {
 			int ret;
 
-			/*
-			 * It's an error if the target doesn't exist,
-			 * unless it's a delete.
-			 *
-			 * Note FPO-enabled attributes generate
-			 * a different error.
-			 */
-			ret = dsdb_module_werror(os->ac->module,
-						 LDB_ERR_NO_SUCH_OBJECT,
-						 WERR_NO_SUCH_USER,
-						"specified dn doesn't exist");
-
-			return ldb_module_done(os->ac->req, NULL, NULL,
-					       ret);
+			ret = extended_dn_handle_fpo_attr(os);
+			if (ret != LDB_SUCCESS) {
+				return ldb_module_done(os->ac->req, NULL, NULL,
+						       ret);
+			}
+			/* os->got_entry is true at this point... */
 		}
 
 		if (!os->got_entry && os->require_object) {
@@ -389,18 +543,38 @@ static int extended_store_replace(struct extended_dn_context *ac,
 	os->require_object = true;
 
 	/*
-	 * Handle FPO-enabled attributes cause a different
-	 * error.
+	 * Handle FPO-enabled attributes, 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.
+	 *
+	 * Note there's no msDS-ServiceAccount in any schema (only
+	 * msDS-HostServiceAccount and that's not an FPO-enabled attribute
+	 * at least not in W2008R2)
+	 *
+	 * msDS-NonMembers always generates NOT_SUPPORTED against W2008R2.
+	 *
+	 * See also [MS-SAMR] 3.1.1.8.9 member.
 	 */
 	switch (schema_attr->attributeID_id) {
 	case DRSUAPI_ATTID_member:
-	case DRSUAPI_ATTID_msDS_NonMembers:
 	case DRSUAPI_ATTID_msDS_MembersForAzRole:
 	case DRSUAPI_ATTID_msDS_NeverRevealGroup:
 	case DRSUAPI_ATTID_msDS_RevealOnDemandGroup:
-	case DRSUAPI_ATTID_msDS_HostServiceAccount:
 		os->fpo_enabled = true;
 		break;
+
+	case DRSUAPI_ATTID_msDS_HostServiceAccount:
+		/* This is NOT a FPO-enabled attribute */
+		break;
+
+	case DRSUAPI_ATTID_msDS_NonMembers:
+		return dsdb_module_werror(os->ac->module,
+					  LDB_ERR_UNWILLING_TO_PERFORM,
+					  WERR_NOT_SUPPORTED,
+					  "msDS-NonMembers is not supported");
 	}
 
 	if (schema_attr->linkID == 0) {
-- 
1.9.1


From 0cc6d430d2824a56e78109782172bb0a48c5f472 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 28 Feb 2018 10:48:59 +0100
Subject: [PATCH 15/28] dsdb:repl_meta_data: improve error message in
 get_parsed_dns()

We may have a dn in '<SID=...>' form and ldb_dn_get_linearized()
just gives in empty string.

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

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 62f58ad..a89a816 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2039,8 +2039,12 @@ static int get_parsed_dns(struct ldb_module *module, TALLOC_CTX *mem_ctx,
 			/* we got a DN without a GUID - go find the GUID */
 			int ret = dsdb_module_guid_by_dn(module, dn, &p->guid, parent);
 			if (ret != LDB_SUCCESS) {
-				ldb_asprintf_errstring(ldb, "Unable to find GUID for DN %s\n",
-						       ldb_dn_get_linearized(dn));
+				char *dn_str = NULL;
+				dn_str = ldb_dn_get_extended_linearized(mem_ctx,
+									(dn), 1);
+				ldb_asprintf_errstring(ldb,
+						"Unable to find GUID for DN %s\n",
+						dn_str);
 				if (ret == LDB_ERR_NO_SUCH_OBJECT &&
 				    LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_DELETE &&
 				    ldb_attr_cmp(el->name, "member") == 0) {
-- 
1.9.1


From c8c228b8a2d7aeb08d57d3cbab009d58e419288e 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 16/28] 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 136dfba7251bb44935609eed84ea43c9c2c7efcb 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 17/28] 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 89ec19dd6cdfdaa28a93760f5f059fcb6c9d86ce 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 18/28] 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 d6dc01bd53eb86e9aab6780de8cb3e7fcb5412cb 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 19/28] 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 df548f9605f4699cf85218e5da6e963f78457450 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 20/28] 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 54df4d3e4f73c87425a6ea8f60a567db5e85c4d0 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 21/28] 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 ef752a5..2711034 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 33ba1d99fb0775ca7d2603307a3131dc5146e09d 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 22/28] 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 aabe5ce362c44d30f9924dd14936043500f714b8 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 23/28] 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 9170fab853154180bc8e7d4babb19e6807a46b5b 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 24/28] 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 16a0c243548f379f72a998a2f1d9df05e06c67da 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 25/28] 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 b412714a04887042e3491a2737da80bfd30fa60d 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 26/28] 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 8173976739d1ff5f0b48d1186880f837f10042b9 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 27/28] 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 572cea28f4c5212ac3016a51e2c9db171ab133b1 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 28/28] 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/20180307/3d026c8b/signature-0001.sig>


More information about the samba-technical mailing list