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/talk@7484 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 0cf46eb..670e1de 100644
--- a/p2p/base/port.cc
+++ b/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 @@
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 @@
ice_protocol_(ICEPROTO_HYBRID),
ice_role_(ICEROLE_UNKNOWN),
tiebreaker_(0),
- shared_socket_(false) {
+ shared_socket_(false),
+ candidate_filter_(CF_ALL) {
ASSERT(factory_ != NULL);
Construct();
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 4893586..f0fcb2e 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -308,6 +308,10 @@
// 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 @@
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 @@
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;
};
diff --git a/p2p/base/stunport.cc b/p2p/base/stunport.cc
index a185a35..ce2d828 100644
--- a/p2p/base/stunport.cc
+++ b/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 @@
// 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();
diff --git a/p2p/base/turnport.cc b/p2p/base/turnport.cc
index 2908d71..64bfed4 100644
--- a/p2p/base/turnport.cc
+++ b/p2p/base/turnport.cc
@@ -588,10 +588,18 @@
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.
- address, // Base address.
- stun_address, // Related address.
+ AddAddress(address, // Candidate address.
+ address, // Base address.
+ related_address, // Related address.
UDP_PROTOCOL_NAME,
"", // TCP canddiate type, empty for turn candidates.
RELAY_PORT_TYPE,
diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc
index 0ec13e7..0b35bdd 100644
--- a/p2p/client/basicportallocator.cc
+++ b/p2p/client/basicportallocator.cc
@@ -497,6 +497,9 @@
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);
diff --git a/p2p/client/portallocator_unittest.cc b/p2p/client/portallocator_unittest.cc
index 84b8d03..3357fe0 100644
--- a/p2p/client/portallocator_unittest.cc
+++ b/p2p/client/portallocator_unittest.cc
@@ -113,6 +113,8 @@
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 @@
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 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_EQ(4U, candidates_.size());
+ EXPECT_PRED5(CheckCandidate,
+ 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 @@
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()));
}
}
