Elevated Commands Redesign

[Nonary]

Fixes

• Remote Access Exploit - Credential Resets #978
• Elevated Do/UNDO Broken in 0.19 (and nightly) #1117

Description

This pull request aims to rework the way commands are executed, specifically on the Windows platform. The main goal is to provide a safe method for users to elevate permissions without being prompted for UAC permissions. By doing this, we also prevent potential privilege exploits that are currently possible in Sunshine due to its existing design.

The core issue with Sunshine is that it allows adding commands without administrator rights, enabling anyone with user-level permissions to reset credentials and run elevated commands without additional safety checks.

Although there is a proof of concept demonstrating that secure administrative rights can be achieved without refactoring a significant portion of the code base, it does not address the fundamental problem. Additionally, it does not provide an option for users to safely disable elevation prompts.

To achieve this goal, we will need to implement several high-level changes, which I will outline below.

Sunshine as a Service is Mandatory

If using the installer, installing Sunshine as a service will no longer be optional. The primary reason for this change is to allow us to securely configure the apps.json file and require elevated permissions to modify it. Please note that this change will not affect portable installations of Sunshine.

All Commands are Un-elevated

As Sunshine runs under the SYSTEM context, it would be unwise to let commands run under this account, as it can lead to issues with certain scripts that need access to the UI on your PC. For instance, if you're trying to create a pre-command and need to swap monitors before the stream starts, this would not work until we execute those commands under the user's profile.

Elevation as an Option

After changing the permissions of the app.json file, we will need administrative access to make modifications to it. This means we can "safely" allow users to mark a command as "elevated." Once that is done, Sunshine will run any command marked with this flag under admin, launching as the current user's profile without any UAC prompts. If the current user's profile does not have admin privileges, then it would execute under the System context instead.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Dependency update (updates to dependencies)
• Documentation update (changes to documentation)
• Repository update (changes to repository files, e.g. .github/...)

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

• I want maintainers to keep my branch updated

[github-actions]

Your PR was set to master, PRs should be sent to nightly.
The base branch of this PR has been automatically changed to nightly.
Please check that there are no merge conflicts

[cgutman]

If the current user's profile does not have admin privileges, then it would execute under the System context instead.

In the "elevation requested" case, I think we should always use the token given by WTSQueryUserToken(). That will be the unrestricted token providing the full privileges of the current user. If that user is not an admin, I don't think we should promote it to admin.

We are considering changing the way WebAPI operates, specifically to prevent it from running everything under the System account, which is a vector for exploits.

However, I would like to gather feedback on how we should handle editing the apps.json file in this scenario. One option is to add a UAC prompt, but this would not be suitable if users are not physically present at their machines.

Any changes we do to potentially move the web service into a separate lower-privileged process should be invisible to users. That process would still communicate with the Sunshine service via some IPC interface that would allow changes to be made to settings without needing elevation.

[ReenigneArcher]

We are considering changing the way WebAPI operates, specifically to prevent it from running everything under the System account, which is a vector for exploits.

What exploits? Or this is just a theoretical statement?

[Nonary]

We are considering changing the way WebAPI operates, specifically to prevent it from running everything under the System account, which is a vector for exploits.

What exploits? Or this is just a theoretical statement?

Not theory, any input received by the web server is vulnerable to exploits if not validated. One vulnerably was fixed in previous pull requests and I'm sure I'll find others once I do a detailed security review over it when I get to that.

Ideally we wouldn't want a web service running as system but I don't have any other solutions that are effective enough to not be any difference to the user experience basically.

[Nonary]

Okay, it's ready for feedback/review

[Nonary]

@cgutman Any other changes needed? Been about two weeks so was just curious

[cgutman]

Almost everything looks good in my testing. I'm seeing a bug though where trying to quit a running app from Moonlight never completes. It works in nightly, so it looks like a regression specific to this PR.

[Nonary]

Almost everything looks good in my testing. I'm seeing a bug though where trying to quit a running app from Moonlight never completes. It works in nightly, so it looks like a regression specific to this PR.

I am not observing this behavior, but I did pull some changes from nightly so maybe it was fixed in nightly? IDK. I am able to quit apps without issues.