Send back ping response if the ping comes from an unknown address.
BUG=webrtc:5171
Review URL: https://codereview.webrtc.org/1424703012
Cr-Commit-Position: refs/heads/master@{#10610}
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 3ac291f..a39ad93 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -605,14 +605,7 @@
<< (remote_candidate_is_new ? "peer reflexive" : "resurrected")
<< " candidate: " << remote_candidate.ToString();
AddConnection(connection);
- connection->ReceivedPing();
-
- bool received_use_candidate =
- stun_msg->GetByteString(STUN_ATTR_USE_CANDIDATE) != nullptr;
- if (received_use_candidate && ice_role_ == ICEROLE_CONTROLLED) {
- connection->set_nominated(true);
- OnNominated(connection);
- }
+ connection->HandleBindingRequest(stun_msg);
// Update the list of connections since we just added another. We do this
// after sending the response since it could (in principle) delete the
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index c5c21ac..8ec6474 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -1932,7 +1932,8 @@
// The controlled side will select a connection as the "best connection" based
// on requests from an unknown address before the controlling side nominates
// a connection, and will nominate a connection from an unknown address if the
-// request contains the use_candidate attribute.
+// request contains the use_candidate attribute. Plus, it will also sends back
+// a ping response.
TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa);
@@ -1948,14 +1949,16 @@
uint32_t prflx_priority = cricket::ICE_TYPE_PREFERENCE_PRFLX << 24;
request.AddAttribute(new cricket::StunUInt32Attribute(
cricket::STUN_ATTR_PRIORITY, prflx_priority));
- cricket::Port* port = GetPort(&ch);
+ cricket::TestUDPPort* port = static_cast<cricket::TestUDPPort*>(GetPort(&ch));
port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1),
cricket::PROTO_UDP, &request, kIceUfrag[1], false);
cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
ASSERT_TRUE(conn1 != nullptr);
+ EXPECT_TRUE(port->sent_binding_response());
EXPECT_EQ(conn1, ch.best_connection());
conn1->ReceivedPingResponse();
EXPECT_EQ(conn1, ch.best_connection());
+ port->set_sent_binding_response(false);
// Another connection is nominated via use_candidate.
ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
@@ -1977,8 +1980,10 @@
cricket::PROTO_UDP, &request, kIceUfrag[1], false);
cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
ASSERT_TRUE(conn3 != nullptr);
+ EXPECT_TRUE(port->sent_binding_response());
conn3->ReceivedPingResponse(); // Become writable.
EXPECT_EQ(conn2, ch.best_connection());
+ port->set_sent_binding_response(false);
// However if the request contains use_candidate attribute, it will be
// selected as the best connection.
@@ -1988,6 +1993,7 @@
cricket::PROTO_UDP, &request, kIceUfrag[1], false);
cricket::Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);
ASSERT_TRUE(conn4 != nullptr);
+ EXPECT_TRUE(port->sent_binding_response());
// conn4 is not the best connection yet because it is not writable.
EXPECT_EQ(conn2, ch.best_connection());
conn4->ReceivedPingResponse(); // Become writable.
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index 69dff94..f04e619 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -567,10 +567,6 @@
response.AddMessageIntegrity(password_);
response.AddFingerprint();
- // The fact that we received a successful request means that this connection
- // (if one exists) should now be receiving.
- Connection* conn = GetConnection(addr);
-
// Send the response message.
rtc::ByteBuffer buf;
response.Write(&buf);
@@ -585,6 +581,7 @@
} else {
// Log at LS_INFO if we send a stun ping response on an unwritable
// connection.
+ Connection* conn = GetConnection(addr);
rtc::LoggingSeverity sev = (conn && !conn->writable()) ?
rtc::LS_INFO : rtc::LS_VERBOSE;
LOG_JV(sev, this)
@@ -592,10 +589,6 @@
<< ", to=" << addr.ToSensitiveString()
<< ", id=" << rtc::hex_encode(response.transaction_id());
}
-
- ASSERT(conn != NULL);
- if (conn)
- conn->ReceivedPing();
}
void Port::SendBindingErrorResponse(StunMessage* request,
@@ -924,29 +917,7 @@
<< ", id=" << rtc::hex_encode(msg->transaction_id());
if (remote_ufrag == remote_candidate_.username()) {
- // Check for role conflicts.
- if (!port_->MaybeIceRoleConflict(addr, msg.get(), remote_ufrag)) {
- // Received conflicting role from the peer.
- LOG(LS_INFO) << "Received conflicting role from the peer.";
- return;
- }
-
- // Incoming, validated stun request from remote peer.
- // This call will also set the connection receiving.
- port_->SendBindingResponse(msg.get(), addr);
-
- // If timed out sending writability checks, start up again
- if (!pruned_ && (write_state_ == STATE_WRITE_TIMEOUT))
- set_write_state(STATE_WRITE_INIT);
-
- if (port_->GetIceRole() == ICEROLE_CONTROLLED) {
- const StunByteStringAttribute* use_candidate_attr =
- msg->GetByteString(STUN_ATTR_USE_CANDIDATE);
- if (use_candidate_attr) {
- set_nominated(true);
- SignalNominated(this);
- }
- }
+ HandleBindingRequest(msg.get());
} else {
// The packet had the right local username, but the remote username
// was not the right one for the remote address.
@@ -986,6 +957,37 @@
}
}
+void Connection::HandleBindingRequest(IceMessage* msg) {
+ // This connection should now be receiving.
+ ReceivedPing();
+
+ const rtc::SocketAddress& remote_addr = remote_candidate_.address();
+ const std::string& remote_ufrag = remote_candidate_.username();
+ // Check for role conflicts.
+ if (!port_->MaybeIceRoleConflict(remote_addr, msg, remote_ufrag)) {
+ // Received conflicting role from the peer.
+ LOG(LS_INFO) << "Received conflicting role from the peer.";
+ return;
+ }
+
+ // This is a validated stun request from remote peer.
+ port_->SendBindingResponse(msg, remote_addr);
+
+ // If it timed out on writing check, start up again
+ if (!pruned_ && write_state_ == STATE_WRITE_TIMEOUT) {
+ set_write_state(STATE_WRITE_INIT);
+ }
+
+ if (port_->GetIceRole() == ICEROLE_CONTROLLED) {
+ const StunByteStringAttribute* use_candidate_attr =
+ msg->GetByteString(STUN_ATTR_USE_CANDIDATE);
+ if (use_candidate_attr) {
+ set_nominated(true);
+ SignalNominated(this);
+ }
+ }
+}
+
void Connection::OnReadyToSend() {
if (write_state_ == STATE_WRITABLE) {
SignalReadyToSend(this);
diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index 01c45f2..2f69f49 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -523,6 +523,8 @@
// public because the connection intercepts the first ping for us.
uint32_t last_ping_received() const { return last_ping_received_; }
void ReceivedPing();
+ // Handles the binding request; sends a response if this is a valid request.
+ void HandleBindingRequest(IceMessage* msg);
// Debugging description of this connection
std::string ToDebugId() const;
diff --git a/webrtc/p2p/client/fakeportallocator.h b/webrtc/p2p/client/fakeportallocator.h
index d59333a..d9af4b3 100644
--- a/webrtc/p2p/client/fakeportallocator.h
+++ b/webrtc/p2p/client/fakeportallocator.h
@@ -24,6 +24,62 @@
namespace cricket {
+class TestUDPPort : public UDPPort {
+ public:
+ 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);
+ if (!port->Init()) {
+ delete port;
+ port = nullptr;
+ }
+ return port;
+ }
+ void SendBindingResponse(StunMessage* request,
+ const rtc::SocketAddress& addr) override {
+ UDPPort::SendBindingResponse(request, addr);
+ sent_binding_response_ = true;
+ }
+ bool sent_binding_response() { return sent_binding_response_; }
+ void set_sent_binding_response(bool response) {
+ sent_binding_response_ = response;
+ }
+
+ protected:
+ 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,
+ const std::string& password,
+ const std::string& origin,
+ bool emit_localhost_for_anyaddress)
+ : UDPPort(thread,
+ factory,
+ network,
+ ip,
+ min_port,
+ max_port,
+ username,
+ password,
+ origin,
+ emit_localhost_for_anyaddress) {}
+
+ bool sent_binding_response_ = false;
+};
+
class FakePortAllocatorSession : public PortAllocatorSession {
public:
FakePortAllocatorSession(rtc::Thread* worker_thread,
@@ -45,16 +101,9 @@
virtual void StartGettingPorts() {
if (!port_) {
- port_.reset(cricket::UDPPort::Create(worker_thread_,
- factory_,
- &network_,
- network_.GetBestIP(),
- 0,
- 0,
- username(),
- password(),
- std::string(),
- false));
+ port_.reset(TestUDPPort::Create(worker_thread_, factory_, &network_,
+ network_.GetBestIP(), 0, 0, username(),
+ password(), std::string(), false));
AddPort(port_.get());
}
++port_config_count_;
