[PATCH] Add tests for AD DC with SMBv1 disabled

Tim Beale timbeale at catalyst.net.nz
Mon Nov 26 22:14:18 UTC 2018


The attached patch-set updates a testenv (restoredc) to have SMBv1
disabled. The backup tests are run against the DC (and fail) to
highlight the problem with SMBv2 samba-tool support.

Noel, I pinched one of your python-3 patches (domain_backup.py
95c6b38e68b65) and tweaked it slightly.

CI link: https://gitlab.com/catalyst-samba/samba/pipelines/37867316

Merge request: https://gitlab.com/samba-team/samba/merge_requests/112

Review appreciated. Thanks.

-------------- next part --------------
From 2e61628cc79689316166ccff3a98d53bf02d2d7f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 26 Nov 2018 13:32:03 +1300
Subject: [PATCH 01/11] selftest: Be explicit about which testenvs use ntvfs

If a testenv didn't specify any other provision arguments, then it
defaulted to using the NTVFS file server.

This patch makes it explicit, so we just pass through "--use-ntvfs" as
extra provision args in the cases we want.

(Whether all these testenvs really need to use NTVFS or not is another
question, but at least now it's easy to see which testenvs use it).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/Samba4.pm | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index c2e9fdb..3562926 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -966,8 +966,6 @@ sub provision($$$$$$$$$$)
 
 	if (defined($extra_provision_options)) {
 		push (@{$ctx->{provision_options}}, @{$extra_provision_options});
-	} else {
-		push (@{$ctx->{provision_options}}, "--use-ntvfs");
 	}
 
 	$ctx->{share} = "$ctx->{prefix_abs}/share";
@@ -1133,6 +1131,7 @@ rpc_server:tcpip = no
 	if ($more_conf) {
 		$extra_smb_conf = $extra_smb_conf . $more_conf . "\n";
 	}
+	my $extra_provision_options = ["--use-ntvfs"];
 	my $ret = $self->provision($prefix,
 				   "member server",
 				   $hostname,
@@ -1142,7 +1141,8 @@ rpc_server:tcpip = no
 				   "locMEMpass3",
 				   $dcvars->{SERVER_IP},
 				   $dcvars->{SERVER_IPV6},
-				   $extra_smb_conf, "", undef);
+				   $extra_smb_conf, "",
+				   $extra_provision_options);
 	unless ($ret) {
 		return undef;
 	}
@@ -1210,6 +1210,7 @@ sub provision_rpc_proxy($$$)
 
 ";
 
+	my $extra_provision_options = ["--use-ntvfs"];
 	my $ret = $self->provision($prefix,
 				   "member server",
 				   "localrpcproxy",
@@ -1219,7 +1220,8 @@ sub provision_rpc_proxy($$$)
 				   "locRPCproxypass4",
 				   $dcvars->{SERVER_IP},
 				   $dcvars->{SERVER_IPV6},
-				   $extra_smbconf_options, "", undef);
+				   $extra_smbconf_options, "",
+				   $extra_provision_options);
 	unless ($ret) {
 		return undef;
 	}
@@ -1570,6 +1572,7 @@ sub provision_ad_dc_ntvfs($$)
 	dsdb group change notification = true
 	server schannel = auto
 	";
+	my $extra_provision_options = ["--use-ntvfs"];
 	my $ret = $self->provision($prefix,
 				   "domain controller",
 				   "localdc",
@@ -1581,7 +1584,7 @@ sub provision_ad_dc_ntvfs($$)
 				   undef,
 				   $extra_conf_options,
 				   "",
-				   undef);
+				   $extra_provision_options);
 	unless ($ret) {
 		return undef;
 	}
@@ -1611,7 +1614,7 @@ sub provision_fl2000dc($$)
 	spnego:simulate_w2k=yes
 	ntlmssp_server:force_old_spnego=yes
 ";
-	my $extra_provision_options = undef;
+	my $extra_provision_options = ["--use-ntvfs"];
 	# This environment uses plain text secrets
 	# i.e. secret attributes are not encrypted on disk.
 	# This allows testing of the --plaintext-secrets option for
