This is the mail archive of the
cygwin-cvs@cygwin.com
mailing list for the Cygwin project.
[newlib-cygwin] Fix pty permssion handling in new permission handling code
- From: Corinna Vinschen <corinna at sourceware dot org>
- To: cygwin-cvs at sourceware dot org
- Date: 12 Apr 2015 18:26:46 -0000
- Subject: [newlib-cygwin] Fix pty permssion handling in new permission handling code
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=cf6ff1c3216ab611b9fa4c46145b684b8d36ce1f
commit cf6ff1c3216ab611b9fa4c46145b684b8d36ce1f
Author: Corinna Vinschen <corinna@vinschen.de>
Date: Sun Apr 12 20:26:27 2015 +0200
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 <corinna@vinschen.de>
Diff:
---
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 3d2bfef..b4c251c 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,17 @@
2015-04-12 Corinna Vinschen <corinna@vinschen.de>
+ * 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 <corinna@vinschen.de>
+
* shm.cc (shmget): Fetch segment size from server rather than using
size argument to accommodate existing segments. Add comment to explain
why.
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index daa24eb..ccb76d9 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 628b221..80e45a4 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 c2d30b5..ea85634 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 db5b9f6..5378b2a 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 *);