1635781 - Cannot replace frame-ancestors directive of the Content-Security-Policy HTTP Header via the webRequest API

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P1)

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

I am the developer of the Tile Pages WE extension.

Tile Pages WE embeds http/https web pages in sandboxed iframe elements arranged in a tiled layout within a moz-extension: container page. If necessary, the extension removes X-Frame-Options headers and CSP frame-ancestors directives from the embedded web pages. This can be done because the container page is an extension page and is not hostile. This design approach was reviewed and approved by Philipp Kewisch (then Senior Add-ons Technical Editor at Mozilla).

In order to remove X-Frame-Options headers and CSP frame-ancestors directives, Tile Pages WE uses the webRequest API. Until recently everything worked fine, but the latest version of Nightly is behaving differently and I think incorrectly.

With Firefox Developer 76.0b8 and earlier versions, removing a 'frame-ancestors: none' directive from the CSP header allowed the page to be loaded into an iframe.

With Firefox Developer 77.0b2 and later versions, this no longer works. However, if the entire CSP header is removed, then the page can be loaded into an iframe, but of course this should not be done.

Actual results:

With Firefox Developer 77.0b2 and later versions, removing a 'frame-ancestors: none' directive from the CSP header does NOT allow the page to be loaded into an iframe. Instead a "Blocked by Content Security Policy" message is displayed.

Expected results:

Removing a 'frame-ancestors: none' directive from the CSP header should allow the page to be loaded into an iframe.

Comment 1

[Firefox Bug Husbandry Bot]

