Bug 1452713 - Update webRTCIPHandlingPolicy to match Chrome r=bwc,mixedpuppy
authorRyan Alderete <ralderete@mozilla.com>
Fri, 02 Aug 2019 21:33:46 +0000
changeset 486033 57b8410f0239f2756915d3ac62f152f3e70c213d
parent 486032 c41e3317531b1405c68fbb3e7c7a3dbbb9225e99
child 486034 39ff0262a9d103575fa22c3f72ee2ff26e31b917
push id36380
push userbtara@mozilla.com
push dateSat, 03 Aug 2019 09:46:28 +0000
treeherdermozilla-central@4a49d88894d8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc, mixedpuppy
bugs1452713
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
media/mtransport/nricectx.cpp
media/mtransport/nricectx.h
media/mtransport/third_party/nICEr/src/ice/ice_component.c
media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
modules/libpref/init/all.js
toolkit/components/extensions/parent/ext-privacy.js
toolkit/components/extensions/schemas/privacy.json
toolkit/components/extensions/test/xpcshell/test_ext_privacy.js
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -269,17 +269,18 @@ NrIceCtx::NrIceCtx(const std::string& na
       streams_(),
       ctx_(nullptr),
       peer_(nullptr),
       ice_handler_vtbl_(nullptr),
       ice_handler_(nullptr),
       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,
                                   bool tcp_enabled, bool allow_link_local,
                                   Policy policy) {
   // InitializeGlobals only executes once
   NrIceCtx::InitializeGlobals(allow_loopback, tcp_enabled, allow_link_local);
 
@@ -857,16 +858,18 @@ void NrIceCtx::SetCtxFlags(bool default_
   }
 }
 
 nsresult NrIceCtx::StartGathering(bool default_route_only, bool proxy_only) {
   ASSERT_ON_THREAD(sts_target_);
   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
   // finished.
   int r = nr_ice_gather(ctx_, &NrIceCtx::gather_cb, this);
 
   if (!r) {
     SetGatheringState(ICE_CTX_GATHER_COMPLETE);
@@ -1047,16 +1050,21 @@ int nr_socket_local_create(void* obj, nr
   using namespace mozilla;
 
   RefPtr<NrSocketBase> sock;
   int r, _status;
   shared_ptr<NrSocketProxyConfig> config = nullptr;
 
   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);
   if (r) {
     ABORT(r);
   }
 
   r = nr_socket_create_int(static_cast<void*>(sock), sock->vtbl(), sockp);
--- a/media/mtransport/nricectx.h
+++ b/media/mtransport/nricectx.h
@@ -303,16 +303,18 @@ class NrIceCtx {
   nsresult SetProxyServer(NrSocketProxyConfig&& config);
 
   const std::shared_ptr<NrSocketProxyConfig>& GetProxyConfig() {
     return proxy_config_;
   }
 
   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);
 
   // Start checking
   nsresult StartChecks();
 
   // Notify that the network has gone online/offline
   void UpdateNetworkState(bool online);
@@ -383,12 +385,13 @@ class NrIceCtx {
   nr_ice_peer_ctx* peer_;
   nr_ice_handler_vtbl* ice_handler_vtbl_;  // Must be pointer
   nr_ice_handler* ice_handler_;            // Must be pointer
   bool trickle_;
   nsCOMPtr<nsIEventTarget> sts_target_;  // The thread to run on
   Policy policy_;
   RefPtr<TestNat> nat_;
   std::shared_ptr<NrSocketProxyConfig> proxy_config_;
+  bool proxy_only_;
 };
 
 }  // namespace mozilla
 #endif
--- a/media/mtransport/third_party/nICEr/src/ice/ice_component.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_component.c
@@ -583,22 +583,16 @@ static int nr_ice_component_initialize_t
         /* Create a local socket */
         if((r=nr_socket_factory_create_socket(ctx->socket_factory,&addr,&local_sock))){
           r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): couldn't create socket for address %s",ctx->label,addr.as_string);
           continue;
         }
 
         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);
 
         /* The TURN socket */
         if(r=nr_socket_turn_create(buffered_sock, &turn_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
@@ -241,31 +241,16 @@ int nr_ice_ctx_set_interface_prioritizer
 
     ctx->interface_prioritizer = ip;
 
     _status=0;
    abort:
     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);
     ctx->socket_factory = factory;
   }
 
 #ifdef USE_TURN
 int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out)
