Question about vfs_acl_common not setting filesystem permissions anymore
Ralph Böhme
slow at samba.org
Sat Aug 27 15:45:54 UTC 2016
Hi!
On Wed, Aug 24, 2016 at 02:11:53PM -0700, Jeremy Allison wrote:
> On Wed, Aug 24, 2016 at 03:04:53PM +0200, Ralph Böhme wrote:
> > Hi Uri,
> >
> > *phew*! This stuff is driving me insane. As I've got to implement some
> > related improvements in the ACL code, I was working on a refactoring
> > of get_nt_acl_internal(). Got it down to 150 lines from 350. Finally
> > even I understand the logic. :)
>
> Oh, didn't think it was that bad, it did grow somewhat organically :-).
>
> > So we have consensus that forcing create mask=0777 and directory
> > mask=0666 in the modules is the way to address this? If yes, I'll file
> > a bugreport and prepare patches. I guess we then must also force all
> > map archive|hidden|system|readonly to no.
>
> There is precedence for this - in source3/modules/vfs_acl_xattr.c:connect_acl_xattr()
> we already have:
>
> lp_do_parameter(SNUM(handle->conn), "inherit acls", "true");
> lp_do_parameter(SNUM(handle->conn), "dos filemode", "true");
> lp_do_parameter(SNUM(handle->conn), "force unknown acl user", "true");
of course, that's was my inspiration. :)
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.
Cheerio!
-slow
-------------- next part --------------
From 092d8a000b8dcaaba0d485b36c9a386eebad31b6 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>
---
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.7.4
From bdec0973446e76fdca3a95e2d8478ab77c4a72eb 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>
---
docs-xml/manpages/vfs_acl_tdb.8.xml | 14 ++++++++++++++
docs-xml/manpages/vfs_acl_xattr.8.xml | 14 ++++++++++++++
source3/modules/vfs_acl_tdb.c | 18 ++++++++++++++++++
source3/modules/vfs_acl_xattr.c | 18 ++++++++++++++++++
source4/torture/vfs/acl_xattr.c | 4 ++--
5 files changed, 66 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..483bf0c 100644
--- a/docs-xml/manpages/vfs_acl_tdb.8.xml
+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
@@ -70,6 +70,20 @@
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>
+ </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..a9fe66c 100644
--- a/docs-xml/manpages/vfs_acl_xattr.8.xml
+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
@@ -74,6 +74,20 @@
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>
+ </itemizedlist>
+ </para>
</listitem>
</varlistentry>
diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index 0c92b72..b3208dd 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,23 @@ 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");
+ }
+
return 0;
}
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index 307ab6a..5ae58d6 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,23 @@ 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");
+ }
+
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.7.4
More information about the samba-technical
mailing list