Closed Bug 1309926 Opened 8 years ago Closed 7 years ago

Allow uBlock Origin to be a WebExtension

Categories

(WebExtensions :: Compatibility, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: andy+bugzilla, Unassigned)

References

Details

(Whiteboard: triaged[awe:uBlock0@raymondhill.net])

This is a general tracking bug to allow uBlock Origin to be a WebExtension.
No longer blocks: 1190687
Depends on: 1190687
Depends on: 1310026
Ok, I created a WebExtensions uBO by wholly importing the Chromium code, and adjusting where necessary to find out what issues I face from converting uBO as a webext.

Things I had to adjust in the code to be able to launch without errors in Nightly:

- Add code to test for the existence of chrome.privacy, and abort its use if it does not exist. uBO currently needs to be able to set the following privacy-related settings (with FF's corresponding about:config entries listed):

  - Disable pre-fetching
    - about:config / network.prefetch-next
    - about:config / network.http.speculative-parallel-limit
    - about:config / network.dns.disablePrefetch
  - Disable hyperlink auditing
    - about:config / browser.send_pings
  - Prevent WebRTC from leaking local IP addresses
    - about:config / media.peerconnection.ice.default_address_only (if it exists)
    - about:config / media.peerconnection

- Comment out the call to chrome.webNavigation.onCreatedNavigationTarget.addListener (reportedly now fixed as seen above)

- Add code to conditionally use chrome.storage.managed and chrome.storage.sync, which are not yet available for Firefox.

This allowed uBO-webext to be able to load successfully in Nightly.

My first test was to launch a Google search, using "buy car" (which invariable returns a page with ads). However an error was reported at the console, and I have no idea how to fix this one.

uBO uses chrome.runtime.onConnect.addListener to manage communications between content scripts, background page (and its Dashboard/logger pages when opened). Once uBO receives an event that a connection occurred, it registers a message listener for the port passed as an argument. Now the problem occurs when that message listener is called, it misses the second argument, and this breaks uBO.

As per Chrome API documentation, the second parameter to a port.onMessage listener is the port that received the message[1]:

> This event is fired when postMessage is called by the other end of the port.
> The first parameter is the message, the second parameter is the port that
> received the message.

So this is currently a showstopper on my side to convert uBO to webext.

1. https://developer.chrome.com/extensions/runtime#type-Port
I forgot to mention that the port issue above is an issue which prevents the content script code from doing its job.

However it does appear that uBO's webRequest code works fine with a couple of very quick tests. So uBO can block content at network level, but is broken for everything which requires message sending through ports:

- Content scripts => uBO's main process
- Popup panel => uBO's main process
- Dashboard => uBO's main process
- Logger => uBO's main process
> the second parameter to a port.onMessage listener is the port that received the message

I am guessing this would be the place where the second argument, portObj, is missing:

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/ExtensionUtils.jsm#1225

Once this is fixed, I am quite optimistic we would have a well working 1st version of webext-based uBO.
Depends on: 1311128
Depends on: 1312802
Depends on: 1313510
Depends on: 1299411
I confirm the port issue is fixed with today's Nightly. From now on, I will release a Firefox's WebExtension version of uBO for those who would want to check it out: https://github.com/gorhill/uBlock/releases  -- see uBlock.webext.zip.

Currently, the webext version of uBO is to plainly use the Chromium version of uBO, with only a trivial patch to the manifest.json file and the removal of the chromium-specific polyfill code (needed for older versions of chromium)[1]. So as I had expected, simply using the chromium code essentially worked as is, expect for the other issues reported here.

[1] https://github.com/gorhill/uBlock/tree/master/platform/webext
Whiteboard: triaged → triaged[awe:uBlock0@raymondhill.net]
(In reply to R. Hill from comment #1)
> Ok, I created a WebExtensions uBO by wholly importing the Chromium code, and
> adjusting where necessary to find out what issues I face from converting uBO
> as a webext.
> 
> Things I had to adjust in the code to be able to launch without errors in
> Nightly:
> 
> - Add code to test for the existence of chrome.privacy, and abort its use if
> it does not exist. uBO currently needs to be able to set the following
> privacy-related settings (with FF's corresponding about:config entries
> listed):
> 
>   - Disable pre-fetching
>     - about:config / network.prefetch-next
>     - about:config / network.http.speculative-parallel-limit
>     - about:config / network.dns.disablePrefetch
>   - Disable hyperlink auditing
>     - about:config / browser.send_pings
>   - Prevent WebRTC from leaking local IP addresses
>     - about:config / media.peerconnection.ice.default_address_only (if it
> exists)
>     - about:config / media.peerconnection

Can you relate these WebRTC prefs back to Chrome's options? Chrome allows one to specify one of the following values for IPHandlingPolicy [1]:
- "default"
- "default_public_and_private_interfaces"
- "default_public_interface_only"
- "disable_non_proxied_udp"

If we implement Chrome's privacy.network.webRTCIPHandlingPolicy, to which of those values would you be setting the IPHandlingPolicy? I want to make sure that our mappings match what you need.

[1] https://developer.chrome.com/extensions/privacy#type-IPHandlingPolicy
Flags: needinfo?(rhill)
> If we implement Chrome's privacy.network.webRTCIPHandlingPolicy, to which of those values would you be setting the IPHandlingPolicy?

uBO uses "disable_non_proxied_udp" if the current setting is "disable_non_proxied_udp", otherwise it will use "default_public_interface_only". In other words, uBO won't select a setting which results in lower privacy, only use a setting value which results in same or higher privacy level.

The Chromium code can be seen here:
https://github.com/gorhill/uBlock/blob/a0c172c13e689c9df2a7f57ada1e115fa4d3db25/platform/chromium/vapi-background.js#L165
Off-topic: Is there an easy way to quote text without having the quoted text end up as a single line with an horizontal scrollbar? I usually find manually but I forgot (again) above.
(In reply to R. Hill from comment #7)
> > If we implement Chrome's privacy.network.webRTCIPHandlingPolicy, to which of those values would you be setting the IPHandlingPolicy?
> 
> uBO uses "disable_non_proxied_udp" if the current setting is
> "disable_non_proxied_udp", otherwise it will use
> "default_public_interface_only". In other words, uBO won't select a setting
> which results in lower privacy, only use a setting value which results in
> same or higher privacy level.
> 

Okay, so we will be implementing all four of Chrome's possible values for webRTCIPHandlingPolicy, which should handle your use cases for both "disable_non_proxied_udp" and "default_public_interface_only". You also listed "about:config / media.peerconnection" in your initial comment, but I'm not sure to what that refers. There is no pref simply called "media.peerconnection" although there is "media.peerconnection.enabled". Is that what you meant, and if so when would you need to set that?

As I understand it, that is used to enable/disable the ability to create RTCPeerConnection objects. Is that what you need? If so, do you think it would make sense to make that correspond to a new possible value for webRTCIPHandlingPolicy, or how else do you imagine having access to it via the privacy API?
Flags: needinfo?(rhill)
Flags: needinfo?(rhill)
I have yet another question for you, R. Hill: In trying to determine exactly which Firefox prefs should be flipped to correspond with each of these values for webRTCIPHandlingPolicy, there has been some confusion about the "disable_non_proxied_udp" setting. You seem to have a good handle on which prefs you need in Firefox, so I wonder if you have an opinion. Here's a quote from someone who works on WebRTC regarding Chrome's disable_non_proxied_udp value:

"The documentation for that is not very good; it could be interpreted to mean media.peerconnection.ice.relay_only, or it could be interpreted to mean media.peerconnection.ice.proxy_only. It depends on whether they're using the word "proxied" correctly. If they actually mean "proxied", then it should be equivalent to proxy_only, and if they actually mean "relayed" it should be equivalent to relay_only. (There are two different mechanisms that could be limited here; there's the standard web proxy stuff, and on top of that there is the TURN media relay stuff, which are kinda similar in that they relay packets back and forth)"

Do you have an opinion about which in fact is meant by Chrome's disable_non_proxied_udp value?
> There is no pref simply called "media.peerconnection" although there 
> is "media.peerconnection.enabled". Is that what you meant, and if so 
> when would you need to set that?

Yes, I really meant "media.peerconnection.enabled".

uBO has a checkbox feature called "Prevent WebRTC from leaking local IP addresses".

In older versions of Firefox, there was no setting to specifically prevent the leakage of local IP addresses, so uBO was wholly disabling WebRTC through "media.peerconnection.enabled".

Once "media.peerconnection.ice.default_address_only" became available[1], uBO started to use this setting.

uBO's latest stable release, v1.10.6, uses "media.peerconnection.ice.no_host" (which is a new setting in Firefox 51)[2].

***

[1] Since version 42 according to my notes: https://github.com/gorhill/uBlock/wiki/Prevent-WebRTC-from-leaking-local-IP-address#firefox

[2] https://github.com/gorhill/uBlock/issues/2337
> Do you have an opinion about which in fact is meant by Chrome's
> disable_non_proxied_udp value?

I believe this may help?
https://bugs.chromium.org/p/webrtc/issues/detail?id=4784#c5

> Disabling this preference prevents non-proxied UDP to be used 
> as it leads to IP leak when using WebRTC. Currently, this is 
> effectively disabling all UDP traffic until UDP supporting proxy 
> is available in chrome.

This issue was originally reported for Chromium:
"When using a proxy, WebRTC leaks unproxied IP address even with multiple routes disabled"
https://bugs.chromium.org/p/chromium/issues/detail?id=497040

Reading through all this, I do wonder if uBO should instead use this setting, I would have to set up a test environment for this -- and so far nobody has reported an issue about this.
The initial implementation of the privacy API has landed on Nightly (although probably won't be available in a build until tomorrow). It implements the following, which was requested earlier in this bug:

privacy.network.networkPredictionEnabled, which interacts with the following prefs:
    "network.predictor.enabled",
    "network.prefetch-next",
    "network.http.speculative-parallel-limit",
    "network.dns.disablePrefetch"

privacy.network.webRTCIPHandlingPolicy, which interacts with the following prefs:
    "media.peerconnection.ice.default_address_only",
    "media.peerconnection.ice.no_host",
    "media.peerconnection.ice.proxy_only"

privacy.websites.hyperlinkAuditingEnabled, which interacts with the following prefs:
    "browser.send_pings"

I believe that addresses uBO's requirements with respect to the privacy API. Please let me know if that is not the case.
As far as I can tell, it's all working fine with the webext version of uBlock Origin.

Un-checking the option "Disable hyperlink auditing" does nothing really since hyperlink auditing is disabled by default with Firefox 51 (which is really the right thing to do). Still useful for uBO users in case the default setting changes in the future.
Flags: needinfo?(rhill)
This has no blockers and I've been running the WebExtension version for a while and it works great. Should we close this one?
Flags: needinfo?(rhill)
Sure, can be closed.
Flags: needinfo?(rhill)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Just found an issue which afflicts the webext version of uBO, but which does not affect the Chromium version of uBO, and neither the non-webext version of uBO on Firefox (I don't know about the webext Edge version at the moment).

The issue is that Firefox webext API may pass URL values with Unicode characters, i.e. it's not guaranteed that all URLs will be plain ASCII strings. With Chromium, all URLs arguments in the chrome API are always plain ASCII -- and this is what uBO expects.

However, I can't find documentation for the chrome API where this is explicitly documented.

My opinion is that the webext API should really be explicit that all URLs values are normalized to plain ASCII, otherwise this forces extension authors to add otherwise avoidable overhead JS code everywhere to normalize all URL values for all webext API entry points.

Test page: http://raymondhill.net/ublock/idn-tricks.html
More specifically, load http://еа.com/ in a tab: the URL was reported with Unicode characters -- i.e. non-plain ASCII string, and this glitches uBO's dynamic filtering popup panel -- the domain name is blank.
Could you spawn that into a seperate bug please?
Reported here: bug 1360285.
Status: RESOLVED → REOPENED
Depends on: 1360285
Resolution: WORKSFORME → ---
Depends on: 1367259
Depends on: 1271354
Depends on: 1313401
Depends on: 1379148
uBlock has been a WebExtension for a while so closing this.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.