Consider merging new mergable headers that didn't exist in the original request

NEW
Unassigned

Status

enhancement
P3
normal
Last year
11 days ago

People

(Reporter: sscarl2417, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3435.0 Safari/537.36

Steps to reproduce:

Have two extensions which apply CSP policies for websites installed. For ex. uBlock Origin and uMatrix.

1) Add a CSP filter such as ||youtube.com^$csp=img-src 'none' in uBlock Origin under My Filters tab and save it.

2) In uMatrix, open the dashboard, browse to My Rules and add "no-workers: * true" and save it which will apply a CSP filter such as worker-src 'none' when browsed to any website.

3) Visit https://www.youtube.com/ and notice none of the images are blocked even though a CSP policy for img-src was set by uBlock Origin was set, but rather Firefox only applies uMatrix's worker-src CSP.

PS - This ONLY occurs in Firefox, while with Chromium, both CSP policies from both the extensions are applied.


It might be related to this - https://bugzilla.mozilla.org/show_bug.cgi?id=1377689



Actual results:

Images not blocked despite img-src CSP policy being set by uBlock Origin which can been seen from the console screenshot here - 

https://i.gyazo.com/862e806400876750681e71fda82c7eb7.png


Expected results:

Images should have been blocked because a CSP policy for images is being set by uBlock Origin along with uMatrix's worker-src CSP policy.
I can reproduce this on Firefox v61/62(Nightly.)
PS - This happens ONLY when there is no original CSP header in response.
Component: Untriaged → WebExtensions: Request Handling
Product: Firefox → Toolkit
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: If two CSP policies from two extensions are set, Firefox only applies one → Consider merging new mergable headers that didn't exist in the original request
Priority: -- → P2
Product: Toolkit → WebExtensions
Is this still being worked on ?
It is still in the queue, yes. But it's not under active development right now.
Which queue ?
It is not a literal queue; P2 means we will work on it after the critical things we're doing now (P1) are done.
New STR as old one is no longer relevant --



Add a CSP filter such as ||w3schools.com^$csp=default-src 'self' 'unsafe-inline' 'unsafe-eval' in uBlock Origin under My Filters tab and save it.

Install uMatrix from https://github.com/gorhill/uMatrix/releases/tag/1.3.17b0 by clicking on the file name uMatrix.firefox.signed.xpi. In uMatrix, open the dashboard, browse to My Rules and add "no-workers: * true" and save it which will apply a CSP filter such as worker-src 'none' when browsed to any website.

Visit https://www.w3schools.com/html/tryit.asp?filename=tryhtml5_webworker and open the uMatrix logger(via opening moz-extension://41cfb02c-c075-468a-b815-858fdbc2a15c/logger-ui.html in a new tab), now go back to the w3schools tab and click Start worker, the count will begin and check the logger tab we opened in a new tab before, the request appears but is not blocked, even though it should have been because of no-workers : * true rule which applies the worker-csp to block all workers.

This bug is really relevant for security as it allows to execute malicious javascript if it is disabled.

