From 1647bf67c146d1e1d7169d1b413afb6a459b9ded Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Sat, 24 Oct 2009 08:26:01 +0000 Subject: [PATCH] * 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. --- winsup/cygwin/ChangeLog | 9 ++++++++ winsup/cygwin/fhandler.cc | 48 +++++++++++++++++---------------------- winsup/cygwin/security.cc | 5 ---- winsup/cygwin/wincap.cc | 11 --------- winsup/cygwin/wincap.h | 2 -- 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,3 +1,12 @@ +2009-10-23 Corinna Vinschen + + * 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 * fhandler_random.cc (fhandler_dev_random::lseek): Allow negative 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. - 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. + This is done for two reasons. - 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 + 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. + + 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)