From 6ad163665b0ea166a3df0543ab43351fc4cfa25a Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 27 Nov 2017 10:45:37 +1300 Subject: [PATCH 1/4] tests dsdb: Add tests for optionally unique objectSID's It is possible for foreign security principals to have duplicate object sids, this can be the result of: a replication race condition generating conflict resolution objects or the foreign security principal being deleted and then re-added on a join. Rather than remove unique check on all objectSIDs we wish to allow duplicate objectSIDs for foreign security principals. But enforce the unique constraint for local objects. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13004 Signed-off-by: Gary Lockyer --- python/samba/tests/dsdb.py | 84 ++++++++++++++++++++++++++++++++++++++++++++++ selftest/knownfail.d/dsdb | 1 + 2 files changed, 85 insertions(+) create mode 100644 selftest/knownfail.d/dsdb diff --git a/python/samba/tests/dsdb.py b/python/samba/tests/dsdb.py index 493ea25..c7e6e13 100644 --- a/python/samba/tests/dsdb.py +++ b/python/samba/tests/dsdb.py @@ -542,3 +542,87 @@ class DsdbTests(TestCase): if e[0] != ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION: self.fail("Got %s should have got ERR_UNSUPPORTED_CRITICAL_EXTENSION" % e[1]) + + # Allocate a unique RID for use in the objectSID tests. + # + def allocate_rid(self): + self.samdb.transaction_start() + try: + rid = self.samdb.allocate_rid() + except: + self.samdb.transaction_cancel() + raise + self.samdb.transaction_commit() + return str(rid) + + # Ensure that duplicate objectSID's are permitted for foreign security + # principals. + # + def test_duplicate_objectSIDs_allowed_on_foreign_security_principals(self): + + # + # We need to build a foreign security principal SID + # i.e a SID not in the current domain. + # + dom_sid = self.samdb.get_domain_sid() + if str(dom_sid)[:-1] == "0": + c = "9" + else: + c = "0" + sid = str(dom_sid)[:-1] + c + "-1000" + basedn = self.samdb.get_default_basedn() + cn = sid + dn = "CN=%s,CN=ForeignSecurityPrincipals,%s" % (cn, basedn) + self.samdb.add({ + "dn": dn, + "objectClass": "foreignSecurityPrincipal", + }) + + self.samdb.delete(dn) + + try: + self.samdb.add({ + "dn": dn, + "objectClass": "foreignSecurityPrincipal", + }) + except ldb.LdbError as e: + (code, msg) = e + self.fail("Got unexpected exception %d - %s " + % (code, msg)) + + # + # 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 + # re-add it, this should fail with a constraint violation + # + # + # Note: This test creates objects as ForeignSecurityPrincipals to use the + # code that maps a ForeignSecurityPrincipals DN to an object. + # Otherwise it would need to bypass the automatic objectSID + # generation + def test_duplicate_objectSIDs_not_allowed_on_local_objects(self): + + dom_sid = self.samdb.get_domain_sid() + rid = self.allocate_rid() + sid = str(dom_sid) + "-" + rid + basedn = self.samdb.get_default_basedn() + cn = sid + dn = "CN=%s,CN=ForeignSecurityPrincipals,%s" % (cn, basedn) + self.samdb.add({ + "dn": dn, + "objectClass": "foreignSecurityPrincipal", + }) + self.samdb.delete(dn) + + try: + self.samdb.add({ + "dn": dn, + "objectClass": "foreignSecurityPrincipal", + }) + self.fail("No exception should get LDB_ERR_CONSTRAINT_VIOLATION") + except ldb.LdbError as e: + (code, msg) = e + if code != ldb.ERR_CONSTRAINT_VIOLATION: + self.fail("Got %d - %s should have got " + "LDB_ERR_CONSTRAINT_VIOLATION" + % (code, msg)) diff --git a/selftest/knownfail.d/dsdb b/selftest/knownfail.d/dsdb new file mode 100644 index 0000000..276b72d --- /dev/null +++ b/selftest/knownfail.d/dsdb @@ -0,0 +1 @@ +^samba.tests.dsdb.samba.tests.dsdb.DsdbTests.test_duplicate_objectSIDs_allowed_on_foreign_security_principals\( -- 2.7.4 From d979d8f8f76c532947ce1847556802e90e03a3c5 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Tue, 21 Nov 2017 07:35:11 +1300 Subject: [PATCH 2/4] ldb ldb_index: Add an attriubute flag to require a unique value. Add attribute flag LDB_FLAG_INTERNAL_UNIQUE_VALUE, to request that the added attribute is unique on the index. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13004 Signed-off-by: Gary Lockyer --- lib/ldb/include/ldb_module.h | 9 ++ lib/ldb/ldb_tdb/ldb_index.c | 3 +- lib/ldb/tests/ldb_mod_op_test.c | 326 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h index ffa6c2e..fd88c62 100644 --- a/lib/ldb/include/ldb_module.h +++ b/lib/ldb/include/ldb_module.h @@ -87,6 +87,15 @@ struct ldb_module; /* force single value checking on this attribute */ #define LDB_FLAG_INTERNAL_FORCE_SINGLE_VALUE_CHECK 0x80 +/* + * ensure that this value is unique on an index + * (despite the index not otherwise being configured as UNIQUE). + * For example, all words starting with 'a' must be unique, but duplicates of + * words starting with b are allowed. This is specifically for Samba's + * objectSid index which is unique in the primary domain only. + */ +#define LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX 0x100 + /* an extended match rule that always fails to match */ #define SAMBA_LDAP_MATCH_ALWAYS_FALSE "1.3.6.1.4.1.7165.4.5.1" diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c index c71e866..0afeae5 100644 --- a/lib/ldb/ldb_tdb/ldb_index.c +++ b/lib/ldb/ldb_tdb/ldb_index.c @@ -1761,7 +1761,8 @@ static int ltdb_index_add1(struct ldb_module *module, */ if (list->count > 0 && ((a != NULL - && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX)) || + && (a->flags & LDB_ATTR_FLAG_UNIQUE_INDEX || + (el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX))) || ldb_attr_cmp(el->name, LTDB_IDXDN) == 0)) { /* * We do not want to print info about a possibly diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c index 48ad20c..cf2288c 100644 --- a/lib/ldb/tests/ldb_mod_op_test.c +++ b/lib/ldb/tests/ldb_mod_op_test.c @@ -3009,7 +3009,317 @@ static void test_read_only(void **state) TALLOC_FREE(tmp_ctx); } +static bool unique_values = false; + +static int unique_index_test_module_add( + struct ldb_module *module, + struct ldb_request *req) +{ + if (unique_values) { + struct ldb_message *msg = discard_const(req->op.add.message); + struct ldb_message_element *el = NULL; + el = ldb_msg_find_element(msg, "cn"); + if (el != NULL) { + el->flags |= LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX; + } + } + + return ldb_next_request(module, req); +} + +static int unique_index_test_module_init(struct ldb_module *module) +{ + return ldb_next_init(module); +} + +static const struct ldb_module_ops ldb_unique_index_test_module_ops = { + .name = "unique_index_test", + .init_context = unique_index_test_module_init, + .add = unique_index_test_module_add, +}; + +static int ldb_unique_index_test_setup(void **state) +{ + int ret; + struct ldb_ldif *ldif; + struct ldbtest_ctx *ldb_test_ctx; + const char *attrs_ldif = \ + "dn: @ATTRIBUTES\n" + "cn: UNIQUE_INDEX\n" + "\n"; + const char *index_ldif = \ + "dn: @INDEXLIST\n" + "@IDXATTR: cn\n" + "\n"; + const char *options[] = {"modules:unique_index_test"}; + + + ret = ldb_register_module(&ldb_unique_index_test_module_ops); + assert_true(ret == LDB_SUCCESS || ret == LDB_ERR_ENTRY_ALREADY_EXISTS); + ldbtest_noconn_setup((void **) &ldb_test_ctx); + + + ret = ldb_connect(ldb_test_ctx->ldb, ldb_test_ctx->dbpath, 0, options); + assert_int_equal(ret, 0); + + while ((ldif = ldb_ldif_read_string(ldb_test_ctx->ldb, &attrs_ldif))) { + ret = ldb_add(ldb_test_ctx->ldb, ldif->msg); + assert_int_equal(ret, LDB_SUCCESS); + } + + while ((ldif = ldb_ldif_read_string(ldb_test_ctx->ldb, &index_ldif))) { + ret = ldb_add(ldb_test_ctx->ldb, ldif->msg); + assert_int_equal(ret, LDB_SUCCESS); + } + + unique_values = true; + + *state = ldb_test_ctx; + return 0; +} + +static int ldb_unique_index_test_teardown(void **state) +{ + int ret; + struct ldbtest_ctx *ldb_test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + struct ldb_dn *del_dn; + + del_dn = ldb_dn_new_fmt(ldb_test_ctx, + ldb_test_ctx->ldb, + "@INDEXLIST"); + assert_non_null(del_dn); + + ret = ldb_delete(ldb_test_ctx->ldb, del_dn); + if (ret != LDB_ERR_NO_SUCH_OBJECT) { + assert_int_equal(ret, LDB_SUCCESS); + } + + assert_dn_doesnt_exist(ldb_test_ctx, + "@INDEXLIST"); + + TALLOC_FREE(del_dn); + + del_dn = ldb_dn_new_fmt(ldb_test_ctx, + ldb_test_ctx->ldb, + "@ATTRIBUTES"); + assert_non_null(del_dn); + + ret = ldb_delete(ldb_test_ctx->ldb, del_dn); + if (ret != LDB_ERR_NO_SUCH_OBJECT) { + assert_int_equal(ret, LDB_SUCCESS); + } + + assert_dn_doesnt_exist(ldb_test_ctx, + "@ATTRIBUTES"); + + ldbtest_teardown((void **) &ldb_test_ctx); + return 0; +} + + +static void test_ldb_add_unique_value_to_unique_index(void **state) +{ + int ret; + struct ldb_message *msg; + struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + TALLOC_CTX *tmp_ctx; + + tmp_ctx = talloc_new(test_ctx); + assert_non_null(tmp_ctx); + + msg = ldb_msg_new(tmp_ctx); + assert_non_null(msg); + + msg->dn = ldb_dn_new_fmt(msg, test_ctx->ldb, "dc=test"); + assert_non_null(msg->dn); + + ret = ldb_msg_add_string(msg, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg); + assert_int_equal(ret, LDB_SUCCESS); + + talloc_free(tmp_ctx); +} + +static int ldb_non_unique_index_test_setup(void **state) +{ + int ret; + struct ldb_ldif *ldif; + struct ldbtest_ctx *ldb_test_ctx; + const char *index_ldif = \ + "dn: @INDEXLIST\n" + "@IDXATTR: cn\n" + "\n"; + const char *options[] = {"modules:unique_index_test"}; + + ret = ldb_register_module(&ldb_unique_index_test_module_ops); + assert_true(ret == LDB_SUCCESS || ret == LDB_ERR_ENTRY_ALREADY_EXISTS); + ldbtest_noconn_setup((void **) &ldb_test_ctx); + + + ret = ldb_connect(ldb_test_ctx->ldb, ldb_test_ctx->dbpath, 0, options); + assert_int_equal(ret, 0); + + while ((ldif = ldb_ldif_read_string(ldb_test_ctx->ldb, &index_ldif))) { + ret = ldb_add(ldb_test_ctx->ldb, ldif->msg); + assert_int_equal(ret, LDB_SUCCESS); + } + + unique_values = true; + + *state = ldb_test_ctx; + return 0; +} + +static int ldb_non_unique_index_test_teardown(void **state) +{ + int ret; + struct ldbtest_ctx *ldb_test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + struct ldb_dn *del_dn; + + del_dn = ldb_dn_new_fmt(ldb_test_ctx, + ldb_test_ctx->ldb, + "@INDEXLIST"); + assert_non_null(del_dn); + + ret = ldb_delete(ldb_test_ctx->ldb, del_dn); + if (ret != LDB_ERR_NO_SUCH_OBJECT) { + assert_int_equal(ret, LDB_SUCCESS); + } + + assert_dn_doesnt_exist(ldb_test_ctx, + "@INDEXLIST"); + + TALLOC_FREE(del_dn); + + ldbtest_teardown((void **) &ldb_test_ctx); + return 0; +} + +static void test_ldb_add_duplicate_value_to_unique_index(void **state) +{ + int ret; + struct ldb_message *msg01; + struct ldb_message *msg02; + struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + TALLOC_CTX *tmp_ctx; + + tmp_ctx = talloc_new(test_ctx); + assert_non_null(tmp_ctx); + + msg01 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01"); + assert_non_null(msg01->dn); + + ret = ldb_msg_add_string(msg01, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg01); + assert_int_equal(ret, LDB_SUCCESS); + + msg02 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02"); + assert_non_null(msg02->dn); + + ret = ldb_msg_add_string(msg02, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg02); + assert_int_equal(ret, LDB_ERR_CONSTRAINT_VIOLATION); + talloc_free(tmp_ctx); +} + +static void test_ldb_add_to_index_duplicates_allowed(void **state) +{ + int ret; + struct ldb_message *msg01; + struct ldb_message *msg02; + struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + TALLOC_CTX *tmp_ctx; + + unique_values = false; + + tmp_ctx = talloc_new(test_ctx); + assert_non_null(tmp_ctx); + + + msg01 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01"); + assert_non_null(msg01->dn); + + ret = ldb_msg_add_string(msg01, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg01); + assert_int_equal(ret, LDB_SUCCESS); + + msg02 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02"); + assert_non_null(msg02->dn); + + ret = ldb_msg_add_string(msg02, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg02); + assert_int_equal(ret, LDB_SUCCESS); + talloc_free(tmp_ctx); +} + +static void test_ldb_add_to_index_unique_values_required(void **state) +{ + int ret; + struct ldb_message *msg01; + struct ldb_message *msg02; + struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + TALLOC_CTX *tmp_ctx; + + unique_values = true; + + tmp_ctx = talloc_new(test_ctx); + assert_non_null(tmp_ctx); + + + msg01 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg01->dn = ldb_dn_new_fmt(msg01, test_ctx->ldb, "dc=test01"); + assert_non_null(msg01->dn); + + ret = ldb_msg_add_string(msg01, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg01); + assert_int_equal(ret, LDB_SUCCESS); + + msg02 = ldb_msg_new(tmp_ctx); + assert_non_null(msg01); + + msg02->dn = ldb_dn_new_fmt(msg02, test_ctx->ldb, "dc=test02"); + assert_non_null(msg02->dn); + + ret = ldb_msg_add_string(msg02, "cn", "test_unique_index"); + assert_int_equal(ret, LDB_SUCCESS); + + ret = ldb_add(test_ctx->ldb, msg02); + assert_int_equal(ret, LDB_ERR_CONSTRAINT_VIOLATION); + talloc_free(tmp_ctx); +} int main(int argc, const char **argv) { const struct CMUnitTest tests[] = { @@ -3133,6 +3443,22 @@ int main(int argc, const char **argv) cmocka_unit_test_setup_teardown(test_read_only, ldb_read_only_setup, ldb_read_only_teardown), + cmocka_unit_test_setup_teardown( + test_ldb_add_unique_value_to_unique_index, + ldb_unique_index_test_setup, + ldb_unique_index_test_teardown), + cmocka_unit_test_setup_teardown( + test_ldb_add_duplicate_value_to_unique_index, + ldb_unique_index_test_setup, + ldb_unique_index_test_teardown), + cmocka_unit_test_setup_teardown( + test_ldb_add_to_index_duplicates_allowed, + ldb_non_unique_index_test_setup, + ldb_non_unique_index_test_teardown), + cmocka_unit_test_setup_teardown( + test_ldb_add_to_index_unique_values_required, + ldb_non_unique_index_test_setup, + ldb_non_unique_index_test_teardown), }; return cmocka_run_group_tests(tests, NULL, NULL); -- 2.7.4 From bc2bb730ad42a66d611f58a52871e3b6454d1dd1 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 27 Nov 2017 11:09:49 +1300 Subject: [PATCH 3/4] source4 dsdb modules: Add new module "unique_object_sids" New module that sets the LDB_FLAG_INTERNAL_UNIQUE_VALUE on all local objectSIDS and ensure it is cleared for any foreign security principals. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13004 Signed-off-by: Gary Lockyer --- .../ldb_modules/tests/test_unique_object_sids.c | 445 +++++++++++++++++++++ .../dsdb/samdb/ldb_modules/unique_object_sids.c | 206 ++++++++++ source4/dsdb/samdb/ldb_modules/wscript_build | 11 + .../dsdb/samdb/ldb_modules/wscript_build_server | 9 + source4/selftest/tests.py | 5 + 5 files changed, 676 insertions(+) create mode 100644 source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c create mode 100644 source4/dsdb/samdb/ldb_modules/unique_object_sids.c diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c b/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c new file mode 100644 index 0000000..24a3e95 --- /dev/null +++ b/source4/dsdb/samdb/ldb_modules/tests/test_unique_object_sids.c @@ -0,0 +1,445 @@ +/* + Unit tests for the unique objectSID code in unique_object_sids.c + + Copyright (C) Andrew Bartlett 2017 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include +#include +#include +#include +#include + +int ldb_unique_object_sids_init(const char *version); +#include "../unique_object_sids.c" + +#include "../libcli/security/dom_sid.h" +#include "librpc/gen_ndr/ndr_security.h" + +#define TEST_BE "tdb" + +#define DOMAIN_SID "S-1-5-21-2470180966-3899876309-2637894779" +#define LOCAL_SID "S-1-5-21-2470180966-3899876309-2637894779-1000" +#define FOREIGN_SID "S-1-5-21-2470180966-3899876309-2637894778-1000" + +static struct ldb_request *last_request; + +/* + * ldb_next_request mock, records the request passed in last_request + * so it can be examined in the test cases. + */ +int ldb_next_request( + struct ldb_module *module, + struct ldb_request *request) +{ + last_request = request; + return ldb_module_done(request, NULL, NULL, LDB_SUCCESS); +} + +/* + * Test context + */ +struct ldbtest_ctx { + struct tevent_context *ev; + struct ldb_context *ldb; + struct ldb_module *module; + + const char *dbfile; + const char *lockfile; /* lockfile is separate */ + + const char *dbpath; + struct dom_sid *domain_sid; +}; + +/* + * Remove any database files created by the tests + */ +static void unlink_old_db(struct ldbtest_ctx *test_ctx) +{ + int ret; + + errno = 0; + ret = unlink(test_ctx->lockfile); + if (ret == -1 && errno != ENOENT) { + fail(); + } + + errno = 0; + ret = unlink(test_ctx->dbfile); + if (ret == -1 && errno != ENOENT) { + fail(); + } +} + +/* + * Empty module to signal the end of the module list + */ +static const struct ldb_module_ops eol_ops = { + .name = "eol", + .search = NULL, + .add = NULL, + .modify = NULL, + .del = NULL, + .rename = NULL, + .init_context = NULL +}; + +/* + * Test set up + */ +static int setup(void **state) +{ + struct ldbtest_ctx *test_ctx = NULL; + struct ldb_module *eol = NULL; + int rc; + + test_ctx = talloc_zero(NULL, struct ldbtest_ctx); + assert_non_null(test_ctx); + + test_ctx->ev = tevent_context_init(test_ctx); + assert_non_null(test_ctx->ev); + + test_ctx->ldb = ldb_init(test_ctx, test_ctx->ev); + assert_non_null(test_ctx->ldb); + + test_ctx->domain_sid = talloc_zero(test_ctx, struct dom_sid); + assert_non_null(test_ctx->domain_sid); + assert_true(string_to_sid(test_ctx->domain_sid, DOMAIN_SID)); + ldb_set_opaque(test_ctx->ldb, "cache.domain_sid", test_ctx->domain_sid); + + test_ctx->module = ldb_module_new( + test_ctx, + test_ctx->ldb, + "unique_object_sids", + &ldb_unique_object_sids_module_ops); + assert_non_null(test_ctx->module); + eol = ldb_module_new(test_ctx, test_ctx->ldb, "eol", &eol_ops); + assert_non_null(eol); + ldb_module_set_next(test_ctx->module, eol); + + test_ctx->dbfile = talloc_strdup(test_ctx, "duptest.ldb"); + assert_non_null(test_ctx->dbfile); + + test_ctx->lockfile = talloc_asprintf(test_ctx, "%s-lock", + test_ctx->dbfile); + assert_non_null(test_ctx->lockfile); + + test_ctx->dbpath = talloc_asprintf(test_ctx, + TEST_BE"://%s", test_ctx->dbfile); + assert_non_null(test_ctx->dbpath); + + unlink_old_db(test_ctx); + + rc = ldb_connect(test_ctx->ldb, test_ctx->dbpath, 0, NULL); + assert_int_equal(rc, LDB_SUCCESS); + + rc = unique_object_sids_init(test_ctx->module); + assert_int_equal(rc, LDB_SUCCESS); + + *state = test_ctx; + + last_request = NULL; + return 0; +} + +/* + * Test clean up + */ +static int teardown(void **state) +{ + struct ldbtest_ctx *test_ctx = talloc_get_type_abort(*state, + struct ldbtest_ctx); + + unlink_old_db(test_ctx); + talloc_free(test_ctx); + return 0; +} + +/* + * Add an objectSID in string form to the supplied message + * + * + */ +static void add_sid( + struct ldb_message *msg, + const char *sid_str) +{ + struct ldb_val v; + enum ndr_err_code ndr_err; + struct dom_sid *sid = NULL; + + sid = talloc_zero(msg, struct dom_sid); + assert_non_null(sid); + assert_true(string_to_sid(sid, sid_str)); + ndr_err = ndr_push_struct_blob(&v, msg, sid, + (ndr_push_flags_fn_t)ndr_push_dom_sid); + assert_true(NDR_ERR_CODE_IS_SUCCESS(ndr_err)); + assert_int_equal(0, ldb_msg_add_value(msg, "objectSID", &v, NULL)); +} + +/* + * The object is in the current local domain so it should have + * DB_FLAG_INTERNAL_UNIQUE_VALUE set + */ +static void test_objectSID_in_domain(void **state) +{ + struct ldbtest_ctx *test_ctx = + talloc_get_type_abort(*state, struct ldbtest_ctx); + struct ldb_context *ldb = test_ctx->ldb; + struct ldb_message *msg = ldb_msg_new(test_ctx); + struct ldb_message_element *el = NULL; + struct ldb_request *request = NULL; + struct ldb_request *original_request = NULL; + int rc; + + msg->dn = ldb_dn_new(msg, ldb, "dc=test"); + add_sid(msg, LOCAL_SID); + + rc = ldb_build_add_req( + &request, + test_ctx->ldb, + test_ctx, + msg, + NULL, + NULL, + ldb_op_default_callback, + NULL); + + assert_int_equal(rc, LDB_SUCCESS); + assert_non_null(request); + original_request = request; + + rc = unique_object_sids_add(test_ctx->module, request); + assert_int_equal(rc, LDB_SUCCESS); + + /* + * Check that a copy of the request was passed to the next module + * and not the original request + */ + assert_ptr_not_equal(last_request, original_request); + + /* + * Check the flag was set on the request passed to the next + * module + */ + el = ldb_msg_find_element(last_request->op.add.message, "objectSID"); + assert_non_null(el); + assert_true(el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX); + + /* + * Check the flag was not set on the original request + */ + el = ldb_msg_find_element(request->op.add.message, "objectSID"); + assert_non_null(el); + assert_false(el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX); + +} + +/* + * The object is not in the current local domain so it should NOT have + * DB_FLAG_INTERNAL_UNIQUE_VALUE set + */ +static void test_objectSID_not_in_domain(void **state) +{ + struct ldbtest_ctx *test_ctx = + talloc_get_type_abort(*state, struct ldbtest_ctx); + struct ldb_context *ldb = test_ctx->ldb; + struct ldb_message *msg = ldb_msg_new(test_ctx); + struct ldb_message_element *el = NULL; + struct ldb_request *request = NULL; + struct ldb_request *original_request = NULL; + int rc; + + msg->dn = ldb_dn_new(msg, ldb, "dc=test"); + add_sid(msg, FOREIGN_SID); + + rc = ldb_build_add_req( + &request, + test_ctx->ldb, + test_ctx, + msg, + NULL, + NULL, + ldb_op_default_callback, + NULL); + + assert_int_equal(rc, LDB_SUCCESS); + assert_non_null(request); + original_request = request; + + rc = unique_object_sids_add(test_ctx->module, request); + assert_int_equal(rc, LDB_SUCCESS); + + /* + * Check that the original request was passed to the next module + * and not a copy + */ + assert_ptr_equal(last_request, original_request); + + /* + * Check that the flag was not set on the objectSID element + */ + el = ldb_msg_find_element(msg, "objectSID"); + assert_non_null(el); + assert_false(el->flags & LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX); +} + +/* + * No objectSID on the record so it should pass through the module untouched + * + */ +static void test_no_objectSID(void **state) +{ + struct ldbtest_ctx *test_ctx = + talloc_get_type_abort(*state, struct ldbtest_ctx); + struct ldb_context *ldb = test_ctx->ldb; + struct ldb_message *msg = ldb_msg_new(test_ctx); + struct ldb_request *request = NULL; + struct ldb_request *original_request = NULL; + int rc; + + msg->dn = ldb_dn_new(msg, ldb, "dc=test"); + assert_int_equal(LDB_SUCCESS, ldb_msg_add_string(msg, "cn", "test")); + + rc = ldb_build_add_req( + &request, + test_ctx->ldb, + test_ctx, + msg, + NULL, + NULL, + ldb_op_default_callback, + NULL); + + assert_int_equal(rc, LDB_SUCCESS); + assert_non_null(request); + original_request = request; + + rc = unique_object_sids_add(test_ctx->module, request); + assert_int_equal(rc, LDB_SUCCESS); + + /* + * Check that the original request was passed to the next module + * and not a copy + */ + assert_ptr_equal(last_request, original_request); + +} + +/* + * Attempt to modify an objectSID + * this should fail with LDB_ERR_UNWILLING_TO_PERFORM + */ +static void test_modify_of_objectSID(void **state) +{ + struct ldbtest_ctx *test_ctx = + talloc_get_type_abort(*state, struct ldbtest_ctx); + struct ldb_context *ldb = test_ctx->ldb; + struct ldb_message *msg = ldb_msg_new(test_ctx); + struct ldb_request *request = NULL; + int rc; + + msg->dn = ldb_dn_new(msg, ldb, "dc=test"); + add_sid(msg, LOCAL_SID); + + rc = ldb_build_mod_req( + &request, + test_ctx->ldb, + test_ctx, + msg, + NULL, + NULL, + ldb_op_default_callback, + NULL); + + assert_int_equal(rc, LDB_SUCCESS); + assert_non_null(request); + + rc = unique_object_sids_modify(test_ctx->module, request); + + assert_int_equal(rc, LDB_ERR_UNWILLING_TO_PERFORM); +} + + +/* + * Test the a modify with no object SID is passed through correctly + * + */ +static void test_modify_no_objectSID(void **state) +{ + struct ldbtest_ctx *test_ctx = + talloc_get_type_abort(*state, struct ldbtest_ctx); + struct ldb_context *ldb = test_ctx->ldb; + struct ldb_message *msg = ldb_msg_new(test_ctx); + struct ldb_request *request = NULL; + struct ldb_request *original_request = NULL; + int rc; + + msg->dn = ldb_dn_new(msg, ldb, "dc=test"); + assert_int_equal(LDB_SUCCESS, ldb_msg_add_string(msg, "cn", "test")); + + rc = ldb_build_mod_req( + &request, + test_ctx->ldb, + test_ctx, + msg, + NULL, + NULL, + ldb_op_default_callback, + NULL); + + assert_int_equal(rc, LDB_SUCCESS); + assert_non_null(request); + original_request = request; + + rc = unique_object_sids_modify(test_ctx->module, request); + assert_int_equal(rc, LDB_SUCCESS); + + /* + * Check that the original request was passed to the next module + * and not a copy + */ + assert_ptr_equal(last_request, original_request); + +} + +int main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown( + test_objectSID_in_domain, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_objectSID_not_in_domain, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_no_objectSID, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_modify_of_objectSID, + setup, + teardown), + cmocka_unit_test_setup_teardown( + test_modify_no_objectSID, + setup, + teardown), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/source4/dsdb/samdb/ldb_modules/unique_object_sids.c b/source4/dsdb/samdb/ldb_modules/unique_object_sids.c new file mode 100644 index 0000000..89f1546 --- /dev/null +++ b/source4/dsdb/samdb/ldb_modules/unique_object_sids.c @@ -0,0 +1,206 @@ +/* + ldb database module to enforce unique local objectSIDs + + Copyright (C) Andrew Bartlett 2017 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +/* + + Duplicate ObjectSIDs are possible on foreign security principals and + replication conflict records. However a duplicate objectSID within + the local domainSID is an error. + + As the uniqueness requirement depends on the source domain it is not possible + to enforce this with a unique index. + + This module sets the LDB_FLAG_FORCE_UNIQUE_INDEX for objectSIDs in the + local domain. +*/ + +#include "includes.h" +#include "ldb_module.h" +#include "dsdb/samdb/samdb.h" +#include "libcli/security/dom_sid.h" +#include "dsdb/samdb/ldb_modules/util.h" + +struct private_data { + const struct dom_sid *domain_sid; +}; + + +/* + * Does the add request contain a local objectSID + */ +static bool add_request_contains_local_objectSID( + struct ldb_module *module, + struct ldb_request *req) +{ + const struct ldb_message *msg = req->op.add.message; + struct dom_sid *objectSID = NULL; + + struct private_data *data = + talloc_get_type( + ldb_module_get_private(module), + struct private_data); + + TALLOC_CTX *frame = talloc_stackframe(); + + objectSID = samdb_result_dom_sid(frame, msg, "objectSID"); + if (objectSID == NULL) { + TALLOC_FREE(frame); + return false; + } + + /* + * data->domain_sid can be NULL but dom_sid_in_domain handles this + * case correctly. See unique_object_sids_init for more details. + */ + if (!dom_sid_in_domain(data->domain_sid, objectSID)) { + TALLOC_FREE(frame); + return false; + } + TALLOC_FREE(frame); + return true; +} + +/* add */ +static int unique_object_sids_add( + struct ldb_module *module, + struct ldb_request *req) +{ + const struct ldb_message *msg = req->op.add.message; + struct ldb_message *new_msg = NULL; + struct ldb_request *new_req = NULL; + struct ldb_context *ldb = NULL; + struct ldb_message_element *el = NULL; + int rc; + + if (!add_request_contains_local_objectSID(module, req)) { + /* + * Request does not contain a local objectSID so chain the + * next module + */ + return ldb_next_request(module, req); + } + + /* + * The add request contains an objectSID for the local domain + */ + new_msg = ldb_msg_copy_shallow(req, msg); + if (!new_msg) { + return ldb_module_oom(module); + } + + ldb = ldb_module_get_ctx(module); + rc = ldb_build_add_req( + &new_req, + ldb, + req, + new_msg, + req->controls, + req, + dsdb_next_callback, + req); + if (rc != LDB_SUCCESS) { + return rc; + } + + el = ldb_msg_find_element(new_msg, "objectSID"); + if (el == NULL) { + ldb_asprintf_errstring( + ldb, + "Unable to locate objectSID in copied request\n"); + return LDB_ERR_OPERATIONS_ERROR; + } + el->flags |= LDB_FLAG_INTERNAL_FORCE_UNIQUE_INDEX; + + return ldb_next_request(module, new_req); +} + +/* modify */ +static int unique_object_sids_modify( + struct ldb_module *module, + struct ldb_request *req) +{ + + const struct ldb_message *msg = req->op.mod.message; + struct ldb_message_element *el = NULL; + + el = ldb_msg_find_element(msg, "objectSID"); + if (el != NULL) { + struct ldb_context *ldb = ldb_module_get_ctx(module); + ldb_asprintf_errstring( + ldb, + "Modify of %s rejected, " + "as it is modifying an objectSID\n", + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_UNWILLING_TO_PERFORM; + } + return ldb_next_request(module, req); +} + +/* init */ +static int unique_object_sids_init( + struct ldb_module *module) +{ + struct ldb_context *ldb = ldb_module_get_ctx(module); + struct private_data *data = NULL; + int ret; + + ret = ldb_next_init(module); + + if (ret != LDB_SUCCESS) { + return ret; + } + + data = talloc_zero(module, struct private_data); + if (!data) { + return ldb_module_oom(module); + } + + data->domain_sid = samdb_domain_sid(ldb); + if (data->domain_sid == NULL) { + /* + * Unable to determine the domainSID, this normally occurs + * when provisioning. As there is no easy way to detect + * that we are provisioning. We currently just log this as a + * warning. + */ + ldb_debug( + ldb, + LDB_DEBUG_WARNING, + "Unable to determine the DomainSID, " + "can not enforce uniqueness constraint on local " + "domainSIDs\n"); + } + + ldb_module_set_private(module, data); + + return LDB_SUCCESS; +} + +static const struct ldb_module_ops ldb_unique_object_sids_module_ops = { + .name = "unique_object_sids", + .init_context = unique_object_sids_init, + .add = unique_object_sids_add, + .modify = unique_object_sids_modify, +}; + +int ldb_unique_object_sids_init(const char *version) +{ + LDB_MODULE_CHECK_VERSION(version); + return ldb_register_module(&ldb_unique_object_sids_module_ops); +} diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build b/source4/dsdb/samdb/ldb_modules/wscript_build index 0df5fe0..b0afcf9 100644 --- a/source4/dsdb/samdb/ldb_modules/wscript_build +++ b/source4/dsdb/samdb/ldb_modules/wscript_build @@ -18,5 +18,16 @@ bld.SAMBA_SUBSYSTEM('DSDB_MODULE_HELPER_RIDALLOC', deps='MESSAGING', ) +# Build the cmocka unit tests +bld.SAMBA_BINARY('test_unique_object_sids', + source='tests/test_unique_object_sids.c', + deps=''' + talloc + samdb + cmocka + DSDB_MODULE_HELPERS + ''', + install=False) + if bld.AD_DC_BUILD_IS_ENABLED(): bld.PROCESS_SEPARATE_RULE("server") diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build_server b/source4/dsdb/samdb/ldb_modules/wscript_build_server index 41b3fe7..4aac7f2 100644 --- a/source4/dsdb/samdb/ldb_modules/wscript_build_server +++ b/source4/dsdb/samdb/ldb_modules/wscript_build_server @@ -400,3 +400,12 @@ bld.SAMBA_MODULE('ldb_vlv', deps='samdb-common', subsystem='ldb' ) + +bld.SAMBA_MODULE('ldb_unique_object_sids', + 'unique_object_sids.c', + init_function='ldb_unique_object_sids_init', + module_init_name='ldb_init_module', + internal_module=False, + deps='samdb-common DSDB_MODULE_HELPERS', + subsystem='ldb' + ) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 8d3d526..02caf69 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -989,3 +989,8 @@ for env in ['vampire_dc', 'promoted_dc', 'rodc']: # check the databases are all OK. PLEASE LEAVE THIS AS THE LAST TEST for env in ["ad_dc_ntvfs", "ad_dc", "fl2000dc", "fl2003dc", "fl2008r2dc", 'vampire_dc', 'promoted_dc']: plantestsuite("samba4.blackbox.dbcheck(%s)" % env, env + ":local" , ["PYTHON=%s" % python, os.path.join(bbdir, "dbcheck.sh"), '$PREFIX/provision', configuration]) + +# cmocka tests not requiring a specific encironment +# +plantestsuite("samba4.dsdb.samdb.ldb_modules.unique_object_sids" , "none", + [os.path.join(bindir(), "test_unique_object_sids")]) -- 2.7.4 From 636bca4320870624e45f3574169ea2e2ceeea0ca Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Mon, 27 Nov 2017 11:11:19 +1300 Subject: [PATCH 4/4] source4 dsdb: Allow duplicate non local objectSIDs Remove the unique constraint on the objectSID index, and enable the unique_object_sids module. This allows duplicate objectSIDs on foreign security principals, and disallows duplicates for local objectSIDs BUG: https://bugzilla.samba.org/show_bug.cgi?id=13004 Signed-off-by: Gary Lockyer --- selftest/knownfail.d/dsdb | 1 - source4/dsdb/samdb/ldb_modules/samba_dsdb.c | 1 + source4/dsdb/schema/schema_init.c | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/dsdb diff --git a/selftest/knownfail.d/dsdb b/selftest/knownfail.d/dsdb deleted file mode 100644 index 276b72d..0000000 --- a/selftest/knownfail.d/dsdb +++ /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/samba_dsdb.c b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c index 9098b52..87d65bd 100644 --- a/source4/dsdb/samdb/ldb_modules/samba_dsdb.c +++ b/source4/dsdb/samdb/ldb_modules/samba_dsdb.c @@ -295,6 +295,7 @@ static int samba_dsdb_init(struct ldb_module *module) "subtree_delete", "repl_meta_data", "operational", + "unique_object_sids", "subtree_rename", "linked_attributes", NULL}; diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c index c76b57c..dbd5045 100644 --- a/source4/dsdb/schema/schema_init.c +++ b/source4/dsdb/schema/schema_init.c @@ -461,7 +461,7 @@ WERROR dsdb_read_prefixes_from_ldb(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, */ static bool dsdb_schema_unique_attribute(const char *attr) { - const char *attrs[] = { "objectGUID", "objectSid" , NULL }; + const char *attrs[] = { "objectGUID", NULL }; unsigned int i; for (i=0;attrs[i];i++) { if (ldb_attr_cmp(attr, attrs[i]) == 0) { -- 2.7.4