microsoft / winget-cli

WinGet is the Windows Package Manager. This project includes a CLI (Command Line Interface), PowerShell modules, and a COM (Component Object Model) API (Application Programming Interface).
https://learn.microsoft.com/windows/package-manager/
MIT License
23.4k stars 1.46k forks source link

Inno setup incompatible with winget when using PrivilegesRequired #254

Open dmex opened 4 years ago

dmex commented 4 years ago

winget does not properly support Inno setup installers due the LowIL of the binary before they're executed.

Inno setup installers use the PrivilegesRequired directive for requesting UAC elevation which is implemented internally by Inno Setup with ShellExecuteEx and the RunAs verb: https://jrsoftware.org/ishelp/index.php?topic=setup_privilegesrequired

AppInstallerCLI downloads binaries into the WinGet directory and it's inheriting both directory and file permissions from the %localappdata%\app_container_name\TempState\ directory which includes the Low integrity level RID: https://i.imgur.com/MBTzvkr.png

ShellExecuteEx doesn't allow processes to use the RunAs verb for binaries with LowIL and this prevents winget from properly executing Inno setup installers.

The InstallCommand::ExecuteInternal function needs to include an extra step which removes the RID for the downloaded binary so that ShellExecute will correctly use the integrity level of the calling process token instead of the integrity level of the file.

VerifyInstallerHash RemoveFileIL <-- winget needs this before calling ShellExecute ExecuteInstaller

For this code: https://github.com/microsoft/winget-cli/blob/4f73d7cfff75d478176cf31d4abad0b8b5ff8dd4/src/AppInstallerCLICore/Commands/InstallCommand.cpp#L62-L65

Alternatively winget should use a directory which doesn't inherit LowIL permissions. (Edit: See second github post below for more information)

You can also reproduce the incorrect behaviour by navigating to the \TempState\ directory and manually executing the Inno setup installers via Windows Explorer which also fails due to winget not removing the LowIL from the file: image

Expected behavior

winget install vlc - successfully shows elevation prompt

Actual behavior

winget install processhacker - elevation prompt failure (see pull request 373 on winget-pkgs repro)

Environment

Windows: Windows.Desktop v10.0.19041.264 Package: Microsoft.DesktopAppInstaller v1.0.41331.0

dmex commented 4 years ago

Related to #189

dmex commented 4 years ago

Alternatively winget should use a directory which doesn't inherit LowIL permissions.

Expanding on this point. Winget already supports using an alternate cache directory with already has the correct permissions but the current checks are incorrect since the winget store package has the runFullTrust capability.

AppInstaller::Runtime::GetPathToTemp() function checks if the process has 'identity' by calling IsRunningInPackagedContext() here:

https://github.com/microsoft/winget-cli/blob/5598f7f6a1399e7b0d09255042563ea7058e3b1e/src/AppInstallerCommonCore/Runtime.cpp#L259-L268

The function queries the current process package using GetPackageFamilyName which internally uses the "WIN://SYSAPPID" token attribute:

https://github.com/microsoft/winget-cli/blob/5598f7f6a1399e7b0d09255042563ea7058e3b1e/src/AppInstallerCommonCore/Runtime.cpp#L22-L24

winget does not execute with AppContainer restrictions since its using the runFullTrust capability and should instead be using GetTempPathW since it doesn't inherit LowIL permissions.

The AppInstaller::Runtime::GetPathToTemp() function should be calling GetTokenInformation(GetCurrentProcessToken(), TokenIsAppContainer) and use GetTempPathW when TokenIsAppContainer==0

wjk commented 3 years ago

I’m seeing this with my Burn-based installer too. It will only install if I run winget from an elevated command prompt. Otherwise, it dies with the HRESULT for Access Denied. A solution would be appreciated. Thanks!

doctordns commented 3 years ago

I’m seeing this with my Burn-based installer too. It will only install if I run winget from an elevated command prompt. Otherwise, it dies with the HRESULT for Access Denied. A solution would be appreciated. Thanks!

Isn't the solution just to use an elevated powershel/cmd prompt assuming you are allowed? In my experience, installers need elevated permissions to write to protected registry areas or protected folders. Winget should not be driving a coach and horses through enterprise security.

wjk commented 3 years ago

@doctordns I’ve seen winget work from from a normal command prompt many times; the installer being run calls for elevation itself. I don’t know why some packages can elevate themselves, and others require the caller to be elevated.

denelon commented 3 years ago

There are several different features related to required installer privileges and UAC prompts: https://github.com/microsoft/winget-cli/issues/218 https://github.com/microsoft/winget-cli/issues/271 https://github.com/microsoft/winget-cli/issues/281

We will need to look at adding this as a feature for Inno installers (and possibly other installer types). So the client can be properly informed about the required permissions, and the user can be informed as well.