1527749 - PAC script privacy support for extensions

Status: NEW

(Core :: Networking: Proxy, enhancement, P2)

Description

[Shane Caraveo (:mixedpuppy)]

Extensions have a way to set the PAC script[1], same as it would be set via about:preferences. When it is set, you would see in preferences that an extension controls the setting.

Bug 1525447 prevents extensions from setting this if they do not have access to private browsing. This is inconvenient as the user must install the extension, go to about:addons and grant the extension permission to private browsing, before the extension can set the PAC (or other) setting.

We would like the PAC code to recognize that the request is private, then check if the PAC was loaded by an extension. If so, verify the extension has been granted permission.

This allows an extension to use a PAC script for non-private windows if the user has not granted permission.

We do something similar for webRequests and proxy.onRequest[2], where we are able to verify the extension has permission before passing the request through[3].

For the PAC, I think it would be a bit more complicated. One would have to identify that the setting is extension controlled via ExtensionSettingsStore.getSetting[4] which returns the id, then use WebExtensionPolicy.getById/privateBrowsingAllowed[5] and loadInfo similar to what was done for ChannelWrapper. We could somehow add the extension id to proxy settings to make it a little simpler if necessary.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy/settings
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/proxy/onRequest
[3] https://searchfox.org/mozilla-central/rev/cb7faaf6b4ad2528390186f1ce64618dea71031e/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#526
[4] https://searchfox.org/mozilla-central/rev/cb7faaf6b4ad2528390186f1ce64618dea71031e/toolkit/components/extensions/ExtensionSettingsStore.jsm#442
[5]

Comment 1

[David Durst [:ddurst]]

Any chance of getting a fix for this in 67?

Bug 1526299 (a P1) is blocked on this, because without this, a user in a particular scenario cannot run an extension at all unless granting it permission to run in private browsing -- which is both not obvious and counter-intuitive. There are about 76 extensions that currently qualify for this bad situation; and when private browsing changes take effect (in 67), any new users for any of those extensions (or possibly new ones) will be subject to this same error case.

Comment 2

[Honza Bambas (:mayhemer)]

Let's see.

Comment 3

[Honza Bambas (:mayhemer)]

In a nut shell: extensions that are not allowed in PB mode can't set PAC even in non-PB mode (for non-PB requests); the feature is disabled for all requests, until the extension is given PB mode rights.

dragana (kudos!) promised to look at this, so assigning to her now.

sorry I didn't get to this this week :/

Comment 4

[Honza Bambas (:mayhemer)]

And as Valentin noted during the Necko meeting, bug 1528873 can be a good code reference.

Comment 5

[Shane Caraveo (:mixedpuppy)]

I'm not clear about that bug being a good reference or not, but here is where the browser.proxy.onRequest API gets checked against the extension policy:

https://searchfox.org/mozilla-central/rev/b2d35912da5b2acecb0274eb113777d344ffb75e/toolkit/components/extensions/ProxyScriptContext.jsm#314

That matches call leads to ChannelWrapper where I linked to it in the OP.

https://searchfox.org/mozilla-central/rev/cb7faaf6b4ad2528390186f1ce64618dea71031e/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#526

Comment 6

[Dragana Damjanovic [:dragana]]

So if an extension do not have access to the private browsing it should be able to set PAC for non-private browsing mode? What about private browsing requests, should they use other PAC or do not use proxy at all? ... this looks like we will need 2 different proxy setting. although it is a bit weird to have different proxy configs for private/non-private mode. Would we need a new UI.

Comment 7

[Dragana Damjanovic [:dragana]]

About how this can be implemented:

should we just add a flag to pac that it is non-private-browsing-only pac? Necko will only check the flag and webextensions can perform other checks and set the flag properly.

I still do not like the idea of having different proxy settings for private browsing and non-private browsing.

Comment 8

[David Durst [:ddurst]]

(In reply to Dragana Damjanovic [:dragana] from comment #6)

So if an extension do not have access to the private browsing it should be able to set PAC for non-private browsing mode? What about private browsing requests, should they use other PAC or do not use proxy at all? ... this looks like we will need 2 different proxy setting. although it is a bit weird to have different proxy configs for private/non-private mode. Would we need a new UI.

(Discussed this with Add-ons Product this morning, putting our position here.)

I don't think we need a new UI. In the case of a PAC script installed by an extension, it's applied (in general) in non-private browsing mode (PBM). If that extension is permitted to run in PBM, then we want the PAC script to also be applied in PBM (just as it is in non-PBM).

I can't speak to the implementation, but given the context of PBM, it makes sense that any PAC script that can be applied in a way which is more specific than browser-wide should only apply to the context it is granted (or rather, excluding the context it is not explicitly granted).

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Hi Shane, I think this is fixed now that we have nsIProxyInfo.sourceId

Comment 10

[Shane Caraveo (:mixedpuppy)]

redirecting ni to rob