[PATCH] SYSVOL ACL fixes Re: [PATCH] Fix 'samba-tool ntacl sysvolcheck' failures and remove NT4 compat

Jeremy Allison jra at samba.org
Tue Nov 13 14:45:08 MST 2012


On Wed, Nov 14, 2012 at 08:32:44AM +1100, Andrew Bartlett wrote:
> 
> It's a fair cop, and I agree.

:-).

> I'm not as convinced that the other patches break up so well, but I
> guess you could remove the smb.conf parameter, and then the manpages for
> "acl compatability" in distinct patches if that was your preference.

No, the others are more logically grouped, so I didn't split them. I
thought the "acl compatability" one could have been split up the way
you describe, but it's not such a big deal when it's not code changes,
but only man page changes.

> I'm certainly not wanting to be a pain here, so please let me know what
> would help you best here.  Are you still splitting them up, or would you
> prefer me to just re-submit?

Already did it :-).

FYI. I'm missing the last patch in your set (the samba-tool
one) as that one didn't apply to master but gave conflicts.

Can you re-evaluate this set and if you're happy then push to
autobuild.

Cheers,

Jeremy.
-------------- next part --------------
>From 7bb0cd31bd95cc235bcb75b615811d1eb7d0cf8f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Nov 2012 12:21:45 -0800
Subject: [PATCH 1/7] Ensure we Correctly set fsp->is_directory before dealing
 with ACLs.

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source3/torture/cmd_vfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c
index a37c9fc..64b1b50 100644
--- a/source3/torture/cmd_vfs.c
+++ b/source3/torture/cmd_vfs.c
@@ -1524,7 +1524,7 @@ static NTSTATUS cmd_set_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int a
 	fsp->print_file = NULL;
 	fsp->modified = False;
 	fsp->sent_oplock_break = NO_BREAK_SENT;
-	fsp->is_directory = False;
+	fsp->is_directory = S_ISDIR(smb_fname->st.st_ex_mode);
 
 
 	sd = sddl_decode(talloc_tos(), argv[2], get_global_sam_sid());
-- 
1.7.7.3


>From 33d1aaa70db66c5283074559ccf8f3440915d918 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Nov 2012 12:34:35 -0800
Subject: [PATCH 2/7] smbd: Correctly set fsp->is_directory before dealing
 with ACLs

