1793969 - Add LPAC access to the application directory created by the installer.

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Bob Owen (:bobowen) PTO until 8th Oct]

Processes running inside a Low Privileged Application Container (LPAC) will require access to our application files. In particular the binary files.

To achieve this we need to grant a specially derived (using DeriveCapabilitySidsFromName) SID read and execute access.

Comment 1

[Bob Owen (:bobowen) PTO until 8th Oct]

Attachment: WIP Bug 1793969: Add LPAC access to the application directory created by the installer.

Hi both,
For running Firefox in an LPAC, we need to grant permissions to the install dir for a specific SID.
We then add this SID to the LPAC profile.

I've come up with a very rough patch that seems to work for the basic full installer, as an example.
Of course, we need to consider multiple other things (updater, stub, background updater, i18n, <other things I don't know about>), so just asking for feedback to get an idea about the extent of this.

Comment 2

[Molly Howell (she/her) (mostly inactive)]

This code itself seems reasonable, but (as I'm betting you've guessed) there are some other considerations. Off the top of my head:

• We'll need to invoke this code during installations, either new or paveover installations, and also during updates. I'd imagine that having this run in the same places we register the WER module would be a good example.
• I don't think we can do this at all for MSIX installations, or at least I'm not sure how we could. I don't understand enough about the requirements for this thing (even after reading the design docs in the metabug) to know if that's a serious problem.
• We'll need to decide how to handle instances that no installer was ever run for, like "portable" apps or copies that had their files just extracted directly into place, where we won't be able to assume within Firefox that this permission is already there. Options for that would include just doing nothing and accepting that it won't work, moving this code completely out of the installer and into Firefox (I'm not sure if this is possible), or additionally supporting adding the permissions from within Firefox (which doesn't necessarily mean writing a second implementation of the same thing, we already have a few bits of NSIS code that we invoke from within Firefox for a few different purposes). I don't have enough information to have an opinion about which to pick, but there should be a decision made.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Molly Howell (she/her) [:mhowell] from comment #2)

This code itself seems reasonable, but (as I'm betting you've guessed) there are some other considerations. Off the top of my head:

• I don't think we can do this at all for MSIX installations, or at least I'm not sure how we could. I don't understand enough about the requirements for this thing (even after reading the design docs in the metabug) to know if that's a serious problem.

We certainly don't have hooks to run code at MSIX install time, and I would be surprised if these APIs would work when running, eg: at first run. Regular users have little to no permissions on MSIX install directories.

I had a look through the AppxManifest.xml options to see if there's anything we can include there that might help. There's two things that may help, although I think both of them are long shots:

• InstallActions allow for a provided exe or msi installer to run at first launch. The detailed documentation on this further states it will run as admin and throw a UAC prompt. What this means for non-admin users is unclear. It's also not clear whether or not the provided exe/msi will run with high enough privileges to modify the DACL of the MSIX install directory. This option also requires a special capability that they claim is quite restricted - it would require approval from Microsoft, and it's not clear we could get it.
• MutablePackageDirectory allows for us to specify a secondary directory that is associated with the package that is user modifiable (so presumably Firefox code could modify the DACL). This is meant for things like game mods, but if doing something like copying a particular file or files to it at runtime might help, it could be considered. (I suspect you really really need the actually install dir to have its DACL modified, but I figured I'd drop this here on the off chance it might help.)

Comment 4

[Robin Steuber (she/her) [:bytesized]]

I'm not sure that there is much for me to add here. AFAICT :mhowell and :bhearsum seem to have covered all of the bases.

Comment 5

[Bob Owen (:bobowen) PTO until 8th Oct]

Thanks all for the information.

(In reply to Molly Howell (she/her) [:mhowell] from comment #2)

This code itself seems reasonable, but (as I'm betting you've guessed) there are some other considerations. Off the top of my head:

• We'll need to invoke this code during installations, either new or paveover installations, and also during updates. I'd imagine that having this run in the same places we register the WER module would be a good example.

Thanks, that's seems like a good plan, would we need to add at this point in installer.nsi as well?

(I also hadn't actually noticed that Program Files already has access for ALL RESTRICTED APPLICATION PACKAGES, so this extra SID will be more important for installs elsewhere. Also means that using an LPAC is probably only a good idea when we can't restrict these things already.)

• I don't think we can do this at all for MSIX installations, or at least I'm not sure how we could. I don't understand enough about the requirements for this thing (even after reading the design docs in the metabug) to know if that's a serious problem.

I'm slightly hopeful that we might already have access to these files. The ALL RESTRICTED APPLICATION PACKAGES doesn't seem to get inherited for WindowsApps directory though, so I'll do some experiments. (I'll file a separate bug)
I'll only initially land actually using the LPAC behind a pref anyway.

• We'll need to decide how to handle instances that no installer was ever run for, like "portable" apps or copies that had their files just extracted directly into place, where we won't be able to assume within Firefox that this permission is already there. Options for that would include just doing nothing and accepting that it won't work, moving this code completely out of the installer and into Firefox (I'm not sure if this is possible), or additionally supporting adding the permissions from within Firefox (which doesn't necessarily mean writing a second implementation of the same thing, we already have a few bits of NSIS code that we invoke from within Firefox for a few different purposes). I don't have enough information to have an opinion about which to pick, but there should be a decision made.

I guess that we need to test for access during start-up and then attempt to add.
There is some hope that if Firefox is being run from a simple extraction, then the main process would have permissions to change the access.
I guess if it doesn't we would probably just have to restrict functionality, with a SUMO article about how to remedy.
Could you point me to an example of NSIS being used like this?
(We could do this using c++ fairly easily as well I think.)

Comment 6

[Molly Howell (she/her) (mostly inactive)]

(In reply to Bob Owen (:bobowen) from comment #5)

Thanks, that's seems like a good plan, would we need to add at this point in installer.nsi as well?

I would say to put it there instead of where the WIP patch has it; this is really an installation task and not a cleanup task.

Could you point me to an example of NSIS being used like this?

I'm thinking of something like this function, which is invoked from the shell service via this bit of dispatching inside helper.exe. In short, we can invoke uninstall/helper.exe with a command line argument that tells it to run a particular NSIS function. helper.exe happens to also be the uninstaller, but it's always present even if the installer wasn't used.

Comment 7

[Bob Owen (:bobowen) PTO until 8th Oct]

Attachment: Bug 1793969: Add LPAC access to the application directory created by the installer. r=mhowell!

Comment 8

[Bob Owen (:bobowen) PTO until 8th Oct]

https://treeherder.mozilla.org/jobs?repo=try&revision=3cbde872d741049aec7346b2d905e98c57b3b101

Comment 9

[Bob Owen (:bobowen) PTO until 8th Oct]

Latest try push with permission just granted win10+:
https://treeherder.mozilla.org/jobs?repo=try&revision=4550e276d9f479848b607b259c7f90e3efa591f6

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4bb6e19c2409
Add LPAC access to the application directory created by the installer. r=mhowell
https://hg.mozilla.org/integration/autoland/rev/a9fc5126c3ad
1793972: apply code formatting via Lando

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/4bb6e19c2409
https://hg.mozilla.org/mozilla-central/rev/a9fc5126c3ad