Fix bug #6082 - smbd_gpfs_getacl failed: Windows client can´t rename or delete file

This fixes the generic rename/delete problem for 3.3.0 and above.
Fixed slightly differently to discussions, user viewable modified
ACLs are not a good idea :-).
Jeremy.

4 files changed, 54 insertions, 20 deletions

diff --git a/source3/include/smb.h b/source3/include/smb.h
index 491dd763ff..b441b3476a 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -1259,7 +1259,7 @@ struct bitmap {
 /* Mapping of access rights to UNIX perms. for a UNIX directory. */
 #define UNIX_DIRECTORY_ACCESS_RWX		FILE_GENERIC_ALL
 #define UNIX_DIRECTORY_ACCESS_R 		FILE_GENERIC_READ
-#define UNIX_DIRECTORY_ACCESS_W			FILE_GENERIC_WRITE
+#define UNIX_DIRECTORY_ACCESS_W			(FILE_GENERIC_WRITE|FILE_DELETE_CHILD)
 #define UNIX_DIRECTORY_ACCESS_X			FILE_GENERIC_EXECUTE
 
 #if 0
diff --git a/source3/lib/util_seaccess.c b/source3/lib/util_seaccess.c
index fdc10f20ab..0da7442d19 100644
--- a/source3/lib/util_seaccess.c
+++ b/source3/lib/util_seaccess.c
@@ -149,7 +149,9 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
 }
 
 /*
-  the main entry point for access checking. 
+  The main entry point for access checking. If returning ACCESS_DENIED
+  this function returns the denied bits in the uint32_t pointed
+  to by the access_granted pointer.
 */
 NTSTATUS se_access_check(const struct security_descriptor *sd, 
 			  const NT_USER_TOKEN *token,
@@ -238,6 +240,7 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
 
 done:
 	if (bits_remaining != 0) {
+		*access_granted = bits_remaining;
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c
index d44e63a89a..fe7ba1cc46 100644
--- a/source3/smbd/file_access.c
+++ b/source3/smbd/file_access.c
@@ -113,16 +113,11 @@ bool can_delete_file_in_directory(connection_struct *conn, const char *fname)
 	 * having the DELETE bit on the file itself and second if that does
 	 * not help, by the DELETE_CHILD bit on the containing directory.
 	 *
-	 * Here we check the other way round because with just posix
-	 * permissions looking at the file itself will never grant DELETE, so
-	 * by looking at the directory first we save one get_acl call.
+	 * Here we only check the directory permissions, we will
+	 * check the file DELETE permission separately.
 	 */
 
-	if (can_access_file_acl(conn, dname, FILE_DELETE_CHILD)) {
-		return true;
-	}
-
-	return can_access_file_acl(conn, fname, DELETE_ACCESS);
+	return can_access_file_acl(conn, dname, FILE_DELETE_CHILD);
 }
 
 /****************************************************************************
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 7d23b92359..bc5107447f 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -67,13 +67,15 @@ NTSTATUS smb1_file_se_access_check(const struct security_descriptor *sd,
 
 static NTSTATUS check_open_rights(struct connection_struct *conn,
 				const char *fname,
-				uint32_t access_mask)
+				uint32_t access_mask,
+				uint32_t *access_granted)
 {
 	/* Check if we have rights to open. */
 	NTSTATUS status;
-	uint32_t access_granted = 0;
 	struct security_descriptor *sd;
 
+	*access_granted = 0;
+
 	status = SMB_VFS_GET_NT_ACL(conn, fname,
 			(OWNER_SECURITY_INFORMATION |
 			GROUP_SECURITY_INFORMATION |
@@ -90,9 +92,17 @@ static NTSTATUS check_open_rights(struct connection_struct *conn,
 	status = smb1_file_se_access_check(sd,
 				conn->server_info->ptok,
 				access_mask,
-				&access_granted);
+				access_granted);
 
 	TALLOC_FREE(sd);
+
+	DEBUG(10,("check_open_rights: file %s requesting "
+		"0x%x returning 0x%x (%s)\n",
+		fname,
+		(unsigned int)access_mask,
+		(unsigned int)*access_granted,
+		nt_errstr(status) ));
+
 	return status;
 }
 
@@ -415,14 +425,35 @@ static NTSTATUS open_file(files_struct *fsp,
 	} else {
 		fsp->fh->fd = -1; /* What we used to call a stat open. */
 		if (file_existed) {
+			uint32_t access_granted = 0;
+
 			status = check_open_rights(conn,
 					path,
-					access_mask);
+					access_mask,
+					&access_granted);
 			if (!NT_STATUS_IS_OK(status)) {
-				DEBUG(10, ("open_file: Access denied on "
-					"file %s\n",
-					path));
-				return status;
+
+				/* Were we trying to do a stat open
+				 * for delete and didn't get DELETE
+				 * access (only) ? Check if the
+				 * directory allows DELETE_CHILD.
+				 * See here:
+				 * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx
+				 * for details. */
+
+				if (!(NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) &&
+						(access_mask & DELETE_ACCESS) &&
+	    					(access_granted == DELETE_ACCESS) &&
+						can_delete_file_in_directory(conn, path))) {
+					DEBUG(10, ("open_file: Access denied on "
+						"file %s\n",
+						path));
+					return status;
+				}
+
+				DEBUG(10,("open_file: overrode ACCESS_DENIED "
+					"on file %s\n",
+					path ));
 			}
 		}
 	}
@@ -2395,9 +2426,11 @@ static NTSTATUS open_directory(connection_struct *conn,
 	}
 
 	if (info == FILE_WAS_OPENED) {
+		uint32_t access_granted = 0;
 		status = check_open_rights(conn,
 					fname,
-					access_mask);
+					access_mask,
+					&access_granted);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("open_directory: check_open_rights on "
 				"file %s failed with %s\n",
@@ -2826,8 +2859,11 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
 	    && (create_disposition != FILE_CREATE)
 	    && (share_access & FILE_SHARE_DELETE)
 	    && (access_mask & DELETE_ACCESS)
-	    && (!can_delete_file_in_directory(conn, fname))) {
+	    && (!(can_delete_file_in_directory(conn, fname) ||
+		 can_access_file_acl(conn, fname, DELETE_ACCESS)))) {
 		status = NT_STATUS_ACCESS_DENIED;
+		DEBUG(10,("create_file_unixpath: open file %s "
+			"for delete ACCESS_DENIED\n", fname ));
 		goto fail;
 	}