Update mDNS service binding

This is a followup patch to:
https://chromium-review.googlesource.com/c/openscreen/+/2643397

To make it RFC compliant, we now send to the multicast
group address, not to ANY. We also bind to ANY address instead of the
local address of the interface, since binding filters multicast traffic
to only bound addresses. Joining the multicast group is what filters
the traffic instead.

Bug: b/178102949
Change-Id: I1d7e44b5e90d2f1f1d395cb9a8a689edf16bfafb
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2665304
Reviewed-by: Ryan Keane <rwkeane@google.com>
Commit-Queue: Jordan Bayles <jophba@chromium.org>

3 files changed, 19 insertions, 18 deletions

diff --git a/cast/common/discovery/e2e_test/tests.cc b/cast/common/discovery/e2e_test/tests.cc
index f39c39d5..9a02053b 100644
--- a/cast/common/discovery/e2e_test/tests.cc
+++ b/cast/common/discovery/e2e_test/tests.cc
@@ -7,6 +7,10 @@
 #include <map>
 #include <string>
 
+// NOTE: although we use gtest here, prefer OSP_CHECKs to
+// ASSERTS due to asynchronous concerns around test failures.
+// Although this causes the entire test binary to fail instead of
+// just a single test, it makes debugging easier/possible.
 #include "cast/common/public/service_info.h"
 #include "discovery/common/config.h"
 #include "discovery/common/reporting_client.h"
@@ -118,22 +122,19 @@ class FailOnErrorReporting : public discovery::ReportingClient {
 };
 
 discovery::Config GetConfigSettings() {
-  discovery::Config config;
-
   // Get the loopback interface to run on.
-  absl::optional<InterfaceInfo> loopback = GetLoopbackInterfaceForTesting();
-  OSP_CHECK(loopback.has_value());
+  InterfaceInfo loopback = GetLoopbackInterfaceForTesting().value();
+  OSP_LOG_INFO << "Selected network interface for testing: " << loopback;
   discovery::Config::NetworkInfo::AddressFamilies address_families =
       discovery::Config::NetworkInfo::kNoAddressFamily;
-  if (loopback->GetIpAddressV4()) {
+  if (loopback.GetIpAddressV4()) {
     address_families |= discovery::Config::NetworkInfo::kUseIpV4;
   }
-  if (loopback->GetIpAddressV6()) {
+  if (loopback.GetIpAddressV6()) {
     address_families |= discovery::Config::NetworkInfo::kUseIpV6;
   }
-  config.network_info.push_back({loopback.value(), address_families});
 
-  return config;
+  return discovery::Config{{{std::move(loopback), address_families}}};
 }
 
 class DiscoveryE2ETest : public testing::Test {
diff --git a/discovery/mdns/mdns_service_impl.cc b/discovery/mdns/mdns_service_impl.cc
index e6a86d5b..6d94c3c7 100644
--- a/discovery/mdns/mdns_service_impl.cc
+++ b/discovery/mdns/mdns_service_impl.cc
@@ -42,11 +42,12 @@ MdnsServiceImpl::MdnsServiceImpl(TaskRunner* task_runner,
   // Create all UDP sockets needed for this object. They should not yet be bound
   // so that they do not send or receive data until the objects on which their
   // callback depends is initialized.
+  // NOTE: we bind to the Any addresses here because traffic is filtered by
+  // the multicast join calls.
   if (network_info.supported_address_families & Config::NetworkInfo::kUseIpV4) {
-    ErrorOr<std::unique_ptr<UdpSocket>> socket =
-        UdpSocket::Create(task_runner, this,
-                          IPEndpoint{network_info.interface.GetIpAddressV4(),
-                                     kDefaultMulticastPort});
+    ErrorOr<std::unique_ptr<UdpSocket>> socket = UdpSocket::Create(
+        task_runner, this,
+        IPEndpoint{IPAddress::kAnyV4(), kDefaultMulticastPort});
     OSP_DCHECK(!socket.is_error());
     OSP_DCHECK(socket.value().get());
     OSP_DCHECK(socket.value()->IsIPv4());
@@ -55,10 +56,9 @@ MdnsServiceImpl::MdnsServiceImpl(TaskRunner* task_runner,
   }
 
   if (network_info.supported_address_families & Config::NetworkInfo::kUseIpV6) {
-    ErrorOr<std::unique_ptr<UdpSocket>> socket =
-        UdpSocket::Create(task_runner, this,
-                          IPEndpoint{network_info.interface.GetIpAddressV6(),
-                                     kDefaultMulticastPort});
+    ErrorOr<std::unique_ptr<UdpSocket>> socket = UdpSocket::Create(
+        task_runner, this,
+        IPEndpoint{IPAddress::kAnyV6(), kDefaultMulticastPort});
     OSP_DCHECK(!socket.is_error());
     OSP_DCHECK(socket.value().get());
     OSP_DCHECK(socket.value()->IsIPv6());
diff --git a/discovery/mdns/public/mdns_constants.h b/discovery/mdns/public/mdns_constants.h
index c65bf4df..c828d3a0 100644
--- a/discovery/mdns/public/mdns_constants.h
+++ b/discovery/mdns/public/mdns_constants.h
@@ -54,9 +54,9 @@ const IPAddress kDefaultMulticastGroupIPv6{
 
 // The send address for multicast mDNS should be the any address (0.*) on the
 // default mDNS multicast port.
-const IPEndpoint kMulticastSendIPv4Endpoint{IPAddress::kAnyV4(),
+const IPEndpoint kMulticastSendIPv4Endpoint{kDefaultMulticastGroupIPv4,
                                             kDefaultMulticastPort};
-const IPEndpoint kMulticastSendIPv6Endpoint{IPAddress::kAnyV6(),
+const IPEndpoint kMulticastSendIPv6Endpoint{kDefaultMulticastGroupIPv6,
                                             kDefaultMulticastPort};
 
 // IPv4 group address for joining cast-specific site-local mDNS multicast group,