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}
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 0420535..98b0b8d 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -292,7 +292,7 @@
uint64_t tiebreaker_;
bool role_conflict_;
bool save_candidates_;
- std::vector<CandidatesData*> saved_candidates_;
+ std::vector<std::unique_ptr<CandidatesData>> saved_candidates_;
bool ready_to_send_ = false;
};
@@ -369,6 +369,8 @@
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 @@
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<CandidatesData>(new CandidatesData(ch, c)));
} else {
main_->Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES,
new CandidatesData(ch, c));
@@ -712,9 +715,8 @@
void ResumeCandidates(int endpoint) {
Endpoint* ed = GetEndpoint(endpoint);
- std::vector<CandidatesData*>::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 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 @@
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 8660ae1..3d6074d 100644
--- a/webrtc/p2p/base/testturnserver.h
+++ b/webrtc/p2p/base/testturnserver.h
@@ -71,6 +71,10 @@
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 824a25e..567916f 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -831,13 +831,13 @@
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 2fa5525..077ca4d 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -460,19 +460,17 @@
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 3e9d669..0b06c44 100644
--- a/webrtc/p2p/base/turnserver.cc
+++ b/webrtc/p2p/base/turnserver.cc
@@ -801,7 +801,8 @@
buf.WriteUInt16(static_cast<uint16_t>(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 2bc3650..ed281b4 100644
--- a/webrtc/p2p/base/turnserver.h
+++ b/webrtc/p2p/base/turnserver.h
@@ -190,6 +190,10 @@
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 @@
// 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_;