Feat/replace blocking web request listeners [WIP]

[kabir-afk]

Description

As the title suggests we had to replace blocking web request listeners with delarativeNetRequest API. The declarativeNetRequest API is used to block or modify network requests by specifying declarative rules. This lets extensions modify network requests without intercepting them and viewing their content, thus providing more privacy. Also it turns out to be more performant than what was being used earlier. If we talk about browser compatibility , then majority of its methods are accessible and compatible across all browsers. Some methods like getMatchedRules is not compatible in FF and similar methods exist for other browsers as well. Not that they were used here, but still , it is something to be kept in mind.

Also , despite the issue #100 being raised in context of background-script.js , the changes apply to both chromium browsers and FF, since changes are being made to manifest.json, they apply to both.

Question that still remains

Why were we modifying headers to begin with ? Like I am aware that we often need to modify headers but what were we achieving here by doing that ?

Changes made :

Anyways , despite all of this, I went ahead to change the current piece of code. I believe all the commit messages are self explanatory. I'd like to emphasize on test : confirming whether the headers have been modified by DNR. As per DNR API , minimum number of static rules that we can add is 30,000 as per mdn. So in order to confirm that our given rule has been added , we should get a number less than 30k i.e., 29,999. There are other ways to confirm it as well , like checking the CSP response header under network tab of DevTools.

Issue

While everything remains same on the front-end , things indeed change with its introduction. Videos stop working on youtube site. They stop at thumbnail. While nobody is going to use our extension on Youtube definitely , if installed and is active can lead to client side confusion. It has something to do with the way I am modifying the header, I haven't fully understood this yet ,hope @hugolpz and @Ishan-Saini can look into it , but with this present #100 can't be closed yet. I'll be refactoring this later , adding either more rules or some kind of urlFilter but until then this is what I could come up with.

[hugolpz]

Hello @0x010C , We are advancing well thank to @kabir-afk who is fighting forward . There is something we don't understand in SignIt, maybe you would be willing to give some rapid legacy code insight on the following :

Why were we modifying headers to begin with ? Like I am aware that we often need to modify headers but what were we achieving here by doing that ?

[0x010C]

Hi @kabir-afk and @hugolpz, I can explain this CSP header modification story to you, but be careful: everything I'm going to write below was valid in 2019 when I wrote the first version of SignIt, I haven't followed the changes since and I I'm not up to date on Maniphest v3.

CSP is a mechanism that allows system administrators of a website to indicate to browsers on which domains they are authorized to retrieve scripts and media, via an HTTP header on the main HTML document of the page. This is a protection to limit the impact of a possible XSS vulnerability. See MDN doc for more details.

To display the popups, SignIt locally injects a JS script into the web page, which adds some cosmetics and, most importantly, the video player. Because of this, the video player runs in the same context as the web page, and is therefore subject to its CSP rules. If there is a rule prohibiting media from all domains except (for example) youtube.com, then all requests to fetch a video from commons.wikimedia.org will be rejected by the browser.

This was the objective of this commit: to intercept responses to add the domain commons.wikimedia.org in the CSP header (if set) before they are taken into account by the browser. Thus, our domain is always in the list of authorized domains and SignIt can work without problems on all websites.

I hope this answers your questions and I wish you the best with your work on SignIt. Best regards, 0x010C

[hugolpz]

Thank Kabiraaa for this PR and associated explanations. Thanks too to @0x010C .

I reviewed the changes on github, LGTM but I didn't test it.

@Ishan-Saini hello , could you review this PR as well ?

[EDIT:] Ishan clarified that further commits are expected on this PR.