diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 0420535cce..98b0b8dae5 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -292,7 +292,7 @@ class P2PTransportChannelTestBase : public testing::Test, uint64_t tiebreaker_; bool role_conflict_; bool save_candidates_; - std::vector saved_candidates_; + std::vector> saved_candidates_; bool ready_to_send_ = false; }; @@ -369,6 +369,8 @@ class P2PTransportChannelTestBase : public testing::Test, P2PTransportChannel* ep2_ch1() { return ep2_.cd1_.ch_.get(); } P2PTransportChannel* ep2_ch2() { return ep2_.cd2_.ch_.get(); } + TestTurnServer* test_turn_server() { return &turn_server_; } + // Common results. static const Result kLocalUdpToLocalUdp; static const Result kLocalUdpToStunUdp; @@ -675,7 +677,8 @@ class P2PTransportChannelTestBase : public testing::Test, return; if (GetEndpoint(ch)->save_candidates_) { - GetEndpoint(ch)->saved_candidates_.push_back(new CandidatesData(ch, c)); + GetEndpoint(ch)->saved_candidates_.push_back( + std::unique_ptr(new CandidatesData(ch, c))); } else { main_->Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES, new CandidatesData(ch, c)); @@ -712,9 +715,8 @@ class P2PTransportChannelTestBase : public testing::Test, void ResumeCandidates(int endpoint) { Endpoint* ed = GetEndpoint(endpoint); - std::vector::iterator it = ed->saved_candidates_.begin(); - for (; it != ed->saved_candidates_.end(); ++it) { - main_->Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES, *it); + for (auto& candidate : ed->saved_candidates_) { + main_->Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES, candidate.release()); } ed->saved_candidates_.clear(); ed->save_candidates_ = false; @@ -1665,7 +1667,7 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) { } // Test that when the "presume_writable_when_fully_relayed" flag is set to -// false and there's a TURN-TURN candidate pair, it's presume to be writable +// true and there's a TURN-TURN candidate pair, it's presumed to be writable // as soon as it's created. TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) { ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, @@ -1698,6 +1700,50 @@ TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) { EXPECT_TRUE(GetEndpoint(0)->ready_to_send_); } +// Test that a TURN/peer reflexive candidate pair is also presumed writable. +TEST_F(P2PTransportChannelTest, TurnToPrflxPresumedWritable) { + rtc::ScopedFakeClock fake_clock; + + ConfigureEndpoints(NAT_SYMMETRIC, NAT_SYMMETRIC, kDefaultPortAllocatorFlags, + kDefaultPortAllocatorFlags); + // We want the remote TURN candidate to show up as prflx. To do this we need + // to configure the server to accept packets from an address we haven't + // explicitly installed permission for. + test_turn_server()->set_enable_permission_checks(false); + IceConfig config; + config.presume_writable_when_fully_relayed = true; + GetEndpoint(0)->cd1_.ch_.reset( + CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[0], + kIcePwd[0], kIceUfrag[1], kIcePwd[1])); + GetEndpoint(1)->cd1_.ch_.reset( + CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[1], + kIcePwd[1], kIceUfrag[0], kIcePwd[0])); + ep1_ch1()->SetIceConfig(config); + ep2_ch1()->SetIceConfig(config); + // Don't signal candidates from channel 2, so that channel 1 sees the TURN + // candidate as peer reflexive. + PauseCandidates(1); + ep1_ch1()->MaybeStartGathering(); + ep2_ch1()->MaybeStartGathering(); + + // Wait for the TURN<->prflx connection. + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable(), + 1000, fake_clock); + ASSERT_NE(nullptr, ep1_ch1()->selected_connection()); + EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep1_ch1())->type()); + EXPECT_EQ(PRFLX_PORT_TYPE, RemoteCandidate(ep1_ch1())->type()); + // Make sure that at this point the connection is only presumed writable, + // not fully writable. + EXPECT_FALSE(ep1_ch1()->selected_connection()->writable()); + + // Now wait for it to actually become writable. + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection()->writable(), 1000, + fake_clock); + + // Explitly destroy channels, before fake clock is destroyed. + DestroyChannels(); +} + // Test that a presumed-writable TURN<->TURN connection is preferred above an // unreliable connection (one that has failed to be pinged for some time). TEST_F(P2PTransportChannelTest, PresumedWritablePreferredOverUnreliable) { diff --git a/webrtc/p2p/base/testturnserver.h b/webrtc/p2p/base/testturnserver.h index 8660ae1ff2..3d6074dc9b 100644 --- a/webrtc/p2p/base/testturnserver.h +++ b/webrtc/p2p/base/testturnserver.h @@ -71,6 +71,10 @@ class TestTurnServer : public TurnAuthInterface { server_.set_redirect_hook(redirect_hook); } + void set_enable_permission_checks(bool enable) { + server_.set_enable_permission_checks(enable); + } + void AddInternalSocket(const rtc::SocketAddress& int_addr, ProtocolType proto) { rtc::Thread* thread = rtc::Thread::Current(); diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 824a25e644..567916f4ba 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -831,13 +831,13 @@ void TurnPort::HandleDataIndication(const char* data, size_t size, return; } - // Verify that the data came from somewhere we think we have a permission for. + // Log a warning if the data didn't come from an address that we think we have + // a permission for. rtc::SocketAddress ext_addr(addr_attr->GetAddress()); if (!HasPermission(ext_addr.ipaddr())) { - LOG_J(LS_WARNING, this) << "Received TURN data indication with invalid " - << "peer address, addr=" - << ext_addr.ToSensitiveString(); - return; + LOG_J(LS_WARNING, this) + << "Received TURN data indication with unknown " + << "peer address, addr=" << ext_addr.ToSensitiveString(); } DispatchPacket(data_attr->bytes(), data_attr->length(), ext_addr, diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 2fa5525718..077ca4d7f1 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -460,19 +460,17 @@ class TurnPortTest : public testing::Test, EXPECT_TRUE_WAIT(turn_unknown_address_, kTimeout); // Flush all requests in the invoker to destroy the TurnEntry. - // Now the turn port cannot receive the ping. + // However, the TURN port should still signal the unknown address. turn_unknown_address_ = false; turn_port_->invoker()->Flush(rtc::Thread::Current()); conn1->Ping(0); - rtc::Thread::Current()->ProcessMessages(500); - EXPECT_FALSE(turn_unknown_address_); + EXPECT_TRUE_WAIT(turn_unknown_address_, kTimeout); // If the connection is created again, it will start to receive pings. conn2 = turn_port_->CreateConnection(udp_port_->Candidates()[0], Port::ORIGIN_MESSAGE); conn1->Ping(0); EXPECT_TRUE_WAIT(conn2->receiving(), kTimeout); - EXPECT_FALSE(turn_unknown_address_); } void TestTurnSendData() { diff --git a/webrtc/p2p/base/turnserver.cc b/webrtc/p2p/base/turnserver.cc index 3e9d66910a..0b06c44ab0 100644 --- a/webrtc/p2p/base/turnserver.cc +++ b/webrtc/p2p/base/turnserver.cc @@ -801,7 +801,8 @@ void TurnServerAllocation::OnExternalPacket( buf.WriteUInt16(static_cast(size)); buf.WriteBytes(data, size); server_->Send(&conn_, buf); - } else if (HasPermission(addr.ipaddr())) { + } else if (!server_->enable_permission_checks_ || + HasPermission(addr.ipaddr())) { // No channel, but a permission exists. Send as a data indication. TurnMessage msg; msg.SetType(TURN_DATA_INDICATION); diff --git a/webrtc/p2p/base/turnserver.h b/webrtc/p2p/base/turnserver.h index 2bc3650f09..ed281b46f0 100644 --- a/webrtc/p2p/base/turnserver.h +++ b/webrtc/p2p/base/turnserver.h @@ -190,6 +190,10 @@ class TurnServer : public sigslot::has_slots<> { reject_private_addresses_ = filter; } + void set_enable_permission_checks(bool enable) { + enable_permission_checks_ = enable; + } + // Starts listening for packets from internal clients. void AddInternalSocket(rtc::AsyncPacketSocket* socket, ProtocolType proto); @@ -268,6 +272,8 @@ class TurnServer : public sigslot::has_slots<> { // sees the same nonce in next transaction. bool enable_otu_nonce_; bool reject_private_addresses_ = false; + // Check for permission when receiving an external packet. + bool enable_permission_checks_ = true; InternalSocketMap server_sockets_; ServerSocketMap server_listen_sockets_;