Closed Bug 1458374 Opened 3 years ago Closed 3 years ago

TP should not block initial third-party iframe loads in WebExtension documents

Categories

(WebExtensions :: Request Handling, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1318532

People

(Reporter: ntim, Unassigned)

References

(Blocks 2 open bugs)

Details

Black menu for google embeds Google pages via iframes, and these are blocked by TP, which prevents almost a half of the extensions' functionality from working properly.
The question here is whether TP should be applied to the main_frame document load of the iframe itself.  It can and should apply to all sub-requests within that document.

so with tp on:

browser with moz-extension principal
-> content iframe loading gmail

Gmail in a tab works.  gmail in the above scenario does not due to TP blocking it.

The extension explicitly requests hosts permissions for the google domains which bypasses TP for requests made *BY* extension code, but that does not extend to iframes loading 3rd party domains.

The question is, do we want to allow this?  Are there any security issues with it?  Are there product issues that would have to be resolved (e.g. how does a user know that iframe is loaded via https)?
Flags: needinfo?(mconca)
Flags: needinfo?(francois)
Flags: needinfo?(dveditz)
If we had bug 1318532, that would possibly handle this situation.
See Also: → 1318532
I have a log, but uncertain if it has stuff in it I don't want public.  Here's some pertinent entries:

[94608:Main Thread]: D/nsChannelClassifier nsChannelClassifier[0x10fcd2c10]: Enabling tracking protection checks on channel[0x15040f040] with uri https://mail.google.com/mail/... for toplevel window https://03a31e9c-d34b-6b44-b8a9-2cf918bc0987/

That toplevel window url looks like the extension guid.

Then a bunch of "Checking fragment" lines with "Probe in" lines

Then

[94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnBlacklistResult aErrorCode=0x805d0022
[94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnBlacklistResult channel [0x15040f040] uri=https://mail.google.com/mail/..., is in blacklist. Start checking whitelist.
[94608:Main Thread]: D/nsChannelClassifier nsChannelClassifier[0x10fcd2c10]: Looking for http://03a31e9c-d34b-6b44-b8a9-2cf918bc0987/?resource=google.com in the whitelist

repeat the probing...

[94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnWhitelistResult aErrorCode=0x805d0022
[94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnWhitelistResult channel [0x15040f040] uri=https://mail.google.com/mail/..., is not in whitelist
From what I understand, the add-on is essentially framing things like this:

    +-------https://03a31e9c-d34b-6b44-b8a9-2cf918bc0987/----------+
    |                                                              |
    |    +------------https://mail.google.com------------+         |
    |    |                                               |         |
    |    |     +------(something else from Google)----+  |         |
    |    |     |                                      |  |         |
    |    |     +--------------------------------------+  |         |
    |    +-----------------------------------------------|         |
    +--------------------------------------------------------------+


My initial thought was that we probably want to use mail.google.com as the toplevel window URI. The way the add-on frames Google sites is very similar to the way that a tab is kinda like a toolbar and a content iframe. If we stop at mail.google.com when looking for the toplevel window URI, we would essentially treat the extension as if it were the browser chrome for the purpose of doing tracking protection checks.

However, that would mean that if an add-on takes over the about:newtab page, then it would be able to create hidden iframes with cryptominers, trackers, etc, bypassing any TP checks.

Given that we want to enforce TP on about: pages (bug 1380448), this could lead to a weird double standard between say about:addons (which has a side-bar and a remote "add-on discovery" iframe) and an add-on-supplied page.
Flags: needinfo?(francois)
(In reply to François Marier [:francois] from comment #4)
> From what I understand, the add-on is essentially framing things like this:

Also my understanding.

> If we
> stop at mail.google.com when looking for the toplevel window URI, we would
> essentially treat the extension as if it were the browser chrome for the
> purpose of doing tracking protection checks.

Extensions are a 3rd level between the two, we shouldn't treat them as one or the other.  In this case, the extension has permission for the domain.  Given that, in this case, I think treating those as toplevel loads (for TP) is fine.

> However, that would mean that if an add-on takes over the about:newtab page,
> then it would be able to create hidden iframes with cryptominers, trackers,
> etc, bypassing any TP checks.

Given the requirement for explicit permission to the domain I don't think it's adding much in the way of enabling this issue.

> Given that we want to enforce TP on about: pages (bug 1380448), this could
> lead to a weird double standard between say about:addons (which has a
> side-bar and a remote "add-on discovery" iframe) and an add-on-supplied page.

With the explicit permission I don't think that's the case.
(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> > If we
> > stop at mail.google.com when looking for the toplevel window URI, we would
> > essentially treat the extension as if it were the browser chrome for the
> > purpose of doing tracking protection checks.
> 
> Extensions are a 3rd level between the two, we shouldn't treat them as one
> or the other.  In this case, the extension has permission for the domain. 
> Given that, in this case, I think treating those as toplevel loads (for TP)
> is fine.

The particular case with this extension is kind of quirky, I think the extension really wants to treat the frame as a top-level load but can't yet do it since we don't have a mozbrowser/webview capability.  I think we would be much better off adding that capability rather than adding special cases and rules for TP.  Or to put it another way, if an extension page has a traditional tracking iframe (eg a facebook like button?  i don't know what a current example is), I as a user would be pretty upset to hear that TP did not block that frame in order to enable this framed-gmail scenario.
> [94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnWhitelistResult
> channel [0x15040f040] uri=https://mail.google.com/mail/..., is not in whitelist

The whitelist is ours, right? Could we solve this (very specific) problem by putting mail.google.com on the whitelist? It's clearly not a tracker! Which is not to say Google doesn't track your use of it, but users would consider it primarily functional. Maybe there's other useful google domains that should go on there, too, like maps and translate and news and so on.

The longer range solution is something like bug 1318532. Not sure why we wouldn't work w/Google to standardize some subset of <webview> instead of making our own incompatible equivalent, but that's a different discussion entirely.
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #7)
> > [94608:Main Thread]: D/nsChannelClassifier TrackingURICallback[0x10fcd2c10]::OnWhitelistResult
> > channel [0x15040f040] uri=https://mail.google.com/mail/..., is not in whitelist
> 
> The whitelist is ours, right? Could we solve this (very specific) problem by
> putting mail.google.com on the whitelist? It's clearly not a tracker! Which
> is not to say Google doesn't track your use of it, but users would consider
> it primarily functional. Maybe there's other useful google domains that
> should go on there, too, like maps and translate and news and so on.

The whitelist is actually not a general-purpose whitelist. It's actually a pairwise whitelist for the purpose of grouping related domains together based on this entity list: https://github.com/mozilla-services/shavar-prod-lists/blob/master/disconnect-entitylist.json

We don't currently whitelist domains and doing so would be a major policy change.
We'll be discussing how to move a mozbrowser/webview/something forwards
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1318532
Flags: needinfo?(mconca)
Product: Toolkit → WebExtensions
See Also: → 1509112
You need to log in before you can comment on or make changes to this bug.