896639 - Should some content policies be run from the chrome process

Status: NEW

(Firefox :: Security, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

In the larch branch, billm implemented a workaround that disables all native-code nsIContentPolicy implementations. This breaks the mixed content blocker, CSP, and our other native-code nsIContentPolicy implementations.

It seems like now is the time to redo the nsIContentPolicy API, probably by splitting it into two APIs. One side of the split would provide an API that does not provide access to DOM nodes, that does automatically support redirects, and that runs in the parent process. The other side of the split could be similar to (or the same as) the current nsIContentPolicy implementation, so that it provides access to DOM nodes, does not handle redirects, and that executes in the content process.

Note that Jonas, long ago, started a discussion about a new nsIContentPolicy API:
https://groups.google.com/forum/#!msg/mozilla.dev.platform/veLFoy09ydg/2XcWUXSiVbEJ

Comment 1

[Benjamin Smedberg]

In what case is it necessary to have a contentpolicy that runs in the chrome process? Don't all the B2G policies run in the content process currently, and that Just Works?

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Brian Smith (:briansmith) from comment #0)
> In the larch branch, billm implemented a workaround that disables all
> native-code nsIContentPolicy implementations. This breaks the mixed content
> blocker, CSP, and our other native-code nsIContentPolicy implementations.

I was wrong. CSP and Mixed Content Blocker aren't broken in larch because of this bug, but rather due to other bugs.

> In the larch branch, billm implemented a workaround that disables all
> native-code nsIContentPolicy implementations. This breaks the mixed content
> blocker, CSP, and our other native-code nsIContentPolicy implementations.

It seems dvander implemented it, not billm. dvander's current hack is here:
https://hg.mozilla.org/projects/larch/file/4f421f83f0b5/browser/base/content/browser-parent.js#l49

Cu.isWrappedJS(policy) returns false if policy is implemented with native code.

It seems like we can just change the hack that dvander implemented so that, instead of skipping the native-code nsIContentPolicy implementations, that instead the nsIContentPolicy is given a NULL aContext if the nsIContentPolicy implementation is native code. (We cannot pass a non-NULL aContext because aContext would be a CPOW and native code cannot touch CPOWs unless it does special things to unwrap the CPOW.) This way, fewer native-code nsIContentPolicy implementations may break.

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> In what case is it necessary to have a contentpolicy that runs in the chrome
> process?

When an extension implements nsIContentPolicy, since extension runs in the parent process.

> Don't all the B2G policies run in the content process currently,
> and that Just Works?

Yes, you are right.

In the long run, I would actually like to have CSP and Mixed Content Blocker (at least) implemented purely in the parent process, so that they can participate in making sandboxing stronger. For example, a CSP on of default-src: 'none' would be able to prevent the app from doing any networking even if the app was exploited to run some shellcode in its process. But, if there is a way to get CSP, Mixed Content Blocker, and other bulit-in CSP implementations work when they are in the child process, then it is OK with me to defer moving them to the parent process to some future date.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not at all in a hurry to have CSP running in the parent. In fact, having CSP in the parent is generally a bad thing since the more stuff we run in the parent, the more code is important that we harden against buffer overflow and other attacks.

A bug in CSP running in the child can only cause privilege escalation to the privileges of other origins in that same app.

A bug in CSP running in the parent can cause privilege escalation to place phone calls and otherwise take the users money.


At the point when a child process is able to circumvent a CSP implementation in the child, the child process is already owned and CSP no longer serves much purpose.

Like you say, there are exceptions to this. A CSP policy that disables all network access to some or all domains would still be valuable to enforce in the parent. But that's a pretty low priority to me.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Jonas Sicking (:sicking) from comment #3)
> I'm not at all in a hurry to have CSP running in the parent. In fact, having
> CSP in the parent is generally a bad thing since the more stuff we run in
> the parent, the more code is important that we harden against buffer
> overflow and other attacks.
>
> A bug in CSP running in the child can only cause privilege escalation to the
> privileges of other origins in that same app.
> 
> A bug in CSP running in the parent can cause privilege escalation to place
> phone calls and otherwise take the users money.

The things you cite are generally things to worry about when code runs in the parent, and I agree with the general notion that we should have as much code in the child (vs. the parent) as possible. But, your concerns not relevant to CSP. The CSP code in the parent would be extremely simple and could easily be demonstrated to be very low risk.

> At the point when a child process is able to circumvent a CSP implementation
> in the child, the child process is already owned and CSP no longer serves
> much purpose.

CSP (and to a lesser extent mixed content blocker) are part of what defines the sandbox in B2G and the sandbox should be enforced by the parent.

> Like you say, there are exceptions to this. A CSP policy that disables all
> network access to some or all domains would still be valuable to enforce in
> the parent. But that's a pretty low priority to me.

As I said in my previous comment, I agree that it isn't a high priority relative to many other things we have to do.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm changing the summary of the bug. We don't make a distinction between native and non-native content policies anymore. I think there's still an issue of which process this stuff should run in though.

Comment 6

[Chris Peterson [:cpeterson]]

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".