Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1416)

Unified Diff: webrtc/p2p/base/tcpport.cc

Issue 2989303002: Make Port (and subclasses) fully "Network"-based, instead of IP-based. (Closed)
Patch Set: Add back Port constructor that takes IP for backwards compatibility. Created 3 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/p2p/base/tcpport.h ('k') | webrtc/p2p/base/tcpport_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/p2p/base/tcpport.cc
diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc
index 86670cd3668bb56ec5e6b01efb44313080d4ec35..cbb25b91927f9fd5268521db5207dc2bd4c2ab3f 100644
--- a/webrtc/p2p/base/tcpport.cc
+++ b/webrtc/p2p/base/tcpport.cc
@@ -75,7 +75,6 @@ namespace cricket {
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 @@ TCPPort::TCPPort(rtc::Thread* thread,
LOCAL_PORT_TYPE,
factory,
network,
- ip,
min_port,
max_port,
username,
@@ -179,11 +177,15 @@ void TCPPort::PrepareAddress() {
// 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::OnNewConnection(rtc::AsyncPacketSocket* socket,
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 @@ TCPConnection::TCPConnection(TCPPort* port,
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::OnConnectionRequestResponse(ConnectionRequest* req,
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 @@ void TCPConnection::CreateOutgoingTcpSocket() {
? 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()
« no previous file with comments | « webrtc/p2p/base/tcpport.h ('k') | webrtc/p2p/base/tcpport_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698