[PATCH] Fix open bug found at Microsoft interop event - version 2

Jeremy Allison jra at samba.org
Thu Jun 20 15:57:26 MDT 2013


Here is a (much better :-) patch to fix the specific
issue found at the MS-test event in Redmond. The underlying
issue is that on ZFS filesystems using the vfs_zfsaacl module
do not add in the FILE_DELETE_CHILD permission on ACE entries
on files that have all other permissions set (as the posix
acl mapping code does).

Introduces (and fully documents :-) a parameter
"nfs4:map full control" that controls this activity. Also
includes a new (previously missing :-) man page for
the vfs_zfsacl module.

Also includes a new torture test that tests the NFSv4
ACL module with this parameter turned on for the raw.acls
test.

I'm pretty happy with this patch. Once it's in the
only follow-up argument is whether the new parameter
should default to "true" by default (as Ira would
like :-), or to "false" by default (which will preserve
any acl_xattr Windows ACLs stacked on top of a vfs_zfsacl
module share).

By default (and to be conservative for the acl_xattr
users) I've set it to false in this patch, but we
can argue about this after it's gone in :-).

Samba-on-ZFS users, please look at this (Ira, Richard,
Andrew).

Cheers,

	Jeremy.
-------------- next part --------------
From 5979bd9cb9d4db7724a01019ab22cfe848d99391 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Jun 2013 14:33:30 -0700
Subject: [PATCH 1/4] Add "nfs4:map full control" parameter to control the
 adding of the DELETE_CHILD parameter on NFSv4/ZFS/GPFS file ACE's.

Windows maps an open request of GENERIC_ALL on files to 0x1FF specific bits, which
includes DELETE_CHILD even though this has no meaning on file ACE's. If a returned
NFSv4 ACE entry for a file has all other specific bits set except for DELETE (which
comes from the containing directory) and DELETE_CHILD (which has no meaning) then
optionally add it into the returned ACE entry.

