* 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.

4 files changed, 46 insertions, 12 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 62623d580..60c156974 100644
--- 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>
 
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index f43b8affa..a8a251297 100644
--- 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
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index d5e371087..7277d5f63 100644
--- 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,
diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h
index 8e66200f9..74955ff63 100644
--- 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)