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

Jeremy Allison jra at samba.org
Fri Jun 21 11:57:57 MDT 2013


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.

Let me know what you think,

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 e1cf16de8a0e1fb6f40d77689115607952cd7521 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_zfs.8.xml | 160 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 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..e98a177
--- /dev/null
+++ b/docs-xml/manpages/vfs_zfs.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.
+		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>
+
+	</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>
+</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 b99a94feaafe149e60d5b8fc2ba3344cd7ffa87c 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