[PATCH] Some more SMB python bindings changes

Tim Beale timbeale at catalyst.net.nz
Fri Dec 14 01:39:42 UTC 2018


Attached are some more SMB Python bindings changes. These apply on top
of the patches I sent out yesterday:
https://gitlab.com/samba-team/samba/merge_requests/155

The changes in this patch-set are:
- added another API (.get_acl()) that will be used by the ntacls python
code.
- added some FILE_ATTRIBUTE_XYZ flags that the python code uses in
conjunction with the .list() results.
- renamed libsmb_samba_internal to libsmb.
- some minor tests/samba-tool changes to make it easier to convert the
SMB connection over.

CI link: https://gitlab.com/catalyst-samba/samba/pipelines/40086568
Merge request: https://gitlab.com/samba-team/samba/merge_requests/163

Review appreciated. I'm on Xmas break now, but I'll address any feedback
first thing in January.

Thanks.

-------------- next part --------------
From 00e82aec3be93b1a3ceab9c519096eaf84d00366 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 16:14:43 +1300
Subject: [PATCH 1/9] s3:pylibsmb: Add .get_acl() API to SMB py bindings

There is no obvious async-equivalent of cli_query_security_descriptor(),
so it will throw an error if anyone tries to use it in multi-threaded
mode. Currently only samba-tool and tests use the (s4) .get_acl() API,
both of which will be fine using the synchronous API.

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source3/libsmb/pylibsmb.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 2a39187..3d7261c 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -29,10 +29,16 @@
 #include "auth/credentials/pycredentials.h"
 #include "trans2.h"
 #include "libsmb/clirap.h"
+#include "librpc/rpc/pyrpc_util.h"
 
 #define LIST_ATTRIBUTE_MASK \
 	(FILE_ATTRIBUTE_DIRECTORY|FILE_ATTRIBUTE_SYSTEM|FILE_ATTRIBUTE_HIDDEN)
 
+#define SECINFO_DEFAULT_FLAGS \
+	(SECINFO_OWNER | SECINFO_GROUP | \
+	 SECINFO_DACL | SECINFO_PROTECTED_DACL | SECINFO_UNPROTECTED_DACL | \
+	 SECINFO_SACL | SECINFO_PROTECTED_SACL | SECINFO_UNPROTECTED_SACL)
+
 static PyTypeObject *get_pytype(const char *module, const char *type)
 {
 	PyObject *mod;
@@ -1472,6 +1478,50 @@ static PyObject *py_smb_deltree(struct py_cli_state *self, PyObject *args)
 	Py_RETURN_NONE;
 }
 
