New issue
Advanced search Search tips

Issue 767557 link

Starred by 1 user
Status: Assigned
Owner:
rdevlin....@chromium.org
Cc:
karandeepb@chromium.org
lazyboy@chromium.org
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ExtensionMsg_UpdateTabSpecificPermissions is sent to *all* extension frames but only to the *main* frame of the tab

Project Member Reported by lukasza@chromium.org, Sep 21 2017 
When looking at ActiveTabPermissionGranter::GrantIfRequested I see that it tries to send ExtensionMsg_UpdateTabSpecificPermissions to a set of processes:

      SendMessageToProcesses(
          ProcessManager::Get(web_contents()->GetBrowserContext())
              ->GetRenderFrameHostsForExtension(extension->id()),
          web_contents()->GeRenderProcessHost(), update_message);

I am a bit concerned that the messages is send to 1) *all* processes hosting a frame with |extension->id()|, but 2) *only* the main frame process of the active tab.

Comment 1 by lukasza@chromium.org, Sep 21 2017

Owner: rdevlin....@chromium.org
Devlin, can you please double-check if the current behavior is okay?  I don't have a good understanding of why the IPC needs to be sent not only to the extension itself, but also to the active tab.  OTOH, it seems to me that is the active tab needs special permissions, then these permissions should apply to all the frames (not just to the main frame / or just to frames hosted in the same process as the main frame).

WDYT?  Does this bug talk about a real problem?  Or is everything WAI?

Comment 2 by rdevlin....@chromium.org, Sep 21 2017 

Good questions!

Starting with 1) - does this need to be sent to all extension frames.  Honestly, I'm not 100% sure.  There are certainly times that having the permission will affect things *outside* of that direct tab - for instance, the ability to use tabs.executeScript() or see the tab's URL in the tab object.  However, both of these are actually determined on the browser side, so I'm not sure the renderer necessarily needs to know about them.

For this one, I'd be curious if we can just do a quick "what breaks" test - stop sending the messages and see if tests fail.  I'm probably forgetting something obvious.

On to 2) - why don't we send this to all frames.  The activeTab permission actually only applies to the top-level frame of a tab.  This was a conscious decision at the time, because we didn't want granting an extension permission on any blog site to inadvertently grant access to facebook, google, and every other social network through those lovely "like it" iframes that are embedded.  However, two notes:
- It *should* probably apply at least to frames of the same origin, e.g. example.com embedding example.com.  Right now, this will still work, because we won't isolate frames in another process that case, but that's kind of an architectural detail.
- In the future, we might need to find a better solution for subframes, since we'd like to be pushing activeTab more strongly.

So sending the message to all frames in the tab would probably be worthwhile.


Overall, I'm not sure that the current behavior results in any buggy behavior, but I think it could benefit from being cleaned up.

Comment 3 by rdevlin....@chromium.org, Sep 22 2017

Cc: karandeepb@chromium.org catmulli...@chromium.org lazyboy@chromium.org
Status: Assigned (was: Untriaged)
Assigning to myself to get it out of triage, but I don't anticipate tackling this soon (unless we discover this is a problem, rather than a beneficial refactor).  Other folks, feel free to jump in.

Comment 4 by catmulli...@chromium.org, Feb 8 2018

Cc: -catmulli...@chromium.org
►

Sign in to add a comment