diff --git a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java index b04885822..c3ec72c4c 100644 --- a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java @@ -600,26 +600,6 @@ public class PeerConnectionTest { sdpLatch = new SdpObserverLatch(); offeringExpectations.expectSignalingChange(SignalingState.STABLE); offeringExpectations.expectAddStream("answeredMediaStream"); - offeringPC.setRemoteDescription(sdpLatch, answerSdp); - assertTrue(sdpLatch.await()); - assertNull(sdpLatch.getSdp()); - - offeringExpectations.waitForAllExpectationsToBeSatisfied(); - answeringExpectations.waitForAllExpectationsToBeSatisfied(); - - assertEquals(offeringPC.getLocalDescription().type, offerSdp.type); - assertEquals(offeringPC.getRemoteDescription().type, answerSdp.type); - assertEquals(answeringPC.getLocalDescription().type, answerSdp.type); - assertEquals(answeringPC.getRemoteDescription().type, offerSdp.type); - - if (!RENDER_TO_GUI) { - // Wait for at least some frames to be delivered at each end (number - // chosen arbitrarily). - offeringExpectations.expectFramesDelivered(10); - answeringExpectations.expectFramesDelivered(10); - offeringExpectations.expectSetSize(); - answeringExpectations.expectSetSize(); - } offeringExpectations.expectIceConnectionChange( IceConnectionState.CHECKING); @@ -635,6 +615,24 @@ public class PeerConnectionTest { answeringExpectations.expectIceConnectionChange( IceConnectionState.CONNECTED); + offeringPC.setRemoteDescription(sdpLatch, answerSdp); + assertTrue(sdpLatch.await()); + assertNull(sdpLatch.getSdp()); + + assertEquals(offeringPC.getLocalDescription().type, offerSdp.type); + assertEquals(offeringPC.getRemoteDescription().type, answerSdp.type); + assertEquals(answeringPC.getLocalDescription().type, answerSdp.type); + assertEquals(answeringPC.getRemoteDescription().type, offerSdp.type); + + if (!RENDER_TO_GUI) { + // Wait for at least some frames to be delivered at each end (number + // chosen arbitrarily). + offeringExpectations.expectFramesDelivered(10); + answeringExpectations.expectFramesDelivered(10); + offeringExpectations.expectSetSize(); + answeringExpectations.expectSetSize(); + } + offeringExpectations.expectStateChange(DataChannel.State.OPEN); // See commentary about SCTP DataChannels above for why this is here. answeringExpectations.expectDataChannel("offeringDC"); diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 54018f223..27b936bf0 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -837,6 +837,19 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, if (error() != cricket::BaseSession::ERROR_NONE) { return BadRemoteSdp(desc->type(), GetSessionErrorMsg(), err_desc); } + + // Set the the ICE connection state to connecting since the connection may + // become writable with peer reflexive candidates before any remote candidate + // is signaled. + // TODO(pthatcher): This is a short-term solution for crbug/446908. A real fix + // is to have a new signal the indicates a change in checking state from the + // transport and expose a new checking() member from transport that can be + // read to determine the current checking state. The existing SignalConnecting + // actually means "gathering candidates", so cannot be be used here. + if (desc->type() != SessionDescriptionInterface::kOffer && + ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew) { + SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking); + } return true; } diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 4b52e7776..d0fb805a5 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -354,7 +354,8 @@ class WebRtcSessionTest : public testing::Test { SocketAddress(), SocketAddress(), SocketAddress())); allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY | - cricket::PORTALLOCATOR_ENABLE_BUNDLE); + cricket::PORTALLOCATOR_ENABLE_BUNDLE | + cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); EXPECT_TRUE(channel_manager_->Init()); desc_factory_->set_add_legacy_streams(false); allocator_->set_step_delay(cricket::kMinimumStepDelay); @@ -1212,7 +1213,8 @@ class WebRtcSessionTest : public testing::Test { allocator_->AddRelay(relay_server); allocator_->set_step_delay(cricket::kMinimumStepDelay); allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | - cricket::PORTALLOCATOR_ENABLE_BUNDLE); + cricket::PORTALLOCATOR_ENABLE_BUNDLE | + cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); } cricket::FakeMediaEngine* media_engine_; diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 9afaca6d9..4bbe9cf23 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -303,6 +303,12 @@ void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag, remote_ice_ufrag_ = ice_ufrag; remote_ice_pwd_ = ice_pwd; + // We need to update the credentials for any peer reflexive candidates. + std::vector::iterator it = connections_.begin(); + for (; it != connections_.end(); ++it) { + (*it)->MaybeSetRemoteIceCredentials(ice_ufrag, ice_pwd); + } + if (ice_restart) { // |candidate.generation()| is not signaled in ICEPROTO_RFC5245. // Therefore we need to keep track of the remote ice restart so @@ -450,12 +456,10 @@ void P2PTransportChannel::OnUnknownAddress( } const Candidate* candidate = NULL; - bool known_username = false; std::string remote_password; for (it = remote_candidates_.begin(); it != remote_candidates_.end(); ++it) { if (it->username() == remote_username) { remote_password = it->password(); - known_username = true; if (ufrag_per_port || (it->address() == address && it->protocol() == ProtoToString(proto))) { @@ -467,19 +471,11 @@ void P2PTransportChannel::OnUnknownAddress( } } - if (!known_username) { - if (port_muxed) { - // When Ports are muxed, SignalUnknownAddress is delivered to all - // P2PTransportChannel belong to a session. Return from here will - // save us from sending stun binding error message from incorrect channel. - return; - } - // Don't know about this username, the request is bogus - // This sometimes happens if a binding response comes in before the ACCEPT - // message. It is totally valid; the retry state machine will try again. - port->SendBindingErrorResponse(stun_msg, address, - STUN_ERROR_STALE_CREDENTIALS, STUN_ERROR_REASON_STALE_CREDENTIALS); - return; + // 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() && remote_username == remote_ice_ufrag_) { + remote_password = remote_ice_pwd_; } Candidate new_remote_candidate; @@ -573,6 +569,8 @@ void P2PTransportChannel::OnUnknownAddress( return; } + LOG(LS_INFO) << "Adding connection from peer reflexive candidate: " + << new_remote_candidate.ToString(); AddConnection(connection); connection->ReceivedPing(); @@ -720,6 +718,8 @@ bool P2PTransportChannel::CreateConnection(PortInterface* port, // found, then we can create a new connection for this address. Connection* connection = port->GetConnection(remote_candidate.address()); if (connection != NULL) { + connection->MaybeUpdatePeerReflexiveCandidate(remote_candidate); + // It is not legal to try to change any of the parameters of an existing // connection; however, the other side can send a duplicate candidate. if (!remote_candidate.IsEquivalent(connection->remote_candidate())) { diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index bc24b6b8b..e5df2d7e8 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -31,6 +31,7 @@ using cricket::kDefaultPortAllocatorFlags; using cricket::kMinimumStepDelay; using cricket::kDefaultStepDelay; +using cricket::PORTALLOCATOR_ENABLE_BUNDLE; using cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; using cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET; using cricket::ServerAddresses; @@ -96,6 +97,10 @@ static const char* kIcePwd[4] = {"TESTICEPWD00000000000000", static const uint64 kTiebreaker1 = 11111; static const uint64 kTiebreaker2 = 22222; +enum { + MSG_CANDIDATE +}; + // This test simulates 2 P2P endpoints that want to establish connectivity // with each other over various network topologies and conditions, which can be // specified in each individial test. @@ -199,10 +204,21 @@ class P2PTransportChannelTestBase : public testing::Test, rtc::scoped_ptr ch_; }; + struct CandidateData : public rtc::MessageData { + CandidateData(cricket::TransportChannel* ch, const cricket::Candidate& c) + : channel(ch), candidate(c) { + } + cricket::TransportChannel* channel; + cricket::Candidate candidate; + }; + struct Endpoint { - Endpoint() : signaling_delay_(0), role_(cricket::ICEROLE_UNKNOWN), - tiebreaker_(0), role_conflict_(false), - protocol_type_(cricket::ICEPROTO_GOOGLE) {} + Endpoint() + : role_(cricket::ICEROLE_UNKNOWN), + tiebreaker_(0), + role_conflict_(false), + save_candidates_(false), + protocol_type_(cricket::ICEPROTO_GOOGLE) {} bool HasChannel(cricket::TransportChannel* ch) { return (ch == cd1_.ch_.get() || ch == cd2_.ch_.get()); } @@ -213,7 +229,6 @@ class P2PTransportChannelTestBase : public testing::Test, else return &cd2_; } - void SetSignalingDelay(int delay) { signaling_delay_ = delay; } void SetIceRole(cricket::IceRole role) { role_ = role; } cricket::IceRole ice_role() { return role_; } @@ -236,19 +251,12 @@ class P2PTransportChannelTestBase : public testing::Test, rtc::scoped_ptr allocator_; ChannelData cd1_; ChannelData cd2_; - int signaling_delay_; cricket::IceRole role_; uint64 tiebreaker_; bool role_conflict_; + bool save_candidates_; cricket::IceProtocolType protocol_type_; - }; - - struct CandidateData : public rtc::MessageData { - CandidateData(cricket::TransportChannel* ch, const cricket::Candidate& c) - : channel(ch), candidate(c) { - } - cricket::TransportChannel* channel; - cricket::Candidate candidate; + std::vector saved_candidates_; }; ChannelData* GetChannelData(cricket::TransportChannel* channel) { @@ -277,11 +285,11 @@ class P2PTransportChannelTestBase : public testing::Test, std::string ice_ufrag_ep2_cd2_ch = kIceUfrag[3]; std::string ice_pwd_ep2_cd2_ch = kIcePwd[3]; // In BUNDLE each endpoint must share common ICE credentials. - if (ep1_.allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_BUNDLE) { + if (ep1_.allocator_->flags() & PORTALLOCATOR_ENABLE_BUNDLE) { ice_ufrag_ep1_cd2_ch = ice_ufrag_ep1_cd1_ch; ice_pwd_ep1_cd2_ch = ice_pwd_ep1_cd1_ch; } - if (ep2_.allocator_->flags() & cricket::PORTALLOCATOR_ENABLE_BUNDLE) { + if (ep2_.allocator_->flags() & PORTALLOCATOR_ENABLE_BUNDLE) { ice_ufrag_ep2_cd2_ch = ice_ufrag_ep2_cd1_ch; ice_pwd_ep2_cd2_ch = ice_pwd_ep2_cd1_ch; } @@ -380,9 +388,6 @@ class P2PTransportChannelTestBase : public testing::Test, void SetAllocatorFlags(int endpoint, int flags) { GetAllocator(endpoint)->set_flags(flags); } - void SetSignalingDelay(int endpoint, int delay) { - GetEndpoint(endpoint)->SetSignalingDelay(delay); - } void SetIceProtocol(int endpoint, cricket::IceProtocolType type) { GetEndpoint(endpoint)->SetIceProtocolType(type); } @@ -629,23 +634,44 @@ class P2PTransportChannelTestBase : public testing::Test, if (force_relay_ && c.type() != cricket::RELAY_PORT_TYPE) return; - main_->PostDelayed(GetEndpoint(ch)->signaling_delay_, this, 0, - new CandidateData(ch, c)); - } - void OnMessage(rtc::Message* msg) { - rtc::scoped_ptr data( - static_cast(msg->pdata)); - cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel); - cricket::Candidate c = data->candidate; - if (clear_remote_candidates_ufrag_pwd_) { - c.set_username(""); - c.set_password(""); + if (GetEndpoint(ch)->save_candidates_) { + GetEndpoint(ch)->saved_candidates_.push_back(new CandidateData(ch, c)); + } else { + main_->Post(this, MSG_CANDIDATE, new CandidateData(ch, c)); + } + } + + void PauseCandidates(int endpoint) { + GetEndpoint(endpoint)->save_candidates_ = true; + } + + 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(this, MSG_CANDIDATE, *it); + } + ed->saved_candidates_.clear(); + ed->save_candidates_ = false; + } + + void OnMessage(rtc::Message* msg) { + switch (msg->message_id) { + case MSG_CANDIDATE: { + rtc::scoped_ptr data( + static_cast(msg->pdata)); + cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel); + cricket::Candidate c = data->candidate; + if (clear_remote_candidates_ufrag_pwd_) { + c.set_username(""); + c.set_password(""); + } + LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" + << rch->component() << "): " << c.ToString(); + rch->OnCandidate(c); + break; + } } - LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" - << rch->component() << "): " << c.type() << ", " << c.protocol() - << ", " << c.address().ToString() << ", " << c.username() - << ", " << c.generation(); - rch->OnCandidate(c); } void OnReadPacket(cricket::TransportChannel* channel, const char* data, size_t len, const rtc::PacketTime& packet_time, @@ -822,6 +848,10 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { SetIceProtocol(1, type); SetAllocatorFlags(1, allocator_flags2); SetAllocationStepDelay(1, delay2); + + if (type == cricket::ICEPROTO_RFC5245) { + set_clear_remote_candidates_ufrag_pwd(true); + } } void ConfigureEndpoint(int endpoint, Config config) { switch (config) { @@ -1163,14 +1193,12 @@ TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeAsIce) { // Test that we restart candidate allocation when local ufrag&pwd changed. // Standard Ice protocol is used. TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeBundleAsIce) { - ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kMinimumStepDelay, kMinimumStepDelay, - cricket::ICEPROTO_RFC5245); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - + ConfigureEndpoints( + OPEN, OPEN, + PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG, + PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG, + kMinimumStepDelay, kMinimumStepDelay, + cricket::ICEPROTO_RFC5245); CreateChannels(2); TestHandleIceUfragPasswordChanged(); DestroyChannels(); @@ -1192,14 +1220,12 @@ TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeAsGice) { // Test that ICE restart works when bundle is enabled. // Google Ice protocol is used. TEST_F(P2PTransportChannelTest, HandleUfragPwdChangeBundleAsGice) { - ConfigureEndpoints(OPEN, OPEN, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - PORTALLOCATOR_ENABLE_SHARED_UFRAG, - kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - + ConfigureEndpoints( + OPEN, OPEN, + PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG, + PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG, + kDefaultStepDelay, kDefaultStepDelay, + cricket::ICEPROTO_GOOGLE); CreateChannels(2); TestHandleIceUfragPasswordChanged(); DestroyChannels(); @@ -1233,25 +1259,68 @@ TEST_F(P2PTransportChannelTest, GetStats) { DestroyChannels(); } -// Test that we properly handle getting a STUN error due to slow signaling. -TEST_F(P2PTransportChannelTest, DISABLED_SlowSignaling) { - ConfigureEndpoints(OPEN, NAT_SYMMETRIC, - kDefaultPortAllocatorFlags, - kDefaultPortAllocatorFlags, +// Test that we properly create a connection on a STUN ping from unknown address +// when the signaling is slow. +TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { + ConfigureEndpoints(OPEN, OPEN, + PORTALLOCATOR_ENABLE_SHARED_UFRAG, + PORTALLOCATOR_ENABLE_SHARED_UFRAG, kDefaultStepDelay, kDefaultStepDelay, - cricket::ICEPROTO_GOOGLE); - // Make signaling from the callee take 500ms, so that the initial STUN pings - // from the callee beat the signaling, and so the caller responds with a - // unknown username error. We should just eat that and carry on; mishandling - // this will instead cause all the callee's connections to be discarded. - SetSignalingDelay(1, 1000); + cricket::ICEPROTO_RFC5245); CreateChannels(1); + + // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive + // candidate. + PauseCandidates(1); + + // The caller should have the best connection connected to the peer reflexive + // candidate. const cricket::Connection* best_connection = NULL; - // Wait until the callee's connections are created. - WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 1000); - // Wait to see if they get culled; they shouldn't. - WAIT(ep2_ch1()->best_connection() != best_connection, 1000); - EXPECT_TRUE(ep2_ch1()->best_connection() == best_connection); + WAIT((best_connection = ep1_ch1()->best_connection()) != NULL, 2000); + EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type()); + + ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ResumeCandidates(1); + + WAIT(ep2_ch1()->best_connection() != NULL, 2000); + + // Verify ep1's best connection is updated to use the 'local' candidate. + EXPECT_EQ_WAIT( + "local", + ep1_ch1()->best_connection()->remote_candidate().type(), + 2000); + EXPECT_EQ(best_connection, ep1_ch1()->best_connection()); + DestroyChannels(); +} + +// Test that we properly create a connection on a STUN ping from unknown address +// when the signaling is slow and the end points are behind NAT. +TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { + ConfigureEndpoints(OPEN, NAT_SYMMETRIC, + PORTALLOCATOR_ENABLE_SHARED_UFRAG, + PORTALLOCATOR_ENABLE_SHARED_UFRAG, + kDefaultStepDelay, kDefaultStepDelay, + cricket::ICEPROTO_RFC5245); + CreateChannels(1); + // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive + // candidate. + PauseCandidates(1); + + // The caller should have the best connection connected to the peer reflexive + // candidate. + WAIT(ep1_ch1()->best_connection() != NULL, 2000); + EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type()); + + ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ResumeCandidates(1); + + const cricket::Connection* best_connection = NULL; + WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 2000); + + // Wait to verify the connection is not culled. + WAIT(ep1_ch1()->writable(), 2000); + EXPECT_EQ(ep2_ch1()->best_connection(), best_connection); + EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type()); DestroyChannels(); } @@ -1359,8 +1428,10 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { TEST_F(P2PTransportChannelTest, TestBundleAllocatorToBundleAllocator) { AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + SetAllocatorFlags( + 0, PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG); + SetAllocatorFlags( + 1, PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG); CreateChannels(2); @@ -1385,7 +1456,8 @@ TEST_F(P2PTransportChannelTest, TestBundleAllocatorToNonBundleAllocator) { AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); // Enable BUNDLE flag at one side. - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + SetAllocatorFlags( + 0, PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG); CreateChannels(2); @@ -1418,8 +1490,10 @@ TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithoutBundle) { TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithBundle) { AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + SetAllocatorFlags( + 0, PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG); + SetAllocatorFlags( + 1, PORTALLOCATOR_ENABLE_BUNDLE | PORTALLOCATOR_ENABLE_SHARED_UFRAG); TestSignalRoleConflict(); } diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index cdef0fc55..0472b1698 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -418,7 +418,8 @@ bool Port::GetStunMessage(const char* data, size_t size, if (IsStandardIce() && !stun_msg->ValidateMessageIntegrity(data, size, password_)) { LOG_J(LS_ERROR, this) << "Received STUN request with bad M-I " - << "from " << addr.ToSensitiveString(); + << "from " << addr.ToSensitiveString() + << ", password_=" << password_; SendBindingErrorResponse(stun_msg.get(), addr, STUN_ERROR_UNAUTHORIZED, STUN_ERROR_REASON_UNAUTHORIZED); return true; @@ -1348,6 +1349,27 @@ void Connection::HandleRoleConflictFromPeer() { port_->SignalRoleConflict(port_); } +void Connection::MaybeSetRemoteIceCredentials(const std::string& ice_ufrag, + const std::string& ice_pwd) { + if (remote_candidate_.username() == ice_ufrag && + remote_candidate_.password().empty()) { + remote_candidate_.set_password(ice_pwd); + } +} + +void Connection::MaybeUpdatePeerReflexiveCandidate( + const Candidate& new_candidate) { + if (remote_candidate_.type() == PRFLX_PORT_TYPE && + new_candidate.type() != PRFLX_PORT_TYPE && + remote_candidate_.protocol() == new_candidate.protocol() && + remote_candidate_.address() == new_candidate.address() && + remote_candidate_.username() == new_candidate.username() && + remote_candidate_.password() == new_candidate.password() && + remote_candidate_.generation() == new_candidate.generation()) { + remote_candidate_ = new_candidate; + } +} + void Connection::OnMessage(rtc::Message *pmsg) { ASSERT(pmsg->message_id == MSG_DELETE); diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 4a5c89985..6d7d6d543 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -535,6 +535,16 @@ class Connection : public rtc::MessageHandler, IceMode remote_ice_mode() const { return remote_ice_mode_; } + // Update the ICE password of the remote candidate if |ice_ufrag| matches + // the candidate's ufrag, and the candidate's passwrod has not been set. + void MaybeSetRemoteIceCredentials(const std::string& ice_ufrag, + const std::string& ice_pwd); + + // If |remote_candidate_| is peer reflexive and is equivalent to + // |new_candidate| except the type, update |remote_candidate_| to + // |new_candidate|. + void MaybeUpdatePeerReflexiveCandidate(const Candidate& new_candidate); + protected: // Constructs a new connection to the given remote port. Connection(Port* port, size_t index, const Candidate& candidate); diff --git a/webrtc/p2p/base/portallocator.cc b/webrtc/p2p/base/portallocator.cc index 86133474d..5ac58ea9d 100644 --- a/webrtc/p2p/base/portallocator.cc +++ b/webrtc/p2p/base/portallocator.cc @@ -28,6 +28,9 @@ PortAllocatorSession::PortAllocatorSession(const std::string& content_name, // by itself. username_(flags_ & PORTALLOCATOR_ENABLE_SHARED_UFRAG ? ice_ufrag : ""), password_(flags_ & PORTALLOCATOR_ENABLE_SHARED_UFRAG ? ice_pwd : "") { + // If bundle is enabled, shared ufrag must be enabled too. + ASSERT((!(flags_ & PORTALLOCATOR_ENABLE_BUNDLE)) || + (flags_ & PORTALLOCATOR_ENABLE_SHARED_UFRAG)); } PortAllocator::~PortAllocator() { diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 0cee3bde7..6f83beb6c 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -644,7 +644,8 @@ TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnlyAndNoNAT) { TEST_F(PortAllocatorTest, TestBasicMuxFeatures) { AddInterface(kClientAddr); - allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_BUNDLE); + allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_BUNDLE | + cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); // Session ID - session1. rtc::scoped_ptr session1( CreateSession("session1", cricket::ICE_CANDIDATE_COMPONENT_RTP)); @@ -671,7 +672,8 @@ TEST_F(PortAllocatorTest, TestBasicMuxFeatures) { // set of candidates. TEST_F(PortAllocatorTest, TestBundleIceRestart) { AddInterface(kClientAddr); - allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_BUNDLE); + allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_BUNDLE | + cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); // Session ID - session1. rtc::scoped_ptr session1( CreateSession("session1", kContentName,