Attachment 9202 Details for Bug 10134 – patchset for master

[patch] patchset for master

patchset-bug-10134.master.mbox (text/plain), 6.81 KB, created by Michael Adam on 2013-09-10 21:42:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Adam

Created: 2013-09-10 21:42:33 UTC

Size: 6.81 KB

patch

obsolete

>From de3bc10ef69f23e7dab9fc3f6990bb403824b14e Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 2 Sep 2013 17:36:59 +0200
>Subject: [PATCH 1/3] loadparm: add new parameter "acl allow execute always"
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>---
> lib/param/param_functions.c |    1 +
> lib/param/param_table.c     |   10 ++++++++++
> source3/include/proto.h     |    1 +
> source3/param/loadparm.c    |    1 +
> 4 files changed, 13 insertions(+)
>
>diff --git a/lib/param/param_functions.c b/lib/param/param_functions.c
>index fed2e95..61f0044 100644
>--- a/lib/param/param_functions.c
>+++ b/lib/param/param_functions.c
>@@ -132,6 +132,7 @@ FN_LOCAL_BOOL(afs_share, bAfs_Share)
> FN_LOCAL_BOOL(acl_check_permissions, bAclCheckPermissions)
> FN_LOCAL_BOOL(acl_group_control, bAclGroupControl)
> FN_LOCAL_BOOL(acl_map_full_control, bAclMapFullControl)
>+FN_LOCAL_BOOL(acl_allow_execute_always, bAclAllowExecuteAlways)
> FN_LOCAL_INTEGER(defaultcase, iDefaultCase)
> FN_LOCAL_INTEGER(minprintspace, iMinPrintSpace)
> FN_LOCAL_INTEGER(printing, iPrinting)
>diff --git a/lib/param/param_table.c b/lib/param/param_table.c
>index 1b1497c..7b32998 100644
>--- a/lib/param/param_table.c
>+++ b/lib/param/param_table.c
>@@ -904,6 +904,16 @@ static struct parm_struct parm_table[] = {
> 		.flags		= FLAG_ADVANCED | FLAG_GLOBAL | FLAG_SHARE,
> 	},
> 	{
>+		.label		= "acl allow execute always",
>+		.type		= P_BOOL,
>+		.p_class	= P_LOCAL,
>+		.offset		= LOCAL_VAR(bAclAllowExecuteAlways),
>+		.special	= NULL,
>+		.enum_list	= NULL,
>+		.flags		= FLAG_ADVANCED | FLAG_GLOBAL | FLAG_SHARE,
>+	},
>+
>+	{
> 		.label		= "create mask",
> 		.type		= P_OCTAL,
> 		.p_class	= P_LOCAL,
>diff --git a/source3/include/proto.h b/source3/include/proto.h
>index df65711..804575a 100644
>--- a/source3/include/proto.h
>+++ b/source3/include/proto.h
>@@ -1330,6 +1330,7 @@ bool lp_afs_share(int );
> bool lp_acl_check_permissions(int );
> bool lp_acl_group_control(int );
> bool lp_acl_map_full_control(int );
>+bool lp_acl_allow_execute_always(int);
> bool lp_durable_handles(int);
> int lp_create_mask(int );
> int lp_force_create_mode(int );
>diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
>index 229ebd8..b9945ac 100644
>--- a/source3/param/loadparm.c
>+++ b/source3/param/loadparm.c
>@@ -264,6 +264,7 @@ static struct loadparm_service sDefault =
> 	.bAclCheckPermissions = true,
> 	.bAclMapFullControl = true,
> 	.bAclGroupControl = false,
>+	.bAclAllowExecuteAlways = false,
> 	.bChangeNotify = true,
> 	.bKernelChangeNotify = true,
> 	.iallocation_roundup_size = SMB_ROUNDUP_ALLOCATION_SIZE,
>-- 
>1.7.9.5
>
>
>From 1e29d730663382875d96c275c60e022a1c33a2d1 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 2 Sep 2013 17:37:50 +0200
>Subject: [PATCH 2/3] s3:smbd: ease file server upgrades from 3.6 and earlier
> with "acl allow execute aways"
>
>3.6 and earlier allowed open for execution when execute permissions are
>not present on a file. This has been fixed in Samba 4.0.
>
>This patch changes smbd to skip the execute bit from the ACL check
>in the open code if "acl allow execute always = yes", hence
>re-establishing the old behaviour in this case.
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>---
> source3/smbd/open.c |   16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index e5ea715..b9618b4 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -76,6 +76,7 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
> 	struct security_descriptor *sd = NULL;
> 	uint32_t rejected_share_access;
> 	uint32_t rejected_mask = access_mask;
>+	uint32_t do_not_check_mask = 0;
> 
> 	rejected_share_access = access_mask & ~(conn->share_access);
> 
>@@ -143,10 +144,23 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
> 	 * se_file_access_check() also takes care of
> 	 * owner WRITE_DAC and READ_CONTROL.
> 	 */
>+	do_not_check_mask = FILE_READ_ATTRIBUTES;
>+
>+	/*
>+	 * Samba 3.6 and earlier granted execute access even
>+	 * if the ACL did not contain execute rights.
>+	 * Samba 4.0 is more correct and checks it.
>+	 * The compatibilty mode allows to skip this check
>+	 * to smoothen upgrades.
>+	 */
>+	if (lp_acl_allow_execute_always(SNUM(conn))) {
>+		do_not_check_mask |= FILE_EXECUTE;
>+	}
>+
> 	status = se_file_access_check(sd,
> 				get_current_nttok(conn),
> 				use_privs,
>-				(access_mask & ~FILE_READ_ATTRIBUTES),
>+				(access_mask & ~do_not_check_mask),
> 				&rejected_mask);
> 
> 	DEBUG(10,("smbd_check_access_rights: file %s requesting "
>-- 
>1.7.9.5
>
>
>From 8f0a79cff7b2dcd50a8c66f8fbe88c5010996ac7 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>Date: Mon, 2 Sep 2013 16:54:15 +0200
>Subject: [PATCH 3/3] docs: document "acl allow execute always"
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>---
> .../smbdotconf/protocol/aclallowexecutealways.xml  |   26 ++++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 docs-xml/smbdotconf/protocol/aclallowexecutealways.xml
>
>diff --git a/docs-xml/smbdotconf/protocol/aclallowexecutealways.xml b/docs-xml/smbdotconf/protocol/aclallowexecutealways.xml
>new file mode 100644
>index 0000000..048c388
>--- /dev/null
>+++ b/docs-xml/smbdotconf/protocol/aclallowexecutealways.xml
>@@ -0,0 +1,26 @@
>+<samba:parameter name="acl allow execute always"
>+                 context="S"
>+                 type="boolean"
>+                 advanced="1" wizard="1"
>+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
>+<description>
>+    <para>
>+    This boolean parameter controls the behaviour of <citerefentry><refentrytitle>smbd</refentrytitle>
>+    <manvolnum>8</manvolnum></citerefentry> when receiving a protocol request of "open for execution"
>+    from a Windows client.
>+    With Samba 3.6 and older, the execution right in the ACL was not checked, so a client
>+    could execute a file even if it did not have execute rights on the file. In Samba 4.0,
>+    this has been fixed, so that by default, i.e. when this parameter is set to "False",
>+    open for execution is now denied when execution permissions are not present.
>+    </para>
>+    <para>
>+    If this parameter is set to "True", Samba does not check execute permissions on
>+    "open for execution, thus re-establishing the behaviour of Samba 3.6.
>+    This can be useful to smoothen upgrades from older Samba versions to 4.0 and newer.
>+    This setting is not not meant to be used as a permanent setting, but as a temporary relief:
>+    It is recommended to fix the permissions in the ACLs and reset this parameter to the
>+    default after a ceratain transition period.
>+    </para>
>+</description>
>+<value type="default">False</value>
>+</samba:parameter>
>-- 
>1.7.9.5
>

Flags:

ddiss: review+
vl: review+

Actions: View

Attachments on bug 10134: 9202 | 9203 | 9206