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

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 475320e8d4913b445a404e69dd6a4d9ace375ab5..9bb75a51f8e4b6e5512433243fe5093a108ad652 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,12 @@
+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

diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 7a7d80148b25af593f2b9cf1dc806df36edbf27f..3e6e158c5ef55383c363940e5e011a19fd45f726 100644 (file)
--- 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 c33be76c7e96ed487dd474ef2f6aae7892ad408e..b8750eb8ac34069b3960eb5ba98832e613ac5724 100644 (file)
--- 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 7277d5f638c94da30c3acd68ff3bc4c4a932b8fa..d5e371087c24ef71d9f0a1e67f79fef4ea41af72 100644 (file)
--- 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 74955ff632ba6eb808bd8272efb06224549939c3..8e66200f9b408b89d58d8ffce11c23cbd0f89fc9 100644 (file)
--- 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)