From 1e1c5d3909d99d5cea5abaae252f9f163fd8d11b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 9 Jun 2022 13:16:31 +1200 Subject: [PATCH 01/13] pytest: add file removal helpers for TestCaseInTempDir In several places we end a test by deleting a number of files and directories, but we do it rather haphazardly with unintentionally differing error handling. For example, in some tests we currently have something like: try: shutil.rmtree(os.path.join(self.tempdir, "a")) os.remove(os.path.join(self.tempdir, "b")) shutil.rmtree(os.path.join(self.tempdir, "c")) except Exception: pass where if, for example, the removal of "b" fails, the removal of "c" will not be attempted. That will result in the tearDown method raising an exception, and we're no better off. If the above code is replaced with self.rm_files('b') self.rm_dirs('a', 'c') the failure to remove 'b' will cause a test error, *unless* the failure was due to a FileNotFoundError (a.k.a. an OSError with errno ENOENT), in which case we ignore it, as was probably the original intention. If on the other hand, we have self.rm_files('b', must_exist=True) self.rm_dirs('a', 'c') then the FileNotFoundError causes a failure (not an error). We take a little bit of care to stay within self.tempdir, to protect test authors who accidentally write something like `self.rm_dirs('/')`. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 2359741b2854a8de9d151fe189be80a4bd087ff9) --- python/samba/tests/__init__.py | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py index 3bb7995052c..e37ceac9bc9 100644 --- a/python/samba/tests/__init__.py +++ b/python/samba/tests/__init__.py @@ -37,6 +37,7 @@ import samba.dcerpc.base from random import randint from random import SystemRandom from contextlib import contextmanager +import shutil import string try: from samba.samdb import SamDB @@ -295,6 +296,40 @@ class TestCaseInTempDir(TestCase): print("could not remove temporary file: %s" % e, file=sys.stderr) + def rm_files(self, *files, allow_missing=False, _rm=os.remove): + """Remove listed files from the temp directory. + + The files must be true files in the directory itself, not in + sub-directories. + + By default a non-existent file will cause a test failure (or + error if used outside a test in e.g. tearDown), but if + allow_missing is true, the absence will be ignored. + """ + for f in files: + path = os.path.join(self.tempdir, f) + + # os.path.join will happily step out of the tempdir, + # so let's just check. + if os.path.dirname(path) != self.tempdir: + raise ValueError("{path} might be outside {self.tempdir}") + + try: + _rm(path) + except FileNotFoundError as e: + if not allow_missing: + raise AssertionError(f"{f} not in {self.tempdir}: {e}") + + print(f"{f} not in {self.tempdir}") + + def rm_dirs(self, *dirs, allow_missing=False): + """Remove listed directories from temp directory. + + This works like rm_files, but only removes directories, + including their contents. + """ + self.rm_files(*dirs, allow_missing=allow_missing, _rm=shutil.rmtree) + def env_loadparm(): lp = param.LoadParm() -- 2.25.1 From 2c901ee22ea9bd10a383df5fd5e2b1627e84666c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:19:28 +1200 Subject: [PATCH 02/13] pytest/downgradedatabase: use TestCaseInTempDir.rm_files BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 85bc1552e3919d049d39a065824172a24933d38b) --- python/samba/tests/blackbox/downgradedatabase.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/python/samba/tests/blackbox/downgradedatabase.py b/python/samba/tests/blackbox/downgradedatabase.py index f6da011ede4..8d80ef6e804 100644 --- a/python/samba/tests/blackbox/downgradedatabase.py +++ b/python/samba/tests/blackbox/downgradedatabase.py @@ -18,7 +18,6 @@ from samba.tests import BlackboxTestCase import os import ldb -import shutil from subprocess import check_output from samba.samdb import SamDB @@ -57,13 +56,12 @@ class DowngradeTestBase(BlackboxTestCase): self.dbs.append(self.sam_path) def tearDown(self): - shutil.rmtree(os.path.join(self.tempdir, "private")) - shutil.rmtree(os.path.join(self.tempdir, "etc")) - shutil.rmtree(os.path.join(self.tempdir, "state")) - shutil.rmtree(os.path.join(self.tempdir, "bind-dns")) - shutil.rmtree(os.path.join(self.tempdir, "msg.lock")) - os.unlink(os.path.join(self.tempdir, "names.tdb")) - os.unlink(os.path.join(self.tempdir, "gencache.tdb")) + self.rm_dirs("private", + "etc", + "state", + "bind-dns", + "msg.lock") + self.rm_files("names.tdb", "gencache.tdb") super(DowngradeTestBase, self).tearDown() # Parse out the comments above each record that ldbdump produces -- 2.25.1 From f920df9315e8715b7d36a2ac1c65deb913e98e16 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:20:41 +1200 Subject: [PATCH 03/13] pytest/samdb_api: use TestCaseInTempDir.rm_files BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 4e3dabad0be0900a203896c2c2acb270d31b0a42) --- python/samba/tests/samdb_api.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/python/samba/tests/samdb_api.py b/python/samba/tests/samdb_api.py index a7260180187..710d0bc310f 100644 --- a/python/samba/tests/samdb_api.py +++ b/python/samba/tests/samdb_api.py @@ -29,15 +29,7 @@ class SamDBApiTestCase(TestCaseInTempDir): super(SamDBApiTestCase, self).setUp() def tearDown(self): - try: - os.remove(self.tempdir + "/test.db") - except OSError as e: - self.assertEqual(e.errno, errno.ENOENT) - - try: - os.remove(self.tempdir + "/existing.db") - except OSError as e: - self.assertEqual(e.errno, errno.ENOENT) + self.rm_files("test.db", "existing.db", allow_missing=True) super(SamDBApiTestCase, self).tearDown() -- 2.25.1 From 51b09ef76deb4c338104df1619c062666329b784 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:21:16 +1200 Subject: [PATCH 04/13] pytest/join: use TestCaseInTempDir.rm_files/dirs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 7455c53fa4f7871b3980f820d22b0fd411195704) --- python/samba/tests/join.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/python/samba/tests/join.py b/python/samba/tests/join.py index db9d8a892b7..da34171da28 100644 --- a/python/samba/tests/join.py +++ b/python/samba/tests/join.py @@ -74,10 +74,8 @@ class JoinTestCase(DNSTKeyTest): if paths is not None: shutil.rmtree(paths.private_dir) shutil.rmtree(paths.state_dir) - shutil.rmtree(os.path.join(self.tempdir, "etc")) - shutil.rmtree(os.path.join(self.tempdir, "msg.lock")) - os.unlink(os.path.join(self.tempdir, "names.tdb")) - shutil.rmtree(os.path.join(self.tempdir, "bind-dns")) + self.rm_dirs("etc", "msg.lock", "bind-dns") + self.rm_files("names.tdb") self.join_ctx.cleanup_old_join(force=True) -- 2.25.1 From b0ec97bbcad154c839fa28d8b6c10f9160cf4827 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:22:24 +1200 Subject: [PATCH 05/13] pytest/samdb: use TestCaseInTempDir.rm_files/.rm_dirs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 251360d6e58986dd53f0317319544e930dc61444) --- python/samba/tests/samdb.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/samdb.py b/python/samba/tests/samdb.py index 834c5a204a6..f7697f83fdc 100644 --- a/python/samba/tests/samdb.py +++ b/python/samba/tests/samdb.py @@ -19,7 +19,6 @@ import logging import os -import shutil from samba.auth import system_session from samba.provision import provision @@ -54,11 +53,8 @@ class SamDBTestCase(TestCaseInTempDir): self.lp = self.result.lp def tearDown(self): - for f in ['names.tdb']: - os.remove(os.path.join(self.tempdir, f)) - - for d in ['etc', 'msg.lock', 'private', 'state', 'bind-dns']: - shutil.rmtree(os.path.join(self.tempdir, d)) + self.rm_files('names.tdb') + self.rm_dirs('etc', 'msg.lock', 'private', 'state', 'bind-dns') super(SamDBTestCase, self).tearDown() -- 2.25.1 From 45a916775659ed141b91cc817b8b45439fc6e4a8 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 8 Jun 2022 19:53:57 +1200 Subject: [PATCH 06/13] pytest/samba_tool_drs: use TestCaseInTempDir.rm_files/.rm_dirs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 3f0aab45c81c9f9b6b87eb68bc785902619dc10d) --- source4/torture/drs/python/samba_tool_drs.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/source4/torture/drs/python/samba_tool_drs.py b/source4/torture/drs/python/samba_tool_drs.py index ac97162e23d..8538c3041a4 100644 --- a/source4/torture/drs/python/samba_tool_drs.py +++ b/source4/torture/drs/python/samba_tool_drs.py @@ -19,7 +19,6 @@ """Blackbox tests for samba-tool drs.""" import samba.tests -import shutil import os import ldb import drs_base @@ -42,15 +41,9 @@ class SambaToolDrsTests(drs_base.DrsBaseTestCase): self._enable_inbound_repl(self.dnsname_dc1) self._enable_inbound_repl(self.dnsname_dc2) - try: - shutil.rmtree(os.path.join(self.tempdir, "private")) - shutil.rmtree(os.path.join(self.tempdir, "etc")) - shutil.rmtree(os.path.join(self.tempdir, "msg.lock")) - os.remove(os.path.join(self.tempdir, "names.tdb")) - shutil.rmtree(os.path.join(self.tempdir, "state")) - shutil.rmtree(os.path.join(self.tempdir, "bind-dns")) - except Exception: - pass + self.rm_files('names.tdb', allow_missing=True) + self.rm_dirs('etc', 'msg.lock', 'private', 'state', 'bind-dns', + allow_missing=True) super(SambaToolDrsTests, self).tearDown() -- 2.25.1 From a16cb6c2ee9b7a85578e7afc871fc21498afb3ff Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:23:32 +1200 Subject: [PATCH 07/13] pytest/samba_tool_drs_no_dns: use TestCaseInTempDir.rm_files/.rm_dirs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15191 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Reviewed-by: Noel Power (cherry picked from commit 24f7d71416753b792d6fe029da6f366adb10383e) --- .../torture/drs/python/samba_tool_drs_no_dns.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/source4/torture/drs/python/samba_tool_drs_no_dns.py b/source4/torture/drs/python/samba_tool_drs_no_dns.py index 33b43b48d05..fb1551a1b43 100644 --- a/source4/torture/drs/python/samba_tool_drs_no_dns.py +++ b/source4/torture/drs/python/samba_tool_drs_no_dns.py @@ -24,7 +24,6 @@ Adapted from samba_tool_drs.py """ import samba.tests -import shutil import os import ldb import drs_base @@ -47,16 +46,9 @@ class SambaToolDrsNoDnsTests(drs_base.DrsBaseTestCase): def tearDown(self): self._enable_inbound_repl(self.dnsname_dc1) - - try: - shutil.rmtree(os.path.join(self.tempdir, "private")) - shutil.rmtree(os.path.join(self.tempdir, "etc")) - shutil.rmtree(os.path.join(self.tempdir, "msg.lock")) - os.remove(os.path.join(self.tempdir, "names.tdb")) - shutil.rmtree(os.path.join(self.tempdir, "state")) - shutil.rmtree(os.path.join(self.tempdir, "bind-dns")) - except Exception: - pass + self.rm_files('names.tdb', allow_missing=True) + self.rm_dirs('etc', 'msg.lock', 'private', 'state', 'bind-dns', + allow_missing=True) super(SambaToolDrsNoDnsTests, self).tearDown() -- 2.25.1 From f34e05d783f4de935421f161d6f093a52c994959 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Sep 2022 09:36:45 +1200 Subject: [PATCH 08/13] selftest: Prepare for "old Samba" mode regarding getncchanges GET_ANC/GET_TGT The chgdcpass environment will emulate older verions of Samba that fail to implement DRSUAPI_DRS_GET_ANC correctly and totally fails to support DRSUAPI_DRS_GET_TGT. This will allow testing of a client-side fallback, allowing migration from sites that run very old Samba versions over DRSUAPI (currently the only option is to attempt an in-place upgrade). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit 62b426243f4eaa4978c249b6e6ce90d35aeaefe4) --- selftest/knownfail.d/samba-4.5-emulation | 2 ++ source4/selftest/tests.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 selftest/knownfail.d/samba-4.5-emulation diff --git a/selftest/knownfail.d/samba-4.5-emulation b/selftest/knownfail.d/samba-4.5-emulation new file mode 100644 index 00000000000..82959240588 --- /dev/null +++ b/selftest/knownfail.d/samba-4.5-emulation @@ -0,0 +1,2 @@ +# This fails as there is no second DC in this enviroment, so it is always the owner +samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_FSMONotOwner\(chgdcpass\) \ No newline at end of file diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index d415f270701..5cc46d703e1 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1511,11 +1511,6 @@ for env in ['vampire_dc', 'promoted_dc']: name="samba4.drs.repl_move.python(%s)" % env, environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) - planoldpythontestsuite(env, "getnc_exop", - extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], - name="samba4.drs.getnc_exop.python(%s)" % env, - environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, - extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) planoldpythontestsuite(env, "getnc_unpriv", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], name="samba4.drs.getnc_unpriv.python(%s)" % env, @@ -1532,6 +1527,15 @@ for env in ['vampire_dc', 'promoted_dc']: environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) +# Environment chgdcpass has the Samba 4.5 GET_ANC behaviour, which we +# set a knownfail to expect +for env in ['vampire_dc', 'promoted_dc', 'chgdcpass']: + planoldpythontestsuite(env, "getnc_exop", + extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], + name="samba4.drs.getnc_exop.python(%s)" % env, + environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, + extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) + for env in ['vampire_dc', 'promoted_dc', 'vampire_2000_dc']: planoldpythontestsuite(env, "repl_schema", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], -- 2.25.1 From cbf8464f32754a2062a40d1116bddf8e4612529b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 03:05:03 +0000 Subject: [PATCH 09/13] selftest: Add tests for GetNCChanges GET_ANC using samba-tool drs clone-dc-database This test, compared with the direct to RPC tests, will succeed, then fail once the server is changed to emulate Samba 4.5 and and again succeed once the python code changes to allow skipping the DRSUAPI_DRS_CRITICAL_ONLY step BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit 7ff743d65dcf27ffe0c6861720e8ce531bfa378d) --- source4/selftest/tests.py | 9 ++ .../drs/python/samba_tool_drs_critical.py | 98 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 source4/torture/drs/python/samba_tool_drs_critical.py diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 5cc46d703e1..3ee001e147d 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1460,6 +1460,9 @@ planoldpythontestsuite(env, "ridalloc_exop", # # That is why this test is run on the isolated environment and not on # those connected with ad_dc (vampiredc/promoteddc) +# +# The chgdcpass enviroment is likewise isolated and emulates Samba 4.5 +# with regard to GET_ANC env = 'schema_pair_dc' planoldpythontestsuite("%s:local" % env, "samba_tool_drs", @@ -1467,6 +1470,12 @@ planoldpythontestsuite("%s:local" % env, "samba_tool_drs", name="samba4.drs.samba_tool_drs.python(%s)" % env, environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) +for env in ['chgdcpass', 'schema_pair_dc']: + planoldpythontestsuite("%s:local" % env, "samba_tool_drs_critical", + extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], + name="samba4.drs.samba_tool_drs_critical.python(%s)" % env, + environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, + extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) planoldpythontestsuite(env, "getnc_schema", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], name="samba4.drs.getnc_schema.python(%s)" % env, diff --git a/source4/torture/drs/python/samba_tool_drs_critical.py b/source4/torture/drs/python/samba_tool_drs_critical.py new file mode 100644 index 00000000000..47f9e942dc9 --- /dev/null +++ b/source4/torture/drs/python/samba_tool_drs_critical.py @@ -0,0 +1,98 @@ +# Blackbox tests for "samba-tool drs" command +# Copyright (C) Kamen Mazdrashki 2011 +# Copyright (C) Andrew Bartlett 2017 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +"""Blackbox tests for samba-tool drs.""" + +import samba.tests +import os +import ldb +import drs_base +import random + +class SambaToolDrsTests(drs_base.DrsBaseTestCase): + """Blackbox test case for samba-tool drs.""" + + def setUp(self): + super(SambaToolDrsTests, self).setUp() + + self.dc1 = samba.tests.env_get_var_value("DC1") + self.dc2 = samba.tests.env_get_var_value("DC2") + + creds = self.get_credentials() + self.cmdline_creds = "-U%s/%s%%%s" % (creds.get_domain(), + creds.get_username(), creds.get_password()) + + def tearDown(self): + self._enable_inbound_repl(self.dnsname_dc1) + self._enable_inbound_repl(self.dnsname_dc2) + + self.rm_files('names.tdb', allow_missing=True) + self.rm_dirs('etc', 'msg.lock', 'private', 'state', 'bind-dns', + allow_missing=True) + + super(SambaToolDrsTests, self).tearDown() + + # This test is for the Samba 4.5 emulation servers (but runs + # against a normal server as well) that fail to correctly + # implement DRSUAPI_DRS_GET_ANC when DRSUAPI_DRS_CRITICAL_ONLY is + # set. + def test_samba_tool_drs_clone_dc_critical_object_chain(self): + """Tests 'samba-tool drs clone-dc-database' command with a Critical/non-critical/critical object chain.""" + + samdb = samba.tests.connect_samdb(self.dc1, lp=self.get_loadparm(), + credentials=self.get_credentials(), + ldap_only=True) + server_rootdse = samdb.search(base="", + scope=samba.tests.ldb.SCOPE_BASE)[0] + nc_name = server_rootdse["defaultNamingContext"][0] + server_ldap_service_name = str(server_rootdse["ldapServiceName"][0]) + server_realm = server_ldap_service_name.split(":")[0] + + not_critical_dn = f"OU=not-critical{random.randint(1, 10000000)},{nc_name}" + samdb.create_ou(not_critical_dn) + self.addCleanup(samdb.delete, + not_critical_dn) + domain_sid = samdb.get_domain_sid() + admin_sid = f"{domain_sid}-500" + samdb.rename(f"", + f"cn=administrator,{not_critical_dn}") + self.addCleanup(samdb.rename, + f"", + f"cn=administrator,cn=users,{nc_name}") + + try: + out = self.check_output("samba-tool drs clone-dc-database %s --server=%s %s --targetdir=%s" + % (server_realm, + self.dc1, + self.cmdline_creds, + self.tempdir)) + except samba.tests.BlackboxProcessError as e: + self.fail("Error calling samba-tool: %s" % e) + + local_samdb = samba.tests.connect_samdb("ldb://" + os.path.join(self.tempdir, "private", "sam.ldb"), + ldap_only=False, lp=self.get_loadparm()) + + # Check administrator was replicated and is in the right place + res = local_samdb.search(base=str(nc_name), + expression="(&(objectclass=user)(cn=administrator))", + attrs=[], scope=ldb.SCOPE_SUBTREE) + self.assertEquals(len(res), 1) + + admin_obj = res[0] + + self.assertEquals(admin_obj.dn, ldb.Dn(samdb, f"cn=administrator,{not_critical_dn}")) -- 2.25.1 From 7b14cd13e7e746f7d0a5b4aeac8860a83fd500a3 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 14:53:38 +1300 Subject: [PATCH 10/13] s4-rpc_server:getncchanges Add "old Samba" mode regarding GET_ANC/GET_TGT This emulates older verions of Samba that fail to implement DRSUAPI_DRS_GET_ANC correctly and totally fails to support DRSUAPI_DRS_GET_TGT. This will allow testing of a client-side fallback, allowing migration from sites that run very old Samba versions over DRSUAPI (currently the only option is to attempt an in-place upgrade). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit 314bc44fa9b8fc99c80bfcfff71f2cec67bbda36) --- source4/rpc_server/drsuapi/getncchanges.c | 52 +++++++++++++++++++---- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index c3330a622af..7f50587dd1e 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -60,6 +60,7 @@ struct drsuapi_getncchanges_state { struct GUID ncRoot_guid; bool is_schema_nc; bool is_get_anc; + bool broken_samba_4_5_get_anc_emulation; bool is_get_tgt; uint64_t min_usn; uint64_t max_usn; @@ -1019,6 +1020,17 @@ struct drsuapi_changed_objects { uint64_t usn; }; + +/* + sort the objects we send by tree order (Samba 4.5 emulation) + */ +static int site_res_cmp_anc_order(struct drsuapi_changed_objects *m1, + struct drsuapi_changed_objects *m2, + struct drsuapi_getncchanges_state *getnc_state) +{ + return ldb_dn_compare(m2->dn, m1->dn); +} + /* sort the objects we send first by uSNChanged */ @@ -2281,8 +2293,13 @@ static WERROR getncchanges_get_obj_to_send(const struct ldb_message *msg, * If required, also add any ancestors that the client may need to know * about before it can resolve this object. These get prepended to the * ret_obj_list so the client adds them first. + * + * We allow this to be disabled to permit testing of a + * client-side fallback for the broken behaviour in Samba 4.5 + * and earlier. */ - if (getnc_state->is_get_anc) { + if (getnc_state->is_get_anc + && !getnc_state->broken_samba_4_5_get_anc_emulation) { werr = getncchanges_add_ancestors(obj, msg->dn, mem_ctx, sam_ctx, getnc_state, schema, session_key, @@ -3097,9 +3114,35 @@ allowed: } } + if (req10->extended_op == DRSUAPI_EXOP_NONE) { + getnc_state->is_get_anc = + ((req10->replica_flags & DRSUAPI_DRS_GET_ANC) != 0); + if (getnc_state->is_get_anc + && lpcfg_parm_bool(dce_call->conn->dce_ctx->lp_ctx, + NULL, + "drs", + "broken_samba_4.5_get_anc_emulation", + false)) { + getnc_state->broken_samba_4_5_get_anc_emulation = true; + } + if (lpcfg_parm_bool(dce_call->conn->dce_ctx->lp_ctx, + NULL, + "drs", + "get_tgt_support", + true)) { + getnc_state->is_get_tgt = + ((req10->more_flags & DRSUAPI_DRS_GET_TGT) != 0); + } + } + /* RID_ALLOC returns 3 objects in a fixed order */ if (req10->extended_op == DRSUAPI_EXOP_FSMO_RID_ALLOC) { /* Do nothing */ + } else if (getnc_state->broken_samba_4_5_get_anc_emulation) { + LDB_TYPESAFE_QSORT(changes, + getnc_state->num_records, + getnc_state, + site_res_cmp_anc_order); } else { LDB_TYPESAFE_QSORT(changes, getnc_state->num_records, @@ -3123,13 +3166,6 @@ allowed: talloc_free(search_res); talloc_free(changes); - if (req10->extended_op == DRSUAPI_EXOP_NONE) { - getnc_state->is_get_anc = - ((req10->replica_flags & DRSUAPI_DRS_GET_ANC) != 0); - getnc_state->is_get_tgt = - ((req10->more_flags & DRSUAPI_DRS_GET_TGT) != 0); - } - /* * when using GET_ANC or GET_TGT, cache the objects that have * been already sent, to avoid sending them multiple times -- 2.25.1 From c8213e80e1c87cec4f705099c4a6484af716ced5 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 14:54:14 +1300 Subject: [PATCH 11/13] selftest: Enable "old Samba" mode regarding GET_ANC/GET_TGT The chgdcpass server now emulates older verions of Samba that fail to implement DRSUAPI_DRS_GET_ANC correctly and totally fails to support DRSUAPI_DRS_GET_TGT. We now show this is in effect by the fact that tests now fail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit b0bbc94d4124d63b1d5a35ccbc88ffd51d520ba0) --- selftest/knownfail.d/samba-4.5-emulation | 5 ++++- selftest/target/Samba4.pm | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail.d/samba-4.5-emulation b/selftest/knownfail.d/samba-4.5-emulation index 82959240588..37baa41822c 100644 --- a/selftest/knownfail.d/samba-4.5-emulation +++ b/selftest/knownfail.d/samba-4.5-emulation @@ -1,2 +1,5 @@ # This fails as there is no second DC in this enviroment, so it is always the owner -samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_FSMONotOwner\(chgdcpass\) \ No newline at end of file +samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_FSMONotOwner\(chgdcpass\) +# This fails because GET_ANC is now poorly implemented (matching Samba 4.5) +^samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_link_utdv_hwm\(chgdcpass\) +^samba4.drs.samba_tool_drs_critical.python\(chgdcpass\).samba_tool_drs_critical.SambaToolDrsTests.test_samba_tool_drs_clone_dc_critical_object_chain\(chgdcpass:local\) \ No newline at end of file diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index 4c263f55de4..b004042738a 100755 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -2053,10 +2053,22 @@ sub provision_chgdcpass($$) # This environment disallows the use of this password # (and also removes the default AD complexity checks) my $unacceptable_password = "Paßßword-widk3Dsle32jxdBdskldsk55klASKQ"; + + # This environment also sets some settings that are unusual, + # to test specific behaviours. In particular, this + # environment fails to correctly support DRSUAPI_DRS_GET_ANC + # like Samba before 4.5 and DRSUAPI_DRS_GET_TGT before 4.8 + # + # Additionally, disabling DRSUAPI_DRS_GET_TGT causes all links + # to be sent last (in the final chunk), which is like Samba + # before 4.8. + my $extra_smb_conf = " check password script = $self->{srcdir}/selftest/checkpassword_arg1.sh ${unacceptable_password} allow dcerpc auth level connect:lsarpc = yes dcesrv:max auth states = 8 + drs:broken_samba_4.5_get_anc_emulation = true + drs:get_tgt_support = false "; my $extra_provision_options = ["--dns-backend=BIND9_DLZ"]; my $ret = $self->provision($prefix, -- 2.25.1 From 5a6e6d2444c39a30a3e3ff64e866ad2ed86bf96f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 20 Sep 2022 13:37:30 +1200 Subject: [PATCH 12/13] s4-libnet: Add messages to object count mismatch failures This helps explain these better than WERR_GEN_FAILURE. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit 483c48f52d6ff5e8149ed12bfeb2b6608c946f01) --- source4/dsdb/repl/replicated_objects.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/source4/dsdb/repl/replicated_objects.c b/source4/dsdb/repl/replicated_objects.c index de61d8e1335..6a07a88961b 100644 --- a/source4/dsdb/repl/replicated_objects.c +++ b/source4/dsdb/repl/replicated_objects.c @@ -747,14 +747,25 @@ WERROR dsdb_replicated_objects_convert(struct ldb_context *ldb, /* Assuming we didn't skip or error, increment the number of objects */ out->num_objects++; } + + DBG_INFO("Proceesed %"PRIu32" DRS objects, saw %"PRIu32" objects " + "and expected %"PRIu32" objects\n", + out->num_objects, i, object_count); + out->objects = talloc_realloc(out, out->objects, struct dsdb_extended_replicated_object, out->num_objects); if (out->num_objects != 0 && out->objects == NULL) { + DBG_ERR("FAILURE: talloc_realloc() failed after " + "processing %"PRIu32" DRS objects!\n", + out->num_objects); talloc_free(out); return WERR_FOOBAR; } if (i != object_count) { + DBG_ERR("FAILURE: saw %"PRIu32" DRS objects, server said we " + "should expected to see %"PRIu32" objects!\n", + i, object_count); talloc_free(out); return WERR_FOOBAR; } -- 2.25.1 From 2bac2f7f9ea6a4b7bdcbbaad8938359770af994f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Sep 2022 17:10:24 +1200 Subject: [PATCH 13/13] python-drs: Add client-side debug and fallback for GET_ANC Samba 4.5 and earlier will fail to do GET_ANC correctly and will not replicate non-critical parents of objects with isCriticalSystemObject=TRUE when DRSUAPI_DRS_CRITICAL_ONLY is set. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15189 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall (cherry picked from commit bff2bc9c7d69ec2fbe9339c2353a0a846182f1ea) --- python/samba/drs_utils.py | 47 ++++++++++++++++++++- python/samba/join.py | 54 ++++++++++++++++++++---- selftest/knownfail.d/samba-4.5-emulation | 1 - 3 files changed, 90 insertions(+), 12 deletions(-) diff --git a/python/samba/drs_utils.py b/python/samba/drs_utils.py index feab89b0d8e..02a32e2f3a6 100644 --- a/python/samba/drs_utils.py +++ b/python/samba/drs_utils.py @@ -207,6 +207,44 @@ class drs_Replicate(object): supports_ext & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10 and (req.more_flags & drsuapi.DRSUAPI_DRS_GET_TGT) == 0) + @staticmethod + def _should_calculate_missing_anc_locally(error_code, req): + # If the error indicates we fail to resolve the parent object + # for a new object, then we assume we are replicating from a + # buggy server (Samba 4.5 and earlier) that doesn't really + # understand how to implement GET_ANC + + return ((error_code == werror.WERR_DS_DRA_MISSING_PARENT) and + (req.replica_flags & drsuapi.DRSUAPI_DRS_GET_ANC) != 0) + + + def _calculate_missing_anc_locally(self, ctr): + self.guids_seen = set() + + # walk objects in ctr, add to guid_seen as we see them + # note if an object doesn't have a parent + + object_to_check = ctr.first_object + + while True: + if object_to_check is None: + break + + self.guids_seen.add(str(object_to_check.object.identifier.guid)) + + if object_to_check.parent_object_guid is not None \ + and object_to_check.parent_object_guid \ + != misc.GUID("00000000-0000-0000-0000-000000000000") \ + and str(object_to_check.parent_object_guid) not in self.guids_seen: + obj_dn = ldb.Dn(self.samdb, object_to_check.object.identifier.dn) + parent_dn = obj_dn.parent() + print(f"Object {parent_dn} with " + f"GUID {object_to_check.parent_object_guid} " + "was not sent by the server in this chunk") + + object_to_check = object_to_check.next_object + + def process_chunk(self, level, ctr, schema, req_level, req, first_chunk): '''Processes a single chunk of received replication data''' # pass the replication into the py_net.c python bindings for processing @@ -329,8 +367,13 @@ class drs_Replicate(object): # of causing the DC to restart the replication from scratch) first_chunk = True continue - else: - raise e + + if self._should_calculate_missing_anc_locally(e.args[0], + req): + print("Missing parent object - calculating missing objects locally") + + self._calculate_missing_anc_locally(ctr) + raise e first_chunk = False num_objects += ctr.object_count diff --git a/python/samba/join.py b/python/samba/join.py index 4399367c817..df2eaf497ab 100644 --- a/python/samba/join.py +++ b/python/samba/join.py @@ -968,17 +968,53 @@ class DCJoinContext(object): destination_dsa_guid, rodc=ctx.RODC, replica_flags=ctx.replica_flags) if not ctx.subdomain: - # Replicate first the critical object for the basedn - if not ctx.domain_replica_flags & drsuapi.DRSUAPI_DRS_CRITICAL_ONLY: - print("Replicating critical objects from the base DN of the domain") - ctx.domain_replica_flags |= drsuapi.DRSUAPI_DRS_CRITICAL_ONLY + # Replicate first the critical objects for the basedn + + # We do this to match Windows. The default case is to + # do a critical objects replication, then a second + # with all objects. + + print("Replicating critical objects from the base DN of the domain") + try: repl.replicate(ctx.base_dn, source_dsa_invocation_id, destination_dsa_guid, rodc=ctx.RODC, - replica_flags=ctx.domain_replica_flags) - ctx.domain_replica_flags ^= drsuapi.DRSUAPI_DRS_CRITICAL_ONLY - repl.replicate(ctx.base_dn, source_dsa_invocation_id, - destination_dsa_guid, rodc=ctx.RODC, - replica_flags=ctx.domain_replica_flags) + replica_flags=ctx.domain_replica_flags | drsuapi.DRSUAPI_DRS_CRITICAL_ONLY) + except WERRORError as e: + + if e.args[0] == werror.WERR_DS_DRA_MISSING_PARENT: + ctx.logger.warning("First pass of replication with " + "DRSUAPI_DRS_CRITICAL_ONLY " + "not possible due to a missing parent object. " + "This is typical of a Samba " + "4.5 or earlier server. " + "We will replicate the all objects instead.") + else: + raise + + # Now replicate all the objects in the domain (unless + # we were run with --critical-only). + # + # Doing the replication of users as a second pass + # matches more closely the Windows behaviour, which is + # actually to do this on first startup. + # + # Use --critical-only if you want that (but you don't + # really, it is better to see any errors here). + if not ctx.domain_replica_flags & drsuapi.DRSUAPI_DRS_CRITICAL_ONLY: + try: + repl.replicate(ctx.base_dn, source_dsa_invocation_id, + destination_dsa_guid, rodc=ctx.RODC, + replica_flags=ctx.domain_replica_flags) + except WERRORError as e: + + if e.args[0] == werror.WERR_DS_DRA_MISSING_PARENT and \ + ctx.domain_replica_flags & drsuapi.DRSUAPI_DRS_CRITICAL_ONLY: + ctx.logger.warning("Replication with DRSUAPI_DRS_CRITICAL_ONLY " + "failed due to a missing parent object. " + "This may be a Samba 4.5 or earlier server " + "and is not compatible with --critical-only") + raise + print("Done with always replicated NC (base, config, schema)") # At this point we should already have an entry in the ForestDNS diff --git a/selftest/knownfail.d/samba-4.5-emulation b/selftest/knownfail.d/samba-4.5-emulation index 37baa41822c..1fc79361e40 100644 --- a/selftest/knownfail.d/samba-4.5-emulation +++ b/selftest/knownfail.d/samba-4.5-emulation @@ -2,4 +2,3 @@ samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_FSMONotOwner\(chgdcpass\) # This fails because GET_ANC is now poorly implemented (matching Samba 4.5) ^samba4.drs.getnc_exop.python\(chgdcpass\).getnc_exop.DrsReplicaSyncTestCase.test_link_utdv_hwm\(chgdcpass\) -^samba4.drs.samba_tool_drs_critical.python\(chgdcpass\).samba_tool_drs_critical.SambaToolDrsTests.test_samba_tool_drs_clone_dc_critical_object_chain\(chgdcpass:local\) \ No newline at end of file -- 2.25.1