change in behaviour regarding "open for execute" from 3.6 to 4.0
Michael Adam
obnox at samba.org
Fri Sep 6 17:50:26 CEST 2013
attachment...
On 2013-09-06 at 17:47 +0200, Michael Adam wrote:
> Hi David,
>
> On 2013-09-03 at 17:25 +0200, David Disseldorp wrote:
> > >
> > > Comment / review / push appreciated..
> >
> > The code looks okay, but I'd prefer an option that better represents
> > the changed behaviour, e.g. "acl allow execute always" or similar.
>
> Attached find a patchset for master with the option named as you
> suggested.
>
> Cheers - Michael
>
-------------- next part --------------
From 7bc3f64531431ec1bf48ceacacfaa0eedb4235b0 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at 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 at 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 6a648f4f55ca3c63f4a18fa0da66e2e460c01706 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at 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 at 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 4e27475d3e7ebaa4299f7a1487aa22b443b1ad11 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at 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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130906/f4217951/attachment.pgp>
More information about the samba-technical
mailing list