@@ -474,17 +459,16 @@ static void nr_ice_ctx_destroy_cb(NR_SOC
 
     STAILQ_FOREACH_SAFE(id1, &ctx->ids, entry, id2){
       STAILQ_REMOVE(&ctx->ids,id1,nr_ice_stun_id_,entry);
       RFREE(id1);
     }
 
     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);
   }
 
 void nr_ice_ctx_add_flags(nr_ice_ctx *ctx, UINT4 flags)
   {
     ctx->flags |= flags;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
@@ -129,17 +129,16 @@ struct nr_ice_ctx_ {
   int stun_server_ct;
   nr_ice_turn_server *turn_servers;           /* The list of turn servers */
   int turn_server_ct;
   nr_local_addr *local_addrs;                 /* The list of available local addresses and corresponding interface information */
   int local_addr_ct;
 
   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;
 
   nr_ice_media_stream_head streams;           /* Media streams */
   int stream_ct;
   nr_ice_socket_head sockets;                 /* The sockets we're using */
   int uninitialized_candidates;
@@ -188,17 +187,16 @@ int nr_ice_ctx_deliver_packet(nr_ice_ctx
 int nr_ice_ctx_is_known_id(nr_ice_ctx *ctx, UCHAR id[12]);
 int nr_ice_ctx_remember_id(nr_ice_ctx *ctx, nr_stun_message *msg);
 int nr_ice_ctx_finalize(nr_ice_ctx *ctx, nr_ice_peer_ctx *pctx);
 int nr_ice_ctx_set_stun_servers(nr_ice_ctx *ctx,nr_ice_stun_server *servers, int ct);
 int nr_ice_ctx_set_turn_servers(nr_ice_ctx *ctx,nr_ice_turn_server *servers, int ct);
 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);
 int nr_ice_get_new_ice_ufrag(char** ufrag);
 int nr_ice_get_new_ice_pwd(char** pwd);
 
 #define NR_ICE_MAX_ATTRIBUTE_SIZE 256
 
--- a/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
@@ -157,16 +157,17 @@ class MediaTransportHandlerSTS : public 
   void GetIceStats(const NrIceMediaStream& aStream, DOMHighResTimeStamp aNow,
                    dom::RTCStatsReportInternal* aReport) const;
 
   virtual ~MediaTransportHandlerSTS() = default;
   nsCOMPtr<nsISerialEventTarget> mStsThread;
   RefPtr<NrIceCtx> mIceCtx;
   RefPtr<NrIceResolver> mDNSResolver;
   std::map<std::string, Transport> mTransports;
+  bool mProxyOnlyIfBehindProxy = false;
   bool mProxyOnly = false;
 
   // mDNS Support
   class DNSListener final : public nsIDNSListener {
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSIDNSLISTENER
 
    public:
@@ -420,16 +421,18 @@ nsresult MediaTransportHandlerSTS::Creat
 
         mIceCtx = NrIceCtx::Create(aName, allowLoopback, tcpEnabled,
                                    allowLinkLocal, toNrIcePolicy(aIcePolicy));
         if (!mIceCtx) {
           return InitPromise::CreateAndReject("NrIceCtx::Create failed",
                                               __func__);
         }
 
+        mProxyOnlyIfBehindProxy = Preferences::GetBool(
+            "media.peerconnection.ice.proxy_only_if_behind_proxy", false);
         mProxyOnly =
             Preferences::GetBool("media.peerconnection.ice.proxy_only", false);
 
         mIceCtx->SignalGatheringStateChange.connect(
             this, &MediaTransportHandlerSTS::OnGatheringStateChange);
         mIceCtx->SignalConnectionStateChange.connect(
             this, &MediaTransportHandlerSTS::OnConnectionStateChange);
 
@@ -653,16 +656,20 @@ void MediaTransportHandlerSTS::SetTarget
       [](const std::string& aError) {});
 }
 
 void MediaTransportHandlerSTS::StartIceGathering(
     bool aDefaultRouteOnly, const nsTArray<NrIceStunAddr>& aStunAddrs) {
   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
         // just set them here, and only do it here.
         mIceCtx->SetCtxFlags(aDefaultRouteOnly, mProxyOnly);
 
         if (aStunAddrs.Length()) {
           mIceCtx->SetStunAddrs(aStunAddrs);
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -372,16 +372,17 @@ pref("media.peerconnection.ice.force_int
 pref("media.peerconnection.ice.relay_only", false); // Limit candidates to TURN
 pref("media.peerconnection.use_document_iceservers", true);
 
 pref("media.peerconnection.identity.timeout", 10000);
 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);
 
 // These values (aec, agc, and noise) are from:
 // media/webrtc/trunk/webrtc/modules/audio_processing/include/audio_processing.h
 #if defined(MOZ_WEBRTC_HARDWARE_AEC_NS)
 pref("media.getusermedia.aec_enabled", false);
 pref("media.getusermedia.noise_enabled", false);
--- a/toolkit/components/extensions/parent/ext-privacy.js
+++ b/toolkit/components/extensions/parent/ext-privacy.js
@@ -90,16 +90,17 @@ ExtensionPreferencesManager.addSetting("
     return { [this.prefNames[0]]: value };
   },
 });
 
 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",
   ],
 
   setCallback(value) {
     let prefs = {};
     // Start with all prefs being reset.
     for (let pref of this.prefNames) {
       prefs[pref] = undefined;
@@ -114,16 +115,22 @@ ExtensionPreferencesManager.addSetting("
         break;
 
       case "default_public_interface_only":
         prefs["media.peerconnection.ice.default_address_only"] = true;
         prefs["media.peerconnection.ice.no_host"] = true;
         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;
     }
     return prefs;
   },
 });
 
 ExtensionPreferencesManager.addSetting("services.passwordSavingEnabled", {
@@ -238,24 +245,34 @@ this.privacy = class extends ExtensionAP
               return Preferences.get("media.peerconnection.enabled");
             }
           ),
           webRTCIPHandlingPolicy: getPrivacyAPI(
             extension,
             "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";
               }
 
               return "default";
             }
           ),
--- a/toolkit/components/extensions/schemas/privacy.json
+++ b/toolkit/components/extensions/schemas/privacy.json
@@ -24,17 +24,17 @@
   {
     "namespace": "privacy.network",
     "description": "Use the <code>browser.privacy</code> API to control usage of the features in the browser that can affect a user's privacy.",
     "permissions": ["privacy"],
     "types": [
       {
         "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."
       }
     ],
     "properties": {
       "networkPredictionEnabled": {
         "$ref": "types.Setting",
         "description": "If enabled, the browser attempts to speed up your web browsing experience by pre-resolving DNS entries, prerendering sites (<code>&lt;link rel='prefetch' ...&gt;</code>), and preemptively opening TCP and SSL connections to servers.  This preference's value is a boolean, defaulting to <code>true</code>."
       },
--- a/toolkit/components/extensions/test/xpcshell/test_ext_privacy.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_privacy.js
@@ -269,16 +269,17 @@ add_task(async function test_privacy() {
 add_task(async function test_privacy_other_prefs() {
   const cookieSvc = Ci.nsICookieService;
 
   // Create an object to hold the values to which we will initialize the prefs.
   const SETTINGS = {
     "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": {
       "media.peerconnection.enabled": true,
     },
     "services.passwordSavingEnabled": {
       "signon.rememberSignons": true,
     },
@@ -383,40 +384,50 @@ add_task(async function test_privacy_oth
   }
 
   await testSetting(
     "network.webRTCIPHandlingPolicy",
     "default_public_and_private_interfaces",
     {
       "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,
     }
   );
   await testSetting(
     "network.webRTCIPHandlingPolicy",
     "default_public_interface_only",
     {
       "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,
     }
   );
   await testSetting(
     "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,
   });
 
   await testSetting("network.peerConnectionEnabled", false, {
     "media.peerconnection.enabled": false,
   });
   await testSetting("network.peerConnectionEnabled", true, {
     "media.peerconnection.enabled": true,