microsoft / perfview

PerfView is a CPU and memory performance-analysis tool
http://channel9.msdn.com/Series/PerfView-Tutorial
MIT License
4.2k stars 712 forks source link

PerfView should run the debugging application as a normal user and not as an elevated (admin) user #135

Open CoderParPlaisir opened 8 years ago

CoderParPlaisir commented 8 years ago

Hello

When we run an application with PerfView (clicking on "Run a command", or by the menus "Collect" / "Run" (Alt-R), or by command line "run"), PerfView elevates itself then run the application as elevated too. That is a security threat, and it prevents debugging Window Store applications (they fail running, see https://social.msdn.microsoft.com/Forums/en-US/41170c0f-405c-45d8-abcd-b7a376c70c48/failure-starting-process-in-perview-with-windows-10-universal-application?forum=wpdevelop).

PerfView, even elevated, should run the application as the normal non-elevated user.

It is obviously feasible as said here: https://blogs.msdn.microsoft.com/winsdk/2013/06/17/launching-a-process-as-a-normal-user-from-an-elevated-user/ It is even easier in PerfView as when PerfView starts non-elevated, it knows who is the normal user and it can collect the non-elevated token and pass it to the elevated PerfView process.

Thank you.

Tested with PerfView 1.9.0.0

TEST:

IMPACT:

vancem commented 8 years ago

I am assuming everyone understands the obvious work-around of simply using the 'Collect' rather than the 'Run' command. If you start PerfView's collection you can then launch your app however you wish, (and in particular non-elevated). This should not block investigations, and thus is a convenience/security feature.

This feature is a bit more complex because you probably want to expose the ability to run the app elevated as well as non-elevated (non-elevated can be the default).

I am generally supportive of this feature. If anyone in the community wishes to tackle it I can provide advice about the details necessary to get it done.

ppozeti commented 6 years ago

Hey @vancem, What about if we add a checkbox next to the Command text box (RunCommandDialog.xml) and if is checked then the process is started elevated.

vancem commented 6 years ago

@ppozeti - Sure having a checkbox to choose is reasonable. The solution is straightfoward, the issue is the priority. Because it has such a simple work-around it has low priority. If anyone wants to take a crack at it by all means do so.

ppozeti commented 6 years ago

Well, can I do this? I'm just starting at contributing to open source projects and I thought that would be a good start. Could you assign this to me?

vancem commented 6 years ago

GitHub won't let me actually set the assigned to field to you, but this note indicate that others should not work on it without interacting here.

brianrob commented 5 years ago

@ppozeti is this something that you're still interested in working on?

ppozeti commented 5 years ago

@ppozeti is this something that you're still interested in working on?

Sure!

brianrob commented 5 years ago

Excellent. Then I'll keep this issue around. Do post if you have any questions or run into issues. Thanks.

HMartyrossian commented 4 years ago

@ppozeti Paulo, I'm assuming that you will address this issue for Windows only, right?

windhandel commented 3 weeks ago

Hi @brianrob ,

Above, Vance seems to be indicating that Collect did not require escalation in the past (and that it would block investigations if it were to), but now it seems to be required. Is that the case? When and why did that begin? Is there a way to navigate around this requirement? Thanks for the help.

brianrob commented 3 weeks ago

@windhandel, collection of kernel events including CPU samples has always required administrative privileges, and last I spoke with the ETW folks, there is no plan to change this. I think Vance's point above is that it's reasonable to not want to run the application being profiled as an admin if you are using the PerfView.exe run command.

pharring commented 2 weeks ago

I think you can use runas /trustlevel:0x20000 yourapp.exe on the run command line in PerfView. I'm not sure if it's exactly the same as "non-elevated", but from experience, it was good enough for my use case when I was working on Visual Studio startup performance many (15?) years ago.