+/*
+ * Read ACL on a given file/directory as a security descriptor object
+ */
+static PyObject *py_smb_getacl(struct py_cli_state *self, PyObject *args)
+{
+	NTSTATUS status;
+	const char *filename = NULL;
+	uint32_t sinfo = SECINFO_DEFAULT_FLAGS;
+	int access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
+	uint16_t fnum;
+	struct security_descriptor *sd = NULL;
+
+	/* there's no async version of cli_query_security_descriptor() */
+	if (self->thread_state != NULL) {
+		PyErr_SetString(PyExc_RuntimeError,
+				"get_acl() is not supported on "
+				"a multi_threaded connection");
+		return NULL;
+	}
+
+	if (!PyArg_ParseTuple(args, "s|Ii:get_acl", &filename, &sinfo,
+			      &access_mask)) {
+		return NULL;
+	}
+
+	/* get a file handle with the desired access */
+	status = cli_ntcreate(self->cli, filename, 0, access_mask, 0,
+			      FILE_SHARE_READ|FILE_SHARE_WRITE,
+			      FILE_OPEN, 0x0, 0x0, &fnum, NULL);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	/* query the security descriptor for this file */
+	status = cli_query_security_descriptor(self->cli, fnum, sinfo,
+					       NULL, &sd);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	/* close the file handle and convert the SD to a python struct */
+	status = cli_close(self->cli, fnum);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+	return py_return_ndr_struct("samba.dcerpc.security", "descriptor",
+				    sd, sd);
+}
+
 static PyMethodDef py_cli_state_methods[] = {
 	{ "create", (PyCFunction)py_cli_create, METH_VARARGS|METH_KEYWORDS,
 	  "Open a file" },
@@ -1519,6 +1569,9 @@ static PyMethodDef py_cli_state_methods[] = {
 	{ "deltree", (PyCFunction)py_smb_deltree, METH_VARARGS,
 	  "deltree(path) -> None\n\n"
 	  "\t\tDelete a directory and all its contents." },
+	{ "get_acl", (PyCFunction)py_smb_getacl, METH_VARARGS,
+	  "get_acl(path[, security_info=0]) -> security_descriptor object\n\n"
+	  "\t\tGet security descriptor for file." },
 	{ NULL, NULL, 0, NULL }
 };
 
-- 
2.7.4


From 6cd136a20579f94ecd18be28e4b5a2fcb46b7387 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 12:32:17 +1300
Subject: [PATCH 2/9] s3:pylibsmb: Rename libsmb_samba_internal Py bindings to
 libsmb

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/libsmb_samba_internal.py | 10 +++++-----
 python/samba/tests/smb.py                   |  6 ++----
 source3/libsmb/pylibsmb.c                   |  8 ++++----
 source3/wscript_build                       |  2 +-
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index ebfed94..5a37b09 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -15,9 +15,9 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-"""Tests for samba.samba3.libsmb_samba_internal."""
+"""Tests for samba.samba3.libsmb."""
 
-from samba.samba3 import libsmb_samba_internal
+from samba.samba3 import libsmb
 from samba.dcerpc import security
 from samba.samba3 import param as s3param
 from samba import credentials
@@ -59,9 +59,9 @@ class LibsmbTestCase(samba.tests.TestCase):
         creds.set_username(os.getenv("USERNAME"))
         creds.set_password(os.getenv("PASSWORD"))
 
-        c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp",
-                                       lp, creds, multi_threaded=True,
-                                       force_smb1=True)
+        c = libsmb.Conn(os.getenv("SERVER_IP"), "tmp",
+                        lp, creds, multi_threaded=True,
+                        force_smb1=True)
 
         mythreads = []
 
diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 9700953..fcb0b7f 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -19,11 +19,10 @@ import samba
 import os
 import random
 import sys
-from samba import smb
 from samba import NTSTATUSError
 from samba.ntstatus import (NT_STATUS_OBJECT_NAME_NOT_FOUND,
                             NT_STATUS_OBJECT_PATH_NOT_FOUND)
-from samba.samba3 import libsmb_samba_internal
+from samba.samba3 import libsmb
 from samba.samba3 import param as s3param
 
 PY3 = sys.version_info[0] == 3
@@ -47,8 +46,7 @@ class SMBTests(samba.tests.TestCase):
         # create an SMB connection to the server
         lp = s3param.get_context()
         lp.load(os.getenv("SMB_CONF_PATH"))
