diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc index 6d4c3de723..7c6ae5cb4a 100644 --- a/p2p/base/connection.cc +++ b/p2p/base/connection.cc @@ -720,6 +720,38 @@ void Connection::HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg) { } } +CandidatePair Connection::ToCandidatePairAndSanitizeIfNecessary() const { + auto get_sanitized_copy = [](const Candidate& c) { + bool use_hostname_address = c.type() == LOCAL_PORT_TYPE; + bool filter_related_address = c.type() == STUN_PORT_TYPE; + return c.ToSanitizedCopy(use_hostname_address, filter_related_address); + }; + + CandidatePair pair; + if (port_->Network()->GetMdnsResponder() != nullptr) { + // When the mDNS obfuscation of local IPs is enabled, we sanitize local + // candidates. + pair.local = get_sanitized_copy(local_candidate()); + } else { + pair.local = local_candidate(); + } + + if (!remote_candidate().address().hostname().empty()) { + // If the remote endpoint signaled us a hostname candidate, we assume it is + // supposed to be sanitized in the stats. + // + // A prflx remote candidate should not have a hostname set. + RTC_DCHECK(remote_candidate().type() != PRFLX_PORT_TYPE); + // A remote hostname candidate should have a resolved IP before we can form + // a candidate pair. + RTC_DCHECK(!remote_candidate().address().IsUnresolvedIP()); + pair.remote = get_sanitized_copy(remote_candidate()); + } else { + pair.remote = remote_candidate(); + } + return pair; +} + void Connection::ReceivedPingResponse( int rtt, const std::string& request_id, @@ -1107,33 +1139,9 @@ void Connection::MaybeUpdateLocalCandidate(ConnectionRequest* request, } void Connection::CopyCandidatesToStatsAndSanitizeIfNecessary() { - auto get_sanitized_copy = [](const Candidate& c) { - bool use_hostname_address = c.type() == LOCAL_PORT_TYPE; - bool filter_related_address = c.type() == STUN_PORT_TYPE; - return c.ToSanitizedCopy(use_hostname_address, filter_related_address); - }; - - if (port_->Network()->GetMdnsResponder() != nullptr) { - // When the mDNS obfuscation of local IPs is enabled, we sanitize local - // candidates. - stats_.local_candidate = get_sanitized_copy(local_candidate()); - } else { - stats_.local_candidate = local_candidate(); - } - - if (!remote_candidate().address().hostname().empty()) { - // If the remote endpoint signaled us a hostname candidate, we assume it is - // supposed to be sanitized in the stats. - // - // A prflx remote candidate should not have a hostname set. - RTC_DCHECK(remote_candidate().type() != PRFLX_PORT_TYPE); - // A remote hostname candidate should have a resolved IP before we can form - // a candidate pair. - RTC_DCHECK(!remote_candidate().address().IsUnresolvedIP()); - stats_.remote_candidate = get_sanitized_copy(remote_candidate()); - } else { - stats_.remote_candidate = remote_candidate(); - } + auto pair = ToCandidatePairAndSanitizeIfNecessary(); + stats_.local_candidate = pair.local_candidate(); + stats_.remote_candidate = pair.remote_candidate(); } bool Connection::rtt_converged() const { diff --git a/p2p/base/connection.h b/p2p/base/connection.h index 82b2c8924e..92fc2ed9cb 100644 --- a/p2p/base/connection.h +++ b/p2p/base/connection.h @@ -36,6 +36,16 @@ class Port; // Forward declaration so that a ConnectionRequest can contain a Connection. class Connection; +struct CandidatePair final : public CandidatePairInterface { + ~CandidatePair() override = default; + + const Candidate& local_candidate() const override { return local; } + const Candidate& remote_candidate() const override { return remote; } + + Candidate local; + Candidate remote; +}; + // A ConnectionRequest is a simple STUN ping used to determine writability. class ConnectionRequest : public StunRequest { public: @@ -228,6 +238,10 @@ class Connection : public CandidatePairInterface, void HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg); int64_t last_data_received() const { return last_data_received_; } + // Returns the equivalent candidate pair and sanitizes the local and the + // remote candidates if necessary. + CandidatePair ToCandidatePairAndSanitizeIfNecessary() const; + // Debugging description of this connection std::string ToDebugId() const; std::string ToString() const; diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h index 1b95a55c42..467ee2ec1c 100644 --- a/p2p/base/fake_ice_transport.h +++ b/p2p/base/fake_ice_transport.h @@ -207,6 +207,10 @@ class FakeIceTransport : public IceTransportInternal { absl::optional GetRttEstimate() override { return absl::nullopt; } const Connection* selected_connection() const override { return nullptr; } + absl::optional GetSelectedCandidatePair() + const override { + return absl::nullopt; + } // Fake PacketTransportInternal implementation. bool writable() const override { return writable_; } diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index 630848f6e6..7f1d70bb94 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -263,8 +263,14 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { // absl::optional if there is none. virtual absl::optional GetRttEstimate() = 0; + // TODO(qingsi): Remove this method once Chrome does not depend on it anymore. virtual const Connection* selected_connection() const = 0; + // Returns the selected candidate pair, or an empty absl::optional if there is + // none. + virtual absl::optional GetSelectedCandidatePair() + const = 0; + sigslot::signal1 SignalGatheringState; // Handles sending and receiving of candidates. diff --git a/p2p/base/mock_ice_transport.h b/p2p/base/mock_ice_transport.h index 6aeb95027c..a28c796970 100644 --- a/p2p/base/mock_ice_transport.h +++ b/p2p/base/mock_ice_transport.h @@ -63,6 +63,10 @@ class MockIceTransport : public IceTransportInternal { void SetIceConfig(const IceConfig& config) override {} absl::optional GetRttEstimate() override { return absl::nullopt; } const Connection* selected_connection() const override { return nullptr; } + absl::optional GetSelectedCandidatePair() + const override { + return absl::nullopt; + } void MaybeStartGathering() override {} void AddRemoteCandidate(const Candidate& candidate) override {} void RemoveRemoteCandidate(const Candidate& candidate) override {} diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 6356859370..01caaa9b10 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -380,6 +380,16 @@ absl::optional P2PTransportChannel::GetRttEstimate() { } } +absl::optional +P2PTransportChannel::GetSelectedCandidatePair() const { + RTC_DCHECK_RUN_ON(network_thread_); + if (selected_connection_ == nullptr) { + return absl::nullopt; + } + + return selected_connection_->ToCandidatePairAndSanitizeIfNecessary(); +} + // A channel is considered ICE completed once there is at most one active // connection per network and at least one active connection. IceTransportState P2PTransportChannel::ComputeState() const { @@ -1965,7 +1975,6 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, } // Create event for candidate pair change. - if (selected_connection_) { CandidatePairChangeEvent pair_change; pair_change.reason = reason; @@ -1975,7 +1984,6 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, selected_connection_->last_data_received(); SignalCandidatePairChanged(pair_change); } - SignalNetworkRouteChanged(network_route_); } diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 0546b36e3d..f9ea59d12e 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -135,6 +135,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { std::vector* candidate_stats_list) override; absl::optional GetRttEstimate() override; const Connection* selected_connection() const override; + absl::optional GetSelectedCandidatePair() const override; // TODO(honghaiz): Remove this method once the reference of it in // Chromoting is removed. diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index a8e83bac60..1b9347565f 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -4991,6 +4991,59 @@ TEST_F(P2PTransportChannelTest, DestroyChannels(); } +// A similar test as above to check the selected candidate pair is sanitized +// when it is queried via GetSelectedCandidatePair. +TEST_F(P2PTransportChannelTest, + SelectedCandidatePairSanitizedWhenMdnsObfuscationEnabled) { + NiceMock mock_async_resolver; + webrtc::MockAsyncResolverFactory mock_async_resolver_factory; + EXPECT_CALL(mock_async_resolver_factory, Create()) + .WillOnce(Return(&mock_async_resolver)); + + // ep1 and ep2 will gather host candidates with addresses + // kPublicAddrs[0] and kPublicAddrs[1], respectively. + ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); + // ICE parameter will be set up when creating the channels. + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); + GetEndpoint(0)->network_manager_.set_mdns_responder( + absl::make_unique(rtc::Thread::Current())); + GetEndpoint(1)->async_resolver_factory_ = &mock_async_resolver_factory; + CreateChannels(); + // Pause sending candidates from both endpoints until we find out what port + // number is assigned to ep1's host candidate. + PauseCandidates(0); + PauseCandidates(1); + ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout); + const auto& candidates_data = GetEndpoint(0)->saved_candidates_[0]; + ASSERT_EQ(1u, candidates_data->candidates.size()); + const auto& local_candidate_ep1 = candidates_data->candidates[0]; + ASSERT_TRUE(local_candidate_ep1.type() == LOCAL_PORT_TYPE); + // This is the underlying private IP address of the same candidate at ep1, + // and let the mock resolver of ep2 receive the correct resolution. + rtc::SocketAddress resolved_address_ep1(local_candidate_ep1.address()); + resolved_address_ep1.SetResolvedIP(kPublicAddrs[0].ipaddr()); + EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(resolved_address_ep1), Return(true))); + ResumeCandidates(0); + ResumeCandidates(1); + + ASSERT_TRUE_WAIT(ep1_ch1()->selected_connection() != nullptr && + ep2_ch1()->selected_connection() != nullptr, + kMediumTimeout); + + const auto pair_ep1 = ep1_ch1()->GetSelectedCandidatePair(); + ASSERT_TRUE(pair_ep1.has_value()); + EXPECT_EQ(LOCAL_PORT_TYPE, pair_ep1->local_candidate().type()); + EXPECT_TRUE(pair_ep1->local_candidate().address().IsUnresolvedIP()); + + const auto pair_ep2 = ep2_ch1()->GetSelectedCandidatePair(); + ASSERT_TRUE(pair_ep2.has_value()); + EXPECT_EQ(LOCAL_PORT_TYPE, pair_ep2->remote_candidate().type()); + EXPECT_TRUE(pair_ep2->remote_candidate().address().IsUnresolvedIP()); + + DestroyChannels(); +} + class MockMdnsResponder : public webrtc::MdnsResponderInterface { public: MOCK_METHOD2(CreateNameForAddress,