From ec6d07e326a8d203830b735b3cb1998977fc3674 Mon Sep 17 00:00:00 2001 From: corinna Date: Mon, 16 Oct 2006 12:26:58 +0000 Subject: [PATCH] * autoload.cc (PrivilegeCheck): Define. * fhandler.cc (fhandler_base::open): Always try opening with backup resp. restore intent. * fhandler_disk_file.cc (fhandler_disk_file::opendir): Always try opening with backup intent. (fhandler_disk_file::readdir): Ditto when trying to retrieve file id explicitely. * security.cc (check_file_access): Replace pbuf with correctly PPRIVILEGE_SET typed pset. Check explicitely for backup and/or restore privileges when AccessCheck fails, to circumvent AccessCheck shortcoming. Add comment to explain. --- winsup/cygwin/ChangeLog | 14 ++++++++++++ winsup/cygwin/autoload.cc | 1 + winsup/cygwin/fhandler.cc | 17 ++++++++++---- winsup/cygwin/fhandler_disk_file.cc | 3 ++- winsup/cygwin/security.cc | 45 +++++++++++++++++++++++++++++++++---- 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 0d4763eea0..5cd520347a 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,17 @@ +2006-10-13 Corinn6 Vinschen + + * autoload.cc (PrivilegeCheck): Define. + * fhandler.cc (fhandler_base::open): Always try opening with backup + resp. restore intent. + * fhandler_disk_file.cc (fhandler_disk_file::opendir): Always try + opening with backup intent. + (fhandler_disk_file::readdir): Ditto when trying to retrieve file id + explicitely. + * security.cc (check_file_access): Replace pbuf with correctly + PPRIVILEGE_SET typed pset. Check explicitely for backup and/or restore + privileges when AccessCheck fails, to circumvent AccessCheck + shortcoming. Add comment to explain. + 2006-10-13 Christopher Faylor * winsup.h: Turn off DEBUGGING. diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc index 63a83d80aa..738d832e26 100644 --- a/winsup/cygwin/autoload.cc +++ b/winsup/cygwin/autoload.cc @@ -347,6 +347,7 @@ LoadDLLfunc (LsaQueryInformationPolicy, 12, advapi32) LoadDLLfunc (MakeSelfRelativeSD, 12, advapi32) LoadDLLfunc (OpenProcessToken, 12, advapi32) LoadDLLfunc (OpenThreadToken, 16, advapi32) +LoadDLLfunc (PrivilegeCheck, 12, advapi32) // LoadDLLfunc (RegCloseKey, 4, advapi32) LoadDLLfunc (RegCreateKeyExA, 36, advapi32) LoadDLLfunc (RegDeleteKeyA, 8, advapi32) diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc index 00e9875873..672b434e59 100644 --- a/winsup/cygwin/fhandler.cc +++ b/winsup/cygwin/fhandler.cc @@ -620,13 +620,22 @@ fhandler_base::open (int flags, mode_t mode) create_options = FILE_OPEN_FOR_BACKUP_INTENT | FILE_OPEN_FOR_RECOVERY; break; default: - create_options = 0; if ((flags & O_ACCMODE) == O_RDONLY) - access = GENERIC_READ; + { + access = GENERIC_READ; + create_options = FILE_OPEN_FOR_BACKUP_INTENT; + } else if ((flags & O_ACCMODE) == O_WRONLY) - access = GENERIC_WRITE | FILE_READ_ATTRIBUTES; + { + access = GENERIC_WRITE | FILE_READ_ATTRIBUTES; + create_options = FILE_OPEN_FOR_RECOVERY; + } else - access = GENERIC_READ | GENERIC_WRITE; + { + access = GENERIC_READ | GENERIC_WRITE; + create_options = FILE_OPEN_FOR_BACKUP_INTENT + | FILE_OPEN_FOR_RECOVERY; + } if (flags & O_SYNC) create_options |= FILE_WRITE_THROUGH; if (flags & O_DIRECT) diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index ae2c90d1f0..66f488b30c 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -1606,6 +1606,7 @@ fhandler_disk_file::opendir () SYNCHRONIZE | FILE_LIST_DIRECTORY, &attr, &io, wincap.shared (), FILE_SYNCHRONOUS_IO_NONALERT + | FILE_OPEN_FOR_BACKUP_INTENT | FILE_DIRECTORY_FILE); if (!NT_SUCCESS (status)) { @@ -1869,7 +1870,7 @@ go_ahead: InitializeObjectAttributes (&attr, &upath, OBJ_CASE_INSENSITIVE, dir->__handle , NULL); if (!NtOpenFile (&hdl, READ_CONTROL, &attr, &io, - wincap.shared (), 0)) + wincap.shared (), FILE_OPEN_FOR_BACKUP_INTENT)) { de->d_ino = readdir_get_ino_by_handle (hdl); CloseHandle (hdl); diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index a0bcba33b0..5484b7028a 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -1953,8 +1953,9 @@ check_file_access (const char *fn, int flags) HANDLE hToken; BOOL status; - char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof (LUID_AND_ATTRIBUTES)]; - DWORD desired = 0, granted, plength = sizeof pbuf; + DWORD desired = 0, granted; + DWORD plength = sizeof (PRIVILEGE_SET) + 3 * sizeof (LUID_AND_ATTRIBUTES); + PPRIVILEGE_SET pset = (PPRIVILEGE_SET) alloca (plength); static GENERIC_MAPPING NO_COPY mapping = { FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_GENERIC_EXECUTE, @@ -1974,10 +1975,46 @@ check_file_access (const char *fn, int flags) if (flags & X_OK) desired |= FILE_EXECUTE; if (!AccessCheck (sd, hToken, desired, &mapping, - (PPRIVILEGE_SET) pbuf, &plength, &granted, &status)) + pset, &plength, &granted, &status)) __seterrno (); else if (!status) - set_errno (EACCES); + { + /* CV, 2006-10-16: Now, that's really weird. Imagine a user who has no + standard access to a file, but who has backup and restore privileges + and these privileges are enabled in the access token. One would + expect that the AccessCheck function takes this into consideration + when returning the access status. Otherwise, why bother with the + pset parameter, right? + But not so. AccessCheck actually returns a status of "false" here, + even though opening a file with backup resp. restore intent + naturally succeeds for this user. This definitely spoils the results + of access(2) for administrative users or the SYSTEM account. So, in + case the access check fails, another check against the user's + backup/restore privileges has to be made. Sigh. */ + int granted_flags = 0; + if (flags & R_OK) + { + pset->PrivilegeCount = 1; + pset->Control = 0; + pset->Privilege[0].Luid = *privilege_luid (SE_BACKUP_PRIV); + pset->Privilege[0].Attributes = 0; + if (PrivilegeCheck (hToken, pset, &status) && status) + granted_flags |= R_OK; + } + if (flags & W_OK) + { + pset->PrivilegeCount = 1; + pset->Control = 0; + pset->Privilege[0].Luid = *privilege_luid (SE_RESTORE_PRIV); + pset->Privilege[0].Attributes = 0; + if (PrivilegeCheck (hToken, pset, &status) && status) + granted_flags |= W_OK; + } + if (granted_flags == flags) + ret = 0; + else + set_errno (EACCES); + } else ret = 0; done: -- 2.11.0