* 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.
This commit is contained in:
Corinna Vinschen 2006-10-16 12:26:59 +00:00
parent 7af26e0cc0
commit 2c1ffdbf5e
5 changed files with 71 additions and 9 deletions

View File

@ -1,3 +1,17 @@
2006-10-13 Corinn6 Vinschen <corinna@vinschen.de>
* 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 <cgf@timesys.com>
* winsup.h: Turn off DEBUGGING.

View File

@ -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)

View File

@ -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)

View File

@ -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);

View File

@ -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: