Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uMatrix WE problem in combination with other WEs [Bug 1417249, 1477696, 1421725...] #265

Closed
earthlng opened this issue Nov 10, 2017 · 78 comments

Comments

@earthlng
Copy link
Contributor

earthlng commented Nov 10, 2017

@gorhill, I found a problem with uMatrix in that, depending on the order in which FF loads the addons, uMatrix "restores" the Content-Security-Policy header when another addon already changed it or removed it completely. Maybe other headers are affected as well, IDK.
uBO doesn't have the same problem so I assume you already fixed it there.

Since you closed your Issue tracker for uMatrix I didn't know how else to contact you. If you're interested let me know, I have a simple example addon ready to share and the steps to reproduce. Thanks

@gorhill
Copy link

gorhill commented Nov 10, 2017

uMatrix "restores" the Content-Security-Policy header

Were 1st-party scripts blocked?

@earthlng
Copy link
Contributor Author

Yes but I just re-tested with 1st-party scripts allowed and it's the same.

@earthlng
Copy link
Contributor Author

earthlng commented Nov 10, 2017

tested in FF nightly 58 with uMatrix 1.0.1rc3

example addon:
manifest.json

{
  "name": "demo",
  "version": "0.1",
  "manifest_version": 2,
  "background": {
    "scripts": ["background.js"]
  },
  "permissions": [
    "<all_urls>",
    "webRequest",
    "webRequestBlocking"
  ],
  "applications": {
    "gecko": { "id": "[email protected]" }
  }
}

background.js:

function logResponse(e) {
  for (let header of e.responseHeaders) {
    if (header.name.toLowerCase() === "content-security-policy" || header.name.toLowerCase() === "test-test") {
      console.log(e.url + "\n" + header.name+":", header.value);
    }
  }
}
function editResponseHdrs(e) {
  e.responseHeaders.push({'name': 'Test-Test', 'value': 'Test123'});
  e.responseHeaders.push({'name': 'content-security-policy', 'value': ''});
  return {responseHeaders: e.responseHeaders};
}
browser.webRequest.onHeadersReceived.addListener(editResponseHdrs, { urls: ["http://*/*", "https://*/*"] }, [ "blocking", "responseHeaders" ]);
browser.webRequest.onResponseStarted.addListener(logResponse, { urls: ["http://*/*", "https://*/*"] }, [ "responseHeaders" ]);

example testpage: https://gist.github.com/arantius/f6fd80b1efad368a45ca35567bc31b18

steps to replicate:

  • install uMatrix, create the 2 files and load it as a temporary addon, open the console and visit the testpage
    • result: the test-header is output on console and the CSP is not => the demo addon removed the CSP header and I assume uMatrix sees that and doesn't need to append its special rule to it.
  • disable and re-enable uMatrix in about:addons then reload the testpage
    • CSP header output on the console --> somehow the demo addon's modifications are ignored and uMatrix still knows the original CSP header
  • clear the console, reload the demo addon in about:debugging then reload the testpage
    • CSP is now cleared again by the addon

I added the test header because I was wondering if uMatrix overwrites all modifications another addon made but it seems to only affect the CSP one.

At first I thought it might be because in my original addon I process the headers async but when I changed it to sync it didn't fix the problem.
I also think CSP is maybe not the only header where this could be a problem.
It particularly sucks because when I start Firefox it seems to always load my addon first and uMatrix always wins and I need to disable + re-enable my own addon for it to work as expected.

@gorhill
Copy link

gorhill commented Nov 10, 2017

I am not seeing anything wrong with uMatrix -- it append its own script-src unsafe-eval * only when first party scripts are blocked, so working as expected.

However the behavior of the webRequest API does not match the documentation, specifically:

If two extensions listen to onHeadersReceived for the same request, then the second listener will see modifications made by the first listener, and will be able to undo any changes made by the first listener.

uMatrix sees the headers without your changes, and your extension sees the headers without uMatrix changes -- depending on whichever listener was first called.

@earthlng
Copy link
Contributor Author

Yeah I guess that would explain what's happening. I assumed that mozilla's documentation is accurate but apparently not. If the behavior matched the documentation it wouldn't be a problem then, right?
Sorry for blaming uMatrix when in fact it seems to be a FF problem ;)
Can you inform them about it, please? I think they're more likely to fix it when you're the one reporting it.

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Nov 10, 2017

I have read a few times about the order of installation causing issues. Earthlng - have you tried installing uM and yur Ext in a TWO new profiles in the two orders possible to see if that makes a difference to which Ext beats the other?

If that works for you, then try uninstalling (and removing all traces) and reinstalling the applicable extension (edit: in your main everyday profile) - not sure if it will work as FF often remembers stuff - I remember a ticket gorhill was concerned about where something has a hook on it - I think it was uBo prefetching/hyperlink/WebRTC settings or something.

@earthlng
Copy link
Contributor Author

earthlng commented Nov 10, 2017

Even if that would work, every new addon update would probably fuck things up again.
They need to fix the behavior to match the documentation otherwise it's a complete mess and totally unpredictable and unreliable.

@earthlng
Copy link
Contributor Author

earthlng commented Nov 10, 2017

Every addon update reloads the background script(s) which re-registers the listeners, meaning the addon that just got an update ends up being the one that has the last word now, so to speak.
For example addon1 appends a cookie and a 2nd addon adds another cookie but since the 2nd addon doesn't see the modified cookie header from the 1st addon, the 1st addon's cookie is never set.

Edit: kind of a bad example because there's a special API to deal with cookies but it can also be done on the request level by modifying the cookie header and in that case it could be a problem.

@earthlng
Copy link
Contributor Author

https://bugzilla.mozilla.org/show_bug.cgi?id=1417249

@Thorin-Oakenpants Thorin-Oakenpants changed the title uMatrix WE problem in combination with other WEs uMatrix WE problem in combination with other WEs [Bug 1417249] Nov 30, 2017
@Atavic
Copy link

Atavic commented Dec 27, 2017

If possible, only one extension should change a specific header to avoid any problem. pyllyukko/user.js#348

@earthlng
Copy link
Contributor Author

Looks like they're not gonna fix this and instead will just change the documentation:
https://bugzilla.mozilla.org/show_bug.cgi?id=1421725#c1

That means it will always be like rolling a dice, and every time you start FF you'll roll the dice again.
Expect the unexpected if, for example, NoScript, uBlock Origin (in advanced mode!) and uMatrix are all used together because afaik they all work by changing the CSP header.

Roll the dice and hope for the best, I guess 🤦‍♂️

@gorhill
Copy link

gorhill commented Jan 22, 2018

At least Chromium reports conflicts to users, that's the least Firefox could do to bring awareness of possible incompatibility between extensions. Imagine an extension which relax CSP directives[1] after NoScript/uMatrix made them more strict -- surely a user would want to know a security/privacy extension is prevented from working properly because of some other extensions.

Comment:

adding cookies is not really a good use case for this

Sure, but extensions changing CSP header is quite a serious use case.


[1] I think Tampermonkey does this.

@gorhill
Copy link

gorhill commented Jan 22, 2018

Is this for cookies and headers and other stuff?

It's for request/response headers. "Cookies" is mentioned because the case brought forth was about the Set-Cookie header.

@gorhill
Copy link

gorhill commented Jan 22, 2018

Side note: I get a LOT of this crap: what does it all mean

Difficult to tell exactly, I would have to know exact URL + extensions and their configuration. It seems some in there are caused by uBO redirecting Google Analytics script to its neutered version.

@earthlng
Copy link
Contributor Author

And once again the argument is performance. I'd think users would prefer that their addons work as expected but what do I know.

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Jan 22, 2018

And once again the argument is performance. I'd think users would prefer that their addons work as expected but what do I know.

How do you two guys feel about putting this out in front via a ghacks article. Seems pretty serious to me

Edit: put some pressure on? We'd need a decent example of security/privacy failure or extension failure

@gorhill
Copy link

gorhill commented Jan 22, 2018

