Attachment 12302 Details for Bug 12028 – Patch for 4.3 and 4.4 cherry-picked from master

[patch] Patch for 4.3 and 4.4 cherry-picked from master

v43,v44-bug12028.patch (text/plain), 11.36 KB, created by Ralph Böhme on 2016-08-02 10:30:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2016-08-02 10:30:13 UTC

Size: 11.36 KB

patch

obsolete

>From dedc6a69450fbacd01b18cf18d241acb2502d184 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 15 Jul 2016 17:56:02 +0200
>Subject: [PATCH 1/2] s3/smbd: move make_default_filesystem_acl() to
> vfs_acl_common.c
>
>This function is only used in vfs_acl_common.c and will be modified in
>the next commit.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12028
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit afc2417b107af572081974ff9d013ddec890d31f)
>---
> source3/modules/vfs_acl_common.c | 108 ++++++++++++++++++++++++++++++++++++++
> source3/smbd/posix_acls.c        | 110 ---------------------------------------
> source3/smbd/proto.h             |   4 --
> 3 files changed, 108 insertions(+), 114 deletions(-)
>
>diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
>index 76ac598..dc96761 100644
>--- a/source3/modules/vfs_acl_common.c
>+++ b/source3/modules/vfs_acl_common.c
>@@ -358,6 +358,114 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle,
> 	return NT_STATUS_OK;
> }
> 
>+static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
>+					    const char *name,
>+					    SMB_STRUCT_STAT *psbuf,
>+					    struct security_descriptor **ppdesc)
>+{
>+	struct dom_sid owner_sid, group_sid;
>+	size_t size = 0;
>+	struct security_ace aces[4];
>+	uint32_t access_mask = 0;
>+	mode_t mode = psbuf->st_ex_mode;
>+	struct security_acl *new_dacl = NULL;
>+	int idx = 0;
>+
>+	DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n",
>+		name, (int)mode ));
>+
>+	uid_to_sid(&owner_sid, psbuf->st_ex_uid);
>+	gid_to_sid(&group_sid, psbuf->st_ex_gid);
>+
>+	/*
>+	 We provide up to 4 ACEs
>+		- Owner
>+		- Group
>+		- Everyone
>+		- NT System
>+	*/
>+
>+	if (mode & S_IRUSR) {
>+		if (mode & S_IWUSR) {
>+			access_mask |= SEC_RIGHTS_FILE_ALL;
>+		} else {
>+			access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>+		}
>+	}
>+	if (mode & S_IWUSR) {
>+		access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
>+	}
>+
>+	init_sec_ace(&aces[idx],
>+			&owner_sid,
>+			SEC_ACE_TYPE_ACCESS_ALLOWED,
>+			access_mask,
>+			0);
>+	idx++;
>+
>+	access_mask = 0;
>+	if (mode & S_IRGRP) {
>+		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>+	}
>+	if (mode & S_IWGRP) {
>+		/* note that delete is not granted - this matches posix behaviour */
>+		access_mask |= SEC_RIGHTS_FILE_WRITE;
>+	}
>+	if (access_mask) {
>+		init_sec_ace(&aces[idx],
>+			&group_sid,
>+			SEC_ACE_TYPE_ACCESS_ALLOWED,
>+			access_mask,
>+			0);
>+		idx++;
>+	}
>+
>+	access_mask = 0;
>+	if (mode & S_IROTH) {
>+		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>+	}
>+	if (mode & S_IWOTH) {
>+		access_mask |= SEC_RIGHTS_FILE_WRITE;
>+	}
>+	if (access_mask) {
>+		init_sec_ace(&aces[idx],
>+			&global_sid_World,
>+			SEC_ACE_TYPE_ACCESS_ALLOWED,
>+			access_mask,
>+			0);
>+		idx++;
>+	}
>+
>+	init_sec_ace(&aces[idx],
>+			&global_sid_System,
>+			SEC_ACE_TYPE_ACCESS_ALLOWED,
>+			SEC_RIGHTS_FILE_ALL,
>+			0);
>+	idx++;
>+
>+	new_dacl = make_sec_acl(ctx,
>+			NT4_ACL_REVISION,
>+			idx,
>+			aces);
>+
>+	if (!new_dacl) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+
>+	*ppdesc = make_sec_desc(ctx,
>+			SECURITY_DESCRIPTOR_REVISION_1,
>+			SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT,
>+			&owner_sid,
>+			&group_sid,
>+			NULL,
>+			new_dacl,
>+			&size);
>+	if (!*ppdesc) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+	return NT_STATUS_OK;
>+}
>+
> /*******************************************************************
>  Pull a DATA_BLOB from an xattr given a pathname.
>  If the hash doesn't match, or doesn't exist - return the underlying
>diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
>index 0c9c749..d87253d 100644
>--- a/source3/smbd/posix_acls.c
>+++ b/source3/smbd/posix_acls.c
>@@ -4662,116 +4662,6 @@ NTSTATUS get_nt_acl_no_snum(TALLOC_CTX *ctx, const char *fname,
> 	return status;
> }
> 
>-/* Stolen shamelessly from pvfs_default_acl() in source4 :-). */
>-
>-NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
>-					const char *name,
>-					SMB_STRUCT_STAT *psbuf,
>-					struct security_descriptor **ppdesc)
>-{
>-	struct dom_sid owner_sid, group_sid;
>-	size_t size = 0;
>-	struct security_ace aces[4];
>-	uint32_t access_mask = 0;
>-	mode_t mode = psbuf->st_ex_mode;
>-	struct security_acl *new_dacl = NULL;
>-	int idx = 0;
>-
>-	DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n",
>-		name, (int)mode ));
>-
>-	uid_to_sid(&owner_sid, psbuf->st_ex_uid);
>-	gid_to_sid(&group_sid, psbuf->st_ex_gid);
>-
>-	/*
>-	 We provide up to 4 ACEs
>-		- Owner
>-		- Group
>-		- Everyone
>-		- NT System
>-	*/
>-
>-	if (mode & S_IRUSR) {
>-		if (mode & S_IWUSR) {
>-			access_mask |= SEC_RIGHTS_FILE_ALL;
>-		} else {
>-			access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>-		}
>-	}
>-	if (mode & S_IWUSR) {
>-		access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
>-	}
>-
>-	init_sec_ace(&aces[idx],
>-			&owner_sid,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			access_mask,
>-			0);
>-	idx++;
>-
>-	access_mask = 0;
>-	if (mode & S_IRGRP) {
>-		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>-	}
>-	if (mode & S_IWGRP) {
>-		/* note that delete is not granted - this matches posix behaviour */
>-		access_mask |= SEC_RIGHTS_FILE_WRITE;
>-	}
>-	if (access_mask) {
>-		init_sec_ace(&aces[idx],
>-			&group_sid,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			access_mask,
>-			0);
>-		idx++;
>-	}
>-
>-	access_mask = 0;
>-	if (mode & S_IROTH) {
>-		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>-	}
>-	if (mode & S_IWOTH) {
>-		access_mask |= SEC_RIGHTS_FILE_WRITE;
>-	}
>-	if (access_mask) {
>-		init_sec_ace(&aces[idx],
>-			&global_sid_World,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			access_mask,
>-			0);
>-		idx++;
>-	}
>-
>-	init_sec_ace(&aces[idx],
>-			&global_sid_System,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			SEC_RIGHTS_FILE_ALL,
>-			0);
>-	idx++;
>-
>-	new_dacl = make_sec_acl(ctx,
>-			NT4_ACL_REVISION,
>-			idx,
>-			aces);
>-
>-	if (!new_dacl) {
>-		return NT_STATUS_NO_MEMORY;
>-	}
>-
>-	*ppdesc = make_sec_desc(ctx,
>-			SECURITY_DESCRIPTOR_REVISION_1,
>-			SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT,
>-			&owner_sid,
>-			&group_sid,
>-			NULL,
>-			new_dacl,
>-			&size);
>-	if (!*ppdesc) {
>-		return NT_STATUS_NO_MEMORY;
>-	}
>-	return NT_STATUS_OK;
>-}
>-
> int posix_sys_acl_blob_get_file(vfs_handle_struct *handle,
> 				const char *path_p,
> 				TALLOC_CTX *mem_ctx,
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index 914951e..1ef84a0 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -764,10 +764,6 @@ bool set_unix_posix_acl(connection_struct *conn, files_struct *fsp, const char *
> NTSTATUS get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname,
> 			     uint32_t security_info_wanted,
> 			     struct security_descriptor **sd);
>-NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
>-					const char *name,
>-					SMB_STRUCT_STAT *psbuf,
>-					struct security_descriptor **ppdesc);
> int posix_sys_acl_blob_get_file(vfs_handle_struct *handle,
> 				const char *path_p,
> 				TALLOC_CTX *mem_ctx,
>-- 
>2.7.4
>
>
>From 562646812856e98c14fb004d741d433d7f55d6c7 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 15 Jul 2016 17:48:19 +0200
>Subject: [PATCH 2/2] vfs_acl_xattr: objects without NT ACL xattr
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Even with "ignore system acls" set to "yes", for objects without NT ACL
>xattr we use the underlying filesystem permissions to construct an NT
>ACL. This can result in *very* unexpected permissions, eg:
>
>- a directory with the following ACL:
>
>$ ./bin/smbcacls -Uslow%pass //localhost/normal ""
>REVISION:1
>CONTROL:SR|DP
>OWNER:SLOW\slow
>GROUP:Unix Group\root
>ACL:SLOW\slow:ALLOWED/0x0/FULL
>
>So only one non-inheritable(!) ACE.
>
>- creating a subdirectory:
>
>$ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir1"
>
>- checking whether there's an ACL xattr:
>
>$ getfattr -m "" /Volumes/normal/dir1
>getfattr: Removing leading '/' from absolute path names
>system.posix_acl_access
>system.posix_acl_default
>user.DOSATTRIB
>
>So there isn't an ACL xattr, because there where no inheritable ACEs on
>the parent folder.
>
>- reading the new subdirectories ACL:
>
>$ ./bin/smbcacls -Uslow%pass //localhost/normal "dir1"
>REVISION:1
>CONTROL:SR|DP
>OWNER:SLOW\slow
>GROUP:Unix Group\slow
>ACL:SLOW\slow:ALLOWED/0x0/FULL
>ACL:Unix Group\slow:ALLOWED/0x0/READ
>ACL:Everyone:ALLOWED/0x0/READ
>ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
>
>The ACES for "SLOW\slow", "Unix Group\slow" and "Everyone" are coming
>from the underlying filesystem. This is the problem.
>
>- Windows assigns the following ACL in this situation:
>
>$ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir"
>REVISION:1
>CONTROL:SR|PD|DI|DP
>OWNER:VORDEFINIERT\Administratoren
>GROUP:WIN2008R2\DomÃ¤nen-Benutzer
>ACL:WIN2008R2\Administrator:ALLOWED/0x0/FULL
>
>$ ./bin/smbclient -UAdministrator%Passw0rd //10.10.10.14/data -c "mkdir dir\dir1"
>
>$ ./bin/smbcacls -UAdministrator%Passw0rd //10.10.10.14/data "dir\dir1"
>REVISION:1
>CONTROL:SR|DI|DP
>OWNER:VORDEFINIERT\Administratoren
>GROUP:WIN2008R2\DomÃ¤nen-Benutzer
>ACL:VORDEFINIERT\Administratoren:ALLOWED/0x0/FULL
>ACL:NT-AUTORITÃT\SYSTEM:ALLOWED/0x0/FULL
>
>By changing make_default_filesystem_acl() to only adds user and system
>ACE to the ACL of objects that lack an ACL xattr, we match Windows
>behaviour:
>
>$ ./bin/smbclient -Uslow%pass //localhost/normal -c "mkdir dir2"
>
>$ ./bin/smbcacls -Uslow%pass //localhost/normal "dir2"
>REVISION:1
>CONTROL:SR|DP
>OWNER:SLOW\slow
>GROUP:Unix Group\slow
>ACL:SLOW\slow:ALLOWED/0x0/FULL
>ACL:NT Authority\SYSTEM:ALLOWED/0x0/FULL
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=12028
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Tue Jul 19 10:22:05 CEST 2016 on sn-devel-144
>
>(cherry picked from commit 961c4b591bb102751079d9cc92d7aa1c37f1958c)
>---
> source3/modules/vfs_acl_common.c | 44 +++++-----------------------------------
> 1 file changed, 5 insertions(+), 39 deletions(-)
>
>diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
>index dc96761..f5af666 100644
>--- a/source3/modules/vfs_acl_common.c
>+++ b/source3/modules/vfs_acl_common.c
>@@ -24,6 +24,7 @@
> #include "../libcli/security/security.h"
> #include "../librpc/gen_ndr/ndr_security.h"
> #include "../lib/util/bitmap.h"
>+#include "passdb/lookup_sid.h"
> 
> static NTSTATUS create_acl_blob(const struct security_descriptor *psd,
> 			DATA_BLOB *pblob,
>@@ -378,12 +379,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
> 	gid_to_sid(&group_sid, psbuf->st_ex_gid);
> 
> 	/*
>-	 We provide up to 4 ACEs
>-		- Owner
>-		- Group
>-		- Everyone
>-		- NT System
>-	*/
>+	 * We provide 2 ACEs:
>+	 * - Owner
>+	 * - NT System
>+	 */
> 
> 	if (mode & S_IRUSR) {
> 		if (mode & S_IWUSR) {
>@@ -403,39 +402,6 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
> 			0);
> 	idx++;
> 
>-	access_mask = 0;
>-	if (mode & S_IRGRP) {
>-		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>-	}
>-	if (mode & S_IWGRP) {
>-		/* note that delete is not granted - this matches posix behaviour */
>-		access_mask |= SEC_RIGHTS_FILE_WRITE;
>-	}
>-	if (access_mask) {
>-		init_sec_ace(&aces[idx],
>-			&group_sid,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			access_mask,
>-			0);
>-		idx++;
>-	}
>-
>-	access_mask = 0;
>-	if (mode & S_IROTH) {
>-		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
>-	}
>-	if (mode & S_IWOTH) {
>-		access_mask |= SEC_RIGHTS_FILE_WRITE;
>-	}
>-	if (access_mask) {
>-		init_sec_ace(&aces[idx],
>-			&global_sid_World,
>-			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-			access_mask,
>-			0);
>-		idx++;
>-	}
>-
> 	init_sec_ace(&aces[idx],
> 			&global_sid_System,
> 			SEC_ACE_TYPE_ACCESS_ALLOWED,
>-- 
>2.7.4
>

Flags:

metze: review+
slow: review? (jra)

Actions: View

Attachments on bug 12028: 12302