summaryrefslogtreecommitdiff
path: root/source3
diff options
context:
space:
mode:
authorJeremy Allison <jra@samba.org>2005-10-20 21:10:05 +0000
committerGerald (Jerry) Carter <jerry@samba.org>2007-10-10 11:05:09 -0500
commit0feff5b60454d7b8c935f61ccbd6c43605901013 (patch)
tree471ee2ac013003d45c12980cc44cdb5769c1729f /source3
parent1113cad9c0c81e9ecec3a0f4317c950943cfc62a (diff)
downloadsamba-0feff5b60454d7b8c935f61ccbd6c43605901013.tar.gz
samba-0feff5b60454d7b8c935f61ccbd6c43605901013.tar.bz2
samba-0feff5b60454d7b8c935f61ccbd6c43605901013.zip
r11237: Fix acl evaluation bug found by Marc Cousin <mcousin@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@sigma.fr> for pointing this out. Jeremy. (This used to be commit 7e1318e09bd4b155707020142b08776a546a646e)
Diffstat (limited to 'source3')
-rw-r--r--source3/smbd/posix_acls.c48
1 files changed, 34 insertions, 14 deletions
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index ffb1698394..91a3f5ed48 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -3910,6 +3910,7 @@ static int check_posix_acl_group_write(connection_struct *conn, const char *fnam
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 @@ static int check_posix_acl_group_write(connection_struct *conn, const char *fnam
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 @@ static int check_posix_acl_group_write(connection_struct *conn, const char *fnam
refusing write due to mask.\n", fname));
goto done;
}
- seen_mask = True;
break;
case SMB_ACL_USER:
{
@@ -4019,8 +4020,16 @@ match on user %u -> %s.\n", fname, (unsigned int)*puid, ret ? "can write" : "can
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 @@ match on group %u -> can write.\n", fname, (unsigned int)cu_gid ));
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@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: