1580268 - sandbox GPU process on OpenBSD with pledge()

Status: RESOLVED FIXED

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

Description

[jcs]

Attachment: ff-gpu-pledge.diff

Add a security.sandbox.pledge.gpu preference that is passed to the GPU process to allow it to call pledge() on OpenBSD for kernel-enforced sandboxing.

Since the user profile preferences are not available in the GPU process, the GPUProcessManager passes them as a command line argument (-GPUpledge).

Comment 1

[jcs]

Attachment: ff-gpu-pledge2.diff

Comment 2

[Landry Breuil (:gaston)]

Comment on attachment 9091832 [details] [diff] [review]
ff-gpu-pledge2.diff

Technically this also adds support for setting MOZ_DISABLE_PLEDGE in the env to temporarly disable sandboxing for debugging purposes.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 9091832 [details] [diff] [review]
ff-gpu-pledge2.diff

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

::: gfx/ipc/GPUProcessManager.cpp
@@ +168,5 @@
> +    nsAutoCString pledgeString;
> +
> +    if (!PR_GetEnv("MOZ_DISABLE_PLEDGE")) {
> +      // Preferences are not loaded by the GPU process, so we need to pass the
> +      // pledge string

This is confusing to me? Doesn't the code you patch up above set up the de-serialization of shared preferences in the GPU Process? What do you mean exactly by "Preferences are not loaded by the GPU process"?

I think we'd strongly prefer using the current map rather than adding an ad-hock argument, so I'd like the understand why that doesn't work.

Comment 4

[jcs]

From my testing, none of the preferences in the profile were available at the time that code runs (fetching it just returns an empty string). I think it was only loading the static preferences from modules/libpref/init/StaticPrefList.h by that time, not any in the user's profile. If I'm mistaken, please let me know how to load them and I can move the sandbox call back to after the SharedPreferenceDeserializer runs.

Comment 5

[jcs]

Ok I figured it out the problem. The sandboxing needed to be done in GPUParent::Init (which is confusingly called from the GPU child, not the parent, in GPUProcessImpl::Init) after NS_InitMinimalXPCOM() is called. This way the preferences are loaded and I can access the GPU pledge string.

I'll update the patch once I figure out some things in #1580271.

Comment 6

[jcs]

Attachment: ff-gpu-pledge3.diff

3rd try at pledge patch for GPU process, now able to read pledge string from preferences

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 9092459 [details] [diff] [review]
ff-gpu-pledge3.diff

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

::: dom/ipc/ContentChild.cpp
@@ +4080,3 @@
>  
> +  if (!PR_GetEnv("MOZ_DISABLE_PLEDGE")) {
> +    if (pledge(promisesString.get(), NULL) == -1) {

nullptr

Comment 8

[jcs]

Attachment: ff-gpu-pledge4.diff

NULL -> nullptr

Comment 9

[Landry Breuil (:gaston)]

Comment on attachment 9092459 [details] [diff] [review]
ff-gpu-pledge3.diff

Great stuff ! At that point that looks good to me, unsure if we want to separate the 'MOZ_DISABLE_PLEDGE' addition to a separate commit.

gcp, at that point, what's your opinion ? Otherwise i could upload two separate hg patches up for review (not via phabricator though, im not up to this)

Comment 10

[jcs]

Attachment: ff-gpu-pledge5.diff

A new revision, now rebased on top of #1584839

Comment 11

[Gian-Carlo Pascutto [:gcp]]

gcp, at that point, what's your opinion ? Otherwise i could upload two separate hg patches up for review (not via phabricator though, im not up to this)

Seems good? Patch is minimally invasive anyway. I can deal with Phabricator for you.

Comment 12

[jcs]

Attachment: ff-gpu-pledge6.diff

New patch against Firefox 70

Comment 13

[Landry Breuil (:gaston)]

technically this depends on bug #1584839 so that pledge config is moved to files first

Comment 14

[Landry Breuil (:gaston)]

jcs, why are you using pledgeFile.Append("pledge.gpu"); instead of OpenBSDFindPledgeFilePath like other process types ? i know this is moot with work from bug #1580271, just curious

Comment 15

[Landry Breuil (:gaston)]

Attachment: Bug 1580268: Sandbox GPU process on OpenBSD with pledge() r?gcp

Depends on D51385

Comment 16

[jcs]

Just a mixup trying to juggle all these different revisions.

Comment 17

[jcs]

Attachment: ff-gpu-pledge7.diff

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e2be8ec03fc
Sandbox GPU process on OpenBSD with pledge() r=gcp

Comment 19

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8e2be8ec03fc