Question about vfs_acl_common not setting filesystem permissions anymore

Jeremy Allison jra at samba.org
Tue Aug 30 23:30:12 UTC 2016


On Tue, Aug 30, 2016 at 12:20:27PM -0700, Jeremy Allison wrote:
> On Tue, Aug 30, 2016 at 05:38:36AM +0200, Ralph Böhme wrote:
> > On Mon, Aug 29, 2016 at 05:22:15PM -0700, Jeremy Allison wrote:
> > > On Mon, Aug 29, 2016 at 09:17:13AM +0200, Ralph Böhme wrote:
> > > > Hi
> > > > 
> > > > On Sat, Aug 27, 2016 at 05:45:54PM +0200, Ralph Böhme wrote:
> > > > > Patch attached, it depends on the other acl_xattr|tdb patch. Please
> > > > > review & comment. I'm still pondering the idea of forcing "store dos
> > > > > attributes = yes" instead of the map xxx stuff.
> > > > 
> > > > sorry, wrong version, didn't build (new DEBUG macro call with a
> > > > surplus closing parenthesis). Correct one attached.
> > > 
> > > If you're setting:
> > > 
> > > map archive = no
> > > map hidden = no
> > > map readonly = no
> > > map system = no
> > > 
> > > Aren't you also going to need to set:
> > > 
> > > store dos attributes = yes
> > > 
> > > be default too ?
> > 
> > that's why I was asking. :)
> > 
> > But my understanding is, while it would make perfect sense to set it
> > (which would also render it needless to set the map xxx stuff to no in
> > the first place), it may be a valid combination to set
> > 
> > map archive = no
> > map hidden = no
> > map readonly = no
> > map system = no
> > store dos attributes = no
> > 
> > Doesn't make sense, but someone out there might be using it for some
> > strange reason.
> 
> I can't see that. IMHO this is an invalid combination. At
> that point there is no way to store DOS attibutes at all
> and clients will break.
> 
> So I think we need to add 'store dos attributes = yes'
> inside this patchset too.

So here it is. Ralph, it you're happy please review
and push !

Jeremy.
-------------- next part --------------
From e5385e744300f23c72b859accbaf6fbf86317964 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 26 Aug 2016 10:22:37 +0200
Subject: [PATCH 1/2] docs: document vfs_acl_xattr|tdb enforced settings

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

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 docs-xml/manpages/vfs_acl_tdb.8.xml   | 9 +++++++++
 docs-xml/manpages/vfs_acl_xattr.8.xml | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml b/docs-xml/manpages/vfs_acl_tdb.8.xml
index 607e344..68e4179 100644
--- a/docs-xml/manpages/vfs_acl_tdb.8.xml
+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
@@ -40,6 +40,15 @@
 	<filename>$LOCKDIR/file_ntacls.tdb</filename>.
 	</para>
 
+	<para>
+	This module forces the following parameters:
+	<itemizedlist>
+	<listitem><para>inherit acls = true</para></listitem>
+	<listitem><para>dos filemode = true</para></listitem>
+	<listitem><para>force unknown acl user = true</para></listitem>
+	</itemizedlist>
+	</para>
+
 	<para>This module is stackable.</para>
 </refsect1>
 
diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml
index 8da73e0..8396ced 100644
--- a/docs-xml/manpages/vfs_acl_xattr.8.xml
+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
@@ -44,6 +44,15 @@
 	</command>).
 	</para>
 
+	<para>
+	This module forces the following parameters:
+	<itemizedlist>
+	<listitem><para>inherit acls = true</para></listitem>
+	<listitem><para>dos filemode = true</para></listitem>
+	<listitem><para>force unknown acl user = true</para></listitem>
+	</itemizedlist>
+	</para>
+
 	<para>This module is stackable.</para>
 </refsect1>
 
-- 
2.8.0.rc3.226.g39d4020


From 12562f2bc3e4ed932a71127dfe8510babed99dff Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 26 Aug 2016 10:04:53 +0200
Subject: [PATCH 2/2] vfs_acl_xattr|tdb: enforced settings when ignore system
 acls=yes

When "ignore system acls" is set to "yes, we need to ensure filesystem
permission always grant access so that when doing our own access checks
we don't run into situations where we grant access but the filesystem
doesn't.

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

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 docs-xml/manpages/vfs_acl_tdb.8.xml   | 15 +++++++++++++++
 docs-xml/manpages/vfs_acl_xattr.8.xml | 15 +++++++++++++++
 source3/modules/vfs_acl_tdb.c         | 20 ++++++++++++++++++++
 source3/modules/vfs_acl_xattr.c       | 20 ++++++++++++++++++++
 source4/torture/vfs/acl_xattr.c       |  4 ++--
 5 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml b/docs-xml/manpages/vfs_acl_tdb.8.xml
