Bug 1742337 - Fix discarding of srflx candidates r=webrtc-reviewers,bwc

If host address obfuscation (media.peerconnection.ice.obfuscate_host_addresses)
is enabled, a srflx candidate "MUST NOT be considered redundant" even if
its IP address is the same as the local IP address.

Reference:
https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-mdns-ice-candidates-03#section-3.1.2.2

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

dom/media/webrtc/transport/nricectx.cpp

@@ -269,8 +269,7 @@ NrIceCtx::NrIceCtx(const std::string& name)
       trickle_(true),
       config_(),
       nat_(nullptr),
-      proxy_config_(nullptr),
-      obfuscate_host_addresses_(false) {}
+      proxy_config_(nullptr) {}
 
 /* static */
 RefPtr<NrIceCtx> NrIceCtx::Create(const std::string& aName) {
@@ -288,17 +287,17 @@ nsresult NrIceCtx::SetIceConfig(const Config& aConfig) {
   switch (config_.mPolicy) {
     case ICE_POLICY_RELAY:
       MOZ_MTLOG(ML_DEBUG, "SetIceConfig: relay only");
-      nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES);
+      nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES);
       nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_RELAY_ONLY);
       break;
     case ICE_POLICY_NO_HOST:
       MOZ_MTLOG(ML_DEBUG, "SetIceConfig: no host");
-      nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES);
+      nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES);
       nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_RELAY_ONLY);
       break;
     case ICE_POLICY_ALL:
       MOZ_MTLOG(ML_DEBUG, "SetIceConfig: all");
-      nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES);
+      nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES);
       nr_ice_ctx_remove_flags(ctx_, NR_ICE_CTX_FLAGS_RELAY_ONLY);
       break;
   }
@@ -478,9 +477,9 @@ void NrIceCtx::trickle_cb(void* arg, nr_ice_ctx* ice_ctx,
 
   // Format the candidate.
   char candidate_str[NR_ICE_MAX_ATTRIBUTE_SIZE];
-  int r = nr_ice_format_candidate_attribute(candidate, candidate_str,
-                                            sizeof(candidate_str),
-                                            ctx->obfuscate_host_addresses_);
+  int r = nr_ice_format_candidate_attribute(
+      candidate, candidate_str, sizeof(candidate_str),
+      (ctx->ctx()->flags & NR_ICE_CTX_FLAGS_OBFUSCATE_HOST_ADDRESSES) ? 1 : 0);
   MOZ_ASSERT(!r);
   if (r) return;
 
@@ -845,7 +844,9 @@ nsresult NrIceCtx::StartGathering(bool default_route_only,
   ASSERT_ON_THREAD(sts_target_);
   MOZ_MTLOG(ML_NOTICE, "NrIceCtx(" << name_ << "): " << __func__);
 
-  obfuscate_host_addresses_ = obfuscate_host_addresses;
+  if (obfuscate_host_addresses) {
+    nr_ice_ctx_add_flags(ctx_, NR_ICE_CTX_FLAGS_OBFUSCATE_HOST_ADDRESSES);
+  }
 
   SetCtxFlags(default_route_only);
 
@@ -1013,7 +1014,8 @@ void NrIceCtx::SetGatheringState(GatheringState state) {
 void NrIceCtx::GenerateObfuscatedAddress(nr_ice_candidate* candidate,
                                          std::string* mdns_address,
                                          std::string* actual_address) {
-  if (candidate->type == HOST && obfuscate_host_addresses_) {
+  if (candidate->type == HOST &&
+      (ctx_->flags & NR_ICE_CTX_FLAGS_OBFUSCATE_HOST_ADDRESSES)) {
     char addr[64];
     if (nr_transport_addr_get_addrstring(&candidate->addr, addr,
                                          sizeof(addr))) {

dom/media/webrtc/transport/nricectx.h

@@ -414,7 +414,6 @@ class NrIceCtx {
   Config config_;
   RefPtr<TestNat> nat_;
   std::shared_ptr<NrSocketProxyConfig> proxy_config_;
-  bool obfuscate_host_addresses_;
   std::map<std::string, std::string> obfuscated_host_addresses_;
 };
 

dom/media/webrtc/transport/test/ice_unittest.cpp

@@ -523,12 +523,13 @@ class IceTestPeer : public sigslot::has_slots<> {
         NS_SUCCEEDED(ice_ctx_->SetResolver(dns_resolver_->AllocateResolver())));
   }
 
-  void Gather(bool default_route_only = false) {
+  void Gather(bool default_route_only = false,
+              bool obfuscate_host_addresses = false) {
     nsresult res;
 
     test_utils_->sts_target()->Dispatch(
         WrapRunnableRet(&res, ice_ctx_, &NrIceCtx::StartGathering,
-                        default_route_only, false),
+                        default_route_only, obfuscate_host_addresses),
         NS_DISPATCH_SYNC);
 
     ASSERT_TRUE(NS_SUCCEEDED(res));
@@ -1419,9 +1420,11 @@ class WebRtcIceGatherTest : public StunTest {
     }
   }
 
-  void Gather(unsigned int waitTime = kDefaultTimeout) {
+  void Gather(unsigned int waitTime = kDefaultTimeout,
+              bool default_route_only = false,
+              bool obfuscate_host_addresses = false) {
     EnsurePeer();
-    peer_->Gather();
+    peer_->Gather(default_route_only, obfuscate_host_addresses);
 
     if (waitTime) {
       WaitForGather(waitTime);
@@ -2553,6 +2556,48 @@ TEST_F(WebRtcIceGatherTest, TestFakeStunServerNoNatNoHost) {
   ASSERT_TRUE(StreamHasMatchingCandidate(0, "srflx"));
 }
 
+// Test that srflx candidate is discarded in non-NATted environment if host
+// address obfuscation is not enabled.
+TEST_F(WebRtcIceGatherTest,
+       TestSrflxCandidateDiscardedWithObfuscateHostAddressesNotEnabled) {
+  {
+    NrIceCtx::GlobalConfig config;
+    config.mTcpEnabled = false;
+    NrIceCtx::InitializeGlobals(config);
+  }
+
+  NrIceCtx::Config config;
+  peer_ = MakeUnique<IceTestPeer>("P1", test_utils_, true, config);
+  UseTestStunServer();
+  peer_->AddStream(1);
+  Gather(0, false, false);
+  WaitForGather();
+  DumpAttributes(0);
+  EXPECT_TRUE(StreamHasMatchingCandidate(0, "host"));
+  EXPECT_FALSE(StreamHasMatchingCandidate(0, "srflx"));
+}
+
+// Test that srflx candidate is generated in non-NATted environment if host
+// address obfuscation is enabled.
+TEST_F(WebRtcIceGatherTest,
+       TestSrflxCandidateGeneratedWithObfuscateHostAddressesEnabled) {
+  {
+    NrIceCtx::GlobalConfig config;
+    config.mTcpEnabled = false;
+    NrIceCtx::InitializeGlobals(config);
+  }
+
+  NrIceCtx::Config config;
+  peer_ = MakeUnique<IceTestPeer>("P1", test_utils_, true, config);
+  UseTestStunServer();
+  peer_->AddStream(1);
+  Gather(0, false, true);
+  WaitForGather();
+  DumpAttributes(0);
+  EXPECT_TRUE(StreamHasMatchingCandidate(0, "host"));
+  EXPECT_TRUE(StreamHasMatchingCandidate(0, "srflx"));
+}
+
 TEST_F(WebRtcIceGatherTest, TestStunTcpServerTrickle) {
   NrIceCtx::GlobalConfig config;
   config.mTcpEnabled = true;

dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_candidate.c

@@ -999,7 +999,7 @@ int nr_ice_format_candidate_attribute(nr_ice_candidate *cand, char *attr, int ma
     /* raddr, rport */
     raddr = (cand->stream->flags &
              (NR_ICE_CTX_FLAGS_RELAY_ONLY |
-              NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES)) ?
+              NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES)) ?
       &cand->addr : &cand->base;
 
     switch(cand->type){

dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_candidate.h

@@ -65,7 +65,6 @@ struct nr_ice_candidate_ {
   nr_transport_addr addr;             /* The advertised address;
                                          JDR calls this the candidate */
   nr_transport_addr base;             /* The base address (S 2.1)*/
-  int obfuscate_addrs;                /* True if addresses should be obfuscated */
   char *mdns_addr;                    /* MDNS address, if any */
   char *foundation;                   /* Foundation for the candidate (S 4) */
   UINT4 priority;                     /* The priority value (S 5.4 */

dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_component.c

@@ -751,7 +751,8 @@ int nr_ice_component_maybe_prune_candidate(nr_ice_ctx *ctx, nr_ice_component *co
          !nr_transport_addr_cmp(&c1->addr,&c2->addr,NR_TRANSPORT_ADDR_CMP_MODE_ALL)){
 
         if((c1->type == c2->type) ||
-           (!(ctx->flags & NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES) &&
+           (!(ctx->flags & NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES) &&
+            !(ctx->flags & NR_ICE_CTX_FLAGS_OBFUSCATE_HOST_ADDRESSES) &&
             ((c1->type==HOST && c2->type == SERVER_REFLEXIVE) ||
              (c2->type==HOST && c1->type == SERVER_REFLEXIVE)))){
 

dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_ctx.c

@@ -1074,7 +1074,7 @@ int nr_ice_ctx_hide_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand)
       return 1;
     }
 
-    if (ctx->flags & NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES) {
+    if (ctx->flags & NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES) {
       if (cand->type == HOST)
         return 1;
     }

dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_ctx.h

@@ -146,9 +146,10 @@ int nr_ice_ctx_create_with_credentials(char *label, UINT4 flags, char* ufrag, ch
 #define NR_ICE_CTX_FLAGS_AGGRESSIVE_NOMINATION             (1)
 #define NR_ICE_CTX_FLAGS_LITE                              (1<<1)
 #define NR_ICE_CTX_FLAGS_RELAY_ONLY                        (1<<2)
-#define NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES              (1<<3)
+#define NR_ICE_CTX_FLAGS_DISABLE_HOST_CANDIDATES           (1<<3)
 #define NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS                (1<<4)
 #define NR_ICE_CTX_FLAGS_ONLY_PROXY                        (1<<5)
+#define NR_ICE_CTX_FLAGS_OBFUSCATE_HOST_ADDRESSES          (1<<6)
 
 void nr_ice_ctx_add_flags(nr_ice_ctx *ctx, UINT4 flags);
 void nr_ice_ctx_remove_flags(nr_ice_ctx *ctx, UINT4 flags);