Bug 1452713 - Update webRTCIPHandlingPolicy to match Chrome r=bwc,mixedpuppy

Previously, the network.webRTCIPHandlingPolicy "disable_non_proxied_udp" only enabled the use of WebRTC if a proxy was configured and the WebRTC service supported TURN TCP. This aims to match Chrome's behavior by forcing the use of a proxy if one is configured, otherwise falling back to mode 3 (no host candidates and default route only). Also, remove some dead code left over from the old way of routing TURN communications through an HTTP proxy. Differential Revision: https://phabricator.services.mozilla.com/D37892 --HG-- extra : moz-landing-system : lando
2024-10-18 07:45:30 +00:00 · 2019-08-02 21:33:46 +00:00 · 2019-08-02 21:33:46 +00:00 · 41872c8747
commit 41872c8747
parent e60778b38f
10 changed files with 54 additions and 31 deletions
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@ -274,7 +274,8 @@ NrIceCtx::NrIceCtx(const std::string& name, Policy policy)
      trickle_(true),
      policy_(policy),
      nat_(nullptr),
-      proxy_config_(nullptr) {}
+      proxy_config_(nullptr),
+      proxy_only_(false) {}

 /* static */
 RefPtr<NrIceCtx> NrIceCtx::Create(const std::string& name, bool allow_loopback,
@ -862,6 +863,8 @@ nsresult NrIceCtx::StartGathering(bool default_route_only, bool proxy_only) {
  SetGatheringState(ICE_CTX_GATHER_STARTED);

  SetCtxFlags(default_route_only, proxy_only);
+    
+  proxy_only_ = proxy_only;

  // This might start gathering for the first time, or again after
  // renegotiation, or might do nothing at all if gathering has already
@ -1052,6 +1055,11 @@ int nr_socket_local_create(void* obj, nr_transport_addr* addr,

  if (obj) {
    config = static_cast<NrIceCtx*>(obj)->GetProxyConfig();
+    bool ctx_proxy_only = static_cast<NrIceCtx*>(obj)->proxy_only();
+
+    if (ctx_proxy_only && !config) {
+      ABORT(R_FAILED);
+    }
  }

  r = NrSocketBase::CreateSocket(addr, &sock, config);
--- a/media/mtransport/nricectx.h
+++ b/media/mtransport/nricectx.h
@ -308,6 +308,8 @@ class NrIceCtx {

  void SetCtxFlags(bool default_route_only, bool proxy_only);

+  bool proxy_only() const { return proxy_only_; }
+
  // Start ICE gathering
  nsresult StartGathering(bool default_route_only, bool proxy_only);

@ -388,6 +390,7 @@ class NrIceCtx {
  Policy policy_;
  RefPtr<TestNat> nat_;
  std::shared_ptr<NrSocketProxyConfig> proxy_config_;
+  bool proxy_only_;
 };

 }  // namespace mozilla
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@ -588,12 +588,6 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon

        r_log(LOG_ICE,LOG_DEBUG,"nr_ice_component_initialize_tcp creating TURN TCP wrappers");

-        if (ctx->turn_tcp_socket_wrapper) {
-          /* The HTTP proxy socket */
-          if((r=nr_socket_wrapper_factory_wrap(ctx->turn_tcp_socket_wrapper, local_sock, &local_sock)))
-            ABORT(r);
-        }
-
        /* The TCP buffered socket */
        if((r=nr_socket_buffered_stun_create(local_sock, NR_STUN_MAX_MESSAGE_SIZE, TURN_TCP_FRAMING, &buffered_sock)))
          ABORT(r);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
@ -246,21 +246,6 @@ int nr_ice_ctx_set_interface_prioritizer(nr_ice_ctx *ctx, nr_interface_prioritiz
    return(_status);
  }

-int nr_ice_ctx_set_turn_tcp_socket_wrapper(nr_ice_ctx *ctx, nr_socket_wrapper_factory *wrapper)
-  {
-    int _status;
-
-    if (ctx->turn_tcp_socket_wrapper) {
-      ABORT(R_ALREADY);
-    }
-
-    ctx->turn_tcp_socket_wrapper = wrapper;
-
-    _status=0;
-   abort:
-    return(_status);
-  }
-
 void nr_ice_ctx_set_socket_factory(nr_ice_ctx *ctx, nr_socket_factory *factory)
  {
    nr_socket_factory_destroy(&ctx->socket_factory);
@ -479,7 +464,6 @@ static void nr_ice_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg)

    nr_resolver_destroy(&ctx->resolver);
    nr_interface_prioritizer_destroy(&ctx->interface_prioritizer);
-    nr_socket_wrapper_factory_destroy(&ctx->turn_tcp_socket_wrapper);
    nr_socket_factory_destroy(&ctx->socket_factory);

    RFREE(ctx);
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
@ -134,7 +134,6 @@ struct nr_ice_ctx_ {

  nr_resolver *resolver;                      /* The resolver to use */
  nr_interface_prioritizer *interface_prioritizer;  /* Priority decision logic */
-  nr_socket_wrapper_factory *turn_tcp_socket_wrapper; /* The TURN TCP socket wrapper to use */
  nr_socket_factory *socket_factory;

  nr_ice_foundation_head foundations;
@ -193,7 +192,6 @@ int nr_ice_ctx_set_turn_servers(nr_ice_ctx *ctx,nr_ice_turn_server *servers, int
 int nr_ice_ctx_copy_turn_servers(nr_ice_ctx *ctx, nr_ice_turn_server *servers, int ct);
 int nr_ice_ctx_set_resolver(nr_ice_ctx *ctx, nr_resolver *resolver);
 int nr_ice_ctx_set_interface_prioritizer(nr_ice_ctx *ctx, nr_interface_prioritizer *prioritizer);
-int nr_ice_ctx_set_turn_tcp_socket_wrapper(nr_ice_ctx *ctx, nr_socket_wrapper_factory *wrapper);
 void nr_ice_ctx_set_socket_factory(nr_ice_ctx *ctx, nr_socket_factory *factory);
 int nr_ice_ctx_set_trickle_cb(nr_ice_ctx *ctx, nr_ice_trickle_candidate_cb cb, void *cb_arg);
 int nr_ice_ctx_hide_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand);
--- a/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
@ -162,6 +162,7 @@ class MediaTransportHandlerSTS : public MediaTransportHandler,
  RefPtr<NrIceCtx> mIceCtx;
  RefPtr<NrIceResolver> mDNSResolver;
  std::map<std::string, Transport> mTransports;
+  bool mProxyOnlyIfBehindProxy = false;
  bool mProxyOnly = false;

  // mDNS Support
@ -425,6 +426,8 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx(
                                              __func__);
        }

+        mProxyOnlyIfBehindProxy = Preferences::GetBool(
+            "media.peerconnection.ice.proxy_only_if_behind_proxy", false);
        mProxyOnly =
            Preferences::GetBool("media.peerconnection.ice.proxy_only", false);

@ -658,6 +661,10 @@ void MediaTransportHandlerSTS::StartIceGathering(
  mInitPromise->Then(
      mStsThread, __func__,
      [=, self = RefPtr<MediaTransportHandlerSTS>(this)]() {
+        if (mIceCtx->GetProxyConfig() && mProxyOnlyIfBehindProxy) {
+          mProxyOnly = true;
+        }
+
        // Belt and suspenders - in e10s mode, the call below to SetStunAddrs
        // needs to have the proper flags set on ice ctx.  For non-e10s,
        // setting those flags happens in StartGathering.  We could probably
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@ -377,6 +377,7 @@ pref("media.peerconnection.ice.stun_client_maximum_transmits", 7);
 pref("media.peerconnection.ice.trickle_grace_period", 5000);
 pref("media.peerconnection.ice.no_host", false);
 pref("media.peerconnection.ice.default_address_only", false);
+pref("media.peerconnection.ice.proxy_only_if_behind_proxy", false);
 pref("media.peerconnection.ice.proxy_only", false);
 pref("media.peerconnection.turn.disable", false);

--- a/toolkit/components/extensions/parent/ext-privacy.js
+++ b/toolkit/components/extensions/parent/ext-privacy.js
@ -95,6 +95,7 @@ ExtensionPreferencesManager.addSetting("network.webRTCIPHandlingPolicy", {
  prefNames: [
    "media.peerconnection.ice.default_address_only",
    "media.peerconnection.ice.no_host",
+    "media.peerconnection.ice.proxy_only_if_behind_proxy",
    "media.peerconnection.ice.proxy_only",
  ],

@ -119,6 +120,12 @@ ExtensionPreferencesManager.addSetting("network.webRTCIPHandlingPolicy", {
        break;

      case "disable_non_proxied_udp":
+        prefs["media.peerconnection.ice.default_address_only"] = true;
+        prefs["media.peerconnection.ice.no_host"] = true;
+        prefs["media.peerconnection.ice.proxy_only_if_behind_proxy"] = true;
+        break;
+
+      case "proxy_only":
        prefs["media.peerconnection.ice.proxy_only"] = true;
        break;
    }
@ -243,14 +250,24 @@ this.privacy = class extends ExtensionAPI {
            "network.webRTCIPHandlingPolicy",
            () => {
              if (Preferences.get("media.peerconnection.ice.proxy_only")) {
-                return "disable_non_proxied_udp";
+                return "proxy_only";
              }

              let default_address_only = Preferences.get(
                "media.peerconnection.ice.default_address_only"
              );
              if (default_address_only) {
-                if (Preferences.get("media.peerconnection.ice.no_host")) {
+                let no_host = Preferences.get(
+                  "media.peerconnection.ice.no_host"
+                );
+                if (no_host) {
+                  if (
+                    Preferences.get(
+                      "media.peerconnection.ice.proxy_only_if_behind_proxy"
+                    )
+                  ) {
+                    return "disable_non_proxied_udp";
+                  }
                  return "default_public_interface_only";
                }
                return "default_public_and_private_interfaces";
--- a/toolkit/components/extensions/schemas/privacy.json
+++ b/toolkit/components/extensions/schemas/privacy.json
@ -29,7 +29,7 @@
      {
        "id": "IPHandlingPolicy",
        "type": "string",
-        "enum": ["default", "default_public_and_private_interfaces", "default_public_interface_only", "disable_non_proxied_udp"],
+        "enum": ["default", "default_public_and_private_interfaces", "default_public_interface_only", "disable_non_proxied_udp", "proxy_only"],
        "description": "The IP handling policy of WebRTC."
      }
    ],
--- a/toolkit/components/extensions/test/xpcshell/test_ext_privacy.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_privacy.js
@ -274,6 +274,7 @@ add_task(async function test_privacy_other_prefs() {
    "network.webRTCIPHandlingPolicy": {
      "media.peerconnection.ice.default_address_only": false,
      "media.peerconnection.ice.no_host": false,
+      "media.peerconnection.ice.proxy_only_if_behind_proxy": false,
      "media.peerconnection.ice.proxy_only": false,
    },
    "network.peerConnectionEnabled": {
@ -388,6 +389,7 @@ add_task(async function test_privacy_other_prefs() {
    {
      "media.peerconnection.ice.default_address_only": true,
      "media.peerconnection.ice.no_host": false,
+      "media.peerconnection.ice.proxy_only_if_behind_proxy": false,
      "media.peerconnection.ice.proxy_only": false,
    }
  );
@ -397,6 +399,7 @@ add_task(async function test_privacy_other_prefs() {
    {
      "media.peerconnection.ice.default_address_only": true,
      "media.peerconnection.ice.no_host": true,
+      "media.peerconnection.ice.proxy_only_if_behind_proxy": false,
      "media.peerconnection.ice.proxy_only": false,
    }
  );
@ -404,14 +407,22 @@ add_task(async function test_privacy_other_prefs() {
    "network.webRTCIPHandlingPolicy",
    "disable_non_proxied_udp",
    {
-      "media.peerconnection.ice.default_address_only": false,
-      "media.peerconnection.ice.no_host": false,
-      "media.peerconnection.ice.proxy_only": true,
+      "media.peerconnection.ice.default_address_only": true,
+      "media.peerconnection.ice.no_host": true,
+      "media.peerconnection.ice.proxy_only_if_behind_proxy": true,
+      "media.peerconnection.ice.proxy_only": false,
    }
  );
+  await testSetting("network.webRTCIPHandlingPolicy", "proxy_only", {
+    "media.peerconnection.ice.default_address_only": false,
+    "media.peerconnection.ice.no_host": false,
+    "media.peerconnection.ice.proxy_only_if_behind_proxy": false,
+    "media.peerconnection.ice.proxy_only": true,
+  });
  await testSetting("network.webRTCIPHandlingPolicy", "default", {
    "media.peerconnection.ice.default_address_only": false,
    "media.peerconnection.ice.no_host": false,
+    "media.peerconnection.ice.proxy_only_if_behind_proxy": false,
    "media.peerconnection.ice.proxy_only": false,
  });