[PATCH] Domain backup and restore samba-tool commands

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Thu Jun 28 01:10:13 UTC 2018


On 28/06/18 10:20, Tim Beale wrote:

> Hi Douglas,
> 
> I've:
> - Fixed whitespace in #3
> - Added return value checks in #2
> - Also initialized pointers to NULL in #2
> - Also tweaked the following logic in #2 (it didn't look quite right to me)
> +    if (ret != LDB_SUCCESS || res == NULL || res->count == 0) {
> +        talloc_free(db_context);
> +        return ret;
> +    }

Yep, that looks better.

Reviewed as attached, with the following changes squashed in.


> Subject: [PATCH 3/6] param: Add non-global smb.cfg option (support 2 different smb.confs)

Here I added a couple of PyErr_NoMemory()s here to present Python with
an explanation, and caught failures in lpcfg_load_no_global():

--- a/source4/param/pyparam.c
+++ b/source4/param/pyparam.c
@@ -465,18 +465,28 @@ static PyObject *py_lp_ctx_new(PyTypeObject *type, PyObject *args, PyObject *kwa
         * default behaviour and creates a separate underlying LoadParm object.
         */
        if (non_global_conf != NULL) {
+               bool ok;
                ctx = loadparm_init(NULL);
                if (ctx == NULL) {
+                       PyErr_NoMemory();
                        return NULL;
                }
 
                lp_ctx = pytalloc_reference(type, ctx);
                if (lp_ctx == NULL) {
+                       PyErr_NoMemory();
                        return NULL;
                }
 
-               lpcfg_load_no_global(PyLoadparmContext_AsLoadparmContext(lp_ctx),
+               ok = lpcfg_load_no_global(
+                       PyLoadparmContext_AsLoadparmContext(lp_ctx),
+                       non_global_conf);
+               if (!ok) {
+                       PyErr_Format(PyExc_ValueError,
+                                    "Could not load non-global conf %s",
                                     non_global_conf);
+                       return NULL;
+               }
                return lp_ctx;
        } else{
                return pytalloc_reference(type, loadparm_init_global(false));



> Subject: [PATCH 4/6] tests: Add basic test for non-global LoadParm behaviour

And the test now has a check for a bad path (and a minor pep-8 change).

--- a/python/samba/tests/loadparm.py
+++ b/python/samba/tests/loadparm.py
@@ -52,7 +52,7 @@ class LoadParmTest(TestCaseInTempDir):
         # we can create a non-global Loadparm that overrides the default
         # behaviour and creates a separate underlying object
         lp1 = param.LoadParm()
-        lp2 = param.LoadParm(filename_for_non_global_lp = smb_conf)
+        lp2 = param.LoadParm(filename_for_non_global_lp=smb_conf)
 
         # setting a value for the global LP does not affect the non-global LP
         lp1_realm = "JUST.A.TEST"
@@ -66,3 +66,18 @@ class LoadParmTest(TestCaseInTempDir):
         lp2.set('realm', lp2_realm)
         self.assertEqual(lp2.get('realm'), lp2_realm)
         self.assertEqual(lp1.get('realm'), lp1_realm)
+
+    def test_non_global_loadparm_bad_path(self):
+        non_existent_file = os.path.join(self.tempdir, 'not-there')
+
+        # we can create a non-global Loadparm that overrides the default
+        # behaviour and creates a separate underlying object
+        self.assertRaises(ValueError,
+                          param.LoadParm,
+                          filename_for_non_global_lp=non_existent_file)
+
+        # still shouldn't be there
+        self.assertRaises(ValueError,
+                          param.LoadParm,
+                          non_existent_file)



More reviewers needed!

Douglas

-------------- next part --------------
From fdc5466012d5fe35410d21a994d118575a7b74c3 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Jun 2018 14:06:54 +1200
Subject: [PATCH 1/6] tests: Add test that Samba cannot be started with a
 backup DB

We don't want users to take a backup file, and then simply untar it and
run Samba (Several modifications to the DB need to be made as part of
the restore process, so users should always run the 'backup restore'
command).

To enforce this, prime_ldb_databases() now refuses to start Samba if the
backupDate marker is present in the DB. This patch adds a test-case that
proves this basic behaviour works.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 selftest/knownfail.d/start_backup            |  1 +
 source4/selftest/tests.py                    |  5 ++
 source4/setup/tests/blackbox_start_backup.sh | 79 ++++++++++++++++++++
 3 files changed, 85 insertions(+)
 create mode 100644 selftest/knownfail.d/start_backup
 create mode 100755 source4/setup/tests/blackbox_start_backup.sh

diff --git a/selftest/knownfail.d/start_backup b/selftest/knownfail.d/start_backup
new file mode 100644
index 00000000000..223a743ef1c
--- /dev/null
+++ b/selftest/knownfail.d/start_backup
@@ -0,0 +1 @@
+samba4.blackbox.start_backup.start-samba-backup\(none\)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 8b1fb7b280a..46451e53baf 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -861,6 +861,11 @@ plantestsuite("samba4.blackbox.supported_features", "none",
                os.path.join(samba4srcdir,
                             "setup/tests/blackbox_supported_features.sh"),
                '$PREFIX/provision'])
+plantestsuite("samba4.blackbox.start_backup", "none",
+              ["PYTHON=%s" % python,
+               os.path.join(samba4srcdir,
+                            "setup/tests/blackbox_start_backup.sh"),
+               '$PREFIX/provision'])
 plantestsuite("samba4.blackbox.upgradeprovision.current", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/blackbox_upgradeprovision.sh"), '$PREFIX/provision'])
 plantestsuite("samba4.blackbox.setpassword.py", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/blackbox_setpassword.sh"), '$PREFIX/provision'])
 plantestsuite("samba4.blackbox.newuser.py", "none", ["PYTHON=%s" % python, os.path.join(samba4srcdir, "setup/tests/blackbox_newuser.sh"), '$PREFIX/provision'])
diff --git a/source4/setup/tests/blackbox_start_backup.sh b/source4/setup/tests/blackbox_start_backup.sh
new file mode 100755
index 00000000000..f1cfd53c5b6
--- /dev/null
+++ b/source4/setup/tests/blackbox_start_backup.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+# Simple test that a DB from a backup file cannot be untarred and started
+# manually (you have to run the samba-tool 'backup restore' command instead).
+
+if [ $# -lt 1 ]; then
+cat <<EOF
+Usage: $0 PREFIX
+EOF
+exit 1;
+fi
+
+PREFIX="$1"
+shift 1
+
+DBPATH=$PREFIX/start-backup
+mkdir -p $DBPATH
+
+. `dirname $0`/../../../testprogs/blackbox/subunit.sh
+
+do_provision()
+{
+    $PYTHON $BINDIR/samba-tool domain provision \
+           --domain=FOO --realm=foo.example.com --use-ntvfs \
+           --targetdir=$DBPATH --option="pid directory = $DBPATH"
+}
+
+add_backup_marker()
+{
+# manually add the backup marker that the backup cmd usually adds
+    $BINDIR/ldbmodify \
+       -H tdb://$DBPATH/private/sam.ldb <<EOF
+dn: @SAMBA_DSDB
+changetype: modify
+add: backupDate
+backupDate: who-knows-when
+-
+
+EOF
+}
+
+start_backup()
+{
+    # start samba in interactive mode (if we don't, samba daemonizes and so the
+    # command's exit status is always zero (success), regardless of whether
+    # samba actually starts up or not). However, this means if this assertion
+    # were ever to fail (i.e. samba DOES startup from a backup file), then the
+    # test case would just hang. So we use a max-run-time of 5 secs so that
+    # samba will self-destruct in the bad case (max_runtime_handler() returns
+    # zero/success in this case, which allows us to tell the good case from the
+    # bad case).
+    OPTS="--maximum-runtime=5 -i"
+
+    # redirect logs to stderr (which we'll then redirect to stdout so we can
+    # capture it in a bash variable)
+    OPTS="$OPTS --debug-stderr"
+
+    # start samba and capture the debug output
+    OUTPUT=$($BINDIR/samba -s $DBPATH/etc/smb.conf $OPTS 2>&1)
+    if [ $? -eq 0 ] ; then
+        echo "ERROR: Samba should not have started successfully"
+        return 1
+    fi
+
+    # check the reason we're failing is because prime_ldb_databases() is
+    # detecting that this is a backup DB (and not some other reason)
+    echo "$OUTPUT" | grep "failed to start: Database is a backup"
+}
+
+# setup a DB and manually mark it as being a "backup"
+testit "provision" do_provision
+testit "add-backup-marker" add_backup_marker
+
+# check that Samba won't start using this DB (because it's a backup)
+testit "start-samba-backup" start_backup
+
+rm -rf $DBPATH
+
+exit $failed
-- 
2.17.1


From 01c54d28c64a7fdfe4f42cc4699b2b43b21ed35e Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Date: Tue, 1 May 2018 15:48:38 +1200
Subject: [PATCH 2/6] samba: read backup date field on init and fail if present

This prevents a backup tar file, created with the new official
backup tools, from being extracted and replicated.

This is done here to ensure that samba-tool and ldbsearch can
still operate on the backup (eg for forensics) but starting
Samba as an AD DC will fail.

Signed-off-by: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 selftest/knownfail.d/start_backup |  1 -
 source4/smbd/server.c             | 83 +++++++++++++++++++++++++------
 2 files changed, 68 insertions(+), 16 deletions(-)
 delete mode 100644 selftest/knownfail.d/start_backup

diff --git a/selftest/knownfail.d/start_backup b/selftest/knownfail.d/start_backup
deleted file mode 100644
index 223a743ef1c..00000000000
--- a/selftest/knownfail.d/start_backup
+++ /dev/null
@@ -1 +0,0 @@
-samba4.blackbox.start_backup.start-samba-backup\(none\)
diff --git a/source4/smbd/server.c b/source4/smbd/server.c
index ed81c01afc1..5eca0b9e869 100644
--- a/source4/smbd/server.c
+++ b/source4/smbd/server.c
@@ -44,6 +44,7 @@
 #include "nsswitch/winbind_client.h"
 #include "libds/common/roles.h"
 #include "lib/util/tfork.h"
+#include "dsdb/samdb/ldb_modules/util.h"
 
 #ifdef HAVE_PTHREAD
 #include <pthread.h>
@@ -232,23 +233,63 @@ _NORETURN_ static void max_runtime_handler(struct tevent_context *ev,
   pre-open the key databases. This saves a lot of time in child
   processes
  */
-static void prime_ldb_databases(struct tevent_context *event_ctx)
+static int prime_ldb_databases(struct tevent_context *event_ctx, bool *am_backup)
 {
-	TALLOC_CTX *db_context;
-	db_context = talloc_new(event_ctx);
-
-	samdb_connect(db_context,
-		      event_ctx,
-		      cmdline_lp_ctx,
-		      system_session(cmdline_lp_ctx),
-		      NULL,
-		      0);
-	privilege_connect(db_context, cmdline_lp_ctx);
-
-	/* we deliberately leave these open, which allows them to be
+	struct ldb_result *res = NULL;
+	struct ldb_dn *samba_dsdb_dn = NULL;
+	struct ldb_context *ldb_ctx = NULL;
+	struct ldb_context *pdb = NULL;
+	static const char *attrs[] = { "backupDate", NULL };
+	const char *msg = NULL;
+	int ret;
+	TALLOC_CTX *db_context = talloc_new(event_ctx);
+	if (db_context == NULL) {
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	*am_backup = false;
+
+	/* note we deliberately leave these open, which allows them to be
 	 * re-used in ldb_wrap_connect() */
-}
+	ldb_ctx = samdb_connect(db_context,
+				event_ctx,
+				cmdline_lp_ctx,
+				system_session(cmdline_lp_ctx),
+				NULL,
+				0);
+	if (ldb_ctx == NULL) {
+		talloc_free(db_context);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+	pdb = privilege_connect(db_context, cmdline_lp_ctx);
+	if (pdb == NULL) {
+		talloc_free(db_context);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
 
+	/* check the root DB object to see if it's marked as a backup */
+	samba_dsdb_dn = ldb_dn_new(db_context, ldb_ctx, "@SAMBA_DSDB");
+	if (!samba_dsdb_dn) {
+		talloc_free(db_context);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
+	ret = dsdb_search_dn(ldb_ctx, db_context, &res, samba_dsdb_dn, attrs,
+			     DSDB_FLAG_AS_SYSTEM);
+	if (ret != LDB_SUCCESS) {
+		talloc_free(db_context);
+		return ret;
+	}
+
+	if (res->count > 0) {
+		msg = ldb_msg_find_attr_as_string(res->msgs[0], "backupDate",
+						  NULL);
+		if (msg != NULL) {
+			*am_backup = true;
+		}
+	}
+	return LDB_SUCCESS;
+}
 
 /*
   called when a fatal condition occurs in a child task
@@ -366,7 +407,9 @@ static int binary_smbd_main(const char *binary_name,
 	bool opt_fork = true;
 	bool opt_interactive = false;
 	bool opt_no_process_group = false;
+	bool db_is_backup = false;
 	int opt;
+	int ret;
 	poptContext pc;
 #define _MODULE_PROTO(init) extern NTSTATUS init(TALLOC_CTX *);
 	STATIC_service_MODULES_PROTO;
@@ -631,7 +674,17 @@ static int binary_smbd_main(const char *binary_name,
 			"and exited. Check logs for details", EINVAL);
 	};
 
-	prime_ldb_databases(state->event_ctx);
+	ret = prime_ldb_databases(state->event_ctx, &db_is_backup);
+	if (ret != LDB_SUCCESS) {
+		TALLOC_FREE(state);
+		exit_daemon("Samba failed to prime database", EINVAL);
+	}
+
+	if (db_is_backup) {
+		TALLOC_FREE(state);
+		exit_daemon("Database is a backup. Please run samba-tool domain"
+			    " backup restore", EINVAL);
+	}
 
 	status = setup_parent_messaging(state, cmdline_lp_ctx);
 	if (!NT_STATUS_IS_OK(status)) {
-- 
2.17.1


From e2c1ba55583998e2c7dce4d22310f1cf4617800f Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Date: Tue, 1 May 2018 11:10:36 +1200
Subject: [PATCH 3/6] param: Add non-global smb.cfg option (support 2 different
 smb.confs)

The default behaviour is that there is only a single global underlying
LoadParm object. E.g. if you create 2 different LoadParm objects in
python, they both modify the same underlying object.

This patch adds a mechanism to override this and create a separate
non-global LoadParm object. The use-case is the backup tool, where we
want to manipulate 2 different smb.conf files (the one used to create
the backup, and the smb.conf in the backup itself).

Signed-off-by: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 lib/param/loadparm.c    | 15 +++++++++++--
 source4/param/pyparam.c | 47 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 3b7f8053e4a..9684a52952b 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -3148,7 +3148,8 @@ bool lpcfg_load_default(struct loadparm_context *lp_ctx)
  *
  * Return True on success, False on failure.
  */
-bool lpcfg_load(struct loadparm_context *lp_ctx, const char *filename)
+static bool lpcfg_load_internal(struct loadparm_context *lp_ctx,
+				const char *filename, bool set_global)
 {
 	char *n2;
 	bool bRetval;
@@ -3183,7 +3184,7 @@ bool lpcfg_load(struct loadparm_context *lp_ctx, const char *filename)
 	   for a missing smb.conf */
 	reload_charcnv(lp_ctx);
 
-	if (bRetval == true) {
+	if (bRetval == true && set_global) {
 		/* set this up so that any child python tasks will
 		   find the right smb.conf */
 		setenv("SMB_CONF_PATH", filename, 1);
@@ -3197,6 +3198,16 @@ bool lpcfg_load(struct loadparm_context *lp_ctx, const char *filename)
 	return bRetval;
 }
 
+bool lpcfg_load_no_global(struct loadparm_context *lp_ctx, const char *filename)
+{
+    return lpcfg_load_internal(lp_ctx, filename, false);
+}
+
+bool lpcfg_load(struct loadparm_context *lp_ctx, const char *filename)
+{
+    return lpcfg_load_internal(lp_ctx, filename, true);
+}
+
 /**
  * Return the max number of services.
  */
diff --git a/source4/param/pyparam.c b/source4/param/pyparam.c
index e7e908fcac3..11257c356aa 100644
--- a/source4/param/pyparam.c
+++ b/source4/param/pyparam.c
@@ -445,7 +445,52 @@ static PyGetSetDef py_lp_ctx_getset[] = {
 
 static PyObject *py_lp_ctx_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
 {
-	return pytalloc_reference(type, loadparm_init_global(false));
+	const char *kwnames[] = {"filename_for_non_global_lp", NULL};
+	PyObject *lp_ctx;
+	const char *non_global_conf = NULL;
+	struct loadparm_context *ctx;
+
+	if (!PyArg_ParseTupleAndKeywords(args,
+					 kwargs,
+					 "|s",
+					 discard_const_p(char *,
+							 kwnames),
+					 &non_global_conf)) {
+		return NULL;
+	}
+
+	/*
+	 * by default, any LoadParm python objects map to a single global
+	 * underlying object. The filename_for_non_global_lp arg overrides this
+	 * default behaviour and creates a separate underlying LoadParm object.
+	 */
+	if (non_global_conf != NULL) {
+		bool ok;
+		ctx = loadparm_init(NULL);
+		if (ctx == NULL) {
+			PyErr_NoMemory();
+			return NULL;
+		}
+
+		lp_ctx = pytalloc_reference(type, ctx);
+		if (lp_ctx == NULL) {
+			PyErr_NoMemory();
+			return NULL;
+		}
+
+		ok = lpcfg_load_no_global(
+			PyLoadparmContext_AsLoadparmContext(lp_ctx),
+			non_global_conf);
+		if (!ok) {
+			PyErr_Format(PyExc_ValueError,
+				     "Could not load non-global conf %s",
+				     non_global_conf);
+			return NULL;
+		}
+		return lp_ctx;
+	} else{
+		return pytalloc_reference(type, loadparm_init_global(false));
+	}
 }
 
 static Py_ssize_t py_lp_ctx_len(PyObject *self)
-- 
2.17.1


From bc973539e754837c457c9de5ca31e3aa55fc5467 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Jun 2018 10:39:23 +1200
Subject: [PATCH 4/6] tests: Add basic test for non-global LoadParm behaviour

Add a simple test to show that the new non-global LoadParm behaviour
works, i.e.
- by default all LoadParm objects are linked to the same underlying
  object
- using a non-global LoadParm creates a separate underlying object.
- using a non-global LoadParm with a bad filename fails.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/tests/loadparm.py | 82 ++++++++++++++++++++++++++++++++++
 source4/selftest/tests.py      |  2 +
 2 files changed, 84 insertions(+)
 create mode 100644 python/samba/tests/loadparm.py

diff --git a/python/samba/tests/loadparm.py b/python/samba/tests/loadparm.py
new file mode 100644
index 00000000000..daa0162b09f
--- /dev/null
+++ b/python/samba/tests/loadparm.py
@@ -0,0 +1,82 @@
+# Unix SMB/CIFS implementation.
+# Copyright (C) Andrew Bartlett <abartlet at samba.org>
+#
+# 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 <http://www.gnu.org/licenses/>.
+#
+
+from samba.tests import TestCaseInTempDir
+from samba import param
+import os
+
+# the python bindings for LoadParm objects map (by default) to a single global
+# object in the underlying C code. E.g. if we create 2 different LoadParm
+# objects in python, really they're just the same object underneath.
+class LoadParmTest(TestCaseInTempDir):
+
+    def test_global_loadparm(self):
+        # create 2 different Loadparm objects (which are really the same
+        # object underneath)
+        lp1 = param.LoadParm()
+        lp2 = param.LoadParm()
+
+        # we can prove this by setting a value on lp1 and assert that the
+        # change is also reflected on lp2
+        lp1_realm = "JUST.A.TEST"
+        self.assertNotEqual(lp2.get('realm'), lp1_realm)
+        lp1.set('realm', lp1_realm)
+        self.assertEqual(lp1.get('realm'), lp1_realm)
+        self.assertEqual(lp2.get('realm'), lp1_realm)
+
+    def touch_temp_file(self, filename):
+        filepath = os.path.join(self.tempdir, filename)
+        open(filepath, 'a').close()
+        # delete the file once the test completes
+        self.addCleanup(os.remove, filepath)
+        return filepath
+
+    def test_non_global_loadparm(self):
+        # create a empty smb.conf file
+        smb_conf = self.touch_temp_file("smb.conf")
+
+        # we can create a non-global Loadparm that overrides the default
+        # behaviour and creates a separate underlying object
+        lp1 = param.LoadParm()
+        lp2 = param.LoadParm(filename_for_non_global_lp=smb_conf)
+
+        # setting a value for the global LP does not affect the non-global LP
+        lp1_realm = "JUST.A.TEST"
+        self.assertNotEqual(lp2.get('realm'), lp1_realm)
+        lp1.set('realm', lp1_realm)
+        self.assertEqual(lp1.get('realm'), lp1_realm)
+        self.assertNotEqual(lp2.get('realm'), lp1_realm)
+
+        # and vice versa
+        lp2_realm = "TEST.REALM.LP2"
+        lp2.set('realm', lp2_realm)
+        self.assertEqual(lp2.get('realm'), lp2_realm)
+        self.assertEqual(lp1.get('realm'), lp1_realm)
+
+    def test_non_global_loadparm_bad_path(self):
+        non_existent_file = os.path.join(self.tempdir, 'not-there')
+
+        # we can create a non-global Loadparm that overrides the default
+        # behaviour and creates a separate underlying object
+        self.assertRaises(ValueError,
+                          param.LoadParm,
+                          filename_for_non_global_lp=non_existent_file)
+
+        # still shouldn't be there
+        self.assertRaises(ValueError,
+                          param.LoadParm,
+                          non_existent_file)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 46451e53baf..1b592c3b0f1 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -771,6 +771,8 @@ planoldpythontestsuite("ad_dc_ntvfs",
 planoldpythontestsuite("ad_dc_ntvfs",
                        "samba.tests.blackbox.traffic_summary",
                        extra_args=['-U"$USERNAME%$PASSWORD"'])
+planoldpythontestsuite("none", "samba.tests.loadparm")
+
 #
 # Want a selection of environments across the process models
 #
-- 
2.17.1


From 0031420bf4c4af5806f368cfa659634f7118b0b4 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 29 May 2018 15:22:07 +1200
Subject: [PATCH 5/6] selftest: Update MAX_WRAPPED_INTERFACES comment to match
 code

Commit 19606e4dc657b0baf3ea84d updated the MAX_WRAPPED_INTERFACES define
in the C code from 40 to 64.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 selftest/target/Samba.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index b0482d36c91..81d3d21d5f4 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -409,8 +409,8 @@ sub get_interface($)
     $interfaces{"vampire2000dc"} = 39;
 
     # update lib/socket_wrapper/socket_wrapper.c
-    #  #define MAX_WRAPPED_INTERFACES 40
-    # if you wish to have more than 40 interfaces
+    #  #define MAX_WRAPPED_INTERFACES 64
+    # if you wish to have more than 64 interfaces
 
     if (not defined($interfaces{$netbiosname})) {
 	die();
-- 
2.17.1


From 7d9decd724da86b2e7fb00a2bc761011f7e1243a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 25 Jun 2018 14:00:59 +1200
Subject: [PATCH 6/6] provision: set 'binddns dir' when making new smb.conf

When creating a new smb.conf from scratch during a join/clone/etc, the
'binddns dir' setting still uses the source smb.conf/default setting,
instead of the targetdir sub-directory.

I noticed this problem when trying to create a new testenv - the
provision() was trying to create /usr/local/samba/bind-dns directory,
which would fail if samba hadn't already been installed on the host
machine.

Now that this is fixed, we also need to fix tests that were explicitly
asserting that no unexpected directories were left behind after the test
completes.

This change also breaks the upgradeprovision script. The upgrade-
provision calls newprovision() to create a reference provision in a
temporary directory. However, previously this temporary provision was
creating the bind-dns directory in the actual upgrade directory as a
side-effect, e.g. it did a provision() with
targetdir=alpha13_upgrade_full/private/referenceprovisionLBKBh2 and this
ended up creating alpha13_upgrade_full/bind-dns as a side-effect.
The provision() now creates bind-dns in the specified targetdir, but
this means check_for_DNS() fails (it tries to create bind-dns sub-
directories, but the upgrade's bind-dns doesn't exist). I've avoided
this problem by making sure bind-dns exists as part of the
check_for_DNS() processing.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/provision/__init__.py           | 2 ++
 python/samba/tests/join.py                   | 1 +
 python/samba/tests/samdb.py                  | 2 +-
 source4/scripting/bin/samba_upgradeprovision | 3 +++
 source4/torture/drs/python/samba_tool_drs.py | 1 +
 5 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 36f50f252ed..e5718949626 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -744,10 +744,12 @@ def make_smbconf(smbconf, hostname, domain, realm, targetdir,
         global_settings["lock dir"] = os.path.abspath(targetdir)
         global_settings["state directory"] = os.path.abspath(os.path.join(targetdir, "state"))
         global_settings["cache directory"] = os.path.abspath(os.path.join(targetdir, "cache"))
+        global_settings["binddns dir"] = os.path.abspath(os.path.join(targetdir, "bind-dns"))
 
         lp.set("lock dir", os.path.abspath(targetdir))
         lp.set("state directory",  global_settings["state directory"])
         lp.set("cache directory", global_settings["cache directory"])
+        lp.set("binddns dir", global_settings["binddns dir"])
 
     if eadb:
         if use_ntvfs and not lp.get("posix:eadb"):
diff --git a/python/samba/tests/join.py b/python/samba/tests/join.py
index 1f9fab1d72a..17de3ab84ce 100644
--- a/python/samba/tests/join.py
+++ b/python/samba/tests/join.py
@@ -73,6 +73,7 @@ class JoinTestCase(DNSTKeyTest):
             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.join_ctx.cleanup_old_join(force=True)
 
diff --git a/python/samba/tests/samdb.py b/python/samba/tests/samdb.py
index d4279b4aed4..8c477b291db 100644
--- a/python/samba/tests/samdb.py
+++ b/python/samba/tests/samdb.py
@@ -59,7 +59,7 @@ class SamDBTestCase(TestCaseInTempDir):
         for f in ['names.tdb']:
             os.remove(os.path.join(self.tempdir, f))
 
-        for d in ['etc', 'msg.lock', 'private', 'state']:
+        for d in ['etc', 'msg.lock', 'private', 'state', 'bind-dns']:
             shutil.rmtree(os.path.join(self.tempdir, d))
 
         super(SamDBTestCase, self).tearDown()
diff --git a/source4/scripting/bin/samba_upgradeprovision b/source4/scripting/bin/samba_upgradeprovision
index 9d3e73604a2..5d040d21d66 100755
--- a/source4/scripting/bin/samba_upgradeprovision
+++ b/source4/scripting/bin/samba_upgradeprovision
@@ -226,6 +226,9 @@ def check_for_DNS(refprivate, private, refbinddns_dir, binddns_dir, dns_backend)
     if not os.path.exists(dnsfile):
         shutil.copy("%s/dns_update_list" % refprivate, "%s" % dnsfile)
 
+    if not os.path.exists(binddns_dir):
+        os.mkdir(binddns_dir)
+
     if dns_backend not in ['BIND9_DLZ', 'BIND9_FLATFILE']:
        return
 
diff --git a/source4/torture/drs/python/samba_tool_drs.py b/source4/torture/drs/python/samba_tool_drs.py
index 502c8096603..29c6016471c 100644
--- a/source4/torture/drs/python/samba_tool_drs.py
+++ b/source4/torture/drs/python/samba_tool_drs.py
@@ -47,6 +47,7 @@ class SambaToolDrsTests(drs_base.DrsBaseTestCase):
             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
 
-- 
2.17.1



More information about the samba-technical mailing list