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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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:
