[Samba] ACLs under Samba 3.3.0

Jeremy Allison jra at samba.org
Tue Feb 3 23:39:00 GMT 2009


On Fri, Jan 30, 2009 at 07:24:34PM +0000, Miguel Medalha wrote:
> Is behavior of ACLs under Samba 3.3.0 (Sernet) completely different from  
> that under version 3.2.7? The release notes only talks about some 
> "fixes".
>
> I installed version 3.3.0 and got completely different result with the  
> same filesystem and the exact same samba configuration. The ACLs behaved  
> strangely and appeared very different under Windows ACL editor. Users  
> were  now unable to delete the exact same files they had just created in  
> a folder.
>
> When seen under the Windows ACL editor, the "Delete" permission was  
> unselected. All efforts to activate it failed. Even resetting the  
> permissions on the command line with setfacl did not have any effect. I  
> then reverted to 3.2.7-38 and all was right again, without any  
> modification whatsoever.
>
> Is this a bug or is it by design? If it is by design, then the release  
> notes really should have warned against such a *huge* difference in  
> behavior...
>
> I observed this under Samba Sernet 3.3.0+CentOS 5.2.

Here is the patch I've committed to the 3.3 code tree
for this problem. It will be in the next release. Please
try it out and let me know if it fixes your problem (it
does here).

Jeremy
-------------- next part --------------
diff --git a/source/include/smb.h b/source/include/smb.h
index e7f08a3..a98d151 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -1251,7 +1251,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/source/lib/util_seaccess.c b/source/lib/util_seaccess.c
index fdc10f2..0da7442 100644
--- a/source/lib/util_seaccess.c
+++ b/source/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/source/smbd/file_access.c b/source/smbd/file_access.c
index c535bc7..743e079 100644
--- a/source/smbd/file_access.c
+++ b/source/smbd/file_access.c
@@ -116,16 +116,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/source/smbd/open.c b/source/smbd/open.c
index 3c07dba..7e127ea 100644
--- a/source/smbd/open.c
+++ b/source/smbd/open.c
@@ -50,13 +50,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 |
@@ -73,9 +75,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;
 }
 
@@ -398,14 +408,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 ));
 			}
 		}
 	}
@@ -2398,9 +2429,11 @@ 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",
@@ -2819,8 +2852,11 @@ 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;
 	}
 


More information about the samba mailing list