@@ -1658,6 +1661,7 @@ sub provision_fl2003dc($$$)
 	my $extra_conf_options = "allow dns updates = nonsecure and secure
 	dcesrv:header signing = no
 	dns forwarder = 127.0.0.$swiface1 127.0.0.$swiface2";
+	my $extra_provision_options = ["--use-ntvfs"];
 	my $ret = $self->provision($prefix,
 				   "domain controller",
 				   "dc6",
@@ -1669,7 +1673,7 @@ sub provision_fl2003dc($$$)
 				   undef,
 				   $extra_conf_options,
 				   "",
-				   undef);
+				   $extra_provision_options);
 	unless (defined $ret) {
 		return undef;
 	}
@@ -1713,6 +1717,7 @@ sub provision_fl2008r2dc($$$)
 
 	print "PROVISIONING DC WITH FOREST LEVEL 2008r2...\n";
         my $extra_conf_options = "ldap server require strong auth = no";
+	my $extra_provision_options = ["--use-ntvfs"];
 	my $ret = $self->provision($prefix,
 				   "domain controller",
 				   "dc7",
@@ -1724,7 +1729,7 @@ sub provision_fl2008r2dc($$$)
 				   undef,
 				   $extra_conf_options,
 				   "",
-				   undef);
+				   $extra_provision_options);
 	unless (defined $ret) {
 		return undef;
 	}
-- 
2.7.4


From c30f8d9f683a3e277de4ce16a80345d2b9f06f02 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 26 Nov 2018 14:59:06 +1300
Subject: [PATCH 02/11] selftest: Make chgdcpass's NTVFS usage more obvious

The chgdcpass testenv was not passing --use-ntvfs to the provision
command, but it was still actually using NTVFS.

The reason is the smb.conf generated by provision_raw_step1() would
always try to use the s4/NTVFS file server. Because the smb.conf already
existed, this trumped what was passed to the provision command.

This patch doesn't change the chgdcpass file server. It just makes it
more obvious that chgdcpass is using NTVFS.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/Samba4.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 3562926..b453d1c 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2029,7 +2029,7 @@ sub provision_chgdcpass($$)
 	my ($self, $prefix) = @_;
 
 	print "PROVISIONING CHGDCPASS...\n";
-	my $extra_provision_options = undef;
+	my $extra_provision_options = ["--use-ntvfs"];
 	# This environment disallows the use of this password
 	# (and also removes the default AD complexity checks)
 	my $unacceptable_password = "widk3Dsle32jxdBdskldsk55klASKQ";
-- 
2.7.4


From 52eee8e03b4f0c2171c0645067e2e7544fc7a05f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 26 Nov 2018 14:28:59 +1300
Subject: [PATCH 03/11] selftest: Make testenv NTVFS usage match --use-ntvfs

Regardless of whether the testenv uses --use-ntvfs as part of its
provision options, the s4 testenvs all default to using the NTVFS file
server.

It's not particularly obvious that this is happening. The new restore
DCS (restoredc, renamedc, labdc) were all using NTVFS unintentionally.

The problem is the s4 testenvs default to using services '-s3fs +smb".
provision_ad_dc() explicitly overrides this to use s3fs again
(technically it ends up with both in its smb.conf and just uses whatever
comes last).

This patch changes the testenv setup to check for the presence of the
'--use-ntvfs' option and to set the 'server services' config option
appropriately. This way, the provision command and the smb.conf options
should always line up, with respect to NTVFS.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/Samba4.pm | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index b453d1c..7fa3234 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -592,6 +592,16 @@ sub provision_raw_prepare($$$$$$$$$$$$)
 	return $ctx;
 }
 
+sub has_option
+{
+	my ($self, $keyword, @options_list) = @_;
+
+	# convert the options-list to a hash-map for easy keyword lookup
+	my %options_dict = map { $_ => 1 } @options_list;
+
+	return exists $options_dict{$keyword};
+}
+
 #
 # Step1 creates the basic configuration
 #
@@ -616,6 +626,13 @@ sub provision_raw_step1($$)
 	my $crlfile = "$ctx->{tlsdir}/crl.pem";
 	$crlfile = "" unless -e ${crlfile};
 