Fixes issue with Microsoft SMB2 torture test suite found at the interop event
in Redmond, WA.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/nfs4_acls.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
index 13e9268..02e32f1 100644
--- a/source3/modules/nfs4_acls.c
+++ b/source3/modules/nfs4_acls.c
@@ -61,6 +61,7 @@ typedef struct _smbacl4_vfs_params {
 	enum smbacl4_mode_enum mode;
 	bool do_chown;
 	enum smbacl4_acedup_enum acedup;
+	bool map_full_control;
 } smbacl4_vfs_params;
 
 /*
@@ -94,11 +95,14 @@ static int smbacl4_get_vfs_params(
 	params->acedup = (enum smbacl4_acedup_enum)lp_parm_enum(
 		SNUM(conn), type_name,
 		"acedup", enum_smbacl4_acedups, e_dontcare);
+	params->map_full_control = lp_parm_bool(SNUM(conn), type_name,
+		"map full control", false);
 
-	DEBUG(10, ("mode:%s, do_chown:%s, acedup: %s\n",
+	DEBUG(10, ("mode:%s, do_chown:%s, acedup: %s map full control:%s\n",
 		enum_smbacl4_modes[params->mode].name,
 		params->do_chown ? "true" : "false",
-		enum_smbacl4_acedups[params->acedup].name));
+		enum_smbacl4_acedups[params->acedup].name,
+		params->map_full_control ? "true" : "false"));
 
 	return 0;
 }
@@ -383,6 +387,18 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx,
 			ace->aceMask |= SMB_ACE4_DELETE_CHILD;
 		}
 
+		if (!is_directory && params->map_full_control) {
+			/*
+			 * Do we have all access except DELETE_CHILD
+			 * (not caring about the delete bit).
+			 */
+			uint32_t test_mask = ((ace->aceMask|SMB_ACE4_DELETE|SMB_ACE4_DELETE_CHILD) &
+						SMB_ACE4_ALL_MASKS);
+			if (test_mask == SMB_ACE4_ALL_MASKS) {
+				ace->aceMask |= SMB_ACE4_DELETE_CHILD;
+			}
+		}
+
 		win_ace_flags = map_nfs4_ace_flags_to_windows_ace_flags(
 			ace->aceFlags);
 		if (!is_directory &&
-- 
1.8.1.2


From 774d26ca1e80dee05c5e1cedf8177a8076bcff09 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Jun 2013 14:37:55 -0700
Subject: [PATCH 2/4] Add test that confirms setting "nfs4:map full control =
 true" doesn't break raw.acl tests.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/knownfail        | 18 +++++++++++++++++-
 selftest/target/Samba3.pm | 14 ++++++++++++++
 source3/selftest/tests.py |  2 ++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 313d6c9..8432e35 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -61,7 +61,23 @@
 ^samba3.raw.acls nfs4acl_xattr-special.inheritance\(s3dc\)
 ^samba3.raw.acls nfs4acl_xattr-special.inherit_creator_owner\(s3dc\)
 ^samba3.raw.acls nfs4acl_xattr-special.inherit_creator_group\(s3dc\)
-^samba3.base.delete.deltest16a
+^samba3.raw.acls nfs4acl_xattr-simple-map.INHERITFLAGS\(s3dc\) # This (and the follow nfs4acl_xattr tests fail because our NFSv4 backend isn't a complete mapping yet.
+^samba3.raw.acls nfs4acl_xattr-simple-map.sd\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-simple-map.create_file\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-simple-map.create_dir\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-simple-map.nulldacl\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-simple-map.generic\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-simple-map.inheritance\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.INHERITFLAGS\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.sd\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.create_file\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.create_dir\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.nulldacl\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.generic\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.inheritance\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.inherit_creator_owner\(s3dc\)
+^samba3.raw.acls nfs4acl_xattr-special-map.inherit_creator_group\(s3dc\)
+^^samba3.base.delete.deltest16a
 ^samba3.base.delete.deltest17a
 ^samba3.unix.whoami anonymous connection.whoami\(plugin_s4_dc\) # We need to resolve if we should be including SID_NT_WORLD and SID_NT_NETWORK in this token
 ^samba3.unix.whoami anonymous connection.whoami\(s3member\) # smbd maps anonymous logins to domain guest in the local domain, not SID_NT_ANONYMOUS
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 91a8133..baf6089 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1086,12 +1086,26 @@ sub provision($$$$$$)
 	nfs4:mode = simple
 	vfs objects = nfs4acl_xattr xattr_tdb
 
+[nfs4acl_simple_map]
+	path = $shrdir
+	comment = smb username is [%U]
+	nfs4:mode = simple
+	nfs4:map full control = true
+	vfs objects = nfs4acl_xattr xattr_tdb
+
 [nfs4acl_special]
 	path = $shrdir
 	comment = smb username is [%U]
 	nfs4:mode = special
 	vfs objects = nfs4acl_xattr xattr_tdb
 
+[nfs4acl_special_map]
+	path = $shrdir
+	comment = smb username is [%U]
+	nfs4:mode = special
+	nfs4:map full control = true
+	vfs objects = nfs4acl_xattr xattr_tdb
+
 [xcopy_share]
 	path = $shrdir
 	comment = smb username is [%U]
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 3fc6684..56840da 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -347,6 +347,8 @@ for t in tests:
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/nfs4acl_simple -U$USERNAME%$PASSWORD', description='nfs4acl_xattr-simple')
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/nfs4acl_special -U$USERNAME%$PASSWORD', description='nfs4acl_xattr-special')
+        plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/nfs4acl_simple_map -U$USERNAME%$PASSWORD', description='nfs4acl_xattr-simple-map')
+        plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/nfs4acl_special_map -U$USERNAME%$PASSWORD', description='nfs4acl_xattr-special-map')
         plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER_IP/tmpcase -U$USERNAME%$PASSWORD')
     else:
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
-- 
1.8.1.2


From 4d053df5c7817cfb39f9a13ef953888df60fe041 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Jun 2013 14:38:57 -0700
Subject: [PATCH 3/4] Add documentation for "nfs4:map full control" to gpfs acl
 module docs.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 docs-xml/manpages/vfs_gpfs.8.xml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/docs-xml/manpages/vfs_gpfs.8.xml b/docs-xml/manpages/vfs_gpfs.8.xml
index 7f560ca..d3baaac 100644
--- a/docs-xml/manpages/vfs_gpfs.8.xml
+++ b/docs-xml/manpages/vfs_gpfs.8.xml
@@ -364,6 +364,20 @@
 		</varlistentry>
 
 		<varlistentry>
+		<term>nfs4:map full control = [yes|no]</term>
+		<listitem>
+		<para>If set to yes, this parameter will add in the FILE_DELETE_CHILD
+		bit on a returned ACE entry for a file (not a directory) that already
+		contains all file permissions except for FILE_DELETE and FILE_DELETE_CHILD.
+		This can prevent Windows applications that request GENERIC_ALL access
+		from getting ACCESS_DENIED errors when running against a filesystem
+		with NFSv4 compatible ACLs. The default setting of this parameter
+		is "no".
+		</para>
+		</listitem>
+		</varlistentry>
+
+		<varlistentry>
 		<term>gpfs:syncio = [yes|no]</term>
 		<listitem>
 		<para>This parameter makes Samba open all files with O_SYNC.
-- 
1.8.1.2


From 5dec0bb50737bd40f6e1585dc8a39cfd622e50b7 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Jun 2013 14:39:27 -0700
Subject: [PATCH 4/4] Add missing documentation for vfs_zfsacl.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 docs-xml/manpages/vfs_zfs.8.xml | 165 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)
 create mode 100644 docs-xml/manpages/vfs_zfs.8.xml

