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

Jeremy Allison jra at samba.org
Fri Jun 21 12:20:54 MDT 2013


On Fri, Jun 21, 2013 at 10:57:57AM -0700, Jeremy Allison wrote:
> On Fri, Jun 21, 2013 at 09:46:20AM -0700, Jeremy Allison wrote:
> > 
> > I'll resubmit the modified version shortly.
> 
> Here is the modified version. It's the same code logic,
> it just re-uses the existing "acl map full control" parameter
> that is already set to "true" and used in the posix ACL
> map code, rather than adding a new parameter.
> 
> As this is now on by default, that means it's tested
> in the default selftests in the raw.acl tests on top
> of vfs_zfsacl, so the extra test suite changes aren't
> needed.
> 
> I still added the module docs for vfs_zfsacl and made
> a note of the behaviour change in the vfs_gpfs module
> documentation. This makes it a much shorter patchset.

Version with typos noticed by Ira (and man page
incorrect file name) fixed.

Think this one should do the trick :-).

Cheers,

	Jeremy.
-------------- next part --------------
From 85109218de3ae51b82ff1f0a9a08d1ab7c07ff44 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/3] Use existing "acl 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.

This is using the same parameter in the same way as it is currently used
in smbd/posix_acls.c. Note that as this parameter is on by default, it
is already being tested in the existing raw.acl tests.

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 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
index 13e9268..255741c 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,13 @@ 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_acl_map_full_control(SNUM(conn));
 
-	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 +386,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 ab441beb768c5b69b25a0858c97d6fca0d97b518 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 2/3] Add missing documentation for vfs_zfsacl.

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

diff --git a/docs-xml/manpages/vfs_zfsacl.8.xml b/docs-xml/manpages/vfs_zfsacl.8.xml
new file mode 100644
index 0000000..f56af1b
--- /dev/null
+++ b/docs-xml/manpages/vfs_zfsacl.8.xml
@@ -0,0 +1,160 @@
+<?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 makes use of the smb.conf parameter
+	<smbconfoption name="acl map full control">acl map full control</smbconfoption>
+	When set to yes (the default), 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.
+	</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.
+		ZFS 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>
+
+	</variablelist>
+</refsect1>
+
+<refsect1>
+	<title>EXAMPLES</title>
+
+	<para>A ZFS 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>
+</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


From 11aa7ba83ccd5dc681b917f50991afd926b4fa51 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Jun 2013 10:36:23 -0700
Subject: [PATCH 3/3] Note how vfs_gpfs uses the "acl map full control"
 parameter.

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

diff --git a/docs-xml/manpages/vfs_gpfs.8.xml b/docs-xml/manpages/vfs_gpfs.8.xml
index 7f560ca..d1243a9 100644
--- a/docs-xml/manpages/vfs_gpfs.8.xml
+++ b/docs-xml/manpages/vfs_gpfs.8.xml
@@ -48,6 +48,16 @@
 	are the responsibility of the underlying filesystem than of Samba.
 	</para>
 
+	<para>This module makes use of the smb.conf parameter
+	<smbconfoption name="acl map full control">acl map full control</smbconfoption>
+	When set to yes (the default), 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.
+	</para>
+
 	<para>This module is stackable.</para>
 
 	<para>Since Samba 4.0 all options are per share options.</para>
-- 
1.8.1.2



More information about the samba-technical mailing list