From 269d2c61bb0e2d50b30a6c382295680c2680904c Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Wed, 2 Sep 2015 00:05:46 +0200 Subject: [PATCH] Fix permission evaluation for !new_style ACLs * security.h (authz_get_user_attribute): Declare bool. * sec_helper.cc (authz_ctx::get_user_attribute): Make bool method. Set S_IxOTH bits in returned attributes rather than S_IxUSR bits. (authz_get_user_attribute): Make bool function. * sec_acl.cc (get_posix_access): Introduce cygsid array to keep track of all SIDs in the ACL. Move AuthZ calls into !new_style permission post processing. When not using AuthZ, use CheckTokenMembership to collect group permissions. --- winsup/cygwin/sec_acl.cc | 91 +++++++++++++++++++------------------ winsup/cygwin/sec_helper.cc | 22 +++++---- winsup/cygwin/security.h | 2 +- 3 files changed, 61 insertions(+), 54 deletions(-) diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc index a84cd5e71..ec6aa7f20 100644 --- a/winsup/cygwin/sec_acl.cc +++ b/winsup/cygwin/sec_acl.cc @@ -580,7 +580,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, gid_t gid; mode_t attr = 0; aclent_t *lacl = NULL; - cygpsid ace_sid; + cygpsid ace_sid, *aclsid; int pos, type, id, idx; bool owner_eq_group; @@ -669,6 +669,11 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[1].a_id = gid; lacl[2].a_type = OTHER_OBJ; lacl[2].a_id = ILLEGAL_GID; + /* Create array to collect SIDs of all entries in lacl. */ + aclsid = (cygpsid *) tp.w_get (); + aclsid[0] = owner_sid; + aclsid[1] = group_sid; + aclsid[2] = well_known_world_sid; /* No ACEs? Everybody has full access. */ if (!acl_exists || !acl || acl->AceCount == 0) @@ -678,15 +683,6 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, goto out; } - /* If we use the Windows user DB, use Authz to make sure the owner - permissions are correctly reflecting the Windows permissions. */ - if (cygheap->pg.nss_pwd_db ()) - { - mode_t attr = 0; - authz_get_user_attribute (&attr, psd, owner_sid); - lacl[0].a_perm = attr >> 6; - } - /* Files and dirs are created with a NULL descriptor, so inheritence rules kick in. If no inheritable entries exist in the parent object, Windows will create entries according to the user token's default DACL. @@ -730,6 +726,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_type = CLASS_OBJ; lacl[pos].a_id = ILLEGAL_GID; lacl[pos].a_perm = CYG_ACE_MASK_TO_POSIX (ace->Mask); + aclsid[pos] = well_known_null_sid; } has_class_perm = true; class_perm = lacl[pos].a_perm; @@ -742,6 +739,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_type = DEF_CLASS_OBJ; lacl[pos].a_id = ILLEGAL_GID; lacl[pos].a_perm = CYG_ACE_MASK_TO_POSIX (ace->Mask); + aclsid[pos] = well_known_null_sid; } has_def_class_perm = true; def_class_perm = lacl[pos].a_perm; @@ -832,21 +830,9 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, } if ((pos = searchace (lacl, MAX_ACL_ENTRIES, type, id)) >= 0) { - /* If we use the Windows user DB, use Authz to check for user - permissions. */ - if (cygheap->pg.nss_pwd_db () && (type & (USER_OBJ | USER))) - { - /* We already handle the USER_OBJ entry above. */ - if (type == USER) - { - mode_t attr = 0; - authz_get_user_attribute (&attr, psd, ace_sid); - lacl[pos].a_perm = attr >> 6; - } - } - else - getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType, - new_style && type & (USER | GROUP_OBJ | GROUP)); + getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType, + new_style && type & (USER | GROUP_OBJ | GROUP)); + aclsid[pos] = ace_sid; if (!new_style) { /* Fix up CLASS_OBJ value. */ @@ -909,6 +895,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, { getace (lacl[pos], type, id, ace->Mask, ace->Header.AceType, new_style && type & (USER | GROUP_OBJ | GROUP)); + aclsid[pos] = ace_sid; if (!new_style) { /* Fix up DEF_CLASS_OBJ value. */ @@ -940,6 +927,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_id = ILLEGAL_GID; class_perm |= lacl[1].a_perm; lacl[pos].a_perm = class_perm; + aclsid[pos] = well_known_null_sid; } /* For ptys, fake a mask if the admins group is neither owner nor group. In that case we have an extra ACE for the admins group, and we need a @@ -954,6 +942,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_type = CLASS_OBJ; lacl[pos].a_id = ILLEGAL_GID; lacl[pos].a_perm = lacl[1].a_perm; /* == group perms */ + aclsid[pos] = well_known_null_sid; } /* If this is a just created file, and this is an ACL with only standard entries, or if standard POSIX permissions are missing (probably no @@ -979,6 +968,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_type = DEF_USER_OBJ; lacl[pos].a_id = uid; lacl[pos].a_perm = lacl[0].a_perm; + aclsid[pos] = well_known_creator_owner_sid; pos++; } if (!(types_def & GROUP_OBJ) && pos < MAX_ACL_ENTRIES) @@ -988,6 +978,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_perm = lacl[1].a_perm; /* Note the position of the DEF_GROUP_OBJ entry. */ def_pgrp_pos = pos; + aclsid[pos] = well_known_creator_group_sid; pos++; } if (!(types_def & OTHER_OBJ) && pos < MAX_ACL_ENTRIES) @@ -995,6 +986,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_type = DEF_OTHER_OBJ; lacl[pos].a_id = ILLEGAL_GID; lacl[pos].a_perm = lacl[2].a_perm; + aclsid[pos] = well_known_world_sid; pos++; } } @@ -1011,6 +1003,7 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_perm = def_class_perm; if (def_pgrp_pos >= 0) lacl[pos].a_perm |= lacl[def_pgrp_pos].a_perm; + aclsid[pos] = well_known_null_sid; } /* Make sure `pos' contains the number of used entries in lacl. */ @@ -1021,26 +1014,36 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, if (!new_style) for (idx = 0; idx < pos; ++idx) { - /* Current user? If the user entry has a deny ACE, don't check. */ - if (lacl[idx].a_id == myself->uid - && lacl[idx].a_type & (USER_OBJ | USER) - && !(lacl[idx].a_type & ACL_DEFAULT) - && !(lacl[idx].a_perm & DENY_RWX)) + if (lacl[idx].a_type & (USER_OBJ | USER) + && !(lacl[idx].a_type & ACL_DEFAULT)) { - int gpos; - gid_t grps[NGROUPS_MAX]; - cyg_ldap cldap; - - /* Sum up all permissions of groups the user is member of, plus - everyone perms, and merge them to user perms. */ - mode_t grp_perm = lacl[2].a_perm & S_IRWXO; - int gnum = internal_getgroups (NGROUPS_MAX, grps, &cldap); - for (int g = 0; g < gnum && grp_perm != S_IRWXO; ++g) - if ((gpos = 1, grps[g] == lacl[gpos].a_id) - || (gpos = searchace (lacl, MAX_ACL_ENTRIES, GROUP, grps[g])) - >= 0) - grp_perm |= lacl[gpos].a_perm & S_IRWXO; - lacl[idx].a_perm |= grp_perm; + mode_t perm; + + /* If we use the Windows user DB, utilize Authz to make sure all + user permissions are correctly reflecting the Windows + permissions. */ + if (cygheap->pg.nss_pwd_db () + && authz_get_user_attribute (&perm, psd, owner_sid)) + lacl[0].a_perm = perm; + /* Otherwise we only check the current user. If the user entry + has a deny ACE, don't check. */ + else if (lacl[idx].a_id == myself->uid + && !(lacl[idx].a_perm & DENY_RWX)) + { + /* Sum up all permissions of groups the user is member of, plus + everyone perms, and merge them to user perms. */ + BOOL ret; + + perm = lacl[2].a_perm & S_IRWXO; + for (int gidx = 1; gidx < pos; ++gidx) + if (lacl[gidx].a_type & (GROUP_OBJ | GROUP) + && CheckTokenMembership (cygheap->user.issetuid () + ? cygheap->user.imp_token () : NULL, + aclsid[gidx], &ret) + && ret) + perm |= lacl[gidx].a_perm & S_IRWXO; + lacl[idx].a_perm |= perm; + } } /* For all groups, if everyone has more permissions, add everyone perms to group perms. Skip groups with deny ACE. */ diff --git a/winsup/cygwin/sec_helper.cc b/winsup/cygwin/sec_helper.cc index a637930b4..551face2a 100644 --- a/winsup/cygwin/sec_helper.cc +++ b/winsup/cygwin/sec_helper.cc @@ -714,7 +714,7 @@ class authz_ctx friend class authz_ctx_cache; public: - void get_user_attribute (mode_t *, PSECURITY_DESCRIPTOR, PSID); + bool get_user_attribute (mode_t *, PSECURITY_DESCRIPTOR, PSID); }; /* Authz handles are not inheritable. */ @@ -776,7 +776,7 @@ authz_ctx_cache::context (PSID user_sid) /* Ask Authz for the effective user permissions of the user with SID user_sid on the object with security descriptor psd. We're caching the handles for the Authz resource manager and the user contexts. */ -void +bool authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, PSID user_sid) { @@ -799,7 +799,7 @@ authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, ctx_hdl = user_ctx_hdl; } if (!ctx_hdl && !(ctx_hdl = ctx_cache.context (user_sid))) - return; + return false; /* All set, check access. */ ACCESS_MASK access = 0; DWORD error = 0; @@ -819,16 +819,20 @@ authz_ctx::get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, if (AuthzAccessCheck (0, ctx_hdl, &req, NULL, psd, NULL, 0, &repl, NULL)) { if (access & FILE_READ_BITS) - *attribute |= S_IRUSR; + *attribute |= S_IROTH; if (access & FILE_WRITE_BITS) - *attribute |= S_IWUSR; + *attribute |= S_IWOTH; if (access & FILE_EXEC_BITS) - *attribute |= S_IXUSR; + *attribute |= S_IXOTH; + return true; } + return false; } -void authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, - PSID user_sid) +bool +authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, + PSID user_sid) { - authz.get_user_attribute (attribute, psd, user_sid); + *attribute = 0; + return authz.get_user_attribute (attribute, psd, user_sid); } diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h index 9c70121ae..b4c7a023f 100644 --- a/winsup/cygwin/security.h +++ b/winsup/cygwin/security.h @@ -456,7 +456,7 @@ void set_security_attribute (path_conv &pc, int attribute, PSECURITY_ATTRIBUTES psa, security_descriptor &sd_buf); -void authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, +bool authz_get_user_attribute (mode_t *attribute, PSECURITY_DESCRIPTOR psd, PSID user_sid); /* sec_acl.cc */ -- 2.43.5