* autoload.cc (GetSecurityInfo): Remove.

	* ntdll.h (RtlConvertToAutoInheritSecurityObject): Declare.
	(RtlDeleteSecurityObject): Declare.
	(RtlGetControlSecurityDescriptor): Declare.
	(RtlLengthSecurityDescriptor): Declare.
	* security.cc (file_mapping): New global variable.
	(get_file_sd): Rewrite.  Clean up code.  Get rid of GetSecurityInfo
	call.
	(alloc_sd): Call RtlSetControlSecurityDescriptor to set
	SE_DACL_PROTECTED flag.
	(check_file_access): Remove mapping.  Use file_mapping instead.
	(check_registry_access): Rename mapping to reg_mapping.
	* wincap.cc: Througout, drop use_get_sec_info_on_dirs,
	* wincap.h (struct wincaps): Drop use_get_sec_info_on_dirs.

1 files changed, 154 insertions, 88 deletions

diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc
index 2c6ef1a9c..25f6a6b72 100644
--- a/winsup/cygwin/security.cc
+++ b/winsup/cygwin/security.cc
@@ -31,94 +31,164 @@ details. */
 				  | GROUP_SECURITY_INFORMATION \
 				  | OWNER_SECURITY_INFORMATION)
 
+static NO_COPY GENERIC_MAPPING file_mapping = { FILE_GENERIC_READ,
+						FILE_GENERIC_WRITE,
+						FILE_GENERIC_EXECUTE,
+						FILE_ALL_ACCESS };
+
 LONG
 get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd,
 	     bool justcreated)
 {
-  DWORD error = ERROR_SUCCESS;
-  int retry = 0;
-  int res = -1;
+  NTSTATUS status;
+  OBJECT_ATTRIBUTES attr;
+  IO_STATUS_BLOCK io;
+  ULONG len = SD_MAXIMUM_SIZE, rlen;
 
-  for (; retry < 2; ++retry)
+  /* Allocate space for the security descriptor. */
+  if (!sd.malloc (len))
     {
-      if (fh)
+      set_errno (ENOMEM);
+      return -1;
+    }
+  /* Try to fetch the security descriptor if the handle is valid. */
+  if (fh)
+    {
+      status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION,
+				      sd, len, &rlen);
+      if (!NT_SUCCESS (status))
 	{
-	  /* Amazing but true.  If you want to know if an ACE is inherited
-	     from the parent object, you can't use the NtQuerySecurityObject
-	     function.  In the DACL returned by this functions, the
-	     INHERITED_ACE flag is never set.  Only by calling GetSecurityInfo
-	     you get this information.
-	     
-	     However, this functionality is slow, and the extra information is
-	     only required when the file has been created and the permissions
-	     are about to be set to POSIX permissions.  Therefore we only use
-	     it in case the file just got created.  In all other cases we
-	     rather call NtQuerySecurityObject directly...
-	     
-	     ...except that there's a problem on 5.1 and 5.2 kernels.  The
-	     GetSecurityInfo call on a file sometimes returns with
-	     ERROR_INVALID_ADDRESS if a former request for the SD of the
-	     parent directory (or one of the parent directories?) used the
-	     NtQuerySecurityObject call, rather than GetSecurityInfo as well.
-	     As soon as all directory SDs are fetched using GetSecurityInfo,
-	     the problem disappears. */
-	  if (justcreated
-	      || (pc.isdir () && wincap.use_get_sec_info_on_dirs ()))
-	    {
-	      PSECURITY_DESCRIPTOR psd;
-	      error = GetSecurityInfo (fh, SE_FILE_OBJECT,
-				       ALL_SECURITY_INFORMATION,
-				       NULL, NULL, NULL, NULL, &psd);
-	      if (error == ERROR_SUCCESS)
-		{
-		  sd = psd;
-		  res = 0;
-		  break;
-		}
-	    }
-	  else
-	    {
-	      NTSTATUS status;
-	      ULONG len = SD_MAXIMUM_SIZE;
-
-	      if (!sd.malloc (len))
-		{
-		  set_errno (ENOMEM);
-		  break;
-		}
-	      status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION,
-					      sd, len, &len);
-	      if (NT_SUCCESS (status))
-		{
-		  res = 0;
-		  break;
-		}
-	      error = RtlNtStatusToDosError (status);
-	    }
+	  debug_printf ("NtQuerySecurityObject (%S), status %p",
+			pc.get_nt_native_path (), status);
+	  fh = NULL;
 	}