+	# work out which file server to use. Default to source3 smbd (s3fs),
+	# unless the source4 NTVFS (smb) file server has been specified
+	my $services = "-smb +s3fs";
+	if ($self->has_option("--use-ntvfs", @{$ctx->{provision_options}})) {
+		$services = "+smb -s3fs";
+	}
+
 	print CONFFILE "
 [global]
 	netbios name = $ctx->{netbiosname}
@@ -639,7 +656,7 @@ sub provision_raw_step1($$)
 	panic action = $RealBin/gdb_backtrace \%d
 	wins support = yes
 	server role = $ctx->{server_role}
-	server services = +echo +smb -s3fs
+	server services = +echo $services
         dcerpc endpoint servers = +winreg +srvsvc
 	notify:inotify = false
 	ldb:nosync = true
@@ -1901,7 +1918,6 @@ sub provision_ad_dc($$$$$$)
 	$password_hash_gpg_key_ids = "" unless defined($config_h->{HAVE_GPGME});
 
 	my $extra_smbconf_options = "
-        server services = -smb +s3fs
         xattr_tdb:file = $prefix_abs/statedir/xattr.tdb
 
 	dbwrap_tdb_mutexes:* = yes
-- 
2.7.4


From 97dfd2d812e8bcdb60d7dfa8cbd2b674ee3eaf57 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 26 Nov 2018 11:58:31 +1300
Subject: [PATCH 04/11] tests: Use s3 smbclient for testenv smoketests

