In webRequest, extensions may only modify requests when they have the host permissions to access to the request URL and to access to the document URL, if any. This is an allowlist-based approach.

In DNR, the declarativeNetRequest permission is meant to offer request blocking functionality without host permission checks. Anything more than just blocking (redirect / modifyHeaders) requires host permissions. DNR currently uses a deny-based approach: Consistent with webRequest, DNR already ignores system requests and requests to restricted domains. And with bug 1810753, requests from other extensions are blocked too. But e.g. about:-URLs are not explicitly rejected.

As a result, if I open a new tab (about:newtab) after installing a DNR extension with rules, an attempt to evaluate the DNR rules occurs. Because about:newtab is unexpected, an error is thrown when the unexpected URL is encountered in #domainFromURI for the initiatorURI (triggeringPrincipal):

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.host]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource://gre/modules/ExtensionDNR.sys.mjs :: #domainFromURI :: line 1160" data: no]

Instead of this accidental implementation detail, we should just filter out URIs whose schemes are certainly not known to be available to extensions. Note: opaque initiators are out of scope here (handled in bug 1798225).

• Let DNR ignore not-explicitly-allowed schemes (e.g. about:) without
  logging an error in the console.
• Improve testMatchOutcome to explicitly ignore unsupported schemes,
  to be consistent with what we do for evaluations from the network.

FYI: Chrome's declarativeNetRequest.testMatchOutcome method does not provide any access checks at all. I'd argue that to be a mistake, since the testMatchOutcome method is supposed to show whether a hypothetical request would match any rule.

This can be verified (e.g. in Chrome 111) by running the following from an unpacked extension that has the declarativeNetRequest permission. (to test in Firefox for comparison: also add the "declarativeNetRequestFeedback" permission + set the extensions.dnr.feedback pref to true).

await chrome.declarativeNetRequest.updateSessionRules({
  addRules: [{ id: 1, condition: {}, action: { type: "block" } }],
});
console.log(await chrome.declarativeNetRequest.testMatchOutcome({ type: "other", url: "chrome://extensions" });
console.log(await chrome.declarativeNetRequest.testMatchOutcome({ type: "other", url: "http://a", initiator: "chrome://extensions" });
console.log(await chrome.declarativeNetRequest.testMatchOutcome({ type: "other", url: "this-is:completely/bogus" });

Backed out for causing DNR related failures.

• Push with failures

• Failure log

• Backout link

The new xpcshell test failed on specific test configurations because it loads about:logo, but the xpcshell test utilities unconditionally load a data:text/javascript,// frame script, which triggers a crash in nsContentSecurityUtils::ValidateScriptFilename,

• Hit MOZ_CRASH(Blocking a script load data:text/javascript,// from file (None))

... or otherwise at least an error message in MaybeValidateFilename:

• "InternalError: unsafe filename: data:text/javascript,//"

While this observation is understandable given the circumstances, it is not helpful in the test. Therefore I am going to reland the test with security.allow_parent_unrestricted_js_loads=true for the specific test task.

https://hg.mozilla.org/mozilla-central/rev/739bfe1bfc6b

Comment on attachment 9327256 [details]
Bug 1826651 - Explicitly check other schemes in DNR

Beta/Release Uplift Approval Request

• User impact if declined: DNR evaluation engine tries to match more requests than intended; If the user installs an extension using DNR and opens a new tab (about:newtab), the console contains some logspam.
• 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: None
• 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

Documentation covered by https://github.com/mdn/content/pull/26189, specifically the part

Some requests are restricted and cannot be matched by extensions. That includes privileged browser requests, requests to or from [restricted domains](/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts#restricted_domains), and requests from other extensions.

Comment on attachment 9327256 [details]
Bug 1826651 - Explicitly check other schemes in DNR

Approved for 113 beta 4, thanks.

https://hg.mozilla.org/releases/mozilla-beta/rev/38a97f59f2e9