From 6705012904e6cbbefce6fbce0a3f615b7aeafd8f Mon Sep 17 00:00:00 2001 From: Honghai Zhang Date: Thu, 12 May 2016 09:27:53 -0700 Subject: [PATCH] This fixes an issue similar to https://bugs.chromium.org/p/webrtc/issues/detail?id=3927 where the localhost IP does not match the turn port address. The issue here is in the TCP port. BUG= R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1914803002 . Cr-Commit-Position: refs/heads/master@{#12707} --- webrtc/base/virtualsocketserver.cc | 4 +- webrtc/base/virtualsocketserver.h | 3 + webrtc/p2p/base/tcpport.cc | 39 +++++++------ webrtc/p2p/base/tcpport_unittest.cc | 85 +++++++++++++++++++++++++++++ webrtc/p2p/p2p.gyp | 1 + 5 files changed, 115 insertions(+), 17 deletions(-) create mode 100644 webrtc/p2p/base/tcpport_unittest.cc diff --git a/webrtc/base/virtualsocketserver.cc b/webrtc/base/virtualsocketserver.cc index c6952b4b99..c76fe42f1e 100644 --- a/webrtc/base/virtualsocketserver.cc +++ b/webrtc/base/virtualsocketserver.cc @@ -578,7 +578,9 @@ AsyncSocket* VirtualSocketServer::CreateAsyncSocket(int family, int type) { } VirtualSocket* VirtualSocketServer::CreateSocketInternal(int family, int type) { - return new VirtualSocket(this, family, type, true); + VirtualSocket* socket = new VirtualSocket(this, family, type, true); + SignalSocketCreated(socket); + return socket; } void VirtualSocketServer::SetMessageQueue(MessageQueue* msg_queue) { diff --git a/webrtc/base/virtualsocketserver.h b/webrtc/base/virtualsocketserver.h index a83be28522..897ba9e5eb 100644 --- a/webrtc/base/virtualsocketserver.h +++ b/webrtc/base/virtualsocketserver.h @@ -122,6 +122,9 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> { bool CloseTcpConnections(const SocketAddress& addr_local, const SocketAddress& addr_remote); + // For testing purpose only. Fired when a client socket is created. + sigslot::signal1 SignalSocketCreated; + protected: // Returns a new IP not used before in this network. IPAddress GetNextIP(int family); diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 59c4f317d4..5ccb8108a0 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -387,28 +387,35 @@ void TCPConnection::OnConnect(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. - const rtc::IPAddress& socket_ip = socket->GetLocalAddress().ipaddr(); - if (socket_ip == port()->ip() || IPIsAny(port()->ip())) { - if (socket_ip == port()->ip()) { - LOG_J(LS_VERBOSE, this) << "Connection established to " - << socket->GetRemoteAddress().ToSensitiveString(); - } else { - LOG(LS_WARNING) << "Socket is bound to a different address:" - << socket->GetLocalAddress().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."; - } - set_connected(true); - connection_pending_ = false; + const rtc::SocketAddress& socket_addr = socket->GetLocalAddress(); + if (socket_addr.ipaddr() == port()->ip()) { + 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_ip.ToSensitiveString() + << socket_addr.ipaddr().ToSensitiveString() << ", different from the local candidate IP " << port()->ip().ToSensitiveString(); OnClose(socket, 0); + return; } + + // Connection is established successfully. + set_connected(true); + connection_pending_ = false; } void TCPConnection::OnClose(rtc::AsyncPacketSocket* socket, int error) { diff --git a/webrtc/p2p/base/tcpport_unittest.cc b/webrtc/p2p/base/tcpport_unittest.cc new file mode 100644 index 0000000000..77fb790c53 --- /dev/null +++ b/webrtc/p2p/base/tcpport_unittest.cc @@ -0,0 +1,85 @@ +/* + * Copyright 2016 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/base/gunit.h" +#include "webrtc/base/physicalsocketserver.h" +#include "webrtc/base/thread.h" +#include "webrtc/base/virtualsocketserver.h" +#include "webrtc/p2p/base/basicpacketsocketfactory.h" +#include "webrtc/p2p/base/tcpport.h" + +using rtc::SocketAddress; +using cricket::Connection; +using cricket::Port; +using cricket::TCPPort; +using cricket::ICE_UFRAG_LENGTH; +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); + +class TCPPortTest : public testing::Test, public sigslot::has_slots<> { + public: + TCPPortTest() + : main_(rtc::Thread::Current()), + pss_(new rtc::PhysicalSocketServer), + ss_(new rtc::VirtualSocketServer(pss_.get())), + ss_scope_(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); + } + + void OnSocketCreated(rtc::VirtualSocket* socket) { + LOG(LS_INFO) << "socket created "; + socket->SignalAddressReady.connect( + this, &TCPPortTest::SetLocalhostAsAlternativeLocalAddress); + } + + 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); + } + + protected: + rtc::Thread* main_; + rtc::scoped_ptr pss_; + rtc::scoped_ptr ss_; + rtc::SocketServerScope ss_scope_; + rtc::Network network_; + rtc::BasicPacketSocketFactory socket_factory_; + std::string username_; + std::string password_; +}; + +TEST_F(TCPPortTest, TestTCPPortWithLocalhostAddress) { + rtc::scoped_ptr lport(CreateTCPPort(kLocalAddr)); + rtc::scoped_ptr 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); + EXPECT_TRUE_WAIT(conn->connected(), kTimeout); +} diff --git a/webrtc/p2p/p2p.gyp b/webrtc/p2p/p2p.gyp index a15fa52bb5..08fac6929d 100644 --- a/webrtc/p2p/p2p.gyp +++ b/webrtc/p2p/p2p.gyp @@ -162,6 +162,7 @@ 'base/transport_unittest.cc', 'base/transportcontroller_unittest.cc', 'base/transportdescriptionfactory_unittest.cc', + 'base/tcpport_unittest.cc', 'base/turnport_unittest.cc', 'client/fakeportallocator.h', 'client/portallocator_unittest.cc',