[Samba] still ACL bug in 3.0.14a

[Jeremy Allison]

On Mon, Apr 18, 2005 at 12:41:06PM -0500, Tom Schaefer wrote:
> 
> I have yet to get it work properly on Solaris.  I'm in the process of
> putting together a level 10 debug log and anything else I think
> might be useful for him and sending it off to Jeremy.  Jeremy had
> suggested that the patch he wrote for Eric might fix my trouble on Solaris
> as well.  It did not.  I noticed there is a new message in this thread
> posted from Yannick Bergeron stating he applied the patch and rebuilt
> 3.0.14a from scratch this morning and the problem persists for him as
> well.

Found the problem in Solaris from the log files. The line :

have_write = SMB_VFS_SYS_ACL_GET_PERM(conn, permset, SMB_ACL_WRITE);

returns *2*, not 1 on Solaris if the permission is
granted. Here is the complete patch for 3.0.14a.

Let me know if this fixes it please.

Jeremy.
-------------- next part --------------
Index: smbd/posix_acls.c
===================================================================
--- smbd/posix_acls.c	(revision 6363)
+++ smbd/posix_acls.c	(working copy)
@@ -3756,6 +3756,7 @@
 
 /****************************************************************************
  Check for POSIX group ACLs. If none use stat entry.
+ Return -1 if no match, 0 if match and denied, 1 if match and allowed.
 ****************************************************************************/
 
 static int check_posix_acl_group_write(connection_struct *conn, const char *fname, SMB_STRUCT_STAT *psbuf)
@@ -3797,6 +3798,12 @@
 			goto check_stat;
 		}
 
+		/*
+		 * Solaris returns 2 for this if write is available.
+		 * canonicalize to 0 or 1.
+		 */	
+		have_write = (have_write ? 1 : 0);
+
 		switch(tagtype) {
 			case SMB_ACL_MASK:
 				if (!have_write) {
@@ -3860,6 +3867,12 @@
 			goto check_stat;
 		}
 
+		/*
+		 * Solaris returns 2 for this if write is available.
+		 * canonicalize to 0 or 1.
+		 */	
+		have_write = (have_write ? 1 : 0);
+
 		switch(tagtype) {
 			case SMB_ACL_GROUP:
 			{
@@ -3867,6 +3880,23 @@
 				if (pgid == NULL) {
 					goto check_stat;
 				}
+
+				/* Does it match the current effective group ? */
+				if (current_user.gid == *pgid) {
+					ret = have_write;
+					DEBUG(10,("check_posix_acl_group_write: file %s \
+match on group %u -> can write.\n", fname, (unsigned int)*pgid ));
+
+					/* If we don't have write permission this entry doesn't
+					 * prevent the subsequent enumeration of the supplementary
+					 * groups.
+					 */
+					if (have_write) {
+						goto done;
+					}
+				}
+
+				/* Continue with the supplementary groups. */
 				for (i = 0; i < current_user.ngroups; i++) {
 					if (current_user.groups[i] == *pgid) {
 						ret = have_write;
@@ -3898,6 +3928,15 @@
 
 	/* Do we match on the owning group entry ? */
 
+	/* First, does it match the current effective group ? */
+	if (current_user.gid == psbuf->st_gid) {
+		ret = (psbuf->st_mode & S_IWGRP) ? 1 : 0;
+		DEBUG(10,("check_posix_acl_group_write: file %s \
+match on owning group %u -> %s.\n", fname, (unsigned int)psbuf->st_gid, ret ? "can write" : "cannot write"));
+		goto done;
+	}
+
+	/* If not look at the supplementary groups. */
 	for (i = 0; i < current_user.ngroups; i++) {
 		if (current_user.groups[i] == psbuf->st_gid) {
 			ret = (psbuf->st_mode & S_IWGRP) ? 1 : 0;