From a4937a96f7e8f9b935cf7433542e14725b90a0b7 Mon Sep 17 00:00:00 2001
From: Erik Boasson <eb@ilities.com>
Date: Wed, 12 Jul 2023 14:18:58 +0200
Subject: [PATCH] Ignore SPDP from non-selected interfaces

Ignore SPDP messages that:

* Provably arrived over a network interface we didn't enable

* Were multicast over a network interface on which did not allow
  multicast discovery.

For an example why this is useful: on Linux, running two proceses one allowing multicast
over loopback but not Ethernet, and one allowing it over Ethernet but not over loopback,
both processes do receive the multicast message despite not having joined the multicast
group on the interface it is sent on.  With this commit, those SPDP messages are ignored
and the two processes do not discover each other.

The direct reason for this change is to make it possible to have discovery over Ethernet
only with unicast messages, while using relying on multicast over loopback.  This turns
out to be a rather important set-up, and is even the new default in ROS 2.

Signed-off-by: Erik Boasson <eb@ilities.com>
---
 src/core/ddsi/src/ddsi_discovery_spdp.c | 54 +++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/src/core/ddsi/src/ddsi_discovery_spdp.c b/src/core/ddsi/src/ddsi_discovery_spdp.c
index fe79c3a547..699974e596 100644
--- a/src/core/ddsi/src/ddsi_discovery_spdp.c
+++ b/src/core/ddsi/src/ddsi_discovery_spdp.c
@@ -586,6 +586,53 @@ static void make_participants_dependent_on_ddsi2 (struct ddsi_domaingv *gv, cons
   }
 }
 
+enum find_internal_interface_index_result {
+  FIIIR_NO_INFO,
+  FIIIR_NO_MATCH,
+  FIIIR_MATCH
+};
+
+static enum find_internal_interface_index_result find_internal_interface_index (const struct ddsi_domaingv *gv, uint32_t nwstack_if_index, int *internal_if_index) ddsrt_nonnull_all;
+
+static enum find_internal_interface_index_result find_internal_interface_index (const struct ddsi_domaingv *gv, uint32_t nwstack_if_index, int *internal_if_index)
+{
+  if (nwstack_if_index == 0)
+    return FIIIR_NO_INFO;
+  for (int i = 0; i < gv->n_interfaces; i++)
+  {
+    if (gv->interfaces[i].if_index == nwstack_if_index)
+    {
+      *internal_if_index = i;
+      return FIIIR_MATCH;
+    }
+  }
+  return FIIIR_NO_MATCH;
+}
+
+static bool accept_packet_from_interface (const struct ddsi_domaingv *gv, const struct ddsi_receiver_state *rst)
+{
+  int internal_if_index;
+  switch (find_internal_interface_index (gv, rst->pktinfo.if_index, &internal_if_index))
+  {
+    case FIIIR_NO_MATCH:
+      // Don't accept SPDP packets received on a interface outside the enabled ones
+      break;
+    case FIIIR_MATCH:
+      // Accept all unicast packets (except those manifestly received over an interface we are not using)
+      // and multicast packets if we chose to do multicast discovery on the interface over we received it
+      if (!ddsi_is_mcaddr (gv, &rst->pktinfo.dst) || gv->interfaces[internal_if_index].allow_multicast & DDSI_AMC_SPDP)
+        return true;
+      break;
+    case FIIIR_NO_INFO:
+      // We could try to match the source address with an interface. Perhaps the destination address
+      // is available even though the interface index is not, allowing some tricks.  On Linux, Windows
+      // and macOS this shouldn't happen, so rather than complicate things unnecessarily, just accept
+      // the packet like we always used to do.
+      return true;
+  }
+  return false;
+}
+
 static int handle_spdp_alive (const struct ddsi_receiver_state *rst, ddsi_seqno_t seq, ddsrt_wctime_t timestamp, const ddsi_plist_t *datap)
 {
   struct ddsi_domaingv * const gv = rst->gv;
@@ -598,6 +645,13 @@ static int handle_spdp_alive (const struct ddsi_receiver_state *rst, ddsi_seqno_
   dds_duration_t lease_duration;
   unsigned custom_flags = 0;
 
+  // Don't just process any SPDP packet but look at the network interface and uni/multicast
+  // One could refine this even further by also looking at the locators advertised in the
+  // packet, but this should suffice to drop unwanted multicast packets, which is the only
+  // use case I am currently aware of.
+  if (!accept_packet_from_interface (gv, rst))
+    return 0;
+
   /* If advertised domain id or domain tag doesn't match, ignore the message.  Do this first to
      minimize the impact such messages have. */
   {