I only get a warning on my side with uBO + uMatrix:

Content Security Policy: Directive ‘frame-src’ has been deprecated. Please use directive ‘child-src’ instead.

With uBO + uMatrix, and this one is because of the "Forbid web workers" setting in uMatrix (warning occurs whether workers are blocked or not). I don't know where the other errors/warnings come from.

@earthlng
Copy link
Contributor Author

@gorhill you could setTimeout + re-register the listeners in the background script of uMatrix to somewhat make sure it's always the winner but that would only work as long as other addons don't start to do that as well. It would also not guarantee that uMatrix keeps having the last word when new updates of other addons are installed. You could do it in intervals but that would only make things even worse if others start doing the same thing, because it would continuously roll the dice during a session and not just on startup.

uBO with only a * * * noop rule should work just fine with uMatrix though, right?
Do no-remote-fonts rules in uBo work by blocking the request based on URL and request-type, or is that CSP-based too?

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Jan 22, 2018

^^ its Violent Monkey, which I only use for our three global scripts in the wiki

@gorhill
Copy link

gorhill commented Jan 22, 2018

uBO with only a * * * noop rule should work just fine with uMatrix though, right?

Well, a * * * noop is meaningless in uBO, there is no block/allow rules to override at * * * level. If you mean that without using dynamic filtering in uBO, there should be no clashing, it's not entirely the case: uBO injects a font-src CSP when "No remote fonts" is checked. Without such block switch however, yes, there should be no clashing.

@earthlng
Copy link
Contributor Author

Pants, if you want to test then check what gorhill alluded to regarding Tampermonkey.
If what he suspects is correct that would mean TM could completely override uMatrix' settings.

But please don't take this to ghacks! I still have enough faith in the mozilla devs to think that they'll do something about it when @gorhill is the one telling them how bad this is.

@earthlng
Copy link
Contributor Author

Well, a * * * noop is meaningless in uBO

well now I feel stupid, thanks xD
I think I added that rule when I noticed the about:addons page making requests to google-analytics and uBO didn't block it. So I removed everything in the whitelist and replaced the 2 default BTS rules with that single noop-everything rule. Does that make some kind of sense at least? ;)

@earthlng
Copy link
Contributor Author

earthlng commented Jan 31, 2018

@gorhill I found a site that illustrates the problem (or at least 1 problem): https://1337x.to/home/

Tested with the latest Firefox Nightly and the latest uBlock + uMatrix versions

CSP when uBlock comes after uMatrix (= disable + re-enable uBlock to achieve that) :
Content-Security-Policy: default-src 'self' * data: 'unsafe-inline' 'unsafe-eval'
-> result: site loads

umatrix comes last (= disable + re-enable uMatrix) :
Content-Security-Policy: script-src 'unsafe-eval' blob: *
-> result: site content is hidden and only the background image is visible

I've already disabled "i'm an advanced user" which I thought might have something to do with it,
and I also disabled font-blocking and CSP-reports blocking, for the same reason.
But apparently the csp comes from this blob filter in easylist:
|blob:$domain=101greatgoals.com|1337x.to ...
... and overwrites uMatrix's more restrictive rule.

Questions:

  1. Is there a way to completely prevent uBlock from changing the CSP header ie ignore these blob filters?
  2. Are there other filter types that can cause something similar?
  3. As for csp-report blocking and given that the header-processing across multiple extension is not very reliable, wouldn't it be better to block CSP-reports on the request-level with the csp_report ResourceType ? Or is that not reliable enough in your opinion?
    There's also the font ResourceType which, if blocked that way, could allow users to block both Fonts and CSP-reports in uBO without the risk of overwriting uMatrix' settings.

And if you don't mind, can you explain why uMatrix is using unsafe-eval instead of none, and what the blob: * is for? Thanks

@gorhill
Copy link

gorhill commented Jan 31, 2018

Is there a way to completely prevent uBlock from changing the CSP header ie ignore these blob filters?

No. I will add an advanced setting in uBO to disable completely CSP header injection/modification.

