[PATCH] vfs_acl_xattr|tdb: set create mask to 0777 if ignore_system_acls is set

[Ralph Böhme]

On Fri, Feb 10, 2017 at 11:31:38AM -0800, Jeremy Allison wrote:
> On Thu, Feb 09, 2017 at 11:03:21AM -0800, Jeremy Allison wrote:
> > On Mon, Feb 06, 2017 at 01:19:48PM +0100, Ralph Böhme wrote:
> > > Hi!
> > > 
> > > Attached is a patch for bug
> > > https://bugzilla.samba.org/show_bug.cgi?id=12562
> > > 
> > > The fix for bug #12181 included a change that should ensure filesystem
> > > permissions are out of the way when using VFS modules acl_xattr or acl_tdb with
> > > "acl_xattr:ignore system acls = yes".
> > > 
> > > At runtime, when the module is loaded, we set "create mask = 0666" which doesn't
> > > contain executable rights files. This should really by "create mask = 0777"
> > > instead.
> > > 
> > > Please review & push if happy. Thanks!
> > 
> > Hi Ralph,
> > 
> > Can you explain the customer scenario that instigated
> > this fix ?
> > 
> > It's *probably* right, but I think Uri is asking the
> > right questions about defauling files to 'x' access
> > and I want to understand the exact failure case before
> > I OK this :-).
> 
> Ping Ralph, I'd love to get this sorted asap.

well, the customer scenario is "support *some* legacy scenario", I don't have
more details. :)

But I have a rewored patch that should work for all of us: it ensures "create
mask" is *at least* 0666. Customer can set "create mask = 0777" and be happy, we
keep the default 0666, Uri is happy. :)

Ok?

Cheerio!
-slow
-------------- next part --------------
From f961c3e10f2b743db2a1e220f6d49d6785b7fc1b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 6 Feb 2017 12:47:41 +0100
Subject: [PATCH] vfs_acl_xattr|tdb: ensure create mask is at least 0666 if
 ignore_system_acls is set

Bug: https://bugzilla.samba.org/show_bug.cgi?id=12562

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_tdb.c   | 24 +++++++++++++++++++++---
 source3/modules/vfs_acl_xattr.c | 24 +++++++++++++++++++++---
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index 174affe..a71bfdc 100644
--- a/source3/modules/vfs_acl_tdb.c
+++ b/source3/modules/vfs_acl_tdb.c
@@ -342,12 +342,30 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
 				return -1);
 
 	if (config->ignore_system_acls) {
-		DBG_NOTICE("setting 'create mask = 0666', "
-			   "'directory mask = 0777', "
+		mode_t create_mask = lp_create_mask(SNUM(handle->conn));
+		char *create_mask_str = NULL;
+
+		if ((create_mask & 0666) != 0666) {
+			create_mask |= 0666;
+			create_mask_str = talloc_asprintf(handle, "0%o",
+							  create_mask);
+			if (create_mask_str == NULL) {
+				DBG_ERR("talloc_asprintf failed\n");
+				return -1;
+			}
+
+			DBG_NOTICE("setting 'create mask = %s'\n", create_mask_str);
+
+			lp_do_parameter (SNUM(handle->conn),
+					"create mask", create_mask_str);
+
+			TALLOC_FREE(create_mask_str);
+		}
+
+		DBG_NOTICE("setting 'directory mask = 0777', "
 			   "'store dos attributes = yes' and all "
 			   "'map ...' options to 'no'\n");
 
-		lp_do_parameter(SNUM(handle->conn), "create mask", "0666");
 		lp_do_parameter(SNUM(handle->conn), "directory mask", "0777");
 		lp_do_parameter(SNUM(handle->conn), "map archive", "no");
 		lp_do_parameter(SNUM(handle->conn), "map hidden", "no");
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index e1f90ff..42028be 100644
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -209,12 +209,30 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
 				return -1);
 
 	if (config->ignore_system_acls) {
-		DBG_NOTICE("setting 'create mask = 0666', "
-			   "'directory mask = 0777', "
+		mode_t create_mask = lp_create_mask(SNUM(handle->conn));
+		char *create_mask_str = NULL;
+
+		if ((create_mask & 0666) != 0666) {
+			create_mask |= 0666;
+			create_mask_str = talloc_asprintf(handle, "0%o",
+							  create_mask);
+			if (create_mask_str == NULL) {
+				DBG_ERR("talloc_asprintf failed\n");
+				return -1;
+			}
+
+			DBG_NOTICE("setting 'create mask = %s'\n", create_mask_str);
+
+			lp_do_parameter (SNUM(handle->conn),
+					"create mask", create_mask_str);
+
+			TALLOC_FREE(create_mask_str);
+		}
+
+		DBG_NOTICE("setting 'directory mask = 0777', "
 			   "'store dos attributes = yes' and all "
 			   "'map ...' options to 'no'\n");
 
-		lp_do_parameter(SNUM(handle->conn), "create mask", "0666");
 		lp_do_parameter(SNUM(handle->conn), "directory mask", "0777");
 		lp_do_parameter(SNUM(handle->conn), "map archive", "no");
 		lp_do_parameter(SNUM(handle->conn), "map hidden", "no");
-- 
2.9.3