From 2f09eaf20b68e287a2d235c1662759e19fd572c1 Mon Sep 17 00:00:00 2001 From: woodsonl <65194841+woodsonl@users.noreply.github.com> Date: Sat, 12 Sep 2026 16:35:04 -0500 Subject: [PATCH 1/3] fix: send mDNS responses from source port 5353 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The responder transmits every announcement and reply from a fresh socket bound to an ephemeral port, so datagrams leave with a high source port. RFC 6762 §6 requires responses to originate from 5353, and a router-level mDNS reflector only classifies a datagram as mDNS when it comes from that port: it relays our queries but drops our answers, so nodes on reflected subnets never discover each other. Bind the per-interface send socket to 5353 on Unix via the same SO_REUSEADDR hook the receive socket uses, which lets it coexist with the group-bound receive socket (and any other local responder) on the shared port. Windows keeps the ephemeral-port path. Add regression coverage for the send socket's local port and for binding it alongside the group receive socket. Fixes #1 Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com> --- services/shared/mdns/responder.go | 16 +++++++- services/shared/mdns/responder_test.go | 43 +++++++++++++++++++++ services/shared/mdns/socketreuse_unix.go | 6 +++ services/shared/mdns/socketreuse_windows.go | 7 ++++ services/versions.json | 2 +- 5 files changed, 72 insertions(+), 2 deletions(-) diff --git a/services/shared/mdns/responder.go b/services/shared/mdns/responder.go index 880b98f1..57100157 100644 --- a/services/shared/mdns/responder.go +++ b/services/shared/mdns/responder.go @@ -570,6 +570,20 @@ func (r *Responder) sendUnicast(buf []byte, ifIndex int, to net.Addr) { } } +// openSendConn opens the per-interface socket sendOnInterface transmits from. +// On Unix it binds the local mDNS port (see sendSourcePort) so our datagrams +// originate from 5353 as RFC 6762 §6 requires; on Windows it binds ephemeral, +// where a second socket on 5353 is unreliable. setReuseAddr lets the socket +// share 5353 with Run's receive socket and any other local mDNS responder. +func openSendConn(src net.IP) (*net.UDPConn, error) { + lc := net.ListenConfig{Control: setReuseAddr} + pktConn, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(src.String(), fmt.Sprint(sendSourcePort))) + if err != nil { + return nil, err + } + return pktConn.(*net.UDPConn), nil +} + // sendOnInterface is the core of the Windows send workaround: it transmits buf // from a fresh unicast-bound socket on the given interface (setting the // multicast interface + TTL for group targets), never from the multicast-bound @@ -584,7 +598,7 @@ func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr if err != nil { return err } - conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: src, Port: 0}) + conn, err := openSendConn(src) if err != nil { slog.Debug("mdns: bind failed", "iface", ifi.Name, "ip", src.String(), "err", err) return err diff --git a/services/shared/mdns/responder_test.go b/services/shared/mdns/responder_test.go index eb7b8c76..2eadab33 100644 --- a/services/shared/mdns/responder_test.go +++ b/services/shared/mdns/responder_test.go @@ -4,6 +4,8 @@ package mdns import ( + "context" + "fmt" "net" "testing" @@ -185,6 +187,47 @@ func TestAppendBrowseRRs(t *testing.T) { } } +// TestOpenSendConnBindsMDNSPort pins RFC 6762 §6: on platforms that advertise +// from a well-known port, the send socket's local port is 5353, not ephemeral. +func TestOpenSendConnBindsMDNSPort(t *testing.T) { + if sendSourcePort == 0 { + t.Skip("ephemeral send port on this platform") + } + conn, err := openSendConn(net.IPv4(127, 0, 0, 1)) + if err != nil { + t.Fatalf("openSendConn: %v", err) + } + defer conn.Close() + addr, ok := conn.LocalAddr().(*net.UDPAddr) + if !ok { + t.Fatalf("LocalAddr = %T, want *net.UDPAddr", conn.LocalAddr()) + } + if addr.Port != mdnsPort { + t.Fatalf("send socket local port = %d, want %d", addr.Port, mdnsPort) + } +} + +// TestOpenSendConnCoexistsWithGroupSocket guards the bind path Run relies on: +// the 5353 send socket must open while the multicast-group receive socket is +// already bound to 5353 in the same process. +func TestOpenSendConnCoexistsWithGroupSocket(t *testing.T) { + if sendSourcePort == 0 { + t.Skip("ephemeral send port on this platform") + } + lc := net.ListenConfig{Control: setReuseAddr} + group, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(mdnsGroupV4.String(), fmt.Sprint(mdnsPort))) + if err != nil { + t.Skipf("cannot bind group receive socket: %v", err) + } + defer group.Close() + + conn, err := openSendConn(net.IPv4(127, 0, 0, 1)) + if err != nil { + t.Fatalf("openSendConn alongside group socket: %v", err) + } + defer conn.Close() +} + func TestIfaceAddrsEqual(t *testing.T) { base := map[int][]net.IP{ 1: {net.IPv4(192, 168, 1, 10), net.IPv4(192, 168, 1, 11)}, diff --git a/services/shared/mdns/socketreuse_unix.go b/services/shared/mdns/socketreuse_unix.go index 88d9d56b..3e70d2db 100644 --- a/services/shared/mdns/socketreuse_unix.go +++ b/services/shared/mdns/socketreuse_unix.go @@ -7,6 +7,12 @@ package mdns import "syscall" +// sendSourcePort is the local UDP port outbound datagrams are sent from. +// RFC 6762 §6 requires responses to originate from the well-known mDNS port, +// and a router-level mDNS reflector only classifies and relays a datagram as +// mDNS when it arrives from 5353. See openSendConn. +const sendSourcePort = mdnsPort + // setReuseAddr is a net.ListenConfig.Control hook that sets SO_REUSEADDR on // the socket before bind. mDNS requires multiple processes on one host to // share UDP 5353; SO_REUSEADDR (the same option Go's ListenMulticastUDP and diff --git a/services/shared/mdns/socketreuse_windows.go b/services/shared/mdns/socketreuse_windows.go index 494fe540..c7e1c161 100644 --- a/services/shared/mdns/socketreuse_windows.go +++ b/services/shared/mdns/socketreuse_windows.go @@ -7,6 +7,13 @@ package mdns import "syscall" +// sendSourcePort is the local UDP port outbound datagrams are sent from. +// Windows keeps the original ephemeral-port behavior: it will not reliably +// send from a socket bound to the contested mDNS port, and a Windows host is +// normally discovered directly rather than through a reflector. See +// openSendConn. +const sendSourcePort = 0 + // setReuseAddr is the Windows counterpart of the Unix build. The only // difference is the socket handle type (syscall.Handle vs int). See the Unix // file for the rationale; SO_REUSEADDR on Windows gives the shared-bind diff --git a/services/versions.json b/services/versions.json index 29d8c230..64df3be7 100644 --- a/services/versions.json +++ b/services/versions.json @@ -6,7 +6,7 @@ "ollama-proxy": "0.26.2", "lmstudio-proxy": "0.16.2", "nvpair-node-info": "0.13.3", - "nvpair-node-scanner": "0.20.3", + "nvpair-node-scanner": "0.20.4", "nvpair-manual-nodes": "0.11.1", "nvpair-workload-manager": "0.13.3", "nvpair-errors": "0.7.4", From 576e55ef07d3ae915120f86ef25817c51aa014ba Mon Sep 17 00:00:00 2001 From: woodsonl <65194841+woodsonl@users.noreply.github.com> Date: Sat, 12 Sep 2026 16:50:48 -0500 Subject: [PATCH 2/3] fix: send mDNS from the receive socket on Unix The previous change bound a fresh per-send socket to the interface address on 5353. That socket is more specific than Run's receive socket, so while it was open the kernel delivered unicast datagrams addressed to the host on 5353 to it, and they were discarded when it closed: a unicast mDNS query arriving during a send was dropped. On Unix, send from Run's receive socket instead (issue #1's first suggestion). It is already bound to 5353, so responses still originate from the well-known port, and no second socket competes for unicast delivery. Set the outgoing interface and TTL per datagram through an ipv4.ControlMessage. Windows keeps the fresh ephemeral-socket path, since it refuses to send from the group-bound receive socket. Add a test that a send goes out the installed shared socket. Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com> --- services/shared/mdns/responder.go | 78 +++++++++++++++++---- services/shared/mdns/responder_test.go | 37 ++++++++++ services/shared/mdns/socketreuse_unix.go | 13 ++-- services/shared/mdns/socketreuse_windows.go | 5 ++ 4 files changed, 117 insertions(+), 16 deletions(-) diff --git a/services/shared/mdns/responder.go b/services/shared/mdns/responder.go index 57100157..3506f1ca 100644 --- a/services/shared/mdns/responder.go +++ b/services/shared/mdns/responder.go @@ -16,9 +16,11 @@ // invisible to LAN peers. // // We keep zeroconf's receive trick (join the group on each multicast interface, -// which works fine on Windows) but send every reply/announcement from a -// per-interface unicast-bound socket with SetMulticastInterface set explicitly. -// That path is well-supported on Windows. +// which works fine on Windows). On Unix, replies and announcements go out that +// same receive socket, so they originate from 5353 as RFC 6762 §6 requires. On +// Windows, which refuses to send from a group-bound socket, they instead go out +// a per-interface unicast-bound socket with SetMulticastInterface set +// explicitly; that path is well-supported there. // // This is the single implementation consolidated (the mDNS dedup) from the five // near-identical copies that lived in nvpair-advertiser, @@ -97,6 +99,14 @@ type Responder struct { addrMu sync.RWMutex // ifaceAddrs maps interface index to its IPv4 unicast addresses. ifaceAddrs map[int][]net.IP + + // sendMu guards sendPC. Run sets it on platforms that transmit from the + // receive socket (see sendFromRecvSocket); while non-nil, sends go out + // that socket so they share its source port and never briefly bind a + // second socket to 5353, which would capture unicast queries meant for + // the receive socket. Nil on Windows and before Run binds. + sendMu sync.Mutex + sendPC *ipv4.PacketConn } // NewResponder builds a Responder for the given service instance. domain @@ -157,6 +167,22 @@ func (r *Responder) ifaces() map[int][]net.IP { return r.ifaceAddrs } +// setSendPC installs (or clears) the socket sends use instead of a fresh +// per-send socket. Called by Run on platforms where sendFromRecvSocket is set. +func (r *Responder) setSendPC(pc *ipv4.PacketConn) { + r.sendMu.Lock() + r.sendPC = pc + r.sendMu.Unlock() +} + +// sharedSendPC returns the shared send socket, or nil when sends should open a +// fresh per-send socket (Windows, or before Run binds). +func (r *Responder) sharedSendPC() *ipv4.PacketConn { + r.sendMu.Lock() + defer r.sendMu.Unlock() + return r.sendPC +} + // UpdateTXT swaps the advertised TXT records and re-announces immediately. It // is safe to call before or during Run (announcements are sent on freshly // opened per-interface sockets, independent of Run's receive socket). Callers @@ -259,6 +285,14 @@ func (r *Responder) Run(ctx context.Context) error { slog.Debug("mdns: control message not available, will reply on all interfaces", "reason", err) } + // Transmit from this same socket on platforms where that is allowed, so + // responses leave from 5353 (RFC 6762 §6) without a second socket on 5353 + // that would capture unicast traffic meant for this one. + if sendFromRecvSocket { + r.setSendPC(pc) + defer r.setSendPC(nil) + } + var joined int for ifIdx := range r.ifaces() { ifi, err := net.InterfaceByIndex(ifIdx) @@ -570,11 +604,11 @@ func (r *Responder) sendUnicast(buf []byte, ifIndex int, to net.Addr) { } } -// openSendConn opens the per-interface socket sendOnInterface transmits from. -// On Unix it binds the local mDNS port (see sendSourcePort) so our datagrams -// originate from 5353 as RFC 6762 §6 requires; on Windows it binds ephemeral, -// where a second socket on 5353 is unreliable. setReuseAddr lets the socket -// share 5353 with Run's receive socket and any other local mDNS responder. +// openSendConn opens a fresh per-interface socket for send paths that cannot +// use the receive socket: Windows, and sends before Run binds. On Unix it +// binds the local mDNS port (see sendSourcePort) so datagrams still originate +// from 5353; on Windows it binds ephemeral, where a second socket on 5353 is +// unreliable. setReuseAddr lets it share 5353 with any local mDNS responder. func openSendConn(src net.IP) (*net.UDPConn, error) { lc := net.ListenConfig{Control: setReuseAddr} pktConn, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(src.String(), fmt.Sprint(sendSourcePort))) @@ -584,11 +618,31 @@ func openSendConn(src net.IP) (*net.UDPConn, error) { return pktConn.(*net.UDPConn), nil } -// sendOnInterface is the core of the Windows send workaround: it transmits buf -// from a fresh unicast-bound socket on the given interface (setting the -// multicast interface + TTL for group targets), never from the multicast-bound -// receive socket that Windows refuses to send from. +// sendOnInterface transmits buf to target on the given interface. It prefers +// the shared receive socket, so datagrams originate from 5353 (RFC 6762 §6) +// and no second socket briefly binds 5353 and captures unicast traffic meant +// for the receive socket. Where the platform forbids sending from the receive +// socket (Windows), it falls back to a fresh per-send socket. func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr) error { + if pc := r.sharedSendPC(); pc != nil { + cm := &ipv4.ControlMessage{IfIndex: ifIndex} + if target.IP.IsMulticast() { + cm.TTL = 255 + } + if _, err := pc.WriteTo(buf, cm, target); err != nil { + slog.Debug("mdns: write failed", "iface", ifIndex, "target", target.String(), "err", err) + return err + } + return nil + } + return r.sendOnFreshConn(buf, ifIndex, target) +} + +// sendOnFreshConn is the Windows path: it transmits buf from a fresh +// unicast-bound socket on the given interface (setting the multicast +// interface + TTL for group targets), never from the multicast-bound receive +// socket that Windows refuses to send from. +func (r *Responder) sendOnFreshConn(buf []byte, ifIndex int, target *net.UDPAddr) error { addrs, ok := r.ifaces()[ifIndex] if !ok || len(addrs) == 0 { return errors.New("no addresses on interface") diff --git a/services/shared/mdns/responder_test.go b/services/shared/mdns/responder_test.go index 2eadab33..07296726 100644 --- a/services/shared/mdns/responder_test.go +++ b/services/shared/mdns/responder_test.go @@ -8,8 +8,10 @@ import ( "fmt" "net" "testing" + "time" "github.com/miekg/dns" + "golang.org/x/net/ipv4" ) // testResponder builds a Responder with fixed fields so the record-building @@ -228,6 +230,41 @@ func TestOpenSendConnCoexistsWithGroupSocket(t *testing.T) { defer conn.Close() } +// TestSendOnInterfaceUsesSharedSocket verifies the Unix send path: once Run +// installs its receive socket, sends go out that socket, so the datagram +// source port is the shared socket's port (5353 in production) instead of a +// fresh per-send socket's. +func TestSendOnInterfaceUsesSharedSocket(t *testing.T) { + rcv, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + if err != nil { + t.Fatalf("listen receiver: %v", err) + } + defer rcv.Close() + + shared, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + if err != nil { + t.Fatalf("listen shared: %v", err) + } + defer shared.Close() + sharedPort := shared.LocalAddr().(*net.UDPAddr).Port + + r := &Responder{ifaceAddrs: map[int][]net.IP{1: {net.IPv4(127, 0, 0, 1)}}} + r.setSendPC(ipv4.NewPacketConn(shared)) + + if err := r.sendOnInterface([]byte("hi"), 1, rcv.LocalAddr().(*net.UDPAddr)); err != nil { + t.Fatalf("sendOnInterface: %v", err) + } + _ = rcv.SetReadDeadline(time.Now().Add(time.Second)) + buf := make([]byte, 16) + _, from, err := rcv.ReadFromUDP(buf) + if err != nil { + t.Fatalf("receive: %v", err) + } + if from.Port != sharedPort { + t.Fatalf("datagram source port = %d, want shared socket port %d", from.Port, sharedPort) + } +} + func TestIfaceAddrsEqual(t *testing.T) { base := map[int][]net.IP{ 1: {net.IPv4(192, 168, 1, 10), net.IPv4(192, 168, 1, 11)}, diff --git a/services/shared/mdns/socketreuse_unix.go b/services/shared/mdns/socketreuse_unix.go index 3e70d2db..33f179c0 100644 --- a/services/shared/mdns/socketreuse_unix.go +++ b/services/shared/mdns/socketreuse_unix.go @@ -7,12 +7,17 @@ package mdns import "syscall" -// sendSourcePort is the local UDP port outbound datagrams are sent from. -// RFC 6762 §6 requires responses to originate from the well-known mDNS port, -// and a router-level mDNS reflector only classifies and relays a datagram as -// mDNS when it arrives from 5353. See openSendConn. +// sendSourcePort is the local UDP port outbound datagrams are sent from when +// the responder falls back to a fresh per-send socket (before Run binds the +// receive socket). See openSendConn. const sendSourcePort = mdnsPort +// sendFromRecvSocket makes Run transmit from its own receive socket. That +// socket is already bound to 5353, so sends originate from the well-known +// port without opening a second socket on 5353 — which would capture unicast +// queries destined for the receive socket for as long as it lived. +const sendFromRecvSocket = true + // setReuseAddr is a net.ListenConfig.Control hook that sets SO_REUSEADDR on // the socket before bind. mDNS requires multiple processes on one host to // share UDP 5353; SO_REUSEADDR (the same option Go's ListenMulticastUDP and diff --git a/services/shared/mdns/socketreuse_windows.go b/services/shared/mdns/socketreuse_windows.go index c7e1c161..6d383e45 100644 --- a/services/shared/mdns/socketreuse_windows.go +++ b/services/shared/mdns/socketreuse_windows.go @@ -14,6 +14,11 @@ import "syscall" // openSendConn. const sendSourcePort = 0 +// sendFromRecvSocket is false on Windows: the platform refuses to send from +// the socket bound to the multicast group, so Run never shares its receive +// socket and sends always go through a fresh per-send socket. +const sendFromRecvSocket = false + // setReuseAddr is the Windows counterpart of the Unix build. The only // difference is the socket handle type (syscall.Handle vs int). See the Unix // file for the rationale; SO_REUSEADDR on Windows gives the shared-bind From 0d319eb6eb9755e1cdf17c1a47f9e8c8d4adfe2a Mon Sep 17 00:00:00 2001 From: woodsonl <65194841+woodsonl@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:01:02 -0500 Subject: [PATCH 3/3] fix: harden mDNS send path against review findings Three defects found reviewing the source-port fix: - Run closed its receive socket from a ctx.Done goroutine before calling sendGoodbye, so on Unix (where sends share that socket) the shutdown goodbye was dropped and peers cached the node for its full record TTL. Drop the closer; the read loop's 500ms deadline plus its ctx check end the loop promptly, and the deferred close now runs after sendGoodbye. - The fallback send socket bound :5353 on Unix. A unicast-bound 5353 socket is more specific than Run's receive socket and would capture its unicast traffic in the window before Run installs the shared socket (e.g. an UpdateTXT from the RPC loop). Bind ephemeral instead. - ControlMessage.TTL is receive-only in x/net/ipv4, so the multicast TTL was never applied. Set it once with SetMulticastTTL in Run and on the fresh-socket path, via a named mdnsTTL constant. Stale comments updated; tests cover ephemeral fallback and the no-address guard. Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com> --- services/shared/mdns/responder.go | 53 ++++++++++++--------- services/shared/mdns/responder_test.go | 31 +++++++----- services/shared/mdns/socketreuse_unix.go | 5 -- services/shared/mdns/socketreuse_windows.go | 7 --- 4 files changed, 48 insertions(+), 48 deletions(-) diff --git a/services/shared/mdns/responder.go b/services/shared/mdns/responder.go index 3506f1ca..72df2629 100644 --- a/services/shared/mdns/responder.go +++ b/services/shared/mdns/responder.go @@ -51,6 +51,8 @@ import ( const ( mdnsPort = 5353 + // mdnsTTL is the multicast TTL RFC 6762 §11 requires for mDNS datagrams. + mdnsTTL = 255 // recordTTL matches what zeroconf advertises for non-A records (3200s) // for service-level records, but RFC 6762 §10 says A records SHOULD use // a TTL of 120s to account for IP address changes. We use the shorter @@ -184,9 +186,9 @@ func (r *Responder) sharedSendPC() *ipv4.PacketConn { } // UpdateTXT swaps the advertised TXT records and re-announces immediately. It -// is safe to call before or during Run (announcements are sent on freshly -// opened per-interface sockets, independent of Run's receive socket). Callers -// with a static TXT never need it. +// is safe to call before or during Run. After Run installs the shared socket +// the announcement is sent from it; before that it falls back to a fresh +// per-interface socket. Callers with a static TXT never need it. func (r *Responder) UpdateTXT(txt []string) { r.txtMu.Lock() r.txt = append([]string(nil), txt...) @@ -287,8 +289,13 @@ func (r *Responder) Run(ctx context.Context) error { // Transmit from this same socket on platforms where that is allowed, so // responses leave from 5353 (RFC 6762 §6) without a second socket on 5353 - // that would capture unicast traffic meant for this one. + // that would capture unicast traffic meant for this one. RFC 6762 §11 + // requires multicast mDNS to carry TTL 255; a ControlMessage.TTL is + // receive-only in x/net/ipv4, so set it as a socket option. if sendFromRecvSocket { + if err := pc.SetMulticastTTL(mdnsTTL); err != nil { + slog.Debug("mdns: set multicast TTL failed", "err", err) + } r.setSendPC(pc) defer r.setSendPC(nil) } @@ -318,11 +325,10 @@ func (r *Responder) Run(ctx context.Context) error { go r.watchAddrs(ctx) - go func() { - <-ctx.Done() - _ = udpConn.Close() - }() - + // No ctx.Done closer here: the read loop's 500ms deadline plus the ctx.Err + // check at its top end the loop promptly, and the deferred udpConn.Close() + // must not run until after sendGoodbye() below has transmitted from this + // same socket. buf := make([]byte, 65536) for { if ctx.Err() != nil { @@ -604,14 +610,15 @@ func (r *Responder) sendUnicast(buf []byte, ifIndex int, to net.Addr) { } } -// openSendConn opens a fresh per-interface socket for send paths that cannot -// use the receive socket: Windows, and sends before Run binds. On Unix it -// binds the local mDNS port (see sendSourcePort) so datagrams still originate -// from 5353; on Windows it binds ephemeral, where a second socket on 5353 is -// unreliable. setReuseAddr lets it share 5353 with any local mDNS responder. +// openSendConn opens a fresh per-interface socket for the send paths that +// cannot use the receive socket: Windows, and sends before Run binds it. It +// binds an ephemeral port, never 5353, because a socket bound to a unicast +// address on 5353 is more specific than the receive socket and would capture +// unicast traffic meant for it. setReuseAddr lets it coexist with any local +// mDNS responder sharing the port. func openSendConn(src net.IP) (*net.UDPConn, error) { lc := net.ListenConfig{Control: setReuseAddr} - pktConn, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(src.String(), fmt.Sprint(sendSourcePort))) + pktConn, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(src.String(), "0")) if err != nil { return nil, err } @@ -625,10 +632,10 @@ func openSendConn(src net.IP) (*net.UDPConn, error) { // socket (Windows), it falls back to a fresh per-send socket. func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr) error { if pc := r.sharedSendPC(); pc != nil { + // IfIndex selects the egress interface per datagram. TTL is not + // settable here (ControlMessage.TTL is receive-only); Run sets the + // multicast TTL on this socket once instead. cm := &ipv4.ControlMessage{IfIndex: ifIndex} - if target.IP.IsMulticast() { - cm.TTL = 255 - } if _, err := pc.WriteTo(buf, cm, target); err != nil { slog.Debug("mdns: write failed", "iface", ifIndex, "target", target.String(), "err", err) return err @@ -638,10 +645,10 @@ func (r *Responder) sendOnInterface(buf []byte, ifIndex int, target *net.UDPAddr return r.sendOnFreshConn(buf, ifIndex, target) } -// sendOnFreshConn is the Windows path: it transmits buf from a fresh -// unicast-bound socket on the given interface (setting the multicast -// interface + TTL for group targets), never from the multicast-bound receive -// socket that Windows refuses to send from. +// sendOnFreshConn is the fallback used on Windows (which refuses to send from +// the multicast-bound receive socket) and for sends before Run installs the +// shared socket. It transmits buf from a fresh unicast-bound socket on the +// given interface, setting the multicast interface and TTL for group targets. func (r *Responder) sendOnFreshConn(buf []byte, ifIndex int, target *net.UDPAddr) error { addrs, ok := r.ifaces()[ifIndex] if !ok || len(addrs) == 0 { @@ -661,7 +668,7 @@ func (r *Responder) sendOnFreshConn(buf []byte, ifIndex int, target *net.UDPAddr if target.IP.IsMulticast() { pc := ipv4.NewPacketConn(conn) _ = pc.SetMulticastInterface(ifi) - _ = pc.SetMulticastTTL(255) + _ = pc.SetMulticastTTL(mdnsTTL) } if _, err := conn.WriteToUDP(buf, target); err != nil { slog.Debug("mdns: write failed", "iface", ifi.Name, "ip", src.String(), "target", target.String(), "err", err) diff --git a/services/shared/mdns/responder_test.go b/services/shared/mdns/responder_test.go index 07296726..b9126841 100644 --- a/services/shared/mdns/responder_test.go +++ b/services/shared/mdns/responder_test.go @@ -189,12 +189,10 @@ func TestAppendBrowseRRs(t *testing.T) { } } -// TestOpenSendConnBindsMDNSPort pins RFC 6762 §6: on platforms that advertise -// from a well-known port, the send socket's local port is 5353, not ephemeral. -func TestOpenSendConnBindsMDNSPort(t *testing.T) { - if sendSourcePort == 0 { - t.Skip("ephemeral send port on this platform") - } +// TestOpenSendConnUsesEphemeralPort guards the unicast-steal fix: the fallback +// send socket must not bind 5353, because a unicast-bound 5353 socket is more +// specific than Run's receive socket and would capture its unicast traffic. +func TestOpenSendConnUsesEphemeralPort(t *testing.T) { conn, err := openSendConn(net.IPv4(127, 0, 0, 1)) if err != nil { t.Fatalf("openSendConn: %v", err) @@ -204,18 +202,15 @@ func TestOpenSendConnBindsMDNSPort(t *testing.T) { if !ok { t.Fatalf("LocalAddr = %T, want *net.UDPAddr", conn.LocalAddr()) } - if addr.Port != mdnsPort { - t.Fatalf("send socket local port = %d, want %d", addr.Port, mdnsPort) + if addr.Port == mdnsPort { + t.Fatalf("fallback send socket bound %d; it would capture unicast", mdnsPort) } } // TestOpenSendConnCoexistsWithGroupSocket guards the bind path Run relies on: -// the 5353 send socket must open while the multicast-group receive socket is -// already bound to 5353 in the same process. +// the fallback send socket must open while the multicast-group receive socket +// is already bound to 5353 in the same process. func TestOpenSendConnCoexistsWithGroupSocket(t *testing.T) { - if sendSourcePort == 0 { - t.Skip("ephemeral send port on this platform") - } lc := net.ListenConfig{Control: setReuseAddr} group, err := lc.ListenPacket(context.Background(), "udp4", net.JoinHostPort(mdnsGroupV4.String(), fmt.Sprint(mdnsPort))) if err != nil { @@ -230,6 +225,16 @@ func TestOpenSendConnCoexistsWithGroupSocket(t *testing.T) { defer conn.Close() } +// TestSendOnInterfaceFreshConnGuard covers the fallback path when no shared +// socket is installed: an interface with no addresses must error rather than +// panic. +func TestSendOnInterfaceFreshConnGuard(t *testing.T) { + r := &Responder{ifaceAddrs: map[int][]net.IP{}} + if err := r.sendOnInterface([]byte("x"), 42, mdnsTargetV4); err == nil { + t.Fatal("sendOnInterface with no addresses on the interface = nil error, want error") + } +} + // TestSendOnInterfaceUsesSharedSocket verifies the Unix send path: once Run // installs its receive socket, sends go out that socket, so the datagram // source port is the shared socket's port (5353 in production) instead of a diff --git a/services/shared/mdns/socketreuse_unix.go b/services/shared/mdns/socketreuse_unix.go index 33f179c0..64378f35 100644 --- a/services/shared/mdns/socketreuse_unix.go +++ b/services/shared/mdns/socketreuse_unix.go @@ -7,11 +7,6 @@ package mdns import "syscall" -// sendSourcePort is the local UDP port outbound datagrams are sent from when -// the responder falls back to a fresh per-send socket (before Run binds the -// receive socket). See openSendConn. -const sendSourcePort = mdnsPort - // sendFromRecvSocket makes Run transmit from its own receive socket. That // socket is already bound to 5353, so sends originate from the well-known // port without opening a second socket on 5353 — which would capture unicast diff --git a/services/shared/mdns/socketreuse_windows.go b/services/shared/mdns/socketreuse_windows.go index 6d383e45..f4b9ee35 100644 --- a/services/shared/mdns/socketreuse_windows.go +++ b/services/shared/mdns/socketreuse_windows.go @@ -7,13 +7,6 @@ package mdns import "syscall" -// sendSourcePort is the local UDP port outbound datagrams are sent from. -// Windows keeps the original ephemeral-port behavior: it will not reliably -// send from a socket bound to the contested mDNS port, and a Windows host is -// normally discovered directly rather than through a reflector. See -// openSendConn. -const sendSourcePort = 0 - // sendFromRecvSocket is false on Windows: the platform refuses to send from // the socket bound to the multicast group, so Run never shares its receive // socket and sends always go through a fresh per-send socket.