815329 - Call nsMixedContentBlocker after all other Content Policies and after all internal redirects

Status: NEW

(Core :: DOM: Security, defect)

Description

[Tanvi Vyas[:tanvi]]

* Order the content policies -  If nsMixedContentBlocker.cpp finds http javascript on an https page, it will flag the page as mixed content, block the script and add the "mixed content icon" to the left side of the url bar.  If the page has a CSP policy that blocks that script, then the "mixed content protection icon" shouldn't show up, since the page is blocking that content with CSP and doesn't need the mixed content blocker to block it.  Hence, perhaps we should ensure that nsMixedContentBlocker.cpp is called after all other Content Policies are called.  Input on this is welcome.

bsmith's suggestion:
> Instead of making the mixed content blocker an just another implementation
> of nsIContentPolicy, we can hard-code the call to
> nsMixedContentBlocker::shouldLoad in the functions that loop through all the
> registered nsIContentPolicy implementations, so that the
> nsMixedContentPolicy call occurs after the loop.

Comment 1

[Boris Zbarsky [:bzbarsky]]

That seems plausible to me, yes.

Comment 2

[Honza Bambas (:mayhemer)]

It seems like a hack to me :)  This is one more example of why CSP might not be a good way to do content security state determination.

Comment 3

[Honza Bambas (:mayhemer)]

After I've read bug 782654 last comments, I think when this would rather be hardcoded (i.e. not added as a CSP listener) it would not be a hack.  

Also hardcoding this in the redirect callback of nsDocLoader (or nsDocShell? I don't recall) we may fix the https->http redirect too.

Comment 4

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

Basically, we cannot implement the mixed content blocker as an implementation of the current nsIContentPolicy, because of bug 838395 and redirect issues as described in comments in bug 456957 and bug 418354. We need to do mixed content blocking/logging much later than nsIContentPolicy allows, and we must be able to do so without having references to the actual DOM nodes.

My plan is to hook the nsMixedContentPolicy logic directly into Necko. This will require that every channel (for web content) to be annotated with the nsContentPolicyType. I think Dev was working on that and I am planning to work with him on it.

One great side-effect of this work is that mixed content blocker will become compatible with e10s and it will be enforced in the trusted (parent) process.

It will also be easier to fix bug 418354 with this.

After this is implemented, we can try to generalize this work into a new content policy API where CSP and other content policies can be enforced in the parent process in e10s, and which works correctly with redirects (both internal and external).

Comment 5

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

test_csp_bug768029.html should be fixed as part of this work. One of the patches in bug 834836 works around this bug (bug 815329) by disabling the mixed content blocker.

Comment 6

[Henri Sivonen (:hsivonen)]

Is this bug the reason why Twitter goes into the mixed content state after a while without an obvious non-TLS request in the Network tab of dev tools?

Comment 7

[Tanvi Vyas[:tanvi]]

(In reply to Henri Sivonen (:hsivonen) from comment #6)
> Is this bug the reason why Twitter goes into the mixed content state after a
> while without an obvious non-TLS request in the Network tab of dev tools?

Maybe related to bug 947079?