tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 3bfe7e545c3f759d985202102efcd51308ee6416
parent ee59ccae6549ed060a176932442c9e3735a8761c
Author: kpatenio <kpatenio@mozilla.com>
Date:   Tue, 16 Dec 2025 17:22:14 +0000

Bug 2004519 — update IPPExceptionsManager to handle exclusions only and IPPChannelFilter to detect those exclusions. r=ip-protection-reviewers,fchasen

This patch:
  -     removes site inclusions and protection mode pref support from IPPExceptionsManager.sys.mjs
  -     removes mentions of site inclusions and the ipProtection.exceptionsMode pref from documentation
  -     adds a new method to IPPExceptionsManager.sys.mjs to detect if an exclusion was saved given a principal
  -     updates test_IPPExceptionsManager.js to only test for site exclusions
  -     updates IPPChannelFilter to use IPPExceptionsManager for channel filtering

Differential Revision: https://phabricator.services.mozilla.com/D275675

Diffstat:
Mbrowser/app/profile/firefox.js | 3---
Mbrowser/components/ipprotection/IPPChannelFilter.sys.mjs | 11+++++++++++
Mbrowser/components/ipprotection/IPPExceptionsManager.sys.mjs | 93++++++++++++++++++++++---------------------------------------------------------
Mbrowser/components/ipprotection/docs/Components.rst | 4++--
Mbrowser/components/ipprotection/docs/Preferences.rst | 6------
Mbrowser/components/ipprotection/tests/browser/browser_IPPChannelFilter.js | 40++++++++++++++++++++++++++++++++++++++++
Mbrowser/components/ipprotection/tests/xpcshell/test_IPPExceptionsManager.js | 182+++++++++++--------------------------------------------------------------------
7 files changed, 104 insertions(+), 235 deletions(-)

diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js @@ -3542,9 +3542,6 @@ pref("browser.ipProtection.variant", ""); pref("browser.ipProtection.panelOpenCount", 0); // Pref to enable support for site exceptions pref("browser.ipProtection.features.siteExceptions", false); -pref("browser.ipProtection.exceptionsMode", "all"); -pref("browser.ipProtection.domainExclusions", ""); -pref("browser.ipProtection.domainInclusions", ""); pref("browser.ipProtection.log", false); pref("browser.ipProtection.guardian.endpoint", "https://vpn.mozilla.org/"); pref("browser.ipProtection.added", false); diff --git a/browser/components/ipprotection/IPPChannelFilter.sys.mjs b/browser/components/ipprotection/IPPChannelFilter.sys.mjs @@ -5,6 +5,8 @@ import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; const lazy = XPCOMUtils.declareLazy({ + IPPExceptionsManager: + "resource:///modules/ipprotection/IPPExceptionsManager.sys.mjs", ProxyService: { service: "@mozilla.org/network/protocol-proxy-service;1", iid: Ci.nsIProtocolProxyService, @@ -265,6 +267,15 @@ export class IPPChannelFilter { return true; } + let loadingPrincipal = channel.loadInfo?.loadingPrincipal; + let hasExclusion = + loadingPrincipal && + lazy.IPPExceptionsManager.hasExclusion(loadingPrincipal); + + if (hasExclusion) { + return true; + } + return this.#excludedOrigins.has(origin); } catch (_) { return true; diff --git a/browser/components/ipprotection/IPPExceptionsManager.sys.mjs b/browser/components/ipprotection/IPPExceptionsManager.sys.mjs @@ -2,22 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; - -const MODE_PREF = "browser.ipProtection.exceptionsMode"; - -const MODE = { - ALL: "all", - SELECT: "select", -}; - const PERM_NAME = "ipp-vpn"; /** - * Manages site inclusions and exclusions for IP Protection. + * Manages site exceptions for IP Protection. * It communicates with Services.perms to update the ipp-vpn permission type. - * Site exclusions are marked as permissions with DENY capabilities, whereas - * site inclusions are marked as permissions with ALLOW capabilities. + * Site exclusions are marked as permissions with DENY capabilities. * * While permissions related UI (eg. panels and dialogs) already handle changes to ipp-vpn, * the intention of this class is to abstract methods for updating ipp-vpn as needed @@ -25,27 +15,12 @@ const PERM_NAME = "ipp-vpn"; */ class ExceptionsManager { #inited = false; - #mode = MODE.ALL; - - /** - * The type of site exceptions for VPN. - * Valid types are "all" and "select". - * - * @returns {"all" | "select"} - * The site exception type. - * - * @see MODE - */ - get mode() { - return this.#mode; - } init() { if (this.#inited) { return; } - this.#mode = this.exceptionsMode; this.#inited = true; } @@ -57,28 +32,14 @@ class ExceptionsManager { this.#inited = false; } - onModeUpdate() { - this.#mode = this.exceptionsMode; - } - /** - * If mode is MODE.ALL, adds a new principal to ipp-vpn with DENY capability. - * If mode is MODE.SELECT, adds a new principal to ipp-vpn with ALLOW capability. + * Adds a principal to ipp-vpn with capability DENY_ACTION + * for site exclusions. * * @param {nsIPrincipal} principal * The principal that we want to add as a site exception. - * - * @see MODE */ - addException(principal) { - if (this.#mode === MODE.ALL) { - this.#addExclusionFromPrincipal(principal); - } else if (this.#mode === MODE.SELECT) { - this.#addInclusionFromPrincipal(principal); - } - } - - #addExclusionFromPrincipal(principal) { + addExclusion(principal) { Services.perms.addFromPrincipal( principal, PERM_NAME, @@ -86,28 +47,35 @@ class ExceptionsManager { ); } - #addInclusionFromPrincipal(principal) { - Services.perms.addFromPrincipal( - principal, - PERM_NAME, - Ci.nsIPermissionManager.ALLOW_ACTION - ); - } - /** * Removes an existing principal from ipp-vpn. * * @param {nsIPrincipal} principal * The principal that we want to remove as a site exception. - * - * @see MODE */ - removeException(principal) { + removeExclusion(principal) { Services.perms.removeFromPrincipal(principal, PERM_NAME); } /** - * Get the permission object for a site exception if it is in ipp-vpn. + * Returns true if the principal already exists in ipp-vpn + * and is registered as a permission with a DENY_ACTION + * capability (site exclusions). + * Else returns false if no such principal exists. + * + * @param {nsIPrincipal} principal + * The principal that we want to check is saved in ipp-vpn + * as a site exclusion. + * @returns {boolean} + * True if the principal exists as a site exclusion. + */ + hasExclusion(principal) { + let permission = this.getExceptionPermissionObject(principal); + return permission?.capability === Ci.nsIPermissionManager.DENY_ACTION; + } + + /** + * Get the permission object for a site exception if it exists in ipp-vpn. * * @param {nsIPrincipal} principal * The principal that we want to check is saved in ipp-vpn. @@ -116,23 +84,14 @@ class ExceptionsManager { * The permission object for a site exception, or null if unavailable. */ getExceptionPermissionObject(principal) { - let permission = Services.perms.getPermissionObject( + let permissionObject = Services.perms.getPermissionObject( principal, PERM_NAME, true /* exactHost */ ); - return permission; + return permissionObject; } } const IPPExceptionsManager = new ExceptionsManager(); - -XPCOMUtils.defineLazyPreferenceGetter( - IPPExceptionsManager, - "exceptionsMode", - MODE_PREF, - MODE.ALL, - () => IPPExceptionsManager.onModeUpdate() -); - export { IPPExceptionsManager }; diff --git a/browser/components/ipprotection/docs/Components.rst b/browser/components/ipprotection/docs/Components.rst @@ -77,8 +77,8 @@ IPProtectionPanel Controls the feature’s panel UI. IPPExceptionsManager - Manages the exceptions UI and logic (for example, domain exclusions and - exceptions mode) in coordination with the panel and preferences. + Manages the exceptions UI and logic (for example, domain exclusions) + in coordination with the panel and preferences. IPProtectionService The main service. It is initialized during browser startup, initializes helpers diff --git a/browser/components/ipprotection/docs/Preferences.rst b/browser/components/ipprotection/docs/Preferences.rst @@ -55,12 +55,6 @@ Networking and routing ``0`` routes all traffic (``MODE_FULL``), ``1`` only private browsing windows (``MODE_PB``), ``2`` only requests classified as tracking (``MODE_TRACKER``). -``browser.ipProtection.exceptionsMode`` (string, default: ``"all"``) - Defines which network requests are processed. Default: all. - -``browser.ipProtection.domainExclusions`` (string) - Comma‑separated list of domains to exclude from the proxy. - ``browser.ipProtection.override.serverlist`` (string) A JSON Payload that overrides the server list. Follows the Remote-Settings Schema. diff --git a/browser/components/ipprotection/tests/browser/browser_IPPChannelFilter.js b/browser/components/ipprotection/tests/browser/browser_IPPChannelFilter.js @@ -6,6 +6,9 @@ const { IPPChannelFilter } = ChromeUtils.importESModule( "resource:///modules/ipprotection/IPPChannelFilter.sys.mjs" ); +const { IPPExceptionsManager } = ChromeUtils.importESModule( + "resource:///modules/ipprotection/IPPExceptionsManager.sys.mjs" +); add_task(async function test_createConnection_and_proxy() { await withProxyServer(async proxyInfo => { @@ -90,6 +93,43 @@ add_task(async function test_essential_exclusion() { }); }); +add_task(async function test_exclusion_manager() { + const server = new HttpServer(); + server.registerPathHandler("/", (request, response) => { + response.setStatusLine(request.httpVersion, 200, "OK"); + response.setHeader("Content-Type", "text/plain"); + response.write("Hello World"); + }); + server.start(-1); + + await withProxyServer(async proxyInfo => { + // Create the IPP connection filter + const filter = IPPChannelFilter.create(); + // Add a site exclusion using IPPExceptionsManager + let principal = + Services.scriptSecurityManager.createContentPrincipalFromOrigin( + "http://localhost:" + server.identity.primaryPort + ); + IPPExceptionsManager.addExclusion(principal); + + filter.initialize("", proxyInfo.server); + proxyInfo.gotConnection.then(() => { + Assert.ok(false, "Proxy connection should not be made for excluded URL"); + }); + filter.start(); + + let tab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + // eslint-disable-next-line @microsoft/sdl/no-insecure-url + "http://localhost:" + server.identity.primaryPort + ); + await BrowserTestUtils.removeTab(tab); + filter.stop(); + + IPPExceptionsManager.removeExclusion(principal); + }); +}); + add_task(async function test_channel_suspend_resume() { const server = new HttpServer(); server.registerPathHandler("/", (request, response) => { diff --git a/browser/components/ipprotection/tests/xpcshell/test_IPPExceptionsManager.js b/browser/components/ipprotection/tests/xpcshell/test_IPPExceptionsManager.js @@ -6,18 +6,10 @@ https://creativecommons.org/publicdomain/zero/1.0/ */ const { IPPExceptionsManager } = ChromeUtils.importESModule( "resource:///modules/ipprotection/IPPExceptionsManager.sys.mjs" ); -const { TestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TestUtils.sys.mjs" -); -const MODE_PREF = "browser.ipProtection.exceptionsMode"; -const ALL_MODE = "all"; -const SELECT_MODE = "select"; const ONBOARDING_MESSAGE_MASK_PREF = "browser.ipProtection.onboardingMessageMask"; -const PERM_NAME = "ipp-vpn"; - /** * Tests the manager can modify exclusions in ipp-vpn permission. */ @@ -25,192 +17,68 @@ add_task(async function test_IPPExceptionsManager_exclusions() { const site1 = "https://www.example.com"; const site2 = "https://www.another.example.com"; - Services.prefs.setStringPref(MODE_PREF, ALL_MODE); - IPPExceptionsManager.init(); // Make mock principals and add two exclusions let contentPrincipal1 = Services.scriptSecurityManager.createContentPrincipalFromOrigin(site1); - let contentPrincpal2 = + let contentPrincipal2 = Services.scriptSecurityManager.createContentPrincipalFromOrigin(site2); // Add two exclusions - IPPExceptionsManager.addException(contentPrincipal1); - IPPExceptionsManager.addException(contentPrincpal2); + IPPExceptionsManager.addExclusion(contentPrincipal1); + IPPExceptionsManager.addExclusion(contentPrincipal2); + + // Verify that hasExclusion can detect the newly added sites + let site1Exists = IPPExceptionsManager.hasExclusion(contentPrincipal1); + let site2Exists = IPPExceptionsManager.hasExclusion(contentPrincipal2); + + Assert.ok(site1Exists, `hasExclusion correctly states that ${site1} exists`); + Assert.ok(site2Exists, `hasExclusion correctly states that ${site2} exists`); // Verify the permission data let permissionObj1 = IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal1); let permissionObj2 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincpal2); + IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal2); Assert.equal( permissionObj1?.capability, Ci.nsIPermissionManager.DENY_ACTION, - `Permission object for ${site1} exists and has capability DENY` + `getExceptionPermissionObject correctly states that ${site1} exists and has capability DENY` ); Assert.equal( permissionObj2?.capability, Ci.nsIPermissionManager.DENY_ACTION, - `Permission object for ${site2} exists and has capability DENY` + `getExceptionPermissionObject correctly states that ${site2} exists and has capability DENY` ); // Now remove the exceptions - IPPExceptionsManager.removeException(contentPrincipal1); - IPPExceptionsManager.removeException(contentPrincpal2); - - // Verify the permission data no longer exists - permissionObj1 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal1); - permissionObj2 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincpal2); + IPPExceptionsManager.removeExclusion(contentPrincipal1); + IPPExceptionsManager.removeExclusion(contentPrincipal2); - Assert.ok(!permissionObj1, `Permission object for ${site1} no longer exists`); - Assert.ok(!permissionObj2, `Permission object for ${site2} no longer exists`); - - Services.prefs.clearUserPref(MODE_PREF); - Services.prefs.clearUserPref(ONBOARDING_MESSAGE_MASK_PREF); - IPPExceptionsManager.uninit(); -}); + // Verify that hasExclusion no longer detects the recently removed sites + site1Exists = IPPExceptionsManager.hasExclusion(contentPrincipal1); + site2Exists = IPPExceptionsManager.hasExclusion(contentPrincipal2); -/** - * Tests the manager can modify inclusions in ipp-vpn permission. - */ -add_task(async function test_IPPExceptionsManager_inclusions() { - const site1 = "https://www.example.com"; - const site2 = "https://www.another.example.com"; - - Services.prefs.setStringPref(MODE_PREF, SELECT_MODE); - - IPPExceptionsManager.init(); - - // Make mock principals and add two inclusions - let contentPrincipal1 = - Services.scriptSecurityManager.createContentPrincipalFromOrigin(site1); - let contentPrincpal2 = - Services.scriptSecurityManager.createContentPrincipalFromOrigin(site2); - - // Add two inclusions - IPPExceptionsManager.addException(contentPrincipal1); - IPPExceptionsManager.addException(contentPrincpal2); - - // Verify the permission data - let permissionObj1 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal1); - let permissionObj2 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincpal2); - - Assert.equal( - permissionObj1?.capability, - Ci.nsIPermissionManager.ALLOW_ACTION, - `Permission object for ${site1} exists and has capability ALLOW` + Assert.ok( + !site1Exists, + `hasExclusion correctly states that ${site1} no longer exists` ); - Assert.equal( - permissionObj2?.capability, - Ci.nsIPermissionManager.ALLOW_ACTION, - `Permission object for ${site2} exists and has capability ALLOW` + Assert.ok( + !site2Exists, + `hasExclusion correctly states that ${site2} no longer exists` ); - // Now remove the exceptions - IPPExceptionsManager.removeException(contentPrincipal1); - IPPExceptionsManager.removeException(contentPrincpal2); - // Verify the permission data no longer exists permissionObj1 = IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal1); permissionObj2 = - IPPExceptionsManager.getExceptionPermissionObject(contentPrincpal2); + IPPExceptionsManager.getExceptionPermissionObject(contentPrincipal2); Assert.ok(!permissionObj1, `Permission object for ${site1} no longer exists`); Assert.ok(!permissionObj2, `Permission object for ${site2} no longer exists`); - Services.prefs.clearUserPref(MODE_PREF); - Services.prefs.clearUserPref(ONBOARDING_MESSAGE_MASK_PREF); - IPPExceptionsManager.uninit(); -}); - -/** - * Tests the manager correctly tracks exclusions and inclusions after - * changing mode. - */ -add_task(async function test_IPPExceptionsManager_switch_mode() { - const site1 = "https://www.example.com"; - const site2 = "https://www.another.example.com"; - - // Start with "all" mode first - Services.prefs.setStringPref(MODE_PREF, ALL_MODE); - - IPPExceptionsManager.init(); - - // Add an exclusion - let contentPrincipal1 = - Services.scriptSecurityManager.createContentPrincipalFromOrigin(site1); - IPPExceptionsManager.addException(contentPrincipal1); - - // Switch mode to "select" - let prefChanged = TestUtils.waitForPrefChange(MODE_PREF); - Services.prefs.setStringPref(MODE_PREF, SELECT_MODE); - await prefChanged; - - // Now add an inclusion - let contentPrincipal2 = - Services.scriptSecurityManager.createContentPrincipalFromOrigin(site2); - IPPExceptionsManager.addException(contentPrincipal2); - - // Ensure exception types were preserved and that we have the right number of exceptions - let savedSites = Services.perms.getAllByTypes([PERM_NAME]); - Assert.equal( - savedSites.length, - 2, - `There should be only 2 site exceptions in ${PERM_NAME}` - ); - - let exclusions = Services.perms - .getAllByTypes([PERM_NAME]) - .filter(perm => perm.capability == Ci.nsIPermissionManager.DENY_ACTION); - let inclusions = Services.perms - .getAllByTypes([PERM_NAME]) - .filter(perm => perm.capability == Ci.nsIPermissionManager.ALLOW_ACTION); - Assert.equal( - exclusions.length, - 1, - `There should be 1 exclusion in ${PERM_NAME}` - ); - Assert.equal( - inclusions.length, - 1, - `There should be 1 inclusion in ${PERM_NAME}` - ); - - // Now let's try adding the same principal for the excluded site as an INCLUSION instead - IPPExceptionsManager.addException(contentPrincipal1); - - savedSites = Services.perms.getAllByTypes([PERM_NAME]); - Assert.equal( - savedSites.length, - 2, - `There should still be 2 site exceptions in ${PERM_NAME}` - ); - - exclusions = Services.perms - .getAllByTypes([PERM_NAME]) - .filter(perm => perm.capability == Ci.nsIPermissionManager.DENY_ACTION); - inclusions = Services.perms - .getAllByTypes([PERM_NAME]) - .filter(perm => perm.capability == Ci.nsIPermissionManager.ALLOW_ACTION); - Assert.equal( - exclusions.length, - 0, - `There should be 0 exclusions now in ${PERM_NAME}` - ); - Assert.equal( - inclusions.length, - 2, - `There should be 2 inclusions now in ${PERM_NAME}` - ); - - Services.prefs.clearUserPref(MODE_PREF); Services.prefs.clearUserPref(ONBOARDING_MESSAGE_MASK_PREF); IPPExceptionsManager.uninit(); });