[PATCH] Fix open bug found at Microsoft interop event.

[Jeremy Allison]

Fix interop bug found by MS-torture test suite.

ACLs on files can have FILE_DELETE_CHILD set
although it has no meaning for files. Ensure
we mask this out when checking allowed access
against files. The raw.acl torture test shows
that FILE_DELETE_CHILD must still be stored in
file ACLs if set/requested.

The specific test case that found this was an
open request with SEC_RIGHTS_FILE_ALL requested,
against an underlying ACL implementation that
filters out FILE_DELETE_CHILD from returned
ACL entries on files (nfsv4 ACLs).

Tested and fixes the issue at the interop event.

Please review,

Jeremy.
-------------- next part --------------
From 829401cb67f01d1476ee2cdcdcd8585cfa09ae7c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 19 Jun 2013 15:15:08 -0700
Subject: [PATCH 1/2] Clarify smbd_check_access_rights().

Add debug at initial entry. Modify access_mask
to reflect what is being requested instead of
masking off temporarily for one function call.

Note this changes access_mask, whereas the
previous code only masked it out for the function
call, but the bit being masked out does not and
cannot affect any of the operations below it (by
definition).

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/open.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 53f8b8e..3c0e23a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -77,6 +77,10 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
 	uint32_t rejected_share_access;
 	uint32_t rejected_mask = access_mask;
 
+	DEBUG(10,("smbd_check_access_rights: requesting 0x%x on file %s\n",
+		(unsigned int)access_mask,
+		smb_fname_str_dbg(smb_fname)));
+
 	rejected_share_access = access_mask & ~(conn->share_access);
 
 	if (rejected_share_access) {
@@ -139,14 +143,18 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
 	 * containing directory. See the section:
 	 * "Algorithm to Check Access to an Existing File"
 	 * in MS-FSA.pdf.
-	 *
+	 */
+
+	access_mask &= ~FILE_READ_ATTRIBUTES;
+
+	 /*
 	 * se_file_access_check() also takes care of
 	 * owner WRITE_DAC and READ_CONTROL.
 	 */
 	status = se_file_access_check(sd,
 				get_current_nttok(conn),
 				use_privs,
-				(access_mask & ~FILE_READ_ATTRIBUTES),
+				access_mask,
 				&rejected_mask);
 
 	DEBUG(10,("smbd_check_access_rights: file %s requesting "
-- 
1.8.1.2


From 0eb4e4f5220c8892b0e9113a726ca6e76fc3874d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 19 Jun 2013 15:21:48 -0700
Subject: [PATCH 2/2] Fix interop bug found by MS-torture test suite.

ACLs on files can have FILE_DELETE_CHILD set
although it has no meaning for files. Ensure
we mask this out when checking allowed access
against files. The raw.acl torture test shows
that FILE_DELETE_CHILD must still be stored in
file ACLs if set/requested.

The specific test case that found this was an
open request with SEC_RIGHTS_FILE_ALL requested,
against an underlying ACL implementation that
filters out FILE_DELETE_CHILD from returned
ACL entries on files (nfsv4 ACLs).

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/open.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 3c0e23a..15438b8 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -147,6 +147,18 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
 
 	access_mask &= ~FILE_READ_ATTRIBUTES;
 
+	/*
+	 * For non-directories, the bit FILE_DELETE_CHILD has no
+	 * meaning even though it is stored and returned when
+	 * a file is opened with SEC_RIGHTS_FILE_ALL (see
+	 * raw.acls torture tests), so remove it for
+	 * non-directories.
+	 */
+
+	if (VALID_STAT(smb_fname->st) && !S_ISDIR(smb_fname->st.st_ex_mode)) {
+		access_mask &= ~FILE_DELETE_CHILD;
+	}
+
 	 /*
 	 * se_file_access_check() also takes care of
 	 * owner WRITE_DAC and READ_CONTROL.
-- 
1.8.1.2