diff --git a/api/asyncresolverfactory.h b/api/asyncresolverfactory.h index 3c3bb1e703..96abee45ec 100644 --- a/api/asyncresolverfactory.h +++ b/api/asyncresolverfactory.h @@ -23,8 +23,7 @@ class AsyncResolverFactory { AsyncResolverFactory() = default; virtual ~AsyncResolverFactory() = default; - // The returned object is responsible for deleting itself after address - // resolution has completed. + // The caller should call Destroy on the returned object to delete it. virtual rtc::AsyncResolverInterface* Create() = 0; }; diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 5efdb0b4c7..f8f70d9889 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -122,6 +122,7 @@ if (rtc_include_tests) { "base/fakeicetransport.h", "base/fakepackettransport.h", "base/fakeportallocator.h", + "base/mockasyncresolver.h", "base/mockicetransport.h", "base/testrelayserver.h", "base/teststunserver.cc", diff --git a/p2p/base/mockasyncresolver.h b/p2p/base/mockasyncresolver.h new file mode 100644 index 0000000000..620137668f --- /dev/null +++ b/p2p/base/mockasyncresolver.h @@ -0,0 +1,47 @@ +/* + * Copyright 2018 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef P2P_BASE_MOCKASYNCRESOLVER_H_ +#define P2P_BASE_MOCKASYNCRESOLVER_H_ + +#include "api/asyncresolverfactory.h" +#include "rtc_base/asyncresolverinterface.h" +#include "test/gmock.h" + +namespace rtc { + +class MockAsyncResolver : public AsyncResolverInterface { + public: + MockAsyncResolver() = default; + ~MockAsyncResolver() = default; + + void Start(const rtc::SocketAddress& addr) { SignalDone(this); } + + MOCK_CONST_METHOD2(GetResolvedAddress, bool(int family, SocketAddress* addr)); + MOCK_CONST_METHOD0(GetError, int()); + + // Note that this won't delete the object like AsyncResolverInterface says in + // order to avoid sanitizer failures caused by this being a synchronous + // implementation. The test code should delete the object instead. + MOCK_METHOD1(Destroy, void(bool)); +}; + +} // namespace rtc + +namespace webrtc { + +class MockAsyncResolverFactory : public AsyncResolverFactory { + public: + MOCK_METHOD0(Create, rtc::AsyncResolverInterface*()); +}; + +} // namespace webrtc + +#endif // P2P_BASE_MOCKASYNCRESOLVER_H_ diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc index 3fbc90c7b3..9a4bb89e23 100644 --- a/p2p/base/p2ptransportchannel.cc +++ b/p2p/base/p2ptransportchannel.cc @@ -24,6 +24,7 @@ #include "rtc_base/crc32.h" #include "rtc_base/logging.h" #include "rtc_base/nethelper.h" +#include "rtc_base/nethelpers.h" #include "rtc_base/stringencode.h" #include "rtc_base/timeutils.h" #include "system_wrappers/include/field_trial.h" @@ -154,6 +155,10 @@ P2PTransportChannel::P2PTransportChannel( } P2PTransportChannel::~P2PTransportChannel() { + for (auto& p : resolvers_) { + p.resolver_->Destroy(false); + } + resolvers_.clear(); RTC_DCHECK(network_thread_ == rtc::Thread::Current()); } @@ -959,6 +964,21 @@ void P2PTransportChannel::OnNominated(Connection* conn) { } } +void P2PTransportChannel::ResolveHostnameCandidate(const Candidate& candidate) { + if (!async_resolver_factory_) { + RTC_LOG(LS_WARNING) << "Dropping ICE candidate with hostname address " + << "(no AsyncResolverFactory)"; + return; + } + + rtc::AsyncResolverInterface* resolver = async_resolver_factory_->Create(); + resolvers_.emplace_back(candidate, resolver); + resolver->SignalDone.connect(this, &P2PTransportChannel::OnCandidateResolved); + resolver->Start(candidate.address()); + RTC_LOG(LS_INFO) << "Asynchronously resolving ICE candidate hostname " + << candidate.address().HostAsSensitiveURIString(); +} + void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) { RTC_DCHECK(network_thread_ == rtc::Thread::Current()); @@ -994,6 +1014,71 @@ void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) { } } + if (new_remote_candidate.address().IsUnresolvedIP()) { + ResolveHostnameCandidate(new_remote_candidate); + return; + } + + FinishAddingRemoteCandidate(new_remote_candidate); +} + +P2PTransportChannel::CandidateAndResolver::CandidateAndResolver( + const Candidate& candidate, + rtc::AsyncResolverInterface* resolver) + : candidate_(candidate), resolver_(resolver) {} + +P2PTransportChannel::CandidateAndResolver::~CandidateAndResolver() {} + +void P2PTransportChannel::OnCandidateResolved( + rtc::AsyncResolverInterface* resolver) { + auto p = std::find_if(resolvers_.begin(), resolvers_.end(), + [resolver](const CandidateAndResolver& cr) { + return cr.resolver_ == resolver; + }); + if (p == resolvers_.end()) { + RTC_LOG(LS_ERROR) << "Unexpected AsyncResolver signal"; + RTC_NOTREACHED(); + return; + } + Candidate candidate = p->candidate_; + resolvers_.erase(p); + AddRemoteCandidateWithResolver(candidate, resolver); + resolver->Destroy(false); +} + +void P2PTransportChannel::AddRemoteCandidateWithResolver( + Candidate candidate, + rtc::AsyncResolverInterface* resolver) { + if (resolver->GetError()) { + RTC_LOG(LS_WARNING) << "Failed to resolve ICE candidate hostname " + << candidate.address().HostAsSensitiveURIString() + << " with error " << resolver->GetError(); + return; + } + + rtc::SocketAddress resolved_address; + // Prefer IPv6 to IPv4 if we have it (see RFC 5245 Section 15.1). + // TODO(zstein): This won't work if we only have IPv4 locally but receive an + // AAAA DNS record. + bool have_address = + resolver->GetResolvedAddress(AF_INET6, &resolved_address) || + resolver->GetResolvedAddress(AF_INET, &resolved_address); + if (!have_address) { + RTC_LOG(LS_INFO) << "ICE candidate hostname " + << candidate.address().HostAsSensitiveURIString() + << " could not be resolved"; + return; + } + + RTC_LOG(LS_INFO) << "Resolved ICE candidate hostname " + << candidate.address().HostAsSensitiveURIString() << " to " + << resolved_address.ipaddr().ToSensitiveString(); + candidate.set_address(resolved_address); + FinishAddingRemoteCandidate(candidate); +} + +void P2PTransportChannel::FinishAddingRemoteCandidate( + const Candidate& new_remote_candidate) { // If this candidate matches what was thought to be a peer reflexive // candidate, we need to update the candidate priority/etc. for (Connection* conn : connections_) { diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h index 1ac898e490..9978cb0070 100644 --- a/p2p/base/p2ptransportchannel.h +++ b/p2p/base/p2ptransportchannel.h @@ -105,6 +105,7 @@ class P2PTransportChannel : public IceTransportInternal { void Connect() {} void MaybeStartGathering() override; IceGatheringState gathering_state() const override; + void ResolveHostnameCandidate(const Candidate& candidate); void AddRemoteCandidate(const Candidate& candidate) override; void RemoveRemoteCandidate(const Candidate& candidate) override; // Sets the parameters in IceConfig. We do not set them blindly. Instead, we @@ -419,6 +420,19 @@ class P2PTransportChannel : public IceTransportInternal { absl::optional network_route_; webrtc::IceEventLog ice_event_log_; + struct CandidateAndResolver final { + CandidateAndResolver(const Candidate& candidate, + rtc::AsyncResolverInterface* resolver); + ~CandidateAndResolver(); + Candidate candidate_; + rtc::AsyncResolverInterface* resolver_; + }; + std::vector resolvers_; + void FinishAddingRemoteCandidate(const Candidate& new_remote_candidate); + void OnCandidateResolved(rtc::AsyncResolverInterface* resolver); + void AddRemoteCandidateWithResolver(Candidate candidate, + rtc::AsyncResolverInterface* resolver); + RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); }; diff --git a/p2p/base/p2ptransportchannel_unittest.cc b/p2p/base/p2ptransportchannel_unittest.cc index c56ace7662..2da0fc3561 100644 --- a/p2p/base/p2ptransportchannel_unittest.cc +++ b/p2p/base/p2ptransportchannel_unittest.cc @@ -15,6 +15,7 @@ #include "absl/memory/memory.h" #include "p2p/base/fakeportallocator.h" #include "p2p/base/icetransportinternal.h" +#include "p2p/base/mockasyncresolver.h" #include "p2p/base/p2ptransportchannel.h" #include "p2p/base/packettransportinternal.h" #include "p2p/base/testrelayserver.h" @@ -41,6 +42,10 @@ namespace { using rtc::SocketAddress; +using ::testing::_; +using ::testing::DoAll; +using ::testing::Return; +using ::testing::SetArgPointee; // Default timeout for tests in this file. // Should be large enough for slow buildbots to run the tests reliably. @@ -317,6 +322,7 @@ class P2PTransportChannelTestBase : public testing::Test, rtc::FakeNetworkManager network_manager_; std::unique_ptr allocator_; + webrtc::AsyncResolverFactory* async_resolver_factory_; ChannelData cd1_; ChannelData cd2_; IceRole role_; @@ -373,7 +379,8 @@ class P2PTransportChannelTestBase : public testing::Test, const IceParameters& local_ice, const IceParameters& remote_ice) { P2PTransportChannel* channel = new P2PTransportChannel( - "test content name", component, GetAllocator(endpoint)); + "test content name", component, GetAllocator(endpoint), + GetEndpoint(endpoint)->async_resolver_factory_); channel->SignalReadyToSend.connect( this, &P2PTransportChannelTestBase::OnReadyToSend); channel->SignalCandidateGathered.connect( @@ -753,6 +760,20 @@ class P2PTransportChannelTestBase : public testing::Test, } } + SocketAddress ReplaceSavedCandidateIpWithHostname( + int endpoint, + const SocketAddress& hostname_address) { + Endpoint* ed = GetEndpoint(endpoint); + + RTC_CHECK(1 == ed->saved_candidates_.size()); + auto& candidates = ed->saved_candidates_[0]; + RTC_CHECK(1 == candidates->candidates.size()); + auto& candidate = candidates->candidates[0]; + SocketAddress ip_address = candidate.address(); + candidate.set_address(hostname_address); + return ip_address; + } + void ResumeCandidates(int endpoint) { Endpoint* ed = GetEndpoint(endpoint); for (auto& candidate : ed->saved_candidates_) { @@ -4551,4 +4572,78 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, TestTcpTurn) { VerifyNextPingableConnection(LOCAL_PORT_TYPE, RELAY_PORT_TYPE); } +// Test that a resolver is created, asked for a result, and destroyed +// when the address is a hostname. +TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) { + rtc::MockAsyncResolver mock_async_resolver; + EXPECT_CALL(mock_async_resolver, GetError()).WillOnce(Return(0)); + EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_async_resolver, Destroy(_)); + webrtc::MockAsyncResolverFactory mock_async_resolver_factory; + EXPECT_CALL(mock_async_resolver_factory, Create()) + .WillOnce(Return(&mock_async_resolver)); + + P2PTransportChannel channel("tn", 0, /*allocator*/ nullptr, + &mock_async_resolver_factory); + Candidate hostname_candidate; + SocketAddress hostname_address("fake.test", 1000); + hostname_candidate.set_address(hostname_address); + channel.AddRemoteCandidate(hostname_candidate); + + ASSERT_EQ_WAIT(1u, channel.remote_candidates().size(), kDefaultTimeout); + const RemoteCandidate& candidate = channel.remote_candidates()[0]; + EXPECT_FALSE(candidate.address().IsUnresolvedIP()); +} + +TEST_F(P2PTransportChannelTest, + PeerReflexiveCandidateBeforeSignalingWithHostname) { + rtc::MockAsyncResolver mock_async_resolver; + webrtc::MockAsyncResolverFactory mock_async_resolver_factory; + EXPECT_CALL(mock_async_resolver_factory, Create()) + .WillOnce(Return(&mock_async_resolver)); + + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + // Emulate no remote parameters coming in. + set_remote_ice_parameter_source(FROM_CANDIDATE); + GetEndpoint(0)->async_resolver_factory_ = &mock_async_resolver_factory; + CreateChannels(); + // Only have remote parameters come in for ep2, not ep1. + ep2_ch1()->SetRemoteIceParameters(kIceParams[0]); + + // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive + // candidate. + PauseCandidates(1); + + // Wait until the callee becomes writable to make sure that a ping request is + // received by the caller before his remote ICE credentials are set. + ASSERT_TRUE_WAIT(ep2_ch1()->selected_connection() != nullptr, kMediumTimeout); + ep1_ch1()->SetRemoteIceParameters(kIceParams[1]); + // The caller should have the selected connection connected to the peer + // reflexive candidate. + const Connection* selected_connection = nullptr; + ASSERT_TRUE_WAIT( + (selected_connection = ep1_ch1()->selected_connection()) != nullptr, + kMediumTimeout); + 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()); + + SocketAddress hostname_address("fake.hostname", 12345); + SocketAddress ip_address = + ReplaceSavedCandidateIpWithHostname(1, hostname_address); + EXPECT_CALL(mock_async_resolver, GetError()).WillOnce(Return(0)); + EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(ip_address), Return(true))); + EXPECT_CALL(mock_async_resolver, Destroy(_)); + + ResumeCandidates(1); + // Verify ep1's selected connection is updated to use the 'local' candidate. + EXPECT_EQ_WAIT(LOCAL_PORT_TYPE, + ep1_ch1()->selected_connection()->remote_candidate().type(), + kMediumTimeout); + EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection()); + DestroyChannels(); +} + } // namespace cricket diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 1b8054f09a..cc97e4ef7d 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -37,6 +37,7 @@ #include "media/engine/fakewebrtcvideoengine.h" #include "media/engine/webrtcmediaengine.h" #include "modules/audio_processing/include/audio_processing.h" +#include "p2p/base/mockasyncresolver.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/portinterface.h" #include "p2p/base/teststunserver.h" @@ -74,7 +75,10 @@ using cricket::StreamParams; using rtc::SocketAddress; using ::testing::Combine; using ::testing::ElementsAre; +using ::testing::Return; +using ::testing::SetArgPointee; using ::testing::Values; +using ::testing::_; using webrtc::DataBuffer; using webrtc::DataChannelInterface; using webrtc::DtmfSender; @@ -208,6 +212,17 @@ class MockRtpReceiverObserver : public webrtc::RtpReceiverObserverInterface { cricket::MediaType expected_media_type_; }; +// Used by PeerConnectionWrapper::OnIceCandidate to allow a test to modify an +// ICE candidate before it is signaled. +class IceCandidateReplacerInterface { + public: + virtual ~IceCandidateReplacerInterface() = default; + // Return nullptr to drop the candidate (it won't be signaled to the other + // side). + virtual std::unique_ptr ReplaceCandidate( + const webrtc::IceCandidateInterface*) = 0; +}; + // Helper class that wraps a peer connection, observes it, and can accept // signaling messages from another wrapper. // @@ -288,6 +303,11 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, remote_offer_handler_ = std::move(handler); } + void SetLocalIceCandidateReplacer( + std::unique_ptr replacer) { + local_ice_candidate_replacer_ = std::move(replacer); + } + // Every ICE connection state in order that has been seen by the observer. std::vector ice_connection_state_history() const { @@ -896,16 +916,46 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, EXPECT_EQ(pc()->ice_gathering_state(), new_state); ice_gathering_state_history_.push_back(new_state); } + std::unique_ptr ReplaceIceCandidate( + const webrtc::IceCandidateInterface* candidate) { + std::string candidate_string; + candidate->ToString(&candidate_string); + + auto owned_candidate = + local_ice_candidate_replacer_->ReplaceCandidate(candidate); + if (!owned_candidate) { + RTC_LOG(LS_INFO) << "LocalIceCandidateReplacer dropped \"" + << candidate_string << "\""; + return nullptr; + } + std::string owned_candidate_string; + owned_candidate->ToString(&owned_candidate_string); + RTC_LOG(LS_INFO) << "LocalIceCandidateReplacer changed \"" + << candidate_string << "\" to \"" << owned_candidate_string + << "\""; + return owned_candidate; + } void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override { RTC_LOG(LS_INFO) << debug_name_ << ": OnIceCandidate"; + const webrtc::IceCandidateInterface* new_candidate = candidate; + std::unique_ptr owned_candidate; + if (local_ice_candidate_replacer_) { + owned_candidate = ReplaceIceCandidate(candidate); + if (!owned_candidate) { + return; // The candidate was dropped. + } + new_candidate = owned_candidate.get(); + } + std::string ice_sdp; - EXPECT_TRUE(candidate->ToString(&ice_sdp)); + EXPECT_TRUE(new_candidate->ToString(&ice_sdp)); if (signaling_message_receiver_ == nullptr || !signal_ice_candidates_) { // Remote party may be deleted. return; } - SendIceMessage(candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp); + SendIceMessage(new_candidate->sdp_mid(), new_candidate->sdp_mline_index(), + ice_sdp); } void OnDataChannel( rtc::scoped_refptr data_channel) override { @@ -949,7 +999,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, std::function received_sdp_munger_; std::function generated_sdp_munger_; std::function remote_offer_handler_; - + std::unique_ptr local_ice_candidate_replacer_; rtc::scoped_refptr data_channel_; std::unique_ptr data_observer_; @@ -3301,6 +3351,93 @@ TEST_P(PeerConnectionIntegrationTest, IceStatesReachCompletion) { callee()->ice_connection_state(), kDefaultTimeout); } +// Replaces the first candidate with a static address and configures a +// MockAsyncResolver to return the replaced address the first time the static +// address is resolved. Candidates past the first will not be signaled. +class ReplaceFirstCandidateAddressDropOthers final + : public IceCandidateReplacerInterface { + public: + ReplaceFirstCandidateAddressDropOthers( + const SocketAddress& new_address, + rtc::MockAsyncResolver* mock_async_resolver) + : mock_async_resolver_(mock_async_resolver), new_address_(new_address) { + RTC_DCHECK(mock_async_resolver); + } + + std::unique_ptr ReplaceCandidate( + const webrtc::IceCandidateInterface* candidate) override { + if (replaced_candidate_) { + return nullptr; + } + + replaced_candidate_ = true; + cricket::Candidate new_candidate(candidate->candidate()); + new_candidate.set_address(new_address_); + EXPECT_CALL(*mock_async_resolver_, GetResolvedAddress(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(candidate->candidate().address()), + Return(true))); + EXPECT_CALL(*mock_async_resolver_, Destroy(_)); + return webrtc::CreateIceCandidate( + candidate->sdp_mid(), candidate->sdp_mline_index(), new_candidate); + } + + private: + rtc::MockAsyncResolver* mock_async_resolver_; + SocketAddress new_address_; + bool replaced_candidate_ = false; +}; + +// Drops all candidates before they are signaled. +class DropAllCandidates final : public IceCandidateReplacerInterface { + public: + std::unique_ptr ReplaceCandidate( + const webrtc::IceCandidateInterface*) override { + return nullptr; + } +}; + +// Replace the first caller ICE candidate IP with a fake hostname and drop the +// other candidates. Drop all candidates on the callee side (to avoid a prflx +// connection). Use a mock resolver to resolve the hostname back to the original +// IP on the callee side and check that the ice connection connects. +TEST_P(PeerConnectionIntegrationTest, + IceStatesReachCompletionWithRemoteHostname) { + webrtc::MockAsyncResolverFactory* callee_mock_async_resolver_factory; + { + auto resolver_factory = + absl::make_unique(); + callee_mock_async_resolver_factory = resolver_factory.get(); + webrtc::PeerConnectionDependencies callee_deps(nullptr); + callee_deps.async_resolver_factory = std::move(resolver_factory); + + ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndDeps( + RTCConfiguration(), webrtc::PeerConnectionDependencies(nullptr), + RTCConfiguration(), std::move(callee_deps))); + } + + rtc::MockAsyncResolver mock_async_resolver; + + // This also verifies that the injected AsyncResolverFactory is used by + // P2PTransportChannel. + EXPECT_CALL(*callee_mock_async_resolver_factory, Create()) + .WillOnce(Return(&mock_async_resolver)); + caller()->SetLocalIceCandidateReplacer( + absl::make_unique( + SocketAddress("a.b", 10000), &mock_async_resolver)); + callee()->SetLocalIceCandidateReplacer( + absl::make_unique()); + + ConnectFakeSignaling(); + caller()->AddAudioVideoTracks(); + callee()->AddAudioVideoTracks(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted, + caller()->ice_connection_state(), kDefaultTimeout); + EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected, + callee()->ice_connection_state(), kDefaultTimeout); +} + // Test that firewalling the ICE connection causes the clients to identify the // disconnected state and then removing the firewall causes them to reconnect. class PeerConnectionIntegrationIceStatesTest