From f97612978a32fd1015242f3e10072f40e6a510e5 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Thu, 28 Apr 2011 07:27:51 +0000 Subject: * 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/security.cc | 242 +++++++++++++++++++++++++++++----------------- 1 file changed, 154 insertions(+), 88 deletions(-) (limited to 'winsup/cygwin/security.cc') 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) { -- cgit v1.2.3