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}

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 @@ Candidate Candidate::ToSanitizedCopy(bool use_hostname_address,
                                      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(

p2p/base/fake_port_allocator.h

@@ -238,10 +238,19 @@ class FakePortAllocator : public cricket::PortAllocator {
 
   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

p2p/base/port_allocator.cc

@@ -317,7 +317,8 @@ Candidate PortAllocator::SanitizeCandidate(const Candidate& c) const {
   // 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 =

p2p/base/port_allocator_unittest.cc

@@ -305,3 +305,56 @@ TEST_F(PortAllocatorTest, RestrictIceCredentialsChange) {
                                           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());
+}