From cf6ff1c3216ab611b9fa4c46145b684b8d36ce1f Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Sun, 12 Apr 2015 20:26:27 +0200 Subject: [PATCH] Fix pty permssion handling in new permission handling code * sec_acl.cc (set_posix_access): Always make sure Admins have WRITE_DAC and WRITE_OWNER permissions. * security.h (create_object_sd_from_attribute): Drop handle parameter from prototype. * security.cc (create_object_sd_from_attribute): Drop handle parameter. Just create the standard POSIXy security descriptor. (set_object_attribute): Accommodate dropped paramter in call to create_object_sd_from_attribute. * fhandler_tty.cc: Ditto, throughout. Signed-off-by: Corinna Vinschen --- winsup/cygwin/ChangeLog | 12 ++++++++++++ winsup/cygwin/fhandler_tty.cc | 15 ++++++--------- winsup/cygwin/sec_acl.cc | 16 ++++++++++++++++ winsup/cygwin/security.cc | 16 ++++++---------- winsup/cygwin/security.h | 4 ++-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 3d2bfef62..b4c251ca4 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,15 @@ +2015-04-12 Corinna Vinschen + + * sec_acl.cc (set_posix_access): Always make sure Admins have + WRITE_DAC and WRITE_OWNER permissions. + * security.h (create_object_sd_from_attribute): Drop handle parameter + from prototype. + * security.cc (create_object_sd_from_attribute): Drop handle parameter. + Just create the standard POSIXy security descriptor. + (set_object_attribute): Accommodate dropped paramter in call to + create_object_sd_from_attribute. + * fhandler_tty.cc: Ditto, throughout. + 2015-04-12 Corinna Vinschen * shm.cc (shmget): Fetch segment size from server rather than using diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index daa24ebcc..ccb76d90e 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -387,9 +387,8 @@ fhandler_pty_slave::open (int flags, mode_t) sd.malloc (sizeof (SECURITY_DESCRIPTOR)); RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION); SECURITY_ATTRIBUTES sa = { sizeof (SECURITY_ATTRIBUTES), NULL, TRUE }; - if (!create_object_sd_from_attribute (NULL, myself->uid, myself->gid, - S_IFCHR | S_IRUSR | S_IWUSR | S_IWGRP, - sd)) + if (!create_object_sd_from_attribute (myself->uid, myself->gid, + S_IRUSR | S_IWUSR | S_IWGRP, sd)) sa.lpSecurityDescriptor = (PSECURITY_DESCRIPTOR) sd; acquire_output_mutex (INFINITE); inuse = get_ttyp ()->create_inuse (&sa); @@ -1093,7 +1092,7 @@ fhandler_pty_slave::fchmod (mode_t mode) sd.malloc (sizeof (SECURITY_DESCRIPTOR)); RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION); if (!get_object_attribute (input_available_event, &uid, &gid, NULL) - && !create_object_sd_from_attribute (NULL, uid, gid, S_IFCHR | mode, sd)) + && !create_object_sd_from_attribute (uid, gid, mode, sd)) ret = fch_set_sd (sd, false); errout: if (to_close) @@ -1126,8 +1125,7 @@ fhandler_pty_slave::fchown (uid_t uid, gid_t gid) if ((uid == ILLEGAL_UID || uid == o_uid) && (gid == ILLEGAL_GID || gid == o_gid)) ret = 0; - else if (!create_object_sd_from_attribute (input_available_event, - uid, gid, S_IFCHR | mode, sd)) + else if (!create_object_sd_from_attribute (uid, gid, mode, sd)) ret = fch_set_sd (sd, true); } errout: @@ -1598,9 +1596,8 @@ fhandler_pty_master::setup () /* Create security attribute. Default permissions are 0620. */ sd.malloc (sizeof (SECURITY_DESCRIPTOR)); RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION); - if (!create_object_sd_from_attribute (NULL, myself->uid, myself->gid, - S_IFCHR | S_IRUSR | S_IWUSR | S_IWGRP, - sd)) + if (!create_object_sd_from_attribute (myself->uid, myself->gid, + S_IRUSR | S_IWUSR | S_IWGRP, sd)) sa.lpSecurityDescriptor = (PSECURITY_DESCRIPTOR) sd; /* Carefully check that the input_available_event didn't already exist. diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc index 628b22146..80e45a484 100644 --- a/winsup/cygwin/sec_acl.cc +++ b/winsup/cygwin/sec_acl.cc @@ -141,6 +141,7 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, mode_t class_obj = 0, other_obj, group_obj, deny; DWORD access; int idx, start_idx, class_idx, tmp_idx; + bool dev_saw_admins = false; /* Initialize local security descriptor. */ RtlCreateSecurityDescriptor (&sd, SECURITY_DESCRIPTOR_REVISION); @@ -335,6 +336,14 @@ 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) @@ -352,6 +361,13 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid, 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. */ + if (S_ISCHR (attr) && !dev_saw_admins + && !add_access_allowed_ace (acl, STD_RIGHTS_OWNER, + well_known_admins_sid, acl_len, + NO_INHERITANCE)) + return NULL; /* Create allow ACE for other. It's preceeded by class_obj if it exists. If so, skip it. */ if (aclbufp[idx].a_type & CLASS_OBJ) diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index c2d30b5af..ea8563417 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -409,14 +409,11 @@ get_object_attribute (HANDLE handle, uid_t *uidret, gid_t *gidret, } int -create_object_sd_from_attribute (HANDLE handle, uid_t uid, gid_t gid, - mode_t attribute, security_descriptor &sd) +create_object_sd_from_attribute (uid_t uid, gid_t gid, mode_t attribute, + security_descriptor &sd) { - path_conv pc; - if ((handle && get_object_sd (handle, sd)) - || !set_posix_access (attribute, uid, gid, NULL, 0, sd, false)) - return -1; - return 0; + return set_posix_access (S_IFCHR | attribute, uid, gid, NULL, 0, sd, false) + ? 0 : -1; } int @@ -434,12 +431,11 @@ set_object_sd (HANDLE handle, security_descriptor &sd, bool chown) } int -set_object_attribute (HANDLE handle, uid_t uid, gid_t gid, - mode_t attribute) +set_object_attribute (HANDLE handle, uid_t uid, gid_t gid, mode_t attribute) { security_descriptor sd; - if (create_object_sd_from_attribute (handle, uid, gid, attribute, sd) + if (create_object_sd_from_attribute (uid, gid, attribute, sd) || set_object_sd (handle, sd, uid != ILLEGAL_UID || gid != ILLEGAL_GID)) return -1; return 0; diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h index db5b9f65f..5378b2a45 100644 --- a/winsup/cygwin/security.h +++ b/winsup/cygwin/security.h @@ -440,8 +440,8 @@ int __reg3 set_created_file_access (HANDLE, path_conv &, mode_t); int __reg2 get_object_sd (HANDLE, security_descriptor &); int __reg3 get_object_attribute (HANDLE, uid_t *, gid_t *, mode_t *); int __reg3 set_object_attribute (HANDLE, uid_t, gid_t, mode_t); -int __reg3 create_object_sd_from_attribute (HANDLE, uid_t, gid_t, - mode_t, security_descriptor &); +int __reg3 create_object_sd_from_attribute (uid_t, gid_t, mode_t, + security_descriptor &); int __reg3 set_object_sd (HANDLE, security_descriptor &, bool); int __reg3 get_reg_attribute (HKEY hkey, mode_t *, uid_t *, gid_t *);