1380448 - Make sure Tracking Protection covers about: pages

Status: NEW

(Toolkit :: Safe Browsing, enhancement, P3)

Description

[Gian-Carlo Pascutto [:gcp]]

Tracking protection currently doesn't seem to apply to about:* pages. Given that some pages load external sites through iframes, this will allow some trackers to get through. This has given rise to some controversy about the "Get Add-ons" page.

We should also make sure this works correctly when having both private and non-private windows open.

Comment 1

[Asa Dotzler [:asa]]

same as bug 1380409?

Comment 3

[François Marier [:francois]]

@tofumatt: Once we fix this oversight, the Shield icon will start to show up on about:addons for people with tracking protection enabled. You may want to avoid loading it for people with DNT enabled. TP will automatically send the DNT signal so that will solve this UX issue.

This hacks post is our recommended way of using Google Analytics: https://hacks.mozilla.org/2016/01/google-analytics-privacy-and-event-tracking/

Comment 4

[François Marier [:francois]]

This is a regression introduced by bug 1337043. We should back it out and come up with a better solution to address the performance problem.

It's now clear that we can't rely on about: pages to not load arbitrary web content, so I guess we'll have to instead whitelist the about: pages which are known to be local-only.

Christoph: do you know if there's a larger problem with our trusting too much the about: scheme in platform code or is it limited to the URL classifier?

Comment 5

[Ralf Jung]

> the Shield icon will start to show up on about:addons for people with tracking protection enabled. You may want to avoid loading it for people with DNT enabled. TP will automatically send the DNT signal so that will solve this UX issue.

Will this also stop showing the shield icon on other websites?
TBH showing the shield icon, while slightly irritating ("What? trackers blocked on about:addons??") is actually accurate, isn't it?

Comment 6

[François Marier [:francois]]

(In reply to Ralf Jung from comment #5)
> > the Shield icon will start to show up on about:addons for people with tracking protection enabled. You may want to avoid loading it for people with DNT enabled. TP will automatically send the DNT signal so that will solve this UX issue.
>
> Will this also stop showing the shield icon on other websites?

Only if those websites avoid loading GA when they are sent the DNT signal. What the Hacks proposes is that you check DNT because you load GA on your website.

If GA is not loaded (because the user has DNT enabled) then there is nothing to block so the TP shield doesn't appear. (The Shield is a sign that something was blocked by TP.)

Comment 7

[Ralf Jung]

Ah, got it -- "loading it" was referring to loading GA, not loading the icon. Thanks :)

Comment 8

[Gian-Carlo Pascutto [:gcp]]

(In reply to François Marier [:francois] from comment #4)
> Christoph: do you know if there's a larger problem with our trusting too
> much the about: scheme in platform code or is it limited to the URL
> classifier?

I'm afraid to open up this can of worms, but given this bug, it's an unavoidable question to be asked: if we don't trust some about:* pages enough to let them bypass classifier, then shouldn't one make the argument that content scripts (from WebExtensions) should be allowed there, too. (AMO is a double-whammy because even outside about:* it has a specific block for WebExtensions)

If we don't allow this, then we're promoting our own adblocker^W tracking protector to privileged status. I hope I don't have to paint a picture why this is a *particularly* undesirable situation to have.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Someone on reddit reported that the issue reproduces just as well in Firefox 54, and I can confirm this. GA is loaded when visiting about:addons, but not when visiting other web properties.

Comment 10

[Ralf Jung]

> shouldn't one make the argument that content scripts (from WebExtensions) should be allowed there, too

According to one addon developer, the current situation is rather confusing here:
Quoting from <https://github.com/mozilla/addons-frontend/issues/2785#issuecomment-314807534>

> Legacy uBlock Origin can block the network request to GA.
> 
> However webext-hybrid uBO as per Network pane in dev tools does not block it. Same for pure webext Ghostery, the network request to GA was not blocked, again as per Network pane in dev tools.
> 
> What is concerning is that both uBO webext-hybrid and Ghostery report the network request to GA as being blocked, while it is really not as per Network pane in dev tools. It's as if the order to block/redirect the network request was silently ignored by the webRequest API, and this causes webext-based blockers to incorrectly and misleadingly report to users what is really happening internally, GA was not really blocked on about:addons, but there is no way for the webext blockers to know this and report properly to users.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

(In reply to Ralf Jung from comment #10)
> > shouldn't one make the argument that content scripts (from WebExtensions) should be allowed there, too
> 
> According to one addon developer, the current situation is rather confusing
> here:


It's not confusing, the implementation is plain buggered, but that's separate from the policy decision whether it should be possible in the first place.

Comment 12

[François Marier [:francois]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #8)
> I'm afraid to open up this can of worms, but given this bug, it's an
> unavoidable question to be asked: if we don't trust some about:* pages
> enough to let them bypass classifier, then shouldn't one make the argument
> that content scripts (from WebExtensions) should be allowed there, too. (AMO
> is a double-whammy because even outside about:* it has a specific block for
> WebExtensions)

This should ideally be discussed in a separate (but related) bug against Web Extensions.

Comment 13

[:Felipe Gomes (needinfo for replies!)]

(In reply to François Marier [:francois] from comment #4)
> This is a regression introduced by bug 1337043. We should back it out and
> come up with a better solution to address the performance problem.
> 
> It's now clear that we can't rely on about: pages to not load arbitrary web
> content, so I guess we'll have to instead whitelist the about: pages which
> are known to be local-only.

Is bug 1337043 really at fault here? My understanding is that the about:addons page is the toplevel one, but the URL Classifier should still be classifying the iframe that is loaded inside it which has a regular URL. Or does the classifier only classify top-level documents?

If yes, I'm fine if we change bug 1337043 to only whitelist about:blank and about:home, as long as we check that it doesn't regress the win of not initializing NSS before first paint (see bug 1337043 comment 63). It's probably better to keep classifying about:newtab anyways.

Comment 14

[Christoph Kerschbaumer [:ckerschb]]

(In reply to François Marier [:francois] from comment #4)
> Christoph: do you know if there's a larger problem with our trusting too
> much the about: scheme in platform code or is it limited to the URL
> classifier?

I think the scope is limited to tracking protection because if iframes load external content, then those go through the regular loading process, meaning that depending on their protocol applicable security checks are performed, nonetheless if toplevel is a local resource like about or not.

Comment 15

[François Marier [:francois]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #9)
> Someone on reddit reported that the issue reproduces just as well in Firefox
> 54, and I can confirm this. GA is loaded when visiting about:addons, but not
> when visiting other web properties.

And it's no longer reproducible on AMO because AMO now honours DNT: https://github.com/mozilla/addons-frontend/issues/2785#issuecomment-315212909

We'll have to create a test about: page with GA on it to make sure the underlying Safe Browsing bug is fixed.

Comment 16

[François Marier [:francois]]

(In reply to :Felipe Gomes (needinfo me!) from comment #13)
> If yes, I'm fine if we change bug 1337043 to only whitelist about:blank and
> about:home, as long as we check that it doesn't regress the win of not
> initializing NSS before first paint (see bug 1337043 comment 63). It's
> probably better to keep classifying about:newtab anyways.

about:blank is clearly not a problem. about:home could in theory cause issues because it loads remote resources (e.g. the snipet), but is probably unlikely to grow a GA dependency anytime soon.