* fhandler.cc (fhandler_base::open): Always create file with default

	security descriptor and fix descriptor afterwards.  Change comment to
	explain why.
	* security.cc (alloc_sd): Drop setting the SE_DACL_PROTECTED flag.
	* wincap.cc: Remove has_dacl_protect throughout.
	* wincap.h: Ditto.

5 files changed, 30 insertions, 45 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 475320e8d..9bb75a51f 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,14 @@
 2009-10-23  Corinna Vinschen  <corinna@vinschen.de>
 
+	* fhandler.cc (fhandler_base::open): Always create file with default
+	security descriptor and fix descriptor afterwards.  Change comment to
+	explain why.
+	* security.cc (alloc_sd): Drop setting the SE_DACL_PROTECTED flag.
+	* wincap.cc: Remove has_dacl_protect throughout.
+	* wincap.h: Ditto.
+
+2009-10-23  Corinna Vinschen  <corinna@vinschen.de>
+
 	* fhandler_random.cc (fhandler_dev_random::lseek): Allow negative
 	dummy file positions as on Linux.
 
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 7a7d80148..3e6e158c5 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -553,20 +553,7 @@ fhandler_base::open (int flags, mode_t mode)
 	{
 	  file_attributes |= FILE_ATTRIBUTE_NORMAL;
 
-	  /* If mode has no write bits set, and ACLs are not used, we set
-	     the DOS R/O attribute. */
-	  if (!has_acls () && !(mode & (S_IWUSR | S_IWGRP | S_IWOTH)))
-	    file_attributes |= FILE_ATTRIBUTE_READONLY;
-
-	  /* If the file should actually be created and has ACLs,
-	     set files attributes, except on remote file systems.
-	     See below. */
-	  if (has_acls () && !pc.isremote ())
-	    {
-	      set_security_attribute (pc, mode, &sa, sd);
-	      attr.SecurityDescriptor = sa.lpSecurityDescriptor;
-	    }
-	  else if (pc.fs_is_nfs ())
+	  if (pc.fs_is_nfs ())
 	    {
 	      /* When creating a file on an NFS share, we have to set the
 		 file mode by writing a NFS fattr3 structure with the
@@ -586,6 +573,10 @@ fhandler_base::open (int flags, mode_t mode)
 	      nfs_attr->type = NF3REG;
 	      nfs_attr->mode = mode;
 	    }
+	  else if (!has_acls () && !(mode & (S_IWUSR | S_IWGRP | S_IWOTH)))
+	    /* If mode has no write bits set, and ACLs are not used, we set
+	       the DOS R/O attribute. */
+	    file_attributes |= FILE_ATTRIBUTE_READONLY;
 	  /* The file attributes are needed for later use in, e.g. fchmod. */
 	  pc.file_attributes (file_attributes);
 	}
@@ -606,24 +597,27 @@ fhandler_base::open (int flags, mode_t mode)
 	goto done;
    }
 
-  /* After some discussion on the samba-technical list, starting here:
-     http://lists.samba.org/archive/samba-technical/2008-July/060247.html
+  /* Always create files using a NULL SD.  Create correct permission bits
+     afterwards, maintaining the owner and group information just like chmod.
+
+     This is done for two reasons.
 
-     Always create files on a remote share using a NULL SD.  Create
-     correct permission bits afterwards, maintaing the owner and group
-     information just like chmod.
+     On Windows filesystems we need to create the file with default
+     permissions to allow inheriting ACEs.  When providing an explicit DACL
+     in calls to [Nt]CreateFile, the created file will not inherit default
+     permissions from the parent object.  This breaks not only Windows
+     inheritance, but also POSIX ACL inheritance.
 
-     The reason to do this is to maintain the Windows behaviour when
-     creating files on a remote share.  Files on a remote share are
-     created as the user used for authentication.  In a domain that's
+     Another reason to do this are remote shares.  Files on a remote share
+     are created as the user used for authentication.  In a domain that's
      usually the user you're logged in as.  Outside of a domain you're
      authenticating using a local user account on the sharing machine.
      If the SIDs of the client machine are used, that's entirely
-     unexpected behaviour.
-
-     Doing it like we do here creates the expected SD in a domain as
-     well as on standalone servers. */
-  if ((flags & O_CREAT) && has_acls () && pc.isremote ())
+     unexpected behaviour.  Doing it like we do here creates the expected SD
+     in a domain as well as on standalone servers.
+     This is the result of a discussion on the samba-technical list, starting at
+     http://lists.samba.org/archive/samba-technical/2008-July/060247.html */
+  if ((flags & O_CREAT) && has_acls ())
     set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, mode);
 
   set_io_handle (fh);
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index c33be76c7..b8750eb8a 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -432,11 +432,6 @@ 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))
     {
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index 7277d5f63..d5e371087 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -25,7 +25,6 @@ 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,
@@ -64,7 +63,6 @@ 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,
@@ -103,7 +101,6 @@ 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,
@@ -142,7 +139,6 @@ 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,
@@ -181,7 +177,6 @@ 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,
@@ -220,7 +215,6 @@ 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,
@@ -259,7 +253,6 @@ 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,
@@ -298,7 +291,6 @@ 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,
@@ -337,7 +329,6 @@ 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,
@@ -376,7 +367,6 @@ 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,
@@ -415,7 +405,6 @@ 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 74955ff63..8e66200f9 100644
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -17,7 +17,6 @@ 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;
@@ -72,7 +71,6 @@ 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)