Bug 1548841 - Do not reveal peer reflexive candidate addresses unless otherwise signaled; r=bwc
authorDan Minor <dminor@mozilla.com>
Wed, 15 May 2019 17:38:54 +0000
changeset 475753 3c4fb056f40c649f8f79d3710a8fb57a0c8fe0ef
parent 475752 15ec66f24d058464e5661742c15daeff1d81e1a4
child 475754 49f7d8f42f5eb06c6c106372965b25fcd122bf51
push id86458
push userdminor@mozilla.com
push dateMon, 27 May 2019 16:57:14 +0000
treeherderautoland@3c4fb056f40c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1548841
milestone69.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 1548841 - Do not reveal peer reflexive candidate addresses unless otherwise signaled; r=bwc Peer reflexive candidates could leak local addresses otherwise hidden by using mDNS. These must not be part of ICE statistics unless the same address has been signaled separately. See: https://tools.ietf.org/html/draft-ietf-rtcweb-mdns-ice-candidates-03#section-3.3.1 Differential Revision: https://phabricator.services.mozilla.com/D30938
media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
--- a/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp
@@ -191,16 +191,17 @@ class MediaTransportHandlerSTS : public 
    private:
     ~DNSListener() = default;
   };
 
   void PendingDNSRequestResolved(DNSListener* aListener);
 
   nsCOMPtr<nsIDNSService> mDNSService;
   std::set<RefPtr<DNSListener>> mPendingDNSRequests;
+  std::set<std::string> mSignaledAddresses;
 };
 
 /* static */
 already_AddRefed<MediaTransportHandler> MediaTransportHandler::Create(
     nsISerialEventTarget* aCallbackThread) {
   RefPtr<MediaTransportHandler> result;
   if (XRE_IsContentProcess() &&
       Preferences::GetBool("media.peerconnection.mtransport_process") &&
@@ -737,17 +738,21 @@ void MediaTransportHandlerSTS::AddIceCan
   RefPtr<NrIceMediaStream> stream(mIceCtx->GetStream(aTransportId));
   if (!stream) {
     CSFLogError(LOGTAG, "No ICE stream for candidate with transport id %s: %s",
                 aTransportId.c_str(), aCandidate.c_str());
     return;
   }
 
   nsresult rv = stream->ParseTrickleCandidate(aCandidate, aUfrag, "");
-  if (NS_FAILED(rv)) {
+  if (NS_SUCCEEDED(rv)) {
+    if (tokens.size() > 4) {
+      mSignaledAddresses.insert(tokens[4]);
+    }
+  } else {
     CSFLogError(LOGTAG,
                 "Couldn't process ICE candidate with transport id %s: "
                 "%s",
                 aTransportId.c_str(), aCandidate.c_str());
   }
 }
 
 void MediaTransportHandlerSTS::UpdateNetworkState(bool aOnline) {
@@ -1060,32 +1065,37 @@ void MediaTransportHandlerSTS::ExitPriva
   if (log) {
     log->ExitPrivateMode();
   }
 }
 
 static void ToRTCIceCandidateStats(
     const std::vector<NrIceCandidate>& candidates,
     dom::RTCStatsType candidateType, const nsString& transportId,
-    DOMHighResTimeStamp now, dom::RTCStatsReportInternal* report) {
+    DOMHighResTimeStamp now, dom::RTCStatsReportInternal* report,
+    const std::set<std::string>& signaledAddresses) {
   MOZ_ASSERT(report);
   for (const auto& candidate : candidates) {
     dom::RTCIceCandidateStats cand;
     cand.mType.Construct(candidateType);
     NS_ConvertASCIItoUTF16 codeword(candidate.codeword.c_str());
     cand.mTransportId.Construct(transportId);
     cand.mId.Construct(codeword);
     cand.mTimestamp.Construct(now);
     cand.mCandidateType.Construct(dom::RTCIceCandidateType(candidate.type));
     cand.mPriority.Construct(candidate.priority);
     // 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 if (candidate.type == NrIceCandidate::ICE_PEER_REFLEXIVE &&
+               signaledAddresses.find(candidate.cand_addr.host) ==
+                   signaledAddresses.end()) {
+      cand.mAddress.Construct(NS_ConvertASCIItoUTF16("(redacted)"));
     } 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()));
     if (candidateType == dom::RTCStatsType::Local_candidate &&
@@ -1143,28 +1153,28 @@ void MediaTransportHandlerSTS::GetIceSta
     s.mState.Construct(dom::RTCStatsIceCandidatePairState(candPair.state));
     s.mComponentId.Construct(candPair.component_id);
     aReport->mIceCandidatePairStats.Value().AppendElement(s, fallible);
   }
 
   std::vector<NrIceCandidate> candidates;
   if (NS_SUCCEEDED(aStream.GetLocalCandidates(&candidates))) {
     ToRTCIceCandidateStats(candidates, dom::RTCStatsType::Local_candidate,
-                           transportId, aNow, aReport);
+                           transportId, aNow, aReport, mSignaledAddresses);
     // add the local candidates unparsed string to a sequence
     for (const auto& candidate : candidates) {
       aReport->mRawLocalCandidates.Value().AppendElement(
           NS_ConvertASCIItoUTF16(candidate.label.c_str()), fallible);
     }
   }
   candidates.clear();
 
   if (NS_SUCCEEDED(aStream.GetRemoteCandidates(&candidates))) {
     ToRTCIceCandidateStats(candidates, dom::RTCStatsType::Remote_candidate,
-                           transportId, aNow, aReport);
+                           transportId, aNow, aReport, mSignaledAddresses);
     // add the remote candidates unparsed string to a sequence
     for (const auto& candidate : candidates) {
       aReport->mRawRemoteCandidates.Value().AppendElement(
           NS_ConvertASCIItoUTF16(candidate.label.c_str()), fallible);
     }
   }
 }
 
@@ -1387,18 +1397,33 @@ nsresult MediaTransportHandlerSTS::DNSLi
         for (size_t i = 0; i < mTokenizedCandidate.size(); ++i) {
           o << mTokenizedCandidate[i];
           if (i + 1 != mTokenizedCandidate.size()) {
             o << " ";
           }
         }
 
         std::string mungedCandidate = o.str();
-        mTransportHandler.AddIceCandidate(mTransportId, mungedCandidate,
-                                          mUfrag);
+
+        RefPtr<NrIceMediaStream> stream(
+            mTransportHandler.mIceCtx->GetStream(mTransportId));
+        if (!stream) {
+          CSFLogError(LOGTAG,
+                      "No ICE stream for candidate with transport id %s: %s",
+                      mTransportId.c_str(), mCandidate.c_str());
+          return NS_OK;
+        }
+
+        nsresult rv = stream->ParseTrickleCandidate(mCandidate, mUfrag, "");
+        if (NS_FAILED(rv)) {
+          CSFLogError(LOGTAG,
+                      "Couldn't process ICE candidate with transport id %s: "
+                      "%s",
+                      mTransportId.c_str(), mCandidate.c_str());
+        }
       } else {
         CSFLogError(LOGTAG,
                     "More than one mDNS result for candidate with transport"
                     " id: %s: %s, candidate ignored",
                     mTransportId.c_str(), mCandidate.c_str());
       }
     } else {
       CSFLogError(LOGTAG,