OpenProcess : change logic when requesting PROCESS_QUERY_INFORMATION which fails for protected processes (win 8+)

[EccoTheFlintstone]

• OpenProcess : change logic when requesting PROCESS_QUERY_INFORMATION which fails for protected processes (win 8+)
• change rights requested in some functions (that fail on win 8+ on protected processes such as csrss.exe, smss.exe, ..)

without this change, getting some fields (username(), nice(), ..) on protected processes fails due to requesting too much rights (PROCESS_QUERY_INFORMATION instead of PROCESS_QUERY_LIMITED_INFORMATION)

[giampaolo]

Uhm... this may be very good in principle as it would allow collecting info for more processes which right now raise AccessDenied. In practice I am a bit wary to force PROCESS_QUERY_LIMITED_INFORMATION so generally and indiscriminately. My fear is that OpenProcess + PROCESS_QUERY_LIMITED_INFORMATION may succeed but we'll get an error later on, when the handle returned by OpenProcess will be passed to another syscall. What I'm afraid of specifically is that a "broken" syscall using the process handle may not return ERROR_ACCESS_DENIED as originally intended by OpenProcess but something different. So basically what I'm saying is that the desired access bit should be handled case by case.

Note: could you please limit lines length to 80 chars per line? Thanks.

[giampaolo]

I'm experimenting with this. Here's my findings. I wrote a script which calls psutil_proc_cpu_times() directly against all process PIDs:

• by using PROCESS_QUERY_INFORMATION | PROCESS_VM_READ (which is the current default) I get 100 AccessDenied exceptions
• by using PROCESS_QUERY_INFORMATION I get 98 AD exceptions
• by using PROCESS_QUERY_LIMITED_INFORMATION I get 96 AD exceptions

...so it seems using PROCESS_QUERY_LIMITED_INFORMATION is desirable and I think it should be the default for psutil_handle_from_pid() function. Other APIs needing higher access privileges can use psutil_handle_from_pid_waccess() but that has to be handled and verified case by case by checking the syscall official documentation. For instance, Process' cmdline(), cwd() and environ() require PROCESS_QUERY_INFORMATION | PROCESS_VM_READ.

[EccoTheFlintstone]

Note: could you please limit lines length to 80 chars per line? Thanks.
Done

As per your concerns, from my experience, the various checks are performed only when getting a handle with specific rights, not when actually getting the data from a function or syscall.

On a "normal" process, OpenProcess + PROCESS_QUERY_INFORMATION would succeed anyway if we have sufficient permissions. The subsequent function calls would succeed too.

On protected processes, OpenProcess + PROCESS_QUERY_INFORMATION fails, OpenProcess + PROCESS_QUERY_LIMITED_INFORMATION might succeed if we have enough permissions. Some of the subsequent function calls would succeed too (I've lookup up in the MSDN lots of the function calls performed after getting a handle with those rights and corrected them accordingly. A lot of functions only need PROCESS_QUERY_LIMITED_INFORMATION. if it requires more (such as PROCESS_VM_READ), specifying PROCESS_QUERY_INFORMATION or PROCESS_QUERY_LIMITED_INFORMATION wouldn't help as the PROCESS_VM_READ part is forbidden.

The "fallback" mechanism written here is only meant for those situations and is normally triggered only for protected processes

PROCESS_QUERY_INFORMATION is more of a windows XP right, PROCESS_QUERY_LIMITED_INFORMATION a win vista+ one.

You can get details here : https://docs.microsoft.com/en-us/windows/desktop/procthread/process-security-and-access-rights

But this page can be misleading : I succeeded in opening csrss.exe process on a win10 x64 machine using PROCESS_QUERY_LIMITED_INFORMATION and then call OpenProcessToken to retrieve the token (and be able to get the username of the running process).

[giampaolo]

I opted making a brand new PR (#1376) which:

• uses PROCESS_QUERY_LIMITED_INFORMATION wherever possible
• get rid of psutil_handle_from_pid_waccess() function
• only expose a psutil_handle_from_pid() function with a mandatory dwDesiredAccess argument

I preferred to do it this way so that it is clear what access right is being used in every function.
I'm gonna close this PR but thanks a lot for exposing this issue.

[giampaolo]

I integrated your changes regarding the free()ing of resources in 790292d.