diff options
| -rw-r--r-- | winsup/cygwin/ChangeLog | 17 | ||||
| -rw-r--r-- | winsup/cygwin/autoload.cc | 1 | ||||
| -rw-r--r-- | winsup/cygwin/ntdll.h | 14 | ||||
| -rw-r--r-- | winsup/cygwin/security.cc | 242 | ||||
| -rw-r--r-- | winsup/cygwin/wincap.cc | 8 | ||||
| -rw-r--r-- | winsup/cygwin/wincap.h | 2 |
6 files changed, 183 insertions, 101 deletions
diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 836a59544..fcb6895a9 100644 --- a/winsup/cygwin/ChangeLog +++ b/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. diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc index a469663c3..693566790 100644 --- a/winsup/cygwin/autoload.cc +++ b/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) diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h index 2937da224..8f13e1c6c 100644 --- a/winsup/cygwin/ntdll.h +++ b/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, 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) { diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc index af1acd520..4a7cc3e23 100644 --- a/winsup/cygwin/wincap.cc +++ b/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)); diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h index dc3312784..631cf30ae 100644 --- a/winsup/cygwin/wincap.h +++ b/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 }; |