1825824 - [DNR] initiator-based access checks is stricter than webRequest for extension:-initiated requests

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P2)

Description

[Rob Wu [:robwu]]

DNR is specified to perform access checks based on the "request initiator" (and "request URL"). The "request initiator" (chrome docs) is currently derived from the "triggering principal" (aka "originUrl" in the webRequest API). But the webRequest API performs access checks on the "loading principal" (aka "documentUrl" in the webRequest API) instead. Because of this, DNR may unexpectedly not match for requests if other extensions are involved, especially in comparison with webRequest (see below for details). This is independent of the restriction from bug 1810753.

The relevant "RequestEvaluator.canModify" state in DNR affects the following matching behavior in DNR:

• with the "declarativeNetRequestWithHostAccess" permission: determining whether DNR rules may be applied to the given request.
• with the "declarativeNetRequest" permission: determining whether to allow request actions that modify the request beyond just blocking (i.e. "modifyHeaders" and "redirect").
• (for completeness: initiatorDomains and excludedInitiatorDomains conditions of DNR rules are specified to match the "request initiator")

The issue described above does not prevent DNR from matching requests initiated by itself, because an extension is always permitted to do so. Different behavior occurs when triggeringPrincipal is set to an extension (distinct from the DNR extension), while loadingPrincipal may be something else (e.g. regular https:-URLs). Besides the cases mentioned in nsILoadInfo.idl, there are also extension APIs that trigger navigations:

• tabs.create (source)
• tabs.update (source)
• windows.create (source)
• search.search and search.query (source)

Note: The downloads API assigns the extension principal to the loadingPrincipal instead of triggeringPrincipal (source), because of the desire to explicitly allow extensions to modify the request through the webRequest API (bug 1579911).

Note: When the triggering principal is the system principal, the request initiator is omitted (and allowed to match DNR rules). This is desirable, because cases such as user-triggered navigations should still be able to match DNR rules.
(system requests are ignored in DNR because that check relies on the loading principal as desired, through channel.canModify, see ChannelWrapper::CanModify.)

Comment 1

[Rob Wu [:robwu]]

A way to address this without breaking the "initiatorDomains" / "excludedInitiatorDomains" use case is to adjust the RequestEvaluator.canModify check to special-case the behavior when triggeringPrincipal is a moz-extension:-principal (in that case, use loadingPrincipal instead, maybe only for navigations).

Another remark that I forgot to add in the initial report: webRequest in Firefox allows extensions to observe requests (not just navigation, but any) from opaque origins from other extensions (e.g. <iframe sandbox>). Chrome does not.

Comment 2

[Rob Wu [:robwu]]

This bug is broader than just moz-extension:-initiated requests. Extensions with limited host permissions are also affected by this, and it appears that Chrome has intentional exceptions for navigations that have not been documented.

I checked the behavior in Chrome, and found something strange. Their documentation seems quite clear:

The declarativeNetRequestWithHostAccess permission always requires host permissions to the request URL and initiator to act on a request.

However, when I create a test case with the following:

• "declarativeNetRequestWithHostAccess" permission + "://example.com/" host_permissions.
• Add DNR rule that blocks any request for resourceTypes: ["main_frame", "xmlhttprequest"].
• ... then fetch() in example.com is blocked as expected.
• ... then fetch() in example.net is allowed as expected (missing host permission).
• open("https://example.com") in example.com is blocked as expected.
• open("https://example.com") in example.net is NOT blocked despite the lack of permission for the request initiator.

Apparently, while this behavior was not documented, it is intentional and done in https://bugs.chromium.org/p/chromium/issues/detail?id=918137#c20 - "To intercept a navigation request (sub-frame or main-frame request), extensions don't need host permissions to the request initiator."

Excluding sub_frame and main_frame requests from the permission checks for the access checks of the initiator (triggering principal) would resolve this bug.

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1825824 - Allow DNR to match navigation requests without permission for initiator

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/013b29254435
Allow DNR to match navigation requests without permission for initiator r=rpl

Comment 5

[Iulian Moraru]

Backed out for causing DNR related failures.

• Push with failures

• Failure log

• Backout link

Comment 6

[Rob Wu [:robwu]]

The test failure is due to bug 1827526. This is a test-only failure unique to M-http3. As a work-around I updated the test to use example.org instead of example.net (see bug 1827526 for why that works).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7728aaf07061
Allow DNR to match navigation requests without permission for initiator r=rpl

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7728aaf07061

Comment 9

[Rob Wu [:robwu]]

Comment on attachment 9327379 [details]
Bug 1825824 - Allow DNR to match navigation requests without permission for initiator

Beta/Release Uplift Approval Request

• User impact if declined: DNR extensions fail to modify (e.g. block) network requests from some navigations, which is inconsistent with the webRequest API and also inconsistent with Chrome. The DNR API was introduced in Firefox 113 (currently beta), so it would be nice to not have this bug when the API rides to release.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1826651
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Change to specific part of extension API, fully covered by unit tests. The API was enabled in Firefox 113 (currently Beta).
• String changes made/needed: none
• Is Android affected?: Yes

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9327379 [details]
Bug 1825824 - Allow DNR to match navigation requests without permission for initiator

Approved for 113 beta 3, thanks.

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/e175a771d3f3

Comment 12

[Rob Wu [:robwu]]

Documented in https://github.com/mdn/content/pull/26189