Do not require elevation for informative functions for Admin in non-admin shell

[majkinetor]

Since version 0.10.4 choco by default "Run with highestAvailable Execution Level by default". This can be disabled via manifest.

However, its ideal for this to be dynamic. The elevation should happen on install, update or uninstall and not on list, info, version and other informative or config functions.

I propose the very easy method to fix this: by changing the cinst, cup and cuninst wrappers to switch to higestAvailable level and restore it back after the run, so that user can set manifest to asInvoker. The non-wrapped commands would still elevate (or not, as per settings) on each command, but users will have an option this way.

With this in place it could be considered to set defaults to asInvoker.

[ferventcoder]

HighestAvailable Execution (the new setting from #1054) does a good job of only asking for elevation when an administrator is not in an administrative command shell, and only in that scenario. Everything else just continues to work as it normally does.

To be sure it is clear how elevation works in highestAvailable, let's understand the scenarios when elevation will be requested:

• [ ] Admin account in elevated shell
• [ ] Non-administrative account
• [ ] SYSTEM account
• [ ] Built-in administrator account on Server SKUs
• [ x ] Administrator in an non-elevated shell

What about existing automation? Does is need to be changed for highestAvailable?

Most likely all existing automation is likely to already be okay with highestAvailable anyway based on the scenarios above. Most likely you are either using SYSTEM, a non-admin account, or using an admin with an administrative shell so it doesn't get hit by UAC for other things anyway.

Details on how this feature could be implemented per command

Assuming that one can determine when it should request for elevation based on all of the rules and constraints, doing so per command tends to become quite a bit more complex. It's why for #1054 we simply just exposed the app.manifest and set things appropriately, allowing a user to make that decision on what works best for them. But it is an all or nothing kind of approach.

One way to think of how the rules would work to elevate if this was moved forward. Elevate only if all of the following are true:

• User is an administrator in a non-elevated shell
• Not using background mode
• Install is in original locked down location
• We are not calling list, search, info, or push
• User is not calling pack from a locked down location
• Other things I will likely remember as this discussion continues...

Updated as conversation continues...

[ferventcoder]

@majkinetor and what about choco <command>? This is the primary way to interact with choco. An acceptable solution IMHO would work with this.

[ferventcoder]

The original issue that has changed this, along with the current way to change functionality back: #1054

[ferventcoder]

This has been asked to be changed by more than 3 folks, however it is not a simple functionality to implement. It's typically one or the other, but not both. The reason the manifest was pulled out of choco.exe and set beside it was to allow for folks to choose what is best for them, but we set to highestAvailable by default.

[ferventcoder]

The only way I could see to do this is to set wrappers appropriately and have the shim inspect the command being passed to choco to decide if it should change the value.

Note that a non-administrator is not going to have permissions to change the file, and an administrator without UAC also will not have privileges. So either way you get a UAC popup if the application attempts to do so. We won't reduce the security of the files themselves. However we are open to finding the best options here.

[flcdrg]

yeah, i think the idea of elevating just for those commands that require it is good. So if I ran choco install then it would prompt, but if I ran choco list -lo then it would not prompt and just run within the current command window (and as a bonus the results wouldn't disappear when it finished).

[ferventcoder]

@flcdrg I agree it would be beneficial. What I'm hoping to get out of this discussion are steps forward to implement this behavior in a way that is technically possible and feasible.

[ferventcoder]

@majkinetor also, you mentioned this in #912 (comment):

You still need to open elevated prompt. For example my team never works in elevated one and in 95% of the cases choco install fails, then everybody remembers "yeah, admin". So its way inconvenient to do so and fully disabling UAC isn't recommended. Perhaps if choco automatically elevate like normal installers do this feature wouldn't be needed !!!

So #1054 fixed that problem, but in doing so it gave you a choice of choosing one way or another.

[flcdrg]

So would an approach like https://stackoverflow.com/a/10905713/25702 be suitable? Or would you need to launch a child process elevated and wait for that to finish before the parent process exited?

[RichiCoder1]

@flcdrg That is basically how ChocolateyGUI works. Should in theory be simpler to implement for choco.
Edit: specifically shelling out to an independently elevated process. Definitely don't need any of Gui's RPC madness :P

Edit: Edit: Here be dragons warning. Assuming you just run pass args straight to an elevated choco process, you might still run into issues as I believe there's some quirks when redirecting stdout from an elevated process to an unelevated one on windows.

Edit: Edit: Edit: Go upvote PowerShell/PowerShell#3232 so we get sudo and stuff like this isn't as painful in the first place :)

[flcdrg]

I'm guessing the parent would need to wait for the child so that things continued to work as expected when you're calling choco in a script and expect execution to happen synchronously.

[RichiCoder1]

Yup. A simplish idea would to have the choco process check whether it's A) in an elevated process and B) whether the command in question is non-informative (or destructive as I call it). If no the to the former and yes to the latter, spin up a new process with runas, pass the appriopriate parameters verbatim (with maybe some additional ones so that the process knows it's a child), pipe the stdout and stderr back, and then just wait for the child process to finish executing (and set some arbitrary long timeout).

[ferventcoder]

Keep in mind that not always would choco elevate - it depends on some factors of where the Chocolatey installation is, whether or not background mode is enabled or not (which allows non-admins to call choco install and have it redirected through a background service).

HighestAvailable does a good job of only asking for that elevation when an administrator is not in an administrative command shell, and that's exactly how it should behave otherwise.

Assuming that one can determine when it should request for elevation based on all of the rules and constraints, this issue gets to be quite a bit more complex. It's why for #1054 we simply just exposed the app.manifest and set things appropriately, allowing a user to make that decision on what works best for them.

Details on how this feature could be implemented

One way to think of how the rules would work to elevate if this was moved forward. Elevate only if all of the following are true:

• User is an administrator in a non-elevated shell
• Not using background mode
• Install is in original locked down location
• We are not calling list, search, info, or push
• User is not calling pack from a locked down location
• Other things I will likely remember as this discussion continues...

[majkinetor]

• We are not calling list, search, info, or push

and new, version

@majkinetor and what about choco ? This is the primary way to interact with choco.

I noted that The non-wrapped commands would still elevate. Clearly suboptimal but way better then nothing.

Note that a non-administrator is not going to have permissions to change the file, and an administrator without UAC also will not have privileges.

Yeah, forgot about this, so wrapper commands idea can't be used the way I describe it, the wrappers wold need to elevate themselves which is similar to what @RichiCoder1
mentions at #1307 (comment)

[ferventcoder]

and new, version

👍
And any others like download, support, outdated, source list (but not source change, or other config views).

Just of note, version has been deprecated and will be removed.

I noted that The non-wrapped commands would still elevate. Clearly suboptimal but way better then nothing.

This would be surprising behavior, which means it is unlikely to be added.

Likely the only way it could be done is to run a process to elevate when the right rules are in place.

[ferventcoder]

Likely the only way it could be done is to run a process to elevate when the right rules are in place.

And by that, each command already knows whether it should run with elevated context (although a bit naive) or not, so if we can find a good way to elevate first, then I'm good with pursuing this.

I do prefer asInvoker, but I also want choco to elevate automatically. However I do want to ensure the solution is feasible and elegant.

[mwallner]

This may sound a bit naive, but following could work:

All commands elevate as soon as they are called, but there might be a switch --dontelevate.
With this switch, a user could use informative functions that do not require administrative privileges such as list, info ...

With this, all existing/out-there automation around choco would still work, and I wouldn't need to change a bit at work 😂

[ferventcoder]

With this, all existing/out-there automation around choco would still work, and I wouldn't need to change a bit at work

I'm going to assume all existing automation is likely to already be okay with highestAvailable anyway based on the scenarios I laid out above where elevation is requested (https://github.com/chocolatey/choco/issues/1307#issuecomment-303284220). Most likely you are either using SYSTEM, a non-admin account, or using an admin with an administrative shell so it doesn't get hit by UAC for other things anyway.

[mwallner]

Most likely you are either using SYSTEM, a non-admin account, or using an admin with an administrative shell so it doesn't get hit by UAC for other things anyway.

Agreed.

[jberezanski]

I'm going to assume all existing automation is likely to already be okay with highestAvailable anyway

My specific automation involves a scheduled task configured to run in the context of the interactive user, who usually is a member of Administrators. The task essentially invokes choco outdated, so it does not need and should not be elevated. This got broken in 0.10.4 (stops on the UAC prompt).

[ferventcoder]

"likely" ;)

