* external.cc (cygwin_internal): Call set_security_attribute with

	additional path_conv argument.
	* fhandler.cc (fhandler_base::open): Ditto.
	* fhandler_disk_file.cc (fhandler_disk_file::fchmod): Never set DOS
	R/O attribute when using ACLs.
	(fhandler_disk_file::mkdir): Ditto.  Set
	security descriptor on remote dirs after creating the dir, same as in
	fhandler_base::open.
	* fhandler_socket.cc (fhandler_socket::bind): Ditto for remote AF_LOCAL
	socket files.
	* path.cc (symlink_worker): Ditto. for remote symlinks.
	* security.cc (alloc_sd): Take additional path_conv argument.
	Accommodate throughout.  Drop setting FILE_WRITE_EA/FILE_READ_EA
	flags unconditionally (was only necessary for "ntea"). Don't set
	FILE_READ_ATTRIBUTES and FILE_WRITE_ATTRIBUTES unconditionally on
	Samba.  Add comment to explain.  Drop useless setting of
	STANDARD_RIGHTS_WRITE, it's in FILE_GENERIC_WRITE anyway.
	Remove FILE_READ_ATTRIBUTES bit from FILE_GENERIC_EXECUTE so as not
	to enforce read permissions on Samba.
	(set_security_attribute): Take additional path_conv argument.
	* security.h (set_security_attribute): Change prototype accordingly.

8 files changed, 69 insertions, 25 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 7d7d6acf8..e6850f679 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,27 @@
+2008-08-14  Corinna Vinschen  <corinna@vinschen.de>
+
+	* external.cc (cygwin_internal): Call set_security_attribute with
+	additional path_conv argument.
+	* fhandler.cc (fhandler_base::open): Ditto.
+	* fhandler_disk_file.cc (fhandler_disk_file::fchmod): Never set DOS
+	R/O attribute when using ACLs.
+	(fhandler_disk_file::mkdir): Ditto.  Set
+	security descriptor on remote dirs after creating the dir, same as in
+	fhandler_base::open.
+	* fhandler_socket.cc (fhandler_socket::bind): Ditto for remote AF_LOCAL
+	socket files.
+	* path.cc (symlink_worker): Ditto. for remote symlinks.
+	* security.cc (alloc_sd): Take additional path_conv argument.
+	Accommodate throughout.  Drop setting FILE_WRITE_EA/FILE_READ_EA
+	flags unconditionally (was only necessary for "ntea"). Don't set
+	FILE_READ_ATTRIBUTES and FILE_WRITE_ATTRIBUTES unconditionally on
+	Samba.  Add comment to explain.  Drop useless setting of
+	STANDARD_RIGHTS_WRITE, it's in FILE_GENERIC_WRITE anyway.
+	Remove FILE_READ_ATTRIBUTES bit from FILE_GENERIC_EXECUTE so as not
+	to enforce read permissions on Samba.
+	(set_security_attribute): Take additional path_conv argument.
+	* security.h (set_security_attribute): Change prototype accordingly.
+
 2008-08-13  Corinna Vinschen  <corinna@vinschen.de>
 
 	* mount.cc (fillout_mntent): Always print noumount option last.
diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
index d9ab4d578..675d8b066 100644
--- a/winsup/cygwin/external.cc
+++ b/winsup/cygwin/external.cc
@@ -288,12 +288,13 @@ cygwin_internal (cygwin_getinfo_types t, ...)
 	}
       case CW_GET_POSIX_SECURITY_ATTRIBUTE:
 	{
+	  path_conv dummy;
 	  security_descriptor sd;
 	  int attribute = va_arg (arg, int);
 	  PSECURITY_ATTRIBUTES psa = va_arg (arg, PSECURITY_ATTRIBUTES);
 	  void *sd_buf = va_arg (arg, void *);
 	  DWORD sd_buf_size = va_arg (arg, DWORD);
-	  set_security_attribute (attribute, psa, sd);
+	  set_security_attribute (dummy, attribute, psa, sd);
 	  if (!psa->lpSecurityDescriptor)
 	    return sd.size ();
 	  psa->lpSecurityDescriptor = sd_buf;
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 0f75ea7f5..7827d2d21 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -577,7 +577,7 @@ fhandler_base::open (int flags, mode_t mode)
 	     See below. */
 	  if (has_acls () && !pc.isremote ())
 	    {
-	      set_security_attribute (mode, &sa, sd);
+	      set_security_attribute (pc, mode, &sa, sd);
 	      attr.SecurityDescriptor = sa.lpSecurityDescriptor;
 	    }
 	  else if (pc.fs_is_nfs ())
diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
index de2b0d06e..5fc82302d 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -807,10 +807,10 @@ fhandler_disk_file::fchmod (mode_t mode)
 	res = 0;
     }
 
