diff options
author | honghaiz <honghaiz@webrtc.org> | 2015-12-30 13:32:47 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-30 21:32:51 +0000 |
commit | 112fe43b9f3f04592f1ccff6b4f23fcd2890bf30 (patch) | |
tree | a6afeb8bad1514f8367e8cbb2c9832db6141758f /webrtc | |
parent | 051b62014cfa18adf7ca7caf327b0c0882353f22 (diff) | |
download | webrtc-112fe43b9f3f04592f1ccff6b4f23fcd2890bf30.tar.gz |
Fill the remote pwd in the ice candidates when an ICE credential is received.
Also when a STUN ping arrives from an unknown address, try to find the pwd and generation from the remote ICE parameters.
BUG=
Review URL: https://codereview.webrtc.org/1549633004
Cr-Commit-Position: refs/heads/master@{#11144}
Diffstat (limited to 'webrtc')
-rw-r--r-- | webrtc/p2p/base/candidate.h | 1 | ||||
-rw-r--r-- | webrtc/p2p/base/p2ptransportchannel.cc | 61 | ||||
-rw-r--r-- | webrtc/p2p/base/p2ptransportchannel.h | 9 | ||||
-rw-r--r-- | webrtc/p2p/base/p2ptransportchannel_unittest.cc | 25 |
4 files changed, 72 insertions, 24 deletions
diff --git a/webrtc/p2p/base/candidate.h b/webrtc/p2p/base/candidate.h index 3f0ea43cde..ac7acabf05 100644 --- a/webrtc/p2p/base/candidate.h +++ b/webrtc/p2p/base/candidate.h @@ -105,6 +105,7 @@ class Candidate { std::min(prio_val, static_cast<uint64_t>(UINT_MAX))); } + // TODO(honghaiz): Change to usernameFragment or ufrag. const std::string & username() const { return username_; } void set_username(const std::string & username) { username_ = username; } diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 5101bf3411..c58a235f2d 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -354,10 +354,15 @@ void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag, remote_ice_parameters_.push_back(new_ice); } + // Update the pwd of remote candidate if needed. + for (RemoteCandidate& candidate : remote_candidates_) { + if (candidate.username() == ice_ufrag && candidate.password().empty()) { + candidate.set_password(ice_pwd); + } + } // We need to update the credentials for any peer reflexive candidates. - std::vector<Connection*>::iterator it = connections_.begin(); - for (; it != connections_.end(); ++it) { - (*it)->MaybeSetRemoteIceCredentials(ice_ufrag, ice_pwd); + for (Connection* conn : connections_) { + conn->MaybeSetRemoteIceCredentials(ice_ufrag, ice_pwd); } } @@ -525,13 +530,16 @@ void P2PTransportChannel::OnUnknownAddress( } } + uint32_t remote_generation = 0; // The STUN binding request may arrive after setRemoteDescription and before // adding remote candidate, so we need to set the password to the shared // password if the user name matches. if (remote_password.empty()) { - IceParameters* current_ice = remote_ice(); - if (current_ice && remote_username == current_ice->ufrag) { - remote_password = current_ice->pwd; + const IceParameters* ice_param = + FindRemoteIceFromUfrag(remote_username, &remote_generation); + // Note: if not found, the remote_generation will still be 0. + if (ice_param != nullptr) { + remote_password = ice_param->pwd; } } @@ -564,9 +572,9 @@ void P2PTransportChannel::OnUnknownAddress( // If the source transport address of the request does not match any // existing remote candidates, it represents a new peer reflexive remote // candidate. - remote_candidate = - Candidate(component(), ProtoToString(proto), address, 0, - remote_username, remote_password, PRFLX_PORT_TYPE, 0U, ""); + remote_candidate = Candidate(component(), ProtoToString(proto), address, 0, + remote_username, remote_password, + PRFLX_PORT_TYPE, remote_generation, ""); // From RFC 5245, section-7.2.1.3: // The foundation of the candidate is set to an arbitrary value, different @@ -626,6 +634,21 @@ void P2PTransportChannel::OnRoleConflict(PortInterface* port) { // from Transport. } +const IceParameters* P2PTransportChannel::FindRemoteIceFromUfrag( + const std::string& ufrag, + uint32_t* generation) { + const auto& params = remote_ice_parameters_; + auto it = std::find_if( + params.rbegin(), params.rend(), + [ufrag](const IceParameters& param) { return param.ufrag == ufrag; }); + if (it == params.rend()) { + // Not found. + return nullptr; + } + *generation = params.rend() - it - 1; + return &(*it); +} + void P2PTransportChannel::OnNominated(Connection* conn) { ASSERT(worker_thread_ == rtc::Thread::Current()); ASSERT(ice_role_ == ICEROLE_CONTROLLED); @@ -788,22 +811,14 @@ bool P2PTransportChannel::FindConnection( uint32_t P2PTransportChannel::GetRemoteCandidateGeneration( const Candidate& candidate) { - // We need to keep track of the remote ice restart so newer - // connections are prioritized over the older. - const auto& params = remote_ice_parameters_; + // If the candidate has a ufrag, use it to find the generation. if (!candidate.username().empty()) { - // If remote side sets the ufrag, we use that to determine the candidate - // generation. - // Search backward as it is more likely to find it near the end. - auto it = std::find_if(params.rbegin(), params.rend(), - [candidate](const IceParameters& param) { - return param.ufrag == candidate.username(); - }); - if (it == params.rend()) { - // If not found, assume it is the next (future) generation. - return static_cast<uint32_t>(remote_ice_parameters_.size()); + uint32_t generation = 0; + if (!FindRemoteIceFromUfrag(candidate.username(), &generation)) { + // If the ufrag is not found, assume the next/future generation. + generation = static_cast<uint32_t>(remote_ice_parameters_.size()); } - return params.rend() - it - 1; + return generation; } // If candidate generation is set, use that. if (candidate.generation() > 0) { diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 92c0534e7c..3927b17659 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -178,6 +178,11 @@ class P2PTransportChannel : public TransportChannelImpl, return allocator_sessions_.back(); } + // Public for unit tests. + const std::vector<RemoteCandidate>& remote_candidates() const { + return remote_candidates_; + } + private: rtc::Thread* thread() { return worker_thread_; } bool IsGettingPorts() { return allocator_session()->IsGettingPorts(); } @@ -247,6 +252,10 @@ class P2PTransportChannel : public TransportChannelImpl, return remote_ice_parameters_.empty() ? nullptr : &remote_ice_parameters_.back(); } + // Returns the remote IceParameters and generation that match |ufrag| + // if found, and returns nullptr otherwise. + const IceParameters* FindRemoteIceFromUfrag(const std::string& ufrag, + uint32_t* generation); // Returns the index of the latest remote ICE parameters, or 0 if no remote // ICE parameters have been received. uint32_t remote_ice_generation() { diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 9f40c46fdb..90ddd43714 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -1901,6 +1901,17 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) { const cricket::Candidate& new_candidate = conn3->remote_candidate(); EXPECT_EQ(kIcePwd[2], new_candidate.password()); EXPECT_EQ(1U, new_candidate.generation()); + + // Check that the pwd of all remote candidates are properly assigned. + for (const cricket::RemoteCandidate& candidate : ch.remote_candidates()) { + EXPECT_TRUE(candidate.username() == kIceUfrag[1] || + candidate.username() == kIceUfrag[2]); + if (candidate.username() == kIceUfrag[1]) { + EXPECT_EQ(kIcePwd[1], candidate.password()); + } else if (candidate.username() == kIceUfrag[2]) { + EXPECT_EQ(kIcePwd[2], candidate.password()); + } + } } TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) { @@ -2040,7 +2051,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { // on requests from an unknown address before the controlling side nominates // a connection, and will nominate a connection from an unknown address if the // request contains the use_candidate attribute. Plus, it will also sends back -// a ping response. +// a ping response and set the ICE pwd in the remote candidate appropriately. TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr); cricket::P2PTransportChannel ch("receiving state change", 1, nullptr, &pa); @@ -2105,6 +2116,18 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { EXPECT_EQ(conn2, ch.best_connection()); conn4->ReceivedPingResponse(); // Become writable. EXPECT_EQ(conn4, ch.best_connection()); + + // Test that the request from an unknown address contains a ufrag from an old + // generation. + port->set_sent_binding_response(false); + ch.SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); + ch.SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); + port->SignalUnknownAddress(port, rtc::SocketAddress("5.5.5.5", 5), + cricket::PROTO_UDP, &request, kIceUfrag[2], false); + cricket::Connection* conn5 = WaitForConnectionTo(&ch, "5.5.5.5", 5); + ASSERT_TRUE(conn5 != nullptr); + EXPECT_TRUE(port->sent_binding_response()); + EXPECT_EQ(kIcePwd[2], conn5->remote_candidate().password()); } // The controlled side will select a connection as the "best connection" |