[ferventcoder]

@jberezanski We also had a conversation about this and agreed it was a reasonable compromise at #1054 (comment) - although it occurs to me that you may just be reiterating that conversation here.

[jberezanski]

@ferventcoder Well, back then I wrote "Hopefully one day Choco will get smart enough to ask for elevation only for actions which require it." - that's why I'm declaring my support for this issue. Also, since that time I've realized this affects me not only in interactive, ad-hoc usage, but also in the automation scenario described above.

I like the idea outlined by @RichiCoder1. Redirecting input/output from the child process will not be trivial, however, because the typical way to elevate (Process.Start with UseShellExecute=true and Verb="runas") does not support stdin/stdout/stderr redirection, so it will require implementing a custom communication mechanism, using e.g. named pipes (a problem already solved by various installer frameworks, by the way).

[ferventcoder]

@jberezanski If you have specific examples for implementation or links, that would be most helpful. Whatever option it is has to be rock solid as we would have a very low tolerance for failures, plus it would need to be able to work in hundreds of diverse Windows environments. As long as those things can be accomplished, I'm 👍.

I do know Windows Installer (msiexec) has this figured out and elevates automatically later in the process of installing.

If named pipes is a way forward, keep in mind we are doing something like this with the Chocolatey Agent already and have seen it run into conflicts with other tools also using named pipes (having an elevated process means all listeners using named pipes have to run elevated). Example: "choco agent is running as a service and uses a net.pipe listener, that listener runs in an elevated security context. So ANY application that leverages the net.pipe listener will ALSO have to run in an elevated security context."

