Fix local address leakage when IceTransportsType is relay

As part of implementing IceTransportsType constraint, we should hide the raddr which is the mapped address to prevent local address leakage.

BUG=1179
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/26889004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7484 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/p2p/base/port.cc

@@ -31,6 +31,7 @@
 #include <vector>
 
 #include "talk/p2p/base/common.h"
+#include "talk/p2p/base/portallocator.h"
 #include "webrtc/base/base64.h"
 #include "webrtc/base/crc32.h"
 #include "webrtc/base/helpers.h"
@@ -187,7 +188,8 @@ Port::Port(rtc::Thread* thread, rtc::PacketSocketFactory* factory,
       ice_protocol_(ICEPROTO_HYBRID),
       ice_role_(ICEROLE_UNKNOWN),
       tiebreaker_(0),
-      shared_socket_(true) {
+      shared_socket_(true),
+      candidate_filter_(CF_ALL) {
   Construct();
 }
 
@@ -213,7 +215,8 @@ Port::Port(rtc::Thread* thread, const std::string& type,
       ice_protocol_(ICEPROTO_HYBRID),
       ice_role_(ICEROLE_UNKNOWN),
       tiebreaker_(0),
-      shared_socket_(false) {
+      shared_socket_(false),
+      candidate_filter_(CF_ALL) {
   ASSERT(factory_ != NULL);
   Construct();
 }

talk/p2p/base/port.h

@@ -308,6 +308,10 @@ class Port : public PortInterface, public rtc::MessageHandler,
   // Returns if Hybrid ICE protocol is used.
   bool IsHybridIce() const;
 
+  void set_candidate_filter(uint32 candidate_filter) {
+    candidate_filter_ = candidate_filter;
+  }
+
  protected:
   enum {
     MSG_CHECKTIMEOUT = 0,
@@ -351,6 +355,8 @@ class Port : public PortInterface, public rtc::MessageHandler,
     return rtc::DSCP_NO_CHANGE;
   }
 
+  uint32 candidate_filter() { return candidate_filter_; }
+
  private:
   void Construct();
   // Called when one of our connections deletes itself.
@@ -392,6 +398,12 @@ class Port : public PortInterface, public rtc::MessageHandler,
   std::string user_agent_;
   rtc::ProxyInfo proxy_;
 
+  // Candidate filter is pushed down to Port such that each Port could
+  // make its own decision on how to create candidates. For example,
+  // when IceTransportsType is set to relay, both RelayPort and
+  // TurnPort will hide raddr to avoid local address leakage.
+  uint32 candidate_filter_;
+
   friend class Connection;
 };
 

talk/p2p/base/stunport.cc

@@ -28,6 +28,7 @@
 #include "talk/p2p/base/stunport.h"
 
 #include "talk/p2p/base/common.h"
+#include "talk/p2p/base/portallocator.h"
 #include "talk/p2p/base/stun.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/helpers.h"
@@ -395,8 +396,17 @@ void UDPPort::OnStunBindingRequestSucceeded(
   // For STUN, related address is the local socket address.
   if ((!SharedSocket() || stun_reflected_addr != socket_->GetLocalAddress()) &&
       !HasCandidateWithAddress(stun_reflected_addr)) {
+
+    rtc::SocketAddress related_address = socket_->GetLocalAddress();
+    if (!(candidate_filter() & CF_HOST)) {
+      // If candidate filter doesn't have CF_HOST specified, empty raddr to
+      // avoid local address leakage.
+      related_address = rtc::EmptySocketAddressWithFamily(
+          related_address.family());
+    }
+
     AddAddress(stun_reflected_addr, socket_->GetLocalAddress(),
-               socket_->GetLocalAddress(), UDP_PROTOCOL_NAME, "",
+               related_address, UDP_PROTOCOL_NAME, "",
                STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, 0, false);
   }
   MaybeSetPortCompleteOrError();

talk/p2p/base/turnport.cc

@@ -588,10 +588,18 @@ void TurnPort::OnStunAddress(const rtc::SocketAddress& address) {
 void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address,
                                  const rtc::SocketAddress& stun_address) {
   connected_ = true;
+
+  rtc::SocketAddress related_address = stun_address;
+    if (!(candidate_filter() & CF_REFLEXIVE)) {
+    // If candidate filter only allows relay type of address, empty raddr to
+    // avoid local address leakage.
+    related_address = rtc::EmptySocketAddressWithFamily(stun_address.family());
+  }
+
   // For relayed candidate, Base is the candidate itself.
-  AddAddress(address,  // Candidate address.
+  AddAddress(address,          // Candidate address.
-             address,  // Base address.
+             address,          // Base address.
-             stun_address,  // Related address.
+             related_address,  // Related address.
              UDP_PROTOCOL_NAME,
              "",  // TCP canddiate type, empty for turn candidates.
              RELAY_PORT_TYPE,

talk/p2p/client/basicportallocator.cc

@@ -497,6 +497,9 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port,
   port->set_send_retransmit_count_attribute((allocator_->flags() &
       PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE) != 0);
 
+  // Push down the candidate_filter to individual port.
+  port->set_candidate_filter(allocator_->candidate_filter());
+
   PortData data(port, seq);
   ports_.push_back(data);
 

talk/p2p/client/portallocator_unittest.cc

@@ -113,6 +113,8 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
         candidate_allocation_done_(false) {
     cricket::ServerAddresses stun_servers;
     stun_servers.insert(kStunAddr);
+    // Passing the addresses of GTURN servers will enable GTURN in
+    // Basicportallocator.
     allocator_.reset(new cricket::BasicPortAllocator(
         &network_manager_,
         stun_servers,
@@ -137,6 +139,14 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
     allocator().set_step_delay(cricket::kMinimumStepDelay);
   }
 
+  // Create a BasicPortAllocator without GTURN and add the TURN servers.
+  void ResetWithTurnServers(const rtc::SocketAddress& udp_turn,
+                            const rtc::SocketAddress& tcp_turn) {
+    allocator_.reset(new cricket::BasicPortAllocator(&network_manager_));
+    allocator().set_step_delay(cricket::kMinimumStepDelay);
+    AddTurnServers(udp_turn, tcp_turn);
+  }
+
   void AddTurnServers(const rtc::SocketAddress& udp_turn,
                       const rtc::SocketAddress& tcp_turn) {
     cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
@@ -565,17 +575,32 @@ TEST_F(PortAllocatorTest, TestGetAllPortsRestarts) {
 }
 
 // Test ICE candidate filter mechanism with options Relay/Host/Reflexive.
+// This test also verifies that when the allocator is only allowed to use
+// relay (i.e. IceTransportsType is relay), the raddr is an empty
+// address with the correct family. This is to prevent any local
+// reflective address leakage in the sdp line.
 TEST_F(PortAllocatorTest, TestCandidateFilterWithRelayOnly) {
   AddInterface(kClientAddr);
+  // GTURN is not configured here.
+  ResetWithTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
   allocator().set_candidate_filter(cricket::CF_RELAY);
   EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
   session_->StartGettingPorts();
   EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
-  // Using GTURN, we will have 4 candidates.
+  EXPECT_PRED5(CheckCandidate,
-  EXPECT_EQ(4U, candidates_.size());
+               candidates_[0],
+               cricket::ICE_CANDIDATE_COMPONENT_RTP,
+               "relay",
+               "udp",
+               rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+
+  EXPECT_EQ(1U, candidates_.size());
   EXPECT_EQ(1U, ports_.size());  // Only Relay port will be in ready state.
   for (size_t i = 0; i < candidates_.size(); ++i) {
     EXPECT_EQ(std::string(cricket::RELAY_PORT_TYPE), candidates_[i].type());
+    EXPECT_EQ(
+        candidates_[0].related_address(),
+        rtc::EmptySocketAddressWithFamily(candidates_[0].address().family()));
   }
 }
 
@@ -611,6 +636,9 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnly) {
   EXPECT_EQ(1U, ports_.size());  // Only UDP port will be in ready state.
   for (size_t i = 0; i < candidates_.size(); ++i) {
     EXPECT_EQ(std::string(cricket::STUN_PORT_TYPE), candidates_[i].type());
+    EXPECT_EQ(
+        candidates_[0].related_address(),
+        rtc::EmptySocketAddressWithFamily(candidates_[0].address().family()));
   }
 }