Are there other filter types that can cause something similar?

Yes, filters with csp= option (currently quite rare).

As for csp-report blocking

No issue here. uBO blocks the csp-report network requests, this is unrelated to the modification of response header. uMatrix injects a CSP-Report (with an invalid URL) only so that a SecurityPolicyViolation event is triggered in the DOM.

can you explain why uMatrix is using unsafe-eval instead of none

Because the rule related to that CSP directive is to block use of inline-script tags, not eval(), which could be used legitimately in some non-blocked (1st- or 3rd-party) scripts.

what the blob: * is for?

To be able to enforce the filters such as |blob: as seen in EasyList. Blob-based URLs do not go through the webRequest API, and thus the only way to block them is through a CSP directive.

@earthlng
Copy link
Contributor Author

No. I will add an advanced setting in uBO to disable completely CSP header injection/modification.

@gorhill have you had the time to implement this yet? I looked through all your release notes but couldn't find anything related.

uMatrix injects a CSP-Report (with an invalid URL) only so that a SecurityPolicyViolation event is triggered in the DOM.

I assume this is used to detect if workers are used, right? Anything else?

Firefox recently added a new pref security.csp.enable_violation_events but AFAIK it still defaults to false ie violation events are disabled - I guess that means uMatrix works with or without this pref enabled?

@gorhill
Copy link

gorhill commented May 31, 2018

Sorry, not implemented.

Yes, uMatrix uses security policy violation events to detect workers. I didn't know about security.csp.enable_violation_events -- I checked and it looks like this is breaking uMatrix's ability to detect workers.

Edit: related: https://bugzilla.mozilla.org/show_bug.cgi?id=1425993

@Thorin-Oakenpants
Copy link
Contributor

You'll have to ask tartpvule - create an issue at his repo where everything in the one place

@Renan19
Copy link

Renan19 commented Apr 8, 2019

@Thorin-Oakenpants

You'll have to ask tartpvule - create an issue at his repo where everything in the one place

I already created with no awaser yet, i will wait.
i asked here too because i see the conversation about this and maybe anyone here can help with this.

@earthlng
Copy link
Contributor Author

earthlng commented Apr 8, 2019

@Renan19

Hi,

@tartpvule wrote the patch based on the ESR code and it might not necessarily work in newer versions, or the underlying code could have changed in subtle ways or it might break with any future release.
If you want to use it, you should best check if the modified code is still somewhat the same.
If you just want to use it as is and hope for the best, then here's how you enable it:

create a file autoconfig.js in <install-dir>\defaults\pref\ with this content:

// Any comment. You must start the file with a single-line comment!

pref("general.config.sandbox_enabled", false);
pref("general.config.filename", "monkey_Bug1421725_WebRequest.cfg");
pref("general.config.obscure_value", 0);

and place the monkey_Bug1421725_WebRequest.cfg file in the root Firefox path (where firefox.exe is located).

FYI disabling the autoconfig sandbox with general.config.sandbox_enabled seems to still be allowed in Release at the moment but they already said that they only want to keep allowing it to be disabled in ESR in the future.

hope that helps :)

@tartpvule
Copy link

@Renan19 see the issue in my repo.
@earthlng About monkey_AddonSettings.cfg that you tried, have you set xpinstall.signatures.required to false? What is the result of Components.utils.import("resource://gre/modules/addons/AddonSettings.jsm", {}).AddonSettings.REQUIRE_SIGNING in Scratchpad in about:config with and without the monkeypatch?

@Renan19
Copy link

Renan19 commented Apr 15, 2019

@earthlng @tartpvule
Thanks, tested and it is working fine now on firefox lastest version.

@earthlng
Copy link
Contributor Author

@tartpvule

have you set xpinstall.signatures.required to false?

I don't remember but I'm pretty sure I did. I had also tested it with a modified version of your script that didn't read the prefs and instead used hardcoded values but that didn't work either.

What is the result of ...

