1481635 - Launcher process improvements for automation and UAC-disabled configurations

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[(No longer employed by Mozilla) Aaron Klotz]

There are a couple of cases that we need to handle for automation and general correctness. Since they are closely intermingled, I'm going to land this stuff in one bug.

1) The firefox process launched by automation is assumed to stay alive for the lifetime of the browsing session. I'm adding a command-line option that makes this happen, as well as an environment variable check (so that hopefully we don't need to go around modifying test harnesses all over the place).

2) I'm going to add a command-line option to prevent the launcher de-elevating the browser. Again, this is in case of potential automation issues.

3) If UAC is disabled and we're started at high integrity, I think that the current elevation code will cause an infinite loop -- the launcher will continuously restart itself using explorer, but explorer will never be a medium integrity user.

To fix this, I'm going to use a combination of TokenElevationType and TokenIntegrityLevel queries on the current token to determine UAC and integrity level.

If the token elevation type is TokenElevationTypeDefault, then UAC is disabled and we need to fall back to checking our integrity level.

Note that if we don't have UAC, the only way that we can drop our integrity level is by duplicating our token and dropping the IL there, then calling CreateProcessAsUser. We also need to use this method if the --wait-for-browser is specified, since we don't get a child process handle out of the remote ShellExecute call made via Explorer.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch

See the bug description for more about what's going on with this patch.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch (r2)

This rev propagates the browser's return code as the launcher's return code when --wait-for-browser is set.

Comment 3

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8998370 [details] [diff] [review]
Patch (r2)

Review of attachment 8998370 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/winlauncher/LaunchUnelevated.cpp
@@ +228,3 @@
>  
> +      return Some(ElevationState::eElevated);
> +    default:

This is really a nit, but I'd be happier with an explicit check for TokenElevationTypeDefault and some sort of error handling for an unknown value, just for a little forward compatibility.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e3106a824d27f327e5dd9178f7e00aa8703f731
Bug 1481635: Add option to allow the launcher process to wait for the browser before terminating, while properly handling UAC or high-integrity cases; r=mhowell

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9e3106a824d2