967967 - Children of Nuwa are being sandboxed twice and crashing.

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Currently seccomp sandboxing is broken with Nuwa, because the content process is created in a preallocated-like state through ContentParent::RecvAddNewProcess, which sets mOSPrivileges to PRIVILEGES_DEFAULT (meaning unprivileged).  However, the grandchild process still has uid/gid 0 at this point, and it is later sent a SetProcessPrivileges to drop privileges.  This combines with the fix for bug 921817 to make the process sandbox itself before dropping permissions, which violates the sandbox (setuid and setgid aren't on the whitelist).

So I think the ContentParent should be constructed with mOSPrivileges == PRIVILEGES_INHERIT (meaning the parent process's privilege level) instead.  I tested this, and the content processes are still correctly deprivileged — the final privileges are obtained from PrivilegesForApp and passed to MaybeTakePreallocatedAppProcess, in CreateBrowserOrApp.

I'll attach the one-line patch once this bug has a number.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug967967-sandbox-nuwa-fix-hg0.diff

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I notice that we don't ever update mOSPrivileges, and nothing else seems to use its value.  It's not entirely clear from bug 782456 what the intent behind it was.

Comment 3

[Worik Stanton]

(In reply to Jed Davis [:jld] from comment #1)
> Created attachment 8370486 [details] [diff] [review]
> bug967967-sandbox-nuwa-fix-hg0.diff

I applied that change to B2G/gecko/dom/ipc/ContentParent.cpp.  Phone now boots.  But there is no keyboard in any app.

Comment 4

[Jason Smith [:jsmith]]

(In reply to Worik Stanton from comment #3)
> (In reply to Jed Davis [:jld] from comment #1)
> > Created attachment 8370486 [details] [diff] [review]
> > bug967967-sandbox-nuwa-fix-hg0.diff
> 
> I applied that change to B2G/gecko/dom/ipc/ContentParent.cpp.  Phone now
> boots.  But there is no keyboard in any app.

The keyboard issue is a separate problem being tracked elsewhere in a different bug.

What device did you test this on?

Comment 6

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/b2g-inbound/rev/b8e0889a9454

Comment 7

[Jason Smith [:jsmith]]

Can we consider this for approval on b2g28? This will unblock Keon testing with nuwa enabled on b2g28, so this would really helpful to get on that branch when nuwa gets uplifted. We have many l10n testers reliant on keons working for localization testing, so I'd rather not block them in their testing here when nuwa gets enabled on 1.3.

Comment 8

[Worik Stanton]

(In reply to Jason Smith [:jsmith] from comment #4)
> (In reply to Worik Stanton from comment #3)
> > (In reply to Jed Davis [:jld] from comment #1)
> > > Created attachment 8370486 [details] [diff] [review]
> > > bug967967-sandbox-nuwa-fix-hg0.diff
> > 
> > I applied that change to B2G/gecko/dom/ipc/ContentParent.cpp.  Phone now
> > boots.  But there is no keyboard in any app.
> 
> The keyboard issue is a separate problem being tracked elsewhere in a
> different bug.
> 
> What device did you test this on?

Excuse me.  On a Keon.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8e0889a9454

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/448c7335b6e9