From 2be593d961e3ccd21a7a19b5a0b716e43d0137dc Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Sun, 22 Oct 2006 14:57:43 +0000 Subject: [PATCH] * external.cc (cygwin_internal): Use security_descriptor::copy method. * sec_helper.cc (security_descriptor::malloc): Use own free method. Set type. (security_descriptor::realloc): Handle the case that psd has been allocated using LocalAlloc. Set type. (security_descriptor::free): Ditto. * security.cc (get_nt_attribute): Remove. (get_reg_security): Remove. (get_nt_object_security): Use GetSecurityInfo which handles all securable objects. (get_nt_object_attribute): Remove. (get_object_attribute): Call get_nt_object_security instead of get_nt_object_attribute. (get_file_attribute): Ditto. (check_registry_access): Call get_nt_object_security instead of get_reg_security. * security.h (cygpsid::operator PSID): Make method const, not the result. (class security_descriptor): Add type member. Accomodate throughout. (security_descriptor::copy): New method. (security_descriptor::operator PSECURITY_DESCRIPTOR *): New operator. --- winsup/cygwin/ChangeLog | 24 ++++++++ winsup/cygwin/external.cc | 5 +- winsup/cygwin/sec_helper.cc | 39 +++++++++--- winsup/cygwin/security.cc | 116 ++++++++---------------------------- winsup/cygwin/security.h | 18 +++++- 5 files changed, 96 insertions(+), 106 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 1f67f578a..de219c637 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,27 @@ +2006-10-22 Corinna Vinschen + + * external.cc (cygwin_internal): Use security_descriptor::copy method. + * sec_helper.cc (security_descriptor::malloc): Use own free method. + Set type. + (security_descriptor::realloc): Handle the case that psd has been + allocated using LocalAlloc. Set type. + (security_descriptor::free): Ditto. + * security.cc (get_nt_attribute): Remove. + (get_reg_security): Remove. + (get_nt_object_security): Use GetSecurityInfo which handles all + securable objects. + (get_nt_object_attribute): Remove. + (get_object_attribute): Call get_nt_object_security instead of + get_nt_object_attribute. + (get_file_attribute): Ditto. + (check_registry_access): Call get_nt_object_security instead of + get_reg_security. + * security.h (cygpsid::operator PSID): Make method const, not the + result. + (class security_descriptor): Add type member. Accomodate throughout. + (security_descriptor::copy): New method. + (security_descriptor::operator PSECURITY_DESCRIPTOR *): New operator. + 2006-10-22 Corinna Vinschen * fhandler.cc (fhandler_base::fhaccess): Check if opening registry diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc index a033df01a..cbfda931e 100644 --- a/winsup/cygwin/external.cc +++ b/winsup/cygwin/external.cc @@ -293,11 +293,10 @@ cygwin_internal (cygwin_getinfo_types t, ...) void *sd_buf = va_arg (arg, void *); DWORD sd_buf_size = va_arg (arg, DWORD); set_security_attribute (attribute, psa, sd); - if (!psa->lpSecurityDescriptor || sd.size () > sd_buf_size) + if (!psa->lpSecurityDescriptor) return sd.size (); - memcpy (sd_buf, sd, sd.size ()); psa->lpSecurityDescriptor = sd_buf; - return 0; + return sd.copy (sd_buf, sd_buf_size); } case CW_GET_SHMLBA: { diff --git a/winsup/cygwin/sec_helper.cc b/winsup/cygwin/sec_helper.cc index a32dcb8f5..002abf48b 100644 --- a/winsup/cygwin/sec_helper.cc +++ b/winsup/cygwin/sec_helper.cc @@ -224,20 +224,37 @@ get_sids_info (cygpsid owner_sid, cygpsid group_sid, __uid32_t * uidret, __gid32 PSECURITY_DESCRIPTOR security_descriptor::malloc (size_t nsize) { - if (psd) - ::free (psd); - psd = (PSECURITY_DESCRIPTOR) ::malloc (nsize); - sd_size = psd ? nsize : 0; + free (); + if ((psd = (PSECURITY_DESCRIPTOR) ::malloc (nsize))) + { + sd_size = nsize; + type = malloced; + } return psd; } PSECURITY_DESCRIPTOR security_descriptor::realloc (size_t nsize) { - PSECURITY_DESCRIPTOR tmp = (PSECURITY_DESCRIPTOR) ::realloc (psd, nsize); - if (!tmp) - return NULL; + PSECURITY_DESCRIPTOR tmp; + + if (type == malloced) + { + if (!(tmp = (PSECURITY_DESCRIPTOR) ::realloc (psd, nsize))) + return NULL; + } + else + { + if (!(tmp = (PSECURITY_DESCRIPTOR) ::malloc (nsize))) + return NULL; + if (psd) + { + memcpy (tmp, psd, LocalSize (psd)); + LocalFree (psd); + } + } sd_size = nsize; + type = malloced; return psd = tmp; } @@ -245,9 +262,15 @@ void security_descriptor::free () { if (psd) - ::free (psd); + { + if (type == local_alloced) + LocalFree (psd); + else + ::free (psd); + } psd = NULL; sd_size = 0; + type = local_alloced; } #if 0 // unused diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index dceb5b571..856ba42ed 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -1389,113 +1389,43 @@ get_info_from_sd (PSECURITY_DESCRIPTOR psd, mode_t *attribute, (!acl_exists || !acl)?"NO ":"", *attribute, uid, gid); } -static void -get_nt_attribute (const char *file, mode_t *attribute, - __uid32_t *uidret, __gid32_t *gidret) -{ - security_descriptor sd; - - if (read_sd (file, sd) <= 0) - debug_printf ("read_sd %E"); - get_info_from_sd (sd, attribute, uidret, gidret); -} - -static int -get_reg_security (HANDLE handle, security_descriptor &sd_ret) -{ - LONG ret; - DWORD len = 0; - - ret = RegGetKeySecurity ((HKEY) handle, - DACL_SECURITY_INFORMATION - | GROUP_SECURITY_INFORMATION - | OWNER_SECURITY_INFORMATION, - sd_ret, &len); - if (ret == ERROR_INSUFFICIENT_BUFFER) - { - if (!sd_ret.malloc (len)) - set_errno (ENOMEM); - else - ret = RegGetKeySecurity ((HKEY) handle, - DACL_SECURITY_INFORMATION - | GROUP_SECURITY_INFORMATION - | OWNER_SECURITY_INFORMATION, - sd_ret, &len); - } - if (ret != ERROR_SUCCESS) - { - __seterrno (); - return -1; - } - return 0; -} - int get_nt_object_security (HANDLE handle, SE_OBJECT_TYPE object_type, security_descriptor &sd_ret) { - NTSTATUS ret; - ULONG len = 0; - - /* Unfortunately, NtQuerySecurityObject doesn't work on predefined registry - keys like HKEY_LOCAL_MACHINE. It fails with "Invalid Handle". So we - have to retreat to the Win32 registry functions for registry keys. - What bugs me is that RegGetKeySecurity is obviously just a wrapper - around NtQuerySecurityObject, but there seems to be no function to - convert pseudo HKEY values to real handles. */ - if (object_type == SE_REGISTRY_KEY) - return get_reg_security (handle, sd_ret); - - ret = NtQuerySecurityObject (handle, + sd_ret.free (); + /* Don't use NtQuerySecurityObject. It doesn't recognize predefined + registry keys. */ + DWORD ret = GetSecurityInfo (handle, object_type, DACL_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION, - sd_ret, len, &len); - if (ret == STATUS_BUFFER_TOO_SMALL) + NULL, NULL, NULL, NULL, sd_ret); + if (ret != ERROR_SUCCESS) { - if (!sd_ret.malloc (len)) - set_errno (ENOMEM); - else - ret = NtQuerySecurityObject (handle, - DACL_SECURITY_INFORMATION - | GROUP_SECURITY_INFORMATION - | OWNER_SECURITY_INFORMATION, - sd_ret, len, &len); - } - if (ret != STATUS_SUCCESS) - { - __seterrno_from_nt_status (ret); + __seterrno_from_win_error (ret); return -1; } return 0; } -static int -get_nt_object_attribute (HANDLE handle, SE_OBJECT_TYPE object_type, - mode_t *attribute, __uid32_t *uidret, - __gid32_t *gidret) -{ - security_descriptor sd; - PSECURITY_DESCRIPTOR psd = NULL; - - if (get_nt_object_security (handle, object_type, sd)) - { - if (object_type == SE_FILE_OBJECT) - return -1; - } - else - psd = sd; - get_info_from_sd (psd, attribute, uidret, gidret); - return 0; -} - int get_object_attribute (HANDLE handle, SE_OBJECT_TYPE object_type, mode_t *attribute, __uid32_t *uidret, __gid32_t *gidret) { if (allow_ntsec) { - get_nt_object_attribute (handle, object_type, attribute, uidret, gidret); + security_descriptor sd; + PSECURITY_DESCRIPTOR psd = NULL; + + if (get_nt_object_security (handle, object_type, sd)) + { + if (object_type == SE_FILE_OBJECT) + return -1; + } + else + psd = sd; + get_info_from_sd (psd, attribute, uidret, gidret); return 0; } /* The entries are already set to default values */ @@ -1511,9 +1441,11 @@ get_file_attribute (int use_ntsec, HANDLE handle, const char *file, if (use_ntsec && allow_ntsec) { - if (!handle || get_nt_object_attribute (handle, SE_FILE_OBJECT, - attribute, uidret, gidret)) - get_nt_attribute (file, attribute, uidret, gidret); + security_descriptor sd; + + if (!handle || get_nt_object_security (handle, SE_FILE_OBJECT, sd)) + read_sd (file, sd); + get_info_from_sd (sd, attribute, uidret, gidret); return 0; } @@ -2039,7 +1971,7 @@ check_registry_access (HANDLE hdl, int flags) desired |= KEY_SET_VALUE; if (flags & X_OK) desired |= KEY_QUERY_VALUE; - if (!get_reg_security (hdl, sd)) + if (!get_nt_object_security (hdl, SE_REGISTRY_KEY, sd)) ret = check_access (sd, mapping, desired, flags); debug_printf ("flags %x, ret %d", flags, ret); return ret; diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h index 94e3cc3f5..2b5b57275 100644 --- a/winsup/cygwin/security.h +++ b/winsup/cygwin/security.h @@ -43,7 +43,7 @@ protected: public: cygpsid () {} cygpsid (PSID nsid) { psid = nsid; } - operator const PSID () { return psid; } + operator PSID () const { return psid; } const PSID operator= (PSID nsid) { return psid = nsid;} __uid32_t get_id (BOOL search_grp, int *type = NULL); int get_uid () { return get_id (FALSE); } @@ -182,16 +182,28 @@ class security_descriptor { protected: PSECURITY_DESCRIPTOR psd; DWORD sd_size; + enum { local_alloced, malloced } type; public: - security_descriptor () : psd (NULL), sd_size (0) {} + security_descriptor () : psd (NULL), sd_size (0), type (local_alloced) {} ~security_descriptor () { free (); } PSECURITY_DESCRIPTOR malloc (size_t nsize); PSECURITY_DESCRIPTOR realloc (size_t nsize); void free (); - inline DWORD size () const { return sd_size; } + inline DWORD size () { + if (!sd_size && psd && type == local_alloced) + sd_size = LocalSize (psd); + return sd_size; + } + inline DWORD copy (void *buf, DWORD buf_size) { + if (buf_size < size ()) + return sd_size; + memcpy (buf, psd, sd_size); + return 0; + } inline operator const PSECURITY_DESCRIPTOR () { return psd; } + inline operator PSECURITY_DESCRIPTOR *() { return &psd; } }; class user_groups {