commit 09fbf5cc1bfc1062aaf752cb7a9df622ca42f188
author Boris Zbarsky <bzbarsky@apple.com> Thu Mar 14 22:06:53 2024 -0400
committer GitHub <noreply@github.com> Thu Mar 14 19:06:53 2024 -0700
tree 0d13e695ad51080e71f90505f0957d4a8703be75
parent 22901a1996f636c5825825bb3d0e7376a8c201f1

Stop forcing a particular interface ID when sending response messages. (#32581)

Because we were storing the PeerAddress in the session when getting a message,
we effectively pinned sessions to particular interface ids at that point.

This can lead to routing failures.  We should only be pinning to interface IDs
for link-local addresses, just like we do for initial IP resolution via DNS-SD.

src/transport/SessionManager.cpp

diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp
index 085fda9..78f9d98 100644
--- a/src/transport/SessionManager.cpp
+++ b/src/transport/SessionManager.cpp
@@ -57,6 +57,20 @@
 
 namespace {
 Global<GroupPeerTable> gGroupPeerTable;
+
+// Helper function that strips off the interface ID from a peer address that is
+// not an IPv6 link-local address.  For any other address type we should rely on
+// the device's routing table to route messages sent.  Forcing messages down a
+// specific interface might fail with "no route to host".
+void CorrectPeerAddressInterfaceID(Transport::PeerAddress & peerAddress)
+{
+    if (peerAddress.GetIPAddress().IsIPv6LinkLocal())
+    {
+        return;
+    }
+    peerAddress.SetInterface(Inet::InterfaceId::Null());
+}
+
 } // namespace
 
 uint32_t EncryptedPacketBufferHandle::GetMessageCounter() const
@@ -633,7 +647,9 @@
 
     const SessionHandle & session                        = optionalSession.Value();
     Transport::UnauthenticatedSession * unsecuredSession = session->AsUnauthenticatedSession();
-    unsecuredSession->SetPeerAddress(peerAddress);
+    Transport::PeerAddress mutablePeerAddress            = peerAddress;
+    CorrectPeerAddressInterfaceID(mutablePeerAddress);
+    unsecuredSession->SetPeerAddress(mutablePeerAddress);
     SessionMessageDelegate::DuplicateMessage isDuplicate = SessionMessageDelegate::DuplicateMessage::No;
 
     unsecuredSession->MarkActiveRx();
@@ -766,12 +782,11 @@
         secureSession->GetSessionMessageCounter().GetPeerMessageCounter().CommitEncryptedUnicast(packetHeader.GetMessageCounter());
     }
 
-    // TODO: once mDNS address resolution is available reconsider if this is required
-    // This updates the peer address once a packet is received from a new address
-    // and serves as a way to auto-detect peer changing IPs.
-    if (secureSession->GetPeerAddress() != peerAddress)
+    Transport::PeerAddress mutablePeerAddress = peerAddress;
+    CorrectPeerAddressInterfaceID(mutablePeerAddress);
+    if (secureSession->GetPeerAddress() != mutablePeerAddress)
     {
-        secureSession->SetPeerAddress(peerAddress);
+        secureSession->SetPeerAddress(mutablePeerAddress);
     }
 
     if (mCB != nullptr)