Because this bug's Severity is normal and has not been changed, and this bug's priority is -- (none,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Comment 2

[Rob Wu [:robwu]]

Are you using other extensions that use the webRequest API to modify response headers?

Could you attach a test case, and use mozregression to identify the commit that changed the behavior?
Based on the little details here, I guess that it may be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1595652#c13

Comment 3

[dw-dev]

Attachment: Frame Ancestors Test extension

An extension to be used as a test case to demonstrate that removing a 'frame-ancestors' directive from the CSP Header does not allow a page to load in an iframe when using FIrefox 77.

Comment 4

[dw-dev]

I do use other extensions, usually my own extensions (Print Edit WE, Save Page WE, Zoom Page WE, etc), but none of these use the webRequest API to modify response headers.

I have created a small extension called 'Frame Ancestors Test' (see attachment) to demonstrate this problem. This extension tries to open https://uk.ask.com in an iframe inside a container in a new tab. The extension removes the X-Frame-Options header and the 'frame-ancestors' directive from the CSP Header.

If you right-click the extension toolbar button, there is a 'Remove CSP Header' menu item, which removes the entire CSP Header instead of just removing the 'frame-ancestors' directive from the CSP Header. This always allows https://uk.ask.com to load in the iframe. This is NOT a feature of my Tile Pages WE extension.

I have not used mozregression for a few years. When I downloaded mozgression from GitHub and tried to install on my Windows 10 system, the installation was blocked by Norton Internet Security. This probably caused by not many people having installed mozregression, but I decided not to take the chance.

Instead I have done a manual regression through recent versions of Nightly:

• The last Nightly version that worked correctly, allowing https://uk.ask.com to load in the iframe:
  https://ftp.mozilla.org/pub/firefox/nightly/2020/04/2020-04-15-09-25-24-mozilla-central/
• The immediately following Nightly version does not allow https://uk.ask.com to load in the iframe:
  https://ftp.mozilla.org/pub/firefox/nightly/2020/04/2020-04-15-21-51-03-mozilla-central/

Please let me know if you need more information.

Comment 5

[dw-dev]

My manual regression shows that this problem was introduced into Nightly on 15th April 2020, but the reversion in https://bugzilla.mozilla.org/show_bug.cgi?id=1595652#c13 was not pushed until 28th April 2020, so I think the cause of this problem must be something else.

What is the simplest way to find all of the commits between the two Nightly versions on 15th April 2020?

Comment 6

[dw-dev]

I did some searching on Bugzilla for "empty CSP" and found this bug report:

Bug 1553017 - Intermittent TEST-UNEXPECTED-PASS | /content-security-policy/embedded-enforcement/subsumption_algorithm-general.html | Iframe with empty returned CSP should be blocked. - expected FAIL

The interesting bit is the phrase "Iframe with empty returned CSP should be blocked".

If this means that a page with an empty CSP should be blocked from loading in an <iframe>, then this explains the behaviour I have been seeing.

However, in practice, pages with empty CSPs were allowed to load from April 2019 (or earlier) until Nightly 77.0a1 (2020-04-15 09-25-24).

Comment 8

[Rob Wu [:robwu]]

Attachment: webRequest-csp-frame-ancestors.zip

STR:

1. Load the attached extension. It will open a tab.
2. Click the "Load as frames to start test" button.
3. Look at the four frames.

Expected:

• The first frame load should be blocked, the other three should be accepted.

Actual:

• All frame loads are blocked.

If I replace the URL with "https://example.com/" (which does not have any CSP), then it works as expected.
It also works if I replace the URL with https://amazon.com/favicon.ico (which has the X-Frame-Options) header.
This means that this is a bug related to the pre-existing CSP header.

I did a bisect and got the following range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9757d7ee7496dccc0b3e31d7803fc930ea10b894&tochange=ee0f0e5aea631efd6f13181fc39026a9928dd2e3

Comment 9

[Rob Wu [:robwu]]

I see what's going wrong. I'll fix it.

Comment 10

[Rob Wu [:robwu]]

Attachment: Bug 1635781 - Fix broken logic in test_ext_webRequest_mergecsp.js

• Expand comment on the meaning of the parameters in test_csp.

• sendMessage is not async, properly await the result.

• Unload extensions before checking assertions, to avoid unhelpful error
  messages about extensions not having been unloaded at the end of the
  test.

• Report which test case is being run to make debugging easier.

Comment 11

[Rob Wu [:robwu]]

Attachment: Bug 1635781 - Fully replace the web page's CSP if modified by add-on

The original addition of CSP to headersAlreadySet in bug 1462989 was
to make sure that CSP response headers from different extensions are
merged as expected. The logic did however not take into account that
unconditionally merging modified headers means that the header would be
merged with the original CSP from the web page, which prevented add-ons
from relaxing a CSP from the web page.

This commit fixes the bug by tracking the CSP status on the
ResponseHeaderChanger instance, which is shared by all webRequest
handlers of a single request.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/866413f80f63
Improve initialization and documentation of test_ext_webRequest_mergecsp.js r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/26d1f4a237a3
Fully replace the web page's CSP if modified by add-on r=mixedpuppy

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on /test_ext_webRequest_cached.js

backout: https://hg.mozilla.org/integration/autoland/rev/8755382e3dc5b324749bedada9343dcd239ac033

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=26d1f4a237a3393c4574aed599ab083ed1d023bc&searchStr=xpcshell

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307410350&repo=autoland&lineNumber=2522

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e91ac5ed3c2
Fix broken logic in test_ext_webRequest_mergecsp.js r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/88e46b72454f
Fully replace the web page's CSP if modified by add-on r=mixedpuppy

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/1e91ac5ed3c2
https://hg.mozilla.org/mozilla-central/rev/88e46b72454f

Comment 16

[Rob Wu [:robwu]]

[Tracking Requested - why for this release]:

This is a recent regression (fx 77) of the popular webRequest extension API, which can cause unexpected behavior (add-on functionality not working). The patch is low-risk (as it reverts the problematic code from bug 1635781 and re-introduces the intended functionality with a more limited scope) and I recommend uplift to ESR78. If that is considered too late (and we accept a regression in functionality on ESR78 compared to ESR68), then it would also be fine to include it in the next ESR78 dot release along with the fix in 79.

Comment 17

[Rob Wu [:robwu]]

Comment on attachment 9158725 [details]
Bug 1635781 - Fully replace the web page's CSP if modified by add-on

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a recent (Fx) regression that didn't happen on ESR68 that affects a popular extension API.
• User impact if declined: Add-ons that modify CSP headers may appear to work at first, but in some cases (on web pages with existing CSP headers), the add-on will unexpected fail to work as expected.
• Fix Landed on Version: 79
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch reverts the problematic code from bug 1635781 and re-introduces the intended functionality with a more limited scope. It is well covered by unit tests.
• String or UUID changes made by this patch:

Comment 18

[tobias]

It looks like this breaks the fix for Bug 1462989. Here are some STR that don't work on nightly 79.0a1 (2020-06-25) (64-bit) any more, but worked with 79.0a1 (2020-06-24) (64-bit):

1. install umatrix and ublock origin from amo
2. navigate to https://gir.st/tmp/webfont.html
3. in ublock, block remote fonts. in umatrix block javascript
4. reload with ctrl-f5 (to clear cache of the webfont) and notice that depending on addon installation order, only 1 of the 2 things are blocked

Comment 19

[tobias]

apologies, it looks like i spoke too soon. repeating the test, it works again.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9158724 [details]
Bug 1635781 - Fix broken logic in test_ext_webRequest_mergecsp.js

Approved for 78.1esr. Thanks for including tests.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/eb8a75f46db0
https://hg.mozilla.org/releases/mozilla-esr78/rev/cdbc208638c3