This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author eryksun
Recipients BreamoreBoy, amaury.forgeotdarc, christian.heimes, eryksun, serhiy.storchaka, tim.golden
Date 2015-05-23.07:25:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1432365943.75.0.0208142654146.issue2528@psf.upfronthosting.co.za>
In-reply-to
Content
In msg243815 you asked me to look over this patch. I hope this helps.

For GetFileSecurity you need to also request LABEL_SECURITY_INFORMATION. To test this in Vista+, use a file in the root directory of the system drive. This will inherit a high integrity level w/ no write up, which denies write access to a medium integrity (non-elevated) process. If you don't include the label information, the check will erroneously claim a non-elevated token has write access. 

You can set a file's integrity level to high using `icacls filename /setintegritylevel High`. This prevents write up. You can prevent read up and execute up by using the Windows API directly, or with other administration tools such as chml.

As a performance tweak you could get the file security with an initial buffer of 512 bytes. That should be big enough for most file security descriptors. I'd use PyMem_RawRealloc in a do loop (i.e. starting with pSD == NULL and cbSD == 512). It should set PyExc_MemoryError on failure.

Regarding ImpersonateSelf, what if the thread is already impersonating? That's the token the kernel will actually use when checking access. Given that, I'd try OpenThreadToken, and only if it fails call ImpersonateSelf. Then RevertToSelf immediately after the 2nd OpenThreadToken call. Reverting doesn't have to be delayed until after the check. An alternative to ImpersonateSelf is to manually duplicate the process token as an impersonation token (i.e. OpenProcessToken; DuplicateToken). The benefit of this approach is that the duplicated token can be cached in a static variable.

For the access check itself, use the FILE_GENERIC_* / FILE_ALL_ACCESS constants, which are the standard and specific rights for the corresponding GENERIC_* rights. These include the standard READ_CONTROL and SYNCHRONIZE rights that are required for accessing files.

Also, there's no need to call MapGenericMask here, since you know that access_desired doesn't contain any generic rights.

Finally, for backward compatibility this new implementation should default to using only the old file-attribute check. Chaining to the ACL-based check should require a keyword-only argument such as "use_acl".
History
Date User Action Args
2015-05-23 07:25:43eryksunsetrecipients: + eryksun, amaury.forgeotdarc, christian.heimes, tim.golden, BreamoreBoy, serhiy.storchaka
2015-05-23 07:25:43eryksunsetmessageid: <1432365943.75.0.0208142654146.issue2528@psf.upfronthosting.co.za>
2015-05-23 07:25:43eryksunlinkissue2528 messages
2015-05-23 07:25:42eryksuncreate