Change set_nt_acl_no_snum() to correctly set up the fsp.
This does a stat on a real fsp in set_nt_acl_no_snum.

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/pysmbd.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 6a6a812..436e881 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -89,7 +89,7 @@ static NTSTATUS set_nt_acl_no_snum(const char *fname,
 	NTSTATUS status = NT_STATUS_OK;
 	files_struct *fsp;
 	struct smb_filename *smb_fname = NULL;
-	int flags;
+	int flags, ret;
 	mode_t saved_umask;
 
 	if (!posix_locking_init(false)) {
@@ -160,6 +160,29 @@ static NTSTATUS set_nt_acl_no_snum(const char *fname,
 		return NT_STATUS_UNSUCCESSFUL;
 	}
 
+	ret = SMB_VFS_FSTAT(fsp, &smb_fname->st);
+	if (ret == -1) {
+		/* If we have an fd, this stat should succeed. */
+		DEBUG(0,("Error doing fstat on open file %s "
+			"(%s)\n",
+			smb_fname_str_dbg(smb_fname),
+			strerror(errno) ));
+		TALLOC_FREE(frame);
+		umask(saved_umask);
+		return map_nt_error_from_unix(errno);
+	}
+
+	fsp->file_id = vfs_file_id_from_sbuf(conn, &smb_fname->st);
+	fsp->vuid = UID_FIELD_INVALID;
+	fsp->file_pid = 0;
+	fsp->can_lock = True;
+	fsp->can_read = True;
+	fsp->can_write = True;
+	fsp->print_file = NULL;
+	fsp->modified = False;
+	fsp->sent_oplock_break = NO_BREAK_SENT;
+	fsp->is_directory = S_ISDIR(smb_fname->st.st_ex_mode);
+
 	status = SMB_VFS_FSET_NT_ACL( fsp, security_info_sent, sd);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0,("set_nt_acl_no_snum: fset_nt_acl returned %s.\n", nt_errstr(status)));
-- 
1.7.7.3


>From 24b4c882d1c226215d1705f38b934ea6d0d51b7e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Nov 2012 12:48:53 -0800
Subject: [PATCH 3/7] Change get_nt_acl_no_snum() to return an NTSTATUS, not a
 struct security_descriptor *.

Internally change the implementation to use SMB_VFS_GET_NT_ACL()
instead of SMB_VFS_FGET_NT_ACL() with a faked-up file struct.

Andrew Bartlett

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source3/rpc_server/eventlog/srv_eventlog_nt.c |   11 +++--
 source3/smbd/posix_acls.c                     |   62 +++++++++----------------
 source3/smbd/proto.h                          |    3 +-
 source3/smbd/pysmbd.c                         |    4 +-
 4 files changed, 34 insertions(+), 46 deletions(-)

diff --git a/source3/rpc_server/eventlog/srv_eventlog_nt.c b/source3/rpc_server/eventlog/srv_eventlog_nt.c
index a05ea3f..a3e719a 100644
--- a/source3/rpc_server/eventlog/srv_eventlog_nt.c
+++ b/source3/rpc_server/eventlog/srv_eventlog_nt.c
@@ -91,12 +91,15 @@ static bool elog_check_access( EVENTLOG_INFO *info, const struct security_token
 
 	/* get the security descriptor for the file */
 
-	sec_desc = get_nt_acl_no_snum( info, tdbname, SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL);
+	status = get_nt_acl_no_snum( info,
+			tdbname,
+			SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL,
+			&sec_desc);
 	TALLOC_FREE( tdbname );
 
-	if ( !sec_desc ) {
-		DEBUG(5,("elog_check_access: Unable to get NT ACL for %s\n",
-			tdbname));
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(5,("elog_check_access: Unable to get NT ACL for %s: %s\n",
+			tdbname, nt_errstr(status)));
 		return False;
 	}
 
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index d46a16d..74ea257 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -4963,30 +4963,34 @@ bool set_unix_posix_acl(connection_struct *conn, files_struct *fsp, const char *
  check.  Caller is responsible for freeing the returned security
  descriptor via TALLOC_FREE().  This is designed for dealing with 
  user space access checks in smbd outside of the VFS.  For example,
- checking access rights in OpenEventlog().
+ checking access rights in OpenEventlog() or from python.
 
- Assume we are dealing with files (for now)
 ********************************************************************/
 
-struct security_descriptor *get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname, uint32 security_info_wanted)
+NTSTATUS get_nt_acl_no_snum(TALLOC_CTX *ctx, const char *fname,
+				uint32 security_info_wanted,
+				struct security_descriptor **sd)
 {
-	struct security_descriptor *ret_sd;
-	connection_struct *conn;
-	files_struct finfo;
-	struct fd_handle fh;
-	NTSTATUS status;
 	TALLOC_CTX *frame = talloc_stackframe();
+	connection_struct *conn;
+	NTSTATUS status = NT_STATUS_OK;
+
+	if (!posix_locking_init(false)) {
+		TALLOC_FREE(frame);
+		return NT_STATUS_NO_MEMORY;
+	}
 
 	conn = talloc_zero(frame, connection_struct);
 	if (conn == NULL) {
+		TALLOC_FREE(frame);
 		DEBUG(0, ("talloc failed\n"));
-		return NULL;
+		return NT_STATUS_NO_MEMORY;
 	}
 
 	if (!(conn->params = talloc(conn, struct share_params))) {
-		DEBUG(0,("get_nt_acl_no_snum: talloc() failed!\n"));
+		DEBUG(0, ("talloc failed\n"));
 		TALLOC_FREE(frame);
-		return NULL;
+		return NT_STATUS_NO_MEMORY;
 	}
 
 	conn->params->service = -1;
@@ -4994,43 +4998,21 @@ struct security_descriptor *get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fna
 	set_conn_connectpath(conn, "/");
 
 	if (!smbd_vfs_init(conn)) {
-		DEBUG(0,("get_nt_acl_no_snum: Unable to create a fake connection struct!\n"));
-		conn_free(conn);
-		TALLOC_FREE(frame);
-		return NULL;
-        }
-
-	ZERO_STRUCT( finfo );
-	ZERO_STRUCT( fh );
-
-	finfo.fnum = FNUM_FIELD_INVALID;
-	finfo.conn = conn;
-	finfo.fh = &fh;
-	finfo.fh->fd = -1;
-
-	status = create_synthetic_smb_fname(frame, fname, NULL, NULL,
-					    &finfo.fsp_name);
-	if (!NT_STATUS_IS_OK(status)) {
-		conn_free(conn);
+		DEBUG(0,("smbd_vfs_init() failed!\n"));
 		TALLOC_FREE(frame);
-		return NULL;
+		return NT_STATUS_INTERNAL_ERROR;
 	}
 
-	if (!NT_STATUS_IS_OK(SMB_VFS_FGET_NT_ACL( &finfo,
-						  security_info_wanted,
-						  ctx, &ret_sd))) {
-		DEBUG(0,("get_nt_acl_no_snum: get_nt_acl returned zero.\n"));
-		TALLOC_FREE(finfo.fsp_name);
-		conn_free(conn);
-		TALLOC_FREE(frame);
-		return NULL;
+	status = SMB_VFS_GET_NT_ACL(conn, fname, security_info_wanted, ctx, sd);
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0,("set_nt_acl_no_snum: fset_nt_acl returned %s.\n",
+			nt_errstr(status)));
 	}
 
-	TALLOC_FREE(finfo.fsp_name);
 	conn_free(conn);
 	TALLOC_FREE(frame);
 
-	return ret_sd;
+	return status;
 }
 
 /* Stolen shamelessly from pvfs_default_acl() in source4 :-). */
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 221499c..aae4bd0 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -732,7 +732,8 @@ bool set_unix_posix_default_acl(connection_struct *conn, const char *fname,
 				const SMB_STRUCT_STAT *psbuf,
 				uint16 num_def_acls, const char *pdata);
 bool set_unix_posix_acl(connection_struct *conn, files_struct *fsp, const char *fname, uint16 num_acls, const char *pdata);
-struct security_descriptor *get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname, uint32 security_info_wanted);
+NTSTATUS get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname, uint32 security_info_wanted,
+				struct security_descriptor **sd);
 NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 					const char *name,
 					SMB_STRUCT_STAT *psbuf,
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 436e881..42694cb 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -495,11 +495,13 @@ static PyObject *py_smbd_get_nt_acl(PyObject *self, PyObject *args)
 	PyObject *py_sd;
 	struct security_descriptor *sd;
 	TALLOC_CTX *tmp_ctx = talloc_new(NULL);
+	NTSTATUS status;
 
 	if (!PyArg_ParseTuple(args, "si", &fname, &security_info_wanted))
 		return NULL;
 
-	sd = get_nt_acl_no_snum(tmp_ctx, fname, security_info_wanted);
+	status = get_nt_acl_no_snum(tmp_ctx, fname, security_info_wanted, &sd);
+	PyErr_NTSTATUS_IS_ERR_RAISE(status);
 
 	py_sd = py_return_ndr_struct("samba.dcerpc.security", "descriptor", sd, sd);
 
-- 
1.7.7.3


>From 2bbe01942c1db12fef335fa8c4d43d0140b006f0 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 12 Nov 2012 17:11:34 +1100
Subject: [PATCH 4/7] smbd: Remove NT4 compatability handling in posix -> NT
 ACL conversion

NT4 is long dead, and we should not change which ACL we return based
on what we think the client is.  The reason we should not do this, is
that if we are using vfs_acl_xattr then the hash will break if we do.
Additionally, it would require that the python VFS interface set the
global remote_arch to fake up being a modern client.

This instead seems cleaner and removes untested code (the tests are
updated to then handle the results of the modern codepath).

The supporting 'acl compatability' parameter is also removed.

Andrew Bartlett

Reviewed by: Jeremy Allison <jra at samba.org>
---
 docs-xml/smbdotconf/vfs/aclcompatibility.xml     |   17 ----
 lib/param/param_functions.c                      |    1 -
 lib/param/param_table.c                          |   19 ----
 source3/include/proto.h                          |    1 -
 source3/smbd/posix_acls.c                        |  108 +---------------------
 source3/smbd/proto.h                             |    1 -
 source4/scripting/python/samba/tests/posixacl.py |   12 +-
 7 files changed, 7 insertions(+), 152 deletions(-)
 delete mode 100644 docs-xml/smbdotconf/vfs/aclcompatibility.xml

diff --git a/docs-xml/smbdotconf/vfs/aclcompatibility.xml b/docs-xml/smbdotconf/vfs/aclcompatibility.xml
deleted file mode 100644
index 95f42cf..0000000
--- a/docs-xml/smbdotconf/vfs/aclcompatibility.xml
+++ /dev/null
@@ -1,17 +0,0 @@
-<samba:parameter name="acl compatibility"
-                 context="G"
-				 type="enum"
-                 advanced="1" developer="1"
-                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
-<description>
-	<para>This parameter specifies what OS ACL semantics should 
-	be compatible with. Possible values are <emphasis>winnt</emphasis> for Windows NT 4, 
-	<emphasis>win2k</emphasis> for Windows 2000 and above and <emphasis>auto</emphasis>.
-	If you specify <emphasis>auto</emphasis>, the value for this parameter 
-	will be based upon the version of the client. There should 
-	be no reason to change this parameter from the default.</para>
-</description>
-
-<value type="default">Auto</value>
-<value type="example">win2k</value>
-</samba:parameter>
diff --git a/lib/param/param_functions.c b/lib/param/param_functions.c
index d5cd018..94652fa 100644
--- a/lib/param/param_functions.c
+++ b/lib/param/param_functions.c
@@ -266,7 +266,6 @@ FN_GLOBAL_CONST_STRING(winbindd_socket_directory, szWinbinddSocketDirectory)
 FN_GLOBAL_CONST_STRING(winbind_separator, szWinbindSeparator)
 FN_GLOBAL_CONST_STRING(workgroup, szWorkgroup)
 FN_GLOBAL_CONST_STRING(wtmpdir, szWtmpDir)
-FN_GLOBAL_INTEGER(acl_compatibility, iAclCompat)
 FN_GLOBAL_INTEGER(afs_token_lifetime, iAfsTokenLifetime)
 FN_GLOBAL_INTEGER(algorithmic_rid_base, AlgorithmicRidBase)
 FN_GLOBAL_INTEGER(allow_dns_updates, allow_dns_updates)
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index 01f65fe..a73cd96 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -180,16 +180,6 @@ static const struct enum_list enum_kerberos_method[] = {
 	{-1, NULL}
 };
 