-        self.smb_conn = libsmb_samba_internal.Conn(self.server, "sysvol",
-                                                   lp, creds)
+        self.smb_conn = libsmb.Conn(self.server, "sysvol", lp, creds)
 
         self.smb_conn.mkdir(test_dir)
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 3d7261c..4690a72 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1577,7 +1577,7 @@ static PyMethodDef py_cli_state_methods[] = {
 
 static PyTypeObject py_cli_state_type = {
 	PyVarObject_HEAD_INIT(NULL, 0)
-	.tp_name = "libsmb_samba_internal.Conn",
+	.tp_name = "libsmb.Conn",
 	.tp_basicsize = sizeof(struct py_cli_state),
 	.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
 	.tp_doc = "libsmb connection",
@@ -1591,17 +1591,17 @@ static PyMethodDef py_libsmb_methods[] = {
 	{ NULL },
 };
 
-void initlibsmb_samba_internal(void);
+void initlibsmb(void);
 
 static struct PyModuleDef moduledef = {
     PyModuleDef_HEAD_INIT,
-    .m_name = "libsmb_samba_internal",
+    .m_name = "libsmb",
     .m_doc = "libsmb wrapper",
     .m_size = -1,
     .m_methods = py_libsmb_methods,
 };
 
-MODULE_INIT_FUNC(libsmb_samba_internal)
+MODULE_INIT_FUNC(libsmb)
 {
 	PyObject *m = NULL;
 
diff --git a/source3/wscript_build b/source3/wscript_build
index a8ea8e5..9d188a8 100644
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -1323,7 +1323,7 @@ for env in bld.gen_python_environments():
     bld.SAMBA3_PYTHON('pylibsmb',
                   source='libsmb/pylibsmb.c',
                   deps='smbclient samba-credentials %s' % pycredentials,
-                  realname='samba/samba3/libsmb_samba_internal.so'
+                  realname='samba/samba3/libsmb.so'
                   )
 
 bld.SAMBA3_BINARY('spotlight2sparql',
-- 
2.7.4


From 3a5a7f9ee15fcfd810e895f64d579b4ab98bf268 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 12:37:33 +1300
Subject: [PATCH 3/9] tests: Rename libsmb_samba_internal test to libsmb

So that it better matches the updated Python bindings name.

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/libsmb.py                | 83 +++++++++++++++++++++++++++++
 python/samba/tests/libsmb_samba_internal.py | 83 -----------------------------
 source4/selftest/tests.py                   |  2 +-
 3 files changed, 84 insertions(+), 84 deletions(-)
 create mode 100644 python/samba/tests/libsmb.py
 delete mode 100644 python/samba/tests/libsmb_samba_internal.py

diff --git a/python/samba/tests/libsmb.py b/python/samba/tests/libsmb.py
new file mode 100644
index 0000000..5a37b09
--- /dev/null
+++ b/python/samba/tests/libsmb.py
@@ -0,0 +1,83 @@
+# Unix SMB/CIFS implementation.
+# Copyright Volker Lendecke <vl at samba.org> 2012
+#
+# 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/>.
+#
+
+"""Tests for samba.samba3.libsmb."""
+
+from samba.samba3 import libsmb
+from samba.dcerpc import security
+from samba.samba3 import param as s3param
+from samba import credentials
+import samba.tests
+import threading
+import sys
+import os
+
+
+class LibsmbTestCase(samba.tests.TestCase):
+
+    class OpenClose(threading.Thread):
+
+        def __init__(self, conn, filename, num_ops):
+            threading.Thread.__init__(self)
+            self.conn = conn
+            self.filename = filename
+            self.num_ops = num_ops
+            self.exc = False
+
+        def run(self):
+            c = self.conn
+            try:
+                for i in range(self.num_ops):
+                    f = c.create(self.filename, CreateDisposition=3,
+                                 DesiredAccess=security.SEC_STD_DELETE)
+                    c.delete_on_close(f, True)
+                    c.close(f)
+            except Exception:
+                self.exc = sys.exc_info()
+
+    def test_OpenClose(self):
+
+        lp = s3param.get_context()
+        lp.load(os.getenv("SMB_CONF_PATH"))
+
+        creds = credentials.Credentials()
+        creds.guess(lp)
+        creds.set_username(os.getenv("USERNAME"))
+        creds.set_password(os.getenv("PASSWORD"))
+
+        c = libsmb.Conn(os.getenv("SERVER_IP"), "tmp",
+                        lp, creds, multi_threaded=True,
+                        force_smb1=True)
+
+        mythreads = []
+
+        for i in range(3):
+            t = LibsmbTestCase.OpenClose(c, "test" + str(i), 10)
+            mythreads.append(t)
+
+        for t in mythreads:
+            t.start()
+
+        for t in mythreads:
+            t.join()
+            if t.exc:
+                raise t.exc[0](t.exc[1])
+
+
+if __name__ == "__main__":
+    import unittest
+    unittest.main()
diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
deleted file mode 100644
index 5a37b09..0000000
--- a/python/samba/tests/libsmb_samba_internal.py
+++ /dev/null
@@ -1,83 +0,0 @@
-# Unix SMB/CIFS implementation.
-# Copyright Volker Lendecke <vl at samba.org> 2012
-#
-# 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/>.
-#
-
-"""Tests for samba.samba3.libsmb."""
-
-from samba.samba3 import libsmb
-from samba.dcerpc import security
-from samba.samba3 import param as s3param
-from samba import credentials
-import samba.tests
-import threading
-import sys
-import os
-
-
-class LibsmbTestCase(samba.tests.TestCase):
-
-    class OpenClose(threading.Thread):
-
-        def __init__(self, conn, filename, num_ops):
-            threading.Thread.__init__(self)
-            self.conn = conn
-            self.filename = filename
-            self.num_ops = num_ops
-            self.exc = False
-
-        def run(self):
-            c = self.conn
-            try:
-                for i in range(self.num_ops):
-                    f = c.create(self.filename, CreateDisposition=3,
-                                 DesiredAccess=security.SEC_STD_DELETE)
-                    c.delete_on_close(f, True)
-                    c.close(f)
-            except Exception:
-                self.exc = sys.exc_info()
-
-    def test_OpenClose(self):
-
-        lp = s3param.get_context()
-        lp.load(os.getenv("SMB_CONF_PATH"))
-
-        creds = credentials.Credentials()
-        creds.guess(lp)
-        creds.set_username(os.getenv("USERNAME"))
-        creds.set_password(os.getenv("PASSWORD"))
-
-        c = libsmb.Conn(os.getenv("SERVER_IP"), "tmp",
-                        lp, creds, multi_threaded=True,
-                        force_smb1=True)
-
-        mythreads = []
-
-        for i in range(3):
-            t = LibsmbTestCase.OpenClose(c, "test" + str(i), 10)
-            mythreads.append(t)
-
-        for t in mythreads:
-            t.start()
-
-        for t in mythreads:
-            t.join()
-            if t.exc:
-                raise t.exc[0](t.exc[1])
-
-
-if __name__ == "__main__":
-    import unittest
-    unittest.main()
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 2c2d723..0084de9 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -421,7 +421,7 @@ for t in smbtorture4_testsuites("dlz_bind9."):
     # The dlz_bind9 tests needs to look at the DNS database
     plansmbtorture4testsuite(t, "chgdcpass:local", ["ncalrpc:$SERVER", '-U$USERNAME%$PASSWORD'])
 
-planpythontestsuite("nt4_dc", "samba.tests.libsmb_samba_internal", py3_compatible=True)
+planpythontestsuite("nt4_dc", "samba.tests.libsmb", py3_compatible=True)
 
 # Blackbox Tests:
 # tests that interact directly with the command-line tools rather than using
-- 
2.7.4


From 3bcc0e4c87a714290c7437f24f3498172ef3aa97 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 12:40:49 +1300
Subject: [PATCH 4/9] s3:pylibsmb: Rename 'credentials' param to match s4

s4 just calls it 'creds'. Renaming it will make it easier to convert
existing SMB connections over to use the new bindings.

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source3/libsmb/pylibsmb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 4690a72..386fcc5 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -446,7 +446,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	int flags = 0;
 
 	static const char *kwlist[] = {
-		"host", "share", "lp", "credentials",
+		"host", "share", "lp", "creds",
 		"multi_threaded", "sign", "force_smb1",
 		NULL
 	};
-- 
2.7.4


From 2f3bc395591f624d4c0804e79732f6e7a8a843ec Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 14 Dec 2018 10:30:38 +1300
Subject: [PATCH 5/9] s3:pylibsmb: Add flags used by .list() to SMB Py bindings

These flags are exposed by the s4 code. Python code that calls .list()
checks the returned attribs/mode for the directory listing, e.g. to work
out whether something is a sub-directory:

  if item['attrib'] & libsmb.FILE_ATTRIBUTE_DIRECTORY...

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source3/libsmb/pylibsmb.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 386fcc5..3e0736d 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1616,5 +1616,25 @@ MODULE_INIT_FUNC(libsmb)
 	}
 	Py_INCREF(&py_cli_state_type);
 	PyModule_AddObject(m, "Conn", (PyObject *)&py_cli_state_type);
+
+#define ADD_FLAGS(val)	PyModule_AddObject(m, #val, PyInt_FromLong(val))
+
+	ADD_FLAGS(FILE_ATTRIBUTE_READONLY);
+	ADD_FLAGS(FILE_ATTRIBUTE_HIDDEN);
+	ADD_FLAGS(FILE_ATTRIBUTE_SYSTEM);
+	ADD_FLAGS(FILE_ATTRIBUTE_VOLUME);
+	ADD_FLAGS(FILE_ATTRIBUTE_DIRECTORY);
+	ADD_FLAGS(FILE_ATTRIBUTE_ARCHIVE);
+	ADD_FLAGS(FILE_ATTRIBUTE_DEVICE);
+	ADD_FLAGS(FILE_ATTRIBUTE_NORMAL);
+	ADD_FLAGS(FILE_ATTRIBUTE_TEMPORARY);
+	ADD_FLAGS(FILE_ATTRIBUTE_SPARSE);
+	ADD_FLAGS(FILE_ATTRIBUTE_REPARSE_POINT);
+	ADD_FLAGS(FILE_ATTRIBUTE_COMPRESSED);
+	ADD_FLAGS(FILE_ATTRIBUTE_OFFLINE);
+	ADD_FLAGS(FILE_ATTRIBUTE_NONINDEXED);
+	ADD_FLAGS(FILE_ATTRIBUTE_ENCRYPTED);
+	ADD_FLAGS(FILE_ATTRIBUTE_ALL_MASK);
+
 	return m;
 }
-- 
2.7.4


From 806102a8892b0834e18846f32b7cb4c33fbf3100 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 14 Dec 2018 10:34:48 +1300
Subject: [PATCH 6/9] python/ntacls: Convert ntacls to use s3 flags

This helper code is just using the flags defined by the Python bindings.
Convert it over to use s3 bindings instead of s4.

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

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

diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
index 6ac5a38..1ab5fd3 100644
--- a/python/samba/ntacls.py
+++ b/python/samba/ntacls.py
@@ -32,13 +32,13 @@ from samba.samba3 import param as s3param
 from samba.dcerpc import security, xattr, idmap
 from samba.ndr import ndr_pack, ndr_unpack
 from samba.samba3 import smbd
-from samba import smb
+from samba.samba3 import libsmb
 
 # don't include volumes
-SMB_FILE_ATTRIBUTE_FLAGS = smb.FILE_ATTRIBUTE_SYSTEM | \
-                           smb.FILE_ATTRIBUTE_DIRECTORY | \
-                           smb.FILE_ATTRIBUTE_ARCHIVE | \
-                           smb.FILE_ATTRIBUTE_HIDDEN
+SMB_FILE_ATTRIBUTE_FLAGS = libsmb.FILE_ATTRIBUTE_SYSTEM | \
+                           libsmb.FILE_ATTRIBUTE_DIRECTORY | \
+                           libsmb.FILE_ATTRIBUTE_ARCHIVE | \
+                           libsmb.FILE_ATTRIBUTE_HIDDEN
 
 
 SECURITY_SECINFO_FLAGS = security.SECINFO_OWNER | \
@@ -348,7 +348,7 @@ class SMBHelper:
 
         attrib is from list method.
         """
-        return bool(attrib & smb.FILE_ATTRIBUTE_DIRECTORY)
+        return bool(attrib & libsmb.FILE_ATTRIBUTE_DIRECTORY)
 
     def join(self, root, name):
         """
-- 
2.7.4


From c6bb3c78a00d80e9e8b7d0beffc980647440d6a1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 14 Dec 2018 09:50:02 +1300
Subject: [PATCH 7/9] tests: Avoid hardcoding domain in GPO tests

The realm varies by testenv. Currently the GPO tests will only run on
the ad_dc testenv.

A slightly better solution is to use the REALM environmental variable,
so that the test can run on any testenv.

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

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

diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index 84e59ed..69252af 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -28,8 +28,13 @@ from samba.gp_sec_ext import gp_sec_ext
 import logging
 from samba.credentials import Credentials
 
-poldir = r'\\addom.samba.example.com\sysvol\addom.samba.example.com\Policies'
-dspath = 'CN=Policies,CN=System,DC=addom,DC=samba,DC=example,DC=com'
+realm = os.environ.get('REALM')
+policies = realm + '/POLICIES'
+realm = realm.lower()
+poldir = r'\\{0}\sysvol\{0}\Policies'.format(realm)
+# the first part of the base DN varies by testenv. Work it out from the realm
+base_dn = 'DC={0},DC=samba,DC=example,DC=com'.format(realm.split('.')[0])
+dspath = 'CN=Policies,CN=System,' + base_dn
 gpt_data = '[General]\nVersion=%d'
 
 def days2rel_nttime(val):
@@ -78,6 +83,7 @@ class GPOTests(tests.TestCase):
     def setUp(self):
         super(GPOTests, self).setUp()
         self.server = os.environ["SERVER"]
+        self.dc_account = self.server.upper() + '$'
         self.lp = LoadParm()
         self.lp.load_default()
         self.creds = self.insta_creds(template=self.get_credentials())
@@ -111,7 +117,6 @@ class GPOTests(tests.TestCase):
     def test_gpt_version(self):
         global gpt_data
         local_path = self.lp.cache_path('gpo_cache')
-        policies = 'ADDOM.SAMBA.EXAMPLE.COM/POLICIES'
         guid = '{31B2F340-016D-11D2-945F-00C04FB984F9}'
         gpo_path = os.path.join(local_path, policies, guid)
         old_vers = gpo.gpo_get_sysvol_gpt_version(gpo_path)[1]
@@ -137,7 +142,7 @@ class GPOTests(tests.TestCase):
                         'GPO cache %s was not created' % cache)
 
         guid = '{31B2F340-016D-11D2-945F-00C04FB984F9}'
-        gpt_ini = os.path.join(cache, 'ADDOM.SAMBA.EXAMPLE.COM/POLICIES',
+        gpt_ini = os.path.join(cache, policies,
                                guid, 'GPT.INI')
         self.assertTrue(os.path.exists(gpt_ini),
                         'GPT.INI was not cached for %s' % guid)
@@ -151,9 +156,9 @@ class GPOTests(tests.TestCase):
         self.assertEqual(check_safe_path('\\\\etc/\\passwd'), 'etc/passwd')
 
         # there should be no backslashes used to delineate paths
-        before = 'sysvol/addom.samba.example.com\\Policies/' \
+        before = 'sysvol/' + realm + '\\Policies/' \
             '{31B2F340-016D-11D2-945F-00C04FB984F9}\\GPT.INI'
-        after = 'addom.samba.example.com/Policies/' \
+        after = realm + '/Policies/' \
             '{31B2F340-016D-11D2-945F-00C04FB984F9}/GPT.INI'
         result = check_safe_path(before)
         self.assertEquals(result, after, 'check_safe_path() didn\'t'
@@ -200,7 +205,7 @@ class GPOTests(tests.TestCase):
         local_path = self.lp.get('path', 'sysvol')
         guids = ['{31B2F340-016D-11D2-945F-00C04FB984F9}',
                  '{6AC1786C-016F-11D2-945F-00C04FB984F9}']
-        gpofile = '%s/addom.samba.example.com/Policies/%s/MACHINE/Microsoft/' \
+        gpofile = '%s/' + realm + '/Policies/%s/MACHINE/Microsoft/' \
                   'Windows NT/SecEdit/GptTmpl.inf'
         stage = '[System Access]\nMinimumPasswordAge = 998\n'
         cache_dir = self.lp.get('cache directory')
@@ -213,7 +218,7 @@ class GPOTests(tests.TestCase):
         ret = gpupdate_force(self.lp)
         self.assertEquals(ret, 0, 'gpupdate force failed')
 
-        gp_db = store.get_gplog('ADDC$')
+        gp_db = store.get_gplog(self.dc_account)
 
         applied_guids = gp_db.get_applied_guids()
         self.assertEquals(len(applied_guids), 2, 'The guids were not found')
@@ -239,7 +244,7 @@ class GPOTests(tests.TestCase):
 
         ads = gpo.ADS_STRUCT(self.server, self.lp, self.creds)
         if ads.connect():
-            gpos = ads.get_gpo_list('ADDC$')
+            gpos = ads.get_gpo_list(self.dc_account)
         del_gpos = get_deleted_gpos_list(gp_db, gpos[:-1])
         self.assertEqual(len(del_gpos), 1, 'Returned delete gpos is incorrect')
         self.assertEqual(guids[-1], del_gpos[0][0],
@@ -260,7 +265,7 @@ class GPOTests(tests.TestCase):
         local_path = self.lp.cache_path('gpo_cache')
         guids = ['{31B2F340-016D-11D2-945F-00C04FB984F9}',
                  '{6AC1786C-016F-11D2-945F-00C04FB984F9}']
-        gpofile = '%s/ADDOM.SAMBA.EXAMPLE.COM/POLICIES/%s/MACHINE/MICROSOFT/' \
+        gpofile = '%s/' + policies + '/%s/MACHINE/MICROSOFT/' \
                   'WINDOWS NT/SECEDIT/GPTTMPL.INF'
         logger = logging.getLogger('gpo_tests')
         cache_dir = self.lp.get('cache directory')
-- 
2.7.4


From 2052d43c94e77a90949bbedcfbdfb4f4dc7ee29a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Fri, 14 Dec 2018 10:47:45 +1300
Subject: [PATCH 8/9] netcmd: Refactor duplicated SMB connect in GPO commands

Do the SMB connection in a single helper function.

Note: this highlights that perhaps we want all SMB connections to be
signed, but we can fix that up separately.

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

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/netcmd/gpo.py | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py
index f1f1e98..b65fb7c 100644
--- a/python/samba/netcmd/gpo.py
+++ b/python/samba/netcmd/gpo.py
@@ -360,6 +360,14 @@ def create_directory_hier(conn, remotedir):
         if not conn.chkpath(path):
             conn.mkdir(path)
 
+def smb_connection(dc_hostname, service, lp, creds, sign=False):
+    # SMB connect to DC
+    try:
+        conn = smb.SMB(dc_hostname, service, lp=lp, creds=creds, sign=sign)
+    except Exception:
+        raise CommandError("Error connecting to '%s' using SMB" % dc_hostname)
+    return conn
+
 
 class GPOCommand(Command):
     def construct_tmpdir(self, tmpdir, gpo):
@@ -964,14 +972,8 @@ class cmd_fetch(GPOCommand):
             raise CommandError("Invalid GPO path (%s)" % unc)
 
         # SMB connect to DC
-        try:
-            conn = smb.SMB(dc_hostname,
-                           service,
-                           lp=self.lp,
-                           creds=self.creds,
-                           sign=True)
-        except Exception:
-            raise CommandError("Error connecting to '%s' using SMB" % dc_hostname)
+        conn = smb_connection(dc_hostname, service, lp=self.lp,
+                              creds=self.creds, sign=True)
 
         # Copy GPT
         tmpdir, gpodir = self.construct_tmpdir(tmpdir, gpo)
@@ -1033,10 +1035,8 @@ class cmd_backup(GPOCommand):
             raise CommandError("Invalid GPO path (%s)" % unc)
 
         # SMB connect to DC
-        try:
-            conn = smb.SMB(dc_hostname, service, lp=self.lp, creds=self.creds)
-        except Exception:
-            raise CommandError("Error connecting to '%s' using SMB" % dc_hostname)
+        conn = smb_connection(dc_hostname, service, lp=self.lp,
+                              creds=self.creds)
 
         # Copy GPT
         tmpdir, gpodir = self.construct_tmpdir(tmpdir, gpo)
@@ -1192,10 +1192,8 @@ class cmd_create(GPOCommand):
         # Connect to DC over SMB
         [dom_name, service, sharepath] = parse_unc(unc_path)
         self.sharepath = sharepath
-        try:
-            conn = smb.SMB(dc_hostname, service, lp=self.lp, creds=self.creds)
-        except Exception as e:
-            raise CommandError("Error connecting to '%s' using SMB" % dc_hostname, e)
+        conn = smb_connection(dc_hostname, service, lp=self.lp,
+                              creds=self.creds)
 
         self.conn = conn
 
@@ -1449,10 +1447,8 @@ class cmd_del(GPOCommand):
 
         # Connect to DC over SMB
         [dom_name, service, sharepath] = parse_unc(unc_path)
-        try:
-            conn = smb.SMB(dc_hostname, service, lp=self.lp, creds=self.creds)
-        except Exception as e:
-            raise CommandError("Error connecting to '%s' using SMB" % dc_hostname, e)
+        conn = smb_connection(dc_hostname, service, lp=self.lp,
+                              creds=self.creds)
 
         self.samdb.transaction_start()
         try:
@@ -1527,10 +1523,8 @@ class cmd_aclcheck(GPOCommand):
                 raise CommandError("Invalid GPO path (%s)" % unc)
 
             # SMB connect to DC
-            try:
-                conn = smb.SMB(dc_hostname, service, lp=self.lp, creds=self.creds)
-            except Exception:
-                raise CommandError("Error connecting to '%s' using SMB" % dc_hostname)
+            conn = smb_connection(dc_hostname, service, lp=self.lp,
+                                  creds=self.creds)
 
             fs_sd = conn.get_acl(sharepath, security.SECINFO_OWNER | security.SECINFO_GROUP | security.SECINFO_DACL, security.SEC_FLAG_MAXIMUM_ALLOWED)
 
-- 
2.7.4


From 4701c356a0b54eb4297ea67e9e056e4d4366239e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 16:50:00 +1300
Subject: [PATCH 9/9] netcmd: Small refactor to SMB connection in domain backup

Rework the code so we only do this in one place.

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

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

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index c7602a8..58d5a4c 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -101,6 +101,11 @@ def get_sid_for_restore(samdb):
     return str(sid) + '-' + str(rid)
 
 
+def smb_sysvol_conn(server, lp, creds):
+    """Returns an SMB connection to the sysvol share on the DC"""
+    return smb.SMB(server, "sysvol", lp=lp, creds=creds, sign=True)
+
+
 def get_timestamp():
     return datetime.datetime.now().isoformat().replace(':', '-')
 
@@ -251,7 +256,7 @@ class cmd_domain_backup_online(samba.netcmd.Command):
 
         # Grab the remote DC's sysvol files and bundle them into a tar file
         sysvol_tar = os.path.join(tmpdir, 'sysvol.tar.gz')
-        smb_conn = smb.SMB(server, "sysvol", lp=lp, creds=creds, sign=True)
+        smb_conn = smb_sysvol_conn(server, lp, creds)
         backup_online(smb_conn, sysvol_tar, remote_sam.get_domain_sid())
 
         # remove the default sysvol files created by the clone (we want to
@@ -836,7 +841,7 @@ class cmd_domain_backup_rename(samba.netcmd.Command):
         # use the old realm) backed here, as well as default files generated
         # for the new realm as part of the clone/join.
         sysvol_tar = os.path.join(tmpdir, 'sysvol.tar.gz')
-        smb_conn = smb.SMB(server, "sysvol", lp=lp, creds=creds, sign=True)
+        smb_conn = smb_sysvol_conn(server, lp, creds)
         backup_online(smb_conn, sysvol_tar, remote_sam.get_domain_sid())
 
         # connect to the local DB (making sure we use the new/renamed config)
-- 
2.7.4



More information about the samba-technical mailing list