From 324b2a7a30d98ec05b76708ba715fbd3d6494424 Mon Sep 17 00:00:00 2001 From: Chris Lamb Date: Sat, 18 Feb 2017 08:59:48 +1300 Subject: [PATCH 01/26] Correct "ommited" typos. Signed-off-by: Chris Lamb Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett --- docs-xml/Samba3-Developers-Guide/internals.xml | 2 +- source3/libads/ndr.c | 2 +- source4/torture/local/fsrvp_state.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs-xml/Samba3-Developers-Guide/internals.xml b/docs-xml/Samba3-Developers-Guide/internals.xml index be27121..e28a2d5 100644 --- a/docs-xml/Samba3-Developers-Guide/internals.xml +++ b/docs-xml/Samba3-Developers-Guide/internals.xml @@ -308,7 +308,7 @@ SSVAL(). I do not know where these numbers are described. An ASCIIZ string describing the parameters to the API function as defined in the LAN Manager documentation. The first parameter, which is the server -name, is ommited. This string is based uppon the API function as described +name, is omitted. This string is based uppon the API function as described in the manual, not the data which is actually passed. diff --git a/source3/libads/ndr.c b/source3/libads/ndr.c index 957c0fa..6cecbb0 100644 --- a/source3/libads/ndr.c +++ b/source3/libads/ndr.c @@ -58,7 +58,7 @@ void ndr_print_ads_struct(struct ndr_print *ndr, const char *name, const struct #ifdef DEBUG_PASSWORD ndr_print_string(ndr, "password", r->auth.password); #else - ndr_print_string(ndr, "password", "(PASSWORD ommited)"); + ndr_print_string(ndr, "password", "(PASSWORD omitted)"); #endif ndr_print_string(ndr, "user_name", r->auth.user_name); ndr_print_string(ndr, "kdc_server", r->auth.kdc_server); diff --git a/source4/torture/local/fsrvp_state.c b/source4/torture/local/fsrvp_state.c index e4be500..9b63ec1 100644 --- a/source4/torture/local/fsrvp_state.c +++ b/source4/torture/local/fsrvp_state.c @@ -97,7 +97,7 @@ static bool test_fsrvp_state_sc(struct torture_context *tctx, sc->id = GUID_random(); sc->id_str = GUID_string(sc, &sc->id); sc->volume_name = talloc_strdup(sc, "/this/is/a/path"); - /* keep snap path NULL, i.e. not yet commited */ + /* keep snap path NULL, i.e. not yet committed */ sc->create_ts = time(NULL); *sc_out = sc; -- 1.9.1 From b7e911a08a07442b1562ed9ef9ecac07e31949bb Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 17:17:27 +1300 Subject: [PATCH 02/26] python/dsdb_dn: Add a generic get_bytes method on DNs Pair-programmed-with: Bob Campbell Signed-off-by: Garming Sam --- python/samba/common.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/samba/common.py b/python/samba/common.py index c2a3584..20f170c 100644 --- a/python/samba/common.py +++ b/python/samba/common.py @@ -19,6 +19,7 @@ import ldb import dsdb +import binascii def confirm(msg, forced=False, allow_all=False): @@ -97,3 +98,7 @@ class dsdb_Dn(object): if self.prefix == '': return None return int(self.binary, 16) + + def get_bytes(self): + '''return binary as a byte string''' + return binascii.unhexlify(self.binary) -- 1.9.1 From d2a40792cc63b1609fc19be15e50ad6d356abd94 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 17:13:40 +1300 Subject: [PATCH 03/26] drsbase: use credentials if supplied Pair-programmed-with: Bob Campbell Signed-off-by: Garming Sam --- source4/torture/drs/python/drs_base.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py index e6f6e48..f07592d 100644 --- a/source4/torture/drs/python/drs_base.py +++ b/source4/torture/drs/python/drs_base.py @@ -352,10 +352,12 @@ class DrsBaseTestCase(SambaToolCmdTest): return req10 - def _ds_bind(self, server_name): + def _ds_bind(self, server_name, creds=None): binding_str = "ncacn_ip_tcp:%s[seal]" % server_name - drs = drsuapi.drsuapi(binding_str, self.get_loadparm(), self.get_credentials()) + if creds is None: + creds = self.get_credentials() + drs = drsuapi.drsuapi(binding_str, self.get_loadparm(), creds) (drs_handle, supported_extensions) = drs_DsBind(drs) return (drs, drs_handle) -- 1.9.1 From decce45689b80ddacca1bb897833f75ce8a2a8af Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 27 Feb 2017 14:40:40 +1300 Subject: [PATCH 04/26] getncchanges: Return correct denied REPL_SECRET error code Signed-off-by: Garming Sam --- libcli/util/werror.h | 2 ++ source4/rpc_server/drsuapi/getncchanges.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libcli/util/werror.h b/libcli/util/werror.h index 7adda29..c25a4ab 100644 --- a/libcli/util/werror.h +++ b/libcli/util/werror.h @@ -99,6 +99,8 @@ typedef uint32_t WERROR; #define WERR_ALERTED W_ERROR(0x000002E3) #define WERR_INVALID_PRIMARY_GROUP W_ERROR(0x0000051C) +#define WERR_DS_DRA_SECRETS_DENIED W_ERROR(0x000021B6) + #define WERR_DNS_ERROR_KEYMASTER_REQUIRED W_ERROR(0x0000238D) #define WERR_DNS_ERROR_NOT_ALLOWED_ON_SIGNED_ZONE W_ERROR(0x0000238E) #define WERR_DNS_ERROR_INVALID_NSEC3_PARAMETERS W_ERROR(0x0000238F) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 09b6d89..6fbebd5 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1117,7 +1117,7 @@ denied: DEBUG(2,(__location__ ": Denied single object with secret replication for %s by RODC %s\n", ldb_dn_get_linearized(obj_dn), ldb_dn_get_linearized(rodc_res->msgs[0]->dn))); ctr6->extended_ret = DRSUAPI_EXOP_ERR_NONE; - return WERR_DS_DRA_ACCESS_DENIED; + return WERR_DS_DRA_SECRETS_DENIED; allowed: DEBUG(2,(__location__ ": Allowed single object with secret replication for %s by %s %s\n", -- 1.9.1 From 9d16eba9ef6fa449c2d47fadf379b8967384dd11 Mon Sep 17 00:00:00 2001 From: Bob Campbell Date: Mon, 13 Feb 2017 15:46:37 +1300 Subject: [PATCH 05/26] python/tests: Add repl_rodc test Currently, this tests the msDS-RevealedUsers feature, which we don't support at the moment. Signed-off-by: Bob Campbell Pair-programmed-with: Garming Sam --- selftest/knownfail | 2 + source4/selftest/tests.py | 7 + source4/torture/drs/python/repl_rodc.py | 416 ++++++++++++++++++++++++++++++++ 3 files changed, 425 insertions(+) create mode 100644 source4/torture/drs/python/repl_rodc.py diff --git a/selftest/knownfail b/selftest/knownfail index 7c5417b..fdb76ff 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,3 +315,5 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* +# We don't yet support msDS-RevealedUsers +^samba4.drs.repl_rodc.python.*repl_rodc.* diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index ac601f5..6e648db 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -714,6 +714,13 @@ for env in ['vampire_dc', 'promoted_dc', 'vampire_2000_dc']: environ={'DC1': "$DC_SERVER", 'DC2': '$%s_SERVER' % env.upper()}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) +for env in ['ad_dc_ntvfs']: + planoldpythontestsuite(env, "repl_rodc", + extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], + name="samba4.drs.repl_rodc.python(%s)" % env, + environ={'DC1': "$DC_SERVER", 'DC2': '$DC_SERVER'}, + extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) + planoldpythontestsuite("chgdcpass:local", "samba.tests.blackbox.samba_dnsupdate", environ={'DNS_SERVER_IP': '$SERVER_IP'}) diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py new file mode 100644 index 0000000..c45afb6 --- /dev/null +++ b/source4/torture/drs/python/repl_rodc.py @@ -0,0 +1,416 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# Test replication scenarios involving an RODC +# +# Copyright (C) Catalyst.Net Ltd. 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 . +# + +# +# Usage: +# export DC1=dc1_dns_name +# export DC2=dc1_dns_name [this is unused for the test, but it'll still try to connect] +# export SUBUNITRUN=$samba4srcdir/scripting/bin/subunitrun +# PYTHONPATH="$PYTHONPATH:$samba4srcdir/torture/drs/python" $SUBUNITRUN repl_rodc -U"$DOMAIN/$DC_USERNAME"%"$DC_PASSWORD" +# + +import drs_base +import samba.tests +import ldb +from ldb import SCOPE_BASE + +from samba import WERRORError +from samba.join import dc_join +from samba.dcerpc import drsuapi, misc, drsblobs, security +from samba.drs_utils import drs_DsBind, drs_Replicate +from samba.ndr import ndr_unpack, ndr_pack +from samba.common import dsdb_Dn +from samba.credentials import Credentials + +import random +import time + +def drs_get_rodc_partial_attribute_set(samdb, samdb1, exceptions=[]): + '''get a list of attributes for RODC replication''' + partial_attribute_set = drsuapi.DsPartialAttributeSet() + partial_attribute_set.version = 1 + + attids = [] + + # the exact list of attids we send is quite critical. Note that + # we do ask for the secret attributes, but set SPECIAL_SECRET_PROCESSING + # to zero them out + schema_dn = samdb.get_schema_basedn() + res = samdb.search(base=schema_dn, scope=ldb.SCOPE_SUBTREE, + expression="objectClass=attributeSchema", + attrs=["lDAPDisplayName", "systemFlags", + "searchFlags"]) + + for r in res: + ldap_display_name = r["lDAPDisplayName"][0] + if "systemFlags" in r: + system_flags = r["systemFlags"][0] + if (int(system_flags) & (samba.dsdb.DS_FLAG_ATTR_NOT_REPLICATED | + samba.dsdb.DS_FLAG_ATTR_IS_CONSTRUCTED)): + continue + if "searchFlags" in r: + search_flags = r["searchFlags"][0] + if (int(search_flags) & samba.dsdb.SEARCH_FLAG_RODC_ATTRIBUTE): + continue + try: + attid = samdb1.get_attid_from_lDAPDisplayName(ldap_display_name) + if not attid in exceptions: + attids.append(int(attid)) + except: + pass + + # the attids do need to be sorted, or windows doesn't return + # all the attributes we need + attids.sort() + partial_attribute_set.attids = attids + partial_attribute_set.num_attids = len(attids) + return partial_attribute_set + +class DrsRodcTestCase(drs_base.DrsBaseTestCase): + """Intended as a semi-black box test case for replication involving + an RODC.""" + + def setUp(self): + super(DrsRodcTestCase, self).setUp() + self.base_dn = self.ldb_dc1.get_default_basedn() + + rand = random.randint(1, 10000000) + + self.ou = "OU=test_drs_rodc%s,%s" % (rand, self.base_dn) + self.ldb_dc1.add({ + "dn": self.ou, + "objectclass": "organizationalUnit" + }) + self.allowed_group = "CN=Allowed RODC Password Replication Group,CN=Users,%s" % self.base_dn + + self.site = self.ldb_dc1.server_site_name() + self.rodc_name = "TESTRODCDRS%s" % rand + self.rodc_pass = "password12#" + self.computer_dn = "CN=%s,OU=Domain Controllers,%s" % (self.rodc_name, self.base_dn) + + + self.rodc_ctx = dc_join(server=self.ldb_dc1.host_dns_name(), creds=self.get_credentials(), lp=self.get_loadparm(), + site=self.site, netbios_name=self.rodc_name, + targetdir=None, domain=None, machinepass=self.rodc_pass) + self._create_rodc(self.rodc_ctx) + self.rodc_ctx.create_tmp_samdb() + self.tmp_samdb = self.rodc_ctx.tmp_samdb + + rodc_creds = Credentials() + rodc_creds.guess(self.rodc_ctx.lp) + rodc_creds.set_username(self.rodc_name+'$') + rodc_creds.set_password(self.rodc_pass) + + (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1) + (self.rodc_drs, self.rodc_drs_handle) = self._ds_bind(self.dnsname_dc1, rodc_creds) + + def tearDown(self): + self.rodc_ctx.cleanup_old_join() + super(DrsRodcTestCase, self).tearDown() + + def test_admin_repl_secrets(self): + """ + When a secret attribute is set to be replicated to an RODC with the + admin credentials, it should always replicate regardless of whether + or not it's in the Allowed RODC Password Replication Group. + """ + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + user_name = "test_rodcA_%s" % rand + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, 'penguin12#', False, user_name) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # Check that the user has been added to msDSRevealedUsers + self._assert_in_revealed_users(user_dn, expected_user_attributes) + + def test_rodc_repl_secrets(self): + """ + When a secret attribute is set to be replicated to an RODC with + the RODC account credentials, it should not replicate if it's in + the Allowed RODC Password Replication Group. Once it is added to + the group, it should replicate. + """ + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + user_name = "test_rodcB_%s" % rand + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, 'penguin12#', False, user_name) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + + try: + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.") + except WERRORError as (enum, estr): + self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + + # Retry with Administrator credentials, ignores password replication groups + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # Check that the user has been added to msDSRevealedUsers + self._assert_in_revealed_users(user_dn, expected_user_attributes) + + def test_msDSRevealedUsers(self): + """ + When a secret attribute is to be replicated to an RODC, the contents + of the attribute should be added to the msDSRevealedUsers attribute + of the computer object corresponding to the RODC. + """ + + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + # Add a user on DC1, add it to allowed password replication + # group, and replicate to RODC with EXOP_REPL_SECRETS + user_name = "test_rodcC_%s" % rand + password = "password12#" + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password, False, user_name) + + self.ldb_dc1.add_remove_group_members("Allowed RODC Password Replication Group", + [user_name], + add_members_operation=True) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # Check that the user has been added to msDSRevealedUsers + (packed_attrs_1, unpacked_attrs_1) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + + # Change the user's password on DC1 + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password+"1", False, user_name) + + (packed_attrs_2, unpacked_attrs_2) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + self._assert_attrlist_equals(unpacked_attrs_1, unpacked_attrs_2) + + # Replicate to RODC again with EXOP_REPL_SECRETS + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # This is important for Windows, because the entry won't have been + # updated in time if we don't have it. Even with this sleep, it only + # passes some of the time... + time.sleep(5) + + # Check that the entry in msDSRevealedUsers has been updated + (packed_attrs_3, unpacked_attrs_3) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + self._assert_attrlist_changed(unpacked_attrs_2, unpacked_attrs_3, expected_user_attributes) + + # We should be able to delete the user + self.ldb_dc1.deleteuser(user_name) + + res = self.ldb_dc1.search(scope=ldb.SCOPE_BASE, base=self.computer_dn, + attrs=["msDS-RevealedUsers"]) + self.assertFalse("msDS-RevealedUsers" in res[0]) + + def test_msDSRevealedUsers_pas(self): + """ + If we provide a Partial Attribute Set when replicating to an RODC, + we should ignore it and replicate all of the secret attributes anyway + msDSRevealedUsers attribute. + """ + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + pas_exceptions = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + # Add a user on DC1, add it to allowed password replication + # group, and replicate to RODC with EXOP_REPL_SECRETS + user_name = "test_rodcD_%s" % rand + password = "password12#" + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password, False, user_name) + + self.ldb_dc1.add_remove_group_members("Allowed RODC Password Replication Group", + [user_name], + add_members_operation=True) + + pas = drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb, exceptions=pas_exceptions) + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=pas, + max_objects=133, + replica_flags=0) + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # Make sure that we still replicate the secrets + for attribute in ctr.first_object.object.attribute_ctr.attributes: + if attribute.attid in pas_exceptions: + pas_exceptions.remove(attribute.attid) + for attribute in pas_exceptions: + self.fail("%d was not replicated even though the partial attribute set should be ignored." + % attribute) + + # Check that the user has been added to msDSRevealedUsers + (packed_attrs_1, unpacked_attrs_1) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + + def _assert_in_revealed_users(self, user_dn, attrlist): + res = self.ldb_dc1.search(scope=ldb.SCOPE_BASE, base=self.computer_dn, + attrs=["msDS-RevealedUsers"]) + revealed_users = res[0]["msDS-RevealedUsers"] + actual_attrids = [] + packed_attrs = [] + unpacked_attrs = [] + for attribute in revealed_users: + dsdb_dn = dsdb_Dn(self.ldb_dc1, attribute) + metadata = ndr_unpack(drsblobs.replPropertyMetaData1, dsdb_dn.get_bytes()) + if user_dn in attribute: + unpacked_attrs.append(metadata) + packed_attrs.append(dsdb_dn.get_bytes()) + actual_attrids.append(metadata.attid) + + self.assertEquals(sorted(actual_attrids), sorted(attrlist)) + + return (packed_attrs, unpacked_attrs) + + def _assert_attrlist_equals(self, list_1, list_2): + return self._assert_attrlist_changed(list_1, list_2, [], num_changes=0, expected_new_usn=False) + + def _assert_attrlist_changed(self, list_1, list_2, changed_attributes, num_changes=1, expected_new_usn=True): + for i in range(len(list_2)): + self.assertEquals(list_1[i].attid, list_2[i].attid) + self.assertEquals(list_1[i].originating_invocation_id, list_2[i].originating_invocation_id) + self.assertEquals(list_1[i].version + num_changes, list_2[i].version) + + if expected_new_usn: + self.assertTrue(list_1[i].originating_usn < list_2[i].originating_usn) + self.assertTrue(list_1[i].local_usn < list_2[i].local_usn) + else: + self.assertEquals(list_1[i].originating_usn, list_2[i].originating_usn) + self.assertEquals(list_1[i].local_usn, list_2[i].local_usn) + + if list_1[i].attid in changed_attributes: + # We do the changes too quickly, so unless we put sleeps + # inbetween calls, these remain the same. Checking the USNs + # is enough. + pass + #self.assertTrue(list_1[i].originating_change_time < list_2[i].originating_change_time) + else: + self.assertEquals(list_1[i].originating_change_time, list_2[i].originating_change_time) + + + def _create_rodc(self, ctx): + ctx.nc_list = [ ctx.base_dn, ctx.config_dn, ctx.schema_dn ] + ctx.full_nc_list = [ ctx.base_dn, ctx.config_dn, ctx.schema_dn ] + ctx.krbtgt_dn = "CN=krbtgt_%s,CN=Users,%s" % (ctx.myname, ctx.base_dn) + + ctx.never_reveal_sid = [ "" % (ctx.domsid, security.DOMAIN_RID_RODC_DENY), + "" % security.SID_BUILTIN_ADMINISTRATORS, + "" % security.SID_BUILTIN_SERVER_OPERATORS, + "" % security.SID_BUILTIN_BACKUP_OPERATORS, + "" % security.SID_BUILTIN_ACCOUNT_OPERATORS ] + ctx.reveal_sid = "" % (ctx.domsid, security.DOMAIN_RID_RODC_ALLOW) + + mysid = ctx.get_mysid() + admin_dn = "" % mysid + ctx.managedby = admin_dn + + ctx.userAccountControl = (samba.dsdb.UF_WORKSTATION_TRUST_ACCOUNT | + samba.dsdb.UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION | + samba.dsdb.UF_PARTIAL_SECRETS_ACCOUNT) + + ctx.connection_dn = "CN=RODC Connection (FRS),%s" % ctx.ntds_dn + ctx.secure_channel_type = misc.SEC_CHAN_RODC + ctx.RODC = True + ctx.replica_flags = (drsuapi.DRSUAPI_DRS_INIT_SYNC | + drsuapi.DRSUAPI_DRS_PER_SYNC | + drsuapi.DRSUAPI_DRS_GET_ANC | + drsuapi.DRSUAPI_DRS_NEVER_SYNCED | + drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING) + + ctx.join_add_objects() -- 1.9.1 From 13b0ebe86d5d28104051206f22dbd4efd01da265 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 13:33:04 +1300 Subject: [PATCH 06/26] tests/repl_rodc: Duplicate msDS-RevealedUsers test for RODC machine acct Signed-off-by: Garming Sam --- source4/torture/drs/python/repl_rodc.py | 80 ++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py index c45afb6..f5d08d4 100644 --- a/source4/torture/drs/python/repl_rodc.py +++ b/source4/torture/drs/python/repl_rodc.py @@ -207,7 +207,7 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): # Check that the user has been added to msDSRevealedUsers self._assert_in_revealed_users(user_dn, expected_user_attributes) - def test_msDSRevealedUsers(self): + def test_msDSRevealedUsers_admin(self): """ When a secret attribute is to be replicated to an RODC, the contents of the attribute should be added to the msDSRevealedUsers attribute @@ -283,6 +283,82 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): attrs=["msDS-RevealedUsers"]) self.assertFalse("msDS-RevealedUsers" in res[0]) + def test_msDSRevealedUsers(self): + """ + When a secret attribute is to be replicated to an RODC, the contents + of the attribute should be added to the msDSRevealedUsers attribute + of the computer object corresponding to the RODC. + """ + + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + # Add a user on DC1, add it to allowed password replication + # group, and replicate to RODC with EXOP_REPL_SECRETS + user_name = "test_rodcD_%s" % rand + password = "password12#" + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password, False, user_name) + + self.ldb_dc1.add_remove_group_members("Allowed RODC Password Replication Group", + [user_name], + add_members_operation=True) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + (level, ctr) = self.drs.DsGetNCChanges(self.drs_handle, 10, req10) + + # Check that the user has been added to msDSRevealedUsers + (packed_attrs_1, unpacked_attrs_1) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + + # Change the user's password on DC1 + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password+"1", False, user_name) + + (packed_attrs_2, unpacked_attrs_2) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + self._assert_attrlist_equals(unpacked_attrs_1, unpacked_attrs_2) + + # Replicate to RODC again with EXOP_REPL_SECRETS + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + + # This is important for Windows, because the entry won't have been + # updated in time if we don't have it. Even with this sleep, it only + # passes some of the time... + time.sleep(5) + + # Check that the entry in msDSRevealedUsers has been updated + (packed_attrs_3, unpacked_attrs_3) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + self._assert_attrlist_changed(unpacked_attrs_2, unpacked_attrs_3, expected_user_attributes) + + # We should be able to delete the user + self.ldb_dc1.deleteuser(user_name) + + res = self.ldb_dc1.search(scope=ldb.SCOPE_BASE, base=self.computer_dn, + attrs=["msDS-RevealedUsers"]) + self.assertFalse("msDS-RevealedUsers" in res[0]) + def test_msDSRevealedUsers_pas(self): """ If we provide a Partial Attribute Set when replicating to an RODC, @@ -302,7 +378,7 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): # Add a user on DC1, add it to allowed password replication # group, and replicate to RODC with EXOP_REPL_SECRETS - user_name = "test_rodcD_%s" % rand + user_name = "test_rodcE_%s" % rand password = "password12#" user_dn = "CN=%s,%s" % (user_name, self.ou) self.ldb_dc1.add({ -- 1.9.1 From a3a4555c585a3a2c02a094b86cbf9c563886ff1b Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 17:12:21 +1300 Subject: [PATCH 07/26] ldb: Do not care about duplicates if single value check disabled This behaviour of ignoring duplicates with the flag LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK is also used in the replace case here. When we add a forward DN+Binary link with a duplicate DN, this prevents us from not being able to add the backlink because it appears to be a duplicate here. Signed-off-by: Garming Sam Pair-programmed-with: Bob Campbell --- lib/ldb/ldb_tdb/ldb_tdb.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c index 707d227..265c93b 100644 --- a/lib/ldb/ldb_tdb/ldb_tdb.c +++ b/lib/ldb/ldb_tdb/ldb_tdb.c @@ -794,31 +794,33 @@ int ltdb_modify_internal(struct ldb_module *module, /* Check that values don't exist yet on multi- valued attributes or aren't provided twice */ /* TODO: This is O(n^2) - replace with more efficient check */ - for (j = 0; j < el->num_values; j++) { - if (ldb_msg_find_val(el2, &el->values[j]) != NULL) { - if (control_permissive) { - /* remove this one as if it was never added */ - el->num_values--; - for (k = j; k < el->num_values; k++) { - el->values[k] = el->values[k + 1]; + if (!(el->flags & LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK)) { + for (j = 0; j < el->num_values; j++) { + if (ldb_msg_find_val(el2, &el->values[j]) != NULL) { + if (control_permissive) { + /* remove this one as if it was never added */ + el->num_values--; + for (k = j; k < el->num_values; k++) { + el->values[k] = el->values[k + 1]; + } + j--; /* rewind */ + + continue; } - j--; /* rewind */ - continue; + ldb_asprintf_errstring(ldb, + "attribute '%s': value #%u on '%s' already exists", + el->name, j, ldb_dn_get_linearized(msg2->dn)); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; + } + if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) { + ldb_asprintf_errstring(ldb, + "attribute '%s': value #%u on '%s' provided more than once", + el->name, j, ldb_dn_get_linearized(msg2->dn)); + ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; + goto done; } - - ldb_asprintf_errstring(ldb, - "attribute '%s': value #%u on '%s' already exists", - el->name, j, ldb_dn_get_linearized(msg2->dn)); - ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto done; - } - if (ldb_msg_find_val(el, &el->values[j]) != &el->values[j]) { - ldb_asprintf_errstring(ldb, - "attribute '%s': value #%u on '%s' provided more than once", - el->name, j, ldb_dn_get_linearized(msg2->dn)); - ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS; - goto done; } } -- 1.9.1 From 3580af6468a227de016b9c75e9d5bd966ad62690 Mon Sep 17 00:00:00 2001 From: Bob Campbell Date: Thu, 16 Feb 2017 10:03:29 +1300 Subject: [PATCH 08/26] drsblobs: Add decode for replPropertyMetaData1 Signed-off-by: Bob Campbell Pair-programmed-with: Garming Sam --- librpc/idl/drsblobs.idl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/librpc/idl/drsblobs.idl b/librpc/idl/drsblobs.idl index 09168a8..44f5fda 100644 --- a/librpc/idl/drsblobs.idl +++ b/librpc/idl/drsblobs.idl @@ -17,6 +17,9 @@ interface drsblobs { * replPropertyMetaData * w2k uses version 1 * w2k3 uses version 1 + * + * Also equivalent to + * MS-DRSR 4.1.10.2.22 PROPERTY_META_DATA */ typedef [public] struct { drsuapi_DsAttributeId attid; @@ -27,6 +30,10 @@ interface drsblobs { hyper local_usn; } replPropertyMetaData1; + void decode_replPropertyMetaData1( + [in] replPropertyMetaData1 blob + ); + typedef struct { uint32 count; [value(0)] uint32 reserved; -- 1.9.1 From 7601dca820e8c14b3b150af6db7506d80d941957 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 17:12:27 +1300 Subject: [PATCH 09/26] getncchanges: Let security of RWDC+ manually replicate secrets to RODCs Signed-off-by: Garming Sam Pair-programmed-with: Bob Campbell --- source4/rpc_server/drsuapi/getncchanges.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 6fbebd5..efad0c9 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1962,14 +1962,17 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_ if (!W_ERROR_IS_OK(werr)) { return werr; } - if (is_secret_request && req10->extended_op != DRSUAPI_EXOP_REPL_SECRET) { + if (is_secret_request) { werr = drs_security_access_check_nc_root(b_state->sam_ctx, mem_ctx, dce_call->conn->auth_state.session_info->security_token, req10->naming_context, GUID_DRS_GET_ALL_CHANGES); if (!W_ERROR_IS_OK(werr)) { - return werr; + /* Only bail if this is not a EXOP_REPL_SECRET */ + if (req10->extended_op != DRSUAPI_EXOP_REPL_SECRET) { + return werr; + } } else { has_get_all_changes = true; } -- 1.9.1 From ca20a1613ef244b4abd52a1f6cbf63eae803d123 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 17:12:32 +1300 Subject: [PATCH 10/26] repl_meta_data: Include extra data on DN in search if it exists This is important for multi-valued DN+Binary (or DN+String) attributes, as otherwise they will be considered duplicates. Signed-off-by: Garming Sam Pair-programmed-with: Bob Campbell --- source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 33 +++++++++++++++++++------ 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 4da182f..8715d75 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -1901,6 +1901,8 @@ struct compare_ctx { const char *ldap_oid; int err; const struct GUID *invocation_id; + DATA_BLOB extra_part; + bool compare_extra_part; }; /* When a parsed_dn comes from the database, sometimes it is not really parsed. */ @@ -1955,6 +1957,7 @@ static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2) static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, struct parsed_dn *p) { + int cmp = 0; /* * This works like a standard compare function in its return values, * but has an extra trick to deal with errors: zero is returned and @@ -1974,7 +1977,12 @@ static int la_guid_compare_with_trusted_dn(struct compare_ctx *ctx, return 0; } } - return ndr_guid_compare(ctx->guid, &p->guid); + cmp = ndr_guid_compare(ctx->guid, &p->guid); + if (cmp == 0 && ctx->compare_extra_part) { + return data_blob_cmp(&ctx->extra_part, &p->dsdb_dn->extra_part); + } + + return cmp; } @@ -1983,9 +1991,11 @@ static int parsed_dn_find(struct ldb_context *ldb, struct parsed_dn *pdn, unsigned int count, struct GUID *guid, struct ldb_dn *target_dn, + DATA_BLOB extra_part, struct parsed_dn **exact, struct parsed_dn **next, - const char *ldap_oid) + const char *ldap_oid, + bool compare_extra_part) { unsigned int i; struct compare_ctx ctx; @@ -2061,6 +2071,8 @@ static int parsed_dn_find(struct ldb_context *ldb, struct parsed_dn *pdn, ctx.ldb = ldb; ctx.mem_ctx = pdn; ctx.ldap_oid = ldap_oid; + ctx.extra_part = extra_part; + ctx.compare_extra_part = compare_extra_part; ctx.err = 0; BINARY_ARRAY_SEARCH_GTE(pdn, count, &ctx, la_guid_compare_with_trusted_dn, @@ -2567,8 +2579,10 @@ static int replmd_modify_la_add(struct ldb_module *module, int err = parsed_dn_find(ldb, old_dns, old_num_values, &dns[i].guid, dns[i].dsdb_dn->dn, + dns[i].dsdb_dn->extra_part, &exact, &next, - schema_attr->syntax->ldap_oid); + schema_attr->syntax->ldap_oid, + true); if (err != LDB_SUCCESS) { talloc_free(tmp_ctx); return err; @@ -2837,8 +2851,10 @@ static int replmd_modify_la_delete(struct ldb_module *module, ret = parsed_dn_find(ldb, old_dns, old_el->num_values, &p->guid, NULL, + p->dsdb_dn->extra_part, &exact, &next, - schema_attr->syntax->ldap_oid); + schema_attr->syntax->ldap_oid, + true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -3843,8 +3859,8 @@ static int replmd_delete_remove_link(struct ldb_module *module, } ret = parsed_dn_find(ldb, link_dns, link_el->num_values, - guid, dn, &p, &unused, - target_attr->syntax->ldap_oid); + guid, dn, data_blob_null, &p, &unused, + target_attr->syntax->ldap_oid, false); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -3866,6 +3882,7 @@ static int replmd_delete_remove_link(struct ldb_module *module, /* This needs to get the Binary DN, by first searching */ dn_str = dsdb_dn_get_linearized(tmp_ctx, p->dsdb_dn); + dn_val = data_blob_string_const(dn_str); el2->values = &dn_val; el2->num_values = 1; @@ -6954,8 +6971,10 @@ linked_attributes[0]: ret = parsed_dn_find(ldb, pdn_list, old_el->num_values, &guid, dsdb_dn->dn, + dsdb_dn->extra_part, &pdn, &next, - attr->syntax->ldap_oid); + attr->syntax->ldap_oid, + true); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; -- 1.9.1 From abe5f866068ee3064b7cbf73f7385c405f00c230 Mon Sep 17 00:00:00 2001 From: Bob Campbell Date: Fri, 17 Feb 2017 15:51:36 +1300 Subject: [PATCH 11/26] getncchanges: Do not filter secrets by PAS in EXOP_REPL_SECRET This conforms with Windows' behaviour. Signed-off-by: Bob Campbell Pair-programmed-with: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index efad0c9..5a5ad0c 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -340,7 +340,7 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem } /* filter by partial_attribute_set */ - if (partial_attribute_set) { + if (partial_attribute_set && !force_attribute) { uint32_t *result = NULL; BINARY_ARRAY_SEARCH_V(local_pas, partial_attribute_set->num_attids, sa->attributeID_id, uint32_t_cmp, result); -- 1.9.1 From ad584cb5fe181d951930f0a0dc8475aaa8c6ad04 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 16:21:12 +1300 Subject: [PATCH 12/26] getncchanges: Implement functionality for msDS-RevealedUsers This multi-valued DN+Binary linked attribute is present on the server object for an RODC. A link to an object is added to it whenever secret attributes from that object are replicated to an RODC to serve as an audit trail. Signed-off-by: Garming Sam Pair-programmed-with: Bob Campbell --- selftest/knownfail | 2 - source4/rpc_server/drsuapi/getncchanges.c | 170 +++++++++++++++++++++++++++++- 2 files changed, 165 insertions(+), 7 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index fdb76ff..7c5417b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,5 +315,3 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* -# We don't yet support msDS-RevealedUsers -^samba4.drs.repl_rodc.python.*repl_rodc.* diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 5a5ad0c..777a7f7 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -190,6 +190,93 @@ fail: } } +static WERROR getncchanges_update_revealed_list(struct ldb_context *sam_ctx, + TALLOC_CTX *mem_ctx, + struct GUID *destination_dsa_guid, + struct ldb_message **msg, + struct ldb_dn *object_dn, + const struct dsdb_attribute *sa, + struct replPropertyMetaData1 *meta_data, + struct ldb_message *revealed_users) +{ + enum ndr_err_code ndr_err; + int ldb_err; + unsigned i; + char *attr_str = NULL; + char *attr_hex = NULL; + DATA_BLOB attr_blob; + struct ldb_message_element *existing = NULL, *el_add = NULL, *el_del = NULL; + const char * const * secret_attributes = ldb_get_opaque(sam_ctx, "LDB_SECRET_ATTRIBUTE_LIST"); + + if (!ldb_attr_in_list(secret_attributes, + sa->lDAPDisplayName)) { + return WERR_OK; + } + + + ndr_err = ndr_push_struct_blob(&attr_blob, mem_ctx, meta_data, (ndr_push_flags_fn_t)ndr_push_replPropertyMetaData1); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + attr_hex = hex_encode_talloc(mem_ctx, attr_blob.data, attr_blob.length); + if (attr_hex == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + + attr_str = talloc_asprintf(mem_ctx, "B:%zd:%s:%s", attr_blob.length*2, attr_hex, ldb_dn_get_linearized(object_dn)); + if (attr_str == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + + existing = ldb_msg_find_element(revealed_users, "msDS-RevealedUsers"); + if (existing != NULL) { + /* Replace the old value (if one exists) with the current one */ + for (i = 0; i < existing->num_values; i++) { + struct dsdb_dn *existing_dn = dsdb_dn_parse_trusted(mem_ctx, sam_ctx, &existing->values[i], DSDB_SYNTAX_BINARY_DN); + if (ldb_dn_compare(object_dn, existing_dn->dn) == 0) { + struct replPropertyMetaData1 existing_meta_data; + ndr_err = ndr_pull_struct_blob_all_noalloc(&existing_dn->extra_part, + &existing_meta_data, + (ndr_pull_flags_fn_t)ndr_pull_replPropertyMetaData1); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + if (existing_meta_data.attid == sa->attributeID_id) { + ldb_err = ldb_msg_add_empty(*msg, "msDS-RevealedUsers", LDB_FLAG_MOD_DELETE, &el_del); + if (ldb_err != LDB_SUCCESS) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + el_del->values = talloc_array((*msg)->elements, struct ldb_val, 1); + if (el_del->values == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + el_del->values[0] = existing->values[i]; + el_del->num_values = 1; + } + } + } + } + + ldb_err = ldb_msg_add_empty(*msg, "msDS-RevealedUsers", LDB_FLAG_MOD_ADD, &el_add); + if (ldb_err != LDB_SUCCESS) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + el_add->values = talloc_array((*msg)->elements, struct ldb_val, 1); + if (el_add->values == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + + } + + el_add->values[0] = data_blob_string_const(attr_str); + el_add->num_values = 1; + + return WERR_OK; +} + /* drsuapi_DsGetNCChanges for one object */ @@ -206,12 +293,14 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem struct drsuapi_DsReplicaCursorCtrEx *uptodateness_vector, enum drsuapi_DsExtendedOperation extended_op, bool force_object_return, - uint32_t *local_pas) + uint32_t *local_pas, + struct GUID *destination_dsa_guid) { const struct ldb_val *md_value; uint32_t i, n; struct replPropertyMetaDataBlob md; uint32_t rid = 0; + int ldb_err; enum ndr_err_code ndr_err; uint32_t *attids; const char *rdn; @@ -219,6 +308,10 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem uint64_t uSNChanged; unsigned int instanceType; struct dsdb_syntax_ctx syntax_ctx; + struct ldb_result *res = NULL; + struct ldb_message *revealed_list_msg = NULL, *existing_revealed_list_msg = NULL; + WERROR werr; + int ret; /* make dsdb sytanx context for conversions */ dsdb_syntax_ctx_init(&syntax_ctx, sam_ctx, schema); @@ -293,6 +386,44 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem dom_sid_split_rid(NULL, &obj->object.identifier->sid, NULL, &rid); obj->meta_data_ctr->meta_data = talloc_array(obj, struct drsuapi_DsReplicaMetaData, md.ctr.ctr1.count); + + if (extended_op == DRSUAPI_EXOP_REPL_SECRET) { + /* Get the existing revealed users for the destination */ + struct ldb_dn *ntds_dn = NULL, *server_dn = NULL, *machine_dn = NULL; + static const char *machine_attrs[] = { "msDS-RevealedUsers", NULL }; + + ldb_err = dsdb_find_dn_by_guid(sam_ctx, obj, + destination_dsa_guid, 0, + &ntds_dn); + if (ldb_err != LDB_SUCCESS) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + server_dn = ldb_dn_get_parent(obj, ntds_dn); + if (server_dn == NULL) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + ldb_err = samdb_reference_dn(sam_ctx, obj, server_dn, + "serverReference", &machine_dn); + if (ldb_err != LDB_SUCCESS) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + revealed_list_msg = ldb_msg_new(sam_ctx); + if (revealed_list_msg == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } + revealed_list_msg->dn = machine_dn; + + ldb_err = dsdb_search_dn(sam_ctx, obj, &res, machine_dn, machine_attrs, 0); + if (ldb_err != LDB_SUCCESS || res->count != 1) { + return WERR_DS_DRA_INTERNAL_ERROR; + } + + existing_revealed_list_msg = res->msgs[0]; + } + for (n=i=0; ilDAPDisplayName, ldb_dn_get_linearized(msg->dn))); + werr = getncchanges_update_revealed_list(sam_ctx, obj, destination_dsa_guid, &revealed_list_msg, msg->dn, sa, &md.ctr.ctr1.array[i], existing_revealed_list_msg); + if (!W_ERROR_IS_OK(werr)) { + return werr; + } } /* filter by uptodateness_vector */ @@ -354,9 +489,34 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem obj->meta_data_ctr->meta_data[n].originating_invocation_id = md.ctr.ctr1.array[i].originating_invocation_id; obj->meta_data_ctr->meta_data[n].originating_usn = md.ctr.ctr1.array[i].originating_usn; attids[n] = md.ctr.ctr1.array[i].attid; + n++; } + if (revealed_list_msg != NULL) { + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(0,(__location__ ": Failed transaction start - %s\n", + ldb_errstring(sam_ctx))); + return WERR_DS_DRA_INTERNAL_ERROR; + } + + ret = ldb_modify(sam_ctx, revealed_list_msg); + if (ret != LDB_SUCCESS) { + DEBUG(0,(__location__ ": Failed to commit revealed links - %s\n", + ldb_errstring(sam_ctx))); + ldb_transaction_cancel(sam_ctx); + return WERR_DS_DRA_INTERNAL_ERROR; + } + + ret = ldb_transaction_commit(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(0,(__location__ ": Failed transaction commit - %s\n", + ldb_errstring(sam_ctx))); + return WERR_DS_DRA_INTERNAL_ERROR; + } + } + /* ignore it if its an empty change. Note that renames always * change the 'name' attribute, so they won't be ignored by * this @@ -391,7 +551,6 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem */ for (i=0; iobject.attribute_ctr.num_attributes; i++) { struct ldb_message_element *el; - WERROR werr; const struct dsdb_attribute *sa; sa = dsdb_attribute_by_attributeID_id(schema, attids[i]); @@ -1134,7 +1293,6 @@ failed: return WERR_DS_DRA_BAD_DN; } - /* handle a DRSUAPI_EXOP_REPL_OBJ call */ @@ -2413,7 +2571,8 @@ allowed: req10->uptodateness_vector, req10->extended_op, max_wait_reached, - local_pas); + local_pas, + &req10->destination_dsa_guid); if (!W_ERROR_IS_OK(werr)) { return werr; } @@ -2524,7 +2683,8 @@ allowed: req10->uptodateness_vector, req10->extended_op, false, /* force_object_return */ - local_pas); + local_pas, + &req10->destination_dsa_guid); if (!W_ERROR_IS_OK(werr)) { return werr; } -- 1.9.1 From 652a176c8066ee035bd6ba45960945a90be9d437 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 14:00:39 +1300 Subject: [PATCH 13/26] tests/repl_rodc: Ensure that the machine account is tied to the destination DSA Signed-off-by: Garming Sam --- selftest/knownfail | 1 + source4/torture/drs/python/repl_rodc.py | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 7c5417b..eec214b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,3 +315,4 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* +^samba4.drs.repl_rodc.python.*repl_rodc.DrsRodcTestCase.test_msDSRevealedUsers_using_other_RODC diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py index f5d08d4..535bd93 100644 --- a/source4/torture/drs/python/repl_rodc.py +++ b/source4/torture/drs/python/repl_rodc.py @@ -415,6 +415,73 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): # Check that the user has been added to msDSRevealedUsers (packed_attrs_1, unpacked_attrs_1) = self._assert_in_revealed_users(user_dn, expected_user_attributes) + def test_msDSRevealedUsers_using_other_RODC(self): + """ + Ensure that the machine account is tied to the destination DSA. + """ + # Create a new identical RODC with just the first letter missing + other_rodc_name = self.rodc_name[1:] + other_rodc_ctx = dc_join(server=self.ldb_dc1.host_dns_name(), creds=self.get_credentials(), lp=self.get_loadparm(), + site=self.site, netbios_name=other_rodc_name, + targetdir=None, domain=None, machinepass=self.rodc_pass) + self._create_rodc(other_rodc_ctx) + + other_rodc_creds = Credentials() + other_rodc_creds.guess(other_rodc_ctx.lp) + other_rodc_creds.set_username(other_rodc_name+'$') + other_rodc_creds.set_password(self.rodc_pass) + + (other_rodc_drs, other_rodc_drs_handle) = self._ds_bind(self.dnsname_dc1, other_rodc_creds) + + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + user_name = "test_rodcF_%s" % rand + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, 'penguin12#', False, user_name) + self.ldb_dc1.add_remove_group_members("Allowed RODC Password Replication Group", + [user_name], + add_members_operation=True) + + req10 = self._getnc_req10(dest_dsa=str(other_rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + + try: + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.") + except WERRORError as (enum, estr): + self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + + try: + (level, ctr) = other_rodc_drs.DsGetNCChanges(other_rodc_drs_handle, 10, req10) + self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.") + except WERRORError as (enum, estr): + self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + def _assert_in_revealed_users(self, user_dn, attrlist): res = self.ldb_dc1.search(scope=ldb.SCOPE_BASE, base=self.computer_dn, attrs=["msDS-RevealedUsers"]) -- 1.9.1 From a1e252c452425594d8eda7aa11476e05eeae993d Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Tue, 28 Feb 2017 16:21:25 +1300 Subject: [PATCH 14/26] getncchanges: Tie destination DSA GUID to authenticating RODC for REPL_SECRET Signed-off-by: Garming Sam --- selftest/knownfail | 1 - source4/rpc_server/drsuapi/getncchanges.c | 76 +++++++++++++++++++------------ 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index eec214b..7c5417b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,4 +315,3 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* -^samba4.drs.repl_rodc.python.*repl_rodc.DrsRodcTestCase.test_msDSRevealedUsers_using_other_RODC diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 777a7f7..ad13f82 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -192,7 +192,6 @@ fail: static WERROR getncchanges_update_revealed_list(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, - struct GUID *destination_dsa_guid, struct ldb_message **msg, struct ldb_dn *object_dn, const struct dsdb_attribute *sa, @@ -294,7 +293,7 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem enum drsuapi_DsExtendedOperation extended_op, bool force_object_return, uint32_t *local_pas, - struct GUID *destination_dsa_guid) + struct ldb_dn *machine_dn) { const struct ldb_val *md_value; uint32_t i, n; @@ -389,27 +388,8 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem if (extended_op == DRSUAPI_EXOP_REPL_SECRET) { /* Get the existing revealed users for the destination */ - struct ldb_dn *ntds_dn = NULL, *server_dn = NULL, *machine_dn = NULL; static const char *machine_attrs[] = { "msDS-RevealedUsers", NULL }; - ldb_err = dsdb_find_dn_by_guid(sam_ctx, obj, - destination_dsa_guid, 0, - &ntds_dn); - if (ldb_err != LDB_SUCCESS) { - return WERR_DS_DRA_INTERNAL_ERROR; - } - - server_dn = ldb_dn_get_parent(obj, ntds_dn); - if (server_dn == NULL) { - return WERR_DS_DRA_INTERNAL_ERROR; - } - - ldb_err = samdb_reference_dn(sam_ctx, obj, server_dn, - "serverReference", &machine_dn); - if (ldb_err != LDB_SUCCESS) { - return WERR_DS_DRA_INTERNAL_ERROR; - } - revealed_list_msg = ldb_msg_new(sam_ctx); if (revealed_list_msg == NULL) { return WERR_NOT_ENOUGH_MEMORY; @@ -459,7 +439,11 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem force_attribute = true; DEBUG(4,("Forcing attribute %s in %s\n", sa->lDAPDisplayName, ldb_dn_get_linearized(msg->dn))); - werr = getncchanges_update_revealed_list(sam_ctx, obj, destination_dsa_guid, &revealed_list_msg, msg->dn, sa, &md.ctr.ctr1.array[i], existing_revealed_list_msg); + werr = getncchanges_update_revealed_list(sam_ctx, obj, + &revealed_list_msg, + msg->dn, sa, + &md.ctr.ctr1.array[i], + existing_revealed_list_msg); if (!W_ERROR_IS_OK(werr)) { return werr; } @@ -1149,13 +1133,15 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, struct drsuapi_DsGetNCChangesRequest10 *req10, struct dom_sid *user_sid, struct drsuapi_DsGetNCChangesCtr6 *ctr6, - bool has_get_all_changes) + bool has_get_all_changes, + struct ldb_dn **machine_dn) { struct drsuapi_DsReplicaObjectIdentifier *ncRoot = req10->naming_context; struct ldb_dn *obj_dn = NULL; + struct ldb_dn *ntds_dn = NULL, *server_dn = NULL; struct ldb_dn *rodc_dn, *krbtgt_link_dn; int ret; - const char *rodc_attrs[] = { "msDS-KrbTgtLink", "msDS-NeverRevealGroup", "msDS-RevealOnDemandGroup", NULL }; + const char *rodc_attrs[] = { "msDS-KrbTgtLink", "msDS-NeverRevealGroup", "msDS-RevealOnDemandGroup", "objectGUID", NULL }; const char *obj_attrs[] = { "tokenGroups", "objectSid", "UserAccountControl", "msDS-KrbTgtLinkBL", NULL }; struct ldb_result *rodc_res, *obj_res; const struct dom_sid **never_reveal_sids, **reveal_sids, **token_sids; @@ -1179,6 +1165,31 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, } /* + * Before we accept or deny, fetch the machine DN for the destination + * DSA GUID. + * + * If we are the RODC, we will check that this matches the SID. + */ + ret = dsdb_find_dn_by_guid(b_state->sam_ctx_system, mem_ctx, + &req10->destination_dsa_guid, 0, + &ntds_dn); + if (ret != LDB_SUCCESS) { + goto failed; + } + + server_dn = ldb_dn_get_parent(mem_ctx, ntds_dn); + if (server_dn == NULL) { + goto failed; + } + + ret = samdb_reference_dn(b_state->sam_ctx_system, mem_ctx, server_dn, + "serverReference", machine_dn); + + if (ret != LDB_SUCCESS) { + goto failed; + } + + /* * In MS-DRSR.pdf 5.99 IsGetNCChangesPermissionGranted * * The pseudo code indicate @@ -1223,6 +1234,14 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, goto allowed; } + /* + * Must be an RODC account at this point, verify machine DN matches the + * SID account + */ + if (ldb_dn_compare(rodc_res->msgs[0]->dn, *machine_dn) != 0) { + goto denied; + } + /* an RODC is allowed to get its own krbtgt account secrets */ krbtgt_link_dn = samdb_result_dn(b_state->sam_ctx_system, mem_ctx, rodc_res->msgs[0], "msDS-KrbTgtLink", NULL); @@ -1994,6 +2013,7 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_ struct dsdb_schema_prefixmap *pfm_remote = NULL; bool full = true; uint32_t *local_pas = NULL; + struct ldb_dn *machine_dn = NULL; /* Only used for REPL SECRET EXOP */ DCESRV_PULL_HANDLE_WERR(h, r->in.bind_handle, DRSUAPI_BIND_HANDLE); b_state = h->data; @@ -2245,7 +2265,8 @@ allowed: werr = getncchanges_repl_secret(b_state, mem_ctx, req10, user_sid, &r->out.ctr->ctr6, - has_get_all_changes); + has_get_all_changes, + &machine_dn); r->out.result = werr; W_ERROR_NOT_OK_RETURN(werr); break; @@ -2571,8 +2592,7 @@ allowed: req10->uptodateness_vector, req10->extended_op, max_wait_reached, - local_pas, - &req10->destination_dsa_guid); + local_pas, machine_dn); if (!W_ERROR_IS_OK(werr)) { return werr; } @@ -2684,7 +2704,7 @@ allowed: req10->extended_op, false, /* force_object_return */ local_pas, - &req10->destination_dsa_guid); + machine_dn); if (!W_ERROR_IS_OK(werr)) { return werr; } -- 1.9.1 From 96e79ddfc52fd64cc4b4a58388627f85ef60074a Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 11:01:36 +1300 Subject: [PATCH 15/26] getncchanges: Refactor filter_attrs from build_object This makes it easier to have a transaction around it. Signed-off-by: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 176 ++++++++++++++++++------------ 1 file changed, 105 insertions(+), 71 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index ad13f82..5bde00b 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -276,6 +276,101 @@ static WERROR getncchanges_update_revealed_list(struct ldb_context *sam_ctx, return WERR_OK; } +static WERROR get_nc_changes_filter_attrs(struct drsuapi_DsReplicaObjectListItemEx *obj, + struct replPropertyMetaDataBlob md, + struct ldb_context *sam_ctx, + const struct ldb_message *msg, + uint32_t *count, + uint64_t highest_usn, + const struct dsdb_attribute *rdn_sa, + struct dsdb_schema *schema, + bool exop_secret, + struct ldb_message **revealed_list_msg, + struct ldb_message *existing_revealed_list_msg, + struct drsuapi_DsReplicaCursorCtrEx *uptodateness_vector, + struct drsuapi_DsPartialAttributeSet *partial_attribute_set, uint32_t *local_pas, + uint32_t *attids) +{ + uint32_t i, n; + WERROR werr; + for (n=i=0; iattributeID_id) continue; + + sa = dsdb_attribute_by_attributeID_id(schema, md.ctr.ctr1.array[i].attid); + if (!sa) { + DEBUG(0,(__location__ ": Failed to find attribute in schema for attrid %u mentioned in replPropertyMetaData of %s\n", + (unsigned int)md.ctr.ctr1.array[i].attid, + ldb_dn_get_linearized(msg->dn))); + return WERR_DS_DRA_INTERNAL_ERROR; + } + + if (sa->linkID) { + struct ldb_message_element *el; + el = ldb_msg_find_element(msg, sa->lDAPDisplayName); + if (el && el->num_values && dsdb_dn_is_upgraded_link_val(&el->values[0])) { + /* don't send upgraded links inline */ + continue; + } + } + + if (exop_secret && + !dsdb_attr_in_rodc_fas(sa)) { + force_attribute = true; + DEBUG(4,("Forcing attribute %s in %s\n", + sa->lDAPDisplayName, ldb_dn_get_linearized(msg->dn))); + werr = getncchanges_update_revealed_list(sam_ctx, obj, + revealed_list_msg, + msg->dn, sa, + &md.ctr.ctr1.array[i], + existing_revealed_list_msg); + if (!W_ERROR_IS_OK(werr)) { + return werr; + } + } + + /* filter by uptodateness_vector */ + if (md.ctr.ctr1.array[i].attid != DRSUAPI_ATTID_instanceType && + !force_attribute && + udv_filter(uptodateness_vector, + &md.ctr.ctr1.array[i].originating_invocation_id, + md.ctr.ctr1.array[i].originating_usn)) { + continue; + } + + /* filter by partial_attribute_set */ + if (partial_attribute_set && !force_attribute) { + uint32_t *result = NULL; + BINARY_ARRAY_SEARCH_V(local_pas, partial_attribute_set->num_attids, sa->attributeID_id, + uint32_t_cmp, result); + if (result == NULL) { + continue; + } + } + + obj->meta_data_ctr->meta_data[n].originating_change_time = md.ctr.ctr1.array[i].originating_change_time; + obj->meta_data_ctr->meta_data[n].version = md.ctr.ctr1.array[i].version; + obj->meta_data_ctr->meta_data[n].originating_invocation_id = md.ctr.ctr1.array[i].originating_invocation_id; + obj->meta_data_ctr->meta_data[n].originating_usn = md.ctr.ctr1.array[i].originating_usn; + attids[n] = md.ctr.ctr1.array[i].attid; + + n++; + } + + *count = n; + + return WERR_OK; +} + /* drsuapi_DsGetNCChanges for one object */ @@ -404,77 +499,16 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem existing_revealed_list_msg = res->msgs[0]; } - for (n=i=0; iattributeID_id) continue; - - sa = dsdb_attribute_by_attributeID_id(schema, md.ctr.ctr1.array[i].attid); - if (!sa) { - DEBUG(0,(__location__ ": Failed to find attribute in schema for attrid %u mentioned in replPropertyMetaData of %s\n", - (unsigned int)md.ctr.ctr1.array[i].attid, - ldb_dn_get_linearized(msg->dn))); - return WERR_DS_DRA_INTERNAL_ERROR; - } - - if (sa->linkID) { - struct ldb_message_element *el; - el = ldb_msg_find_element(msg, sa->lDAPDisplayName); - if (el && el->num_values && dsdb_dn_is_upgraded_link_val(&el->values[0])) { - /* don't send upgraded links inline */ - continue; - } - } - - if (extended_op == DRSUAPI_EXOP_REPL_SECRET && - !dsdb_attr_in_rodc_fas(sa)) { - force_attribute = true; - DEBUG(4,("Forcing attribute %s in %s\n", - sa->lDAPDisplayName, ldb_dn_get_linearized(msg->dn))); - werr = getncchanges_update_revealed_list(sam_ctx, obj, - &revealed_list_msg, - msg->dn, sa, - &md.ctr.ctr1.array[i], - existing_revealed_list_msg); - if (!W_ERROR_IS_OK(werr)) { - return werr; - } - } - - /* filter by uptodateness_vector */ - if (md.ctr.ctr1.array[i].attid != DRSUAPI_ATTID_instanceType && - !force_attribute && - udv_filter(uptodateness_vector, - &md.ctr.ctr1.array[i].originating_invocation_id, - md.ctr.ctr1.array[i].originating_usn)) { - continue; - } - - /* filter by partial_attribute_set */ - if (partial_attribute_set && !force_attribute) { - uint32_t *result = NULL; - BINARY_ARRAY_SEARCH_V(local_pas, partial_attribute_set->num_attids, sa->attributeID_id, - uint32_t_cmp, result); - if (result == NULL) { - continue; - } - } - - obj->meta_data_ctr->meta_data[n].originating_change_time = md.ctr.ctr1.array[i].originating_change_time; - obj->meta_data_ctr->meta_data[n].version = md.ctr.ctr1.array[i].version; - obj->meta_data_ctr->meta_data[n].originating_invocation_id = md.ctr.ctr1.array[i].originating_invocation_id; - obj->meta_data_ctr->meta_data[n].originating_usn = md.ctr.ctr1.array[i].originating_usn; - attids[n] = md.ctr.ctr1.array[i].attid; - - n++; + werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, + highest_usn, rdn_sa, schema, + extended_op == DRSUAPI_EXOP_REPL_SECRET, + &revealed_list_msg, + existing_revealed_list_msg, + uptodateness_vector, + partial_attribute_set, local_pas, + attids); + if (!W_ERROR_IS_OK(werr)) { + return werr; } if (revealed_list_msg != NULL) { -- 1.9.1 From dfb25c6e1bee828dcfefef342c952e47a43f84d2 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 11:14:24 +1300 Subject: [PATCH 16/26] getncchanges: Prevent a small, but possible race condition in build_object Signed-off-by: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 64 +++++++++++++++++++------------ 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 5bde00b..4de6647 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -491,27 +491,6 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem } revealed_list_msg->dn = machine_dn; - ldb_err = dsdb_search_dn(sam_ctx, obj, &res, machine_dn, machine_attrs, 0); - if (ldb_err != LDB_SUCCESS || res->count != 1) { - return WERR_DS_DRA_INTERNAL_ERROR; - } - - existing_revealed_list_msg = res->msgs[0]; - } - - werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, - highest_usn, rdn_sa, schema, - extended_op == DRSUAPI_EXOP_REPL_SECRET, - &revealed_list_msg, - existing_revealed_list_msg, - uptodateness_vector, - partial_attribute_set, local_pas, - attids); - if (!W_ERROR_IS_OK(werr)) { - return werr; - } - - if (revealed_list_msg != NULL) { ret = ldb_transaction_start(sam_ctx); if (ret != LDB_SUCCESS) { DEBUG(0,(__location__ ": Failed transaction start - %s\n", @@ -519,20 +498,55 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem return WERR_DS_DRA_INTERNAL_ERROR; } - ret = ldb_modify(sam_ctx, revealed_list_msg); - if (ret != LDB_SUCCESS) { - DEBUG(0,(__location__ ": Failed to commit revealed links - %s\n", - ldb_errstring(sam_ctx))); + ldb_err = dsdb_search_dn(sam_ctx, obj, &res, machine_dn, machine_attrs, 0); + if (ldb_err != LDB_SUCCESS || res->count != 1) { ldb_transaction_cancel(sam_ctx); return WERR_DS_DRA_INTERNAL_ERROR; } + existing_revealed_list_msg = res->msgs[0]; + + werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, + highest_usn, rdn_sa, schema, + true, + &revealed_list_msg, + existing_revealed_list_msg, + uptodateness_vector, + partial_attribute_set, local_pas, + attids); + if (!W_ERROR_IS_OK(werr)) { + ldb_transaction_cancel(sam_ctx); + return werr; + } + + if (revealed_list_msg != NULL) { + ret = ldb_modify(sam_ctx, revealed_list_msg); + if (ret != LDB_SUCCESS) { + DEBUG(0,(__location__ ": Failed to alter revealed links - %s\n", + ldb_errstring(sam_ctx))); + ldb_transaction_cancel(sam_ctx); + return WERR_DS_DRA_INTERNAL_ERROR; + } + } + ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { DEBUG(0,(__location__ ": Failed transaction commit - %s\n", ldb_errstring(sam_ctx))); return WERR_DS_DRA_INTERNAL_ERROR; } + } else { + werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, + highest_usn, rdn_sa, schema, + false, + &revealed_list_msg, + existing_revealed_list_msg, + uptodateness_vector, + partial_attribute_set, local_pas, + attids); + if (!W_ERROR_IS_OK(werr)) { + return werr; + } } /* ignore it if its an empty change. Note that renames always -- 1.9.1 From b445ec63c973c4bd8cc36c37fd11298436935837 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 11:18:33 +1300 Subject: [PATCH 17/26] getncchanges: reduce scope of a few variables Signed-off-by: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 4de6647..7460e01 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -403,7 +403,6 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem unsigned int instanceType; struct dsdb_syntax_ctx syntax_ctx; struct ldb_result *res = NULL; - struct ldb_message *revealed_list_msg = NULL, *existing_revealed_list_msg = NULL; WERROR werr; int ret; @@ -483,7 +482,12 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem if (extended_op == DRSUAPI_EXOP_REPL_SECRET) { /* Get the existing revealed users for the destination */ - static const char *machine_attrs[] = { "msDS-RevealedUsers", NULL }; + struct ldb_message *revealed_list_msg = NULL; + struct ldb_message *existing_revealed_list_msg = NULL; + const char *machine_attrs[] = { + "msDS-RevealedUsers", + NULL + }; revealed_list_msg = ldb_msg_new(sam_ctx); if (revealed_list_msg == NULL) { @@ -539,8 +543,8 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, highest_usn, rdn_sa, schema, false, - &revealed_list_msg, - existing_revealed_list_msg, + NULL, + NULL, uptodateness_vector, partial_attribute_set, local_pas, attids); -- 1.9.1 From 4c6f9707c3fcfeec8af14f7df7bbdc4e06f62140 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 11:42:04 +1300 Subject: [PATCH 18/26] getncchanges: Add a comment for new function Signed-off-by: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 7460e01..385e2b6 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -276,6 +276,13 @@ static WERROR getncchanges_update_revealed_list(struct ldb_context *sam_ctx, return WERR_OK; } +/* + * This function filter attributes for build_object based on the + * uptodatenessvector and partial attribute set. + * + * Any secret attributes are forced here for REPL_SECRET, and audited at this + * point with msDS-RevealedUsers. + */ static WERROR get_nc_changes_filter_attrs(struct drsuapi_DsReplicaObjectListItemEx *obj, struct replPropertyMetaDataBlob md, struct ldb_context *sam_ctx, -- 1.9.1 From b1231095e35f9870a570c11c800ac2b492ea9d97 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 11:43:46 +1300 Subject: [PATCH 19/26] getncchanges: Reorder parameters in filter_attrs for clarity Signed-off-by: Garming Sam --- source4/rpc_server/drsuapi/getncchanges.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 385e2b6..d334de7 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -291,12 +291,13 @@ static WERROR get_nc_changes_filter_attrs(struct drsuapi_DsReplicaObjectListItem uint64_t highest_usn, const struct dsdb_attribute *rdn_sa, struct dsdb_schema *schema, + struct drsuapi_DsReplicaCursorCtrEx *uptodateness_vector, + struct drsuapi_DsPartialAttributeSet *partial_attribute_set, + uint32_t *local_pas, + uint32_t *attids, bool exop_secret, struct ldb_message **revealed_list_msg, - struct ldb_message *existing_revealed_list_msg, - struct drsuapi_DsReplicaCursorCtrEx *uptodateness_vector, - struct drsuapi_DsPartialAttributeSet *partial_attribute_set, uint32_t *local_pas, - uint32_t *attids) + struct ldb_message *existing_revealed_list_msg) { uint32_t i, n; WERROR werr; @@ -519,12 +520,12 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, highest_usn, rdn_sa, schema, - true, - &revealed_list_msg, - existing_revealed_list_msg, uptodateness_vector, partial_attribute_set, local_pas, - attids); + attids, + true, + &revealed_list_msg, + existing_revealed_list_msg); if (!W_ERROR_IS_OK(werr)) { ldb_transaction_cancel(sam_ctx); return werr; @@ -549,12 +550,12 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem } else { werr = get_nc_changes_filter_attrs(obj, md, sam_ctx, msg, &n, highest_usn, rdn_sa, schema, - false, - NULL, - NULL, uptodateness_vector, partial_attribute_set, local_pas, - attids); + attids, + false, + NULL, + NULL); if (!W_ERROR_IS_OK(werr)) { return werr; } -- 1.9.1 From f326bee5a0ac96bcb6ef321190e6af11bbc30cf8 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 16:05:25 +1300 Subject: [PATCH 20/26] tests/repl_rodc: Test the direct allow/deny attribute works Signed-off-by: Garming Sam --- selftest/knownfail | 1 + source4/torture/drs/python/repl_rodc.py | 86 +++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 7c5417b..fe90a4f 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,3 +315,4 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* +^samba4.drs.repl_rodc.python.*repl_rodc.DrsRodcTestCase.test_msDSRevealedUsers_local_deny_allow diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py index 535bd93..01c9c6d 100644 --- a/source4/torture/drs/python/repl_rodc.py +++ b/source4/torture/drs/python/repl_rodc.py @@ -118,6 +118,7 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): rodc_creds.guess(self.rodc_ctx.lp) rodc_creds.set_username(self.rodc_name+'$') rodc_creds.set_password(self.rodc_pass) + self.rodc_creds = rodc_creds (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1) (self.rodc_drs, self.rodc_drs_handle) = self._ds_bind(self.dnsname_dc1, rodc_creds) @@ -482,6 +483,91 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase): except WERRORError as (enum, estr): self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + def test_msDSRevealedUsers_local_deny_allow(self): + """ + Ensure that the deny trumps allow, and we can modify these + attributes directly instead of the global groups. + + This may fail on Windows due to tokenGroup calculation caching. + """ + rand = random.randint(1, 10000000) + expected_user_attributes = [drsuapi.DRSUAPI_ATTID_lmPwdHistory, + drsuapi.DRSUAPI_ATTID_supplementalCredentials, + drsuapi.DRSUAPI_ATTID_ntPwdHistory, + drsuapi.DRSUAPI_ATTID_unicodePwd, + drsuapi.DRSUAPI_ATTID_dBCSPwd] + + # Add a user on DC1, add it to allowed password replication + # group, and replicate to RODC with EXOP_REPL_SECRETS + user_name = "test_rodcF_%s" % rand + password = "password12#" + user_dn = "CN=%s,%s" % (user_name, self.ou) + self.ldb_dc1.add({ + "dn": user_dn, + "objectclass": "user", + "sAMAccountName": user_name + }) + + # Store some secret on this user + self.ldb_dc1.setpassword("(sAMAccountName=%s)" % user_name, password, False, user_name) + + req10 = self._getnc_req10(dest_dsa=str(self.rodc_ctx.ntds_guid), + invocation_id=self.ldb_dc1.get_invocation_id(), + nc_dn_str=user_dn, + exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET, + partial_attribute_set=drs_get_rodc_partial_attribute_set(self.ldb_dc1, self.tmp_samdb), + max_objects=133, + replica_flags=0) + + m = ldb.Message() + m.dn = ldb.Dn(self.ldb_dc1, self.computer_dn) + + m["msDS-RevealOnDemandGroup"] = \ + ldb.MessageElement(user_dn, ldb.FLAG_MOD_ADD, + "msDS-RevealOnDemandGroup") + self.ldb_dc1.modify(m) + + # In local allow, should be success + try: + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + except: + self.fail("Should have succeeded when in local allow group") + + self._assert_in_revealed_users(user_dn, expected_user_attributes) + + (self.rodc_drs, self.rodc_drs_handle) = self._ds_bind(self.dnsname_dc1, self.rodc_creds) + + m = ldb.Message() + m.dn = ldb.Dn(self.ldb_dc1, self.computer_dn) + + m["msDS-NeverRevealGroup"] = \ + ldb.MessageElement(user_dn, ldb.FLAG_MOD_ADD, + "msDS-NeverRevealGroup") + self.ldb_dc1.modify(m) + + # In local allow and deny, should be failure + try: + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.") + except WERRORError as (enum, estr): + self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + + m = ldb.Message() + m.dn = ldb.Dn(self.ldb_dc1, self.computer_dn) + + m["msDS-RevealOnDemandGroup"] = \ + ldb.MessageElement(user_dn, ldb.FLAG_MOD_DELETE, + "msDS-RevealOnDemandGroup") + self.ldb_dc1.modify(m) + + # In local deny, should be failure + (self.rodc_drs, self.rodc_drs_handle) = self._ds_bind(self.dnsname_dc1, self.rodc_creds) + try: + (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 10, req10) + self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.") + except WERRORError as (enum, estr): + self.assertEquals(enum, 8630) # ERROR_DS_DRA_SECRETS_DENIED + def _assert_in_revealed_users(self, user_dn, attrlist): res = self.ldb_dc1.search(scope=ldb.SCOPE_BASE, base=self.computer_dn, attrs=["msDS-RevealedUsers"]) -- 1.9.1 From 79e23eb471efdb591bf7a80960a9f392654e8625 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 16:02:40 +1300 Subject: [PATCH 21/26] getncchanges: include object SID in tokenGroups calculation for repl secret Signed-off-by: Garming Sam --- selftest/knownfail | 1 - source4/rpc_server/drsuapi/getncchanges.c | 18 +++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index fe90a4f..7c5417b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,4 +315,3 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* -^samba4.drs.repl_rodc.python.*repl_rodc.DrsRodcTestCase.test_msDSRevealedUsers_local_deny_allow diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index d334de7..82a1762 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1129,13 +1129,14 @@ static WERROR samdb_result_sid_array_dn(struct ldb_context *sam_ctx, /* return an array of SIDs from a ldb_message given an attribute name - assumes the SIDs are in NDR form + assumes the SIDs are in NDR form (with an additional sid at the end) */ static WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, struct ldb_message *msg, TALLOC_CTX *mem_ctx, const char *attr, - const struct dom_sid ***sids) + const struct dom_sid ***sids, + const struct dom_sid *user_sid) { struct ldb_message_element *el; unsigned int i; @@ -1146,7 +1147,8 @@ static WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, return WERR_OK; } - (*sids) = talloc_array(mem_ctx, const struct dom_sid *, el->num_values + 1); + /* Make array long enough for NULL and additional SID */ + (*sids) = talloc_array(mem_ctx, const struct dom_sid *, el->num_values + 2); W_ERROR_HAVE_NO_MEMORY(*sids); for (i=0; inum_values; i++) { @@ -1163,7 +1165,8 @@ static WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, } (*sids)[i] = sid; } - (*sids)[i] = NULL; + (*sids)[i] = user_sid; + (*sids)[i+1] = NULL; return WERR_OK; } @@ -1205,6 +1208,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, const char *obj_attrs[] = { "tokenGroups", "objectSid", "UserAccountControl", "msDS-KrbTgtLinkBL", NULL }; struct ldb_result *rodc_res, *obj_res; const struct dom_sid **never_reveal_sids, **reveal_sids, **token_sids; + const struct dom_sid *object_sid = NULL; WERROR werr; DEBUG(3,(__location__ ": DRSUAPI_EXOP_REPL_SECRET extended op on %s\n", @@ -1289,8 +1293,8 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, if (ret != LDB_SUCCESS || obj_res->count != 1) goto failed; /* if the object SID is equal to the user_sid, allow */ - if (dom_sid_equal(user_sid, - samdb_result_dom_sid(mem_ctx, obj_res->msgs[0], "objectSid"))) { + object_sid = samdb_result_dom_sid(mem_ctx, obj_res->msgs[0], "objectSid"); + if (dom_sid_equal(user_sid, object_sid)) { goto allowed; } @@ -1335,7 +1339,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, } werr = samdb_result_sid_array_ndr(b_state->sam_ctx_system, obj_res->msgs[0], - mem_ctx, "tokenGroups", &token_sids); + mem_ctx, "tokenGroups", &token_sids, object_sid); if (!W_ERROR_IS_OK(werr) || token_sids==NULL) { goto denied; } -- 1.9.1 From a08e178bbb8532dc265728f8a3418b83d1da97a3 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Fri, 3 Mar 2017 17:31:46 +1300 Subject: [PATCH 22/26] dbcheck: Improve dbcheck to find (and may fix) dangling msDS-RevealedUsers We cannot add missing backlinks because of the duplicate checking. There seems to be no trivial way to add the bypass. Signed-off-by: Garming Sam --- python/samba/dbchecker.py | 48 ++++++++++++++ selftest/knownfail | 2 + .../add-dangling-multi-backlink.ldif | 10 +++ .../add-dangling-multilink-users.ldif | 20 ++++++ .../add-initially-normal-multilink.ldif | 19 ++++++ .../delete-only-multi-backlink.ldif | 13 ++++ testprogs/blackbox/dbcheck-links.sh | 77 ++++++++++++++++++++++ 7 files changed, 189 insertions(+) create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multi-backlink.ldif create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multilink-users.ldif create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/add-initially-normal-multilink.ldif create mode 100644 source4/selftest/provisions/release-4-5-0-pre1/delete-only-multi-backlink.ldif diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py index 032c0e7..1a73fe0 100644 --- a/python/samba/dbchecker.py +++ b/python/samba/dbchecker.py @@ -973,6 +973,54 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base))) if v_guid == obj_guid: match_count += 1 if match_count != 1: + reverse_syntax_oid = self.samdb_schema.get_syntax_oid_from_lDAPDisplayName(reverse_link_name) + if syntax_oid == dsdb.DSDB_SYNTAX_BINARY_DN or reverse_syntax_oid == dsdb.DSDB_SYNTAX_BINARY_DN: + if not linkID & 1: + # Forward binary multi-valued linked attribute + forward_count = 0 + for w in obj[attrname]: + w_guid = dsdb_Dn(self.samdb, w).dn.get_extended_component("GUID") + if w_guid == guid: + forward_count += 1 + + if match_count == forward_count: + continue + + error_count += 1 + + # Add or remove the missing number of backlinks + diff_count = forward_count - match_count + + # Loop until the difference between the forward and + # the backward links is resolved. + while diff_count != 0: + if diff_count > 0: + # self.err_missing_backlink(obj, attrname, + # obj.dn.extended_str(), + # reverse_link_name, + # dsdb_dn.dn) + # diff_count -= 1 + # TODO no method to fix these right now + self.report("ERROR: Can't fix missing " + "multi-valued backlinks on %s" % str(dsdb_dn.dn)) + break + else: + self.err_orphaned_backlink(res[0], reverse_link_name, + obj.dn.extended_str(), attrname, + dsdb_dn.dn) + diff_count += 1 + + else: + # If there's a backward link on binary multi-valued linked attribute, + # let the check on the forward link remedy the value. + # UNLESS, there is no forward link detected. + if match_count == 0: + self.err_orphaned_backlink(obj, attrname, + val, reverse_link_name, + dsdb_dn.dn) + + continue + error_count += 1 if linkID & 1: # Backlink exists, but forward link does not diff --git a/selftest/knownfail b/selftest/knownfail index 7c5417b..cfd4b35 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -315,3 +315,5 @@ ^samba3.smb2.credits.session_setup_credits_granted.* ^samba3.smb2.credits.single_req_credits_granted.* ^samba3.smb2.credits.skipped_mid.* +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_dbcheck +^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing diff --git a/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multi-backlink.ldif b/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multi-backlink.ldif new file mode 100644 index 0000000..f62877e --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multi-backlink.ldif @@ -0,0 +1,10 @@ +dn: CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modify +delete: msDS-RevealedUsers +msDS-RevealedUsers: B:96:3700090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5A00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:;;CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp + +dn: CN=dangling-multi5,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modify +add: msDS-RevealedDSAs +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp diff --git a/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multilink-users.ldif b/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multilink-users.ldif new file mode 100644 index 0000000..bb40087 --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/add-dangling-multilink-users.ldif @@ -0,0 +1,20 @@ +dn: CN=dangling-multi1,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +objectclass: user +samaccountname: dangling-multi1 + +dn: CN=dangling-multi2,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +objectclass: user +samaccountname: dangling-multi2 + +dn: CN=dangling-multi3,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +objectclass: user +samaccountname: dangling-multi3 + +dn: CN=dangling-multi4,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +objectclass: user +samaccountname: dangling-multi4 + +dn: CN=dangling-multi5,CN=users,DC=release-4-5-0-pre1,DC=samba,DC=corp +objectclass: user +samaccountname: dangling-multi5 + diff --git a/source4/selftest/provisions/release-4-5-0-pre1/add-initially-normal-multilink.ldif b/source4/selftest/provisions/release-4-5-0-pre1/add-initially-normal-multilink.ldif new file mode 100644 index 0000000..6633e61 --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/add-initially-normal-multilink.ldif @@ -0,0 +1,19 @@ +dn: CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modify +add: msDS-RevealedUsers +msDS-RevealedUsers: B:96:3700090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:3700090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:3700090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:3700090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5A00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5A00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5A00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5A00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5E00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5E00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5E00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:5E00090002000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:7D00090001000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:7D00090001000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:7D00090001000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=dangling-multi3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedUsers: B:96:7D00090001000000E078C90E0300000097016C30ACF34E469BDF57F1E468694CA10E000000000000A10E000000000000:CN=Administrator,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp diff --git a/source4/selftest/provisions/release-4-5-0-pre1/delete-only-multi-backlink.ldif b/source4/selftest/provisions/release-4-5-0-pre1/delete-only-multi-backlink.ldif new file mode 100644 index 0000000..1890b57 --- /dev/null +++ b/source4/selftest/provisions/release-4-5-0-pre1/delete-only-multi-backlink.ldif @@ -0,0 +1,13 @@ +dn: CN=dangling-multi2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modify +delete: msDS-RevealedDSAs +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp + +dn: CN=dangling-multi3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp +changetype: modify +delete: msDS-RevealedDSAs +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp +msDS-RevealedDSAs: ;;CN=DOUGLASB-DESKTO,OU=Domain Controllers,DC=release-4-5-0-pre1,DC=samba,DC=corp diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh index 2a1bfba..9376e2a 100755 --- a/testprogs/blackbox/dbcheck-links.sh +++ b/testprogs/blackbox/dbcheck-links.sh @@ -200,6 +200,78 @@ dangling_one_way() { fi } +dangling_multi_valued() { + # multi1 - All 4 backlinks + # multi2 - Missing all 4 backlinks + # multi3 - Missing 2 backlinks + # Administrator - Has 2 too many backlinks + # multi5 - Has 2 backlinks but no forward links + ldif=$release_dir/add-dangling-multilink-users.ldif + TZ=UTC $ldbadd -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif + if [ "$?" != "0" ]; then + return 1 + fi + + ldif=$release_dir/add-initially-normal-multilink.ldif + TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $ldif + if [ "$?" != "0" ]; then + return 1 + fi + + ldif=$release_dir/delete-only-multi-backlink.ldif + TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-5-0-PRE1,DC%3DSAMBA,DC%3DCORP.ldb $ldif + if [ "$?" != "0" ]; then + return 1 + fi + + ldif=$release_dir/add-dangling-multi-backlink.ldif + TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-5-0-PRE1,DC%3DSAMBA,DC%3DCORP.ldb $ldif + if [ "$?" != "0" ]; then + return 1 + fi + + $PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --fix --yes + if [ "$?" != "1" ]; then + return 1 + fi +} + +dangling_multi_valued_check_missing() { + WORDS=`TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(samaccountname=dangling-multi2)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted msDS-RevealedDSAs | grep msDS-RevealedDSAs | wc -l` + if [ $WORDS -ne 4 ]; then + echo Got only $WORDS links for dangling-multi2 + return 1 + fi + WORDS=`TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(samaccountname=dangling-multi3)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted msDS-RevealedDSAs | grep msDS-RevealedDSAs | wc -l` + if [ $WORDS -ne 4 ]; then + echo Got only $WORDS links for dangling-multi3 + return 1 + fi +} + +dangling_multi_valued_check_equal_or_too_many() { + WORDS=`TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(samaccountname=dangling-multi1)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted msDS-RevealedDSAs | grep msDS-RevealedDSAs | wc -l` + if [ $WORDS -ne 4 ]; then + echo Got $WORDS links for dangling-multi1 + return 1 + fi + + WORDS=`TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(samaccountname=dangling-multi5)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted msDS-RevealedDSAs | grep msDS-RevealedDSAs | wc -l` + + if [ $WORDS -ne 0 ]; then + echo Got $WORDS links for dangling-multi5 + return 1 + fi + + WORDS=`TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb '(samaccountname=Administrator)' -s sub -b DC=release-4-5-0-pre1,DC=samba,DC=corp --show-deleted --reveal --sorted msDS-RevealedDSAs | grep msDS-RevealedDSAs | wc -l` + + if [ $WORDS -ne 2 ]; then + echo Got $WORDS links for Administrator + return 1 + fi +} + + if [ -d $release_dir ]; then testit $RELEASE undump testit "add_two_more_users" add_two_more_users @@ -216,6 +288,11 @@ if [ -d $release_dir ]; then testit "check_expected_after_objects" check_expected_after_objects testit "dangling_one_way" dangling_one_way testit "dbcheck_clean" dbcheck_clean + testit "dangling_multi_valued" dangling_multi_valued + testit "dangling_multi_valued_check_missing" dangling_multi_valued_check_missing + testit "dangling_multi_valued_check_equal_or_too_many" dangling_multi_valued_check_equal_or_too_many + # Currently this cannot pass + testit "dangling_multi_valued_dbcheck" dbcheck_clean else subunit_start_test $RELEASE subunit_skip_test $RELEASE < Date: Wed, 8 Mar 2017 15:16:49 +1300 Subject: [PATCH 23/26] tests/match_rules: Use system privilege for msDS-RevealedUsers Must be done before the systemOnly attribute is enforced. Signed-off-by: Garming Sam --- source4/selftest/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 6e648db..ba93454 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -595,7 +595,7 @@ planoldpythontestsuite("ad_dc_ntvfs", "dsdb_schema_info", extra_args=['-U"$DOMAIN/$DC_USERNAME%$DC_PASSWORD"']) plantestsuite_loadlist("samba4.urgent_replication.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(samba4srcdir, "dsdb/tests/python/urgent_replication.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.dirsync.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/dirsync.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) -plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) +plantestsuite_loadlist("samba4.ldap.match_rules.python", "ad_dc_ntvfs", [python, os.path.join(srcdir(), "lib/ldb-samba/tests/match_rules.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.notification.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/notification.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.sites.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/sites.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) -- 1.9.1 From ee497a9e97553c7e6661b3b0c4efd0e80688d824 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Tue, 7 Mar 2017 12:30:09 +1300 Subject: [PATCH 24/26] objectclass_attrs: Restrict systemOnly attributes This allows restriction of auditing attributes from being wiped. Modifications of the RID Set must be done as SYSTEM. Signed-off-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 28 +++++++++++++++++++++- source4/dsdb/samdb/ldb_modules/ridalloc.c | 8 ++++--- .../dsdb/samdb/ldb_modules/wscript_build_server | 2 +- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c index e239fb9..83894b7 100644 --- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c +++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c @@ -36,6 +36,7 @@ #include "includes.h" #include "ldb_module.h" #include "dsdb/samdb/samdb.h" +#include "dsdb/samdb/ldb_modules/util.h" struct oc_context { @@ -214,7 +215,32 @@ static int attr_handler(struct oc_context *ac) ldb_dn_get_linearized(msg->dn)); return LDB_ERR_UNWILLING_TO_PERFORM; } - + + if (ac->req->operation == LDB_MODIFY) { + if (attr->systemOnly) { + if (!ldb_request_get_control(ac->req, LDB_CONTROL_RELAX_OID) && + !ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK) && + !ldb_request_get_control(ac->req, DSDB_CONTROL_RESTORE_TOMBSTONE_OID) && + ldb_attr_cmp(attr->lDAPDisplayName, "objectClass") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "name") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "distinguishedName") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "msDS-AdditionalDnsHostName") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "wellKnownObjects") != 0) { + if (ldb_dn_compare_base(ldb_get_schema_basedn(ldb), msg->dn) != 0) { + struct ldb_control *as_system = ldb_request_get_control(ac->req, + LDB_CONTROL_AS_SYSTEM_OID); + if (!dsdb_module_am_system(ac->module) && !as_system) { + ldb_asprintf_errstring(ldb, + "objectclass_attrs: attribute '%s' on entry '%s' must can only be modified as system", + msg->elements[i].name, + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_CONSTRAINT_VIOLATION; + } + } + } + } + } + if (!(msg->elements[i].flags & LDB_FLAG_INTERNAL_DISABLE_VALIDATION)) { werr = attr->syntax->validate_ldb(&syntax_ctx, attr, &msg->elements[i]); diff --git a/source4/dsdb/samdb/ldb_modules/ridalloc.c b/source4/dsdb/samdb/ldb_modules/ridalloc.c index 730272a..abfe14a 100644 --- a/source4/dsdb/samdb/ldb_modules/ridalloc.c +++ b/source4/dsdb/samdb/ldb_modules/ridalloc.c @@ -387,7 +387,9 @@ static int ridalloc_create_rid_set_ntds(struct ldb_module *module, TALLOC_CTX *m } msg->elements[0].flags = LDB_FLAG_MOD_ADD; - ret = dsdb_module_modify(module, msg, DSDB_FLAG_NEXT_MODULE, parent); + ret = dsdb_module_modify(module, msg, + DSDB_FLAG_NEXT_MODULE|DSDB_FLAG_AS_SYSTEM, + parent); if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb, "Failed to add rIDSetReferences to %s - %s", ldb_dn_get_linearized(msg->dn), @@ -677,7 +679,7 @@ int ridalloc_allocate_rid(struct ldb_module *module, uint32_t *rid, struct ldb_r return ret; } - ret = dsdb_module_modify(module, msg, DSDB_FLAG_NEXT_MODULE, parent); + ret = dsdb_module_modify(module, msg, DSDB_FLAG_NEXT_MODULE|DSDB_FLAG_AS_SYSTEM, parent); if (ret != LDB_SUCCESS) { talloc_free(tmp_ctx); return ret; @@ -807,7 +809,7 @@ int ridalloc_allocate_rid_pool_fsmo(struct ldb_module *module, struct dsdb_fsmo_ return ret; } - ret = dsdb_module_modify(module, msg, DSDB_FLAG_NEXT_MODULE, parent); + ret = dsdb_module_modify(module, msg, DSDB_FLAG_NEXT_MODULE|DSDB_FLAG_AS_SYSTEM, parent); if (ret != LDB_SUCCESS) { ldb_asprintf_errstring(ldb, "Failed to modify RID Set object %s - %s", ldb_dn_get_linearized(rid_set_dn), ldb_errstring(ldb)); diff --git a/source4/dsdb/samdb/ldb_modules/wscript_build_server b/source4/dsdb/samdb/ldb_modules/wscript_build_server index 8131566..41b3fe7 100644 --- a/source4/dsdb/samdb/ldb_modules/wscript_build_server +++ b/source4/dsdb/samdb/ldb_modules/wscript_build_server @@ -224,7 +224,7 @@ bld.SAMBA_MODULE('ldb_objectclass_attrs', subsystem='ldb', init_function='ldb_objectclass_attrs_module_init', module_init_name='ldb_init_module', - deps='talloc samdb samba-util', + deps='talloc samdb DSDB_MODULE_HELPERS samba-util', internal_module=False, ) -- 1.9.1 From 3d6326895cd98c3b8435a17389e93b35464063ca Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 8 Mar 2017 16:45:44 +1300 Subject: [PATCH 25/26] objectclass_attrs: Add comments linking back to ADTS Signed-off-by: Garming Sam --- source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 54 ++++++++++++++-------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c index 83894b7..f28c7fb 100644 --- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c +++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c @@ -216,26 +216,40 @@ static int attr_handler(struct oc_context *ac) return LDB_ERR_UNWILLING_TO_PERFORM; } - if (ac->req->operation == LDB_MODIFY) { - if (attr->systemOnly) { - if (!ldb_request_get_control(ac->req, LDB_CONTROL_RELAX_OID) && - !ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK) && - !ldb_request_get_control(ac->req, DSDB_CONTROL_RESTORE_TOMBSTONE_OID) && - ldb_attr_cmp(attr->lDAPDisplayName, "objectClass") != 0 && - ldb_attr_cmp(attr->lDAPDisplayName, "name") != 0 && - ldb_attr_cmp(attr->lDAPDisplayName, "distinguishedName") != 0 && - ldb_attr_cmp(attr->lDAPDisplayName, "msDS-AdditionalDnsHostName") != 0 && - ldb_attr_cmp(attr->lDAPDisplayName, "wellKnownObjects") != 0) { - if (ldb_dn_compare_base(ldb_get_schema_basedn(ldb), msg->dn) != 0) { - struct ldb_control *as_system = ldb_request_get_control(ac->req, - LDB_CONTROL_AS_SYSTEM_OID); - if (!dsdb_module_am_system(ac->module) && !as_system) { - ldb_asprintf_errstring(ldb, - "objectclass_attrs: attribute '%s' on entry '%s' must can only be modified as system", - msg->elements[i].name, - ldb_dn_get_linearized(msg->dn)); - return LDB_ERR_CONSTRAINT_VIOLATION; - } + /* + * Enforce systemOnly checks from [ADTS] 3.1.1.5.3.2 + * Constraints in Modify Operation + */ + if (ac->req->operation == LDB_MODIFY && attr->systemOnly) { + /* + * Allow dbcheck and relax to bypass. objectClass, name + * and distinguishedName are generally handled + * elsewhere. + * + * The remaining cases, undelete, msDS-AdditionalDnsHostName + * and wellKnownObjects are documented in the specification. + */ + if (!ldb_request_get_control(ac->req, LDB_CONTROL_RELAX_OID) && + !ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK) && + !ldb_request_get_control(ac->req, DSDB_CONTROL_RESTORE_TOMBSTONE_OID) && + ldb_attr_cmp(attr->lDAPDisplayName, "objectClass") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "name") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "distinguishedName") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "msDS-AdditionalDnsHostName") != 0 && + ldb_attr_cmp(attr->lDAPDisplayName, "wellKnownObjects") != 0) { + /* + * Comparison against base schema DN is used as a substitute for + * fschemaUpgradeInProgress and other specific schema checks. + */ + if (ldb_dn_compare_base(ldb_get_schema_basedn(ldb), msg->dn) != 0) { + struct ldb_control *as_system = ldb_request_get_control(ac->req, + LDB_CONTROL_AS_SYSTEM_OID); + if (!dsdb_module_am_system(ac->module) && !as_system) { + ldb_asprintf_errstring(ldb, + "objectclass_attrs: attribute '%s' on entry '%s' must can only be modified as system", + msg->elements[i].name, + ldb_dn_get_linearized(msg->dn)); + return LDB_ERR_CONSTRAINT_VIOLATION; } } } -- 1.9.1 From 0a8fbece76742602cd6f161c8492086dabc48593 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Tue, 7 Mar 2017 15:42:59 +1300 Subject: [PATCH 26/26] tests/dbcheck-links: remove spurious sleeping Signed-off-by: Garming Sam --- testprogs/blackbox/dbcheck-links.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/testprogs/blackbox/dbcheck-links.sh b/testprogs/blackbox/dbcheck-links.sh index 9376e2a..fb66d14 100755 --- a/testprogs/blackbox/dbcheck-links.sh +++ b/testprogs/blackbox/dbcheck-links.sh @@ -103,7 +103,6 @@ add_dangling_link() { if [ "$?" != "0" ]; then return 1 fi - sleep 6 ldif=$release_dir/delete-only-backlink.ldif TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-5-0-PRE1,DC%3DSAMBA,DC%3DCORP.ldb $ldif -- 1.9.1