diff --git a/docs-xml/manpages/vfs_zfs.8.xml b/docs-xml/manpages/vfs_zfs.8.xml
new file mode 100644
index 0000000..7e16716
--- /dev/null
+++ b/docs-xml/manpages/vfs_zfs.8.xml
@@ -0,0 +1,165 @@
+<?xml version="1.0" encoding="iso-8859-1"?>
+<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
+<refentry id="vfs_zfsacl.8">
+
+<refmeta>
+	<refentrytitle>vfs_zfsacl</refentrytitle>
+	<manvolnum>8</manvolnum>
+	<refmiscinfo class="source">Samba</refmiscinfo>
+	<refmiscinfo class="manual">System Administration tools</refmiscinfo>
+	<refmiscinfo class="version">4.0</refmiscinfo>
+</refmeta>
+
+
+<refnamediv>
+	<refname>vfs_zfsacl</refname>
+	<refpurpose>ZFS ACL samba module</refpurpose>
+</refnamediv>
+
+<refsynopsisdiv>
+	<cmdsynopsis>
+		<command>vfs objects = zfsacl</command>
+	</cmdsynopsis>
+</refsynopsisdiv>
+
+<refsect1>
+	<title>DESCRIPTION</title>
+
+	<para>This VFS module is part of the
+	<citerefentry><refentrytitle>samba</refentrytitle>
+	<manvolnum>7</manvolnum></citerefentry> suite.</para>
+
+	<para>The <command>zfsacl</command> VFS module is the home
+	for all ACL extensions that Samba requires for proper integration
+	with ZFS.
+	</para>
+
+	<para>Currently the zfsacl vfs module provides extensions in following areas :
+	<itemizedlist>
+	<listitem><para>NFSv4 ACL Interfaces with configurable options for ZFS</para></listitem>
+	</itemizedlist>
+	</para>
+
+	<para><command>NOTE:</command>This module follows the posix-acl behaviour
+	and hence allows permission stealing via chown. Samba might allow at a later
+	point in time, to restrict the chown via this module as such restrictions
+	are the responsibility of the underlying filesystem than of Samba.
+	</para>
+
+	<para>This module is stackable.</para>
+
+	<para>Since Samba 4.0 all options are per share options.</para>
+
+</refsect1>
+
+
+<refsect1>
+	<title>OPTIONS</title>
+
+	<variablelist>
+
+		<varlistentry>
+
+		<term>nfs4:mode = [ simple | special ]</term>
+		<listitem>
+		<para>
+		Controls substitution of special IDs (OWNER@ and GROUP@) on ZFS.
+                The use of mode simple is recommended.
+                In this mode only non inheriting ACL entries for the file owner
+                and group are mapped to special IDs.
+		</para>
+
+		<para>The following MODEs are understood by the module:</para>
+		<itemizedlist>
+		<listitem><para><command>simple(default)</command> - use OWNER@ and GROUP@ special IDs for non inheriting ACEs only.</para></listitem>
+		<listitem><para><command>special(deprecated)</command> - use OWNER@ and GROUP@ special IDs in ACEs for all file owner and group ACEs.</para></listitem>
+		</itemizedlist>
+		</listitem>
+
+		</varlistentry>
+
+
+		<varlistentry>
+		<term>nfs4:acedup = [dontcare|reject|ignore|merge]</term>
+		<listitem>
+		<para>
+		This parameter configures how Samba handles duplicate ACEs encountered in ZFS ACLs.
+		GPFS allows/creates duplicate ACE for different bits for same ID.
+		</para>
+
+		<para>Following is the behaviour of Samba for different values :</para>
+		<itemizedlist>
+		<listitem><para><command>dontcare (default)</command> - copy the ACEs as they come</para></listitem>
+		<listitem><para><command>reject</command> - stop operation and exit with error on ACL set op</para></listitem>
+		<listitem><para><command>ignore</command> - don't include the second matching ACE</para></listitem>
+		<listitem><para><command>merge</command> - bitwise OR the 2 ace.flag fields and 2 ace.mask fields of the 2 duplicate ACEs into 1 ACE</para></listitem>
+		</itemizedlist>
+		</listitem>
+		</varlistentry>
+
+
+		<varlistentry>
+		<term>nfs4:chown = [yes|no]</term>
+		<listitem>
+		<para>This parameter allows enabling or disabling the chown supported
+		by the underlying filesystem. This parameter should be enabled with
+		care as it might leave your system insecure.</para>
+		<para>Some filesystems allow chown as a) giving b) stealing. It is the latter
+		that is considered a risk.</para>
+
+		<para>Following is the behaviour of Samba for different values : </para>
+		<itemizedlist>
+		<listitem><para><command>yes</command> - Enable chown if as supported by the under filesystem</para></listitem>
+		<listitem><para><command>no (default)</command> - Disable chown</para></listitem>
+		</itemizedlist>
+		</listitem>
+		</varlistentry>
+
+		<varlistentry>
+		<term>nfs4:map full control = [yes|no]</term>
+		<listitem>
+		<para>If set to yes, this parameter will add in the FILE_DELETE_CHILD
+		bit on a returned ACE entry for a file (not a directory) that already
+		contains all file permissions except for FILE_DELETE and FILE_DELETE_CHILD.
+		This can prevent Windows applications that request GENERIC_ALL access
+		from getting ACCESS_DENIED errors when running against a filesystem
+		with NFSv4 compatible ACLs. The default setting of this parameter
+		is "no".
+		</para>
+		</listitem>
+		</varlistentry>
+
+	</variablelist>
+</refsect1>
+
+<refsect1>
+	<title>EXAMPLES</title>
+
+	<para>A GPFS mount can be exported via Samba as follows :</para>
+
+<programlisting>
+        <smbconfsection name="[samba_zfs_share]"/>
+	<smbconfoption name="vfs objects">zfsacl</smbconfoption>
+	<smbconfoption name="path">/test/zfs_mount</smbconfoption>
+	<smbconfoption name="nfs4: mode">special</smbconfoption>
+	<smbconfoption name="nfs4: acedup">merge</smbconfoption>
+	<smbconfoption name="nfs4: map full control">yes</smbconfoption>
+</programlisting>
+</refsect1>
+
+<refsect1>
+	<title>VERSION</title>
+	<para>This man page is correct for version 4.0.x of the Samba suite.
+	</para>
+</refsect1>
+
+<refsect1>
+	<title>AUTHOR</title>
+
+	<para>The original Samba software and related utilities
+	were created by Andrew Tridgell. Samba is now developed
+	by the Samba Team as an Open Source project similar
+	to the way the Linux kernel is developed.</para>
+</refsect1>
+
+</refentry>
-- 
1.8.1.2



More information about the samba-technical mailing list