From ef184702f696c6cc9d5e704d03b5a78d83440d78 Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Thu, 23 Jun 2016 17:35:47 -0700 Subject: [PATCH] Allow receiving a packet on a TURN port from an unknown address. This may occur if the TURN server allows the packet through due to its policies around CreatePermission requirements, but the candidate has not yet been signaled. R=honghaiz@webrtc.org, pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/2086203004 . Cr-Commit-Position: refs/heads/master@{#13278} --- .../p2p/base/p2ptransportchannel_unittest.cc | 58 +++++++++++++++++-- webrtc/p2p/base/testturnserver.h | 4 ++ webrtc/p2p/base/turnport.cc | 10 ++-- webrtc/p2p/base/turnport_unittest.cc | 6 +- webrtc/p2p/base/turnserver.cc | 3 +- webrtc/p2p/base/turnserver.h | 6 ++ 6 files changed, 71 insertions(+), 16 deletions(-) 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_;