-
-/* ACL compatibility options. */
-static const struct enum_list enum_acl_compat_vals[] = {
-    { ACL_COMPAT_AUTO, "auto" },
-    { ACL_COMPAT_WINNT, "winnt" },
-    { ACL_COMPAT_WIN2K, "win2k" },
-    { -1, NULL}
-};
-
-
 static const struct enum_list enum_printing[] = {
 	{PRINT_SYSV, "sysv"},
 	{PRINT_AIX, "aix"},
@@ -1459,15 +1449,6 @@ static struct parm_struct parm_table[] = {
 		.flags		= FLAG_ADVANCED,
 	},
 	{
-		.label		= "acl compatibility",
-		.type		= P_ENUM,
-		.p_class	= P_GLOBAL,
-		.offset		= GLOBAL_VAR(iAclCompat),
-		.special	= NULL,
-		.enum_list	= enum_acl_compat_vals,
-		.flags		= FLAG_ADVANCED | FLAG_SHARE | FLAG_GLOBAL,
-	},
-	{
 		.label		= "defer sharing violations",
 		.type		= P_BOOL,
 		.p_class	= P_GLOBAL,
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 7c5a5a7..5f3d937 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -1068,7 +1068,6 @@ char *lp_wins_hook(TALLOC_CTX *ctx);
 const char *lp_template_homedir(void);
 const char *lp_template_shell(void);
 const char *lp_winbind_separator(void);
-int lp_acl_compatibility(void);
 bool lp_winbind_enum_users(void);
 bool lp_winbind_enum_groups(void);
 bool lp_winbind_use_default_domain(void);
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 74ea257..b8e0d4a 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1059,24 +1059,6 @@ static void merge_aces( canon_ace **pp_list_head, bool dir_acl)
 }
 
 /****************************************************************************
- Check if we need to return NT4.x compatible ACL entries.
-****************************************************************************/
-
-bool nt4_compatible_acls(void)
-{
-	int compat = lp_acl_compatibility();
-
-	if (compat == ACL_COMPAT_AUTO) {
-		enum remote_arch_types ra_type = get_remote_arch();
-
-		/* Automatically adapt to client */
-		return (ra_type <= RA_WINNT);
-	} else
-		return (compat == ACL_COMPAT_WINNT);
-}
-
-
-/****************************************************************************
  Map canon_ace perms to permission bits NT.
  The attr element is not used here - we only process deny entries on set,
  not get. Deny entries are implicit on get with ace->perms = 0.
@@ -1107,10 +1089,7 @@ uint32_t map_canon_ace_perms(int snum,
 		 * to be changed in the future.
 		 */
 
-		if (nt4_compatible_acls())
-			nt_mask = UNIX_ACCESS_NONE;
-		else
-			nt_mask = 0;
+		nt_mask = 0;
 	} else {
 		if (directory_ace) {
 			nt_mask |= ((perms & S_IRUSR) ? UNIX_DIRECTORY_ACCESS_R : 0 );
@@ -1954,26 +1933,6 @@ static bool create_canon_ace_lists(files_struct *fsp,
 			DEBUG(3,("create_canon_ace_lists: unable to set anything but an ALLOW or DENY ACE.\n"));
 			return False;
 		}
-
-		if (nt4_compatible_acls()) {
-			/*
-			 * The security mask may be UNIX_ACCESS_NONE which should map into
-			 * no permissions (we overload the WRITE_OWNER bit for this) or it
-			 * should be one of the ALL/EXECUTE/READ/WRITE bits. Arrange for this
-			 * to be so. Any other bits override the UNIX_ACCESS_NONE bit.
-			 */
-
-			/*
-			 * Convert GENERIC bits to specific bits.
-			 */
- 
-			se_map_generic(&psa->access_mask, &file_generic_mapping);
-
-			psa->access_mask &= (UNIX_ACCESS_NONE|FILE_ALL_ACCESS);
-
-			if(psa->access_mask != UNIX_ACCESS_NONE)
-				psa->access_mask &= ~UNIX_ACCESS_NONE;
-		}
 	}
 
 	/*
@@ -3164,22 +3123,6 @@ static bool set_canon_ace_list(files_struct *fsp,
 }
 
 /****************************************************************************
- Find a particular canon_ace entry.
-****************************************************************************/
-
-static struct canon_ace *canon_ace_entry_for(struct canon_ace *list, SMB_ACL_TAG_T type, struct unixid *id)
-{
-	while (list) {
-		if (list->type == type && ((type != SMB_ACL_USER && type != SMB_ACL_GROUP) ||
-				(type == SMB_ACL_USER  && id && id->id == list->unix_ug.id) ||
-				(type == SMB_ACL_GROUP && id && id->id == list->unix_ug.id)))
-			break;
-		list = list->next;
-	}
-	return list;
-}
-
-/****************************************************************************
  
 ****************************************************************************/
 
@@ -3461,55 +3404,6 @@ static NTSTATUS posix_get_nt_acl_common(struct connection_struct *conn,
 			canon_ace *ace;
 			enum security_ace_type nt_acl_type;
 
-			if (nt4_compatible_acls() && dir_ace) {
-				/*
-				 * NT 4 chokes if an ACL contains an INHERIT_ONLY entry
-				 * but no non-INHERIT_ONLY entry for one SID. So we only
-				 * remove entries from the Access ACL if the
-				 * corresponding Default ACL entries have also been
-				 * removed. ACEs for CREATOR-OWNER and CREATOR-GROUP
-				 * are exceptions. We can do nothing
-				 * intelligent if the Default ACL contains entries that
-				 * are not also contained in the Access ACL, so this
-				 * case will still fail under NT 4.
-				 */
-
-				ace = canon_ace_entry_for(dir_ace, SMB_ACL_OTHER, NULL);
-				if (ace && !ace->perms) {
-					DLIST_REMOVE(dir_ace, ace);
-					TALLOC_FREE(ace);
-
-					ace = canon_ace_entry_for(file_ace, SMB_ACL_OTHER, NULL);
-					if (ace && !ace->perms) {
-						DLIST_REMOVE(file_ace, ace);
-						TALLOC_FREE(ace);
-					}
-				}
-
-				/*
-				 * WinNT doesn't usually have Creator Group
-				 * in browse lists, so we send this entry to
-				 * WinNT even if it contains no relevant
-				 * permissions. Once we can add
-				 * Creator Group to browse lists we can
-				 * re-enable this.
-				 */
-
-#if 0
-				ace = canon_ace_entry_for(dir_ace, SMB_ACL_GROUP_OBJ, NULL);
-				if (ace && !ace->perms) {
-					DLIST_REMOVE(dir_ace, ace);
-					TALLOC_FREE(ace);
-				}
-#endif
-
-				ace = canon_ace_entry_for(file_ace, SMB_ACL_GROUP_OBJ, NULL);
-				if (ace && !ace->perms) {
-					DLIST_REMOVE(file_ace, ace);
-					TALLOC_FREE(ace);
-				}
-			}
-
 			num_acls = count_canon_ace_list(file_ace);
 			num_def_acls = count_canon_ace_list(dir_ace);
 
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index aae4bd0..f95fddd 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -703,7 +703,6 @@ void reply_pipe_read_and_X(struct smb_request *req);
 /* The following definitions come from smbd/posix_acls.c  */
 
 void create_file_sids(const SMB_STRUCT_STAT *psbuf, struct dom_sid *powner_sid, struct dom_sid *pgroup_sid);
-bool nt4_compatible_acls(void);
 uint32_t map_canon_ace_perms(int snum,
                                 enum security_ace_type *pacl_type,
                                 mode_t perms,
diff --git a/source4/scripting/python/samba/tests/posixacl.py b/source4/scripting/python/samba/tests/posixacl.py
index f949ab4..652721f 100644
--- a/source4/scripting/python/samba/tests/posixacl.py
+++ b/source4/scripting/python/samba/tests/posixacl.py
@@ -92,7 +92,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
 
     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)"
-        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;;WO;;;WD)"
+        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, 0750)
         setntacl(self.lp, self.tempf, acl, "S-1-5-21-2212615479-2695158682-2101375467", use_ntvfs=False)
 
