Ensuring that UDP TURN servers are always used as STUN servers.
This was already working in most cases, but not for some corner cases:
* If the PORTALLOCATOR_ENABLE_SHARED_SOCKET flag is not set
* If both a STUN server and TURN server are configured
I added unit tests for these cases, and centralized the code that gets
STUN server addresses in order to fix these and any related issues.
BUG=webrtc:4215
Review URL: https://codereview.webrtc.org/1215713003
Cr-Original-Commit-Position: refs/heads/master@{#9596}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: c5d0d95fd8957a7a6645b1196e5f1e9cee33525c
diff --git a/base/nat_unittest.cc b/base/nat_unittest.cc
index 36b9327..af691e4 100644
--- a/base/nat_unittest.cc
+++ b/base/nat_unittest.cc
@@ -18,6 +18,7 @@
#include "webrtc/base/network.h"
#include "webrtc/base/physicalsocketserver.h"
#include "webrtc/base/testclient.h"
+#include "webrtc/base/asynctcpsocket.h"
#include "webrtc/base/virtualsocketserver.h"
#include "webrtc/test/testsupport/gtest_disable.h"
@@ -36,6 +37,11 @@
return new TestClient(socket);
}
+TestClient* CreateTCPTestClient(AsyncSocket* socket) {
+ AsyncTCPSocket* packet_socket = new AsyncTCPSocket(socket, false);
+ return new TestClient(packet_socket);
+}
+
// Tests that when sending from internal_addr to external_addrs through the
// NAT type specified by nat_type, all external addrs receive the sent packet
// and, if exp_same is true, all use the same mapped-address on the NAT.
@@ -48,10 +54,11 @@
SocketAddress server_addr = internal_addr;
server_addr.SetPort(0); // Auto-select a port
- NATServer* nat = new NATServer(
- nat_type, internal, server_addr, external, external_addrs[0]);
+ NATServer* nat = new NATServer(nat_type, internal, server_addr, server_addr,
+ external, external_addrs[0]);
NATSocketFactory* natsf = new NATSocketFactory(internal,
- nat->internal_address());
+ nat->internal_udp_address(),
+ nat->internal_tcp_address());
TestClient* in = CreateTestClient(natsf, internal_addr);
TestClient* out[4];
@@ -99,10 +106,11 @@
SocketAddress server_addr = internal_addr;
server_addr.SetPort(0); // Auto-select a port
- NATServer* nat = new NATServer(
- nat_type, internal, server_addr, external, external_addrs[0]);
+ NATServer* nat = new NATServer(nat_type, internal, server_addr, server_addr,
+ external, external_addrs[0]);
NATSocketFactory* natsf = new NATSocketFactory(internal,
- nat->internal_address());
+ nat->internal_udp_address(),
+ nat->internal_tcp_address());
TestClient* in = CreateTestClient(natsf, internal_addr);
TestClient* out[4];
@@ -295,56 +303,86 @@
}
}
-// TODO: Finish this test
class NatTcpTest : public testing::Test, public sigslot::has_slots<> {
public:
- NatTcpTest() : connected_(false) {}
- virtual void SetUp() {
- int_vss_ = new TestVirtualSocketServer(new PhysicalSocketServer());
- ext_vss_ = new TestVirtualSocketServer(new PhysicalSocketServer());
- nat_ = new NATServer(NAT_OPEN_CONE, int_vss_, SocketAddress(),
- ext_vss_, SocketAddress());
- natsf_ = new NATSocketFactory(int_vss_, nat_->internal_address());
+ NatTcpTest()
+ : int_addr_("192.168.0.1", 0),
+ ext_addr_("10.0.0.1", 0),
+ connected_(false),
+ int_pss_(new PhysicalSocketServer()),
+ ext_pss_(new PhysicalSocketServer()),
+ int_vss_(new TestVirtualSocketServer(int_pss_)),
+ ext_vss_(new TestVirtualSocketServer(ext_pss_)),
+ int_thread_(new Thread(int_vss_.get())),
+ ext_thread_(new Thread(ext_vss_.get())),
+ nat_(new NATServer(NAT_OPEN_CONE, int_vss_.get(), int_addr_, int_addr_,
+ ext_vss_.get(), ext_addr_)),
+ natsf_(new NATSocketFactory(int_vss_.get(),
+ nat_->internal_udp_address(),
+ nat_->internal_tcp_address())) {
+ int_thread_->Start();
+ ext_thread_->Start();
}
+
void OnConnectEvent(AsyncSocket* socket) {
connected_ = true;
}
+
void OnAcceptEvent(AsyncSocket* socket) {
- accepted_ = server_->Accept(NULL);
+ accepted_.reset(server_->Accept(NULL));
}
+
void OnCloseEvent(AsyncSocket* socket, int error) {
}
+
void ConnectEvents() {
server_->SignalReadEvent.connect(this, &NatTcpTest::OnAcceptEvent);
client_->SignalConnectEvent.connect(this, &NatTcpTest::OnConnectEvent);
}
- TestVirtualSocketServer* int_vss_;
- TestVirtualSocketServer* ext_vss_;
- NATServer* nat_;
- NATSocketFactory* natsf_;
- AsyncSocket* client_;
- AsyncSocket* server_;
- AsyncSocket* accepted_;
+
+ SocketAddress int_addr_;
+ SocketAddress ext_addr_;
bool connected_;
+ PhysicalSocketServer* int_pss_;
+ PhysicalSocketServer* ext_pss_;
+ rtc::scoped_ptr<TestVirtualSocketServer> int_vss_;
+ rtc::scoped_ptr<TestVirtualSocketServer> ext_vss_;
+ rtc::scoped_ptr<Thread> int_thread_;
+ rtc::scoped_ptr<Thread> ext_thread_;
+ rtc::scoped_ptr<NATServer> nat_;
+ rtc::scoped_ptr<NATSocketFactory> natsf_;
+ rtc::scoped_ptr<AsyncSocket> client_;
+ rtc::scoped_ptr<AsyncSocket> server_;
+ rtc::scoped_ptr<AsyncSocket> accepted_;
};
-TEST_F(NatTcpTest, DISABLED_TestConnectOut) {
- server_ = ext_vss_->CreateAsyncSocket(SOCK_STREAM);
- server_->Bind(SocketAddress());
+TEST_F(NatTcpTest, TestConnectOut) {
+ server_.reset(ext_vss_->CreateAsyncSocket(SOCK_STREAM));
+ server_->Bind(ext_addr_);
server_->Listen(5);
- client_ = int_vss_->CreateAsyncSocket(SOCK_STREAM);
- EXPECT_GE(0, client_->Bind(SocketAddress()));
+ client_.reset(natsf_->CreateAsyncSocket(SOCK_STREAM));
+ EXPECT_GE(0, client_->Bind(int_addr_));
EXPECT_GE(0, client_->Connect(server_->GetLocalAddress()));
-
ConnectEvents();
EXPECT_TRUE_WAIT(connected_, 1000);
EXPECT_EQ(client_->GetRemoteAddress(), server_->GetLocalAddress());
- EXPECT_EQ(client_->GetRemoteAddress(), accepted_->GetLocalAddress());
- EXPECT_EQ(client_->GetLocalAddress(), accepted_->GetRemoteAddress());
+ EXPECT_EQ(accepted_->GetRemoteAddress().ipaddr(), ext_addr_.ipaddr());
- client_->Close();
+ rtc::scoped_ptr<rtc::TestClient> in(CreateTCPTestClient(client_.release()));
+ rtc::scoped_ptr<rtc::TestClient> out(
+ CreateTCPTestClient(accepted_.release()));
+
+ const char* buf = "test_packet";
+ size_t len = strlen(buf);
+
+ in->Send(buf, len);
+ SocketAddress trans_addr;
+ EXPECT_TRUE(out->CheckNextPacket(buf, len, &trans_addr));
+
+ out->Send(buf, len);
+ EXPECT_TRUE(in->CheckNextPacket(buf, len, &trans_addr));
}
-//#endif
+// #endif
diff --git a/base/natserver.cc b/base/natserver.cc
index 0ce04d7..b071e01 100644
--- a/base/natserver.cc
+++ b/base/natserver.cc
@@ -11,6 +11,7 @@
#include "webrtc/base/natsocketfactory.h"
#include "webrtc/base/natserver.h"
#include "webrtc/base/logging.h"
+#include "webrtc/base/socketadapters.h"
namespace rtc {
@@ -63,14 +64,77 @@
return false;
}
+// Proxy socket that will capture the external destination address intended for
+// a TCP connection to the NAT server.
+class NATProxyServerSocket : public AsyncProxyServerSocket {
+ public:
+ NATProxyServerSocket(AsyncSocket* socket)
+ : AsyncProxyServerSocket(socket, kNATEncodedIPv6AddressSize) {
+ BufferInput(true);
+ }
+
+ void SendConnectResult(int err, const SocketAddress& addr) override {
+ char code = err ? 1 : 0;
+ BufferedReadAdapter::DirectSend(&code, sizeof(char));
+ }
+
+ protected:
+ void ProcessInput(char* data, size_t* len) override {
+ if (*len < 2) {
+ return;
+ }
+
+ int family = data[1];
+ ASSERT(family == AF_INET || family == AF_INET6);
+ if ((family == AF_INET && *len < kNATEncodedIPv4AddressSize) ||
+ (family == AF_INET6 && *len < kNATEncodedIPv6AddressSize)) {
+ return;
+ }
+
+ SocketAddress dest_addr;
+ size_t address_length = UnpackAddressFromNAT(data, *len, &dest_addr);
+
+ *len -= address_length;
+ if (*len > 0) {
+ memmove(data, data + address_length, *len);
+ }
+
+ bool remainder = (*len > 0);
+ BufferInput(false);
+ SignalConnectRequest(this, dest_addr);
+ if (remainder) {
+ SignalReadEvent(this);
+ }
+ }
+
+};
+
+class NATProxyServer : public ProxyServer {
+ public:
+ NATProxyServer(SocketFactory* int_factory, const SocketAddress& int_addr,
+ SocketFactory* ext_factory, const SocketAddress& ext_ip)
+ : ProxyServer(int_factory, int_addr, ext_factory, ext_ip) {
+ }
+
+ protected:
+ AsyncProxyServerSocket* WrapSocket(AsyncSocket* socket) override {
+ return new NATProxyServerSocket(socket);
+ }
+};
+
NATServer::NATServer(
- NATType type, SocketFactory* internal, const SocketAddress& internal_addr,
+ NATType type, SocketFactory* internal,
+ const SocketAddress& internal_udp_addr,
+ const SocketAddress& internal_tcp_addr,
SocketFactory* external, const SocketAddress& external_ip)
: external_(external), external_ip_(external_ip.ipaddr(), 0) {
nat_ = NAT::Create(type);
- server_socket_ = AsyncUDPSocket::Create(internal, internal_addr);
- server_socket_->SignalReadPacket.connect(this, &NATServer::OnInternalPacket);
+ udp_server_socket_ = AsyncUDPSocket::Create(internal, internal_udp_addr);
+ udp_server_socket_->SignalReadPacket.connect(this,
+ &NATServer::OnInternalUDPPacket);
+ tcp_proxy_server_ = new NATProxyServer(internal, internal_tcp_addr, external,
+ external_ip);
int_map_ = new InternalMap(RouteCmp(nat_));
ext_map_ = new ExternalMap();
@@ -83,15 +147,15 @@
delete iter->second;
delete nat_;
- delete server_socket_;
+ delete udp_server_socket_;
+ delete tcp_proxy_server_;
delete int_map_;
delete ext_map_;
}
-void NATServer::OnInternalPacket(
+void NATServer::OnInternalUDPPacket(
AsyncPacketSocket* socket, const char* buf, size_t size,
const SocketAddress& addr, const PacketTime& packet_time) {
-
// Read the intended destination from the wire.
SocketAddress dest_addr;
size_t length = UnpackAddressFromNAT(buf, size, &dest_addr);
@@ -113,10 +177,9 @@
iter->second->socket->SendTo(buf + length, size - length, dest_addr, options);
}
-void NATServer::OnExternalPacket(
+void NATServer::OnExternalUDPPacket(
AsyncPacketSocket* socket, const char* buf, size_t size,
const SocketAddress& remote_addr, const PacketTime& packet_time) {
-
SocketAddress local_addr = socket->GetLocalAddress();
// Find the translation for this addresses.
@@ -139,8 +202,8 @@
// Copy the data part after the address.
rtc::PacketOptions options;
memcpy(real_buf.get() + addrlength, buf, size);
- server_socket_->SendTo(real_buf.get(), size + addrlength,
- iter->second->route.source(), options);
+ udp_server_socket_->SendTo(real_buf.get(), size + addrlength,
+ iter->second->route.source(), options);
}
void NATServer::Translate(const SocketAddressPair& route) {
@@ -154,7 +217,7 @@
TransEntry* entry = new TransEntry(route, socket, nat_);
(*int_map_)[route] = entry;
(*ext_map_)[socket->GetLocalAddress()] = entry;
- socket->SignalReadPacket.connect(this, &NATServer::OnExternalPacket);
+ socket->SignalReadPacket.connect(this, &NATServer::OnExternalUDPPacket);
}
bool NATServer::ShouldFilterOut(TransEntry* entry,
diff --git a/base/natserver.h b/base/natserver.h
index 5975620..e987021 100644
--- a/base/natserver.h
+++ b/base/natserver.h
@@ -19,6 +19,7 @@
#include "webrtc/base/thread.h"
#include "webrtc/base/socketfactory.h"
#include "webrtc/base/nattypes.h"
+#include "webrtc/base/proxyserver.h"
namespace rtc {
@@ -46,27 +47,40 @@
// Implements the NAT device. It listens for packets on the internal network,
// translates them, and sends them out over the external network.
+//
+// TCP connections initiated from the internal side of the NAT server are
+// also supported, by making a connection to the NAT server's TCP address and
+// then sending the remote address in quasi-STUN format. The connection status
+// will be indicated back to the client as a 1 byte status code, where '0'
+// indicates success.
-const int NAT_SERVER_PORT = 4237;
+const int NAT_SERVER_UDP_PORT = 4237;
+const int NAT_SERVER_TCP_PORT = 4238;
class NATServer : public sigslot::has_slots<> {
public:
NATServer(
- NATType type, SocketFactory* internal, const SocketAddress& internal_addr,
+ NATType type, SocketFactory* internal,
+ const SocketAddress& internal_udp_addr,
+ const SocketAddress& internal_tcp_addr,
SocketFactory* external, const SocketAddress& external_ip);
~NATServer() override;
- SocketAddress internal_address() const {
- return server_socket_->GetLocalAddress();
+ SocketAddress internal_udp_address() const {
+ return udp_server_socket_->GetLocalAddress();
+ }
+
+ SocketAddress internal_tcp_address() const {
+ return tcp_proxy_server_->GetServerAddress();
}
// Packets received on one of the networks.
- void OnInternalPacket(AsyncPacketSocket* socket, const char* buf,
- size_t size, const SocketAddress& addr,
- const PacketTime& packet_time);
- void OnExternalPacket(AsyncPacketSocket* socket, const char* buf,
- size_t size, const SocketAddress& remote_addr,
- const PacketTime& packet_time);
+ void OnInternalUDPPacket(AsyncPacketSocket* socket, const char* buf,
+ size_t size, const SocketAddress& addr,
+ const PacketTime& packet_time);
+ void OnExternalUDPPacket(AsyncPacketSocket* socket, const char* buf,
+ size_t size, const SocketAddress& remote_addr,
+ const PacketTime& packet_time);
private:
typedef std::set<SocketAddress, AddrCmp> AddressSet;
@@ -98,8 +112,8 @@
SocketFactory* internal_;
SocketFactory* external_;
SocketAddress external_ip_;
- AsyncUDPSocket* server_socket_;
- AsyncSocket* tcp_server_socket_;
+ AsyncUDPSocket* udp_server_socket_;
+ ProxyServer* tcp_proxy_server_;
InternalMap* int_map_;
ExternalMap* ext_map_;
DISALLOW_COPY_AND_ASSIGN(NATServer);
diff --git a/base/natsocketfactory.cc b/base/natsocketfactory.cc
index 9c6756b..a23a7e8 100644
--- a/base/natsocketfactory.cc
+++ b/base/natsocketfactory.cc
@@ -179,8 +179,7 @@
// Decode the wire packet into the actual results.
SocketAddress real_remote_addr;
- size_t addrlength =
- UnpackAddressFromNAT(buf_, result, &real_remote_addr);
+ size_t addrlength = UnpackAddressFromNAT(buf_, result, &real_remote_addr);
memcpy(data, buf_ + addrlength, result - addrlength);
// Make sure this packet should be delivered before returning it.
@@ -230,7 +229,7 @@
}
void OnConnectEvent(AsyncSocket* socket) {
- // If we're NATed, we need to send a request with the real addr to use.
+ // If we're NATed, we need to send a message with the real addr to use.
ASSERT(socket == socket_);
if (server_addr_.IsNil()) {
connected_ = true;
@@ -269,7 +268,7 @@
// Sends the destination address to the server to tell it to connect.
void SendConnectRequest() {
- char buf[256];
+ char buf[kNATEncodedIPv6AddressSize];
size_t length = PackAddressForNAT(buf, ARRAY_SIZE(buf), remote_addr_);
socket_->Send(buf, length);
}
@@ -279,6 +278,7 @@
char code;
socket_->Recv(&code, sizeof(code));
if (code == 0) {
+ connected_ = true;
SignalConnectEvent(this);
} else {
Close();
@@ -299,8 +299,10 @@
// NATSocketFactory
NATSocketFactory::NATSocketFactory(SocketFactory* factory,
- const SocketAddress& nat_addr)
- : factory_(factory), nat_addr_(nat_addr) {
+ const SocketAddress& nat_udp_addr,
+ const SocketAddress& nat_tcp_addr)
+ : factory_(factory), nat_udp_addr_(nat_udp_addr),
+ nat_tcp_addr_(nat_tcp_addr) {
}
Socket* NATSocketFactory::CreateSocket(int type) {
@@ -321,7 +323,11 @@
AsyncSocket* NATSocketFactory::CreateInternalSocket(int family, int type,
const SocketAddress& local_addr, SocketAddress* nat_addr) {
- *nat_addr = nat_addr_;
+ if (type == SOCK_STREAM) {
+ *nat_addr = nat_tcp_addr_;
+ } else {
+ *nat_addr = nat_udp_addr_;
+ }
return factory_->CreateAsyncSocket(family, type);
}
@@ -385,7 +391,7 @@
if (nat) {
socket = nat->internal_factory()->CreateAsyncSocket(family, type);
*nat_addr = (type == SOCK_STREAM) ?
- nat->internal_tcp_address() : nat->internal_address();
+ nat->internal_tcp_address() : nat->internal_udp_address();
} else {
socket = server_->CreateAsyncSocket(family, type);
}
@@ -403,7 +409,7 @@
VirtualSocketServer* internal_server = new VirtualSocketServer(server_);
internal_server->SetMessageQueue(server_->queue());
internal_factory_.reset(internal_server);
- nat_server_.reset(new NATServer(type, internal_server, int_ip,
+ nat_server_.reset(new NATServer(type, internal_server, int_ip, int_ip,
ext_factory, ext_ip));
}
diff --git a/base/natsocketfactory.h b/base/natsocketfactory.h
index 9a189d8..3519cf0 100644
--- a/base/natsocketfactory.h
+++ b/base/natsocketfactory.h
@@ -37,7 +37,8 @@
// from a socket factory, given to the constructor.
class NATSocketFactory : public SocketFactory, public NATInternalSocketFactory {
public:
- NATSocketFactory(SocketFactory* factory, const SocketAddress& nat_addr);
+ NATSocketFactory(SocketFactory* factory, const SocketAddress& nat_udp_addr,
+ const SocketAddress& nat_tcp_addr);
// SocketFactory implementation
Socket* CreateSocket(int type) override;
@@ -53,7 +54,8 @@
private:
SocketFactory* factory_;
- SocketAddress nat_addr_;
+ SocketAddress nat_udp_addr_;
+ SocketAddress nat_tcp_addr_;
DISALLOW_COPY_AND_ASSIGN(NATSocketFactory);
};
@@ -94,8 +96,8 @@
~Translator();
SocketFactory* internal_factory() { return internal_factory_.get(); }
- SocketAddress internal_address() const {
- return nat_server_->internal_address();
+ SocketAddress internal_udp_address() const {
+ return nat_server_->internal_udp_address();
}
SocketAddress internal_tcp_address() const {
return SocketAddress(); // nat_server_->internal_tcp_address();
diff --git a/base/proxyserver.cc b/base/proxyserver.cc
index 8f12a99..d91a92f 100644
--- a/base/proxyserver.cc
+++ b/base/proxyserver.cc
@@ -36,6 +36,10 @@
}
}
+SocketAddress ProxyServer::GetServerAddress() {
+ return server_socket_->GetLocalAddress();
+}
+
void ProxyServer::OnAcceptEvent(AsyncSocket* socket) {
ASSERT(socket != NULL && socket == server_socket_.get());
AsyncSocket* int_socket = socket->Accept(NULL);
diff --git a/base/proxyserver.h b/base/proxyserver.h
index 83fe6c2..711ed55 100644
--- a/base/proxyserver.h
+++ b/base/proxyserver.h
@@ -64,6 +64,9 @@
SocketFactory* ext_factory, const SocketAddress& ext_ip);
~ProxyServer() override;
+ // Returns the address to which the proxy server is bound
+ SocketAddress GetServerAddress();
+
protected:
void OnAcceptEvent(AsyncSocket* socket);
virtual AsyncProxyServerSocket* WrapSocket(AsyncSocket* socket) = 0;
diff --git a/base/socketadapters.cc b/base/socketadapters.cc
index 4a2da0a..b1c2a87 100644
--- a/base/socketadapters.cc
+++ b/base/socketadapters.cc
@@ -81,10 +81,18 @@
// FIX: If cb == 0, we won't generate another read event
int res = AsyncSocketAdapter::Recv(pv, cb);
- if (res < 0)
- return res;
+ if (res >= 0) {
+ // Read from socket and possibly buffer; return combined length
+ return res + static_cast<int>(read);
+ }
- return res + static_cast<int>(read);
+ if (read > 0) {
+ // Failed to read from socket, but still read something from buffer
+ return static_cast<int>(read);
+ }
+
+ // Didn't read anything; return error from socket
+ return res;
}
void BufferedReadAdapter::BufferInput(bool on) {
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 57618b7..4877af3 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -47,8 +47,8 @@
static const int kTimeout = 1000;
static const SocketAddress kLocalAddr1("192.168.1.2", 0);
static const SocketAddress kLocalAddr2("192.168.1.3", 0);
-static const SocketAddress kNatAddr1("77.77.77.77", rtc::NAT_SERVER_PORT);
-static const SocketAddress kNatAddr2("88.88.88.88", rtc::NAT_SERVER_PORT);
+static const SocketAddress kNatAddr1("77.77.77.77", rtc::NAT_SERVER_UDP_PORT);
+static const SocketAddress kNatAddr2("88.88.88.88", rtc::NAT_SERVER_UDP_PORT);
static const SocketAddress kStunAddr("99.99.99.1", STUN_SERVER_PORT);
static const SocketAddress kRelayUdpIntAddr("99.99.99.2", 5000);
static const SocketAddress kRelayUdpExtAddr("99.99.99.3", 5001);
@@ -343,8 +343,8 @@
ss_scope_(ss_.get()),
network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32),
socket_factory_(rtc::Thread::Current()),
- nat_factory1_(ss_.get(), kNatAddr1),
- nat_factory2_(ss_.get(), kNatAddr2),
+ nat_factory1_(ss_.get(), kNatAddr1, SocketAddress()),
+ nat_factory2_(ss_.get(), kNatAddr2, SocketAddress()),
nat_socket_factory1_(&nat_factory1_),
nat_socket_factory2_(&nat_factory2_),
stun_server_(TestStunServer::Create(main_, kStunAddr)),
@@ -507,7 +507,7 @@
}
rtc::NATServer* CreateNatServer(const SocketAddress& addr,
rtc::NATType type) {
- return new rtc::NATServer(type, ss_.get(), addr, ss_.get(), addr);
+ return new rtc::NATServer(type, ss_.get(), addr, addr, ss_.get(), addr);
}
static const char* StunName(NATType type) {
switch (type) {
diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc
index bdafe35..1965ad9 100644
--- a/p2p/client/basicportallocator.cc
+++ b/p2p/client/basicportallocator.cc
@@ -208,7 +208,7 @@
BasicPortAllocator::~BasicPortAllocator() {
}
-PortAllocatorSession *BasicPortAllocator::CreateSessionInternal(
+PortAllocatorSession* BasicPortAllocator::CreateSessionInternal(
const std::string& content_name, int component,
const std::string& ice_ufrag, const std::string& ice_pwd) {
return new BasicPortAllocatorSession(
@@ -927,18 +927,10 @@
// If STUN is not disabled, setting stun server address to port.
if (!IsFlagSet(PORTALLOCATOR_DISABLE_STUN)) {
- // If config has stun_servers, use it to get server reflexive candidate
- // otherwise use first TURN server which supports UDP.
if (config_ && !config_->StunServers().empty()) {
LOG(LS_INFO) << "AllocationSequence: UDPPort will be handling the "
<< "STUN candidate generation.";
port->set_server_addresses(config_->StunServers());
- } else if (config_ &&
- config_->SupportsProtocol(RELAY_TURN, PROTO_UDP)) {
- port->set_server_addresses(config_->GetRelayServerAddresses(
- RELAY_TURN, PROTO_UDP));
- LOG(LS_INFO) << "AllocationSequence: TURN Server address will be "
- << " used for generating STUN candidate.";
}
}
}
@@ -1171,6 +1163,13 @@
stun_servers.find(stun_address) == stun_servers.end()) {
stun_servers.insert(stun_address);
}
+ // Every UDP TURN server should also be used as a STUN server.
+ ServerAddresses turn_servers = GetRelayServerAddresses(RELAY_TURN, PROTO_UDP);
+ for (const rtc::SocketAddress& turn_server : turn_servers) {
+ if (stun_servers.find(turn_server) == stun_servers.end()) {
+ stun_servers.insert(turn_server);
+ }
+ }
return stun_servers;
}
diff --git a/p2p/client/basicportallocator.h b/p2p/client/basicportallocator.h
index 96468d3..f2d92ef 100644
--- a/p2p/client/basicportallocator.h
+++ b/p2p/client/basicportallocator.h
@@ -220,7 +220,8 @@
const std::string& username,
const std::string& password);
- // TODO(jiayl): remove when |stun_address| is removed.
+ // Returns addresses of both the explicitly configured STUN servers,
+ // and TURN servers that should be used as STUN servers.
ServerAddresses StunServers();
// Adds another relay server, with the given ports and modifier, to the list.
diff --git a/p2p/client/portallocator_unittest.cc b/p2p/client/portallocator_unittest.cc
index 017c37a..6940ea1 100644
--- a/p2p/client/portallocator_unittest.cc
+++ b/p2p/client/portallocator_unittest.cc
@@ -40,7 +40,8 @@
static const SocketAddress kClientIPv6Addr(
"2401:fa00:4:1000:be30:5bff:fee5:c3", 0);
static const SocketAddress kClientAddr2("22.22.22.22", 0);
-static const SocketAddress kNatAddr("77.77.77.77", rtc::NAT_SERVER_PORT);
+static const SocketAddress kNatUdpAddr("77.77.77.77", rtc::NAT_SERVER_UDP_PORT);
+static const SocketAddress kNatTcpAddr("77.77.77.77", rtc::NAT_SERVER_TCP_PORT);
static const SocketAddress kRemoteClientAddr("22.22.22.22", 0);
static const SocketAddress kStunAddr("99.99.99.1", cricket::STUN_SERVER_PORT);
static const SocketAddress kRelayUdpIntAddr("99.99.99.2", 5000);
@@ -85,7 +86,7 @@
vss_(new rtc::VirtualSocketServer(pss_.get())),
fss_(new rtc::FirewallSocketServer(vss_.get())),
ss_scope_(fss_.get()),
- nat_factory_(vss_.get(), kNatAddr),
+ nat_factory_(vss_.get(), kNatUdpAddr, kNatTcpAddr),
nat_socket_factory_(&nat_factory_),
stun_server_(cricket::TestStunServer::Create(Thread::Current(),
kStunAddr)),
@@ -113,7 +114,8 @@
}
void ResetWithNatServer(const rtc::SocketAddress& stun_server) {
nat_server_.reset(new rtc::NATServer(
- rtc::NAT_OPEN_CONE, vss_.get(), kNatAddr, vss_.get(), kNatAddr));
+ rtc::NAT_OPEN_CONE, vss_.get(), kNatUdpAddr, kNatTcpAddr, vss_.get(),
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0)));
ServerAddresses stun_servers;
if (!stun_server.IsNil()) {
@@ -241,7 +243,7 @@
EXPECT_PRED5(CheckCandidate, candidates_[0],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
- rtc::SocketAddress(kNatAddr.ipaddr(), 0));
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
EXPECT_EQ(
rtc::EmptySocketAddressWithFamily(candidates_[0].address().family()),
candidates_[0].related_address());
@@ -249,7 +251,7 @@
EXPECT_PRED5(CheckCandidate, candidates_[1],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
- EXPECT_EQ(kNatAddr.ipaddr(), candidates_[1].related_address().ipaddr());
+ EXPECT_EQ(kNatUdpAddr.ipaddr(), candidates_[1].related_address().ipaddr());
}
protected:
@@ -779,12 +781,12 @@
cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", kClientAddr);
EXPECT_PRED5(CheckCandidate, candidates_[1],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
- rtc::SocketAddress(kNatAddr.ipaddr(), 0));
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
EXPECT_EQ(3U, candidates_.size());
}
-// Test TURN port in shared socket mode with UDP and TCP TURN server adderesses.
+// Test TURN port in shared socket mode with UDP and TCP TURN server addresses.
TEST_F(PortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) {
turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
AddInterface(kClientAddr);
@@ -865,7 +867,7 @@
cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", kClientAddr);
EXPECT_PRED5(CheckCandidate, candidates_[1],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
- rtc::SocketAddress(kNatAddr.ipaddr(), 0));
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
EXPECT_PRED5(CheckCandidate, candidates_[2],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
@@ -903,7 +905,7 @@
cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", kClientAddr);
EXPECT_PRED5(CheckCandidate, candidates_[1],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
- rtc::SocketAddress(kNatAddr.ipaddr(), 0));
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
EXPECT_PRED5(CheckCandidate, candidates_[2],
cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
@@ -916,6 +918,111 @@
EXPECT_EQ(1U, ports_[1]->Candidates().size());
}
+// Test that when only a TCP TURN server is available, we do NOT use it as
+// a UDP STUN server, as this could leak our IP address. Thus we should only
+// expect two ports, a UDPPort and TurnPort.
+TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnTcpOnly) {
+ turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
+ AddInterface(kClientAddr);
+ ResetWithNatServer(rtc::SocketAddress());
+ AddTurnServers(rtc::SocketAddress(), kTurnTcpIntAddr);
+
+ allocator_->set_flags(allocator().flags() |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ cricket::PORTALLOCATOR_DISABLE_TCP);
+
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+
+ ASSERT_EQ_WAIT(2U, candidates_.size(), kDefaultAllocationTimeout);
+ ASSERT_EQ(2U, ports_.size());
+ EXPECT_PRED5(CheckCandidate, candidates_[0],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp",
+ kClientAddr);
+ EXPECT_PRED5(CheckCandidate, candidates_[1],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
+ rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+ EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+ EXPECT_EQ(2U, candidates_.size());
+ EXPECT_EQ(1U, ports_[0]->Candidates().size());
+ EXPECT_EQ(1U, ports_[1]->Candidates().size());
+}
+
+// Test that even when PORTALLOCATOR_ENABLE_SHARED_SOCKET is NOT enabled, the
+// TURN server is used as the STUN server and we get 'local', 'stun', and
+// 'relay' candidates.
+// TODO(deadbeef): Remove this test when support for non-shared socket mode
+// is removed.
+TEST_F(PortAllocatorTest, TestNonSharedSocketWithNatUsingTurnAsStun) {
+ AddInterface(kClientAddr);
+ // Use an empty SocketAddress to add a NAT without STUN server.
+ ResetWithNatServer(SocketAddress());
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ allocator_->set_flags(allocator().flags() |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+ cricket::PORTALLOCATOR_DISABLE_TCP);
+
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+
+ ASSERT_EQ_WAIT(3U, candidates_.size(), kDefaultAllocationTimeout);
+ ASSERT_EQ(3U, ports_.size());
+ EXPECT_PRED5(CheckCandidate, candidates_[0],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp",
+ kClientAddr);
+ EXPECT_PRED5(CheckCandidate, candidates_[1],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
+ EXPECT_PRED5(CheckCandidate, candidates_[2],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
+ rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+ // Not using shared socket, so the STUN request's server reflexive address
+ // should be different than the TURN request's server reflexive address.
+ EXPECT_NE(candidates_[2].related_address(), candidates_[1].address());
+
+ EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+ EXPECT_EQ(3U, candidates_.size());
+ EXPECT_EQ(1U, ports_[0]->Candidates().size());
+ EXPECT_EQ(1U, ports_[1]->Candidates().size());
+ EXPECT_EQ(1U, ports_[2]->Candidates().size());
+}
+
+// Test that even when both a STUN and TURN server are configured, the TURN
+// server is used as a STUN server and we get a 'stun' candidate.
+TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnAndStun) {
+ AddInterface(kClientAddr);
+ // Configure with STUN server but destroy it, so we can ensure that it's
+ // the TURN server actually being used as a STUN server.
+ ResetWithNatServer(kStunAddr);
+ stun_server_.reset();
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ allocator_->set_flags(allocator().flags() |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ cricket::PORTALLOCATOR_DISABLE_TCP);
+
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+
+ ASSERT_EQ_WAIT(3U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_PRED5(CheckCandidate, candidates_[0],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp",
+ kClientAddr);
+ EXPECT_PRED5(CheckCandidate, candidates_[1],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
+ rtc::SocketAddress(kNatUdpAddr.ipaddr(), 0));
+ EXPECT_PRED5(CheckCandidate, candidates_[2],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
+ rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+ EXPECT_EQ(candidates_[2].related_address(), candidates_[1].address());
+
+ // Don't bother waiting for STUN timeout, since we already verified
+ // that we got a STUN candidate from the TURN server.
+}
+
// This test verifies when PORTALLOCATOR_ENABLE_SHARED_SOCKET flag is enabled
// and fail to generate STUN candidate, local UDP candidate is generated
// properly.