-  /* if the mode we want has any write bits set, we can't be read only. */
+  /* If the mode has any write bits set, the DOS R/O flag is in the way. */
   if (mode & (S_IWUSR | S_IWGRP | S_IWOTH))
     pc &= (DWORD) ~FILE_ATTRIBUTE_READONLY;
-  else
+  else if (!pc.has_acls ())	/* Never set DOS R/O if security is used. */
     pc |= (DWORD) FILE_ATTRIBUTE_READONLY;
   if (S_ISSOCK (mode))
     pc |= (DWORD) FILE_ATTRIBUTE_SYSTEM;
@@ -1380,8 +1380,10 @@ fhandler_disk_file::mkdir (mode_t mode)
   SECURITY_ATTRIBUTES sa = sec_none_nih;
   security_descriptor sd;
 
-  if (has_acls ())
-    set_security_attribute (S_IFDIR | ((mode & 07777) & ~cygheap->umask),
+  /* See comments in fhander_base::open () for an explanation why we defer
+     setting security attributes on remote files. */
+  if (has_acls () && !pc.isremote ())
+    set_security_attribute (pc, S_IFDIR | ((mode & 07777) & ~cygheap->umask),
 			    &sa, sd);
 
   NTSTATUS status;
@@ -1418,6 +1420,9 @@ fhandler_disk_file::mkdir (mode_t mode)
 			 p, plen);
   if (NT_SUCCESS (status))
     {
+      if (has_acls () && pc.isremote ())
+	set_file_attribute (dir, pc, ILLEGAL_UID, ILLEGAL_GID,
+			    S_IFDIR | ((mode & 07777) & ~cygheap->umask));
       NtClose (dir);
       res = 0;
     }
diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 176838de8..1ab5170ea 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -882,8 +882,10 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
 	fattr |= FILE_ATTRIBUTE_READONLY;
       SECURITY_ATTRIBUTES sa = sec_none_nih;
       security_descriptor sd;
-      if (pc.has_acls ())
-	set_security_attribute (mode, &sa, sd);
+      /* See comments in fhander_base::open () for an explanation why we defer
+	 setting security attributes on remote files. */
+      if (pc.has_acls () && !pc.isremote ())
+	set_security_attribute (pc, mode, &sa, sd);
       NTSTATUS status;
       HANDLE fh;
       OBJECT_ATTRIBUTES attr;
@@ -904,6 +906,8 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
 	}
       else
 	{
+	  if (pc.has_acls () && pc.isremote ())
+	    set_file_attribute (fh, pc, ILLEGAL_UID, ILLEGAL_GID, mode);
 	  char buf[sizeof (SOCKET_COOKIE) + 80];
 	  __small_sprintf (buf, "%s%u %c ", SOCKET_COOKIE, sin.sin_port,
 			   get_socket_type () == SOCK_STREAM ? 's'
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 64d8759b6..4a91ff876 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -1766,8 +1766,10 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
 	  goto done;
 	}
     }
-  if (win32_newpath.has_acls ())
-    set_security_attribute (S_IFLNK | STD_RBITS | STD_WBITS,
+  /* See comments in fhander_base::open () for an explanation why we defer
+     setting security attributes on remote files. */
+  if (win32_newpath.has_acls () && !win32_newpath.isremote ())
+    set_security_attribute (win32_newpath, S_IFLNK | STD_RBITS | STD_WBITS,
 			    &sa, sd);
   status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE,
 			 win32_newpath.get_object_attr (attr, sa),
@@ -1783,6 +1785,9 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym,
       __seterrno_from_nt_status (status);
       goto done;
     }