index 68e4179..2510f08 100644
--- a/docs-xml/manpages/vfs_acl_tdb.8.xml
+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
@@ -70,6 +70,21 @@
 		access the data via Samba you might set this to yes to achieve
 		better NT ACL compatibility.
 		</para>
+
+		<para>
+		If <emphasis>acl_tdb:ignore system acls</emphasis>
+		is set to <emphasis>yes</emphasis>, the following
+		additional settings will be enforced:
+		<itemizedlist>
+		<listitem><para>create mask = 0666</para></listitem>
+		<listitem><para>directory mask = 0777</para></listitem>
+		<listitem><para>map archive = no</para></listitem>
+		<listitem><para>map hidden = no</para></listitem>
+		<listitem><para>map readonly = no</para></listitem>
+		<listitem><para>map system = no</para></listitem>
+		<listitem><para>store dos attributes = yes</para></listitem>
+		</itemizedlist>
+		</para>
 		</listitem>
 		</varlistentry>
 
diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml
index 8396ced..9d21290 100644
--- a/docs-xml/manpages/vfs_acl_xattr.8.xml
+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
@@ -74,6 +74,21 @@
 		access the data via Samba you might set this to yes to achieve
 		better NT ACL compatibility.
 		</para>
+
+		<para>
+		If <emphasis>acl_xattr:ignore system acls</emphasis>
+		is set to <emphasis>yes</emphasis>, the following
+		additional settings will be enforced:
+		<itemizedlist>
+		<listitem><para>create mask = 0666</para></listitem>
+		<listitem><para>directory mask = 0777</para></listitem>
+		<listitem><para>map archive = no</para></listitem>
+		<listitem><para>map hidden = no</para></listitem>
+		<listitem><para>map readonly = no</para></listitem>
+		<listitem><para>map system = no</para></listitem>
+		<listitem><para>store dos attributes = yes</para></listitem>
+		</itemizedlist>
+		</para>
 		</listitem>
 		</varlistentry>
 
diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index 0c92b72..7543487 100644
--- a/source3/modules/vfs_acl_tdb.c
+++ b/source3/modules/vfs_acl_tdb.c
@@ -309,6 +309,7 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
 {
 	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
 	bool ok;
+	struct acl_common_config *config = NULL;
 
 	if (ret < 0) {
 		return ret;
@@ -336,6 +337,25 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
 	lp_do_parameter(SNUM(handle->conn), "dos filemode", "true");
 	lp_do_parameter(SNUM(handle->conn), "force unknown acl user", "true");
 
+	SMB_VFS_HANDLE_GET_DATA(handle, config,
+				struct acl_common_config,
+				return -1);
+
+	if (config->ignore_system_acls) {
+		DBG_NOTICE("setting 'create mask = 0666', "
+			   "'directory mask = 0777' 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");
+		lp_do_parameter(SNUM(handle->conn), "map readonly", "no");
+		lp_do_parameter(SNUM(handle->conn), "map system", "no");
+		lp_do_parameter(SNUM(handle->conn), "store dos attributes",
+				"yes");
+	}
+
 	return 0;
 }
 
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index 307ab6a..ad8615c 100644
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -181,6 +181,7 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
 {
 	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
 	bool ok;
+	struct acl_common_config *config = NULL;
 
 	if (ret < 0) {
 		return ret;
@@ -203,6 +204,25 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
         lp_do_parameter(SNUM(handle->conn), "dos filemode", "true");
         lp_do_parameter(SNUM(handle->conn), "force unknown acl user", "true");
 
+	SMB_VFS_HANDLE_GET_DATA(handle, config,
+				struct acl_common_config,
+				return -1);
+
+	if (config->ignore_system_acls) {
+		DBG_NOTICE("setting 'create mask = 0666', "
+			   "'directory mask = 0777' 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");
+		lp_do_parameter(SNUM(handle->conn), "map readonly", "no");
+		lp_do_parameter(SNUM(handle->conn), "map system", "no");
+		lp_do_parameter(SNUM(handle->conn), "store dos attributes",
+				"yes");
+	}
+
 	return 0;
 }
 
diff --git a/source4/torture/vfs/acl_xattr.c b/source4/torture/vfs/acl_xattr.c
index 7fd10d0..df4dd29 100644
--- a/source4/torture/vfs/acl_xattr.c
+++ b/source4/torture/vfs/acl_xattr.c
@@ -169,8 +169,8 @@ static bool test_default_acl_posix(struct torture_context *tctx,
 	exp_sd = security_descriptor_dacl_create(
 		tctx, 0, owner_sid, group_sid,
 		owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
-		group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
-		SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
+		group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, FILE_GENERIC_READ|FILE_GENERIC_WRITE|FILE_GENERIC_EXECUTE, 0,
+		SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, FILE_GENERIC_READ|FILE_GENERIC_WRITE|FILE_GENERIC_EXECUTE, 0,
 		SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
 		NULL);
 
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list