diff --git a/p2p/base/p2p_constants.cc b/p2p/base/p2p_constants.cc index 619b73922a..3414939a6f 100644 --- a/p2p/base/p2p_constants.cc +++ b/p2p/base/p2p_constants.cc @@ -41,6 +41,8 @@ const char CONNECTIONROLE_PASSIVE_STR[] = "passive"; const char CONNECTIONROLE_ACTPASS_STR[] = "actpass"; const char CONNECTIONROLE_HOLDCONN_STR[] = "holdconn"; +const char LOCAL_TLD[] = ".local"; + const int MIN_CHECK_RECEIVING_INTERVAL = 50; const int RECEIVING_TIMEOUT = MIN_CHECK_RECEIVING_INTERVAL * 50; const int RECEIVING_SWITCHING_DELAY = 1000; diff --git a/p2p/base/p2p_constants.h b/p2p/base/p2p_constants.h index a2be32de6f..07257d5e18 100644 --- a/p2p/base/p2p_constants.h +++ b/p2p/base/p2p_constants.h @@ -48,6 +48,9 @@ extern const char CONNECTIONROLE_PASSIVE_STR[]; extern const char CONNECTIONROLE_ACTPASS_STR[]; extern const char CONNECTIONROLE_HOLDCONN_STR[]; +// RFC 6762, the .local pseudo-top-level domain used for mDNS names. +extern const char LOCAL_TLD[]; + // Constants for time intervals are in milliseconds unless otherwise stated. // // Most of the following constants are the default values of IceConfig diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index a1c14505e5..f95e7abaff 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -16,6 +16,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/strings/match.h" #include "api/candidate.h" #include "logging/rtc_event_log/ice_logger.h" #include "p2p/base/candidate_pair_interface.h" @@ -2700,10 +2701,9 @@ Candidate P2PTransportChannel::SanitizeLocalCandidate( Candidate P2PTransportChannel::SanitizeRemoteCandidate( const Candidate& c) const { RTC_DCHECK_RUN_ON(network_thread_); - // If the remote endpoint signaled us a hostname host candidate, we assume it + // If the remote endpoint signaled us an mDNS candidate, we assume it // is supposed to be sanitized. - bool use_hostname_address = - c.type() == LOCAL_PORT_TYPE && !c.address().hostname().empty(); + bool use_hostname_address = absl::EndsWith(c.address().hostname(), LOCAL_TLD); // Remove the address for prflx remote candidates. See // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats. use_hostname_address |= c.type() == PRFLX_PORT_TYPE; diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 8f7fd4aa8a..0c3474bb32 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -5247,6 +5247,59 @@ TEST_F(P2PTransportChannelTest, DestroyChannels(); } +TEST_F(P2PTransportChannelTest, + NoPairOfLocalRelayCandidateWithRemoteMdnsCandidate) { + const int kOnlyRelayPorts = cricket::PORTALLOCATOR_DISABLE_UDP | + cricket::PORTALLOCATOR_DISABLE_STUN | + cricket::PORTALLOCATOR_DISABLE_TCP; + // We use one endpoint to test the behavior of adding remote candidates, and + // this endpoint only gathers relay candidates. + ConfigureEndpoints(OPEN, OPEN, kOnlyRelayPorts, kDefaultPortAllocatorFlags); + GetEndpoint(0)->cd1_.ch_.reset(CreateChannel( + 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1])); + IceConfig config; + // Start gathering and we should have only a single relay port. + ep1_ch1()->SetIceConfig(config); + ep1_ch1()->MaybeStartGathering(); + EXPECT_EQ_WAIT(IceGatheringState::kIceGatheringComplete, + ep1_ch1()->gathering_state(), kDefaultTimeout); + EXPECT_EQ(1u, ep1_ch1()->ports().size()); + // Add a plain remote host candidate and three remote mDNS candidates with the + // host, srflx and relay types. Note that the candidates differ in their + // ports. + cricket::Candidate host_candidate = CreateUdpCandidate( + LOCAL_PORT_TYPE, "1.1.1.1", 1 /* port */, 0 /* priority */); + ep1_ch1()->AddRemoteCandidate(host_candidate); + + std::vector mdns_candidates; + mdns_candidates.push_back(CreateUdpCandidate(LOCAL_PORT_TYPE, "example.local", + 2 /* port */, 0 /* priority */)); + mdns_candidates.push_back(CreateUdpCandidate(STUN_PORT_TYPE, "example.local", + 3 /* port */, 0 /* priority */)); + mdns_candidates.push_back(CreateUdpCandidate(RELAY_PORT_TYPE, "example.local", + 4 /* port */, 0 /* priority */)); + // We just resolve the hostname to 1.1.1.1, and add the candidates with this + // address directly to simulate the process of adding remote candidates with + // the name resolution. + for (auto& mdns_candidate : mdns_candidates) { + rtc::SocketAddress resolved_address(mdns_candidate.address()); + resolved_address.SetResolvedIP(0x1111); // 1.1.1.1 + mdns_candidate.set_address(resolved_address); + EXPECT_FALSE(mdns_candidate.address().IsUnresolvedIP()); + ep1_ch1()->AddRemoteCandidate(mdns_candidate); + } + + // All remote candidates should have been successfully added. + EXPECT_EQ(4u, ep1_ch1()->remote_candidates().size()); + + // Expect that there is no connection paired with any mDNS candidate. + ASSERT_EQ(1u, ep1_ch1()->connections().size()); + ASSERT_NE(nullptr, ep1_ch1()->connections()[0]); + EXPECT_EQ( + "1.1.1.1:1", + ep1_ch1()->connections()[0]->remote_candidate().address().ToString()); +} + class MockMdnsResponder : public webrtc::MdnsResponderInterface { public: MOCK_METHOD2(CreateNameForAddress, diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 68535b7373..f0795ee5e5 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -16,8 +16,10 @@ #include #include "absl/algorithm/container.h" +#include "absl/strings/match.h" #include "absl/types/optional.h" #include "p2p/base/connection.h" +#include "p2p/base/p2p_constants.h" #include "p2p/base/stun.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/byte_order.h" @@ -540,11 +542,18 @@ Connection* TurnPort::CreateConnection(const Candidate& remote_candidate, CandidateOrigin origin) { // TURN-UDP can only connect to UDP candidates. if (!SupportsProtocol(remote_candidate.protocol())) { - return NULL; + return nullptr; } if (state_ == STATE_DISCONNECTED || state_ == STATE_RECEIVEONLY) { - return NULL; + return nullptr; + } + + // If the remote endpoint signaled us an mDNS candidate, we do not form a pair + // with the relay candidate to avoid IP leakage in the CreatePermission + // request. + if (absl::EndsWith(remote_candidate.address().hostname(), LOCAL_TLD)) { + return nullptr; } // A TURN port will have two candiates, STUN and TURN. STUN may not @@ -568,7 +577,7 @@ Connection* TurnPort::CreateConnection(const Candidate& remote_candidate, return conn; } } - return NULL; + return nullptr; } bool TurnPort::FailAndPruneConnection(const rtc::SocketAddress& address) {