(In reply to Klaus Ethgen from comment #8)

This bug is really relevant for security as it allows to execute malicious javascript if it is disabled.

Sorry, I don't understand what you mean. Can you clarify?

(In reply to Kris Maglione [:kmag] from comment #9)

What Klaus means is that when two add-ons 'fight' for CSP policies, the less strict might win, and this can cause third party Javascript to be downloaded and executed.

This is a real problem I face regularly; µMatrix is overridden by some other add-on. This causes some Javascript to load, until µMatrix is dis- and then reenabled, putting it back on top of the priority list.

Unless the priority level gets bumped to P1, this bug is not getting fixed anytime soon.

This bug outright breaks extensions. I have several extensions that modify csp, but only one of them can go through, and I can't add any extension that requires it. The worst part about this is that the average user might not notice anything wrong, as this bug can be, for the lack of a better term, silent unless the extension is completely broken, like HTTPS Everywhere's EASE/Any addon that injects the upgrade mixed content header.

Type: defect → enhancement

@Kris Why is it tagged now as enhancement ? It's a bug, it's not something I requested to enhance Firefox.

It's not a bug. It works as documented and expected. Changing the expected behavior in cases like this is an enhancement.

So was breakage of extensions also expected ? Because this only happens in Firefox and not in Chromium and why was this considered expected in the first place if ultimately it breaks extensions ?

The same thing happens in Chromium, at least, if they still follow their documented behavior. If multiple extensions modify the same header, there is a conflict, and no matter how we try to resolve that conflict, it will upset someone. Chrome's documented behavior is "If more than one extension attempts to modify the request, the most recently installed extension wins and all others are ignored" though its actual behavior may differ.

The same thing happens in Chromium

AFAIK, the headers are merged to avoid this issue happening there. uBO inserts a CSP in Chromium the same way it does in Firefox, but yet headers are merged in Chromium and not in Firefox, this is why I call it a bug as the merging of headers is something I expected on the contrary.

Not sure what info you need from me here. We don't recommend installing additional extensions or messing with extensions settings (as that could have weird side-effects endangering your privacy) + EASE mode is off in HTTPS Everywhere.

Flags: needinfo?(gk)

Chrome's documented behavior is "If more than one extension attempts to modify the request, the most recently installed extension wins and all others are ignored"

That is a generic sentence. You ignored the more specific preceding sentence -- which actually explains why it works with Chromium:

Only one extension is allowed to redirect a request or modify a header at a time.

"A header".

NoScript, uBO, uMatrix, AdGuard, etc. are not modifying "a[n existing] header", they are pushing a new one, so there should be no such limitation with regard to mergeable headers (i.e CSP headers), which Firefox legacy perfectly supported. The hint that this is the key sentence to pay attention to is that merging CSP headers work fine in Chromium.

Of course it makes sense that the same existing, non-mergeable header can't be modified by two extensions, one would overwrite the other's change, but here extensions are injecting a new header (which happens to be a mergeable one), not modifying an existing one.

Also:

It works as documented and expected.

It does not even work as documented. The documentation says:

If two extensions listen to onHeadersReceived for the same request, then the second listener will see modifications made by the first listener

uBO can't see modifications made by other listeners. This would be better than the current situation, at least this would allow me to use an already existing code path in uBO to deal with platforms which do not support merging of headers -- but even the documentation is wrong, uBO is unable to see the CSP header injected by another extension.

Tails OS ships the Tor browser with both uBlock Origin and NoScript. There are over 300 CSP-injecting rules in uBO with default filter lists which may silently fail if there happens to be another extension injecting a CSP header, and that is not counting the JS/font switches. NoScript uses some trickery to ensure its CSP headers are injected, and the dismissal of the seriousness of the issue here suggest that all extensions which want their important CSP-tightening to be honored should do the same. This will risk users ending up with a broken browser but at least they will become aware of the incompatibility of some of their installed extensions and force them to make a choice.

The webRequest API has already been modified to accommodate for actually needs in the actual world, no "but Chromium documentation" was invoked as an obstacle to fix/enhance the API (ex: browser.webRequest.filterResponseData). Why not do the same for this critical shortcoming and provide an API to explicitly merge new (mergeable) headers given a request id?

(In reply to Kris Maglione [:kmag] from comment #16)

If multiple extensions modify the same header, there is a conflict, and no matter how we try to resolve that conflict, it will upset someone.

This cannot be "merging headers" problem, because merging already works!
There is no issue when response contains their own, original CSP header.
For example, here on bugzilla:

  • add ||bugzilla.mozilla.org^$csp=img-src 'none' to uBO Dashboard -> My filters
  • block scripts in µMartix (mark "script" cell red in popup) (µMarix uses CSP to block inline scripts)
  • open console and reload page

Result: img-src and script-src are in effect.

Problem is - shady websites, where we will benefit from this type of blocking most, don't use CSP.

Supporting multiple extensions to inject mergeable headers (with no need to see each others' injection, therefore coherently to current specs) and be accounted for by the underlying HTTP header merging algorithms, rather than all dropped except for the last one, wouldn't cause any conflict or performance issue I could think of, and wouldn't require any special new API IMHO.
CSP in particular can only be tightened, never relaxed, by subsequent headers, therefore I can't see any security issue either.
And this could allow me to remove those awful hacks allowing NoScript to be always the last to inject (and breaking everyone else).

If the ToR browser can unexpectedly execute Javascript, as Gorhill explained, and Giorgio's suggestion has no disadvantages except the implementation work required, that sounds like a good solution.

Would it then still be possible for extensions to remove/ignore CSP headers set by the page itself? (I believe the philosophy behind Firefox is that the browser should not let other parties (website owners) restrict what users can do without the possibility of an override of sorts?)

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. If you're following this bug, you'll receive notifications if it moves into active development.

Restrict Comments: true

Could you please reopen this for comments? Comment activity and dupes are a useful measure in detecting issues devs feel are important to address.

Flags: needinfo?(cneiman)
Priority: P2 → P3

Re-opening comments.

Flags: needinfo?(cneiman)
Restrict Comments: false

Why set priority to P3 ?

Also what addional info do you need ?

I'd say most users have no idea what CSP is, and would not know why their extensions are failing.

The important thing here is that to the end user, there is zero indication that only one CSP extension is allowed. Extension developers shouldn't have to put a big fat warning that that indicates every feature that uses CSP. The end user shouldn't have to search online to find out why their extensions don't work as intended. And that's if they realize the extension doesn't work in the first place.

For an example use case, I have three extensions that modify CSP, uBlock Origin, HTTPS Everywhere, and CanvasBlocker. If I want uBlock Origin at its full potential, I can't use EASE from HTTPS Everywhere, nor can I have the "Block data URL page" option checked in CanvasBlocker, which is hidden under the "Expert Settings" options, and is enabled by default. And it was only until I tried enabling EASE that I realized that things weren't working right. There was zero indication that uBlock Origin and CanvasBlocker conflicted with each other.

Some WebExtensions will conflict with each other. Having two tab hiding extensions is obviously going to cause a conflict. But uBo, HTTPS Everywhere, and CanvasBlocker serve three completely different purposes. It's illogical for them to have any sort of conflict. At the end of the day, the way things are right now is completely user hostile.

Either all extensions should be able to modify CSP at the same time, or none of them should be able to modify CSP at all.

You need to log in before you can comment on or make changes to this bug.