+  if (win32_newpath.has_acls () && win32_newpath.isremote ())
+    set_file_attribute (fh, win32_newpath, ILLEGAL_UID, ILLEGAL_GID,
+    			S_IFLNK | STD_RBITS | STD_WBITS);
   status = NtWriteFile (fh, NULL, NULL, NULL, &io, buf, cp - buf, NULL, NULL);
   if (NT_SUCCESS (status) && io.Information == (ULONG) (cp - buf))
     {
diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index b4c7caf2c..5d9846abc 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -380,7 +380,7 @@ add_access_denied_ace (PACL acl, int offset, DWORD attributes,
 }
 
 static PSECURITY_DESCRIPTOR
-alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
+alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
 	  security_descriptor &sd_ret)
 {
   BOOL dummy;
@@ -462,28 +462,31 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
   size_t acl_len = sizeof (ACL);
   int ace_off = 0;
 
-  /* Construct allow attribute for owner. */
+  /* Construct allow attribute for owner.
+     Don't set FILE_READ/WRITE_ATTRIBUTES unconditionally on Samba, otherwise
+     it enforces read permissions.  Same for other's below. */
   DWORD owner_allow = STANDARD_RIGHTS_ALL
-		      | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
+		      | pc.fs_is_samba ()
+			? 0 : (FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES);
   if (attribute & S_IRUSR)
     owner_allow |= FILE_GENERIC_READ;
   if (attribute & S_IWUSR)
     owner_allow |= FILE_GENERIC_WRITE;
   if (attribute & S_IXUSR)
-    owner_allow |= FILE_GENERIC_EXECUTE;
+    owner_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES;
   if (S_ISDIR (attribute)
       && (attribute & (S_IWUSR | S_IXUSR)) == (S_IWUSR | S_IXUSR))
     owner_allow |= FILE_DELETE_CHILD;
 
   /* Construct allow attribute for group. */
-  DWORD group_allow = STANDARD_RIGHTS_READ
-		      | FILE_READ_ATTRIBUTES | FILE_READ_EA;
+  DWORD group_allow = STANDARD_RIGHTS_READ |
+		      (pc.fs_is_samba () ? 0 : FILE_READ_ATTRIBUTES);
   if (attribute & S_IRGRP)
     group_allow |= FILE_GENERIC_READ;
   if (attribute & S_IWGRP)
-    group_allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE;
+    group_allow |= FILE_GENERIC_WRITE;
   if (attribute & S_IXGRP)
-    group_allow |= FILE_GENERIC_EXECUTE;
+    group_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES;
   if (S_ISDIR (attribute)
       && (attribute & (S_IWGRP | S_IXGRP)) == (S_IWGRP | S_IXGRP)
       && !(attribute & S_ISVTX))
@@ -491,13 +494,13 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 
   /* Construct allow attribute for everyone. */
   DWORD other_allow = STANDARD_RIGHTS_READ
-		      | FILE_READ_ATTRIBUTES | FILE_READ_EA;
+		      | (pc.fs_is_samba () ? 0 : FILE_READ_ATTRIBUTES);
   if (attribute & S_IROTH)
     other_allow |= FILE_GENERIC_READ;
   if (attribute & S_IWOTH)
-    other_allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE;
+    other_allow |= FILE_GENERIC_WRITE;
   if (attribute & S_IXOTH)
-    other_allow |= FILE_GENERIC_EXECUTE;
+    other_allow |= FILE_GENERIC_EXECUTE & ~FILE_READ_ATTRIBUTES;
   if (S_ISDIR (attribute)
       && (attribute & (S_IWOTH | S_IXOTH)) == (S_IWOTH | S_IXOTH)
       && !(attribute & S_ISVTX))
@@ -682,13 +685,13 @@ alloc_sd (__uid32_t uid, __gid32_t gid, int attribute,
 }
 
 void
-set_security_attribute (int attribute, PSECURITY_ATTRIBUTES psa,
+set_security_attribute (path_conv &pc, int attribute, PSECURITY_ATTRIBUTES psa,
 			security_descriptor &sd)
 {
   psa->lpSecurityDescriptor = sd.malloc (SECURITY_DESCRIPTOR_MIN_LENGTH);
   InitializeSecurityDescriptor ((PSECURITY_DESCRIPTOR)psa->lpSecurityDescriptor,
 				SECURITY_DESCRIPTOR_REVISION);
-  psa->lpSecurityDescriptor = alloc_sd (geteuid32 (), getegid32 (),
+  psa->lpSecurityDescriptor = alloc_sd (pc, geteuid32 (), getegid32 (),
 					attribute, sd);
 }
 
@@ -702,7 +705,8 @@ set_file_attribute (HANDLE handle, path_conv &pc,
     {
       security_descriptor sd;
 
-      if (!get_file_sd (handle, pc, sd) && alloc_sd (uid, gid, attribute, sd))
+      if (!get_file_sd (handle, pc, sd)
+	  && alloc_sd (pc, uid, gid, attribute, sd))
 	ret = set_file_sd (handle, pc, sd);
     }
   else
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index 9be7a28e2..27a2d0046 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -349,7 +349,8 @@ bool __stdcall add_access_denied_ace (PACL acl, int offset, DWORD attributes, PS
 int __stdcall check_file_access (path_conv &, int);
 int __stdcall check_registry_access (HANDLE, int);
 
-void set_security_attribute (int attribute, PSECURITY_ATTRIBUTES psa,
+void set_security_attribute (path_conv &pc, int attribute,
+			     PSECURITY_ATTRIBUTES psa,
 			     security_descriptor &sd_buf);
 
 bool get_sids_info (cygpsid, cygpsid, __uid32_t * , __gid32_t *);