* security.cc (alloc_sd): Re-introduce setting the SE_DACL_PROTECTED

flag. Remove INHERITED_ACE flag from all inherited ACEs. Add comment. Fix ace_off counter in unrelated ACE loop. * wincap.cc: Re-add has_dacl_protect throughout. * wincap.h: Ditto.
2009-11-03 09:31:45 +00:00
parent 1bb3825949
commit 18f7c50401
4 changed files with 46 additions and 12 deletions
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,11 @@
+2009-11-03  Corinna Vinschen  <corinna@vinschen.de>
+
+	* security.cc (alloc_sd): Re-introduce setting the SE_DACL_PROTECTED
+	flag.  Remove INHERITED_ACE flag from all inherited ACEs.  Add comment.
+	Fix ace_off counter in unrelated ACE loop.
+	* wincap.cc: Re-add has_dacl_protect throughout.
+	* wincap.h: Ditto.
+
 2009-11-02  Corinna Vinschen  <corinna@vinschen.de>

 	* security.cc (alloc_sd): Re-enable generating default permission
@@ -21,7 +29,7 @@
 	(sys_cp_wcstombs): Make UNICODE private use area conversion roundtrip
 	save for all characters.
 	(sys_cp_mbstowcs): Ditto, by removing special case for UTF-8 sequences
-	representing U+f0XX UNICODE chars.  Fix typo in comment.
+	representing U+F0xx UNICODE chars.  Fix typo in comment.

 2009-11-02  Corinna Vinschen  <corinna@vinschen.de>

--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -434,6 +434,11 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
      return NULL;
    }

+  /* We set the SE_DACL_PROTECTED flag here to prevent the DACL from being
+   * modified by inheritable ACEs.  This flag is available since Win2K.  */
+  if (wincap.has_dacl_protect ())
+    sd.Control |= SE_DACL_PROTECTED;
+
  /* Create owner for local security descriptor. */
  if (!SetSecurityDescriptorOwner (&sd, owner_sid, FALSE))
    {
@@ -591,27 +596,36 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
 	      else
 		continue;
 	    }
-	  else if ((attribute & S_JUSTCREATED)
-		   && !(ace->Header.AceFlags & INHERITED_ACE))
-	    /* Since files and dirs are created with a NULL descriptor,
-	       inheritence rules kick in.  However, if no inheritable entries
-	       exist in the parent object, Windows will create entries from the
-	       user token's default DACL in the file DACL.  These entries are
-	       not desired and we drop them silently here. */
-	    continue;
+	  else if (attribute & S_JUSTCREATED)
+	    {
+	      /* Since files and dirs are created with a NULL descriptor,
+		 inheritence rules kick in.  If no inheritable entries exist
+		 in the parent object, Windows will create entries from the
+		 user token's default DACL in the file DACL.  These entries
+		 are not desired and we drop them silently. */
+	      if (!(ace->Header.AceFlags & INHERITED_ACE))
+		continue;
+	      /* Remove the INHERITED_ACE flag since on POSIX systems
+		 inheritance is settled when the file has been created.
+		 This also avoids error messages in Windows Explorer when
+		 opening a file's security tab.  Explorer complains if
+		 inheritable ACEs are preceding non-inheritable ACEs. */
+	      ace->Header.AceFlags &= ~INHERITED_ACE;
+	    }
 	  /*
 	   * Add unrelated ACCESS_DENIED_ACE to the beginning but
 	   * behind the owner_deny, ACCESS_ALLOWED_ACE to the end.
 	   * FIXME: this would break the order of the inherit-only ACEs
 	   */
 	  if (!AddAce (acl, ACL_REVISION,
-		       ace->Header.AceType == ACCESS_DENIED_ACE_TYPE?
-		       (owner_deny ? 1 : 0) : MAXDWORD,
+		       ace->Header.AceType == ACCESS_DENIED_ACE_TYPE
+		       ?  (owner_deny ? 1 : 0) : MAXDWORD,
 		       (LPVOID) ace, ace->Header.AceSize))
 	    {
 	      __seterrno ();
 	      return NULL;
 	    }
+	  ace_off++;
 	  acl_len += ace->Header.AceSize;
 	}

@@ -620,7 +634,6 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
    {
      const DWORD inherit = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE
 			    | INHERIT_ONLY_ACE;
-
 #if 0 /* FIXME: Not done currently as this breaks the canonical order */
      /* Set deny ACE for owner. */
      if (owner_deny
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -25,6 +25,7 @@ wincaps wincap_unknown __attribute__((section (".cygwin_dll_common"), shared)) =
  heapslop:0x0,
  max_sys_priv:SE_CHANGE_NOTIFY_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:false,
  has_ip_helper_lib:false,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -63,6 +64,7 @@ wincaps wincap_nt4 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x0,
  max_sys_priv:SE_CHANGE_NOTIFY_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:false,
  has_ip_helper_lib:false,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -101,6 +103,7 @@ wincaps wincap_nt4sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
  heapslop:0x0,
  max_sys_priv:SE_CHANGE_NOTIFY_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:false,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:true,
  has_physical_mem_access:true,
@@ -139,6 +142,7 @@ wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x0,
  max_sys_priv:SE_MANAGE_VOLUME_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -177,6 +181,7 @@ wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
  heapslop:0x0,
  max_sys_priv:SE_CREATE_GLOBAL_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -215,6 +220,7 @@ wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x0,
  max_sys_priv:SE_MANAGE_VOLUME_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -253,6 +259,7 @@ wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x0,
  max_sys_priv:SE_MANAGE_VOLUME_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -291,6 +298,7 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x0,
  max_sys_priv:SE_CREATE_GLOBAL_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:true,
@@ -329,6 +337,7 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x4,
  max_sys_priv:SE_CREATE_GLOBAL_PRIVILEGE,
  is_server:true,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:false,
@@ -367,6 +376,7 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x4,
  max_sys_priv:SE_CREATE_SYMBOLIC_LINK_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:false,
@@ -405,6 +415,7 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
  heapslop:0x4,
  max_sys_priv:SE_CREATE_SYMBOLIC_LINK_PRIVILEGE,
  is_server:false,
+  has_dacl_protect:true,
  has_ip_helper_lib:true,
  has_broken_if_oper_status:false,
  has_physical_mem_access:false,
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -17,6 +17,7 @@ struct wincaps
  DWORD    heapslop;
  DWORD    max_sys_priv;
  unsigned is_server                                    : 1;
+  unsigned has_dacl_protect                             : 1;
  unsigned has_ip_helper_lib                            : 1;
  unsigned has_broken_if_oper_status                    : 1;
  unsigned has_physical_mem_access                      : 1;
@@ -71,6 +72,7 @@ public:
  DWORD IMPLEMENT (heapslop)
  DWORD IMPLEMENT (max_sys_priv)
  bool  IMPLEMENT (is_server)
+  bool  IMPLEMENT (has_dacl_protect)
  bool  IMPLEMENT (has_ip_helper_lib)
  bool  IMPLEMENT (has_broken_if_oper_status)
  bool  IMPLEMENT (has_physical_mem_access)