winget-cli icon indicating copy to clipboard operation
winget-cli copied to clipboard

Inno setup incompatible with winget when using PrivilegesRequired

Open dmex opened this issue 5 years ago • 6 comments

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 avatar May 20 '20 19:05 dmex

Related to #189

dmex avatar May 20 '20 20:05 dmex

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

dmex avatar May 21 '20 18:05 dmex

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!

wjk avatar Jan 10 '21 20:01 wjk

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.

doctordns avatar Jan 11 '21 10:01 doctordns

@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.

wjk avatar Jan 11 '21 13:01 wjk

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.

denelon avatar Apr 20 '21 21:04 denelon