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

winsup/cygwin/ChangeLog

@@ -1,3 +1,20 @@
+2011-04-28  Corinna Vinschen  <corinna@vinschen.de>
+
+	* 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.
+
 2011-04-24  Corinna Vinschen  <corinna@vinschen.de>
 
 	* include/fenv.h: Add missing _FENV_H_ define.

winsup/cygwin/autoload.cc

@@ -357,7 +357,6 @@ LoadDLLfunc (CryptAcquireContextW, 20, advapi32)
 LoadDLLfunc (CryptGenRandom, 12, advapi32)
 LoadDLLfunc (CryptReleaseContext, 8, advapi32)
 LoadDLLfunc (DeregisterEventSource, 4, advapi32)
-LoadDLLfunc (GetSecurityInfo, 32, advapi32)
 LoadDLLfunc (LogonUserW, 24, advapi32)
 LoadDLLfunc (LookupAccountNameW, 28, advapi32)
 LoadDLLfunc (LookupAccountSidW, 28, advapi32)

winsup/cygwin/ntdll.h

@@ -1,7 +1,7 @@
 /* ntdll.h.  Contains ntdll specific stuff not defined elsewhere.
 
    Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008,
-   2009, 2010 Red Hat, Inc.
+   2009, 2010, 2011 Red Hat, Inc.
 
    This file is part of Cygwin.
 
@@ -1105,12 +1105,18 @@ extern "C"
   LONG NTAPI RtlCompareUnicodeString (PUNICODE_STRING, PUNICODE_STRING,
 				      BOOLEAN);
   NTSTATUS NTAPI RtlConvertSidToUnicodeString (PUNICODE_STRING, PSID, BOOLEAN);
-  VOID NTAPI RtlCopyUnicodeString (PUNICODE_STRING, PUNICODE_STRING);
+  NTSTATUS NTAPI RtlConvertToAutoInheritSecurityObject (PSECURITY_DESCRIPTOR,
+  							PSECURITY_DESCRIPTOR,
+							PSECURITY_DESCRIPTOR *,
+							GUID *, BOOLEAN,
+							PGENERIC_MAPPING);
   NTSTATUS NTAPI RtlCopySid (ULONG, PSID, PSID);
+  VOID NTAPI RtlCopyUnicodeString (PUNICODE_STRING, PUNICODE_STRING);
   NTSTATUS NTAPI RtlCreateAcl (PACL, ULONG, ULONG);
   NTSTATUS NTAPI RtlCreateRegistryKey (ULONG, PCWSTR);
   NTSTATUS NTAPI RtlCreateSecurityDescriptor (PSECURITY_DESCRIPTOR, ULONG);
   BOOLEAN NTAPI RtlCreateUnicodeStringFromAsciiz (PUNICODE_STRING, PCSTR);
+  NTSTATUS NTAPI RtlDeleteSecurityObject (PSECURITY_DESCRIPTOR *);
   NTSTATUS NTAPI RtlDowncaseUnicodeString (PUNICODE_STRING, PUNICODE_STRING,
 					   BOOLEAN);
   NTSTATUS NTAPI RtlEnterCriticalSection (PRTL_CRITICAL_SECTION);
@@ -1124,6 +1130,9 @@ extern "C"
   VOID NTAPI RtlFreeUnicodeString (PUNICODE_STRING);
   BOOLEAN NTAPI RtlFirstFreeAce (PACL, PVOID *);
   NTSTATUS NTAPI RtlGetAce (PACL, ULONG, PVOID);
+  NTSTATUS NTAPI RtlGetControlSecurityDescriptor (PSECURITY_DESCRIPTOR,
+						  PSECURITY_DESCRIPTOR_CONTROL,
+						  PULONG);
   HANDLE NTAPI RtlGetCurrentTransaction ();
   NTSTATUS NTAPI RtlGetDaclSecurityDescriptor (PSECURITY_DESCRIPTOR, PBOOLEAN,
 					       PACL *, PBOOLEAN);
@@ -1138,6 +1147,7 @@ extern "C"
   NTSTATUS NTAPI RtlIntegerToUnicodeString (ULONG, ULONG, PUNICODE_STRING);
   ULONG NTAPI RtlIsDosDeviceName_U (PCWSTR);
   NTSTATUS NTAPI RtlLeaveCriticalSection (PRTL_CRITICAL_SECTION);
+  ULONG NTAPI RtlLengthSecurityDescriptor (PSECURITY_DESCRIPTOR);
   ULONG NTAPI RtlLengthSid (PSID);
   ULONG NTAPI RtlNtStatusToDosError (NTSTATUS);
   NTSTATUS NTAPI RtlOemStringToUnicodeString (PUNICODE_STRING, POEM_STRING,

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;
+  NTSTATUS status;
-  int retry = 0;
+  OBJECT_ATTRIBUTES attr;
-  int res = -1;
+  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
+	  debug_printf ("NtQuerySecurityObject (%S), status %p",
-	     from the parent object, you can't use the NtQuerySecurityObject
+			pc.get_nt_native_path (), status);
-	     function.  In the DACL returned by this functions, the
+	  fh = NULL;
-	     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);
-	    }
-	}
-      if (!retry)
-	{
-	  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;
-	    }
 	}
     }
-  if (retry && fh)
+  /* If the handle was NULL, or fetching with the original handle didn't work,
-    NtClose (fh);
+     try to reopen the file with READ_CONTROL and fetch the security descriptor
-  if (error != ERROR_SUCCESS)
+     using that handle. */
-    __seterrno_from_win_error (error);
+  if (!fh)
-  return res;
+    {
+      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))
+	{
+	  sd.free ();
+	  __seterrno_from_nt_status (status);
+	  return -1;
+	}
+    }
+  /* 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,
+  static GENERIC_MAPPING NO_COPY reg_mapping = { KEY_READ,
-					     KEY_WRITE,
+						 KEY_WRITE,
-					     KEY_EXECUTE,
+						 KEY_EXECUTE,
-					     KEY_ALL_ACCESS };
+						 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)
     {

winsup/cygwin/wincap.cc

@@ -53,7 +53,6 @@ wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
-  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -86,7 +85,6 @@ wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) =
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:false,
-  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -119,7 +117,6 @@ wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
-  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -152,7 +149,6 @@ wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
-  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -185,7 +181,6 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
-  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -218,7 +213,6 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:false,
   has_restricted_raw_disk_access:false,
   use_dont_resolve_hack:true,
-  use_get_sec_info_on_dirs:true,
 };
 
 wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -251,7 +245,6 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:true,
   has_restricted_raw_disk_access:true,
   use_dont_resolve_hack:false,
-  use_get_sec_info_on_dirs:false,
 };
 
 wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
@@ -284,7 +277,6 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
   has_fast_cwd:true,
   has_restricted_raw_disk_access:true,
   use_dont_resolve_hack:false,
-  use_get_sec_info_on_dirs:false,
 };
 
 wincapc wincap __attribute__((section (".cygwin_dll_common"), shared));

winsup/cygwin/wincap.h

@@ -43,7 +43,6 @@ struct wincaps
   unsigned has_fast_cwd					: 1;
   unsigned has_restricted_raw_disk_access		: 1;
   unsigned use_dont_resolve_hack			: 1;
-  unsigned use_get_sec_info_on_dirs			: 1;
 };
 
 class wincapc
@@ -92,7 +91,6 @@ public:
   bool	IMPLEMENT (has_fast_cwd)
   bool	IMPLEMENT (has_restricted_raw_disk_access)
   bool	IMPLEMENT (use_dont_resolve_hack)
-  bool	IMPLEMENT (use_get_sec_info_on_dirs)
 
 #undef IMPLEMENT
 };