We have not determined the full validity of this aspect yet, but this could have implications running both choco.exe and the chocolatey agent service, as choco.exe would be a non-elevated listener. So we'd need to determine if it is feasible first.

[ferventcoder]

We have not determined the full validity of this aspect yet, but this could have implications running both choco.exe and the chocolatey agent service, as choco.exe would be a non-elevated listener. So we'd need to determine if it is feasible first.

I'm going to know how this turns out pretty quickly as we get the next version of the ChocolateyGUI working as it just moved to named pipes.

[jberezanski]

No, I don't have any specific implementation examples to show (yet). I had done IPC over named pipes in .NET 2.0, but it was over 10 years ago and the code was proprietary anyway.

The problems you describe with the Chocolatey Agent surprise me. At present job, I have a production application consisting of multiple executables, some of them expose WCF endpoints over net.pipe. One exec is a NT service which runs as LocalSystem, another NT service runs as a low-privileged account (a virtual service account, to be precise), yet another exec is a worker process started on-demand under various security contexts. There have never been any conflicts between them (or with third-party apps) with regard to named pipe listeners (but care is taken to ensure uniqueness of pipe names). The application runs on OSes ranging from newest Win10 to XP.

In this case, however, I wouldn't use the messaging infrastructure of WCF/net.pipe, just simple text over NamedPipeServerStream/NamedPipeClientStream. The pipe names should be autogenerated (guids, for instance) and passed to the child process as command line arguments. The pipe creator (the low-priv parent process) should set the appropriate access rights to maximize security (the exact set is to be determined). I'll think about it some more and probably write a POC.

[ferventcoder]

There have never been any conflicts between them (or with third-party apps) with regard to named pipe listeners (but care is taken to ensure uniqueness of pipe names). The application runs on OSes ranging from newest Win10 to XP.

Surprising for us as well as the name isn't shared.

[RichiCoder1]

GUI actually uses WCF over named pipes to do IPC to it's subprocess, and I don't believe we've run into any pipe security issues with GUI being no being elevated and Subprocess being elevated. It must be a configurable knob,

