Closed Bug 827269 Opened 8 years ago Closed 8 years ago

Restablish use of on-modify-request and friends in e10s child

Categories

(Core :: Networking: HTTP, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)

References

(Depends on 1 open bug)

Details

Bug 806753 was my idea, but not a good one.  We may have imperfect behavior with cookies in child on-modify-request, but we still want to use it for other things, and though there's no production code that needs it yet, we want it for tests (like to test bug 765934).

Mostly just a patch -R of bug 806753 but we might as well fix some of the missing notifications I found in that bug.
(In reply to Jason Duell (:jduell) from comment #0)
> Bug 806753 was my idea, but not a good one.  We may have imperfect behavior
> with cookies in child on-modify-request, but we still want to use it for
> other things, and though there's no production code that needs it yet, we
> want it for tests (like to test bug 765934).

I think it is a very good to keep the restriction that Necko callbacks can only be done in the parent process, if at all possible, because the parent process is where Necko lives.

For on-modify-request, in particular, there are also security issues. If we allow the child process to modify the request in on-modify-request, then we must sanity-check *everything* in the request after calling on-modify-request.

Why do the tests for bug 765934 need this? Bug 765934 is about an extension point primarily for addons (which should be running code in the parent process, in any product with parent/child process separation), not an extension point for web content. So, I would expect that the tests for that bug should not require any interaction with child processes.
>  If we allow the child process to modify the request in on-modify-request, 
> then we must sanity-check *everything* in the request after calling 
> on-modify-request.

We have to sanity-check everything that comes from the child anyway, so we save nothing security-wise by blocking OMR AFACIT.

We're not currently using addons in the child, and may never, but there's likely to be some other internal uses of OMR at some point in gaia or whatever (we've already come close).  We can either do the effort to keep our powder dry and everything working in e10s with tests, or we can turn it off and deal with fixing/architecting things as we need them.  So far I'm leaning toward the former.
Blocks: 828739
Assignee: nobody → jduell.mcbugs
Can we please avoid doing this until we actually have a need for it. As I said in another bug:

I suggest we WONTFIX this. If/when we have extensions in e10s-enabled builds, the extensions will run in the parent process and they will only be able to process on-modify-request in the parent process. (As far as we can tell, it is necessary to keep addon code running in the parent process in order to have any hope of having an effective sandbox.)
I suspect we'll wind up needing this for things other than extensions.  But I guess we can wait for now and see if that happens (worst case people will work around the lack of OMR in some inferior way w/o us knowing about it, but I guess I can live with that risk).

WORKSFORME until proven otherwise.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
(In reply to Jason Duell (:jduell) from comment #4)
> I guess we can wait for now and see if that happens (worst case people will
> work around the lack of OMR in some inferior way w/o us knowing about it,
> but I guess I can live with that risk).

If they can work around it then that's indicative of a security bug.

I understand your concerns, especially about Gaia. But, keep in mind that Gaia apps are just HTML apps, and they use the same APIs as web content. So, exposing this functionality to Gaia apps would require us to expose an extension to XHR for it. The most likely extension we will add is a redirect handler extension. I don't see how a web app (including Gaia apps) would need on-modify-request because it could just set up the request the way it wants from the beginning. In other words on-modify-request is for allowing an extension to MitM the request, but MitM'ing yourself is pretty circuitous.
Resolution: WORKSFORME → WONTFIX
According this query [1] it seems like the only content process consumer can be web console utils.

I presume /toolkit/components/url-classifier/content/xml-fetcher.js runs on parent (does anybody know?).

WONTFIX is the right approach IMO.  For web utils we can find some other way around.


[1] http://mxr.mozilla.org/mozilla-central/search?string=http-on
(In reply to Honza Bambas (:mayhemer) from comment #6)
> According this query [1] it seems like the only content process consumer can
> be web console utils.

In e10s, (eventually) web console and all developer tools will need to run in the parent for security reasons.

> I presume /toolkit/components/url-classifier/content/xml-fetcher.js runs on
> parent (does anybody know?).

Same here. URL classifier is a security feature, so it has to be done in the parent process.
You need to log in before you can comment on or make changes to this bug.