[Patches] Expanded group memberships on boundaries of outgoing trusts (bugs #13299, #13300, #13307)
Stefan Metzmacher
metze at samba.org
Fri Mar 16 10:30:52 UTC 2018
Hi,
>> Here's the current patchset.
>>
>> It passed autobuild also with this on top:
>
> ...
>
>> Andrew, do you have any further objections?
>> Or can Ralph just continue with his review?
>
> No further objections and thank you so much for all the extra time you
> put into this.
Thanks!
> My only comment is that in auth_winbind, I would note that in the
> future we may use this same pattern to forward bad passwords to our
> PDC, which might have heard about the password change already.
> (Windows does this).
Feel free to propose a patch on top:-)
Does someone has time to finish the review?
The rebased patchset is attached.
Thanks!
metze
-------------- next part --------------
From 2f5c10f64ff733a97babe552919ec136ec1f7795 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 d718e34255905a49a416460ad0e840877d89a783 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 96ecf0770df76482077424875aaffde4b5747ab4 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 65046de8ec2530194644a2497aee0785084c0ef1 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 45469e1a9007ae8707c4417d010bc500dd663891 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 ec988f1a00dcb2d49a63ff29f88d13d872bd9e04 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 874856e84519cb97ed67108168b4861ba2080f93 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 068468d6a55a327aeee90cc62e258c213df70b2d 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 ea5279158a623f96805fc382cdde84fd275bc9bf 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 ac32dbaea8afc8435af7ba7101c890bb8b23d5d3 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 1b9c31b8f4126bb304e680e504808b475e847001 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 8b49475e616110beddaa29d750322b84329f47c7 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 49960e6f7ff60460e9b2619d4d2d308b37c2cd31 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 eaf19f30a75d84d339b5052002b4b285c66804fb 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 b63716406364209bdc21dca10697385ec7051351 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 f9ca9dce87dd8ae3b11316228604053cd0c46062 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 9e087094d9c4e358ea4cebc033c57183c5301f1f 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 42ba61d83f7b4bf7a3f27b0a6de0b303a7a747f0 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 0d4b565f56ee8a48017ecd65b2b63c95c0116b14 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 0f68a57183623b299f9c2422e5fbca853165821d 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 cd518a4d7d020710e2241afc3f53283ab9576bfb 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 0d08f7d6ecfebb472b3b6726103bddc4406d56d5 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 4c05d40c12ee07fc84b678973bbf605c20ead979 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 7880d92..2977b75 100644
--- a/source4/auth/auth.h
+++ b/source4/auth/auth.h
@@ -130,6 +130,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 382f05994dfa50473de0340d89ac5a7f8d825235 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 6b191231ae38c42233673f9bdd9bdbfe751b1fe2 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 5dea8f94509824c04757161b8dd4ffc7b3e7eb33 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 d17a83d5f1cb227b0d2f63885374647c9b3ae013 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 4fdacf1cc6592a89537770c7c400ce51a7e7f1b6 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/20180316/3fb4e493/signature-0001.sig>
More information about the samba-technical
mailing list