diff --git a/webrtc/base/virtualsocketserver.cc b/webrtc/base/virtualsocketserver.cc index c2937bad2..59587bb84 100644 --- a/webrtc/base/virtualsocketserver.cc +++ b/webrtc/base/virtualsocketserver.cc @@ -51,6 +51,7 @@ const int NUM_SAMPLES = 1000; enum { MSG_ID_PACKET, + MSG_ID_ADDRESS_BOUND, MSG_ID_CONNECT, MSG_ID_DISCONNECT, }; @@ -130,11 +131,14 @@ SocketAddress VirtualSocket::GetRemoteAddress() const { return remote_addr_; } -// Used by server sockets to set the local address without binding. void VirtualSocket::SetLocalAddress(const SocketAddress& addr) { 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; @@ -148,6 +152,9 @@ int VirtualSocket::Bind(const SocketAddress& addr) { } else { bound_ = true; was_any_ = addr.IsAnyIP(); + // Post a message here such that test case could have chance to + // process the local address. (i.e. SetAlternativeLocalAddress). + server_->msg_queue_->Post(this, MSG_ID_ADDRESS_BOUND); } return result; } @@ -411,6 +418,8 @@ void VirtualSocket::OnMessage(Message* pmsg) { SignalCloseEvent(this, error); } } + } else if (pmsg->message_id == MSG_ID_ADDRESS_BOUND) { + SignalAddressReady(this, GetLocalAddress()); } else { ASSERT(false); } diff --git a/webrtc/base/virtualsocketserver.h b/webrtc/base/virtualsocketserver.h index cecce924c..96df9415f 100644 --- a/webrtc/base/virtualsocketserver.h +++ b/webrtc/base/virtualsocketserver.h @@ -246,6 +246,11 @@ class VirtualSocket : public AsyncSocket, public MessageHandler { // Used by server sockets to set the local address without binding. void SetLocalAddress(const SocketAddress& addr); + // 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); + virtual int Bind(const SocketAddress& addr); virtual int Connect(const SocketAddress& addr); virtual int Close(); diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index e564db346..8994aa895 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -309,11 +309,24 @@ void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) { // 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. + + // Further, to workaround issue 3927 in which a proxy is forcing TCP bound to + // localhost only, we're allowing Loopback IP even if it's not the same as the + // local Turn port. if (socket->GetLocalAddress().ipaddr() != ip()) { - LOG(LS_WARNING) << "Socket is bound to a different address then the " - << "local port. Discarding TURN port."; - OnAllocateError(); - return; + 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() + << ". Still allowing it since it's localhost."; + } 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."; + OnAllocateError(); + return; + } } if (server_address_.address.IsUnresolved()) { diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 4b524d5b0..748ed9d20 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -86,6 +86,14 @@ static int GetFDCount() { } #endif +class TurnPortTestVirtualSocketServer : public rtc::VirtualSocketServer { + public: + explicit TurnPortTestVirtualSocketServer(SocketServer* ss) + : VirtualSocketServer(ss) {} + + using rtc::VirtualSocketServer::LookupBinding; +}; + class TurnPortTest : public testing::Test, public sigslot::has_slots<>, public rtc::MessageHandler { @@ -93,7 +101,7 @@ class TurnPortTest : public testing::Test, TurnPortTest() : main_(rtc::Thread::Current()), pss_(new rtc::PhysicalSocketServer), - ss_(new rtc::VirtualSocketServer(pss_.get())), + ss_(new TurnPortTestVirtualSocketServer(pss_.get())), ss_scope_(ss_.get()), network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32), socket_factory_(rtc::Thread::Current()), @@ -113,6 +121,21 @@ class TurnPortTest : public testing::Test, 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; } @@ -312,7 +335,7 @@ class TurnPortTest : public testing::Test, protected: rtc::Thread* main_; rtc::scoped_ptr pss_; - rtc::scoped_ptr ss_; + rtc::scoped_ptr ss_; rtc::SocketServerScope ss_scope_; rtc::Network network_; rtc::BasicPacketSocketFactory socket_factory_; @@ -356,6 +379,22 @@ TEST_F(TurnPortTest, TestTurnTcpAllocate) { EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); } +// Test case for WebRTC issue 3927 where a proxy binds to the local host address +// instead the address that TurnPort originally bound to. The candidate pair +// impacted by this behavior should still be used. +TEST_F(TurnPortTest, TestTurnTcpAllocationWhenProxyChangesAddressToLocalHost) { + turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP); + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr); + EXPECT_EQ(0, turn_port_->SetOption(rtc::Socket::OPT_SNDBUF, 10 * 1024)); + turn_port_->PrepareAddress(); + ConnectSignalAddressReadyToSetLocalhostAsAltenertativeLocalAddress(); + EXPECT_TRUE_WAIT(turn_ready_, kTimeout); + 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()); +} + // Testing turn port will attempt to create TCP socket on address resolution // failure. TEST_F(TurnPortTest, DISABLED_TestTurnTcpOnAddressResolveFailure) {