From f63dffb818f9856a43ed6cfb3395d882b21d94b8 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Fri, 17 Apr 2015 19:54:59 +0200 Subject: [PATCH] Support acl(2) method for reading pty ACLs, fix pty chown * fhandler.h (fhandler_pty_slave::facl): Add prototype. * fhandler_tty.cc (fhandler_pty_slave::facl): New method. (fhandler_pty_slave::fchown): Fix uid/gid handling. * sec_acl.cc (set_posix_access): Drop superfluous class_idx variable. Simplify and move around code in a few places. To improve ACL readability, add r/w permissions to Admins ACE appended to pty ACL. Add comment to explain Windows ACE Mask filtering being in the way of creating a real CLASS_OBJ. (get_posix_access): Fake CLASS_OBJ for ptys. Explain why. * security.cc (get_object_attribute): Add S_IFCHR flag to attributes when calling get_posix_access. Signed-off-by: Corinna Vinschen --- winsup/cygwin/ChangeLog | 14 ++++++++ winsup/cygwin/fhandler.h | 1 + winsup/cygwin/fhandler_tty.cc | 64 +++++++++++++++++++++++++++++++++-- winsup/cygwin/sec_acl.cc | 56 +++++++++++++++++++----------- winsup/cygwin/security.cc | 9 +++-- 5 files changed, 120 insertions(+), 24 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index fb66f39f3..04bd520d8 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,17 @@ +2015-04-17 Corinna Vinschen + + * fhandler.h (fhandler_pty_slave::facl): Add prototype. + * fhandler_tty.cc (fhandler_pty_slave::facl): New method. + (fhandler_pty_slave::fchown): Fix uid/gid handling. + * sec_acl.cc (set_posix_access): Drop superfluous class_idx variable. + Simplify and move around code in a few places. To improve ACL + readability, add r/w permissions to Admins ACE appended to pty ACL. + Add comment to explain Windows ACE Mask filtering being in the way of + creating a real CLASS_OBJ. + (get_posix_access): Fake CLASS_OBJ for ptys. Explain why. + * security.cc (get_object_attribute): Add S_IFCHR flag to attributes + when calling get_posix_access. + 2015-04-17 Corinna Vinschen * uinfo.cc (pwdgrp::fetch_account_from_windows): Always revert SID diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 694c23bdd..05269e59b 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1545,6 +1545,7 @@ class fhandler_pty_slave: public fhandler_pty_common select_record *select_read (select_stuff *); virtual char const *ttyname () { return pc.dev.name; } int __reg2 fstat (struct stat *buf); + int __reg3 facl (int, int, struct acl *); int __reg1 fchmod (mode_t mode); int __reg2 fchown (uid_t uid, gid_t gid); diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index ccb76d90e..d243d515a 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -12,6 +12,7 @@ details. */ #include "winsup.h" #include #include +#include #include #include "cygerrno.h" #include "security.h" @@ -1018,6 +1019,62 @@ fhandler_pty_slave::fstat (struct stat *st) return 0; } +int __reg3 +fhandler_pty_slave::facl (int cmd, int nentries, aclent_t *aclbufp) +{ + int res = -1; + bool to_close = false; + security_descriptor sd; + mode_t attr = S_IFCHR; + + switch (cmd) + { + case SETACL: + if (!aclsort32 (nentries, 0, aclbufp)) + set_errno (ENOTSUP); + break; + case GETACL: + if (!aclbufp) + { + set_errno (EFAULT); + break; + } + /*FALLTHRU*/ + case GETACLCNT: + if (!input_available_event) + { + char buf[MAX_PATH]; + shared_name (buf, INPUT_AVAILABLE_EVENT, get_minor ()); + input_available_event = OpenEvent (READ_CONTROL, TRUE, buf); + if (input_available_event) + to_close = true; + } + if (!input_available_event + || get_object_sd (input_available_event, sd)) + { + res = get_posix_access (NULL, &attr, NULL, NULL, aclbufp, nentries); + if (aclbufp && res == MIN_ACL_ENTRIES) + { + aclbufp[0].a_perm = S_IROTH | S_IWOTH; + aclbufp[0].a_id = 18; + aclbufp[1].a_id = 544; + } + break; + } + if (cmd == GETACL) + res = get_posix_access (sd, &attr, NULL, NULL, aclbufp, nentries); + else + res = get_posix_access (sd, &attr, NULL, NULL, NULL, 0); + break; + default: + set_errno (EINVAL); + break; + } + if (to_close) + CloseHandle (input_available_event); + return res; +} + /* Helper function for fchmod and fchown, which just opens all handles and signals success via bool return. */ bool @@ -1122,8 +1179,11 @@ fhandler_pty_slave::fchown (uid_t uid, gid_t gid) RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION); if (!get_object_attribute (input_available_event, &o_uid, &o_gid, &mode)) { - if ((uid == ILLEGAL_UID || uid == o_uid) - && (gid == ILLEGAL_GID || gid == o_gid)) + if (uid == ILLEGAL_UID) + uid = o_uid; + if (gid == ILLEGAL_GID) + gid = o_gid; + if (uid == o_uid && gid == o_gid) ret = 0; else if (!create_object_sd_from_attribute (uid, gid, mode, sd)) ret = fch_set_sd (sd, true); diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc index b25e9b337..9336dea35 100644 --- a/winsup/cygwin/sec_acl.cc +++ b/winsup/cygwin/sec_acl.cc @@ -140,9 +140,9 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, size_t acl_len = sizeof (ACL); mode_t class_obj = 0, other_obj, group_obj, deny; DWORD access; - int idx, start_idx, class_idx, tmp_idx; + int idx, start_idx, tmp_idx; bool owner_eq_group = false; - bool dev_saw_admins = false; + bool dev_has_admins = false; /* Initialize local security descriptor. */ RtlCreateSecurityDescriptor (&sd, SECURITY_DESCRIPTOR_REVISION); @@ -172,8 +172,9 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, return NULL; } owner_eq_group = RtlEqualSid (owner, group); - - + if (S_ISCHR (attr)) + dev_has_admins = well_known_admins_sid == owner + || well_known_admins_sid == group; /* No POSIX ACL? Use attr to generate one from scratch. */ if (!aclbufp) @@ -276,11 +277,12 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, /* ... class_obj. Create Cygwin ACE. Only the S_ISGID attribute gets inherited. */ - access = CYG_ACE_ISBITS_TO_WIN (def ? attr & S_ISGID : attr); - class_idx = searchace (aclbufp, nentries, def | CLASS_OBJ); - if (class_idx >= 0) + access = CYG_ACE_ISBITS_TO_WIN (def ? attr & S_ISGID : attr) + | CYG_ACE_NEW_STYLE; + tmp_idx = searchace (aclbufp, nentries, def | CLASS_OBJ); + if (tmp_idx >= 0) { - class_obj = aclbufp[class_idx].a_perm; + class_obj = aclbufp[tmp_idx].a_perm; access |= CYG_ACE_MASK_TO_WIN (class_obj); } else @@ -288,9 +290,11 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, /* Setting class_obj to group_obj allows to write below code without additional checks for existence of a CLASS_OBJ. */ class_obj = group_obj; - class_idx = -1; } - access |= CYG_ACE_NEW_STYLE; + /* Note that Windows filters the ACE Mask value so it only reflects + the bit values supported by the object type. The result is that + we can't set a CLASS_OBJ value for ptys. The get_posix_access + function has to workaround that. */ if (!add_access_denied_ace (acl, access, well_known_null_sid, acl_len, inherit)) return NULL; @@ -359,14 +363,6 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, if (aclbufp[idx].a_perm == S_IRWXO) access |= FILE_DELETE_CHILD; } - /* For ptys check if the admins group is in the ACL. If so, - make sure the group has WRITE_DAC and WRITE_OWNER perms. */ - if (S_ISCHR (attr) && !dev_saw_admins - && aclsid[idx] == well_known_admins_sid) - { - access |= STD_RIGHTS_OWNER; - dev_saw_admins = true; - } if (aclbufp[idx].a_perm & S_IROTH) access |= FILE_ALLOW_READ; if (aclbufp[idx].a_perm & S_IWOTH) @@ -379,6 +375,10 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, == (S_IWOTH | S_IXOTH) && (!(attr & S_ISVTX) || aclbufp[idx].a_type & USER_OBJ)) access |= FILE_DELETE_CHILD; + /* For ptys, make sure the Administrators group has WRITE_DAC + and WRITE_OWNER perms. */ + if (dev_has_admins && aclsid[idx] == well_known_admins_sid) + access |= STD_RIGHTS_OWNER; if (!add_access_allowed_ace (acl, access, aclsid[idx], acl_len, inherit)) return NULL; @@ -386,8 +386,10 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, } /* 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. */ - if (S_ISCHR (attr) && !dev_saw_admins - && !add_access_allowed_ace (acl, STD_RIGHTS_OWNER, + if (S_ISCHR (attr) && !dev_has_admins + && !add_access_allowed_ace (acl, + STD_RIGHTS_OWNER | FILE_ALLOW_READ + | FILE_ALLOW_WRITE, well_known_admins_sid, acl_len, NO_INHERITANCE)) return NULL; @@ -860,6 +862,20 @@ get_posix_access (PSECURITY_DESCRIPTOR psd, lacl[pos].a_id = ILLEGAL_GID; lacl[pos].a_perm = class_perm | lacl[1].a_perm; } + /* 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 + CLASS_OBJ to get a valid POSIX ACL. However, Windows filters the ACE + Mask value so it only reflects the bit values supported by the object + type. The result is that we can't set an explicit CLASS_OBJ value for + ptys in the NULL SID ACE. */ + else if (S_ISCHR (attr) && owner_sid != well_known_admins_sid + && group_sid != well_known_admins_sid + && (pos = searchace (lacl, MAX_ACL_ENTRIES, CLASS_OBJ)) >= 0) + { + lacl[pos].a_type = CLASS_OBJ; + lacl[pos].a_id = ILLEGAL_GID; + lacl[pos].a_perm = lacl[1].a_perm; /* == group perms */ + } /* If this is a just created file, and there are no default permissions (probably no inherited ACEs so created from a default DACL), assign the permissions specified by the file creation mask. The values get diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index 170dc1621..6d891c1ac 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -401,11 +401,16 @@ get_object_attribute (HANDLE handle, uid_t *uidret, gid_t *gidret, mode_t *attribute) { security_descriptor sd; + mode_t attr = S_IFCHR; if (get_object_sd (handle, sd)) return -1; - return get_posix_access (sd, attribute, uidret, gidret, NULL, 0) >= 0 - ? 0 : -1; + if (attribute) + *attribute |= S_IFCHR; + else + attribute = &attr; + return get_posix_access (sd, attribute, uidret, gidret, NULL, 0) + >= 0 ? 0 : -1; } int