From 365801164044164f4f9b0d826000cdcbb0ad4c9e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 4 Sep 2021 12:28:20 +1200 Subject: [PATCH 01/14] selftest: Split up targets for samba_tool_drs from samba_tool_drs_showrepl These now run in the disconnected sets schema_dc/schema_pair_dc and ad_dc/vampire_dc/promoted_dc. By aiming at different sets ofservers we can't cause cross-contamination in terms of which servers are listed as outbound connections. Also, by running the tests only once we reduce the chaces of trouble by half. Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison (cherry picked from commit e8b4599e0935290c5e59df9fd4f695ad8d6f361c) --- source4/selftest/tests.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 9e351f195fc..a62036f1e56 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1188,6 +1188,18 @@ planoldpythontestsuite(env, "ridalloc_exop", environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) +# This test can pollute the environment a little by creating and +# deleting DCs which can get into the replication state for a while. +# +# The setting of DC1 to $DC_SERVER means that it will join towards and +# operate on schema_dc. This matters most when running +# test_samba_tool_replicate_local as this sets up a full temp DC and +# does new replication to it, which can show up in the replication +# topology. +# +# That is why this test is run on the isolated environment and not on +# those connected with ad_dc (vampiredc/promoteddc) + env = 'schema_pair_dc' planoldpythontestsuite("%s:local" % env, "samba_tool_drs", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], @@ -1202,11 +1214,6 @@ planoldpythontestsuite(env, "getnc_schema", extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) for env in ['vampire_dc', 'promoted_dc']: - planoldpythontestsuite("%s:local" % env, "samba_tool_drs", - extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], - name="samba4.drs.samba_tool_drs.python(%s)" % env, - environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, - extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) planoldpythontestsuite("%s:local" % env, "samba_tool_drs_showrepl", extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], name="samba4.drs.samba_tool_drs_showrepl.python(%s)" % env, -- 2.25.1 From 31e91b5678faa97a0a2a6d56d854b0337d5b17ae Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 9 Jun 2022 13:16:31 +1200 Subject: [PATCH 02/14] 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 ffb8e58f471..9edda2dc496 100644 --- a/python/samba/tests/__init__.py +++ b/python/samba/tests/__init__.py @@ -40,6 +40,7 @@ from samba.compat import string_types from random import randint from random import SystemRandom from contextlib import contextmanager +import shutil import string try: from samba.samdb import SamDB @@ -400,6 +401,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 dad7556a28367a7fe113fe277f777af1d1aaa5de Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:19:28 +1200 Subject: [PATCH 03/14] 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 3d230609efc..ddbf0d8f55d 100644 --- a/python/samba/tests/blackbox/downgradedatabase.py +++ b/python/samba/tests/blackbox/downgradedatabase.py @@ -19,7 +19,6 @@ from __future__ import print_function from samba.tests import BlackboxTestCase import os import ldb -import shutil from subprocess import check_output from samba.samdb import SamDB @@ -58,13 +57,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 48a3655e344f70c008e7aa8196709cfd4bfec3f0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:20:41 +1200 Subject: [PATCH 04/14] 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 6009391dd6c38c820b09fbdbb9f05632c8f22bad Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:21:16 +1200 Subject: [PATCH 05/14] 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 319090eb75e638406e1d162210225ac5ee7173b2 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:22:24 +1200 Subject: [PATCH 06/14] 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 e5c2b84760eed7714b6975d070ff69247942297f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 8 Jun 2022 19:53:57 +1200 Subject: [PATCH 07/14] 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 988f1dc7a3c..9ddedb4f646 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 3a30a4ad07d1f620efaedcedc795a4ef8141f4a1 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Wed, 15 Jun 2022 13:23:32 +1200 Subject: [PATCH 08/14] 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 326181a7fd7..a54e70668e1 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 dea40df01cd4238d3b23920660b1a5a7fd37bdfa Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Sep 2022 09:36:45 +1200 Subject: [PATCH 09/14] 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 a62036f1e56..1e2c387de0c 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1244,11 +1244,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, @@ -1265,6 +1260,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 0cf816b8655e2ee956216ba145325649f1534e79 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 03:05:03 +0000 Subject: [PATCH 10/14] 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 1e2c387de0c..8c03a15fccb 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1199,6 +1199,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", @@ -1206,6 +1209,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 905eaa117302afc99f5814d968dcbffd133b7907 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 14:53:38 +1300 Subject: [PATCH 11/14] 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 388ac4d6fbf9e3f7b0f04414e25e0879900680a2 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 29 Sep 2022 14:54:14 +1300 Subject: [PATCH 12/14] 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 6d6a44190ec..edc1bf2369b 100755 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -1905,10 +1905,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 1a293998b3f9a00068667f5a6dd5500c9951169b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 20 Sep 2022 13:37:30 +1200 Subject: [PATCH 13/14] 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 5186a362947..8f73b111375 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 9ab4f901a41cf5f9bf0f9b22b2457ea33a4c6a40 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 15 Sep 2022 17:10:24 +1200 Subject: [PATCH 14/14] 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 43d1b4a2488..009389267b8 100644 --- a/python/samba/drs_utils.py +++ b/python/samba/drs_utils.py @@ -230,6 +230,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 @@ -352,8 +390,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 4dab3ca2a9c..12911dad92c 100644 --- a/python/samba/join.py +++ b/python/samba/join.py @@ -984,17 +984,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