The branch, master has been updated via 256684c7a86 join: Use a specific attribute order for the DsAddEntry nTDSDSA object via 630857c71ef traffic_replay: Avoid DB full scans in LDAP searches via 7abfa6778f3 traffic replay test: Populate total_converations and instance_id via 6d6fe499387 traffic replay: Store the instance id in the replay context via 6b1f9b4a5ff traffic_replay: Make use of SCOPE_BASE explicit via 9f504fd5a32 traffic_replay: Store total conversations on the replay context from a073799ded5 nfs4_acls: Use fsp stat buffer in smb_fget_nt_acl_nfs4
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 256684c7a86301d26d6cf7298fb70e647bf45cf5 Author: Tim Beale <timbe...@catalyst.net.nz> Date: Wed Jul 24 11:00:01 2019 +1200 join: Use a specific attribute order for the DsAddEntry nTDSDSA object Joining a Windows domain can throw an error if the HasMasterNCs attribute occurs before msDS-HasMasterNCs. This patch changes the attribute order so that msDS-HasMasterNCs is always first. Previously on python2, the dictionary hash order was arbitrary but constant. By luck, msDS-HasMasterNCs was always before HasMasterNCs, so we never noticed any problem. With python3, the dictionary hash order now changes everytime you run the command, so the order is unpredictable. To enforce a order, we can change to use an OrderedDict, which will return the keys in the order they're added. I've asked Microsoft to clarify the protocol requirement here WRT attribute order. However, in the meantime we may as well fix the problem for users. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14046 RN: When trying to join a Windows domain (with functional level 2008R2) as an AD domain controller, the 'samba-tool domain join' command could throw a python exception: 'RuntimeError ("DsAddEntry failed")'. When this problem occurred, you would also see the message "DsAddEntry failed with status WERR_ACCESS_DENIED info (8363, 'WERR_DS_NO_CROSSREF_FOR_NC')" in the command output. This issue has now been resolved. Note that this problem would only occur on Samba v4.10 when using the Python3 packages. Signed-off-by: Tim Beale <timbe...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Wed Jul 24 04:18:21 UTC 2019 on sn-devel-184 commit 630857c71eff6332a6c94292ee840fa86a727f10 Author: Tim Beale <timbe...@catalyst.net.nz> Date: Thu Jun 20 09:20:09 2019 +1200 traffic_replay: Avoid DB full scans in LDAP searches When generating LDAP search traffic, a full DB scan can be very costly. Avoiding full-scan LDAP searches means that we can run traffic_replay against a 100K user DB and get some sane results. Because the traffic_learner doesn't record the LDAP search filter at all, the traffic_replay LDAP searches default to being full scans. Doing full scans meant that the LDAP search was usually the first packet type to exceed the max latency and fail the test. It could also skew results for the other packet types by creating big demands on memory/CPU/ DB-lock-time. It's hard to know for sure exactly what real-world LDAP searches will look like, but let's assume full scan searches will be fairly rare. In traffic-model files we've collected previously, some of the attributes are fairly unique (e.g. pKIExtendedKeyUsage), and as there are some LDAP queries specified in MS specs (such as MS-GPOL and MS-WCCE), it allows us to infer what the search filter might be. Signed-off-by: Tim Beale <timbe...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 7abfa6778f309bc3c1cfdb45042f708e81cfad9d Author: Gary Lockyer <g...@catalyst.net.nz> Date: Thu Jul 18 15:29:26 2019 +1200 traffic replay test: Populate total_converations and instance_id Ensure that the total_conversations and instance_id attributes are assigned a value in the replay contexts passed to test cases. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 6d6fe4993876d3374ae7ffeaf55d660ffa6b450b Author: Gary Lockyer <g...@catalyst.net.nz> Date: Thu Jul 18 13:39:20 2019 +1200 traffic replay: Store the instance id in the replay context Store the traffic runner instance id in the replay context. Will be used in subsequent commits. Signed-off-by: Gary Lockyer <g...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 6b1f9b4a5ff5450b0858d27dcc1af3021694be7d Author: Tim Beale <timbe...@catalyst.net.nz> Date: Thu Jun 13 16:18:27 2019 +1200 traffic_replay: Make use of SCOPE_BASE explicit i.e. avoid hard-coded numbers. Signed-off-by: Tim Beale <timbe...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9f504fd5a320edcbd3d763d4e0464f3cf7d270e1 Author: Tim Beale <timbe...@catalyst.net.nz> Date: Thu Jun 13 16:04:46 2019 +1200 traffic_replay: Store total conversations on the replay context This is useful info to know, and will be used in subsequent commits. Signed-off-by: Tim Beale <timbe...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/emulate/traffic.py | 64 +++++++++++++++++++++++++++- python/samba/emulate/traffic_packets.py | 10 ++++- python/samba/join.py | 23 +++++++--- python/samba/tests/emulate/traffic_packet.py | 4 +- script/traffic_replay | 3 +- 5 files changed, 92 insertions(+), 12 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/emulate/traffic.py b/python/samba/emulate/traffic.py index 7f720c98671..d0a2ffc8f2c 100644 --- a/python/samba/emulate/traffic.py +++ b/python/samba/emulate/traffic.py @@ -355,6 +355,7 @@ class ReplayContext(object): server=None, lp=None, creds=None, + total_conversations=None, badpassword_frequency=None, prefer_kerberos=None, tempdir=None, @@ -362,7 +363,8 @@ class ReplayContext(object): ou=None, base_dn=None, domain=os.environ.get("DOMAIN"), - domain_sid=None): + domain_sid=None, + instance_id=None): self.server = server self.netlogon_connection = None self.creds = creds @@ -378,6 +380,7 @@ class ReplayContext(object): self.global_tempdir = tempdir self.domain_sid = domain_sid self.realm = lp.get('realm') + self.instance_id = instance_id # Bad password attempt controls self.badpassword_frequency = badpassword_frequency @@ -389,6 +392,7 @@ class ReplayContext(object): self.last_drsuapi_bad = False self.last_netlogon_bad = False self.last_samlogon_bad = False + self.total_conversations = total_conversations self.generate_ldap_search_tables() def generate_ldap_search_tables(self): @@ -441,6 +445,63 @@ class ReplayContext(object): self.dn_map = dn_map self.attribute_clue_map = attribute_clue_map + # pre-populate DN-based search filters (it's simplest to generate them + # once, when the test starts). These are used by guess_search_filter() + # to avoid full-scans + self.search_filters = {} + + # lookup all the GPO DNs + res = db.search(db.domain_dn(), scope=ldb.SCOPE_SUBTREE, attrs=['dn'], + expression='(objectclass=groupPolicyContainer)') + gpos_by_dn = "" + for msg in res: + gpos_by_dn += "(distinguishedName={0})".format(msg['dn']) + + # a search for the 'gPCFileSysPath' attribute is probably a GPO search + # (as per the MS-GPOL spec) which searches for GPOs by DN + self.search_filters['gPCFileSysPath'] = "(|{0})".format(gpos_by_dn) + + # likewise, a search for gpLink is probably the Domain SOM search part + # of the MS-GPOL, in which case it's looking up a few OUs by DN + ou_str = "" + for ou in ["Domain Controllers,", "traffic_replay,", ""]: + ou_str += "(distinguishedName={0}{1})".format(ou, db.domain_dn()) + self.search_filters['gpLink'] = "(|{0})".format(ou_str) + + # The CEP Web Service can query the AD DC to get pKICertificateTemplate + # objects (as per MS-WCCE) + self.search_filters['pKIExtendedKeyUsage'] = \ + '(objectCategory=pKICertificateTemplate)' + + # assume that anything querying the usnChanged is some kind of + # synchronization tool, e.g. AD Change Detection Connector + res = db.search('', scope=ldb.SCOPE_BASE, attrs=['highestCommittedUSN']) + self.search_filters['usnChanged'] = \ + '(usnChanged>={0})'.format(res[0]['highestCommittedUSN']) + + # The traffic_learner script doesn't preserve the LDAP search filter, and + # having no filter can result in a full DB scan. This is costly for a large + # DB, and not necessarily representative of real world traffic. As there + # several standard LDAP queries that get used by AD tools, we can apply + # some logic and guess what the search filter might have been originally. + def guess_search_filter(self, attrs, dn_sig, dn): + + # there are some standard spec-based searches that query fairly unique + # attributes. Check if the search is likely one of these + for key in self.search_filters.keys(): + if key in attrs: + return self.search_filters[key] + + # if it's the top-level domain, assume we're looking up a single user, + # e.g. like powershell Get-ADUser or a similar tool + if dn_sig == 'DC,DC': + random_user_id = random.random() % self.total_conversations + account_name = user_name(self.instance_id, random_user_id) + return '(&(sAMAccountName=%s)(objectClass=user))' % account_name + + # otherwise just return everything in the sub-tree + return '(objectClass=*)' + def generate_process_local_config(self, account, conversation): self.ldap_connections = [] self.dcerpc_connections = [] @@ -1607,6 +1668,7 @@ def replay(conversation_seq, context = ReplayContext(server=host, creds=creds, lp=lp, + total_conversations=len(conversation_seq), **kwargs) if len(accounts) < len(conversation_seq): diff --git a/python/samba/emulate/traffic_packets.py b/python/samba/emulate/traffic_packets.py index 518bffac390..a585482ccd4 100644 --- a/python/samba/emulate/traffic_packets.py +++ b/python/samba/emulate/traffic_packets.py @@ -37,7 +37,7 @@ from samba.ntstatus import ( ) import samba import dns.resolver - +from ldb import SCOPE_BASE def uint32(v): return ctypes.c_uint32(v).value @@ -329,12 +329,18 @@ def packet_ldap_3(packet, conversation, context): (scope, dn_sig, filter, attrs, extra, desc, oid) = packet.extra if not scope: - scope = 0 + scope = SCOPE_BASE samdb = context.get_ldap_connection() dn = context.get_matching_dn(dn_sig) + # try to guess the search expression (don't bother for base searches, as + # they're only looking up a single object) + if (filter is None or filter is '') and scope != SCOPE_BASE: + filter = context.guess_search_filter(attrs, dn_sig, dn) + samdb.search(dn, + expression=filter, scope=int(scope), attrs=attrs.split(','), controls=["paged_results:1:1000"]) diff --git a/python/samba/join.py b/python/samba/join.py index ac4346c62a3..40920f4f8e5 100644 --- a/python/samba/join.py +++ b/python/samba/join.py @@ -48,6 +48,7 @@ import time import re import os import tempfile +from collections import OrderedDict from samba.compat import text_type from samba.compat import get_string from samba.netcmd import CommandError @@ -555,11 +556,14 @@ class DCJoinContext(object): '''return the ntdsdsa object to add''' print("Adding %s" % ctx.ntds_dn) - rec = { - "dn": ctx.ntds_dn, - "objectclass": "nTDSDSA", - "systemFlags": str(samba.dsdb.SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE), - "dMDLocation": ctx.schema_dn} + + # When joining Windows, the order of certain attributes (mostly only + # msDS-HasMasterNCs and HasMasterNCs) seems to matter + rec = OrderedDict([ + ("dn", ctx.ntds_dn), + ("objectclass", "nTDSDSA"), + ("systemFlags", str(samba.dsdb.SYSTEM_FLAG_DISALLOW_MOVE_ON_DELETE)), + ("dMDLocation", ctx.schema_dn)]) nc_list = [ctx.base_dn, ctx.config_dn, ctx.schema_dn] @@ -575,12 +579,17 @@ class DCJoinContext(object): rec["options"] = "37" else: rec["objectCategory"] = "CN=NTDS-DSA,%s" % ctx.schema_dn + + # Note that Windows seems to have an undocumented requirement that + # the msDS-HasMasterNCs attribute occurs before HasMasterNCs + if ctx.behavior_version >= samba.dsdb.DS_DOMAIN_FUNCTION_2003: + rec["msDS-HasMasterNCs"] = ctx.full_nc_list + rec["HasMasterNCs"] = [] for nc in nc_list: if nc in ctx.full_nc_list: rec["HasMasterNCs"].append(nc) - if ctx.behavior_version >= samba.dsdb.DS_DOMAIN_FUNCTION_2003: - rec["msDS-HasMasterNCs"] = ctx.full_nc_list + rec["options"] = "1" rec["invocationId"] = ndr_pack(ctx.invocation_id) diff --git a/python/samba/tests/emulate/traffic_packet.py b/python/samba/tests/emulate/traffic_packet.py index 56b126759b2..6533c679ea4 100644 --- a/python/samba/tests/emulate/traffic_packet.py +++ b/python/samba/tests/emulate/traffic_packet.py @@ -55,7 +55,9 @@ class TrafficEmulatorPacketTests(samba.tests.TestCase): creds=self.credentials, tempdir=self.tempdir, ou=traffic.ou_name(self.ldb, 1), - domain_sid=self.domain_sid) + domain_sid=self.domain_sid, + total_conversations=3, + instance_id=1) self.conversation = traffic.Conversation() self.conversation.conversation_id = 1 diff --git a/script/traffic_replay b/script/traffic_replay index 0d74c876d12..d29f0a9839c 100755 --- a/script/traffic_replay +++ b/script/traffic_replay @@ -416,7 +416,8 @@ def main(): ou=traffic.ou_name(ldb, opts.instance_id), tempdir=tempdir, stop_on_any_error=opts.stop_on_any_error, - domain_sid=ldb.get_domain_sid()) + domain_sid=ldb.get_domain_sid(), + instance_id=opts.instance_id) if opts.timing_data == '-': timing_dest = sys.stdout -- Samba Shared Repository