[PATCH] Use conn->session_info->security_token in posix_acls.c to make sysvolreset faster (was: Re: [PATCH] improve performance for samba-tool ntacl sysvolreset)

Andrew Bartlett abartlet at samba.org
Wed Jul 11 05:33:01 UTC 2018


On Tue, 2018-07-10 at 07:49 +0300, Uri Simchoni wrote:
> Hi,
> 
> The idea of using whatever information we already have instead of
> re-calculating it is correct IMHO.
> 
> What I like less is the idea of an incomplete session_info that we have
> to second-guess, as in 3/16 (checking for null unix_info). We may miss
> checks now or on in the future, and null-case-handling may be
> non-uniform. AFAICT we don't have checks like this in the code, which is
> (hopefully) a testament for this field always being populated when the
> session_info is built.
> 
> So I'm just asking - what would it take to construct a session info with
> unix_info on the Python side.

I was very hopeful, but sadly the security_token_to_unix_token() that I
wrapped via auth_session_info_fill_unix() fails if winbindd isn't
running.  

Winbindd can't start until after provision, but provision and the
domain restore is an important user of this API.  (Internally in smbd
the SID->ID handling falls back to passdb if winbindd isn't up, but
this is not available to the token manipulation APIs right now. 

I could try for this, catch the exception and cope without the token in
that case, but for now I would prefer to proceed with the patch as-was.

I've added the bindings and tests that operate no token and with and
without the unix part of the token.

Let me know if you are OK with the above, or would prefer no token (and
slower if winbind is not up) to the partial token.

CI: https://gitlab.com/catalyst-samba/samba/pipelines/25524636

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 744fe7648d5af28c24e099b2807de159e3956f05 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 10:05:50 +1200
Subject: [PATCH 01/20] pysmbd: add session_info arg to get_conn_tos

Add session_info arg, so caller can pass it in to reuse authentication info
later. This will improve performance a lot while doing ntacl operations
on large amount of files, e.g.: sysvolreset.

Modification for upstream caller will come in following patches.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/smbd/pysmbd.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index b220fbe691f..faf4565fff9 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -44,7 +44,10 @@ extern const struct generic_mapping file_generic_mapping;
 #define DIRECTORY_FLAGS O_RDONLY
 #endif
 
-static connection_struct *get_conn_tos(const char *service)
+
+static connection_struct *get_conn_tos(
+	const char *service,
+	const struct auth_session_info *session_info)
 {
 	struct conn_struct_tos *c = NULL;
 	int snum = -1;
@@ -66,7 +69,7 @@ static connection_struct *get_conn_tos(const char *service)
 	status = create_conn_struct_tos(NULL,
 					snum,
 					"/",
-					NULL,
+					session_info,
 					&c);
 	PyErr_NTSTATUS_IS_ERR_RAISE(status);
 
@@ -410,7 +413,7 @@ static PyObject *py_smbd_set_simple_acl(PyObject *self, PyObject *args, PyObject
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -451,7 +454,7 @@ static PyObject *py_smbd_chown(PyObject *self, PyObject *args, PyObject *kwargs)
 
 	frame = talloc_stackframe();
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -510,7 +513,7 @@ static PyObject *py_smbd_unlink(PyObject *self, PyObject *args, PyObject *kwargs
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -576,7 +579,7 @@ static PyObject *py_smbd_set_nt_acl(PyObject *self, PyObject *args, PyObject *kw
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -611,7 +614,7 @@ static PyObject *py_smbd_get_nt_acl(PyObject *self, PyObject *args, PyObject *kw
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -653,7 +656,7 @@ static PyObject *py_smbd_set_sys_acl(PyObject *self, PyObject *args, PyObject *k
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -694,7 +697,7 @@ static PyObject *py_smbd_get_sys_acl(PyObject *self, PyObject *args, PyObject *k
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -739,7 +742,7 @@ static PyObject *py_smbd_mkdir(PyObject *self, PyObject *args, PyObject *kwargs)
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
@@ -792,7 +795,7 @@ static PyObject *py_smbd_create_file(PyObject *self, PyObject *args, PyObject *k
 		return NULL;
 	}
 
-	conn = get_conn_tos(service);
+	conn = get_conn_tos(service, NULL);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
-- 
2.11.0


From ba05d5530688539482f4132683dacf06a78353ea Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 10:18:30 +1200
Subject: [PATCH 02/20] pysmbd: add session_info arg to py_smbd_set_nt_acl

Add session_info arg as optional and pass it down to get_conn_tos.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/smbd/pysmbd.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index faf4565fff9..1431925efd0 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -556,20 +556,26 @@ static PyObject *py_smbd_have_posix_acls(PyObject *self)
  */
 static PyObject *py_smbd_set_nt_acl(PyObject *self, PyObject *args, PyObject *kwargs)
 {
-	const char * const kwnames[] = { "fname", "security_info_sent", "sd", "service", NULL };
+	const char * const kwnames[] = {
+		"fname", "security_info_sent", "sd",
+		"service", "session_info", NULL };
+
 	NTSTATUS status;
 	char *fname, *service = NULL;
 	int security_info_sent;
 	PyObject *py_sd;
 	struct security_descriptor *sd;
+	PyObject *py_session = Py_None;
+	struct auth_session_info *session_info = NULL;
 	connection_struct *conn;
 	TALLOC_CTX *frame;
 
 	frame = talloc_stackframe();
 
-	if (!PyArg_ParseTupleAndKeywords(args, kwargs,
-					 "siO|z", discard_const_p(char *, kwnames),
-					 &fname, &security_info_sent, &py_sd, &service)) {
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "siO|zO",
+				         discard_const_p(char *, kwnames),
+					 &fname, &security_info_sent, &py_sd,
+					 &service, &py_session)) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -579,7 +585,24 @@ static PyObject *py_smbd_set_nt_acl(PyObject *self, PyObject *args, PyObject *kw
 		return NULL;
 	}
 
-	conn = get_conn_tos(service, NULL);
+	if (py_session != Py_None) {
+		if (!py_check_dcerpc_type(py_session,
+					  "samba.dcerpc.auth",
+					  "session_info")) {
+			TALLOC_FREE(frame);
+			return NULL;
+		}
+		session_info = pytalloc_get_type(py_session,
+						 struct auth_session_info);
+		if (!session_info) {
+			PyErr_Format(PyExc_TypeError,
+				     "Expected auth_session_info for session_info argument got %s",
+				     talloc_get_name(pytalloc_get_ptr(py_session)));
+			return NULL;
+		}
+	}
+
+	conn = get_conn_tos(service, session_info);
 	if (!conn) {
 		TALLOC_FREE(frame);
 		return NULL;
-- 
2.11.0


From 9d6bd352c2392f2e8c90378e1ec1c6b5b4eb46b4 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 11:03:42 +1200
Subject: [PATCH 03/20] smbd/msdfs: add null check for session_info.unix_info

When a session_info passed down to here, the unix_info could be NULL.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/smbd/msdfs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index bac9d8f6bf6..f0ec6b84892 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -307,7 +307,12 @@ static NTSTATUS create_conn_struct_as_root(TALLOC_CTX *ctx,
 			TALLOC_FREE(conn);
 			return NT_STATUS_NO_MEMORY;
 		}
-		vfs_user = conn->session_info->unix_info->unix_name;
+		/* unix_info could be NULL in session_info */
+		if (conn->session_info->unix_info != NULL) {
+			vfs_user = conn->session_info->unix_info->unix_name;
+		} else {
+			vfs_user = get_current_username();
+		}
 	} else {
 		/* use current authenticated user in absence of session_info */
 		vfs_user = get_current_username();
-- 
2.11.0


From 38bac9f24aa76a4fc2abcd8de337ce89cdddd55f Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 11:09:50 +1200
Subject: [PATCH 04/20] smbd/posix_acls: reuse secutiry token from session info
 if exist

If session info was passed down from upstream, then try to use it to get
security token, other then creating token every time.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/smbd/posix_acls.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 70834d5fc7d..8cc9cf1f2fc 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1251,12 +1251,38 @@ static void ensure_minimal_owner_ace_perms(const bool is_directory,
 
 static bool uid_entry_in_group(connection_struct *conn, canon_ace *uid_ace, canon_ace *group_ace )
 {
+	bool is_sid = false;
+	bool has_sid = false;
+	struct security_token *security_token = NULL;
+
 	/* "Everyone" always matches every uid. */
 
 	if (dom_sid_equal(&group_ace->trustee, &global_sid_World))
 		return True;
 
 	/*
+	 * if we have session info in conn, we already have the (SID
+	 * based) NT token and don't need to do the complex
+	 * user_in_group_sid() call
+	 */
+	if (conn->session_info) {
+		security_token = conn->session_info->security_token;
+		/* security_token should not be NULL */
+		SMB_ASSERT(security_token);
+		is_sid = security_token_is_sid(security_token,
+					       &uid_ace->trustee);
+		if (is_sid) {
+			has_sid = security_token_has_sid(security_token,
+							 &group_ace->trustee);
+
+			if (has_sid) {
+				return true;
+			}
+		}
+
+	}
+
+	/*
 	 * if it's the current user, we already have the unix token
 	 * and don't need to do the complex user_in_group_sid() call
 	 */
-- 
2.11.0


From 7321556dd511f127c902dc3c4862ca4b87bde062 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Tue, 3 Jul 2018 10:20:39 +1200
Subject: [PATCH 05/20] ntacls: reuse predefined SECURITY_SECINFO_FLAGS

Use predefined SECURITY_SECINFO_FLAGS to replace bitwise or operations
on flag list.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/ntacls.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
index e5178115f66..dee906acd21 100644
--- a/python/samba/ntacls.py
+++ b/python/samba/ntacls.py
@@ -114,7 +114,7 @@ def getntacl(lp, file, backend=None, eadbfile=None, direct_db_access=True, servi
         elif ntacl.version == 4:
             return ntacl.info.sd
     else:
-        return smbd.get_nt_acl(file, security.SECINFO_OWNER | security.SECINFO_GROUP | security.SECINFO_DACL | security.SECINFO_SACL, service=service)
+        return smbd.get_nt_acl(file, SECURITY_SECINFO_FLAGS, service=service)
 
 
 def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True, skip_invalid_chown=False, passdb=None, service=None):
@@ -150,7 +150,7 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True
                     sd2 = sd
                     sd2.owner_sid = administrator
 
-                    smbd.set_nt_acl(file, security.SECINFO_OWNER |security.SECINFO_GROUP | security.SECINFO_DACL | security.SECINFO_SACL, sd2, service=service)
+                    smbd.set_nt_acl(file, SECURITY_SECINFO_FLAGS, sd2, service=service)
 
                     # and then set an NTVFS ACL (which does not set the posix ACL) to pretend the owner really was set
                     use_ntvfs = True
@@ -184,7 +184,7 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True
             samba.xattr_native.wrap_setxattr(file, xattr.XATTR_NTACL_NAME,
                                              ndr_pack(ntacl))
     else:
-        smbd.set_nt_acl(file, security.SECINFO_OWNER | security.SECINFO_GROUP | security.SECINFO_DACL | security.SECINFO_SACL, sd, service=service)
+        smbd.set_nt_acl(file, SECURITY_SECINFO_FLAGS, sd, service=service)
 
 
 def ldapmask2filemask(ldm):
-- 
2.11.0


From e4de608ab88c799d7db01039ded9dee174925518 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 10:27:23 +1200
Subject: [PATCH 06/20] ntacls: add session_info arg to setntacl and pass down
 to set_nt_acl api

Then underneath code can reuse the authentication info in session to
improve performance.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/ntacls.py | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
index dee906acd21..32ceb54fd1b 100644
--- a/python/samba/ntacls.py
+++ b/python/samba/ntacls.py
@@ -30,6 +30,7 @@ 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.auth import admin_session
 from samba import smb
 
 # don't include volumes
@@ -117,7 +118,28 @@ def getntacl(lp, file, backend=None, eadbfile=None, direct_db_access=True, servi
         return smbd.get_nt_acl(file, SECURITY_SECINFO_FLAGS, service=service)
 
 
-def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True, skip_invalid_chown=False, passdb=None, service=None):
+def setntacl(lp, file, sddl, domsid,
+             backend=None, eadbfile=None,
+             use_ntvfs=True, skip_invalid_chown=False,
+             passdb=None, service=None, session_info=None):
+    """
+    A wrapper for smbd set_nt_acl api.
+
+    Args:
+        lp (LoadParam): load param from conf
+        file (str): a path to file or dir
+        sddl (str): ntacl sddl string
+        service (str): name of share service, e.g.: sysvol
+        session_info (auth_session_info): session info for authentication
+
+    Note:
+        Get `session_info` with `samba.auth.user_session`, do not use the
+        `admin_session` api.
+
+    Returns:
+        None
+    """
+
     assert(isinstance(domsid, str) or isinstance(domsid, security.dom_sid))
     if isinstance(domsid, str):
         sid = security.dom_sid(domsid)
@@ -150,7 +172,9 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True
                     sd2 = sd
                     sd2.owner_sid = administrator
 
-                    smbd.set_nt_acl(file, SECURITY_SECINFO_FLAGS, sd2, service=service)
+                    smbd.set_nt_acl(
+                        file, SECURITY_SECINFO_FLAGS, sd2,
+                        service=service, session_info=session_info)
 
                     # and then set an NTVFS ACL (which does not set the posix ACL) to pretend the owner really was set
                     use_ntvfs = True
@@ -163,7 +187,12 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True
                 # This won't work in test environments, as it tries a real (rather than xattr-based fake) chown
 
                 os.chown(file, 0, 0)
-                smbd.set_nt_acl(file, security.SECINFO_GROUP | security.SECINFO_DACL | security.SECINFO_SACL, sd, service=service)
+                smbd.set_nt_acl(
+                    file,
+                    security.SECINFO_GROUP |
+                    security.SECINFO_DACL |
+                    security.SECINFO_SACL,
+                    sd, service=service, session_info=session_info)
 
     if use_ntvfs:
         (backend_obj, dbname) = checkset_backend(lp, backend, eadbfile)
@@ -184,7 +213,9 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None, use_ntvfs=True
             samba.xattr_native.wrap_setxattr(file, xattr.XATTR_NTACL_NAME,
                                              ndr_pack(ntacl))
     else:
-        smbd.set_nt_acl(file, SECURITY_SECINFO_FLAGS, sd, service=service)
+        smbd.set_nt_acl(
+            file, SECURITY_SECINFO_FLAGS, sd,
+            service=service, session_info=session_info)
 
 
 def ldapmask2filemask(ldm):
-- 
2.11.0


From e745a57948ad87390dc03855e6e0e737f730c898 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 12:07:25 +1200
Subject: [PATCH 07/20] provision/setsysvolacl: build session_info and pass
 down to setntacl

Get the admin session info, and pass it down to setntacl.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/provision/__init__.py | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 8bdb95ccfa8..976503ecc0c 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -46,6 +46,7 @@ import ldb
 
 from samba.auth import system_session, admin_session
 import samba
+from samba import auth
 from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
 from samba.dsdb import DS_DOMAIN_FUNCTION_2000
@@ -1687,23 +1688,36 @@ def setsysvolacl(samdb, netlogon, sysvol, uid, gid, domainsid, dnsdomain,
     else:
         canchown = True
 
+    # use admin sid dn as user dn, since admin should own most of the files,
+    # the operation will be much faster
+    userdn = '<SID={}-{}>'.format(domainsid, security.DOMAIN_RID_ADMINISTRATOR)
+
+    flags = (auth.AUTH_SESSION_INFO_DEFAULT_GROUPS |
+             auth.AUTH_SESSION_INFO_AUTHENTICATED |
+             auth.AUTH_SESSION_INFO_SIMPLE_PRIVILEGES)
+
+    session_info = auth.user_session(samdb, lp_ctx=lp, dn=userdn,
+                                     session_info_flags=flags)
+
     # Set the SYSVOL_ACL on the sysvol folder and subfolder (first level)
     setntacl(lp,sysvol, SYSVOL_ACL, str(domainsid), use_ntvfs=use_ntvfs,
              skip_invalid_chown=True, passdb=s4_passdb,
-             service=SYSVOL_SERVICE)
+             service=SYSVOL_SERVICE, session_info=session_info)
     for root, dirs, files in os.walk(sysvol, topdown=False):
         for name in files:
             if use_ntvfs and canchown:
                 os.chown(os.path.join(root, name), -1, gid)
             setntacl(lp, os.path.join(root, name), SYSVOL_ACL, str(domainsid),
                      use_ntvfs=use_ntvfs, skip_invalid_chown=True,
-                     passdb=s4_passdb, service=SYSVOL_SERVICE)
+                     passdb=s4_passdb, service=SYSVOL_SERVICE,
+                     session_info=session_info)
         for name in dirs:
             if use_ntvfs and canchown:
                 os.chown(os.path.join(root, name), -1, gid)
             setntacl(lp, os.path.join(root, name), SYSVOL_ACL, str(domainsid),
                      use_ntvfs=use_ntvfs, skip_invalid_chown=True,
-                     passdb=s4_passdb, service=SYSVOL_SERVICE)
+                     passdb=s4_passdb, service=SYSVOL_SERVICE,
+                     session_info=session_info)
 
     # Set acls on Policy folder and policies folders
     set_gpos_acl(sysvol, dnsdomain, domainsid, domaindn, samdb, lp, use_ntvfs, passdb=s4_passdb)
-- 
2.11.0


From 34fd290cf8d1060b6b5dca8e4e498bf675ebe251 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 13:03:44 +1200
Subject: [PATCH 08/20] provision/setsysvolacl: create helper function to
 simplify code

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/provision/__init__.py | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 976503ecc0c..066411ab8d7 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1699,25 +1699,24 @@ def setsysvolacl(samdb, netlogon, sysvol, uid, gid, domainsid, dnsdomain,
     session_info = auth.user_session(samdb, lp_ctx=lp, dn=userdn,
                                      session_info_flags=flags)
 
+    def _setntacl(path):
+        """A helper to reuse args"""
+        return setntacl(
+            lp, path, SYSVOL_ACL, str(domainsid),
+            use_ntvfs=use_ntvfs, skip_invalid_chown=True, passdb=s4_passdb,
+            service=SYSVOL_SERVICE, session_info=session_info)
+
     # Set the SYSVOL_ACL on the sysvol folder and subfolder (first level)
-    setntacl(lp,sysvol, SYSVOL_ACL, str(domainsid), use_ntvfs=use_ntvfs,
-             skip_invalid_chown=True, passdb=s4_passdb,
-             service=SYSVOL_SERVICE, session_info=session_info)
+    _setntacl(sysvol)
     for root, dirs, files in os.walk(sysvol, topdown=False):
         for name in files:
             if use_ntvfs and canchown:
                 os.chown(os.path.join(root, name), -1, gid)
-            setntacl(lp, os.path.join(root, name), SYSVOL_ACL, str(domainsid),
-                     use_ntvfs=use_ntvfs, skip_invalid_chown=True,
-                     passdb=s4_passdb, service=SYSVOL_SERVICE,
-                     session_info=session_info)
+            _setntacl(os.path.join(root, name))
         for name in dirs:
             if use_ntvfs and canchown:
                 os.chown(os.path.join(root, name), -1, gid)
-            setntacl(lp, os.path.join(root, name), SYSVOL_ACL, str(domainsid),
-                     use_ntvfs=use_ntvfs, skip_invalid_chown=True,
-                     passdb=s4_passdb, service=SYSVOL_SERVICE,
-                     session_info=session_info)
+            _setntacl(os.path.join(root, name))
 
     # Set acls on Policy folder and policies folders
     set_gpos_acl(sysvol, dnsdomain, domainsid, domaindn, samdb, lp, use_ntvfs, passdb=s4_passdb)
-- 
2.11.0


From dff1c4541d6558ffbc67d30c00ea62614a24abc2 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 14:52:02 +1200
Subject: [PATCH 09/20] tests/posixacl: rm commented code

The example is already in code, no need to keep it here.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 74cabf1bb70..38f578e0d35 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -28,14 +28,6 @@ import os
 from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
 
-# To print a posix ACL use:
-#        for entry in posix_acl.acl:
-#            print "a_type: %d" % entry.a_type
-#            print "a_perm: %o" % entry.a_perm
-#            if entry.a_type == smb_acl.SMB_ACL_USER:
-#                print "uid: %d" % entry.uid
-#            if entry.a_type == smb_acl.SMB_ACL_GROUP:
-#                print "gid: %d" % entry.gid
 
 class PosixAclMappingTests(TestCaseInTempDir):
 
-- 
2.11.0


From 4566d437fea84f0eb16c64a7cdf8e711e1d4698f Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 15:18:26 +1200
Subject: [PATCH 10/20] tests/posixacl: define global DOM_SID to make code DRY

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 38f578e0d35..462ee7ef12d 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -28,6 +28,8 @@ import os
 from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
 
+DOM_SID = "S-1-5-21-2212615479-2695158682-2101375467"
+
 
 class PosixAclMappingTests(TestCaseInTempDir):
 
@@ -44,18 +46,18 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
 
     def test_setntacl_smbd_getntacl(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
         facl = getntacl(self.lp, self.tempf, direct_db_access=True)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_setposixacl_getntacl(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
 
         # This will invalidate the ACL, as we have a hook!
         smbd.set_simple_acl(self.tempf, 0o640)
@@ -69,7 +71,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_invalidate_getntacl(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -83,7 +85,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_invalidate_getntacl_smbd(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -99,7 +101,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x001200a9;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
         os.chmod(self.tempf, 0o750)
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -113,14 +115,14 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_getntacl_smbd(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_getntacl_smbd(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
@@ -128,7 +130,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
     def test_setntacl_smbd_setposixacl_getntacl_smbd(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         # This invalidates the hash of the NT acl just set because there is a hook in the posix ACL set code
         smbd.set_simple_acl(self.tempf, 0o640)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
@@ -139,7 +141,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
         BA_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS)
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;BA)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         # This invalidates the hash of the NT acl just set because there is a hook in the posix ACL set code
         s4_passdb = passdb.PDB(self.lp.get("passdb backend"))
         (BA_gid,BA_type) = s4_passdb.sid_to_id(BA_sid)
@@ -152,14 +154,14 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_smbd_getntacl_smbd_gpo(self):
         acl = "O:DAG:DUD:P(A;OICI;0x001f01ff;;;DA)(A;OICI;0x001f01ff;;;EA)(A;OICIIO;0x001f01ff;;;CO)(A;OICI;0x001f01ff;;;DA)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)(A;OICI;0x001200a9;;;ED)S:AI(OU;CIIDSA;WP;f30e3bbe-9ff0-11d1-b603-0000f80367c1;bf967aa5-0de6-11d0-a285-00aa003049e2;WD)(OU;CIIDSA;WP;f30e3bbf-9ff0-11d1-b603-0000f80367c1;bf967aa5-0de6-11d0-a285-00aa003049e2;WD)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
-        domsid = security.dom_sid("S-1-5-21-2212615479-2695158682-2101375467")
+        domsid = security.dom_sid(DOM_SID)
         self.assertEquals(facl.as_sddl(domsid),acl)
 
     def test_setntacl_getposixacl(self):
         acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
-        setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         facl = getntacl(self.lp, self.tempf)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
-- 
2.11.0


From e5387f8d7a6b9307921ac8964138d1305b3f0ea4 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 15:25:56 +1200
Subject: [PATCH 11/20] tests/posixacl: define global ACL to make code DRY

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 462ee7ef12d..982861c824c 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -29,6 +29,7 @@ from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
 
 DOM_SID = "S-1-5-21-2212615479-2695158682-2101375467"
+ACL = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
 
 
 class PosixAclMappingTests(TestCaseInTempDir):
@@ -45,18 +46,18 @@ class PosixAclMappingTests(TestCaseInTempDir):
         return aclstr
 
     def test_setntacl(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
 
     def test_setntacl_smbd_getntacl(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
         facl = getntacl(self.lp, self.tempf, direct_db_access=True)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_setposixacl_getntacl(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
 
         # This will invalidate the ACL, as we have a hook!
@@ -70,7 +71,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
             pass
 
     def test_setntacl_invalidate_getntacl(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
@@ -84,7 +85,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(acl, facl.as_sddl(anysid))
 
     def test_setntacl_invalidate_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
@@ -98,7 +99,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(acl, facl.as_sddl(anysid))
 
     def test_setntacl_smbd_invalidate_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x001200a9;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
         os.chmod(self.tempf, 0o750)
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
@@ -114,21 +115,21 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(simple_acl_from_posix, facl.as_sddl(anysid))
 
     def test_setntacl_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_setposixacl_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         # This invalidates the hash of the NT acl just set because there is a hook in the posix ACL set code
@@ -138,7 +139,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(simple_acl_from_posix, facl.as_sddl(anysid))
 
     def test_setntacl_smbd_setposixacl_group_getntacl_smbd(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         BA_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS)
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;BA)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
@@ -160,7 +161,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(facl.as_sddl(domsid),acl)
 
     def test_setntacl_getposixacl(self):
-        acl = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
+        acl = ACL
         setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
         facl = getntacl(self.lp, self.tempf)
         anysid = security.dom_sid(security.SID_NT_SELF)
-- 
2.11.0


From a7eefc43666b59d74734b6f376c6637e663ac69e Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 15:28:16 +1200
Subject: [PATCH 12/20] tests/posixacl: remove unused imports

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 982861c824c..b01234d239d 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -19,11 +19,9 @@
 """Tests for the Samba3 NT -> posix ACL layer"""
 
 from samba.ntacls import setntacl, getntacl, checkset_backend
-from samba.dcerpc import xattr, security, smb_acl, idmap
-from samba.param import LoadParm
+from samba.dcerpc import security, smb_acl, idmap
 from samba.tests import TestCaseInTempDir
 from samba import provision
-import random
 import os
 from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
-- 
2.11.0


From fb9d94c6a3b98f831c6aab4bccafcaf782d0dadc Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 15:35:14 +1200
Subject: [PATCH 13/20] tests/posixacl: use assertRaises to simplify code

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index b01234d239d..72059bc8f84 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -62,11 +62,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
         smbd.set_simple_acl(self.tempf, 0o640)
 
         # However, this only asks the xattr
-        try:
-            facl = getntacl(self.lp, self.tempf, direct_db_access=True)
-            self.assertTrue(False)
-        except TypeError:
-            pass
+        self.assertRaises(
+            TypeError, getntacl, self.lp, self.tempf, direct_db_access=True)
 
     def test_setntacl_invalidate_getntacl(self):
         acl = ACL
@@ -184,14 +181,9 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(posix_acl.acl[3].a_perm, 6)
 
     def test_setposixacl_getntacl(self):
-        acl = ""
         smbd.set_simple_acl(self.tempf, 0o750)
-        try:
-            facl = getntacl(self.lp, self.tempf)
-            self.assertTrue(False)
-        except TypeError:
-            # We don't expect the xattr to be filled in in this case
-            pass
+        # We don't expect the xattr to be filled in in this case
+        self.assertRaises(TypeError, getntacl, self.lp, self.tempf)
 
     def test_setposixacl_getntacl_smbd(self):
         s4_passdb = passdb.PDB(self.lp.get("passdb backend"))
-- 
2.11.0


From 9ca2a6aae5729fb3decfdc9bc0058f990c128113 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Wed, 4 Jul 2018 15:50:40 +1200
Subject: [PATCH 14/20] tests/posixacl: rm duplicated test

There are 2 copy of `test_setposixacl_getposixacl`, this patch removed
the first copy, which was overwritten by the second one.

They are 99% the same except in the last line a_perm is 6 vs 7, and 7 is
the correct number.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 72059bc8f84..5c4cdad9d31 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -163,23 +163,6 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(facl.as_sddl(anysid),acl)
         posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
 
-    def test_setposixacl_getposixacl(self):
-        smbd.set_simple_acl(self.tempf, 0o640)
-        posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
-        self.assertEquals(posix_acl.count, 4, self.print_posix_acl(posix_acl))
-
-        self.assertEquals(posix_acl.acl[0].a_type, smb_acl.SMB_ACL_USER_OBJ)
-        self.assertEquals(posix_acl.acl[0].a_perm, 6)
-
-        self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_GROUP_OBJ)
-        self.assertEquals(posix_acl.acl[1].a_perm, 4)
-
-        self.assertEquals(posix_acl.acl[2].a_type, smb_acl.SMB_ACL_OTHER)
-        self.assertEquals(posix_acl.acl[2].a_perm, 0)
-
-        self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_MASK)
-        self.assertEquals(posix_acl.acl[3].a_perm, 6)
-
     def test_setposixacl_getntacl(self):
         smbd.set_simple_acl(self.tempf, 0o750)
         # We don't expect the xattr to be filled in in this case
-- 
2.11.0


From a3a74984a6a67754bb31bab8adc529e0d2139246 Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Fri, 6 Jul 2018 10:32:17 +1200
Subject: [PATCH 15/20] tests/posixacl: move setUp and tearDown to top

Make it clear to find out what we have in test.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 5c4cdad9d31..175fcb2c177 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -32,6 +32,20 @@ ACL = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695
 
 class PosixAclMappingTests(TestCaseInTempDir):
 
+    def setUp(self):
+        super(PosixAclMappingTests, self).setUp()
+        s3conf = s3param.get_context()
+        s3conf.load(self.get_loadparm().configfile)
+        s3conf.set("xattr_tdb:file", os.path.join(self.tempdir, "xattr.tdb"))
+        self.lp = s3conf
+        self.tempf = os.path.join(self.tempdir, "test")
+        open(self.tempf, 'w').write("empty")
+
+    def tearDown(self):
+        smbd.unlink(self.tempf)
+        os.unlink(os.path.join(self.tempdir, "xattr.tdb"))
+        super(PosixAclMappingTests, self).tearDown()
+
     def print_posix_acl(self, posix_acl):
         aclstr = ""
         for entry in posix_acl.acl:
@@ -774,19 +788,3 @@ class PosixAclMappingTests(TestCaseInTempDir):
 # a_perm: 7
 # uid: -1
 # gid: -1
-
-#
-
-    def setUp(self):
-        super(PosixAclMappingTests, self).setUp()
-        s3conf = s3param.get_context()
-        s3conf.load(self.get_loadparm().configfile)
-        s3conf.set("xattr_tdb:file", os.path.join(self.tempdir,"xattr.tdb"))
-        self.lp = s3conf
-        self.tempf = os.path.join(self.tempdir, "test")
-        open(self.tempf, 'w').write("empty")
-
-    def tearDown(self):
-        smbd.unlink(self.tempf)
-        os.unlink(os.path.join(self.tempdir,"xattr.tdb"))
-        super(PosixAclMappingTests, self).tearDown()
-- 
2.11.0


From e5764546c318e6713e0110aaaa8660d90bbe5f1c Mon Sep 17 00:00:00 2001
From: Joe Guo <joeg at catalyst.net.nz>
Date: Fri, 6 Jul 2018 10:36:54 +1200
Subject: [PATCH 16/20] tests/posixacl: derive a new testcase to run same tests
 with session

1. existing tests still run with session_info=None
2. new class override `get_session_info` to return a session, so same
set of tests will run again, but with session.

Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 92 +++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 20 deletions(-)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 175fcb2c177..4261ef36544 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -25,6 +25,8 @@ from samba import provision
 import os
 from samba.samba3 import smbd, passdb
 from samba.samba3 import param as s3param
+from samba import auth
+from samba.samdb import SamDB
 
 DOM_SID = "S-1-5-21-2212615479-2695158682-2101375467"
 ACL = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)"
@@ -40,12 +42,22 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.lp = s3conf
         self.tempf = os.path.join(self.tempdir, "test")
         open(self.tempf, 'w').write("empty")
+        self.samdb = SamDB(lp=self.lp, session_info=auth.system_session())
 
     def tearDown(self):
         smbd.unlink(self.tempf)
         os.unlink(os.path.join(self.tempdir, "xattr.tdb"))
         super(PosixAclMappingTests, self).tearDown()
 
+    def get_session_info(self, domsid=DOM_SID):
+        """
+        Get session_info for setntacl.
+
+        This test case always return None, to run tests without session_info
+        like before. To be overrided in derived class.
+        """
+        return None
+
     def print_posix_acl(self, posix_acl):
         aclstr = ""
         for entry in posix_acl.acl:
@@ -59,18 +71,21 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
 
     def test_setntacl_smbd_getntacl(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True,
+                 session_info=self.get_session_info())
         facl = getntacl(self.lp, self.tempf, direct_db_access=True)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_setposixacl_getntacl(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True,
+                 session_info=self.get_session_info())
 
         # This will invalidate the ACL, as we have a hook!
         smbd.set_simple_acl(self.tempf, 0o640)
@@ -81,7 +96,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_invalidate_getntacl(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True,
+                 session_info=self.get_session_info())
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -95,7 +111,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_invalidate_getntacl_smbd(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -111,7 +128,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
         acl = ACL
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x001200a9;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
         os.chmod(self.tempf, 0o750)
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
 
         # This should invalidate the ACL, as we include the posix ACL in the hash
         (backend_obj, dbname) = checkset_backend(self.lp, None, None)
@@ -125,14 +143,16 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_getntacl_smbd(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=True,
+                 session_info=self.get_session_info())
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
 
     def test_setntacl_smbd_getntacl_smbd(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
@@ -140,7 +160,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
     def test_setntacl_smbd_setposixacl_getntacl_smbd(self):
         acl = ACL
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
         # This invalidates the hash of the NT acl just set because there is a hook in the posix ACL set code
         smbd.set_simple_acl(self.tempf, 0o640)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
@@ -151,7 +172,8 @@ class PosixAclMappingTests(TestCaseInTempDir):
         acl = ACL
         BA_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS)
         simple_acl_from_posix = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;;0x001f019f;;;S-1-5-21-2212615479-2695158682-2101375467-512)(A;;0x00120089;;;BA)(A;;0x00120089;;;S-1-5-21-2212615479-2695158682-2101375467-513)(A;;;;;WD)"
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
         # This invalidates the hash of the NT acl just set because there is a hook in the posix ACL set code
         s4_passdb = passdb.PDB(self.lp.get("passdb backend"))
         (BA_gid,BA_type) = s4_passdb.sid_to_id(BA_sid)
@@ -164,14 +186,16 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     def test_setntacl_smbd_getntacl_smbd_gpo(self):
         acl = "O:DAG:DUD:P(A;OICI;0x001f01ff;;;DA)(A;OICI;0x001f01ff;;;EA)(A;OICIIO;0x001f01ff;;;CO)(A;OICI;0x001f01ff;;;DA)(A;OICI;0x001f01ff;;;SY)(A;OICI;0x001200a9;;;AU)(A;OICI;0x001200a9;;;ED)S:AI(OU;CIIDSA;WP;f30e3bbe-9ff0-11d1-b603-0000f80367c1;bf967aa5-0de6-11d0-a285-00aa003049e2;WD)(OU;CIIDSA;WP;f30e3bbf-9ff0-11d1-b603-0000f80367c1;bf967aa5-0de6-11d0-a285-00aa003049e2;WD)"
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         domsid = security.dom_sid(DOM_SID)
         self.assertEquals(facl.as_sddl(domsid),acl)
 
     def test_setntacl_getposixacl(self):
         acl = ACL
-        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False)
+        setntacl(self.lp, self.tempf, acl, DOM_SID, use_ntvfs=False,
+                 session_info=self.get_session_info())
         facl = getntacl(self.lp, self.tempf)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(facl.as_sddl(anysid),acl)
@@ -287,7 +311,9 @@ class PosixAclMappingTests(TestCaseInTempDir):
     def test_setntacl_sysvol_check_getposixacl(self):
         acl = provision.SYSVOL_ACL
         domsid = passdb.get_global_sam_sid()
-        setntacl(self.lp, self.tempf,acl,str(domsid), use_ntvfs=False)
+        session_info = self.get_session_info(domsid)
+        setntacl(self.lp, self.tempf, acl, str(domsid), use_ntvfs=False,
+                 session_info=session_info)
         facl = getntacl(self.lp, self.tempf)
         self.assertEquals(facl.as_sddl(domsid),acl)
         posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
@@ -327,7 +353,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(posix_acl.acl[0].info.gid, BA_gid)
 
         self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_USER)
-        if nwrap_winbind_active:
+        if nwrap_winbind_active or session_info:
             self.assertEquals(posix_acl.acl[1].a_perm, 7)
         else:
             self.assertEquals(posix_acl.acl[1].a_perm, 6)
@@ -337,7 +363,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(posix_acl.acl[2].a_perm, 0)
 
         self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ)
-        if nwrap_winbind_active:
+        if nwrap_winbind_active or session_info:
             self.assertEquals(posix_acl.acl[3].a_perm, 7)
         else:
             self.assertEquals(posix_acl.acl[3].a_perm, 6)
@@ -433,7 +459,9 @@ class PosixAclMappingTests(TestCaseInTempDir):
     def test_setntacl_sysvol_dir_check_getposixacl(self):
         acl = provision.SYSVOL_ACL
         domsid = passdb.get_global_sam_sid()
-        setntacl(self.lp, self.tempdir,acl,str(domsid), use_ntvfs=False)
+        session_info = self.get_session_info(domsid)
+        setntacl(self.lp, self.tempdir, acl, str(domsid), use_ntvfs=False,
+                 session_info=session_info)
         facl = getntacl(self.lp, self.tempdir)
         self.assertEquals(facl.as_sddl(domsid),acl)
         posix_acl = smbd.get_sys_acl(self.tempdir, smb_acl.SMB_ACL_TYPE_ACCESS)
@@ -526,7 +554,9 @@ class PosixAclMappingTests(TestCaseInTempDir):
     def test_setntacl_policies_dir_check_getposixacl(self):
         acl = provision.POLICIES_ACL
         domsid = passdb.get_global_sam_sid()
-        setntacl(self.lp, self.tempdir,acl,str(domsid), use_ntvfs=False)
+        session_info = self.get_session_info(domsid)
+        setntacl(self.lp, self.tempdir, acl, str(domsid), use_ntvfs=False,
+                 session_info=session_info)
         facl = getntacl(self.lp, self.tempdir)
         self.assertEquals(facl.as_sddl(domsid),acl)
         posix_acl = smbd.get_sys_acl(self.tempdir, smb_acl.SMB_ACL_TYPE_ACCESS)
@@ -633,7 +663,9 @@ class PosixAclMappingTests(TestCaseInTempDir):
         acl = provision.POLICIES_ACL
 
         domsid = passdb.get_global_sam_sid()
-        setntacl(self.lp, self.tempf, acl, str(domsid), use_ntvfs=False)
+        session_info = self.get_session_info(domsid)
+        setntacl(self.lp, self.tempf, acl, str(domsid), use_ntvfs=False,
+            session_info=session_info)
         facl = getntacl(self.lp, self.tempf)
         self.assertEquals(facl.as_sddl(domsid),acl)
         posix_acl = smbd.get_sys_acl(self.tempf, smb_acl.SMB_ACL_TYPE_ACCESS)
@@ -676,7 +708,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(posix_acl.acl[0].info.gid, BA_gid)
 
         self.assertEquals(posix_acl.acl[1].a_type, smb_acl.SMB_ACL_USER)
-        if nwrap_winbind_active:
+        if nwrap_winbind_active or session_info:
             self.assertEquals(posix_acl.acl[1].a_perm, 7)
         else:
             self.assertEquals(posix_acl.acl[1].a_perm, 6)
@@ -686,7 +718,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         self.assertEquals(posix_acl.acl[2].a_perm, 0)
 
         self.assertEquals(posix_acl.acl[3].a_type, smb_acl.SMB_ACL_USER_OBJ)
-        if nwrap_winbind_active:
+        if nwrap_winbind_active or session_info:
             self.assertEquals(posix_acl.acl[3].a_perm, 7)
         else:
             self.assertEquals(posix_acl.acl[3].a_perm, 6)
@@ -788,3 +820,23 @@ class PosixAclMappingTests(TestCaseInTempDir):
 # a_perm: 7
 # uid: -1
 # gid: -1
+
+class SessionedPosixAclMappingTests(PosixAclMappingTests):
+    """
+    Run same test suite with session enabled.
+    """
+
+    def get_session_info(self, domsid=DOM_SID):
+        """
+        Get session_info for setntacl.
+        """
+        if str(domsid) != str(self.samdb.get_domain_sid()):
+            # fake it with admin session as domsid is not in local db
+            return auth.admin_session(self.lp, str(domsid))
+
+        dn = '<SID={}-{}>'.format(domsid, security.DOMAIN_RID_ADMINISTRATOR)
+        flags = (auth.AUTH_SESSION_INFO_DEFAULT_GROUPS |
+                 auth.AUTH_SESSION_INFO_AUTHENTICATED |
+                 auth.AUTH_SESSION_INFO_SIMPLE_PRIVILEGES)
+        return auth.user_session(self.samdb, lp_ctx=self.lp, dn=dn,
+                                 session_info_flags=flags)
-- 
2.11.0


From 3d08bfafbe4915507b20571ca1ddb123e48dea16 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Jul 2018 16:48:07 +1200
Subject: [PATCH 17/20] selftest: Add tests for samba.auth.admin_session()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
Pair-programmed-with: Joe Guo <joeg at catalyst.net.nz>
---
 python/samba/tests/auth.py | 41 +++++++++++++++++++++++++++++++++++++----
 selftest/tests.py          |  2 +-
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/python/samba/tests/auth.py b/python/samba/tests/auth.py
index a1b8115dba1..6318bec40a0 100644
--- a/python/samba/tests/auth.py
+++ b/python/samba/tests/auth.py
@@ -24,11 +24,12 @@ the functionality, that's already done in other tests.
 from samba import auth
 import samba.tests
 
-class AuthTests(samba.tests.TestCase):
+class AuthSystemSessionTests(samba.tests.TestCase):
 
     def setUp(self):
-        super(AuthTests, self).setUp()
+        super(AuthSystemSessionTests, self).setUp()
         self.system_session = auth.system_session()
+        self.lp = samba.tests.env_loadparm()
 
     def test_system_session_attrs(self):
         self.assertTrue(hasattr(self.system_session, 'credentials'))
@@ -39,8 +40,9 @@ class AuthTests(samba.tests.TestCase):
 
     def test_system_session_credentials(self):
         self.assertIsNone(self.system_session.credentials.get_bind_dn())
-        self.assertIsNone(self.system_session.credentials.get_password())
-        self.assertEqual(self.system_session.credentials.get_username(), '')
+        self.assertIsNotNone(self.system_session.credentials.get_password())
+        self.assertEqual(self.system_session.credentials.get_username(),
+                         self.lp.get('netbios name').upper() + "$")
 
     def test_system_session_info(self):
         self.assertEqual(self.system_session.info.full_name, 'System')
@@ -54,3 +56,34 @@ class AuthTests(samba.tests.TestCase):
     def test_system_session_security_token(self):
         self.assertTrue(self.system_session.security_token.is_system())
         self.assertFalse(self.system_session.security_token.is_anonymous())
+
+class AuthAdminSessionTests(samba.tests.TestCase):
+
+    def setUp(self):
+        super(AuthAdminSessionTests, self).setUp()
+        self.lp = samba.tests.env_loadparm()
+        self.admin_session = auth.admin_session(self.lp,
+                                                "S-1-5-21-2212615479-2695158682-2101375467")
+
+    def test_admin_session_attrs(self):
+        self.assertTrue(hasattr(self.admin_session, 'credentials'))
+        self.assertTrue(hasattr(self.admin_session, 'info'))
+        self.assertTrue(hasattr(self.admin_session, 'security_token'))
+        self.assertTrue(hasattr(self.admin_session, 'session_key'))
+        self.assertTrue(hasattr(self.admin_session, 'torture'))
+
+    def test_admin_session_credentials(self):
+        self.assertIsNone(self.admin_session.credentials)
+
+    def test_session_info_details(self):
+        self.assertEqual(self.admin_session.info.full_name,
+                         'Administrator')
+        self.assertEqual(self.admin_session.info.domain_name,
+                         self.lp.get('workgroup'))
+        self.assertEqual(self.admin_session.info.account_name,
+                         'Administrator')
+
+    def test_security_token(self):
+        self.assertFalse(self.admin_session.security_token.is_system())
+        self.assertFalse(self.admin_session.security_token.is_anonymous())
+        self.assertTrue(self.admin_session.security_token.has_builtin_administrators())
diff --git a/selftest/tests.py b/selftest/tests.py
index f354bb57ef5..6808925ffde 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -56,7 +56,7 @@ planpythontestsuite("none", "samba.tests.blackbox.check_output", py3_compatible=
 planpythontestsuite("none", "api", name="ldb.python", extra_path=['lib/ldb/tests/python'])
 planpythontestsuite("none", "samba.tests.credentials", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.registry", py3_compatible=True)
-planpythontestsuite("none", "samba.tests.auth", py3_compatible=True)
+planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.get_opt", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.security", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.dcerpc.misc", py3_compatible=True)
-- 
2.11.0


From 79b17f2af07dea274c7f3cb6b945680536fe13d8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Jul 2018 16:48:40 +1200
Subject: [PATCH 18/20] python: Add samba.auth.session_info_fill_unix()

This fills in the unix portions of the token needed by smbd and the pysmbd bindings

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Pair-programmed-with: Joe Guo <joeg at catalyst.net.nz>
Signed-off-by: Joe Guo <joeg at catalyst.net.nz>
---
 python/samba/tests/auth.py | 11 +++++++++
 source4/auth/pyauth.c      | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/python/samba/tests/auth.py b/python/samba/tests/auth.py
index 6318bec40a0..27284721d3e 100644
--- a/python/samba/tests/auth.py
+++ b/python/samba/tests/auth.py
@@ -87,3 +87,14 @@ class AuthAdminSessionTests(samba.tests.TestCase):
         self.assertFalse(self.admin_session.security_token.is_system())
         self.assertFalse(self.admin_session.security_token.is_anonymous())
         self.assertTrue(self.admin_session.security_token.has_builtin_administrators())
+
+    def test_session_info_unix_details(self):
+        samba.auth.session_info_fill_unix(session_info = self.admin_session,
+                                          lp_ctx=self.lp,
+                                          user_name="Administrator")
+        self.assertEqual(self.admin_session.unix_info.sanitized_username,
+                         'Administrator')
+        self.assertEqual(self.admin_session.unix_info.unix_name,
+                         self.lp.get('workgroup').upper() +
+                         self.lp.get('winbind separator') + 'Administrator')
+        self.assertIsNotNone(self.admin_session.unix_token)
diff --git a/source4/auth/pyauth.c b/source4/auth/pyauth.c
index fc22637564d..df13d9a2708 100644
--- a/source4/auth/pyauth.c
+++ b/source4/auth/pyauth.c
@@ -164,6 +164,63 @@ static PyObject *py_user_session(PyObject *module, PyObject *args, PyObject *kwa
 	return PyAuthSession_FromSession(session);
 }
 
+static PyObject *py_session_info_fill_unix(PyObject *module,
+					   PyObject *args,
+					   PyObject *kwargs)
+{
+	NTSTATUS nt_status;
+	char *user_name = NULL;
+	struct loadparm_context *lp_ctx = NULL;
+	struct auth_session_info *session_info;
+	PyObject *py_lp_ctx = Py_None;
+	PyObject *py_session = Py_None;
+	TALLOC_CTX *frame;
+	
+	const char * const kwnames[] = { "session_info",
+					 "user_name",
+					 "lp_ctx",
+					 NULL };
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "Oz|O",
+					 discard_const_p(char *, kwnames),
+					 &py_session,
+					 &user_name,
+					 &py_lp_ctx)) {
+		return NULL;
+	}
+
+	if (!py_check_dcerpc_type(py_session,
+				  "samba.dcerpc.auth",
+				  "session_info")) {
+		return NULL;
+	}
+	session_info = pytalloc_get_type(py_session,
+					 struct auth_session_info);
+	if (!session_info) {
+		PyErr_Format(PyExc_TypeError,
+			     "Expected auth_session_info for session_info argument got %s",
+			     talloc_get_name(pytalloc_get_ptr(py_session)));
+		return NULL;
+	}
+
+	frame = talloc_stackframe();
+	
+	lp_ctx = lpcfg_from_py_object(frame, py_lp_ctx);
+	if (lp_ctx == NULL) {
+		TALLOC_FREE(frame);
+		return NULL;
+	}
+
+	nt_status = auth_session_info_fill_unix(lp_ctx,
+					       user_name,
+					       session_info);
+	TALLOC_FREE(frame);
+	if (!NT_STATUS_IS_OK(nt_status)) {
+		PyErr_NTSTATUS_IS_ERR_RAISE(nt_status);
+	}
+
+	Py_RETURN_NONE;
+}
+
 
 static const char **PyList_AsStringList(TALLOC_CTX *mem_ctx, PyObject *list, 
 					const char *paramname)
@@ -304,6 +361,10 @@ static PyMethodDef py_auth_methods[] = {
 	{ "system_session", (PyCFunction)py_system_session, METH_VARARGS, NULL },
 	{ "admin_session", (PyCFunction)py_admin_session, METH_VARARGS, NULL },
 	{ "user_session", (PyCFunction)py_user_session, METH_VARARGS|METH_KEYWORDS, NULL },
+	{ "session_info_fill_unix",
+	  (PyCFunction)py_session_info_fill_unix,
+	  METH_VARARGS|METH_KEYWORDS,
+	  NULL },
 	{ NULL },
 };
 
-- 
2.11.0


From 53769df7e3ca044e567bbe89e1e59e11018887b7 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Jul 2018 17:08:34 +1200
Subject: [PATCH 19/20] tests/posixacl: Test with and without filling in the
 unix_token

Sadly the unix token cannot be created without a running winbindd,
which is not available during provision and a domain restore.

(Internally in smbd a backup API via passdb is used, but this
is not connected to this function at this time)

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

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/posixacl.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py
index 4261ef36544..1090e239328 100644
--- a/python/samba/tests/posixacl.py
+++ b/python/samba/tests/posixacl.py
@@ -840,3 +840,28 @@ class SessionedPosixAclMappingTests(PosixAclMappingTests):
                  auth.AUTH_SESSION_INFO_SIMPLE_PRIVILEGES)
         return auth.user_session(self.samdb, lp_ctx=self.lp, dn=dn,
                                  session_info_flags=flags)
+
+class UnixSessionedPosixAclMappingTests(PosixAclMappingTests):
+    """
+    Run same test suite with session enabled.
+    """
+
+    def get_session_info(self, domsid=DOM_SID):
+        """
+        Get session_info for setntacl.
+        """
+        if str(domsid) != str(self.samdb.get_domain_sid()):
+            # fake it with admin session as domsid is not in local db
+            return auth.admin_session(self.lp, str(domsid))
+
+        dn = '<SID={}-{}>'.format(domsid, security.DOMAIN_RID_ADMINISTRATOR)
+        flags = (auth.AUTH_SESSION_INFO_DEFAULT_GROUPS |
+                 auth.AUTH_SESSION_INFO_AUTHENTICATED |
+                 auth.AUTH_SESSION_INFO_SIMPLE_PRIVILEGES)
+        
+        session = auth.user_session(self.samdb, lp_ctx=self.lp, dn=dn,
+                                    session_info_flags=flags)
+        auth.session_info_fill_unix(session,
+                                    lp_ctx=self.lp,
+                                    user_name="Administrator")
+        return session
-- 
2.11.0


From e48cc55ce544b05380d83d630ee15cf17923856b Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 11 Jul 2018 16:12:53 +1200
Subject: [PATCH 20/20] WHATSNEW: document sysvolreset improvement

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13521
---
 WHATSNEW.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 7823612ee81..4ff6c80e388 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -163,6 +163,13 @@ rename' command will clone the domain DB, renaming it in the process, and
 producing a backup-file. Then, the 'samba-tool domain backup restore' command
 takes the backup-file and restores the renamed DB to disk on a fresh DC.
 
+samba-tool ntacl sysvolreset is now much faster
+-----------------------------------------------
+
+The 'samba-tool ntacl sysvolreset' command, used on the Samba AD DC,
+is now much faster than in previous versions, after an internal
+rework.
+
 REMOVED FEATURES
 ================
 
-- 
2.11.0



More information about the samba-technical mailing list