-      if (!retry)
+    }
+  /* If the handle was NULL, or fetching with the original handle didn't work,
+     try to reopen the file with READ_CONTROL and fetch the security descriptor
+     using that handle. */
+  if (!fh)
+    {
+      status = NtOpenFile (&fh, READ_CONTROL,
+			   pc.get_object_attr (attr, sec_none_nih), &io,
+			   FILE_SHARE_VALID_FLAGS, FILE_OPEN_FOR_BACKUP_INTENT);
+      if (!NT_SUCCESS (status))
+      	{
+	  sd.free ();
+	  __seterrno_from_nt_status (status);
+	  return -1;
+	}
+      status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION,
+				      sd, len, &rlen);
+      NtClose (fh);
+      if (!NT_SUCCESS (status))
 	{
-	  OBJECT_ATTRIBUTES attr;
-	  IO_STATUS_BLOCK io;
-	  NTSTATUS status;
-
-	  status = NtOpenFile (&fh, READ_CONTROL,
-			       pc.get_object_attr (attr, sec_none_nih),
-			       &io, FILE_SHARE_VALID_FLAGS,
-			       FILE_OPEN_FOR_BACKUP_INTENT);
-	  if (!NT_SUCCESS (status))
-	    {
-	      fh = NULL;
-	      error = RtlNtStatusToDosError (status);
-	      break;
-	    }
+	  sd.free ();
+	  __seterrno_from_nt_status (status);
+	  return -1;
 	}
     }
-  if (retry && fh)
-    NtClose (fh);
-  if (error != ERROR_SUCCESS)
-    __seterrno_from_win_error (error);
-  return res;
+  /* Ok, so we have a security descriptor now.  Unfortunately, if you want
+     to know if an ACE is inherited from the parent object, you can't just
+     call NtQuerySecurityObject once.  The problem is this:
+
+     In the simple case, the SDs control word contains one of the
+     SE_DACL_AUTO_INHERITED or SE_DACL_PROTECTED flags, or at least one of
+     the ACEs has the INHERITED_ACE flag set.  In all of these cases the
+     GetSecurityInfo function calls NtQuerySecurityObject only once, too,
+     apparently because it figures that the DACL is self-sufficient, which
+     it usually is.  Windows Explorer, for instance, takes great care to
+     set these flags in a security descriptor if you change the ACL in the
+     GUI property dialog.
+
+     The tricky case is if none of these flags is set in the SD.  That means
+     the information whether or not an ACE has been inherited is not available
+     in the DACL of the object.  In this case GetSecurityInfo also fetches the
+     SD from the parent directory and tests if the object's SD contains
+     inherited ACEs from the parent.  The below code is closly emulating the
+     behaviour of GetSecurityInfo so we can get rid of this advapi32 dependency.
+
+     However, this functionality is slow, and the extra information is only
+     required when the file has been created and the permissions are about
+     to be set to POSIX permissions.  Therefore we only use it in case the
+     file just got created.
+
+     Note that GetSecurityInfo has a problem on 5.1 and 5.2 kernels.  Sometimes
+     it returns ERROR_INVALID_ADDRESS if a former request for the parent
+     directories' SD used NtQuerySecurityObject, rather than GetSecurityInfo
+     as well.  See http://cygwin.com/ml/cygwin-developers/2011-03/msg00027.html
+     for the solution.  This problem does not occur with the below code, so
+     the workaround could be removed. */
+  if (justcreated)
+    {
+      SECURITY_DESCRIPTOR_CONTROL ctrl;
+      ULONG dummy;
+      PACL dacl;
+      BOOLEAN exists, def;
+      ACCESS_ALLOWED_ACE *ace;
+      UNICODE_STRING dirname;
+      PSECURITY_DESCRIPTOR psd, nsd;
+      tmp_pathbuf tp;
+
+      /* Check SDs control flags.  If SE_DACL_AUTO_INHERITED or
+	 SE_DACL_PROTECTED is set we're done. */
+      RtlGetControlSecurityDescriptor (sd, &ctrl, &dummy);
+      if (ctrl & (SE_DACL_AUTO_INHERITED | SE_DACL_PROTECTED))
+	return 0;
+      /* Otherwise iterate over the ACEs and see if any one of them has the
+	 INHERITED_ACE flag set.  If so, we're done. */
+      if (NT_SUCCESS (RtlGetDaclSecurityDescriptor (sd, &exists, &dacl, &def))
+	  && exists && dacl)
+	for (ULONG idx = 0; idx < dacl->AceCount; ++idx)
+	  if (RtlGetAce (dacl, idx, (PVOID *) &ace)
+	      && (ace->Header.AceFlags & INHERITED_ACE))
+	    return 0;
+      /* Otherwise, open the parent directory with READ_CONTROL... */
+      RtlSplitUnicodePath (pc.get_nt_native_path (), &dirname, NULL);
+      InitializeObjectAttributes (&attr, &dirname, pc.objcaseinsensitive (),
+				  NULL, NULL);
+      status = NtOpenFile (&fh, READ_CONTROL, &attr, &io,
+			   FILE_SHARE_VALID_FLAGS,
+			   FILE_OPEN_FOR_BACKUP_INTENT
+			   | FILE_OPEN_REPARSE_POINT);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("NtOpenFile (%S), status %p", &dirname, status);
+	  return 0;
+	}
+      /* ... fetch the parent's security descriptor ... */
+      psd = (PSECURITY_DESCRIPTOR) tp.w_get ();
+      status = NtQuerySecurityObject (fh, ALL_SECURITY_INFORMATION,
+				      psd, len, &rlen);
+      NtClose (fh);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("NtQuerySecurityObject (%S), status %p",
+			&dirname, status);
+	  return 0;
+	}
+      /* ... and create a new security descriptor in which all inherited ACEs
+         are marked with the INHERITED_ACE flag.  For a description of the
+	 undocumented RtlConvertToAutoInheritSecurityObject function from
+	 ntdll.dll see the MSDN man page for the advapi32 function
+	 ConvertToAutoInheritPrivateObjectSecurity.  Fortunately the latter
+	 is just a shim. */
+      status = RtlConvertToAutoInheritSecurityObject (psd, sd, &nsd, NULL,
+						      pc.isdir (),
+						      &file_mapping);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("RtlConvertToAutoInheritSecurityObject (%S), status %p",
+			&dirname, status);
+	  return 0;
+	}
+      /* Eventually copy the new security descriptor into sd and delete the
+      	 original one created by RtlConvertToAutoInheritSecurityObject from
+	 the heap. */
+      len = RtlLengthSecurityDescriptor (nsd);
+      memcpy ((PSECURITY_DESCRIPTOR) sd, nsd, len);
+      RtlDeleteSecurityObject (&nsd);
+    }
+  return 0;
 }
 
 LONG
