Pin mDNS sends per interface and rank LAN addresses

companion/src/listener.ts

@@ -13,18 +13,42 @@ import { execFile } from "node:child_process";
 import { homedir, networkInterfaces } from "node:os";
 import { join } from "node:path";
 
-/** Every IPv4 address a phone on the same network could dial. Link-local
- * (169.254/16) is dropped: it means DHCP failed and nothing will reach us. */
-export function lanAddresses(): string[] {
-  const out: string[] = [];
-  for (const entries of Object.values(networkInterfaces())) {
+/** Interfaces that exist to tunnel, bridge or mesh traffic — utun (Tailscale
+ * and every other VPN), vmnet/bridge (VMs, containers, internet sharing),
+ * awdl/llw (AirDrop's side channels), feth/tap/tun. Their addresses stay in
+ * the list, because the tailnet one is exactly what a phone off-network
+ * dials — but a phone on the same wifi can reach none of them, so none of
+ * them may come first. */
+const VIRTUAL_INTERFACES = /^(utun|tun|tap|bridge|vmnet|awdl|llw|feth)/;
+
+/** Lower sorts earlier. `en0`, `en1`, … are macOS's built-in wifi and
+ * ethernet — the networks a phone is actually standing on. */
+const interfaceRank = (name: string): number => {
+  if (/^en\d+$/.test(name)) return 0;
+  if (VIRTUAL_INTERFACES.test(name)) return 2;
+  return 1;
+};
+
+/** Every IPv4 address a phone on the same network could dial, most reachable
+ * first. Link-local (169.254/16) is dropped: it means DHCP failed and nothing
+ * will reach us.
+ *
+ * Ranked, not merely collected: `networkInterfaces()` promises nothing about
+ * order, callers put the first non-tailnet entry into the pairing QR, and on
+ * a Mac with a VPN or a VM running the first entry can be a utun or bridge100
+ * address the phone cannot route to. Real interfaces lead, tunnels and
+ * bridges trail; the sort is stable, so enumeration order still breaks ties.
+ * The parameter exists for tests — the interface table is the machine's. */
+export function lanAddresses(interfaces = networkInterfaces()): string[] {
+  const found: Array<{ rank: number; address: string }> = [];
+  for (const [name, entries] of Object.entries(interfaces)) {
     for (const entry of entries ?? []) {
       if (entry.family !== "IPv4" || entry.internal) continue;
       if (entry.address.startsWith("169.254.")) continue;
-      out.push(entry.address);
+      found.push({ rank: interfaceRank(name), address: entry.address });
     }
   }
-  return out;
+  return found.sort((a, b) => a.rank - b.rank).map((entry) => entry.address);
 }
 
 /** Tailscale hands its nodes an address in 100.64.0.0/10 — the CGNAT range

companion/src/mdns.ts

@@ -15,7 +15,7 @@
 // network, and the cost of that is a duplicate row in a picker rather than
 // anything broken.
 import { createHash } from "node:crypto";
-import { createSocket, type Socket } from "node:dgram";
+import { createSocket } from "node:dgram";
 import { hostname, networkInterfaces } from "node:os";
 
 import { lanAddresses } from "./listener.ts";
@@ -424,28 +424,53 @@ export function advertisableAddresses(): string[] {
 
 // ── the responder ──────────────────────────────────────────────────────
 
-/** Bind port and mode. Both exist for tests: the real thing is 5353,
- * multicast, and has no reason to be anything else. */
+/** The slice of `dgram.Socket` the responder drives. A structural seam
+ * rather than the concrete class, because the multicast behaviour that
+ * matters most — every group send pinned to each advertised interface —
+ * is observable only from the socket's side of the call, and CI runners
+ * rarely route multicast at all. The real socket satisfies this shape. */
+export interface ResponderSocket {
+  on(event: "error", listener: (error: Error) => void): void;
+  on(event: "message", listener: (message: Buffer, remote: { address: string; port: number }) => void): void;
+  once(event: "error", listener: (error: Error) => void): void;
+  bind(port: number, callback?: () => void): void;
+  setMulticastTTL(ttl: number): void;
+  addMembership(group: string, membershipInterface?: string): void;
+  setMulticastInterface(multicastInterface: string): void;
+  send(packet: Buffer, port: number, address: string, callback?: (error: Error | null) => void): void;
+  close(callback?: () => void): void;
+  address(): { port: number };
+}
+
+/** Bind port and mode. All of these exist for tests: the real thing is 5353,
+ * multicast, on a real socket, and has no reason to be anything else. */
 export interface ResponderOptions {
   /** Test rigs bind an ephemeral port and skip the group join; the packet
    * handling below is the same code either way. */
   port?: number;
   multicast?: boolean;
+  /** Test rigs substitute a recording socket: interface pinning never
+   * reaches the wire in CI, so the socket is where it can be asserted. */
+  socketFactory?: () => ResponderSocket;
 }
 
 /** A Bonjour responder, small enough to read: it announces one service, and
  * answers questions about that service from the local link. No dependency,
  * because a discovery nicety is not worth a supply chain. */
 export class MdnsResponder {
-  private socket: Socket | null = null;
+  private socket: ResponderSocket | null = null;
   private service: ServiceInfo | null = null;
   private timers: ReturnType<typeof setTimeout>[] = [];
+  /** Multicast sends, strictly one after another — see `send`. */
+  private sendQueue: Promise<void> = Promise.resolve();
   private readonly port: number;
   private readonly multicast: boolean;
+  private readonly socketFactory: () => ResponderSocket;
 
   constructor(options: ResponderOptions = {}) {
     this.port = options.port ?? MDNS_PORT;
     this.multicast = options.multicast ?? true;
+    this.socketFactory = options.socketFactory ?? (() => createSocket({ type: "udp4", reuseAddr: true }));
   }
 
   /** Whether the socket is up. False is normal and not an error. */
@@ -469,7 +494,7 @@ export class MdnsResponder {
     await this.stop();
     if (!service.addresses.length) return false;
 
-    const socket = createSocket({ type: "udp4", reuseAddr: true });
+    const socket = this.socketFactory();
     // Bind errors arrive as events, and an unhandled 'error' on a socket
     // is an uncaught exception that would take the harness with it.
     socket.on("error", () => void this.stop());
@@ -553,7 +578,7 @@ export class MdnsResponder {
         const timer = setTimeout(finish, GOODBYE_FLUSH_MS);
         timer.unref?.();
         try {
-          this.send(socket, encodeResponse(announcement(service), [], { ttl: 0 }), () => {
+          this.send(socket, encodeResponse(announcement(service), [], { ttl: 0 }), service.addresses, () => {
             clearTimeout(timer);
             finish();
           });
@@ -577,7 +602,7 @@ export class MdnsResponder {
   private announce() {
     if (!this.socket || !this.service) return;
     try {
-      this.send(this.socket, encodeResponse(announcement(this.service)));
+      this.send(this.socket, encodeResponse(announcement(this.service)), this.service.addresses);
     } catch {
       /* the interface may have gone away between timer and send */
     }
@@ -608,27 +633,68 @@ export class MdnsResponder {
       legacy ? { id: message.id, questions: message.questions } : {},
     );
     try {
+      // A unicast answer goes back the way the question came — the kernel
+      // routes it like any datagram, and pinning it to an advertised
+      // interface would be exactly wrong. Only group sends are pinned.
       if (unicast) this.socket.send(packet, fromPort, from);
-      else this.send(this.socket, packet);
+      else this.send(this.socket, packet, this.service.addresses);
     } catch {
       /* a send failure is one lost answer; the asker retries */
     }
   }
 
-  /** Multicast a packet to the group.
+  /** Multicast a packet to the group, once per advertised interface.
    *
    * Always to 5353, whatever port this responder is bound to: the destination
    * is where mDNS listens, not where we happen to be. Using the bind port
    * sent announcements to a port with nobody on it — and threw outright when
    * that port was 0, which is what an ephemeral bind gives you.
    *
+   * Once per interface, because a bare group send leaves on whichever single
+   * interface the kernel routes 224.0.0.251 to — with a VPN or a VM bridge
+   * up, that can be a network the phone is not on, and the responder then
+   * believes it is advertising while nobody can hear it. Joining the group
+   * per interface (in `advertise`) only fixes the receive side; the send
+   * side is pinned here with `setMulticastInterface`.
+   *
+   * And strictly serialized, because `setMulticastInterface` redirects every
+   * *subsequent* send on the socket while `send` itself completes a tick
+   * later: two bursts running interleaved could race their pins and both
+   * leave on whichever interface was pinned last. One queue, pin, wait for
+   * the datagram out, pin the next.
+   *
    * Unicast mode has no group to announce to, so there is nothing to send;
    * it exists for tests, which ask directly and are answered in `handle`. */
-  private send(socket: Socket, packet: Buffer, done?: (error: Error | null) => void) {
+  private send(
+    socket: ResponderSocket,
+    packet: Buffer,
+    addresses: string[],
+    done?: (error: Error | null) => void,
+  ) {
     if (!this.multicast) {
       done?.(null);
       return;
     }
-    socket.send(packet, MDNS_PORT, MDNS_ADDRESS, done);
+    this.sendQueue = this.sendQueue.then(async () => {
+      for (const address of addresses) {
+        await new Promise<void>((resolve) => {
+          try {
+            socket.setMulticastInterface(address);
+          } catch {
+            // the interface vanished between enumeration and send — sleep,
+            // VPN drop, cable pulled. Its packet is lost either way; the
+            // remaining interfaces still matter, so skip rather than throw.
+            resolve();
+            return;
+          }
+          try {
+            socket.send(packet, MDNS_PORT, MDNS_ADDRESS, () => resolve());
+          } catch {
+            resolve();
+          }
+        });
+      }
+      done?.(null);
+    });
   }
 }

companion/test/mdns.test.ts

@@ -3,9 +3,11 @@
 // invisible — so it is tested byte by byte, against packets built by hand
 // rather than by the encoder under test.
 import { createSocket } from "node:dgram";
+import { EventEmitter } from "node:events";
+import type { NetworkInterfaceInfoIPv4 } from "node:os";
 import { describe, expect, it } from "vitest";
 
-import { tailscaleAddress } from "../src/listener.ts";
+import { lanAddresses, tailscaleAddress } from "../src/listener.ts";
 import {
   advertisableAddresses,
   announcement,
@@ -349,6 +351,167 @@ describe("MdnsResponder", () => {
   });
 });
 
+// The outbound half of multicast. Joining the group per interface only fixes
+// what we *hear*; every group send must also be pinned per interface, or the
+// kernel routes 224.0.0.251 out of exactly one interface of its choosing —
+// with a VPN or VM bridge up, a network the phone is not on. None of this
+// reaches the wire in CI, so a recording socket is where it gets asserted.
+describe("multicast interface pinning", () => {
+  /** A socket that logs what the responder does to it. Satisfies
+   * `ResponderSocket` structurally — no casting required. */
+  class FakeSocket extends EventEmitter {
+    readonly ops: Array<{ op: "pin" | "send"; address: string; port?: number }> = [];
+    bind(_port: number, callback?: () => void) {
+      callback?.();
+    }
+    setMulticastTTL(_ttl: number) {}
+    addMembership(_group: string, _membershipInterface?: string) {}
+    setMulticastInterface(multicastInterface: string) {
+      this.ops.push({ op: "pin", address: multicastInterface });
+    }
+    send(_packet: Buffer, port: number, address: string, callback?: (error: Error | null) => void) {
+      this.ops.push({ op: "send", address, port });
+      callback?.(null);
+    }
+    close(callback?: () => void) {
+      callback?.();
+    }
+    address() {
+      return { port: 5353 };
+    }
+  }
+
+  const homes = ["192.168.1.42", "10.0.0.7"];
+  /** One announcement burst, as the ops log should show it: pin, send, pin,
+   * send — the pin *before* each send, per advertised interface, because
+   * `setMulticastInterface` redirects the sends that come after it. */
+  const pinnedBurst = homes.flatMap((address) => [
+    { op: "pin", address },
+    { op: "send", address: "224.0.0.251", port: 5353 },
+  ]);
+
+  // The fake's callbacks fire synchronously, so the first announcement (the
+  // 0 ms timer) has fully drained once one later macrotask runs.
+  const drained = () => new Promise((resolve) => setTimeout(resolve, 25));
+
+  it("pins every announcement and goodbye to each advertised interface, in order", async () => {
+    const socket = new FakeSocket();
+    const responder = new MdnsResponder({ socketFactory: () => socket });
+    expect(await responder.advertise({ ...service, addresses: homes })).toBe(true);
+    await drained();
+    expect(socket.ops).toEqual(pinnedBurst);
+
+    // the goodbye withdraws the records everywhere they were announced
+    socket.ops.length = 0;
+    await responder.stop();
+    expect(socket.ops).toEqual(pinnedBurst);
+  });
+
+  it("pins a multicast answer the same way", async () => {
+    const socket = new FakeSocket();
+    const responder = new MdnsResponder({ socketFactory: () => socket });
+    await responder.advertise({ ...service, addresses: homes });
+    await drained();
+    socket.ops.length = 0;
+    try {
+      // port 5353, no QU bit: the answer goes back to the group
+      socket.emit("message", query(SERVICE_NAME, TYPE.PTR), { address: "127.0.0.1", port: 5353 });
+      await drained();
+      expect(socket.ops).toEqual(pinnedBurst);
+    } finally {
+      await responder.stop();
+    }
+  });
+
+  it("sends a unicast answer as-is: routed to the asker, never pinned", async () => {
+    const socket = new FakeSocket();
+    const responder = new MdnsResponder({ socketFactory: () => socket });
+    await responder.advertise({ ...service, addresses: homes });
+    await drained();
+    socket.ops.length = 0;
+    try {
+      // an ephemeral source port marks a legacy resolver (RFC 6762 §6.7)
+      socket.emit("message", query(SERVICE_NAME, TYPE.PTR, { id: 9 }), { address: "127.0.0.1", port: 40404 });
+      await drained();
+      expect(socket.ops).toEqual([{ op: "send", address: "127.0.0.1", port: 40404 }]);
+    } finally {
+      await responder.stop();
+    }
+  });
+
+  it("skips an interface that vanished between enumeration and send", async () => {
+    class VanishingSocket extends FakeSocket {
+      setMulticastInterface(multicastInterface: string) {
+        // what a dropped VPN or pulled cable looks like from here
+        if (multicastInterface === "10.0.0.7") throw new Error("EADDRNOTAVAIL");
+        super.setMulticastInterface(multicastInterface);
+      }
+    }
+    const socket = new VanishingSocket();
+    const responder = new MdnsResponder({ socketFactory: () => socket });
+    await responder.advertise({ ...service, addresses: homes });
+    await drained();
+    try {
+      // the surviving interface still gets its packet, and nothing crashed
+      expect(socket.ops).toEqual([
+        { op: "pin", address: "192.168.1.42" },
+        { op: "send", address: "224.0.0.251", port: 5353 },
+      ]);
+      expect(responder.advertising).toBe(true);
+    } finally {
+      await responder.stop();
+    }
+  });
+});
+
+// Which address leads matters: callers print the first non-tailnet entry into
+// the pairing QR, and `networkInterfaces()` promises nothing about order — a
+// Mac with a VPN or a VM can enumerate utun or bridge100 first, and a QR
+// carrying one of those addresses points the phone at a network it is not on.
+describe("lanAddresses", () => {
+  const ipv4 = (address: string, internal = false): NetworkInterfaceInfoIPv4 => ({
+    address,
+    netmask: "255.255.255.0",
+    family: "IPv4",
+    mac: "00:00:00:00:00:00",
+    internal,
+    cidr: `${address}/24`,
+  });
+
+  it("ranks en0 first however the interfaces enumerate", () => {
+    // deliberately listed worst-first: insertion order must not win
+    const addresses = lanAddresses({
+      utun4: [ipv4("100.102.178.88")],
+      bridge100: [ipv4("192.168.64.1")],
+      vmnet1: [ipv4("192.168.105.1")],
+      en0: [ipv4("192.168.1.42")],
+    });
+    expect(addresses[0]).toBe("192.168.1.42");
+    // the virtual interfaces are kept — the tailnet address is dialable
+    // from off-network — but only after everything real
+    expect(addresses).toEqual(["192.168.1.42", "100.102.178.88", "192.168.64.1", "192.168.105.1"]);
+  });
+
+  it("slots an unrecognized real interface between en0 and the tunnels", () => {
+    expect(
+      lanAddresses({
+        utun0: [ipv4("100.102.178.88")],
+        ap1: [ipv4("172.20.10.1")],
+        en1: [ipv4("192.168.1.42")],
+      }),
+    ).toEqual(["192.168.1.42", "172.20.10.1", "100.102.178.88"]);
+  });
+
+  it("still drops loopback and link-local, wherever they rank", () => {
+    expect(
+      lanAddresses({
+        lo0: [ipv4("127.0.0.1", true)],
+        en0: [ipv4("169.254.7.7")],
+      }),
+    ).toEqual([]);
+  });
+});
+
 // Tailscale hands out 100.64.0.0/10 (RFC 6598 shared address space), which
 // is what makes a tailnet address distinguishable from a LAN one — and
 // worth distinguishing, because it is the address that still works when the