diff --git a/p2p/base/connection.h b/p2p/base/connection.h index 4be8483f9a..c0a19b510f 100644 --- a/p2p/base/connection.h +++ b/p2p/base/connection.h @@ -316,10 +316,22 @@ class Connection : public CandidatePairInterface { Port* PortForTest() { return port_.get(); } const Port* PortForTest() const { return port_.get(); } + std::unique_ptr BuildPingRequestForTest() { + RTC_DCHECK_RUN_ON(network_thread_); + return BuildPingRequest(); + } + // Public for unit tests. uint32_t acked_nomination() const; void set_remote_nomination(uint32_t remote_nomination); + const std::string& remote_password_for_test() const { + return remote_candidate().password(); + } + void set_remote_password_for_test(absl::string_view pwd) { + remote_candidate_.set_password(pwd); + } + protected: // A ConnectionRequest is a simple STUN ping used to determine writability. class ConnectionRequest; diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 3879ceb63a..33662a95ba 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -687,6 +687,16 @@ bool TurnPort::CanHandleIncomingPacketsFrom( return server_address_.address == addr; } +void TurnPort::SendBindingErrorResponse(StunMessage* message, + const rtc::SocketAddress& addr, + int error_code, + absl::string_view reason) { + if (!GetConnection(addr)) + return; + + Port::SendBindingErrorResponse(message, addr, error_code, reason); +} + bool TurnPort::HandleIncomingPacket(rtc::AsyncPacketSocket* socket, const char* data, size_t size, diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 0672931d1c..ac660d6599 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -150,6 +150,14 @@ class TurnPort : public Port { int64_t packet_time_us) override; bool CanHandleIncomingPacketsFrom( const rtc::SocketAddress& addr) const override; + + // Checks if a connection exists for `addr` before forwarding the call to + // the base class. + void SendBindingErrorResponse(StunMessage* message, + const rtc::SocketAddress& addr, + int error_code, + absl::string_view reason) override; + virtual void OnReadPacket(rtc::AsyncPacketSocket* socket, const char* data, size_t size, diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 3f01fbef1b..d63dd4a75c 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -683,6 +683,30 @@ class TurnPortTest : public ::testing::Test, // unknown address. turn_unknown_address_ = false; fake_clock_.AdvanceTime(webrtc::TimeDelta::Seconds(5 * 60)); + + // TODO(chromium:1395625): When `TurnPort` doesn't find connection objects + // for incoming packets, it forwards calls to the parent class, `Port`. This + // happens inside `TurnPort::DispatchPacket`. The `Port` implementation may + // need to send a binding error back over a connection which, unless the + // `TurnPort` implementation handles it, could result in a null deref. + // This special check tests if dispatching messages via `TurnPort` for which + // there's no connection, results in a no-op rather than crashing. + // See `TurnPort::SendBindingErrorResponse` for the check. + // This should probably be done in a neater way both from a testing pov and + // how incoming messages are handled in the `Port` class, when an assumption + // is made about connection objects existing and when those assumptions + // may not hold. + std::string pwd = conn1->remote_password_for_test(); + conn1->set_remote_password_for_test("bad"); + auto msg = conn1->BuildPingRequestForTest(); + + rtc::ByteBufferWriter buf; + msg->Write(&buf); + conn1->Send(buf.Data(), buf.Length(), options); + + // Now restore the password before continuing. + conn1->set_remote_password_for_test(pwd); + conn1->Ping(0); EXPECT_TRUE_SIMULATED_WAIT(turn_unknown_address_, kSimulatedRtt, fake_clock_);