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 <corinna@vinschen.de>

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,5 +1,19 @@
 2015-04-17  Corinna Vinschen  <corinna@vinschen.de>
 
+	* 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  <corinna@vinschen.de>
+
 	* uinfo.cc (pwdgrp::fetch_account_from_windows): Always revert SID
 	subauth count after checking for known domain.
 
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 <stdlib.h>
 #include <sys/param.h>
+#include <sys/acl.h>
 #include <cygwin/kd.h>
 #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