diff --git a/services/shared/mdns/responder.go b/services/shared/mdns/responder.go index 880b98f1..72df2629 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, @@ -49,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 @@ -97,6 +101,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,10 +169,26 @@ 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 -// 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...) @@ -259,6 +287,19 @@ 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. 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) + } + var joined int for ifIdx := range r.ifaces() { ifi, err := net.InterfaceByIndex(ifIdx) @@ -284,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 { @@ -570,11 +610,46 @@ func (r *Responder) sendUnicast(buf []byte, ifIndex int, to net.Addr) { } } -// 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. +// 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(), "0")) + if err != nil { + return nil, err + } + return pktConn.(*net.UDPConn), nil +} + +// 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 { + // 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 _, 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 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 { return errors.New("no addresses on interface") @@ -584,7 +659,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 @@ -593,7 +668,7 @@ func (r *Responder) sendOnInterface(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 eb7b8c76..b9126841 100644 --- a/services/shared/mdns/responder_test.go +++ b/services/shared/mdns/responder_test.go @@ -4,10 +4,14 @@ package mdns import ( + "context" + "fmt" "net" "testing" + "time" "github.com/miekg/dns" + "golang.org/x/net/ipv4" ) // testResponder builds a Responder with fixed fields so the record-building @@ -185,6 +189,87 @@ func TestAppendBrowseRRs(t *testing.T) { } } +// 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) + } + 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("fallback send socket bound %d; it would capture unicast", mdnsPort) + } +} + +// TestOpenSendConnCoexistsWithGroupSocket guards the bind path Run relies on: +// 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) { + 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() +} + +// 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 +// 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 88d9d56b..64378f35 100644 --- a/services/shared/mdns/socketreuse_unix.go +++ b/services/shared/mdns/socketreuse_unix.go @@ -7,6 +7,12 @@ package mdns import "syscall" +// 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 494fe540..f4b9ee35 100644 --- a/services/shared/mdns/socketreuse_windows.go +++ b/services/shared/mdns/socketreuse_windows.go @@ -7,6 +7,11 @@ package mdns import "syscall" +// 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 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",