smbclient4 doesn't support SMBv2 connections, i.e. it won't work against
a DC with SMBv1 disabled. The smoke-test here is that the DC accepts
the connection, so we don't really care what SMB tool we use to connect.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 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 6a1e144..37b7a04 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -971,7 +971,7 @@ for env in ['rodc', 'offlinebackupdc', 'restoredc', 'renamedc', 'labdc']:
     plansmbtorture4testsuite('rpc.echo', "%s:local" % env, ['ncacn_np:$SERVER', "-k", "yes", '-P', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo")
     plansmbtorture4testsuite('rpc.echo', "%s:local" % env, ['ncacn_np:$SERVER', "-k", "no", '-Utestallowed\ account%$DC_PASSWORD', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo.testallowed")
     plansmbtorture4testsuite('rpc.echo', "%s:local" % env, ['ncacn_np:$SERVER', "-k", "no", '-Utestdenied%$DC_PASSWORD', '--workgroup=$DOMAIN'], modname="samba4.rpc.echo.testdenied")
-    plantestsuite("samba4.blackbox.smbclient(%s:local)" % env, "%s:local" % env, [os.path.join(samba4srcdir, "utils/tests/test_smbclient.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', smbclient4])
+    plantestsuite("samba4.blackbox.smbclient(%s:local)" % env, "%s:local" % env, [os.path.join(samba4srcdir, "utils/tests/test_smbclient.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', binpath('smbclient')])
 
 planpythontestsuite("rodc:local", "samba.tests.samba_tool.rodc", py3_compatible=True)
 
-- 
2.7.4


From 7bdfd1618d2d204908da8e4aedf690545eafbb14 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 22 Nov 2018 13:22:19 +1300
Subject: [PATCH 05/11] selftest: Designate one testenv as having SMBv1
 disabled

We recommend users disable SMBv1 to avoid potential security holes.
However, none of the AD DC testenvs have SMBv1 disabled.

This patch disables SMBv1 on an arbitrarily-chosen testenv (restoredc).

I chose restoredc as we'll want to run the backup tool tests against
this target, and it might be useful to check we can backup a DC if it's
already been restored once.

Note that SMBv2 doesn't support POSIX extensions (only SMBv1 does),
which is why we haven't just disabled SMBv1 on *all* testenvs.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/target/README    |  1 +
 selftest/target/Samba4.pm | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/selftest/target/README b/selftest/target/README
index 3fd283e..b25dbab 100644
--- a/selftest/target/README
+++ b/selftest/target/README
@@ -67,6 +67,7 @@ are separate testenvs for each one.
     exist.
 - restoredc: tests the 'backup online' option. Online backups are similar to
     doing a DC join.
+    Restoredc's other unique feature is that is has SMBv1 disabled.
 - offlinebackupdc: tests the 'backup offline' option. Offline backups capture
     the raw DB files on disk (safely).
 - renamedc: tests the 'backup rename' option, where the domain and realm are
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 7fa3234..0a6c85d 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2816,7 +2816,8 @@ sub restore_backup_file
 # (without actually doing a 'domain join')
 sub prepare_dc_testenv
 {
-	my ($self, $prefix, $dcname, $domain, $realm, $password) = @_;
+	my ($self, $prefix, $dcname, $domain, $realm,
+		$password, $conf_options) = @_;
 
 	my $ctx = $self->provision_raw_prepare($prefix, "domain controller",
 					       $dcname,
@@ -2837,6 +2838,7 @@ sub prepare_dc_testenv
 	push(@{$ctx->{directories}}, "$ctx->{share}");
 
 	$ctx->{smb_conf_extra_options} = "
+	$conf_options
 	max xmit = 32K
 	server max protocol = SMB2
 
@@ -2879,10 +2881,16 @@ sub setup_restoredc
 	my ($self, $prefix, $dcvars) = @_;
 	print "Preparing RESTORE DC...\n";
 
+	# we arbitrarily designate the restored DC as having SMBv1 disabled
+	my $extra_conf = "
+	server min protocol = SMB2
+	client min protocol = SMB2";
+
 	my ($env, $ctx) = $self->prepare_dc_testenv($prefix, "restoredc",
 						    $dcvars->{DOMAIN},
 						    $dcvars->{REALM},
-						    $dcvars->{PASSWORD});
+						    $dcvars->{PASSWORD},
+						    $extra_conf);
 
 	# create a backup of the 'backupfromdc'
 	my $backupdir = File::Temp->newdir();
@@ -2923,7 +2931,7 @@ sub setup_renamedc
 	my $realm = "renamedom.samba.example.com";
 	my ($env, $ctx) = $self->prepare_dc_testenv($prefix, "renamedc",
 						    "RENAMEDOMAIN", $realm,
-						    $dcvars->{PASSWORD});
+						    $dcvars->{PASSWORD}, "");
 
 	# create a backup of the 'backupfromdc' which renames the domain
 	my $backupdir = File::Temp->newdir();
@@ -2970,7 +2978,7 @@ sub setup_offlinebackupdc
 	my ($env, $ctx) = $self->prepare_dc_testenv($prefix, "offlinebackupdc",
 						    $dcvars->{DOMAIN},
 						    $dcvars->{REALM},
-						    $dcvars->{PASSWORD});
+						    $dcvars->{PASSWORD}, "");
 
 	# create an offline backup of the 'backupfromdc' target
 	my $backupdir = File::Temp->newdir();
@@ -3014,7 +3022,7 @@ sub setup_labdc
 	my ($env, $ctx) = $self->prepare_dc_testenv($prefix, "labdc",
 						    "LABDOMAIN",
 						    "labdom.samba.example.com",
-						    $dcvars->{PASSWORD});
+						    $dcvars->{PASSWORD}, "");
 
 	# create a backup of the 'backupfromdc' which renames the domain and uses
 	# the --no-secrets option to scrub any sensitive info
@@ -3120,7 +3128,7 @@ sub setup_customdc
 
 	# create a placeholder directory and smb.conf, as well as the env vars.
 	my ($env, $ctx) = $self->prepare_dc_testenv($prefix, $dc_name,
-						    $domain, $realm, $password);
+						    $domain, $realm, $password, "");
 
 	# restore the specified backup file to populate the testenv
 	my $restore_dir = abs_path($prefix);
-- 
2.7.4


From 9ce1b553a0450e131a45d18af19feba173cc1b8c Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 22 Nov 2018 14:35:58 +1300
Subject: [PATCH 06/11] tests: Rework backup test_backup_invalid_args test-case

self.run_cmd() is a wrapper around self.check_output(). Rework the code
to call the underlying function instead.

The reason we're doing this is we want run_cmd() to catch exceptions and
fail the test. However, we can't do that because this test case relies
on receiving the exceptions.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/domain_backup.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index e9fcd31..5b68ea9 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -479,14 +479,17 @@ class DomainBackupRename(DomainBackupBase):
         """Checks that rename commands with invalid args are rejected"""
 
         # try a "rename" using the same realm as the DC currently has
-        self.base_cmd = ["domain", "backup", "rename", self.restore_domain,
-                         os.environ["REALM"]]
-        self.assertRaises(BlackboxProcessError, self.create_backup)
+        rename_cmd = "samba-tool domain backup rename "
+        bad_cmd = "{cmd} {domain} {realm}".format(cmd=rename_cmd,
+                                                  domain=self.restore_domain,
+                                                  realm=os.environ["REALM"])
+        self.assertRaises(BlackboxProcessError, self.check_output, bad_cmd)
 
         # try a "rename" using the same domain as the DC currently has
-        self.base_cmd = ["domain", "backup", "rename", os.environ["DOMAIN"],
-                         self.restore_realm]
-        self.assertRaises(BlackboxProcessError, self.create_backup)
+        bad_cmd = "{cmd} {domain} {realm}".format(cmd=rename_cmd,
+                                                  domain=os.environ["DOMAIN"],
+                                                  realm=self.restore_realm)
+        self.assertRaises(BlackboxProcessError, self.check_output, bad_cmd)
 
     def add_link(self, attr, source, target):
         m = ldb.Message()
-- 
2.7.4


From 02a5867b75b5ca85e613d5bea78ea811dcf61d9b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 22 Nov 2018 14:35:58 +1300
Subject: [PATCH 07/11] tests: Handle backup command exceptions as test
 failures, not errors

If the backup command fails (i.e. throws an exception), we want the test
to fail. This makes it easier to mark tests as 'knownfail' (because we
can't knownfail test errors).

In theory, this should just involve updating run_cmd() to catch any
exceptions from the command and then call self.fail().

However, if the backup command fails, it can leave behind files in the
targetdir. Partly this is intentional, as these files may provide clues
to users as to why the command failed. However, in selftest, it causes
the TestCaseInTempDir._remove_tempdir() assertion to fire. Because this
assert actually gets run as part of the teardown, the assertion gets
treated as an error rather than a failure (and so we can't knownfail the
backup tests). To get around this, we remove any files in the tempdir
prior to calling self.fail().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/__init__.py      |  1 +
 python/samba/tests/domain_backup.py | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py
index d79fcfb..9b30d2e 100644
--- a/python/samba/tests/__init__.py
+++ b/python/samba/tests/__init__.py
@@ -294,6 +294,7 @@ class TestCaseInTempDir(TestCase):
         self.addCleanup(self._remove_tempdir)
 
     def _remove_tempdir(self):
+        # Note asserting here is treated as an error rather than a test failure
         self.assertEquals([], os.listdir(self.tempdir))
         os.rmdir(self.tempdir)
         self.tempdir = None
diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index 5b68ea9..b9e97eb 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -360,6 +360,11 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
                             not in owner.extended_str(),
                             "%s found as FSMO %s role owner" % (server, role))
 
+    def cleanup_tempdir(self):
+        for filename in os.listdir(self.tempdir):
+            filepath = os.path.join(self.tempdir, filename)
+            shutil.rmtree(filepath)
+
     def run_cmd(self, args):
         """Executes a samba-tool backup/restore command"""
 
@@ -369,7 +374,14 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         # settings can bleed from one test case to another).
         cmd = " ".join(args)
         print("Executing: samba-tool %s" % cmd)
-        out = self.check_output("samba-tool " + cmd)
+        try:
+            out = self.check_output("samba-tool " + cmd)
+        except BlackboxProcessError as e:
+            # if the command failed, it may have left behind temporary files.
+            # We're going to fail the test, but first cleanup any temp files so
+            # that we skip the TestCaseInTempDir._remove_tempdir() assertions
+            self.cleanup_tempdir()
+            self.fail("Error calling samba-tool: %s" % e)
         print(out)
 
     def create_backup(self, extra_args=None):
-- 
2.7.4


From 7959e0c8eb8b308fc2b68e7424ae71e785ecd66c Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power at suse.com>
Date: Mon, 5 Nov 2018 19:00:20 +0000
Subject: [PATCH 08/11] python/samba/test: PY3 port samba.tests.domain_backup

The restoredc already runs under python3, so before we can run the
domain_backup tests against the restoredc, we need to make sure they
work under python3.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Noel Power <noel.power at suse.com>
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/domain_backup.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index b9e97eb..7147f0f 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -271,7 +271,7 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         self.assertEqual(len(bkp_pd), 1)
         acn = bkp_pd[0].get('samAccountName')
         self.assertIsNotNone(acn)
-        self.assertEqual(acn[0].replace('$', ''), self.new_server)
+        self.assertEqual(str(acn[0]), self.new_server + '$')
         self.assertIsNotNone(bkp_pd[0].get('secret'))
 
         samdb = SamDB(url=paths.samdb, session_info=system_session(),
@@ -552,8 +552,9 @@ class DomainBackupRename(DomainBackupBase):
         self.assertEqual(len(res), 1,
                          "Failed to find renamed link source object")
         self.assertTrue(link_attr in res[0], "Missing link attribute")
-        self.assertTrue(new_target_dn in res[0][link_attr])
-        self.assertTrue(new_server_dn in res[0][link_attr])
+        link_values = [str(x) for x in res[0][link_attr]]
+        self.assertTrue(new_target_dn in link_values)
+        self.assertTrue(new_server_dn in link_values)
 
     # extra checks we run on the restored DB in the rename case
     def check_restored_database(self, lp, expect_secrets=True):
-- 
2.7.4


From dca1021c69ec29b8231261871c848e8c703f1a2d Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 22 Nov 2018 14:05:01 +1300
Subject: [PATCH 09/11] tests: Run backup tests against restoredc (SMBv1
 disabled)

Running the backup tests against the restoredc highlights that the
backup online/rename commands don't work if SMBv1 is disabled. Note that
the offline commands still work because they don't rely on an SMB
connection to the server.

(Note that running the backup tests against the restoredc is probably a
good idea anyway, to prove that there's no limit to the number of times
you can restore a domain from backup, i.e. we support more than just a
one-off restore).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 selftest/knownfail.d/domain_backup | 12 ++++++++++++
 source4/selftest/tests.py          | 11 ++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 selftest/knownfail.d/domain_backup

diff --git a/selftest/knownfail.d/domain_backup b/selftest/knownfail.d/domain_backup
new file mode 100644
index 0000000..24f4d87
--- /dev/null
+++ b/selftest/knownfail.d/domain_backup
@@ -0,0 +1,12 @@
+# these tests only work with SMBv1, which is disabled on the restoredc
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_one_way_links\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_untar\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_with_conf\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_no_secrets\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_into_site\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_untar\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_with_conf\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_no_secrets\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_into_site\(restoredc:local\)
+samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore\(restoredc:local\)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 37b7a04..dc58072 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -53,6 +53,9 @@ smbclient4 = binpath('smbclient4')
 
 bbdir = os.path.join(srcdir(), "testprogs/blackbox")
 
+# alias to highlight what tests we want to run against a DC with SMBv1 disabled
+smbv1_disabled_testenv = "restoredc"
+
 # Simple tests for LDAP and CLDAP
 for auth_type in ['', '-k no', '-k yes']:
     for auth_level in ['--option=clientldapsaslwrapping=plain', '--sign', '--encrypt']:
@@ -779,9 +782,11 @@ planoldpythontestsuite("fl2003dc:local",
 planoldpythontestsuite("ad_dc",
                        "samba.tests.password_hash_ldap",
                        extra_args=['-U"$USERNAME%$PASSWORD"'], py3_compatible=True)
-planoldpythontestsuite("ad_dc:local",
-                       "samba.tests.domain_backup",
-                       extra_args=['-U"$USERNAME%$PASSWORD"'])
+
+for env in ["ad_dc", smbv1_disabled_testenv]:
+    planoldpythontestsuite(env + ":local", "samba.tests.domain_backup",
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
+
 planoldpythontestsuite("none",
                        "samba.tests.domain_backup_offline")
 # Encrypted secrets
-- 
2.7.4


From 6c7ed0d99d1ab2f66ec6a876513672b41fe45114 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 23 Nov 2018 09:46:38 +1300
Subject: [PATCH 10/11] tests: Work out DOMSID via samdb rather than environs

Not all testenvs have the DOMSID set as an environment variable.
However, it's easy enough to work out from querying the samdb.

This is a slight change in that we use a source4-generated loadparm
to connect to the DB (self.lp is source3-generated, presumably for
some SMB connection dependency).

This change is so we can run the ntacls_backup tests against a DC with
SMBv1 disabled (the restoredc). Note that currently the tests fail in
the smb.SMB() connection in the setUp(), so we can't run them as part
of autobuild just yet (because we can't known-fail test errors).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/ntacls_backup.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
index bb1bc97..0ee044f 100644
--- a/python/samba/tests/ntacls_backup.py
+++ b/python/samba/tests/ntacls_backup.py
@@ -25,9 +25,8 @@ from samba import samdb
 from samba import ntacls
 
 from samba.auth import system_session
-from samba.param import LoadParm
 from samba.dcerpc import security
-from samba.tests import TestCaseInTempDir
+from samba.tests import TestCaseInTempDir, env_loadparm
 
 
 class NtaclsBackupRestoreTests(TestCaseInTempDir):
@@ -47,21 +46,20 @@ class NtaclsBackupRestoreTests(TestCaseInTempDir):
             os.environ["LOCAL_PATH"], self.service)
 
         self.smb_conf_path = os.environ['SMB_CONF_PATH']
-        self.dom_sid = security.dom_sid(os.environ['DOMSID'])
-
         self.creds = self.insta_creds(template=self.get_credentials())
 
+        self.samdb_conn = samdb.SamDB(
+            url=samdb_url, session_info=system_session(),
+            credentials=self.creds, lp=env_loadparm())
+
+        self.dom_sid = security.dom_sid(self.samdb_conn.get_domain_sid())
+
         # helper will load conf into lp, that's how smbd can find services.
         self.ntacls_helper = ntacls.NtaclsHelper(self.service,
                                                  self.smb_conf_path,
                                                  self.dom_sid)
-
         self.lp = self.ntacls_helper.lp
 
-        self.samdb_conn = samdb.SamDB(
-            url=samdb_url, session_info=system_session(),
-            credentials=self.creds, lp=self.lp)
-
         self.smb_conn = smb.SMB(
             self.server, self.service, lp=self.lp, creds=self.creds)
 
-- 
2.7.4


From 00959bc5999ae575e437f3be4d4d0fa033df4d7f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 22 Nov 2018 16:56:22 +1300
Subject: [PATCH 11/11] tests: Avoid redundant inheritance in backup tests

By inheriting from SambaToolCmdTest, we are already inheriting from
TestCaseInTempDir. The actual inheritance goes:
  SambaToolCmdTest --> BlackboxTestCase --> TestCaseInTempDir

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/domain_backup.py         | 5 ++---
 python/samba/tests/domain_backup_offline.py | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index 7147f0f..ead8ba2 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -19,8 +19,7 @@ import tarfile
 import os
 import shutil
 from samba.tests.samba_tool.base import SambaToolCmdTest
-from samba.tests import (TestCaseInTempDir, env_loadparm, create_test_ou,
-                         BlackboxProcessError)
+from samba.tests import (env_loadparm, create_test_ou, BlackboxProcessError)
 import ldb
 from samba.samdb import SamDB
 from samba.auth import system_session
@@ -40,7 +39,7 @@ def get_prim_dom(secrets_path, lp):
                               expression="(objectClass=kerberosSecret)")
 
 
-class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
+class DomainBackupBase(SambaToolCmdTest):
 
     def setUp(self):
         super(DomainBackupBase, self).setUp()
diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
index f5fa156..5ecef46 100644
--- a/python/samba/tests/domain_backup_offline.py
+++ b/python/samba/tests/domain_backup_offline.py
@@ -20,10 +20,9 @@ import os
 import shutil
 import tempfile
 from samba.tests.samba_tool.base import SambaToolCmdTest
-from samba.tests import TestCaseInTempDir
 from samba.netcmd import CommandError
 
-class DomainBackupOfflineCmp(SambaToolCmdTest, TestCaseInTempDir):
+class DomainBackupOfflineCmp(SambaToolCmdTest):
 
     def test_domain_backup_offline_untar_tdb(self):
         self.untar_testcase('tdb')
-- 
2.7.4



More information about the samba-technical mailing list