@@ -122,7 +122,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;;WO;;;WD)"
+        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)
         # 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, 0640)
@@ -133,7 +133,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
     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)"
         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;;WO;;;WD)"
+        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)
         # 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"))
@@ -193,7 +193,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         user_SID = s4_passdb.uid_to_sid(os.stat(self.tempf).st_uid)
         smbd.set_simple_acl(self.tempf, 0640)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
-        acl = "O:%sG:%sD:(A;;0x001f019f;;;%s)(A;;0x00120089;;;%s)(A;;WO;;;WD)" % (user_SID, group_SID, user_SID, group_SID)
+        acl = "O:%sG:%sD:(A;;0x001f019f;;;%s)(A;;0x00120089;;;%s)(A;;;;;WD)" % (user_SID, group_SID, user_SID, group_SID)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(acl, facl.as_sddl(anysid))
 
@@ -210,7 +210,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         smbd.chown(self.tempdir, BA_id, SO_id)
         smbd.set_simple_acl(self.tempdir, 0750)
         facl = getntacl(self.lp, self.tempdir, direct_db_access=False)
-        acl = "O:BAG:SOD:(A;;0x001f01ff;;;BA)(A;;0x001200a9;;;SO)(A;;WO;;;WD)(A;OICIIO;0x001f01ff;;;CO)(A;OICIIO;0x001f01ff;;;CG)(A;OICIIO;0x001f01ff;;;WD)"
+        acl = "O:BAG:SOD:(A;;0x001f01ff;;;BA)(A;;0x001200a9;;;SO)(A;;;;;WD)(A;OICIIO;0x001f01ff;;;CO)(A;OICIIO;0x001f01ff;;;CG)(A;OICIIO;0x001f01ff;;;WD)"
 
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(acl, facl.as_sddl(anysid))
@@ -225,7 +225,7 @@ class PosixAclMappingTests(TestCaseInTempDir):
         smbd.set_simple_acl(self.tempf, 0640, BA_gid)
         facl = getntacl(self.lp, self.tempf, direct_db_access=False)
         domsid = passdb.get_global_sam_sid()
