From 974123cddb8c1dbb51cf53f290e71c5adaa01cca Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 1 Sep 2015 22:23:59 +0200 Subject: [PATCH] Fix up POSIX permission handling * fhandler_disk_file.cc (fhandler_disk_file::fchmod): Disable deviation from POSIX 1003.1e in terms of GROUP_OBJ/CLASS_OBJ permissions. Follow POSIX 1003.1e again. Keep old code in for future reference. * sec_acl.cc: Accommodate changes in ACE creation in leading comment. (set_posix_access): Fix user deny ACE creation. Split group deny ACE creation into two steps, one to reflect CLASS_OBJ, the other to reflect OTHER_OBJ. --- winsup/cygwin/fhandler_disk_file.cc | 10 ++++++ winsup/cygwin/sec_acl.cc | 54 ++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index 204ba6020..48858854f 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -915,6 +915,7 @@ fhandler_disk_file::fchmod (mode_t mode) /* Overwrite ACL permissions as required by POSIX 1003.1e draft 17. */ aclp[0].a_perm = (mode >> 6) & S_IRWXO; +#if 0 /* Deliberate deviation from POSIX 1003.1e here. We're not writing CLASS_OBJ *or* GROUP_OBJ, but both. Otherwise we're going to be in constant trouble with user expectations. */ @@ -923,6 +924,15 @@ fhandler_disk_file::fchmod (mode_t mode) if (nentries > MIN_ACL_ENTRIES && (idx = searchace (aclp, nentries, CLASS_OBJ)) >= 0) aclp[idx].a_perm = (mode >> 3) & S_IRWXO; +#else + /* POSIXly correct: If CLASS_OBJ is present, chmod only modifies + CLASS_OBJ, not GROUP_OBJ. */ + if (nentries > MIN_ACL_ENTRIES + && (idx = searchace (aclp, nentries, CLASS_OBJ)) >= 0) + aclp[idx].a_perm = (mode >> 3) & S_IRWXO; + else if ((idx = searchace (aclp, nentries, GROUP_OBJ)) >= 0) + aclp[idx].a_perm = (mode >> 3) & S_IRWXO; +#endif if ((idx = searchace (aclp, nentries, OTHER_OBJ)) >= 0) aclp[idx].a_perm = mode & S_IRWXO; if (pc.isdir ()) diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc index 82e9cb53f..a84cd5e71 100644 --- a/winsup/cygwin/sec_acl.cc +++ b/winsup/cygwin/sec_acl.cc @@ -37,25 +37,30 @@ details. */ or USER_OBJ deny ACE == ~USER_OBJ & (GROUP_OBJ | OTHER_OBJ) - - USER deny. If a user has different permissions from CLASS_OBJ, or if the + - USER deny. If a user has more permissions than CLASS_OBJ, or if the user has less permissions than OTHER_OBJ, deny the excess permissions. - USER deny ACE == (USER ^ CLASS_OBJ) | (~USER & OTHER_OBJ) + USER deny ACE == (USER & ~CLASS_OBJ) | (~USER & OTHER_OBJ) - USER_OBJ allow ACE - USER allow ACEs - The POSIX permissions returned for a USER entry are the allow bits alone! + The POSIX permissions returned for a USER entry are the allow bits alone! - GROUP{_OBJ} deny. If a group has more permissions than CLASS_OBJ, or less permissions than OTHER_OBJ, deny the excess permissions. - GROUP{_OBJ} deny ACEs == (GROUP & ~CLASS_OBJ) | (~GROUP & OTHER_OBJ) + GROUP{_OBJ} deny ACEs == (GROUP & ~CLASS_OBJ) - GROUP_OBJ allow ACE - GROUP allow ACEs - The POSIX permissions returned for a GROUP entry are the allow bits alone! + The POSIX permissions returned for a GROUP entry are the allow bits alone! + + - 2. GROUP{_OBJ} deny. If a group has less permissions than OTHER_OBJ, + deny the excess permissions. + + 2. GROUP{_OBJ} deny ACEs == (~GROUP & OTHER_OBJ) - OTHER_OBJ allow ACE @@ -311,7 +316,9 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, check_types < CLASS_OBJ; check_types <<= 2) { - /* Create deny ACEs for users, then groups. */ + /* Create deny ACEs for users, then 1st run for groups. For groups, + only take CLASS_OBJ permissions into account. Class permissions + are handled in the 2nd deny loop below. */ for (start_idx = idx; idx < nentries && aclbufp[idx].a_type & check_types; ++idx) @@ -327,7 +334,7 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, if (aclbufp[idx].a_type & USER_OBJ) deny = ~aclbufp[idx].a_perm & (class_obj | other_obj); else if (aclbufp[idx].a_type & USER) - deny = (aclbufp[idx].a_perm ^ class_obj) + deny = (aclbufp[idx].a_perm & ~class_obj) | (~aclbufp[idx].a_perm & other_obj); /* Accommodate Windows: Only generate deny masks for SYSTEM and the Administrators group in terms of the execute bit, @@ -337,8 +344,7 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, || aclsid[idx] == well_known_admins_sid)) deny = aclbufp[idx].a_perm & ~(class_obj | S_IROTH | S_IWOTH); else - deny = (aclbufp[idx].a_perm & ~class_obj) - | (~aclbufp[idx].a_perm & other_obj); + deny = (aclbufp[idx].a_perm & ~class_obj); if (!deny) continue; access = 0; @@ -391,6 +397,36 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, inherit)) return NULL; } + /* 2nd deny loop: Create deny ACEs for groups when they have less + permissions than OTHER_OBJ. */ + if (check_types == (GROUP_OBJ | GROUP)) + for (idx = start_idx; + idx < nentries && aclbufp[idx].a_type & check_types; + ++idx) + { + if (aclbufp[idx].a_type & GROUP && aclsid[idx] == group) + continue; + /* Only generate deny masks for SYSTEM and the Administrators + group if they are the primary group. */ + if (aclbufp[idx].a_type & GROUP + && (aclsid[idx] == well_known_system_sid + || aclsid[idx] == well_known_admins_sid)) + deny = 0; + else + deny = (~aclbufp[idx].a_perm & other_obj); + if (!deny) + continue; + access = 0; + if (deny & S_IROTH) + access |= FILE_DENY_READ; + if (deny & S_IWOTH) + access |= FILE_DENY_WRITE; + if (deny & S_IXOTH) + access |= FILE_DENY_EXEC; + if (!add_access_denied_ace (acl, access, aclsid[idx], acl_len, + inherit)) + return NULL; + } } /* For ptys if the admins group isn't in the ACL, add an ACE to make sure the group has WRITE_DAC and WRITE_OWNER perms. */ -- 2.43.5