Make Port (and subclasses) fully "Network"-based, instead of IP-based.
For ICE, we want sockets that are bound to specific network interfaces,
rather than to specific IP addresses. So, a while ago, we added a
"Network" class that gets passed into the Port constructor, in
addition to the IP address as before.
But we never finished the job of removing the IP address field, such that
a Port only guarantees something about the network interface it's
associated with, and not the specific IP address it ends up with.
This CL does that, and as a consequence, if a port ends up bound to
an IP address other than the "best" one (returned by Network::GetBestIP),
this *won't* be treated as an error.
This is relevant to Android, where even though we pass an IP address
into "Bind" as a way of identifying the network, the socket actually
gets bound using "android_setsocknetwork", which doesn't provide any
guarantees about the IP address. So, if a network interface has multiple
IPv6 addresses (for instance), we may not correctly predict the one
the OS will choose, and that's ok.
This CL also moves "SetAlternateLocalAddress" from VirtualSocket to
VirtualSocketServer, which makes for much more readable test code.
The next step, if there is one, is to pass along the Network class all
the way to SocketServer::Bind. Then the socket server could do smart
things with the network information. We could even stick a platform-
specific network handle in the Network object, such that the socket
server could use it for the binding, or for "sendmsg", for example.
See bug 7026 for more context about the sendmsg idea.
BUG=webrtc:7715
Review-Url: https://codereview.webrtc.org/2989303002
Cr-Commit-Position: refs/heads/master@{#19251}
diff --git a/webrtc/p2p/base/fakeportallocator.h b/webrtc/p2p/base/fakeportallocator.h
index 7bf7f96..d7c30f0 100644
--- a/webrtc/p2p/base/fakeportallocator.h
+++ b/webrtc/p2p/base/fakeportallocator.h
@@ -32,16 +32,15 @@
static TestUDPPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
const std::string& password,
const std::string& origin,
bool emit_localhost_for_anyaddress) {
- TestUDPPort* port = new TestUDPPort(thread, factory, network, ip, min_port,
- max_port, username, password, origin,
- emit_localhost_for_anyaddress);
+ TestUDPPort* port =
+ new TestUDPPort(thread, factory, network, min_port, max_port, username,
+ password, origin, emit_localhost_for_anyaddress);
if (!port->Init()) {
delete port;
port = nullptr;
@@ -62,7 +61,6 @@
TestUDPPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -72,7 +70,6 @@
: UDPPort(thread,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -128,9 +125,9 @@
(rtc::HasIPv6Enabled() && (flags() & PORTALLOCATOR_ENABLE_IPV6))
? ipv6_network_
: ipv4_network_;
- port_.reset(TestUDPPort::Create(network_thread_, factory_, &network,
- network.GetBestIP(), 0, 0, username(),
- password(), std::string(), false));
+ port_.reset(TestUDPPort::Create(network_thread_, factory_, &network, 0, 0,
+ username(), password(), std::string(),
+ false));
port_->SignalDestroyed.connect(
this, &FakePortAllocatorSession::OnPortDestroyed);
AddPort(port_.get());
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index a101633..40b2502 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -145,7 +145,6 @@
const std::string& type,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
const std::string& username_fragment,
const std::string& password)
: thread_(thread),
@@ -153,7 +152,6 @@
type_(type),
send_retransmit_count_attribute_(false),
network_(network),
- ip_(ip),
min_port_(0),
max_port_(0),
component_(ICE_CANDIDATE_COMPONENT_DEFAULT),
@@ -172,7 +170,6 @@
const std::string& type,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username_fragment,
@@ -182,7 +179,6 @@
type_(type),
send_retransmit_count_attribute_(false),
network_(network),
- ip_(ip),
min_port_(min_port),
max_port_(max_port),
component_(ICE_CANDIDATE_COMPONENT_DEFAULT),
@@ -471,14 +467,15 @@
}
bool Port::IsCompatibleAddress(const rtc::SocketAddress& addr) {
- int family = ip().family();
+ // Get a representative IP for the Network this port is configured to use.
+ rtc::IPAddress ip = network_->GetBestIP();
// We use single-stack sockets, so families must match.
- if (addr.family() != family) {
+ if (addr.family() != ip.family()) {
return false;
}
// Link-local IPv6 ports can only connect to other link-local IPv6 ports.
- if (family == AF_INET6 &&
- (IPIsLinkLocal(ip()) != IPIsLinkLocal(addr.ipaddr()))) {
+ if (ip.family() == AF_INET6 &&
+ (IPIsLinkLocal(ip) != IPIsLinkLocal(addr.ipaddr()))) {
return false;
}
return true;
diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index 5764738..19391b5 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -142,14 +142,21 @@
const std::string& type,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
const std::string& username_fragment,
const std::string& password);
+ // TODO(deadbeef): Delete this constructor once clients are moved off of it.
Port(rtc::Thread* thread,
const std::string& type,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
const rtc::IPAddress& ip,
+ const std::string& username_fragment,
+ const std::string& password)
+ : Port(thread, type, factory, network, username_fragment, password) {}
+ Port(rtc::Thread* thread,
+ const std::string& type,
+ rtc::PacketSocketFactory* factory,
+ rtc::Network* network,
uint16_t min_port,
uint16_t max_port,
const std::string& username_fragment,
@@ -283,7 +290,6 @@
// Debugging description of this port
virtual std::string ToString() const;
- const rtc::IPAddress& ip() const { return ip_; }
uint16_t min_port() { return min_port_; }
uint16_t max_port() { return max_port_; }
@@ -397,7 +403,6 @@
std::string type_;
bool send_retransmit_count_attribute_;
rtc::Network* network_;
- rtc::IPAddress ip_;
uint16_t min_port_;
uint16_t max_port_;
std::string content_name_;
diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc
index eb54760..757c1b3 100644
--- a/webrtc/p2p/base/port_unittest.cc
+++ b/webrtc/p2p/base/port_unittest.cc
@@ -8,6 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
+#include <list>
#include <memory>
#include "webrtc/p2p/base/basicpacketsocketfactory.h"
@@ -109,7 +110,6 @@
const std::string& type,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username_fragment,
@@ -118,7 +118,6 @@
type,
factory,
network,
- ip,
min_port,
max_port,
username_fragment,
@@ -144,7 +143,9 @@
}
virtual void PrepareAddress() {
- rtc::SocketAddress addr(ip(), min_port());
+ // Act as if the socket was bound to the best IP on the network, to the
+ // first port in the allowed range.
+ rtc::SocketAddress addr(Network()->GetBestIP(), min_port());
AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", "", Type(),
ICE_TYPE_PREFERENCE_HOST, 0, "", true);
}
@@ -382,7 +383,6 @@
PortTest()
: ss_(new rtc::VirtualSocketServer()),
main_(ss_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
socket_factory_(rtc::Thread::Current()),
nat_factory1_(ss_.get(), kNatAddr1, SocketAddress()),
nat_factory2_(ss_.get(), kNatAddr2, SocketAddress()),
@@ -401,7 +401,6 @@
password_(rtc::CreateRandomString(ICE_PWD_LENGTH)),
role_conflict_(false),
ports_destroyed_(0) {
- network_.AddIP(rtc::IPAddress(INADDR_ANY));
}
protected:
@@ -483,32 +482,36 @@
TestConnectivity("ssltcp", port1, RelayName(rtype, proto), port2,
rtype == RELAY_GTURN, false, true, true);
}
+
+ rtc::Network* MakeNetwork(const SocketAddress& addr) {
+ networks_.emplace_back("unittest", "unittest", addr.ipaddr(), 32);
+ networks_.back().AddIP(addr.ipaddr());
+ return &networks_.back();
+ }
+
// helpers for above functions
UDPPort* CreateUdpPort(const SocketAddress& addr) {
return CreateUdpPort(addr, &socket_factory_);
}
UDPPort* CreateUdpPort(const SocketAddress& addr,
PacketSocketFactory* socket_factory) {
- return UDPPort::Create(&main_, socket_factory, &network_, addr.ipaddr(), 0,
- 0, username_, password_, std::string(), true);
+ return UDPPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0,
+ username_, password_, std::string(), true);
}
TCPPort* CreateTcpPort(const SocketAddress& addr) {
return CreateTcpPort(addr, &socket_factory_);
}
TCPPort* CreateTcpPort(const SocketAddress& addr,
PacketSocketFactory* socket_factory) {
- return TCPPort::Create(&main_, socket_factory, &network_,
- addr.ipaddr(), 0, 0, username_, password_,
- true);
+ return TCPPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0,
+ username_, password_, true);
}
StunPort* CreateStunPort(const SocketAddress& addr,
rtc::PacketSocketFactory* factory) {
ServerAddresses stun_servers;
stun_servers.insert(kStunAddr);
- return StunPort::Create(&main_, factory, &network_,
- addr.ipaddr(), 0, 0,
- username_, password_, stun_servers,
- std::string());
+ return StunPort::Create(&main_, factory, MakeNetwork(addr), 0, 0, username_,
+ password_, stun_servers, std::string());
}
Port* CreateRelayPort(const SocketAddress& addr, RelayType rtype,
ProtocolType int_proto, ProtocolType ext_proto) {
@@ -530,8 +533,8 @@
PacketSocketFactory* socket_factory,
ProtocolType int_proto, ProtocolType ext_proto,
const rtc::SocketAddress& server_addr) {
- return TurnPort::Create(&main_, socket_factory, &network_, addr.ipaddr(), 0,
- 0, username_, password_,
+ return TurnPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0,
+ username_, password_,
ProtocolAddress(server_addr, int_proto),
kRelayCredentials, 0, std::string());
}
@@ -547,8 +550,8 @@
// TODO(pthatcher): Remove GTURN.
// Generate a username with length of 16 for Gturn only.
std::string username = rtc::CreateRandomString(kGturnUserNameLength);
- return RelayPort::Create(&main_, &socket_factory_, &network_, addr.ipaddr(),
- 0, 0, username, password_);
+ return RelayPort::Create(&main_, &socket_factory_, MakeNetwork(addr), 0, 0,
+ username, password_);
// TODO: Add an external address for ext_proto, so that the
// other side can connect to this port using a non-UDP protocol.
}
@@ -600,10 +603,6 @@
}
}
- void SetNetworkType(rtc::AdapterType adapter_type) {
- network_.set_type(adapter_type);
- }
-
void TestCrossFamilyPorts(int type);
void ExpectPortsCanConnect(bool can_connect, Port* p1, Port* p2);
@@ -764,8 +763,8 @@
TestPort* CreateTestPort(const rtc::SocketAddress& addr,
const std::string& username,
const std::string& password) {
- TestPort* port = new TestPort(&main_, "test", &socket_factory_, &network_,
- addr.ipaddr(), 0, 0, username, password);
+ TestPort* port = new TestPort(&main_, "test", &socket_factory_,
+ MakeNetwork(addr), 0, 0, username, password);
port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict);
return port;
}
@@ -779,6 +778,15 @@
port->SetIceTiebreaker(tiebreaker);
return port;
}
+ // Overload to create a test port given an rtc::Network directly.
+ TestPort* CreateTestPort(rtc::Network* network,
+ const std::string& username,
+ const std::string& password) {
+ TestPort* port = new TestPort(&main_, "test", &socket_factory_, network, 0,
+ 0, username, password);
+ port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict);
+ return port;
+ }
void OnRoleConflict(PortInterface* port) {
role_conflict_ = true;
@@ -799,9 +807,12 @@
rtc::VirtualSocketServer* vss() { return ss_.get(); }
private:
+ // When a "create port" helper method is called with an IP, we create a
+ // Network with that IP and add it to this list. Using a list instead of a
+ // vector so that when it grows, pointers aren't invalidated.
+ std::list<rtc::Network> networks_;
std::unique_ptr<rtc::VirtualSocketServer> ss_;
rtc::AutoSocketServerThread main_;
- rtc::Network network_;
rtc::BasicPacketSocketFactory socket_factory_;
std::unique_ptr<rtc::NATServer> nat_server1_;
std::unique_ptr<rtc::NATServer> nat_server2_;
@@ -1931,10 +1942,11 @@
// change, the network cost of the local candidates will change. Also tests that
// the remote network costs are updated with the stun binding requests.
TEST_F(PortTest, TestNetworkCostChange) {
+ rtc::Network* test_network = MakeNetwork(kLocalAddr1);
std::unique_ptr<TestPort> lport(
- CreateTestPort(kLocalAddr1, "lfrag", "lpass"));
+ CreateTestPort(test_network, "lfrag", "lpass"));
std::unique_ptr<TestPort> rport(
- CreateTestPort(kLocalAddr2, "rfrag", "rpass"));
+ CreateTestPort(test_network, "rfrag", "rpass"));
lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
lport->SetIceTiebreaker(kTiebreaker1);
rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
@@ -1950,7 +1962,7 @@
}
// Change the network type to wifi.
- SetNetworkType(rtc::ADAPTER_TYPE_WIFI);
+ test_network->set_type(rtc::ADAPTER_TYPE_WIFI);
EXPECT_EQ(rtc::kNetworkCostLow, lport->network_cost());
for (const cricket::Candidate& candidate : lport->Candidates()) {
EXPECT_EQ(rtc::kNetworkCostLow, candidate.network_cost());
@@ -1960,16 +1972,16 @@
Connection* lconn =
lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
// Change the network type to cellular.
- SetNetworkType(rtc::ADAPTER_TYPE_CELLULAR);
+ test_network->set_type(rtc::ADAPTER_TYPE_CELLULAR);
EXPECT_EQ(rtc::kNetworkCostHigh, lport->network_cost());
for (const cricket::Candidate& candidate : lport->Candidates()) {
EXPECT_EQ(rtc::kNetworkCostHigh, candidate.network_cost());
}
- SetNetworkType(rtc::ADAPTER_TYPE_WIFI);
+ test_network->set_type(rtc::ADAPTER_TYPE_WIFI);
Connection* rconn =
rport->CreateConnection(lport->Candidates()[0], Port::ORIGIN_MESSAGE);
- SetNetworkType(rtc::ADAPTER_TYPE_CELLULAR);
+ test_network->set_type(rtc::ADAPTER_TYPE_CELLULAR);
lconn->Ping(0);
// The rconn's remote candidate cost is rtc::kNetworkCostLow, but the ping
// contains an attribute of network cost of rtc::kNetworkCostHigh. Once the
@@ -1988,10 +2000,11 @@
}
TEST_F(PortTest, TestNetworkInfoAttribute) {
+ rtc::Network* test_network = MakeNetwork(kLocalAddr1);
std::unique_ptr<TestPort> lport(
- CreateTestPort(kLocalAddr1, "lfrag", "lpass"));
+ CreateTestPort(test_network, "lfrag", "lpass"));
std::unique_ptr<TestPort> rport(
- CreateTestPort(kLocalAddr2, "rfrag", "rpass"));
+ CreateTestPort(test_network, "rfrag", "rpass"));
lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
lport->SetIceTiebreaker(kTiebreaker1);
rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
@@ -2017,7 +2030,7 @@
// Set the network type to be cellular so its cost will be kNetworkCostHigh.
// Send a fake ping from rport to lport.
- SetNetworkType(rtc::ADAPTER_TYPE_CELLULAR);
+ test_network->set_type(rtc::ADAPTER_TYPE_CELLULAR);
uint16_t rnetwork_id = 8;
rport->Network()->set_id(rnetwork_id);
Connection* rconn =
diff --git a/webrtc/p2p/base/relayport.cc b/webrtc/p2p/base/relayport.cc
index efb9670..387ec4b 100644
--- a/webrtc/p2p/base/relayport.cc
+++ b/webrtc/p2p/base/relayport.cc
@@ -182,7 +182,6 @@
RelayPort::RelayPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -191,7 +190,6 @@
RELAY_PORT_TYPE,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -489,14 +487,14 @@
if (ra->proto == PROTO_UDP) {
// UDP sockets are simple.
socket = port_->socket_factory()->CreateUdpSocket(
- rtc::SocketAddress(port_->ip(), 0),
- port_->min_port(), port_->max_port());
+ rtc::SocketAddress(port_->Network()->GetBestIP(), 0), port_->min_port(),
+ port_->max_port());
} else if (ra->proto == PROTO_TCP || ra->proto == PROTO_SSLTCP) {
int opts = (ra->proto == PROTO_SSLTCP)
? rtc::PacketSocketFactory::OPT_TLS_FAKE
: 0;
socket = port_->socket_factory()->CreateClientTcpSocket(
- rtc::SocketAddress(port_->ip(), 0), ra->address,
+ rtc::SocketAddress(port_->Network()->GetBestIP(), 0), ra->address,
port_->proxy(), port_->user_agent(), opts);
} else {
LOG(LS_WARNING) << "Unknown protocol (" << ra->proto << ")";
diff --git a/webrtc/p2p/base/relayport.h b/webrtc/p2p/base/relayport.h
index 8fa2235..e2e6800 100644
--- a/webrtc/p2p/base/relayport.h
+++ b/webrtc/p2p/base/relayport.h
@@ -38,13 +38,12 @@
static RelayPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
const std::string& password) {
- return new RelayPort(thread, factory, network, ip, min_port, max_port,
- username, password);
+ return new RelayPort(thread, factory, network, min_port, max_port, username,
+ password);
}
~RelayPort() override;
@@ -82,7 +81,6 @@
RelayPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network*,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
diff --git a/webrtc/p2p/base/relayport_unittest.cc b/webrtc/p2p/base/relayport_unittest.cc
index e1bed76..a5d919a 100644
--- a/webrtc/p2p/base/relayport_unittest.cc
+++ b/webrtc/p2p/base/relayport_unittest.cc
@@ -46,19 +46,20 @@
RelayPortTest()
: virtual_socket_server_(new rtc::VirtualSocketServer()),
main_(virtual_socket_server_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
+ network_("unittest", "unittest", kLocalAddress.ipaddr(), 32),
socket_factory_(rtc::Thread::Current()),
username_(rtc::CreateRandomString(16)),
password_(rtc::CreateRandomString(16)),
relay_port_(cricket::RelayPort::Create(&main_,
&socket_factory_,
&network_,
- kLocalAddress.ipaddr(),
0,
0,
username_,
password_)),
- relay_server_(new cricket::RelayServer(&main_)) {}
+ relay_server_(new cricket::RelayServer(&main_)) {
+ network_.AddIP(kLocalAddress.ipaddr());
+ }
void OnReadPacket(rtc::AsyncPacketSocket* socket,
const char* data, size_t size,
diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc
index fa7a63c..5f6889e 100644
--- a/webrtc/p2p/base/stunport.cc
+++ b/webrtc/p2p/base/stunport.cc
@@ -170,7 +170,6 @@
LOCAL_PORT_TYPE,
factory,
network,
- socket->GetLocalAddress().ipaddr(),
username,
password),
requests_(thread),
@@ -185,7 +184,6 @@
UDPPort::UDPPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -196,7 +194,6 @@
LOCAL_PORT_TYPE,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -215,7 +212,7 @@
if (!SharedSocket()) {
RTC_DCHECK(socket_ == NULL);
socket_ = socket_factory()->CreateUdpSocket(
- rtc::SocketAddress(ip(), 0), min_port(), max_port());
+ rtc::SocketAddress(Network()->GetBestIP(), 0), min_port(), max_port());
if (!socket_) {
LOG_J(LS_WARNING, this) << "UDP socket creation failed";
return false;
@@ -379,8 +376,8 @@
RTC_DCHECK(resolver_.get() != NULL);
rtc::SocketAddress resolved;
- if (error != 0 ||
- !resolver_->GetResolvedAddress(input, ip().family(), &resolved)) {
+ if (error != 0 || !resolver_->GetResolvedAddress(
+ input, Network()->GetBestIP().family(), &resolved)) {
LOG_J(LS_WARNING, this) << "StunPort: stun host lookup received error "
<< error;
OnStunBindingOrResolveRequestFailed(input);
diff --git a/webrtc/p2p/base/stunport.h b/webrtc/p2p/base/stunport.h
index de5127c..8fbc0d5 100644
--- a/webrtc/p2p/base/stunport.h
+++ b/webrtc/p2p/base/stunport.h
@@ -54,7 +54,6 @@
static UDPPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -62,7 +61,7 @@
const std::string& origin,
bool emit_local_for_anyaddress) {
UDPPort* port =
- new UDPPort(thread, factory, network, ip, min_port, max_port, username,
+ new UDPPort(thread, factory, network, min_port, max_port, username,
password, origin, emit_local_for_anyaddress);
if (!port->Init()) {
delete port;
@@ -127,7 +126,6 @@
UDPPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -259,17 +257,14 @@
static StunPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
const std::string& password,
const ServerAddresses& servers,
const std::string& origin) {
- StunPort* port = new StunPort(thread, factory, network,
- ip, min_port, max_port,
- username, password, servers,
- origin);
+ StunPort* port = new StunPort(thread, factory, network, min_port, max_port,
+ username, password, servers, origin);
if (!port->Init()) {
delete port;
port = NULL;
@@ -287,7 +282,6 @@
StunPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -297,7 +291,6 @@
: UDPPort(thread,
factory,
network,
- ip,
min_port,
max_port,
username,
diff --git a/webrtc/p2p/base/stunport_unittest.cc b/webrtc/p2p/base/stunport_unittest.cc
index 25f5a74..e994f67 100644
--- a/webrtc/p2p/base/stunport_unittest.cc
+++ b/webrtc/p2p/base/stunport_unittest.cc
@@ -43,7 +43,7 @@
StunPortTestBase()
: ss_(new rtc::VirtualSocketServer()),
thread_(ss_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
+ network_("unittest", "unittest", kLocalAddr.ipaddr(), 32),
socket_factory_(rtc::Thread::Current()),
stun_server_1_(cricket::TestStunServer::Create(rtc::Thread::Current(),
kStunAddr1)),
@@ -52,7 +52,9 @@
done_(false),
error_(false),
stun_keepalive_delay_(1),
- stun_keepalive_lifetime_(-1) {}
+ stun_keepalive_lifetime_(-1) {
+ network_.AddIP(kLocalAddr.ipaddr());
+ }
cricket::UDPPort* port() const { return stun_port_.get(); }
bool done() const { return done_; }
@@ -70,9 +72,9 @@
void CreateStunPort(const ServerAddresses& stun_servers) {
stun_port_.reset(cricket::StunPort::Create(
- rtc::Thread::Current(), &socket_factory_, &network_,
- kLocalAddr.ipaddr(), 0, 0, rtc::CreateRandomString(16),
- rtc::CreateRandomString(22), stun_servers, std::string()));
+ rtc::Thread::Current(), &socket_factory_, &network_, 0, 0,
+ rtc::CreateRandomString(16), rtc::CreateRandomString(22), stun_servers,
+ std::string()));
stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_);
// If |stun_keepalive_lifetime_| is negative, let the stun port
// choose its lifetime from the network type.
diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc
index 86670cd..cbb25b9 100644
--- a/webrtc/p2p/base/tcpport.cc
+++ b/webrtc/p2p/base/tcpport.cc
@@ -75,7 +75,6 @@
TCPPort::TCPPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -85,7 +84,6 @@
LOCAL_PORT_TYPE,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -179,11 +177,15 @@
// Note: We still add the address, since otherwise the remote side won't
// recognize our incoming TCP connections. According to
// https://tools.ietf.org/html/rfc6544#section-4.5, for active candidate,
- // the port must be set to the discard port, i.e. 9.
- AddAddress(rtc::SocketAddress(ip(), DISCARD_PORT),
- rtc::SocketAddress(ip(), 0), rtc::SocketAddress(),
- TCP_PROTOCOL_NAME, "", TCPTYPE_ACTIVE_STR, LOCAL_PORT_TYPE,
- ICE_TYPE_PREFERENCE_HOST_TCP, 0, "", true);
+ // the port must be set to the discard port, i.e. 9. We can't be 100% sure
+ // which IP address will actually be used, so GetBestIP is as good as we
+ // can do.
+ // TODO(deadbeef): We could do something like create a dummy socket just to
+ // see what IP we get. But that may be overkill.
+ AddAddress(rtc::SocketAddress(Network()->GetBestIP(), DISCARD_PORT),
+ rtc::SocketAddress(Network()->GetBestIP(), 0),
+ rtc::SocketAddress(), TCP_PROTOCOL_NAME, "", TCPTYPE_ACTIVE_STR,
+ LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST_TCP, 0, "", true);
}
}
@@ -262,7 +264,8 @@
void TCPPort::TryCreateServerSocket() {
socket_ = socket_factory()->CreateServerTcpSocket(
- rtc::SocketAddress(ip(), 0), min_port(), max_port(), false /* ssl */);
+ rtc::SocketAddress(Network()->GetBestIP(), 0), min_port(), max_port(),
+ false /* ssl */);
if (!socket_) {
LOG_J(LS_WARNING, this)
<< "TCP server socket creation failed; continuing anyway.";
@@ -323,11 +326,16 @@
if (outgoing_) {
CreateOutgoingTcpSocket();
} else {
- // Incoming connections should match the network address.
+ // Incoming connections should match one of the network addresses. Same as
+ // what's being checked in OnConnect, but just DCHECKing here.
LOG_J(LS_VERBOSE, this)
<< "socket ipaddr: " << socket_->GetLocalAddress().ToString()
- << ",port() ip:" << port->ip().ToString();
- RTC_DCHECK(socket_->GetLocalAddress().ipaddr() == port->ip());
+ << ", port() Network:" << port->Network()->ToString();
+ const std::vector<rtc::InterfaceAddress>& desired_addresses =
+ port_->Network()->GetIPs();
+ RTC_DCHECK(std::find(desired_addresses.begin(), desired_addresses.end(),
+ socket_->GetLocalAddress().ipaddr()) !=
+ desired_addresses.end());
ConnectSocketSignals(socket);
}
}
@@ -390,34 +398,49 @@
void TCPConnection::OnConnect(rtc::AsyncPacketSocket* socket) {
RTC_DCHECK(socket == socket_.get());
- // Do not use this connection if the socket bound to a different address than
- // the one we asked for. This is seen in Chrome, where TCP sockets cannot be
- // given a binding address, and the platform is expected to pick the
- // correct local address.
- const rtc::SocketAddress& socket_addr = socket->GetLocalAddress();
- if (socket_addr.ipaddr() == port()->ip()) {
+ // Do not use this port if the socket bound to an address not associated with
+ // the desired network interface. This is seen in Chrome, where TCP sockets
+ // cannot be given a binding address, and the platform is expected to pick
+ // the correct local address.
+ //
+ // However, there are two situations in which we allow the bound address to
+ // not be one of the addresses of the requested interface:
+ // 1. The bound address is the loopback address. This happens when a proxy
+ // forces TCP to bind to only the localhost address (see issue 3927).
+ // 2. The bound address is the "any address". This happens when
+ // multiple_routes is disabled (see issue 4780).
+ //
+ // Note that, aside from minor differences in log statements, this logic is
+ // identical to that in TurnPort.
+ const rtc::SocketAddress& socket_address = socket->GetLocalAddress();
+ const std::vector<rtc::InterfaceAddress>& desired_addresses =
+ port_->Network()->GetIPs();
+ if (std::find(desired_addresses.begin(), desired_addresses.end(),
+ socket_address.ipaddr()) != desired_addresses.end()) {
LOG_J(LS_VERBOSE, this) << "Connection established to "
<< socket->GetRemoteAddress().ToSensitiveString();
- } else if (IPIsAny(port()->ip())) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket_addr.ipaddr().ToString()
- << ", rather then the local port:"
- << port()->ip().ToString()
- << ". Still allowing it since it's any address"
- << ", possibly caused by multi-routes being disabled.";
- } else if (socket_addr.IsLoopbackIP()) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket_addr.ipaddr().ToString()
- << ", rather then the local port:"
- << port()->ip().ToString()
- << ". Still allowing it since it's localhost.";
} else {
- LOG_J(LS_WARNING, this) << "Dropping connection as TCP socket bound to IP "
- << socket_addr.ipaddr().ToSensitiveString()
- << ", different from the local candidate IP "
- << port()->ip().ToSensitiveString();
- OnClose(socket, 0);
- return;
+ if (socket->GetLocalAddress().IsLoopbackIP()) {
+ LOG(LS_WARNING) << "Socket is bound to the address:"
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << port_->Network()->ToString()
+ << ". Still allowing it since it's localhost.";
+ } else if (IPIsAny(port_->Network()->GetBestIP())) {
+ LOG(LS_WARNING) << "Socket is bound to the address:"
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << port_->Network()->ToString()
+ << ". Still allowing it since it's the 'any' address"
+ << ", possibly caused by multiple_routes being disabled.";
+ } else {
+ LOG(LS_WARNING) << "Dropping connection as TCP socket bound to IP "
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << port_->Network()->ToString();
+ OnClose(socket, 0);
+ return;
+ }
}
// Connection is established successfully.
@@ -501,8 +524,9 @@
? rtc::PacketSocketFactory::OPT_TLS_FAKE
: 0;
socket_.reset(port()->socket_factory()->CreateClientTcpSocket(
- rtc::SocketAddress(port()->ip(), 0), remote_candidate().address(),
- port()->proxy(), port()->user_agent(), opts));
+ rtc::SocketAddress(port()->Network()->GetBestIP(), 0),
+ remote_candidate().address(), port()->proxy(), port()->user_agent(),
+ opts));
if (socket_) {
LOG_J(LS_VERBOSE, this)
<< "Connecting from " << socket_->GetLocalAddress().ToSensitiveString()
diff --git a/webrtc/p2p/base/tcpport.h b/webrtc/p2p/base/tcpport.h
index 0974fa7..a0a36df 100644
--- a/webrtc/p2p/base/tcpport.h
+++ b/webrtc/p2p/base/tcpport.h
@@ -33,14 +33,13 @@
static TCPPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
const std::string& password,
bool allow_listen) {
- return new TCPPort(thread, factory, network, ip, min_port, max_port,
- username, password, allow_listen);
+ return new TCPPort(thread, factory, network, min_port, max_port, username,
+ password, allow_listen);
}
~TCPPort() override;
@@ -62,7 +61,6 @@
TCPPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
diff --git a/webrtc/p2p/base/tcpport_unittest.cc b/webrtc/p2p/base/tcpport_unittest.cc
index 0f7bd1f..884488e 100644
--- a/webrtc/p2p/base/tcpport_unittest.cc
+++ b/webrtc/p2p/base/tcpport_unittest.cc
@@ -8,6 +8,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
+#include <list>
#include <memory>
#include "webrtc/p2p/base/basicpacketsocketfactory.h"
@@ -24,61 +25,134 @@
using cricket::ICE_PWD_LENGTH;
static int kTimeout = 1000;
-static const SocketAddress kLocalAddr("11.11.11.11", 1);
-static const SocketAddress kRemoteAddr("22.22.22.22", 2);
+static const SocketAddress kLocalAddr("11.11.11.11", 0);
+static const SocketAddress kAlternateLocalAddr("1.2.3.4", 0);
+static const SocketAddress kRemoteAddr("22.22.22.22", 0);
+
+class ConnectionObserver : public sigslot::has_slots<> {
+ public:
+ ConnectionObserver(Connection* conn) {
+ conn->SignalDestroyed.connect(this, &ConnectionObserver::OnDestroyed);
+ }
+
+ bool connection_destroyed() { return connection_destroyed_; }
+
+ private:
+ void OnDestroyed(Connection*) { connection_destroyed_ = true; }
+
+ bool connection_destroyed_ = false;
+};
class TCPPortTest : public testing::Test, public sigslot::has_slots<> {
public:
TCPPortTest()
: ss_(new rtc::VirtualSocketServer()),
main_(ss_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
socket_factory_(rtc::Thread::Current()),
username_(rtc::CreateRandomString(ICE_UFRAG_LENGTH)),
password_(rtc::CreateRandomString(ICE_PWD_LENGTH)) {
- network_.AddIP(rtc::IPAddress(INADDR_ANY));
}
- void ConnectSignalSocketCreated() {
- ss_->SignalSocketCreated.connect(this, &TCPPortTest::OnSocketCreated);
+ rtc::Network* MakeNetwork(const SocketAddress& addr) {
+ networks_.emplace_back("unittest", "unittest", addr.ipaddr(), 32);
+ networks_.back().AddIP(addr.ipaddr());
+ return &networks_.back();
}
- void OnSocketCreated(rtc::VirtualSocket* socket) {
- LOG(LS_INFO) << "socket created ";
- socket->SignalAddressReady.connect(
- this, &TCPPortTest::SetLocalhostAsAlternativeLocalAddress);
+ std::unique_ptr<TCPPort> CreateTCPPort(const SocketAddress& addr) {
+ return std::unique_ptr<TCPPort>(
+ TCPPort::Create(&main_, &socket_factory_, MakeNetwork(addr), 0, 0,
+ username_, password_, true));
}
- void SetLocalhostAsAlternativeLocalAddress(rtc::VirtualSocket* socket,
- const SocketAddress& address) {
- SocketAddress local_address("127.0.0.1", 2000);
- socket->SetAlternativeLocalAddress(local_address);
- }
-
- TCPPort* CreateTCPPort(const SocketAddress& addr) {
- return TCPPort::Create(&main_, &socket_factory_, &network_, addr.ipaddr(),
- 0, 0, username_, password_, true);
+ std::unique_ptr<TCPPort> CreateTCPPort(rtc::Network* network) {
+ return std::unique_ptr<TCPPort>(TCPPort::Create(
+ &main_, &socket_factory_, network, 0, 0, username_, password_, true));
}
protected:
+ // When a "create port" helper method is called with an IP, we create a
+ // Network with that IP and add it to this list. Using a list instead of a
+ // vector so that when it grows, pointers aren't invalidated.
+ std::list<rtc::Network> networks_;
std::unique_ptr<rtc::VirtualSocketServer> ss_;
rtc::AutoSocketServerThread main_;
- rtc::Network network_;
rtc::BasicPacketSocketFactory socket_factory_;
std::string username_;
std::string password_;
};
TEST_F(TCPPortTest, TestTCPPortWithLocalhostAddress) {
- std::unique_ptr<TCPPort> lport(CreateTCPPort(kLocalAddr));
- std::unique_ptr<TCPPort> rport(CreateTCPPort(kRemoteAddr));
- lport->PrepareAddress();
- rport->PrepareAddress();
- // Start to listen to new socket creation event.
- ConnectSignalSocketCreated();
- Connection* conn =
- lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
+ SocketAddress local_address("127.0.0.1", 0);
+ // After calling this, when TCPPort attempts to get a socket bound to
+ // kLocalAddr, it will end up using localhost instead.
+ ss_->SetAlternativeLocalAddress(kLocalAddr.ipaddr(), local_address.ipaddr());
+ auto local_port = CreateTCPPort(kLocalAddr);
+ auto remote_port = CreateTCPPort(kRemoteAddr);
+ local_port->PrepareAddress();
+ remote_port->PrepareAddress();
+ Connection* conn = local_port->CreateConnection(remote_port->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
EXPECT_TRUE_WAIT(conn->connected(), kTimeout);
+ // Verify that the socket actually used localhost, otherwise this test isn't
+ // doing what it meant to.
+ ASSERT_EQ(local_address.ipaddr(),
+ local_port->Candidates()[0].address().ipaddr());
+}
+
+// If the address the socket ends up bound to does not match any address of the
+// TCPPort's Network, then the socket should be discarded and no candidates
+// should be signaled. In the context of ICE, where one TCPPort is created for
+// each Network, when this happens it's likely that the unexpected address is
+// associated with some other Network, which another TCPPort is already
+// covering.
+TEST_F(TCPPortTest, TCPPortDiscardedIfBoundAddressDoesNotMatchNetwork) {
+ // Sockets bound to kLocalAddr will actually end up with kAlternateLocalAddr.
+ ss_->SetAlternativeLocalAddress(kLocalAddr.ipaddr(),
+ kAlternateLocalAddr.ipaddr());
+
+ // Create ports (local_port is the one whose IP will end up reassigned).
+ auto local_port = CreateTCPPort(kLocalAddr);
+ auto remote_port = CreateTCPPort(kRemoteAddr);
+ local_port->PrepareAddress();
+ remote_port->PrepareAddress();
+
+ // Tell port to create a connection; it should be destroyed when it's
+ // realized that it's using an unexpected address.
+ Connection* conn = local_port->CreateConnection(remote_port->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ ConnectionObserver observer(conn);
+ EXPECT_TRUE_WAIT(observer.connection_destroyed(), kTimeout);
+}
+
+// A caveat for the above logic: if the socket ends up bound to one of the IPs
+// associated with the Network, just not the "best" one, this is ok.
+TEST_F(TCPPortTest, TCPPortNotDiscardedIfNotBoundToBestIP) {
+ // Sockets bound to kLocalAddr will actually end up with kAlternateLocalAddr.
+ ss_->SetAlternativeLocalAddress(kLocalAddr.ipaddr(),
+ kAlternateLocalAddr.ipaddr());
+
+ // Set up a network with kLocalAddr1 as the "best" IP, and kAlternateLocalAddr
+ // as an alternate.
+ rtc::Network* network = MakeNetwork(kLocalAddr);
+ network->AddIP(kAlternateLocalAddr.ipaddr());
+ ASSERT_EQ(kLocalAddr.ipaddr(), network->GetBestIP());
+
+ // Create ports (using our special 2-IP Network for local_port).
+ auto local_port = CreateTCPPort(network);
+ auto remote_port = CreateTCPPort(kRemoteAddr);
+ local_port->PrepareAddress();
+ remote_port->PrepareAddress();
+
+ // Expect connection to succeed.
+ Connection* conn = local_port->CreateConnection(remote_port->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ EXPECT_TRUE_WAIT(conn->connected(), kTimeout);
+
+ // Verify that the socket actually used the alternate address, otherwise this
+ // test isn't doing what it meant to.
+ ASSERT_EQ(kAlternateLocalAddr.ipaddr(),
+ local_port->Candidates()[0].address().ipaddr());
}
class SentPacketCounter : public sigslot::has_slots<> {
diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc
index 94f54dc..2828fea 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -10,6 +10,7 @@
#include "webrtc/p2p/base/turnport.h"
+#include <algorithm>
#include <functional>
#include "webrtc/p2p/base/common.h"
@@ -194,7 +195,6 @@
RELAY_PORT_TYPE,
factory,
network,
- socket->GetLocalAddress().ipaddr(),
username,
password),
server_address_(server_address),
@@ -214,7 +214,6 @@
TurnPort::TurnPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
@@ -227,7 +226,6 @@
RELAY_PORT_TYPE,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -293,7 +291,7 @@
if (!IsCompatibleAddress(server_address_.address)) {
LOG(LS_ERROR) << "IP address family does not match: "
<< "server: " << server_address_.address.family()
- << " local: " << ip().family();
+ << " local: " << Network()->GetBestIP().family();
OnAllocateError();
return;
}
@@ -322,7 +320,7 @@
if (server_address_.proto == PROTO_UDP && !SharedSocket()) {
socket_ = socket_factory()->CreateUdpSocket(
- rtc::SocketAddress(ip(), 0), min_port(), max_port());
+ rtc::SocketAddress(Network()->GetBestIP(), 0), min_port(), max_port());
} else if (server_address_.proto == PROTO_TCP ||
server_address_.proto == PROTO_TLS) {
RTC_DCHECK(!SharedSocket());
@@ -339,7 +337,7 @@
}
socket_ = socket_factory()->CreateClientTcpSocket(
- rtc::SocketAddress(ip(), 0), server_address_.address,
+ rtc::SocketAddress(Network()->GetBestIP(), 0), server_address_.address,
proxy(), user_agent(), opts);
}
@@ -381,33 +379,43 @@
RTC_DCHECK(server_address_.proto == PROTO_TCP ||
server_address_.proto == PROTO_TLS);
- // Do not use this port if the socket bound to a different address than
- // the one we asked for. This is seen in Chrome, where TCP sockets cannot be
- // given a binding address, and the platform is expected to pick the
- // correct local address.
-
+ // Do not use this port if the socket bound to an address not associated with
+ // the desired network interface. This is seen in Chrome, where TCP sockets
+ // cannot be given a binding address, and the platform is expected to pick
+ // the correct local address.
+ //
// However, there are two situations in which we allow the bound address to
- // differ from the requested address: 1. The bound address is the loopback
- // address. This happens when a proxy forces TCP to bind to only the
- // localhost address (see issue 3927). 2. The bound address is the "any
- // address". This happens when multiple_routes is disabled (see issue 4780).
- if (socket->GetLocalAddress().ipaddr() != ip()) {
+ // not be one of the addresses of the requested interface:
+ // 1. The bound address is the loopback address. This happens when a proxy
+ // forces TCP to bind to only the localhost address (see issue 3927).
+ // 2. The bound address is the "any address". This happens when
+ // multiple_routes is disabled (see issue 4780).
+ //
+ // Note that, aside from minor differences in log statements, this logic is
+ // identical to that in TcpPort.
+ const rtc::SocketAddress& socket_address = socket->GetLocalAddress();
+ const std::vector<rtc::InterfaceAddress>& desired_addresses =
+ Network()->GetIPs();
+ if (std::find(desired_addresses.begin(), desired_addresses.end(),
+ socket_address.ipaddr()) == desired_addresses.end()) {
if (socket->GetLocalAddress().IsLoopbackIP()) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket->GetLocalAddress().ipaddr().ToString()
- << ", rather then the local port:" << ip().ToString()
+ LOG(LS_WARNING) << "Socket is bound to the address:"
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << Network()->ToString()
<< ". Still allowing it since it's localhost.";
- } else if (IPIsAny(ip())) {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket->GetLocalAddress().ipaddr().ToString()
- << ", rather then the local port:" << ip().ToString()
- << ". Still allowing it since it's any address"
+ } else if (IPIsAny(Network()->GetBestIP())) {
+ LOG(LS_WARNING) << "Socket is bound to the address:"
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << Network()->ToString()
+ << ". Still allowing it since it's the 'any' address"
<< ", possibly caused by multiple_routes being disabled.";
} else {
- LOG(LS_WARNING) << "Socket is bound to a different address:"
- << socket->GetLocalAddress().ipaddr().ToString()
- << ", rather then the local port:" << ip().ToString()
- << ". Discarding TURN port.";
+ LOG(LS_WARNING) << "Socket is bound to the address:"
+ << socket_address.ipaddr().ToString()
+ << ", rather then an address associated with network:"
+ << Network()->ToString() << ". Discarding TURN port.";
OnAllocateError();
return;
}
@@ -701,7 +709,8 @@
// sockets we need hostname along with resolved address.
rtc::SocketAddress resolved_address = server_address_.address;
if (resolver_->GetError() != 0 ||
- !resolver_->GetResolvedAddress(ip().family(), &resolved_address)) {
+ !resolver_->GetResolvedAddress(Network()->GetBestIP().family(),
+ &resolved_address)) {
LOG_J(LS_WARNING, this) << "TURN host lookup received error "
<< resolver_->GetError();
error_ = resolver_->GetError();
diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h
index 4e4d4b1..abdaa3d 100644
--- a/webrtc/p2p/base/turnport.h
+++ b/webrtc/p2p/base/turnport.h
@@ -42,6 +42,7 @@
STATE_DISCONNECTED, // TCP connection died, cannot send/receive any
// packets.
};
+ // Create a TURN port using the shared UDP socket, |socket|.
static TurnPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
@@ -56,10 +57,11 @@
server_address, credentials, server_priority, origin);
}
+ // Create a TURN port that will use a new socket, bound to |network| and
+ // using a port in the range between |min_port| and |max_port|.
static TurnPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username, // ice username.
@@ -68,9 +70,9 @@
const RelayCredentials& credentials,
int server_priority,
const std::string& origin) {
- return new TurnPort(thread, factory, network, ip, min_port, max_port,
- username, password, server_address, credentials,
- server_priority, origin);
+ return new TurnPort(thread, factory, network, min_port, max_port, username,
+ password, server_address, credentials, server_priority,
+ origin);
}
virtual ~TurnPort();
@@ -177,7 +179,6 @@
TurnPort(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
- const rtc::IPAddress& ip,
uint16_t min_port,
uint16_t max_port,
const std::string& username,
diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc
index 73426cc..7e0e291 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -11,6 +11,7 @@
#include <dirent.h>
#endif
+#include <list>
#include <memory>
#include "webrtc/p2p/base/basicpacketsocketfactory.h"
@@ -143,7 +144,6 @@
TurnPortTest()
: ss_(new TurnPortTestVirtualSocketServer()),
main_(ss_.get()),
- network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
socket_factory_(rtc::Thread::Current()),
turn_server_(&main_, kTurnUdpIntAddr, kTurnUdpExtAddr),
turn_ready_(false),
@@ -156,7 +156,6 @@
// so far", so we need to start the fake clock at a nonzero time...
// TODO(deadbeef): Fix this.
fake_clock_.AdvanceTime(rtc::TimeDelta::FromSeconds(1));
- network_.AddIP(rtc::IPAddress(INADDR_ANY));
}
virtual void OnMessage(rtc::Message* msg) {
@@ -165,21 +164,6 @@
test_finish_ = true;
}
- void ConnectSignalAddressReadyToSetLocalhostAsAltenertativeLocalAddress() {
- rtc::AsyncPacketSocket* socket = turn_port_->socket();
- rtc::VirtualSocket* virtual_socket =
- ss_->LookupBinding(socket->GetLocalAddress());
- virtual_socket->SignalAddressReady.connect(
- this, &TurnPortTest::SetLocalhostAsAltenertativeLocalAddress);
- }
-
- void SetLocalhostAsAltenertativeLocalAddress(
- rtc::VirtualSocket* socket,
- const rtc::SocketAddress& address) {
- SocketAddress local_address("127.0.0.1", 2000);
- socket->SetAlternativeLocalAddress(local_address);
- }
-
void OnTurnPortComplete(Port* port) {
turn_ready_ = true;
}
@@ -229,24 +213,24 @@
return socket;
}
+ rtc::Network* MakeNetwork(const SocketAddress& addr) {
+ networks_.emplace_back("unittest", "unittest", addr.ipaddr(), 32);
+ networks_.back().AddIP(addr.ipaddr());
+ return &networks_.back();
+ }
+
void CreateTurnPort(const std::string& username,
const std::string& password,
const ProtocolAddress& server_address) {
- CreateTurnPort(kLocalAddr1, username, password, server_address);
+ CreateTurnPortWithAllParams(MakeNetwork(kLocalAddr1), username, password,
+ server_address, std::string());
}
void CreateTurnPort(const rtc::SocketAddress& local_address,
const std::string& username,
const std::string& password,
const ProtocolAddress& server_address) {
- RelayCredentials credentials(username, password);
- turn_port_.reset(TurnPort::Create(&main_, &socket_factory_, &network_,
- local_address.ipaddr(), 0, 0,
- kIceUfrag1, kIcePwd1,
- server_address, credentials, 0,
- std::string()));
- // This TURN port will be the controlling.
- turn_port_->SetIceRole(ICEROLE_CONTROLLING);
- ConnectSignals();
+ CreateTurnPortWithAllParams(MakeNetwork(local_address), username, password,
+ server_address, std::string());
}
// Should be identical to CreateTurnPort but specifies an origin value
@@ -256,12 +240,30 @@
const std::string& password,
const ProtocolAddress& server_address,
const std::string& origin) {
+ CreateTurnPortWithAllParams(MakeNetwork(local_address), username, password,
+ server_address, origin);
+ }
+
+ void CreateTurnPortWithNetwork(rtc::Network* network,
+ const std::string& username,
+ const std::string& password,
+ const ProtocolAddress& server_address) {
+ CreateTurnPortWithAllParams(network, username, password, server_address,
+ std::string());
+ }
+
+ // Version of CreateTurnPort that takes all possible parameters; all other
+ // helper methods call this, such that "SetIceRole" and "ConnectSignals" (and
+ // possibly other things in the future) only happen in one place.
+ void CreateTurnPortWithAllParams(rtc::Network* network,
+ const std::string& username,
+ const std::string& password,
+ const ProtocolAddress& server_address,
+ const std::string& origin) {
RelayCredentials credentials(username, password);
- turn_port_.reset(TurnPort::Create(&main_, &socket_factory_, &network_,
- local_address.ipaddr(), 0, 0,
- kIceUfrag1, kIcePwd1,
- server_address, credentials, 0,
- origin));
+ turn_port_.reset(TurnPort::Create(&main_, &socket_factory_, network, 0, 0,
+ kIceUfrag1, kIcePwd1, server_address,
+ credentials, 0, origin));
// This TURN port will be the controlling.
turn_port_->SetIceRole(ICEROLE_CONTROLLING);
ConnectSignals();
@@ -282,8 +284,8 @@
RelayCredentials credentials(username, password);
turn_port_.reset(TurnPort::Create(
- &main_, &socket_factory_, &network_, socket_.get(), kIceUfrag1,
- kIcePwd1, server_address, credentials, 0, std::string()));
+ &main_, &socket_factory_, MakeNetwork(kLocalAddr1), socket_.get(),
+ kIceUfrag1, kIcePwd1, server_address, credentials, 0, std::string()));
// This TURN port will be the controlling.
turn_port_->SetIceRole(ICEROLE_CONTROLLING);
ConnectSignals();
@@ -305,8 +307,8 @@
void CreateUdpPort() { CreateUdpPort(kLocalAddr2); }
void CreateUdpPort(const SocketAddress& address) {
- udp_port_.reset(UDPPort::Create(&main_, &socket_factory_, &network_,
- address.ipaddr(), 0, 0, kIceUfrag2,
+ udp_port_.reset(UDPPort::Create(&main_, &socket_factory_,
+ MakeNetwork(address), 0, 0, kIceUfrag2,
kIcePwd2, std::string(), false));
// UDP port will be controlled.
udp_port_->SetIceRole(ICEROLE_CONTROLLED);
@@ -616,9 +618,12 @@
protected:
rtc::ScopedFakeClock fake_clock_;
+ // When a "create port" helper method is called with an IP, we create a
+ // Network with that IP and add it to this list. Using a list instead of a
+ // vector so that when it grows, pointers aren't invalidated.
+ std::list<rtc::Network> networks_;
std::unique_ptr<TurnPortTestVirtualSocketServer> ss_;
rtc::AutoSocketServerThread main_;
- rtc::Network network_;
rtc::BasicPacketSocketFactory socket_factory_;
std::unique_ptr<rtc::AsyncPacketSocket> socket_;
TestTurnServer turn_server_;
@@ -704,16 +709,79 @@
// instead the address that TurnPort originally bound to. The candidate pair
// impacted by this behavior should still be used.
TEST_F(TurnPortTest, TestTurnTcpAllocationWhenProxyChangesAddressToLocalHost) {
+ SocketAddress local_address("127.0.0.1", 0);
+ // After calling this, when TurnPort attempts to get a socket bound to
+ // kLocalAddr, it will end up using localhost instead.
+ ss_->SetAlternativeLocalAddress(kLocalAddr1.ipaddr(), local_address.ipaddr());
+
turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
- CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
+ CreateTurnPort(kLocalAddr1, kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
EXPECT_EQ(0, turn_port_->SetOption(rtc::Socket::OPT_SNDBUF, 10 * 1024));
turn_port_->PrepareAddress();
- ConnectSignalAddressReadyToSetLocalhostAsAltenertativeLocalAddress();
EXPECT_TRUE_SIMULATED_WAIT(turn_ready_, kSimulatedRtt * 3, fake_clock_);
ASSERT_EQ(1U, turn_port_->Candidates().size());
EXPECT_EQ(kTurnUdpExtAddr.ipaddr(),
turn_port_->Candidates()[0].address().ipaddr());
EXPECT_NE(0, turn_port_->Candidates()[0].address().port());
+
+ // Verify that the socket actually used localhost, otherwise this test isn't
+ // doing what it meant to.
+ ASSERT_EQ(local_address.ipaddr(),
+ turn_port_->Candidates()[0].related_address().ipaddr());
+}
+
+// If the address the socket ends up bound to does not match any address of the
+// TurnPort's Network, then the socket should be discarded and no candidates
+// should be signaled. In the context of ICE, where one TurnPort is created for
+// each Network, when this happens it's likely that the unexpected address is
+// associated with some other Network, which another TurnPort is already
+// covering.
+TEST_F(TurnPortTest,
+ TurnTcpAllocationDiscardedIfBoundAddressDoesNotMatchNetwork) {
+ // Sockets bound to kLocalAddr1 will actually end up with kLocalAddr2.
+ ss_->SetAlternativeLocalAddress(kLocalAddr1.ipaddr(), kLocalAddr2.ipaddr());
+
+ // Set up TURN server to use TCP (this logic only exists for TCP).
+ turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
+
+ // Create TURN port and tell it to start allocation.
+ CreateTurnPort(kLocalAddr1, kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
+ turn_port_->PrepareAddress();
+
+ // Shouldn't take more than 1 RTT to realize the bound address isn't the one
+ // expected.
+ EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt, fake_clock_);
+}
+
+// A caveat for the above logic: if the socket ends up bound to one of the IPs
+// associated with the Network, just not the "best" one, this is ok.
+TEST_F(TurnPortTest, TurnTcpAllocationNotDiscardedIfNotBoundToBestIP) {
+ // Sockets bound to kLocalAddr1 will actually end up with kLocalAddr2.
+ ss_->SetAlternativeLocalAddress(kLocalAddr1.ipaddr(), kLocalAddr2.ipaddr());
+
+ // Set up a network with kLocalAddr1 as the "best" IP, and kLocalAddr2 as an
+ // alternate.
+ rtc::Network* network = MakeNetwork(kLocalAddr1);
+ network->AddIP(kLocalAddr2.ipaddr());
+ ASSERT_EQ(kLocalAddr1.ipaddr(), network->GetBestIP());
+
+ // Set up TURN server to use TCP (this logic only exists for TCP).
+ turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
+
+ // Create TURN port using our special Network, and tell it to start
+ // allocation.
+ CreateTurnPortWithNetwork(network, kTurnUsername, kTurnPassword,
+ kTurnTcpProtoAddr);
+ turn_port_->PrepareAddress();
+
+ // Candidate should be gathered as normally.
+ EXPECT_TRUE_SIMULATED_WAIT(turn_ready_, kSimulatedRtt * 3, fake_clock_);
+ ASSERT_EQ(1U, turn_port_->Candidates().size());
+
+ // Verify that the socket actually used the alternate address, otherwise this
+ // test isn't doing what it meant to.
+ ASSERT_EQ(kLocalAddr2.ipaddr(),
+ turn_port_->Candidates()[0].related_address().ipaddr());
}
// Testing turn port will attempt to create TCP socket on address resolution
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 091819a..ce95895 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -1096,7 +1096,6 @@
uint32_t flags)
: session_(session),
network_(network),
- ip_(network->GetBestIP()),
config_(config),
state_(kInit),
flags_(flags),
@@ -1108,8 +1107,8 @@
void AllocationSequence::Init() {
if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) {
udp_socket_.reset(session_->socket_factory()->CreateUdpSocket(
- rtc::SocketAddress(ip_, 0), session_->allocator()->min_port(),
- session_->allocator()->max_port()));
+ rtc::SocketAddress(network_->GetBestIP(), 0),
+ session_->allocator()->min_port(), session_->allocator()->max_port()));
if (udp_socket_) {
udp_socket_->SignalReadPacket.connect(
this, &AllocationSequence::OnReadPacket);
@@ -1143,7 +1142,7 @@
return;
}
- if (!((network == network_) && (ip_ == network->GetBestIP()))) {
+ if (!((network == network_) && (previous_best_ip_ == network->GetBestIP()))) {
// Different network setup; nothing is equivalent.
return;
}
@@ -1173,6 +1172,9 @@
void AllocationSequence::Start() {
state_ = kRunning;
session_->network_thread()->Post(RTC_FROM_HERE, this, MSG_ALLOCATION_PHASE);
+ // Take a snapshot of the best IP, so that when DisableEquivalentPhases is
+ // called next time, we enable all phases if the best IP has since changed.
+ previous_best_ip_ = network_->GetBestIP();
}
void AllocationSequence::Stop() {
@@ -1267,7 +1269,7 @@
session_->allocator()->origin(), emit_local_candidate_for_anyaddress);
} else {
port = UDPPort::Create(
- session_->network_thread(), session_->socket_factory(), network_, ip_,
+ session_->network_thread(), session_->socket_factory(), network_,
session_->allocator()->min_port(), session_->allocator()->max_port(),
session_->username(), session_->password(),
session_->allocator()->origin(), emit_local_candidate_for_anyaddress);
@@ -1300,13 +1302,11 @@
return;
}
- Port* port = TCPPort::Create(session_->network_thread(),
- session_->socket_factory(),
- network_, ip_,
- session_->allocator()->min_port(),
- session_->allocator()->max_port(),
- session_->username(), session_->password(),
- session_->allocator()->allow_tcp_listen());
+ Port* port = TCPPort::Create(
+ session_->network_thread(), session_->socket_factory(), network_,
+ session_->allocator()->min_port(), session_->allocator()->max_port(),
+ session_->username(), session_->password(),
+ session_->allocator()->allow_tcp_listen());
if (port) {
session_->AddAllocatedPort(port, this, true);
// Since TCPPort is not created using shared socket, |port| will not be
@@ -1330,14 +1330,11 @@
return;
}
- StunPort* port = StunPort::Create(session_->network_thread(),
- session_->socket_factory(),
- network_, ip_,
- session_->allocator()->min_port(),
- session_->allocator()->max_port(),
- session_->username(), session_->password(),
- config_->StunServers(),
- session_->allocator()->origin());
+ StunPort* port = StunPort::Create(
+ session_->network_thread(), session_->socket_factory(), network_,
+ session_->allocator()->min_port(), session_->allocator()->max_port(),
+ session_->username(), session_->password(), config_->StunServers(),
+ session_->allocator()->origin());
if (port) {
session_->AddAllocatedPort(port, this, true);
// Since StunPort is not created using shared socket, |port| will not be
@@ -1373,12 +1370,10 @@
void AllocationSequence::CreateGturnPort(const RelayServerConfig& config) {
// TODO(mallinath) - Rename RelayPort to GTurnPort.
- RelayPort* port = RelayPort::Create(session_->network_thread(),
- session_->socket_factory(),
- network_, ip_,
- session_->allocator()->min_port(),
- session_->allocator()->max_port(),
- config_->username, config_->password);
+ RelayPort* port = RelayPort::Create(
+ session_->network_thread(), session_->socket_factory(), network_,
+ session_->allocator()->min_port(), session_->allocator()->max_port(),
+ config_->username, config_->password);
if (port) {
// Since RelayPort is not created using shared socket, |port| will not be
// added to the dequeue.
@@ -1417,12 +1412,12 @@
// Do not create a port if the server address family is known and does
// not match the local IP address family.
int server_ip_family = relay_port->address.ipaddr().family();
- int local_ip_family = ip_.family();
+ int local_ip_family = network_->GetBestIP().family();
if (server_ip_family != AF_UNSPEC && server_ip_family != local_ip_family) {
LOG(LS_INFO) << "Server and local address families are not compatible. "
<< "Server address: "
<< relay_port->address.ipaddr().ToString()
- << " Local address: " << ip_.ToString();
+ << " Local address: " << network_->GetBestIP().ToString();
continue;
}
@@ -1444,15 +1439,11 @@
// remove entrt from it's map.
port->SignalDestroyed.connect(this, &AllocationSequence::OnPortDestroyed);
} else {
- port = TurnPort::Create(session_->network_thread(),
- session_->socket_factory(),
- network_, ip_,
- session_->allocator()->min_port(),
- session_->allocator()->max_port(),
- session_->username(),
- session_->password(),
- *relay_port, config.credentials, config.priority,
- session_->allocator()->origin());
+ port = TurnPort::Create(
+ session_->network_thread(), session_->socket_factory(), network_,
+ session_->allocator()->min_port(), session_->allocator()->max_port(),
+ session_->username(), session_->password(), *relay_port,
+ config.credentials, config.priority, session_->allocator()->origin());
}
RTC_DCHECK(port != NULL);
port->SetTlsCertPolicy(config.tls_cert_policy);
diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h
index a991417..5a4999c 100644
--- a/webrtc/p2p/client/basicportallocator.h
+++ b/webrtc/p2p/client/basicportallocator.h
@@ -365,7 +365,8 @@
BasicPortAllocatorSession* session_;
bool network_failed_ = false;
rtc::Network* network_;
- rtc::IPAddress ip_;
+ // Compared with the new best IP in DisableEquivalentPhases.
+ rtc::IPAddress previous_best_ip_;
PortConfiguration* config_;
State state_;
uint32_t flags_;
diff --git a/webrtc/rtc_base/virtualsocketserver.cc b/webrtc/rtc_base/virtualsocketserver.cc
index e47b01b..23e94a2 100644
--- a/webrtc/rtc_base/virtualsocketserver.cc
+++ b/webrtc/rtc_base/virtualsocketserver.cc
@@ -127,8 +127,6 @@
}
SocketAddress VirtualSocket::GetLocalAddress() const {
- if (!alternative_local_addr_.IsNil())
- return alternative_local_addr_;
return local_addr_;
}
@@ -140,10 +138,6 @@
local_addr_ = addr;
}
-void VirtualSocket::SetAlternativeLocalAddress(const SocketAddress& addr) {
- alternative_local_addr_ = addr;
-}
-
int VirtualSocket::Bind(const SocketAddress& addr) {
if (!local_addr_.IsNil()) {
error_ = EINVAL;
@@ -642,6 +636,12 @@
wakeup_.Set();
}
+void VirtualSocketServer::SetAlternativeLocalAddress(
+ const rtc::IPAddress& address,
+ const rtc::IPAddress& alternative) {
+ alternative_address_mapping_[address] = alternative;
+}
+
bool VirtualSocketServer::ProcessMessagesUntilIdle() {
RTC_DCHECK(msg_queue_ == Thread::Current());
stop_on_idle_ = true;
@@ -699,12 +699,22 @@
int VirtualSocketServer::Bind(VirtualSocket* socket, SocketAddress* addr) {
RTC_DCHECK(nullptr != socket);
+ // Normalize the IP.
if (!IPIsUnspec(addr->ipaddr())) {
addr->SetIP(addr->ipaddr().Normalized());
} else {
RTC_NOTREACHED();
}
+ // If the IP appears in |alternative_address_mapping_|, meaning the test has
+ // configured sockets bound to this IP to actually use another IP, replace
+ // the IP here.
+ auto alternative = alternative_address_mapping_.find(addr->ipaddr());
+ if (alternative != alternative_address_mapping_.end()) {
+ addr->SetIP(alternative->second);
+ }
+
+ // Assign a port if not assigned.
if (addr->port() == 0) {
for (int i = 0; i < kEphemeralPortCount; ++i) {
addr->SetPort(GetNextPort());
diff --git a/webrtc/rtc_base/virtualsocketserver.h b/webrtc/rtc_base/virtualsocketserver.h
index a08bc0c..089e73a 100644
--- a/webrtc/rtc_base/virtualsocketserver.h
+++ b/webrtc/rtc_base/virtualsocketserver.h
@@ -118,6 +118,16 @@
delay_by_ip_[address.ipaddr()] = delay_ms;
}
+ // Used by TurnPortTest and TcpPortTest (for example), to mimic a case where
+ // a proxy returns the local host address instead of the original one the
+ // port was bound against. Please see WebRTC issue 3927 for more detail.
+ //
+ // If SetAlternativeLocalAddress(A, B) is called, then when something
+ // attempts to bind a socket to address A, it will get a socket bound to
+ // address B instead.
+ void SetAlternativeLocalAddress(const rtc::IPAddress& address,
+ const rtc::IPAddress& alternative);
+
typedef std::pair<double, double> Point;
typedef std::vector<Point> Function;
@@ -273,6 +283,7 @@
uint32_t delay_samples_;
std::map<rtc::IPAddress, int> delay_by_ip_;
+ std::map<rtc::IPAddress, rtc::IPAddress> alternative_address_mapping_;
std::unique_ptr<Function> delay_dist_;
CriticalSection delay_crit_;
@@ -294,11 +305,6 @@
SocketAddress GetLocalAddress() const override;
SocketAddress GetRemoteAddress() const override;
- // Used by TurnPortTest to mimic a case where proxy returns local host address
- // instead of the original one TurnPort was bound against. Please see WebRTC
- // issue 3927 for more detail.
- void SetAlternativeLocalAddress(const SocketAddress& addr);
-
int Bind(const SocketAddress& addr) override;
int Connect(const SocketAddress& addr) override;
int Close() override;
@@ -353,7 +359,6 @@
ConnState state_;
int error_;
SocketAddress local_addr_;
- SocketAddress alternative_local_addr_;
SocketAddress remote_addr_;
// Pending sockets which can be Accepted