-        acl = "O:%sG:%sD:(A;;0x001f019f;;;%s)(A;;0x00120089;;;BA)(A;;0x00120089;;;%s)(A;;WO;;;WD)" % (user_SID, group_SID, user_SID, group_SID)
+        acl = "O:%sG:%sD:(A;;0x001f019f;;;%s)(A;;0x00120089;;;BA)(A;;0x00120089;;;%s)(A;;;;;WD)" % (user_SID, group_SID, user_SID, group_SID)
         anysid = security.dom_sid(security.SID_NT_SELF)
         self.assertEquals(acl, facl.as_sddl(anysid))
 
-- 
1.7.7.3


>From 9a2ac0c077ccde82d362e6dccb8e317affc670bb Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 5 Nov 2012 20:44:57 +1100
Subject: [PATCH 5/7] pysmb: Allow callers to ask for access_mask values like
 SEC_FLAG_SYSTEM_SECURITY

This is in case we want to read the SACL

Andrew Bartlett

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source4/libcli/pysmb.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/source4/libcli/pysmb.c b/source4/libcli/pysmb.c
index 1122305..fb981c7 100644
--- a/source4/libcli/pysmb.c
+++ b/source4/libcli/pysmb.c
@@ -317,10 +317,11 @@ static PyObject *py_smb_getacl(pytalloc_Object *self, PyObject *args, PyObject *
 	union smb_fileinfo fio;
 	struct smb_private_data *spdata;
 	const char *filename;
-	int sinfo = 0;
+	uint32_t sinfo = 0;
+	int access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
 	int fnum;
 
-	if (!PyArg_ParseTuple(args, "s|i:get_acl", &filename, &sinfo)) {
+	if (!PyArg_ParseTuple(args, "s|Ii:get_acl", &filename, &sinfo, &access_mask)) {
 		return NULL;
 	}
 
@@ -331,7 +332,7 @@ static PyObject *py_smb_getacl(pytalloc_Object *self, PyObject *args, PyObject *
 	io.generic.level = RAW_OPEN_NTCREATEX;
 	io.ntcreatex.in.root_fid.fnum = 0;
 	io.ntcreatex.in.flags = 0;
-	io.ntcreatex.in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
+	io.ntcreatex.in.access_mask = access_mask;
 	io.ntcreatex.in.create_options = 0;
 	io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL;
 	io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_READ | 
-- 
1.7.7.3


>From ab9f845a90b25836649c42f2aa443ad2bcc21c95 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Nov 2012 16:45:03 +1100
Subject: [PATCH 6/7] ntvfs: Fill in sd->type based on the new ACL being added

Previously we would not change the type field, and just relied on what
was in the original ACL based on the default SD.

This is required to ensure the SEC_DESC_DACL_PROTECTED is set
which is in turn required for GPOs to be set correctly
to match what windows does.

Andrew Bartlett

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source4/ntvfs/posix/pvfs_acl.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/source4/ntvfs/posix/pvfs_acl.c b/source4/ntvfs/posix/pvfs_acl.c
index 1519631..4e9c1ac 100644
--- a/source4/ntvfs/posix/pvfs_acl.c
+++ b/source4/ntvfs/posix/pvfs_acl.c
@@ -330,6 +330,7 @@ NTSTATUS pvfs_acl_set(struct pvfs_state *pvfs,
 		}
 		sd->owner_sid = new_sd->owner_sid;
 	}
+
 	if (secinfo_flags & SECINFO_GROUP) {
 		if (!(access_mask & SEC_STD_WRITE_OWNER)) {
 			return NT_STATUS_ACCESS_DENIED;
@@ -349,19 +350,39 @@ NTSTATUS pvfs_acl_set(struct pvfs_state *pvfs,
 		}
 		sd->group_sid = new_sd->group_sid;
 	}
+
 	if (secinfo_flags & SECINFO_DACL) {
 		if (!(access_mask & SEC_STD_WRITE_DAC)) {
 			return NT_STATUS_ACCESS_DENIED;
 		}
 		sd->dacl = new_sd->dacl;
 		pvfs_translate_generic_bits(sd->dacl);
+		sd->type |= SEC_DESC_DACL_PRESENT;
 	}
+
 	if (secinfo_flags & SECINFO_SACL) {
 		if (!(access_mask & SEC_FLAG_SYSTEM_SECURITY)) {
 			return NT_STATUS_ACCESS_DENIED;
 		}
 		sd->sacl = new_sd->sacl;
 		pvfs_translate_generic_bits(sd->sacl);
+		sd->type |= SEC_DESC_SACL_PRESENT;
+	}
+
+	if (secinfo_flags & SECINFO_PROTECTED_DACL) {
+		if (new_sd->type & SEC_DESC_DACL_PROTECTED) {
+			sd->type |= SEC_DESC_DACL_PROTECTED;
+		} else {
+			sd->type &= ~SEC_DESC_DACL_PROTECTED;
+		}
+	}
+
+	if (secinfo_flags & SECINFO_PROTECTED_SACL) {
+		if (new_sd->type & SEC_DESC_SACL_PROTECTED) {
+			sd->type |= SEC_DESC_SACL_PROTECTED;
+		} else {
+			sd->type &= ~SEC_DESC_SACL_PROTECTED;
+		}
 	}
 
 	if (new_uid == old_uid) {
-- 
1.7.7.3


>From e04ec87137963033a32707ca419c07fba2d07f1f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 13 Nov 2012 16:03:27 +1100
Subject: [PATCH 7/7] scripting ntacls: Do not place a SACL in the GPO
 filesystem ACL

On a new GPO created on windows, the SACL is not used.

Andrew Bartlett

Reviewed by: Jeremy Allison <jra at samba.org>
---
 source4/scripting/python/samba/ntacls.py |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/source4/scripting/python/samba/ntacls.py b/source4/scripting/python/samba/ntacls.py
index 89d450a..8992b61 100644
--- a/source4/scripting/python/samba/ntacls.py
+++ b/source4/scripting/python/samba/ntacls.py
@@ -211,7 +211,6 @@ def dsacl2fsacl(dssddl, sid):
     fdescr.group_sid = ref.group_sid
     fdescr.type = ref.type
     fdescr.revision = ref.revision
-    fdescr.sacl = ref.sacl
     aces = ref.dacl.aces
     for i in range(0, len(aces)):
         ace = aces[i]
-- 
1.7.7.3



More information about the samba-technical mailing list