I just tested it again and REQUIRE_SIGNING is false with the patch and true w/o it. Ran it on both about:config and about:addons and the values are the same ie it should theoretically allow me to install unsigned extensions but it doesn't and IDK why. Maybe something to do with lazy loading of components that they changed since ESR or FF somehow keeping a cache of the values, I have no idea.

@tartpvule
Copy link

@earthlng Thank you for pointing out the problem
As it turns out, my monkey_AddonSettings.cfg has not been working for quite a while, even in ESR 60.
The problem is that dependent modules like XPIProvider.jsm would have already acquired a reference to the old AddonSettings by the time the patch is applied (a race condition? I don't know).
I have updated the file, though "no reliability guarantees", at least it seems to work on my ESR 60 enough and that's what I am focusing on.

@Thorin-Oakenpants
Copy link
Contributor

from Jan 2018 #265 (comment)

Pants... but please don't take this to ghacks! I still have enough faith in the mozilla devs to think that they'll do something about it when @gorhill is the one telling them how bad this is.

It wasn't me: https://www.ghacks.net/2019/05/23/firefox-csp-issue-may-cause-extension-conflicts/

But given that HTTPS-E are aware of the issue, and that extension is used in Tor Browser, and given that it finally got an issue at uBlock-issues, and some other external factors - it seems as if KM is considering doing something about it. Just watch the relevant bugzillas

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented May 25, 2019

I don't get it. WTF is wrong with people. A non-event such as a pref to control profile/chrome/ lookups, where nothing is lost, gets so much outrage and attention and yet this issue which actually breaks things and can compromise a user's security and privacy gets absolutely no comments or anything.

reddit-csp

reddit-chrome

r/Firefox usually has some knowledgeable rational and well spoken people - but I can't understand why no-one has picked up on it. Are they all fixed fixated (edit) on looks over form?

@Thorin-Oakenpants
Copy link
Contributor

why are you confused @gwarser, or is that just the best of a limited set of emojis you could choose ?

I don't expect everyone on r/Firefox to understand the ramifications of this bug, but not a single person has commented, and yet they're always happen to discuss in depth technical issues about other security issues. Isn't anyone there concerned that their extension can often not work as expected? Extensions that help security, tracking, FPing ...

I think I'll do a Wonko the Sane and build an inside out house called The Outside of the Asylum to house the world. I shall live inside it and any of you are welcome to come visit (if you are sane)

@jingofett
Copy link

I don't get it. WTF is wrong with people. A non-event such as a pref to control profile/chrome/ lookups, where nothing is lost, gets so much outrage and attention and yet this issue which actually breaks things and can compromise a user's security and privacy gets absolutely no comments or anything.

reddit-csp

reddit-chrome

r/Firefox usually has some knowledgeable rational and well spoken people - but I can't understand why no-one has picked up on it. Are they all fixed fixated (edit) on looks over form?

https://old.reddit.com/r/firefox/comments/btvixo/firefox_bug_causes_addons_ublock_origin_https/ this thread seems to be getting a bit more traction and has a more informative headline

@Thorin-Oakenpants
Copy link
Contributor

Well, I kept track of that for all of 8 hrs. It trended up very quickly into second place on hot (after the PSA userChrome thing which I think is a sticky). 251 points, 35 comments. It was there when looking at new items in its right place. Now you can't find it unless you know it exists. It doesn't even show when looking at items in chronological order. r/Firefox, AFAIK, has demoted it so it doesn't show. And it has now died and nothing has changed for over an hour.

@Thorin-Oakenpants
Copy link
Contributor

yup .. shadowbanned. I don't understand why? People are now asking why it's shadowbanned. FFS, do they not want a better product? Fucking jerks. For some reason they've always hated this repo and associated it with the Mozilla haters at ghacks itself

@jthile
Copy link

jthile commented May 28, 2019

It just popped back up, but it was definitely shadowbanned for a few hours. Maybe there was disagreement among the mods of the sub. Still very disappointing to see that suppressing information about a bug is on the table at all.

@Thorin-Oakenpants
Copy link
Contributor

I guess its par for the course (I don't hang around and read these sorts of things) but 2/3rds of the comments are off topic - I guess you get that with everyone wanting to chime in and bitch about something, and/or lack of knowledge

@Thorin-Oakenpants
Copy link
Contributor

OMFG ... https://bugzilla.mozilla.org/show_bug.cgi?id=1462989#c24

Hey folks, we're getting a lot of advocacy chatter on this bug, so I'm going to close comments on it for the time being

First of all, only one comment was marked advocacy - five days ago. No-one has said anything on the bug for over 3 days. WTF? Way to win friends and influence people. Fucking bollocks. I don't care about looking backwards, I just want the problem addressed. But they seem to have their mind somewhere else right now. WTF?!

@shree1717
Copy link

Instead of begging Mozilla on our knees wouldn’t it make more sense to

  1. write an extension that taps into the WebRequest API and provides a way for other extensions to register with this extension in some (user-configurable?) way
  2. have co-operating extensions (uMatrix, uBlock0, CanvasBlocker, I would hope) use this extension instead of WebRequest directly?

I know, it’s a tall order, but at least it seems a way forward.

@claustromaniac
Copy link
Contributor

claustromaniac commented Jun 9, 2019

I considered that idea a long time ago, but there would be more than a few problems with that, technically speaking... It can be done, but it wouldn't be easy for extension developers to adopt in the least. They would need to make pretty extreme changes to their respective extensions and, even then, that approach wouldn't allow them to do some of the things they can do now. It's not worthwhile.

@tartpvule
Copy link

tartpvule commented Jun 15, 2019

I just developed a new experimental patch for Bug1421725 against mozilla-central, not yet extensively tested.
This patch do what MDN says: "the second listener will see modifications made by the first listener".
Anyone interested in checking it out?

https://github.com/tartpvule/my-firefox-patches/blob/master/Bug1421725_WebRequest_central.patch

@girst
Copy link

girst commented Jun 28, 2019

@tartpvule: I've tested the monkey-patch version and it seems to work. I've got a relatively small test case that I used to verify it:

  1. install canvasblocker and javascript toggle
  2. toggle js off by clicking the 'js' icon in the toolbar
  3. dis- and reenable canvasblocker, so its csp directives get applied
  4. visit https://gir.st/tmp/javascript.html

do you have intentions of submitting this patch to mozilla? (please do!)

@tartpvule
Copy link

@girst Thank you for your interest in my patch.
https://bugzilla.mozilla.org/show_bug.cgi?id=1421725
The patch was rejected, as expected, citing performance reasons :(
I don't see any performance regressions in my daily use, though.

Oh well, back to maintaining private patches and builds I guess.
IMO: "merging" headers is a mess of glaringly obvious edge cases.

@crssi
Copy link

crssi commented Jul 28, 2019

The patch was rejected, as expected, citing performance reasons :(

Yes, that is shame, I really don't know what is the hold reason at Mozilla... 😞

I don't see any performance regressions in my daily use, though.

... and performance for sure its not the real reason.

@girst
Copy link

girst commented Jul 29, 2019 via email

@M83tUt3
Copy link

M83tUt3 commented Aug 23, 2019

@tartpvule
Thanks for your work. Is this patch still valid for FF68? I'm doing my own builds anyway so I could easily apply it myself. I'm also staying on ESR so if it still works I'll be good for another year, which would be wonderful.

@tartpvule
Copy link

tartpvule commented Aug 24, 2019

@M83tUt3
Use my Bug1421725_WebRequest_central.patch from https://github.com/tartpvule/my-firefox-patches/blob/4dd8c06da18d2137d86ea43a2a4f936b5b100228/Bug1421725_WebRequest_central.patch, after patching in https://hg.mozilla.org/mozilla-central/rev/308ef98e6e4b (Bug 1450965)

After ESR68.1 is released and I finalized my private build, I will update my Firefox patches repo with updated instructions.

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Sep 9, 2019

... aaaaand now they changed the docs

edit: FYI: @gorhill @gwarser @uBlock-user

@crssi
Copy link

crssi commented Sep 9, 2019

^^ lamest way to do it. 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests