Commit 3e81a727 authored by Alexandre Poirot's avatar Alexandre Poirot
Browse files

Bug 1775503 - [devtools] Prevent showing content process request when the... 

Bug 1775503 - [devtools] Prevent showing content process request when the browser toolbox focuses on the parent process. r=bomsy

This is still one blind spot with privileged request done by content processes.
They aren't related to any BrowsingContext/WindowGlobal and there isn't any
attribute on channel/loadInfo which helps knowing they originates from a content process.

Differential Revision: https://phabricator.services.mozilla.com/D150014
parent e23e2a02

devtools/client/framework/browser-toolbox/test/browser_browser_toolbox_netmonitor.js

+13 −27
Original line number Diff line number Diff line
@@ -7,38 +7,24 @@ 


add_task(async function() {

  // Disable several prefs to avoid network requests.

  Services.prefs.setBoolPref("browser.safebrowsing.blockedURIs.enabled", false);

  Services.prefs.setBoolPref("browser.safebrowsing.downloads.enabled", false);

  Services.prefs.setBoolPref("browser.safebrowsing.malware.enabled", false);

  Services.prefs.setBoolPref("browser.safebrowsing.passwords.enabled", false);

  Services.prefs.setBoolPref("browser.safebrowsing.phishing.enabled", false);

  Services.prefs.setBoolPref("privacy.query_stripping.enabled", false);

  Services.prefs.setBoolPref("extensions.systemAddon.update.enabled", false);

  await pushPref("browser.safebrowsing.blockedURIs.enabled", false);

  await pushPref("browser.safebrowsing.downloads.enabled", false);

  await pushPref("browser.safebrowsing.malware.enabled", false);

  await pushPref("browser.safebrowsing.passwords.enabled", false);

  await pushPref("browser.safebrowsing.phishing.enabled", false);

  await pushPref("privacy.query_stripping.enabled", false);

  await pushPref("extensions.systemAddon.update.enabled", false);



  const servicesSettingsServer = Services.prefs.getCharPref(

    "services.settings.server"

  );

  Services.prefs.setCharPref("services.settings.server", "invalid://err");

  await pushPref("services.settings.server", "invalid://err");



  // Define a set list of visible columns

  Services.prefs.setCharPref(

  await pushPref(

    "devtools.netmonitor.visibleColumns",

    JSON.stringify(["file", "url", "status"])

  );

  registerCleanupFunction(() => {

    Services.prefs.clearUserPref("devtools.netmonitor.visibleColumns");

    Services.prefs.clearUserPref("browser.safebrowsing.blockedURIs.enabled");

    Services.prefs.clearUserPref("browser.safebrowsing.downloads.enabled");

    Services.prefs.clearUserPref("browser.safebrowsing.malware.enabled");

    Services.prefs.clearUserPref("browser.safebrowsing.passwords.enabled");

    Services.prefs.clearUserPref("browser.safebrowsing.phishing.enabled");

    Services.prefs.clearUserPref("privacy.query_stripping.enabled");

    Services.prefs.clearUserPref("extensions.systemAddon.update.enabled");

    Services.prefs.setCharPref(

      "services.settings.server",

      servicesSettingsServer

    );

  });



  // Force observice all processes to see the content process requests

  await pushPref("devtools.browsertoolbox.scope", "everything");



  const ToolboxTask = await initBrowserToolboxTask({

    enableBrowserToolboxFission: true,

devtools/server/actors/network-monitor/network-observer.js

+35 −12
Original line number Diff line number Diff line
@@ -97,6 +97,9 @@ const HTTP_TEMPORARY_REDIRECT = 307; 
 *          given the initial network request information as an argument.

 *          onNetworkEvent() must return an object which holds several add*()

 *          methods which are used to add further network request/response information.

 *        - shouldIgnoreChannel(channel) [optional]

 *          In additional to `NetworkUtils.matchRequest`, allow to ignore even more

 *          requests.

 */

function NetworkObserver(filters, owner) {

  this.filters = filters;
@@ -236,12 +239,31 @@ NetworkObserver.prototype = { 
    return this._throttler;

  },



  /**

   * Given a network channel, should this observer ignore this request

   * based on the filters passed as constructor arguments.

   *

   * @param {nsIHttpChannel/nsIWebSocketChannel} channel

   *        The request that should be inspected or ignored.

   * @return {boolean}

   *         Return true, if the request should be ignored.

   */

  _shouldIgnoreChannel(channel) {

    if (

      typeof this.owner.shouldIgnoreChannel == "function" &&

      this.owner.shouldIgnoreChannel(channel)

    ) {

      return true;

    }

    return !NetworkUtils.matchRequest(channel, this.filters);

  },



  _decodedCertificateCache: null,



  _serviceWorkerRequest(subject, topic, data) {

    const channel = subject.QueryInterface(Ci.nsIHttpChannel);



    if (!NetworkUtils.matchRequest(channel, this.filters)) {

    if (this._shouldIgnoreChannel(channel)) {

      return;

    }
@@ -268,7 +290,7 @@ NetworkObserver.prototype = { 
    }



    const channel = subject.QueryInterface(Ci.nsIHttpChannel);

    if (!NetworkUtils.matchRequest(channel, this.filters)) {

    if (this._shouldIgnoreChannel(channel)) {

      return;

    }
@@ -295,7 +317,7 @@ NetworkObserver.prototype = { 
    }



    const channel = subject.QueryInterface(Ci.nsIHttpChannel);

    if (!NetworkUtils.matchRequest(channel, this.filters)) {

    if (this._shouldIgnoreChannel(channel)) {

      return;

    }
@@ -380,7 +402,7 @@ NetworkObserver.prototype = { 
    subject.QueryInterface(Ci.nsIClassifiedChannel);

    const channel = subject.QueryInterface(Ci.nsIHttpChannel);



    if (!NetworkUtils.matchRequest(channel, this.filters)) {

    if (this._shouldIgnoreChannel(channel)) {

      return;

    }
@@ -509,7 +531,9 @@ NetworkObserver.prototype = { 
    const throttler = this._getThrottler();

    if (throttler) {

      const channel = subject.QueryInterface(Ci.nsIHttpChannel);

      if (NetworkUtils.matchRequest(channel, this.filters)) {

      if (this._shouldIgnoreChannel(channel)) {

        return;

      }

      logPlatformEvent("http-on-modify-request", channel);



      // Read any request body here, before it is throttled.
@@ -517,7 +541,6 @@ NetworkObserver.prototype = { 
      this._onRequestBodySent(httpActivity);

      throttler.manageUpload(channel);

    }

    }

  },



  /**
@@ -749,7 +772,7 @@ NetworkObserver.prototype = { 
   * @return void

   */

  _onRequestHeader(channel, timestamp, extraStringData) {

    if (!NetworkUtils.matchRequest(channel, this.filters)) {

    if (this._shouldIgnoreChannel(channel)) {

      return;

    }

devtools/server/actors/resources/network-events.js

+50 −1
Original line number Diff line number Diff line
@@ -9,6 +9,10 @@ const { Pool } = require("devtools/shared/protocol/Pool"); 
const {

  isWindowGlobalPartOfContext,

} = require("devtools/server/actors/watcher/browsing-context-helpers.jsm");

const {

  WatcherRegistry,

} = require("devtools/server/actors/watcher/WatcherRegistry.jsm");

const Targets = require("devtools/server/actors/targets/index");



loader.lazyRequireGetter(

  this,
@@ -17,6 +21,12 @@ loader.lazyRequireGetter( 
  true

);



loader.lazyRequireGetter(

  this,

  "NetworkUtils",

  "devtools/server/actors/network-monitor/utils/network-utils"

);



loader.lazyRequireGetter(

  this,

  "NetworkEventActor",
@@ -53,7 +63,10 @@ class NetworkEventWatcher { 
    this.persist = false;

    this.listener = new NetworkObserver(

      { sessionContext: watcherActor.sessionContext },

      { onNetworkEvent: this.onNetworkEvent.bind(this) }

      {

        onNetworkEvent: this.onNetworkEvent.bind(this),

        shouldIgnoreChannel: this.shouldIgnoreChannel.bind(this),

      }

    );



    this.listener.init();
@@ -206,6 +219,41 @@ class NetworkEventWatcher { 
    }

  }



  /**

   * Called by NetworkObserver in order to know if the channel should be ignored

   */

  shouldIgnoreChannel(channel) {

    // When we are in the browser toolbox in parent process scope,

    // the session context is still "all", but we are no longer watching frame and process targets.

    // In this case, we should ignore all requests belonging to a BrowsingContext that isn't in the parent process

    // (i.e. the process where this Watcher runs)

    const isParentProcessOnlyBrowserToolbox =

      this.watcherActor.sessionContext.type == "all" &&

      !WatcherRegistry.isWatchingTargets(

        this.watcherActor,

        Targets.TYPES.FRAME

      );

    if (isParentProcessOnlyBrowserToolbox) {

      // We should ignore all requests coming from BrowsingContext running in another process

      const browsingContextID = NetworkUtils.getChannelBrowsingContextID(

        channel

      );

      const browsingContext = BrowsingContext.get(browsingContextID);

      // We accept any request that isn't bound to any BrowsingContext.

      // This is most likely a privileged request done from a JSM/C++.

      // `isInProcess` will be true, when the document executes in the parent process.

      //

      // Note that we will still accept all requests that aren't bound to any BrowsingContext

      // See browser_resources_network_events_parent_process.js test with privileged request

      // made from the content processes.

      // We miss some attribute on channel/loadInfo to know that it comes from the content process.

      if (browsingContext?.currentWindowGlobal.isInProcess === false) {

        return true;

      }

    }

    return false;

  }



  onNetworkEvent(event) {

    const { channelId } = event;
@@ -214,6 +262,7 @@ class NetworkEventWatcher { 
        `Got notified about channel ${channelId} more than once.`

      );

    }



    const actor = new NetworkEventActor(

      this.watcherActor.conn,

      this.watcherActor.sessionContext,

devtools/shared/commands/resource/tests/browser_resources_network_events_parent_process.js

+89 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ add_task(async function testParentProcessRequests() { 
  // The test expects the main process commands instance to receive resources

  // for content process requests.

  await pushPref("devtools.browsertoolbox.fission", true);

  await pushPref("devtools.browsertoolbox.scope", "everything");



  const commands = await CommandsFactory.forMainProcess();

  await commands.targetCommand.startListening();
@@ -147,6 +148,94 @@ add_task(async function testParentProcessRequests() { 
    "The second image request is privileged"

  );



  info(

    "Open a content page to ensure we also receive request from content processes"

  );

  const pageUrl = "https://example.org/document-builder.sjs?html=foo";

  const requestUrl = "https://example.org/document-builder.sjs?html=bar";

  const tab = await addTab(pageUrl);



  await waitFor(() => receivedNetworkEvents.length == 4);

  const tabRequest = receivedNetworkEvents[3];

  is(tabRequest.url, pageUrl, "The 4th resource is for the tab request");

  ok(!tabRequest.chromeContext, "The 4th request is content");



  info(

    "Also spawn a privileged request from the content process, not bound to any WindowGlobal"

  );

  await SpecialPowers.spawn(tab.linkedBrowser, [requestUrl], async function(

    uri

  ) {

    const { NetUtil } = ChromeUtils.import(

      "resource://gre/modules/NetUtil.jsm"

    );

    const channel = NetUtil.newChannel({

      uri,

      loadUsingSystemPrincipal: true,

    });

    channel.open();

  });

  await removeTab(tab);



  await waitFor(() => receivedNetworkEvents.length == 5);

  const privilegedContentRequest = receivedNetworkEvents[4];

  is(

    privilegedContentRequest.url,

    requestUrl,

    "The 5th resource is for the privileged content process request"

  );

  ok(privilegedContentRequest.chromeContext, "The 5th request is privileged");



  info("Now focus only on parent process resources");

  await pushPref("devtools.browsertoolbox.scope", "parent-process");



  info(

    "Retrigger the two last requests. The tab document request and a privileged request. Both happening in the tab's content process."

  );

  const secondTab = await addTab(pageUrl);

  await SpecialPowers.spawn(

    secondTab.linkedBrowser,

    [requestUrl],

    async function(uri) {

      const { NetUtil } = ChromeUtils.import(

        "resource://gre/modules/NetUtil.jsm"

      );

      const channel = NetUtil.newChannel({

        uri,

        loadUsingSystemPrincipal: true,

      });

      channel.open();

    }

  );



  await waitFor(() => receivedNetworkEvents.length == 6);



  // nsIHttpChannel doesn't expose any attribute allowing to identify

  // privileged requests done in content processes.

  // Thus, preventing us from filtering them out correctly.

  // Ideally, we would need some new attribute to know from which (content) process

  // any channel originates from.

  info(

    "For now, we are still notified about the privileged content process request"

  );

  const secondPrivilegedContentRequest = receivedNetworkEvents[5];

  is(

    secondPrivilegedContentRequest.url,

    requestUrl,

    "The 6th resource is for the second privileged content process request"

  );

  ok(privilegedContentRequest.chromeContext, "The 6th request is privileged");



  // Let some time to receive the tab request if that's not correctly filtered out

  await wait(1000);

  is(

    receivedNetworkEvents.length,

    6,

    "But we don't receive the request for the tab request"

  );



  await removeTab(secondTab);



  await resourceCommand.unwatchResources(

    [resourceCommand.TYPES.NETWORK_EVENT],

    {