svn commit: samba r11237 - in branches/SAMBA_3_0/source/smbd: .

[jra at samba.org]

Author: jra
Date: 2005-10-20 21:10:05 +0000 (Thu, 20 Oct 2005)
New Revision: 11237

WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=11237

Log:
Fix acl evaluation bug found by Marc Cousin <mcousin at sigma.fr>
We should only check the S_IWGRP permissions if we haven't already
seen an owning group SMB_ACL_GROUP_OBJ ace entry. If there is an
SMB_ACL_GROUP_OBJ ace entry then the group bits in st_gid are
the same as the SMB_ACL_MASK bits, not the SMB_ACL_GROUP_OBJ
bits. Thanks to Marc Cousin <mcousin at sigma.fr> for pointing
this out.
Jeremy.

Modified:
   branches/SAMBA_3_0/source/smbd/posix_acls.c


Changeset:
Modified: branches/SAMBA_3_0/source/smbd/posix_acls.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/posix_acls.c	2005-10-20 20:40:47 UTC (rev 11236)
+++ branches/SAMBA_3_0/source/smbd/posix_acls.c	2005-10-20 21:10:05 UTC (rev 11237)
@@ -3910,6 +3910,7 @@
 	SMB_ACL_ENTRY_T entry;
 	int i;
 	BOOL seen_mask = False;
+	BOOL seen_owning_group = False;
 	int ret = -1;
 	gid_t cu_gid;
 
@@ -3950,6 +3951,7 @@
 
 		switch(tagtype) {
 			case SMB_ACL_MASK:
+				seen_mask = True;
 				if (!have_write) {
 					/* We don't have any group or explicit user write permission. */
 					ret = -1; /* Allow caller to check "other" permissions. */
@@ -3957,7 +3959,6 @@
 refusing write due to mask.\n", fname));
 					goto done;
 				}
-				seen_mask = True;
 				break;
 			case SMB_ACL_USER:
 			{
@@ -4019,8 +4020,16 @@
 
 		switch(tagtype) {
 			case SMB_ACL_GROUP:
+			case SMB_ACL_GROUP_OBJ:
 			{
-				gid_t *pgid = (gid_t *)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
+				gid_t *pgid = NULL;
+
+				if (tagtype == SMB_ACL_GROUP) {
+					pgid = (gid_t *)SMB_VFS_SYS_ACL_GET_QUALIFIER(conn, entry);
+				} else {
+					seen_owning_group = True;
+					pgid = &psbuf->st_gid;
+				}
 				if (pgid == NULL) {
 					goto check_stat;
 				}
@@ -4059,24 +4068,35 @@
 
   check_stat:
 
-	/* Do we match on the owning group entry ? */
 	/*
-	 * Does it match the current effective group
-	 * or supplementary groups ?
+	 * We only check the S_IWGRP permissions if we haven't already
+	 * seen an owning group SMB_ACL_GROUP_OBJ ace entry. If there is an
+	 * SMB_ACL_GROUP_OBJ ace entry then the group bits in st_gid are
+	 * the same as the SMB_ACL_MASK bits, not the SMB_ACL_GROUP_OBJ
+	 * bits. Thanks to Marc Cousin <mcousin at sigma.fr> for pointing
+	 * this out. JRA.
 	 */
-	for (cu_gid = get_current_user_gid_first(&i); cu_gid != (gid_t)-1;
-					cu_gid = get_current_user_gid_next(&i)) {
-		if (cu_gid == psbuf->st_gid) {
-			ret = (psbuf->st_mode & S_IWGRP) ? 1 : 0;
-			DEBUG(10,("check_posix_acl_group_write: file %s \
+
+	if (!seen_owning_group) {
+		/* Do we match on the owning group entry ? */
+		/*
+		 * Does it match the current effective group
+		 * or supplementary groups ?
+		 */
+		for (cu_gid = get_current_user_gid_first(&i); cu_gid != (gid_t)-1;
+						cu_gid = get_current_user_gid_next(&i)) {
+			if (cu_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"));
-			break;
+				break;
+			}
 		}
-	}
 
-	if (cu_gid == (gid_t)-1) {
-		DEBUG(10,("check_posix_acl_group_write: file %s \
+		if (cu_gid == (gid_t)-1) {
+			DEBUG(10,("check_posix_acl_group_write: file %s \
 failed to match on user or group in token (ret = %d).\n", fname, ret ));
+		}
 	}
 
   done: