zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix a bug caused by an early return when a TURN port receives a role

Browse Source 
conflict.


A role conflict received from an unknown address (peer reflexive
candidate) results in an early return before signaling the unknown
address to P2PTransportChannel. Without this signal, there is no
candidate pair or TURN entry created, and sending the error response
when handling the role conflict fails.

Bug: webrtc:9034
Change-Id: I0f1b232a574449e98025618d93aac8a91b30e14b
Reviewed-on: https://webrtc-review.googlesource.com/63840
Commit-Queue: Qingsi Wang <qingsi@google.com>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22588}
This commit is contained in:
Qingsi Wang
2018-03-23 14:28:37 -07:00
committed by Commit Bot
parent 2b85792b01
commit 2bd41f9e0e
2 changed files with 62 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
p2p/base/p2ptransportchannel_unittest.cc
View File

@ -1488,14 +1488,14 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {
ASSERT_TRUE_WAIT(
(selected_connection = ep1_ch1()->selected_connection()) != nullptr,
kMediumTimeout);
EXPECT_EQ("prflx", selected_connection->remote_candidate().type());
EXPECT_EQ(PRFLX_PORT_TYPE, selected_connection->remote_candidate().type());
EXPECT_EQ(kIceUfrag[1], selected_connection->remote_candidate().username());
EXPECT_EQ(kIcePwd[1], selected_connection->remote_candidate().password());
EXPECT_EQ(1u, selected_connection->remote_candidate().generation());
ResumeCandidates(1);
// Verify ep1's selected connection is updated to use the 'local' candidate.
EXPECT_EQ_WAIT("local",
EXPECT_EQ_WAIT(LOCAL_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type(),
kMediumTimeout);
EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection());
@ -1530,14 +1530,14 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {
ASSERT_TRUE_WAIT(
(selected_connection = ep1_ch1()->selected_connection()) != nullptr,
kMediumTimeout);
EXPECT_EQ("prflx", selected_connection->remote_candidate().type());
EXPECT_EQ(PRFLX_PORT_TYPE, selected_connection->remote_candidate().type());
EXPECT_EQ(kIceUfrag[1], selected_connection->remote_candidate().username());
EXPECT_EQ(kIcePwd[1], selected_connection->remote_candidate().password());
EXPECT_EQ(1u, selected_connection->remote_candidate().generation());
ResumeCandidates(1);
EXPECT_EQ_WAIT("prflx",
EXPECT_EQ_WAIT(PRFLX_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type(),
kMediumTimeout);
EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection());
@ -1581,7 +1581,7 @@ TEST_F(P2PTransportChannelTest,
// The caller should have the selected connection connected to the peer
// reflexive candidate.
EXPECT_EQ_WAIT("prflx",
EXPECT_EQ_WAIT(PRFLX_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type(),
kDefaultTimeout);
const Connection* prflx_selected_connection =
@ -1597,7 +1597,7 @@ TEST_F(P2PTransportChannelTest,
// their information to update the peer reflexive candidate.
ResumeCandidates(1);
EXPECT_EQ_WAIT("relay",
EXPECT_EQ_WAIT(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type(),
kDefaultTimeout);
EXPECT_EQ(prflx_selected_connection, ep1_ch1()->selected_connection());
@ -1875,10 +1875,10 @@ TEST_F(P2PTransportChannelTest, TestForceTurn) {
EXPECT_TRUE(ep1_ch1()->selected_connection() &&
ep2_ch1()->selected_connection());
EXPECT_EQ("relay", RemoteCandidate(ep1_ch1())->type());
EXPECT_EQ("relay", LocalCandidate(ep1_ch1())->type());
EXPECT_EQ("relay", RemoteCandidate(ep2_ch1())->type());
EXPECT_EQ("relay", LocalCandidate(ep2_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, RemoteCandidate(ep1_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep1_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, RemoteCandidate(ep2_ch1())->type());
EXPECT_EQ(RELAY_PORT_TYPE, LocalCandidate(ep2_ch1())->type());
TestSendRecv(&clock);
DestroyChannels();
@ -2173,6 +2173,50 @@ TEST_F(P2PTransportChannelTest, SignalReadyToSendWithPresumedWritable) {
EXPECT_EQ(len, SendData(ep1_ch1(), data, len));
}
// Test that role conflict error responses are sent as expected when receiving a
// ping from an unknown address over a TURN connection. Regression test for
// crbug.com/webrtc/9034.
TEST_F(P2PTransportChannelTest,
TurnToPrflxSelectedAfterResolvingIceControllingRoleConflict) {
rtc::ScopedFakeClock clock;
// Gather only relay candidates.
ConfigureEndpoints(NAT_SYMMETRIC, NAT_SYMMETRIC,
kDefaultPortAllocatorFlags | PORTALLOCATOR_DISABLE_UDP |
PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP,
kDefaultPortAllocatorFlags | PORTALLOCATOR_DISABLE_UDP |
PORTALLOCATOR_DISABLE_STUN |
PORTALLOCATOR_DISABLE_TCP);
// With conflicting ICE roles, endpoint 1 has the higher tie breaker and will
// send a binding error response.
SetIceRole(0, ICEROLE_CONTROLLING);
SetIceTiebreaker(0, kHighTiebreaker);
SetIceRole(1, ICEROLE_CONTROLLING);
SetIceTiebreaker(1, kLowTiebreaker);
// 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);
GetEndpoint(0)->cd1_.ch_.reset(CreateChannel(
0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1]));
GetEndpoint(1)->cd1_.ch_.reset(CreateChannel(
1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[1], kIceParams[0]));
// 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();
EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable(),
kMediumTimeout, 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());
DestroyChannels();
}
// Test what happens when we have 2 users behind the same NAT. This can lead
// to interesting behavior because the STUN server will only give out the
// address of the outermost NAT.

15
p2p/base/port.cc
View File

@ -465,14 +465,15 @@ void Port::OnReadPacket(
RTC_LOG(LS_INFO) << "Received STUN ping "
<< " id=" << rtc::hex_encode(msg->transaction_id())
<< " from unknown address " << addr.ToSensitiveString();
// We need to signal an unknown address before we handle any role conflict
// below. Otherwise there would be no candidate pair and TURN entry created
// to send the error response in case of a role conflict.
SignalUnknownAddress(this, addr, proto, msg.get(), remote_username, false);
// Check for role conflicts.
if (!MaybeIceRoleConflict(addr, msg.get(), remote_username)) {
RTC_LOG(LS_INFO) << "Received conflicting role from the peer.";
return;
}
SignalUnknownAddress(this, addr, proto, msg.get(), remote_username, false);
} else {
// NOTE(tschmelcher): STUN_BINDING_RESPONSE is benign. It occurs if we
// pruned a connection for this port while it had STUN requests in flight,
@ -1606,10 +1607,10 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request,
StunMessage* response) {
int error_code = response->GetErrorCodeValue();
LOG_J(LS_INFO, this) << "Received STUN error response"
<< " id=" << rtc::hex_encode(request->id())
<< " code=" << error_code
<< " rtt=" << request->Elapsed();
LOG_J(LS_WARNING, this) << "Received STUN error response"
<< " id=" << rtc::hex_encode(request->id())
<< " code=" << error_code
<< " rtt=" << request->Elapsed();
if (error_code == STUN_ERROR_UNKNOWN_ATTRIBUTE ||
error_code == STUN_ERROR_SERVER_ERROR ||