[ferventcoder]

@RichiCoder1 the report we had was about listeners, so it would only be the GUI, not the subprocess. Again, we haven't had time to dive in to really investigate just yet.

[ferventcoder]

Maybe the right answer is to go back to "asInvoker" until we can find a good answer here?

[ferventcoder]

We are moving 0.10.7 back while we discuss a better way to handle this - see #1324 for details.

[jberezanski]

I've started on the POC, but it's going very slowly due to my acute lack of free time.
One thing I'm thinking about is the level at which I/O redirection should take place. Simple redirection of stdin/stdout/stderr is quite easy to implement and the least invasive (no need to modify the existing application code), but we lose console features such as coloring. Perhaps we could exploit the fact that Chocolatey has its own Console abstraction - conditionally subsitute the standard implementation with one that talks with the parent process over WCF/net.pipe...

[AeliusSaionji]

This may be the best place for my bug report. Let me know if not.

I encountered a chocolatey package which has exposed a small issue relating to this. The package installs to $ChocolateyInstall when it really belongs in C:\tools (the maintainer is open to a PR and I will write it when I get the chance). By the nature of the package, its files are subject to external tools and user fiddling, so invariably the ownership and permissions of the files will be dirty.

The issue this exposed is that all nonadmin functions of chocolatey can apparently be broken because of a sanity check that doesn't seem strictly necessary?

Of course I run cup all in an elevated adminuser shell where I managed to not notice this for about a week. The elevated user of course can have whatever files it wants, so this issue just never happened when elevated.
But outside of the elevated shell, nothing works! Not even just choco. This pained me when trying to do something as simple as cpack, a simple maintainer action that only ever operates in the working directory. As you can see, a permission issue quite unrelated to the current task just prevented me from zipping up files into a nupkg in my own user folder.

PS C:\Users\aelius\chocopkgs\obs-studio.install> choco pack -d
Chocolatey v0.10.15
Chocolatey is running on Windows v 10.0.18362.0
Command line: "C:\ProgramData\chocolatey\choco.exe" pack -d
Received arguments: pack -d
Chocolatey had an error occur:
System.UnauthorizedAccessException: Access to the path 'C:\ProgramData\chocolatey\lib\cemu\tools\cemu\graphicPacks\BCML\0100_WearableLanternBrighterLightsDarkerNights' is denied.
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.FileSystemEnumerableIterator`1.AddSearchableDirsToStack(SearchData localSearchData)
   at System.IO.FileSystemEnumerableIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at chocolatey.infrastructure.app.tasks.RemovePendingPackagesTask.handle_message(PreRunMessage message)
   at System.Reactive.AnonymousSafeObserver`1.OnNext(T value)
   at System.Reactive.Linq.Observαble.Where`1._.OnNext(TSource value)
   at System.Reactive.Linq.Observαble.OfType`2._.OnNext(TSource value)
   at chocolatey.infrastructure.events.EventManager.publish[Event](Event message)
   at chocolatey.infrastructure.app.runners.GenericRunner.run(ChocolateyConfiguration config, Container container, Boolean isConsole, Action`1 parseArgs)
   at chocolatey.infrastructure.app.runners.ConsoleApplication.run(String[] args, ChocolateyConfiguration config, Container container)
   at chocolatey.console.Program.Main(String[] args)
Exiting with 1

There was some trouble in gitter where one or two people failed to grasp that this is even an issue, so I will attempt to make this clearer here:

• Yes I know how to edit the permissions to "fix" this. However, for as long as this package remains in $ChocolateyInstall, this will just happen again as I use the package in question.
• Yes I know running choco in an elevated admin shell will work around this problem. I consider it suboptimal to insist nonadmin functions be run as admin...

I don't really know why this sanity check is run in any context and as such I am not in a position to suggest how to fix this. I can only suggest that perhaps this particular sanity check should not be show-stopping for unrelated functions.

[TheCakeIsNaOH]

@AeliusSaionji see #1470, which should fix that issue, at least for pack