@@ -482,7 +552,7 @@ alloc_sd (path_conv &pc, __uid32_t uid, __gid32_t gid, int attribute,
 
   /* We set the SE_DACL_PROTECTED flag here to prevent the DACL from being
      modified by inheritable ACEs. */
-  sd.Control |= SE_DACL_PROTECTED;
+  RtlSetControlSecurityDescriptor (&sd, SE_DACL_PROTECTED, SE_DACL_PROTECTED);
 
   /* Create owner for local security descriptor. */
   if (!SetSecurityDescriptorOwner (&sd, owner_sid, FALSE))
@@ -974,10 +1044,6 @@ check_file_access (path_conv &pc, int flags, bool effective)
 {
   security_descriptor sd;
   int ret = -1;
-  static GENERIC_MAPPING NO_COPY mapping = { FILE_GENERIC_READ,
-					     FILE_GENERIC_WRITE,
-					     FILE_GENERIC_EXECUTE,
-					     FILE_ALL_ACCESS };
   DWORD desired = 0;
   if (flags & R_OK)
     desired |= FILE_READ_DATA;
@@ -986,7 +1052,7 @@ check_file_access (path_conv &pc, int flags, bool effective)
   if (flags & X_OK)
     desired |= FILE_EXECUTE;
   if (!get_file_sd (NULL, pc, sd, false))
-    ret = check_access (sd, mapping, desired, flags, effective);
+    ret = check_access (sd, file_mapping, desired, flags, effective);
   debug_printf ("flags %x, ret %d", flags, ret);
   return ret;
 }
@@ -996,10 +1062,10 @@ check_registry_access (HANDLE hdl, int flags, bool effective)
 {
   security_descriptor sd;
   int ret = -1;
-  static GENERIC_MAPPING NO_COPY mapping = { KEY_READ,
-					     KEY_WRITE,
-					     KEY_EXECUTE,
-					     KEY_ALL_ACCESS };
+  static GENERIC_MAPPING NO_COPY reg_mapping = { KEY_READ,
+						 KEY_WRITE,
+						 KEY_EXECUTE,
+						 KEY_ALL_ACCESS };
   DWORD desired = 0;
   if (flags & R_OK)
     desired |= KEY_ENUMERATE_SUB_KEYS;
@@ -1008,7 +1074,7 @@ check_registry_access (HANDLE hdl, int flags, bool effective)
   if (flags & X_OK)
     desired |= KEY_QUERY_VALUE;
   if (!get_reg_sd (hdl, sd))
-    ret = check_access (sd, mapping, desired, flags, effective);
+    ret = check_access (sd, reg_mapping, desired, flags, effective);
   /* As long as we can't write the registry... */
   if (flags & W_OK)
     {