Sanitize hostname literals when mDNS obfuscation is on. Also applies sanitizing to prflx candidates, not just local ones. Also add tests for the port allocator Sanitize function. Bug: chromium:1218346 Change-Id: Ifbc7843cd6e289c09ca72b6ec610a34bbbf7e04e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222581 Reviewed-by: Henrik Boström <hbos@webrtc.org> Commit-Queue: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/master@{#34292}
diff --git a/api/candidate.cc b/api/candidate.cc index c857f89..d5fe3a0 100644 --- a/api/candidate.cc +++ b/api/candidate.cc
@@ -12,6 +12,7 @@ #include "rtc_base/helpers.h" #include "rtc_base/ip_address.h" +#include "rtc_base/logging.h" #include "rtc_base/strings/string_builder.h" namespace cricket { @@ -129,9 +130,21 @@ bool filter_related_address) const { Candidate copy(*this); if (use_hostname_address) { - rtc::SocketAddress hostname_only_addr(address().hostname(), - address().port()); - copy.set_address(hostname_only_addr); + rtc::IPAddress ip; + if (address().hostname().empty()) { + // IP needs to be redacted, but no hostname available. + rtc::SocketAddress redacted_addr("redacted-ip.invalid", address().port()); + copy.set_address(redacted_addr); + } else if (IPFromString(address().hostname(), &ip)) { + // The hostname is an IP literal, and needs to be redacted too. + rtc::SocketAddress redacted_addr("redacted-literal.invalid", + address().port()); + copy.set_address(redacted_addr); + } else { + rtc::SocketAddress hostname_only_addr(address().hostname(), + address().port()); + copy.set_address(hostname_only_addr); + } } if (filter_related_address) { copy.set_related_address(
diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 9e0e333..efe9a53 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h
@@ -238,10 +238,19 @@ bool initialized() const { return initialized_; } + // For testing: Manipulate MdnsObfuscationEnabled() + bool MdnsObfuscationEnabled() const override { + return mdns_obfuscation_enabled_; + } + void SetMdnsObfuscationEnabledForTesting(bool enabled) { + mdns_obfuscation_enabled_ = enabled; + } + private: rtc::Thread* network_thread_; rtc::PacketSocketFactory* factory_; std::unique_ptr<rtc::BasicPacketSocketFactory> owned_factory_; + bool mdns_obfuscation_enabled_ = false; }; } // namespace cricket
diff --git a/p2p/base/port_allocator.cc b/p2p/base/port_allocator.cc index b13896c..d8ff637 100644 --- a/p2p/base/port_allocator.cc +++ b/p2p/base/port_allocator.cc
@@ -317,7 +317,8 @@ // For a local host candidate, we need to conceal its IP address candidate if // the mDNS obfuscation is enabled. bool use_hostname_address = - c.type() == LOCAL_PORT_TYPE && MdnsObfuscationEnabled(); + (c.type() == LOCAL_PORT_TYPE || c.type() == PRFLX_PORT_TYPE) && + MdnsObfuscationEnabled(); // If adapter enumeration is disabled or host candidates are disabled, // clear the raddr of STUN candidates to avoid local address leakage. bool filter_stun_related_address =
diff --git a/p2p/base/port_allocator_unittest.cc b/p2p/base/port_allocator_unittest.cc index 70946a3..cbac5cc 100644 --- a/p2p/base/port_allocator_unittest.cc +++ b/p2p/base/port_allocator_unittest.cc
@@ -305,3 +305,56 @@ credentials[0].pwd)); allocator_->DiscardCandidatePool(); } + +// Constants for testing candidates +const char kIpv4Address[] = "12.34.56.78"; +const char kIpv4AddressWithPort[] = "12.34.56.78:443"; + +TEST_F(PortAllocatorTest, SanitizeEmptyCandidateDefaultConfig) { + cricket::Candidate input; + cricket::Candidate output = allocator_->SanitizeCandidate(input); + EXPECT_EQ("", output.address().ipaddr().ToString()); +} + +TEST_F(PortAllocatorTest, SanitizeIpv4CandidateDefaultConfig) { + cricket::Candidate input(1, "udp", rtc::SocketAddress(kIpv4Address, 443), 1, + "username", "password", cricket::LOCAL_PORT_TYPE, 1, + "foundation", 1, 1); + cricket::Candidate output = allocator_->SanitizeCandidate(input); + EXPECT_EQ(kIpv4AddressWithPort, output.address().ToString()); + EXPECT_EQ(kIpv4Address, output.address().ipaddr().ToString()); +} + +TEST_F(PortAllocatorTest, SanitizeIpv4CandidateMdnsObfuscationEnabled) { + allocator_->SetMdnsObfuscationEnabledForTesting(true); + cricket::Candidate input(1, "udp", rtc::SocketAddress(kIpv4Address, 443), 1, + "username", "password", cricket::LOCAL_PORT_TYPE, 1, + "foundation", 1, 1); + cricket::Candidate output = allocator_->SanitizeCandidate(input); + EXPECT_NE(kIpv4AddressWithPort, output.address().ToString()); + EXPECT_EQ("", output.address().ipaddr().ToString()); +} + +TEST_F(PortAllocatorTest, SanitizePrflxCandidateMdnsObfuscationEnabled) { + allocator_->SetMdnsObfuscationEnabledForTesting(true); + // Create the candidate from an IP literal. This populates the hostname. + cricket::Candidate input(1, "udp", rtc::SocketAddress(kIpv4Address, 443), 1, + "username", "password", cricket::PRFLX_PORT_TYPE, 1, + "foundation", 1, 1); + cricket::Candidate output = allocator_->SanitizeCandidate(input); + EXPECT_NE(kIpv4AddressWithPort, output.address().ToString()); + EXPECT_EQ("", output.address().ipaddr().ToString()); +} + +TEST_F(PortAllocatorTest, SanitizeIpv4NonLiteralMdnsObfuscationEnabled) { + // Create the candidate with an empty hostname. + allocator_->SetMdnsObfuscationEnabledForTesting(true); + rtc::IPAddress ip; + EXPECT_TRUE(IPFromString(kIpv4Address, &ip)); + cricket::Candidate input(1, "udp", rtc::SocketAddress(ip, 443), 1, "username", + "password", cricket::LOCAL_PORT_TYPE, 1, + "foundation", 1, 1); + cricket::Candidate output = allocator_->SanitizeCandidate(input); + EXPECT_NE(kIpv4AddressWithPort, output.address().ToString()); + EXPECT_EQ("", output.address().ipaddr().ToString()); +}