Bug 1548841 - Obfuscate mDNS ICE candidate addresses; r=bwc

This adds a mdns_addr field to nICEr ICE candidates to track the mDNS address
associated with a candidate, if any. This is used to hide the real address
when generating ICE stats. This potentially could be handled at the
MediaTransportHandler level, but we need to know if a candidate is mDNS to
prevent pairing it with relay candidates, which is part of the next commit.

This adds a unit tests to check that the mDNS addresses are handled properly.
As part of doing this, TestBogusCandidate was fixed. As written, it was never
parsing the bogus candidate because it was in the wrong ICE state.

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

--HG--
extra : moz-landing-system : lando

media/mtransport/nricemediastream.cpp

@@ -114,6 +114,10 @@ static bool ToNrIceCandidate(const nr_ice_candidate& candc,
 
   if (!ToNrIceAddr(cand->addr, &out->cand_addr)) return false;
 
+  if (cand->mdns_addr) {
+    out->mdns_addr = cand->mdns_addr;
+  }
+
   if (cand->isock) {
     nr_transport_addr addr;
     r = nr_socket_getaddr(cand->isock->sock, &addr);
@@ -270,7 +274,8 @@ nsresult NrIceMediaStream::SetIceCredentials(const std::string& ufrag,
 
 // Parse trickle ICE candidate
 nsresult NrIceMediaStream::ParseTrickleCandidate(const std::string& candidate,
-                                                 const std::string& ufrag) {
+                                                 const std::string& ufrag,
+                                                 const std::string& mdns_addr) {
   nr_ice_media_stream* stream = GetStreamForRemoteUfrag(ufrag);
   if (!stream) {
     return NS_ERROR_FAILURE;
@@ -281,7 +286,7 @@ nsresult NrIceMediaStream::ParseTrickleCandidate(const std::string& candidate,
                                   << candidate);
 
   int r = nr_ice_peer_ctx_parse_trickle_candidate(
-      ctx_peer_, stream, const_cast<char*>(candidate.c_str()));
+      ctx_peer_, stream, const_cast<char*>(candidate.c_str()), mdns_addr.c_str());
 
   if (r) {
     if (r == R_ALREADY) {

media/mtransport/nricemediastream.h

@@ -80,6 +80,7 @@ struct NrIceCandidate {
 
   NrIceAddr cand_addr;
   NrIceAddr local_addr;
+  std::string mdns_addr;
   Type type;
   TcpType tcp_type;
   std::string codeword;
@@ -153,7 +154,8 @@ class NrIceMediaStream {
 
   // Parse trickle ICE candidate
   nsresult ParseTrickleCandidate(const std::string& candidate,
-                                 const std::string& ufrag);
+                                 const std::string& ufrag,
+                                 const std::string& mdns_addr);
 
   // Disable a component
   nsresult DisableComponent(int component);

media/mtransport/test/ice_unittest.cpp

@@ -809,7 +809,7 @@ class IceTestPeer : public sigslot::has_slots<> {
       // stream might have gone away before the trickle timer popped
       return NS_OK;
     }
-    return stream->ParseTrickleCandidate(candidate, ufrag);
+    return stream->ParseTrickleCandidate(candidate, ufrag, "");
   }
 
   void DumpCandidate(std::string which, const NrIceCandidate& cand) {
@@ -1001,7 +1001,8 @@ class IceTestPeer : public sigslot::has_slots<> {
         if (GetStream_s(i) == stream) {
           ASSERT_GT(remote_->stream_counter_, i);
           nsresult res =
-              remote_->GetStream_s(i)->ParseTrickleCandidate(candidate, ufrag);
+              remote_->GetStream_s(i)->ParseTrickleCandidate(candidate, ufrag,
+                                                             "");
           ASSERT_TRUE(NS_SUCCEEDED(res));
           return;
         }
@@ -1219,16 +1220,19 @@ class IceTestPeer : public sigslot::has_slots<> {
     candidate_filter_ = filter;
   }
 
-  void ParseCandidate_s(size_t i, const std::string& candidate) {
+  void ParseCandidate_s(size_t i, const std::string& candidate,
+                        const std::string& mdns_addr) {
     auto media_stream = GetStream_s(i);
     ASSERT_TRUE(media_stream.get())
     << "No such stream " << i;
-    media_stream->ParseTrickleCandidate(candidate, "");
+    media_stream->ParseTrickleCandidate(candidate, "", mdns_addr);
   }
 
-  void ParseCandidate(size_t i, const std::string& candidate) {
+  void ParseCandidate(size_t i, const std::string& candidate,
+                      const std::string &mdns_addr) {
     test_utils_->sts_target()->Dispatch(
-        WrapRunnable(this, &IceTestPeer::ParseCandidate_s, i, candidate),
+        WrapRunnable(this, &IceTestPeer::ParseCandidate_s, i, candidate,
+                     mdns_addr),
         NS_DISPATCH_SYNC);
   }
 
@@ -2274,13 +2278,6 @@ TEST_F(WebRtcIceGatherTest, TestGatherTcpDisabled) {
   ASSERT_TRUE(StreamHasMatchingCandidate(0, " UDP "));
 }
 
-// Verify that a bogus candidate doesn't cause crashes on the
-// main thread. See bug 856433.
-TEST_F(WebRtcIceGatherTest, TestBogusCandidate) {
-  Gather();
-  peer_->ParseCandidate(0, kBogusIceCandidate);
-}
-
 TEST_F(WebRtcIceGatherTest, VerifyTestStunServer) {
   UseFakeStunUdpServerWithResponse("192.0.2.133", 3333);
   Gather();
@@ -3610,6 +3607,46 @@ TEST_F(WebRtcIceConnectTest, TestRLogConnector) {
   }
 }
 
+// Verify that a bogus candidate doesn't cause crashes on the
+// main thread. See bug 856433.
+TEST_F(WebRtcIceConnectTest, TestBogusCandidate) {
+  AddStream(1);
+  Gather();
+  ConnectTrickle();
+  p1_->ParseCandidate(0, kBogusIceCandidate, "");
+
+  std::vector<NrIceCandidatePair> pairs;
+  nsresult res = p1_->GetCandidatePairs(0, &pairs);
+  ASSERT_EQ(NS_OK, res);
+  ASSERT_EQ(0U, pairs.size());
+}
+
+TEST_F(WebRtcIceConnectTest, TestNonMDNSCandidate) {
+  AddStream(1);
+  Gather();
+  ConnectTrickle();
+  p1_->ParseCandidate(0, kUnreachableHostIceCandidate, "");
+
+  std::vector<NrIceCandidatePair> pairs;
+  nsresult res = p1_->GetCandidatePairs(0, &pairs);
+  ASSERT_EQ(NS_OK, res);
+  ASSERT_EQ(1U, pairs.size());
+  ASSERT_EQ(pairs[0].remote.mdns_addr, "");
+}
+
+TEST_F(WebRtcIceConnectTest, TestMDNSCandidate) {
+  AddStream(1);
+  Gather();
+  ConnectTrickle();
+  p1_->ParseCandidate(0, kUnreachableHostIceCandidate, "host.local");
+
+  std::vector<NrIceCandidatePair> pairs;
+  nsresult res = p1_->GetCandidatePairs(0, &pairs);
+  ASSERT_EQ(NS_OK, res);
+  ASSERT_EQ(1U, pairs.size());
+  ASSERT_EQ(pairs[0].remote.mdns_addr, "host.local");
+}
+
 TEST_F(WebRtcIcePrioritizerTest, TestPrioritizer) {
   SetPriorizer(::mozilla::CreateInterfacePrioritizer());
 

media/mtransport/third_party/nICEr/src/ice/ice_candidate.c

@@ -346,6 +346,7 @@ int nr_ice_candidate_destroy(nr_ice_candidate **candp)
         break;
     }
 
+    RFREE(cand->mdns_addr);
     RFREE(cand->foundation);
     RFREE(cand->label);
     RFREE(cand);

media/mtransport/third_party/nICEr/src/ice/ice_candidate.h

@@ -65,6 +65,7 @@ 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)*/
+  char *mdns_addr;                    /* MDNS address, if any */
   char *foundation;                   /* Foundation for the candidate (S 4) */
   UINT4 priority;                     /* The priority value (S 5.4 */
   nr_ice_stun_server *stun_server;

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c

@@ -44,7 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg);
 static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct);
-static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled);
+static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled, const char *mdns_addr);
 static void nr_ice_peer_ctx_start_trickle_timer(nr_ice_peer_ctx *pctx);
 
 int nr_ice_peer_ctx_create(nr_ice_ctx *ctx, nr_ice_handler *handler,char *label, nr_ice_peer_ctx **pctxp)
@@ -158,7 +158,7 @@ static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr
         }
       }
       else if (!strncmp(attrs[i],"candidate",9)){
-        if(r=nr_ice_ctx_parse_candidate(pctx,pstream,attrs[i],0)) {
+        if(r=nr_ice_ctx_parse_candidate(pctx,pstream,attrs[i],0,0)) {
           r_log(LOG_ICE,LOG_WARNING,"ICE(%s): peer (%s) specified bogus candidate",pctx->ctx->label,pctx->label);
           continue;
         }
@@ -172,7 +172,7 @@ static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr
     return(0);
   }
 
-static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled)
+static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled, const char *mdns_addr)
   {
     nr_ice_candidate *cand=0;
     nr_ice_component *comp;
@@ -185,6 +185,13 @@ static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream
     /* set the trickled flag on the candidate */
     cand->trickled = trickled;
 
+    if (mdns_addr) {
+      cand->mdns_addr = r_strdup(mdns_addr);
+      if (!cand->mdns_addr) {
+        ABORT(R_NO_MEMORY);
+      }
+    }
+
     /* Not the fastest way to find a component, but it's what we got */
     j=1;
     for(comp=STAILQ_FIRST(&pstream->components);comp;comp=STAILQ_NEXT(comp,entry)){
@@ -265,7 +272,7 @@ int nr_ice_peer_ctx_remove_pstream(nr_ice_peer_ctx *pctx, nr_ice_media_stream **
     return(_status);
   }
 
-int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char *candidate)
+int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char *candidate, const char *mdns_addr)
   {
     nr_ice_media_stream *pstream;
     int r,_status;
@@ -289,7 +296,7 @@ int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_
         break;
     }
 
-    if(r=nr_ice_ctx_parse_candidate(pctx,pstream,candidate,1)){
+    if(r=nr_ice_ctx_parse_candidate(pctx,pstream,candidate,1,mdns_addr)){
       ABORT(r);
     }
 

media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.h

@@ -74,7 +74,7 @@ int nr_ice_peer_ctx_destroy(nr_ice_peer_ctx **pctxp);
 int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char **attrs, int attr_ct);
 int nr_ice_peer_ctx_find_pstream(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream **pstreamp);
 int nr_ice_peer_ctx_remove_pstream(nr_ice_peer_ctx *pctx, nr_ice_media_stream **pstreamp);
-int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char *cand);
+int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char *cand, const char *mdns_addr);
 
 int nr_ice_peer_ctx_pair_candidates(nr_ice_peer_ctx *pctx);
 int nr_ice_peer_ctx_parse_global_attributes(nr_ice_peer_ctx *pctx, char **attrs, int attr_ct);

media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp

@@ -741,7 +741,7 @@ void MediaTransportHandlerSTS::AddIceCandidate(const std::string& aTransportId,
     return;
   }
 
-  nsresult rv = stream->ParseTrickleCandidate(aCandidate, aUfrag);
+  nsresult rv = stream->ParseTrickleCandidate(aCandidate, aUfrag, "");
   if (NS_FAILED(rv)) {
     CSFLogError(LOGTAG,
                 "Couldn't process ICE candidate with transport id %s: "
@@ -1076,8 +1076,15 @@ static void ToRTCIceCandidateStats(
     cand.mTimestamp.Construct(now);
     cand.mCandidateType.Construct(dom::RTCIceCandidateType(candidate.type));
     cand.mPriority.Construct(candidate.priority);
-    cand.mAddress.Construct(
-        NS_ConvertASCIItoUTF16(candidate.cand_addr.host.c_str()));
+    // https://tools.ietf.org/html/draft-ietf-rtcweb-mdns-ice-candidates-03#section-3.3.1
+    // This obfuscates the address with the mDNS address if one exists
+    if (!candidate.mdns_addr.empty()) {
+      cand.mAddress.Construct(
+          NS_ConvertASCIItoUTF16(candidate.mdns_addr.c_str()));
+    } else {
+      cand.mAddress.Construct(
+          NS_ConvertASCIItoUTF16(candidate.cand_addr.host.c_str()));
+    }
     cand.mPort.Construct(candidate.cand_addr.port);
     cand.mProtocol.Construct(
         NS_ConvertASCIItoUTF16(candidate.cand_addr.transport.c_str()));