Skip to content

Commit 83bcb8e

Browse files
mcuelenaereclaude
andcommitted
fix(mdns): respect IPv4/IPv6 listen flags and drop dead exports
Cursorbot review on jetkvm#1454 flagged two issues: 1. hashicorp/mdns.NewServer always binds both 224.0.0.251:5353 and [ff02::fb]:5353 with no toggle, so MDNSMode=ipv4_only/ipv6_only was silently ignored — clients on the disabled family could still browse and discover a service whose A/AAAA they cannot resolve. Replace the call with a small custom responder that only binds the requested transports. Record generation still uses hashicorp/mdns.MDNSService via the existing jetkvmZone, and the query/response loop mirrors hashicorp/mdns to keep on-the-wire behavior identical otherwise. 2. DefaultAddressIPv4 / DefaultAddressIPv6 were leftover unused exports from the pion era. Removed. Also corrected misleading comments that claimed Stop() emits DNS-SD goodbye packets — neither hashicorp/mdns nor this responder do; we rely on TTL expiry like before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 72464af commit 83bcb8e

3 files changed

Lines changed: 183 additions & 16 deletions

File tree

internal/mdns/mdns.go

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@
77
// clients (NWBrowser, dns-sd, avahi-browse) can discover the
88
// device.
99
//
10-
// Implementation: hashicorp/mdns. We previously used pion/mdns/v2,
11-
// which is hostname-only and does not support DNS-SD. Running both
12-
// in the same process would mean two responders binding to
13-
// 224.0.0.251:5353 and producing duplicate A/AAAA replies, so the
14-
// pion responder was replaced rather than supplemented.
10+
// Implementation: a small custom server in server.go that handles
11+
// the multicast transport, plus hashicorp/mdns.MDNSService for
12+
// DNS-SD record generation. We previously used pion/mdns/v2, which
13+
// is hostname-only. We don't use hashicorp/mdns.NewServer directly
14+
// because it has no toggle for binding to IPv4 vs IPv6 multicast,
15+
// which would silently ignore MDNSMode=ipv4_only / ipv6_only.
1516
package mdns
1617

1718
import (
@@ -28,11 +29,6 @@ import (
2829
"github.com/rs/zerolog"
2930
)
3031

31-
const (
32-
DefaultAddressIPv4 = "224.0.0.251:5353"
33-
DefaultAddressIPv6 = "[ff02::fb]:5353"
34-
)
35-
3632
type MDNSListenOptions struct {
3733
IPv4 bool
3834
IPv6 bool
@@ -62,7 +58,7 @@ type MDNSOptions struct {
6258
}
6359

6460
type MDNS struct {
65-
server *hashicorp_mdns.Server
61+
server *jetkvmServer
6662
lock sync.Mutex
6763
l *zerolog.Logger
6864

@@ -286,7 +282,7 @@ func (m *MDNS) start(allowRestart bool) error {
286282
service: hashSvc,
287283
}
288284

289-
server, err := hashicorp_mdns.NewServer(&hashicorp_mdns.Config{Zone: zone})
285+
server, err := newJetkvmServer(zone, m.listenOptions.IPv4, m.listenOptions.IPv6)
290286
if err != nil {
291287
scopeLogger.Warn().Err(err).Msg("failed to start mDNS server")
292288
return err
@@ -308,8 +304,9 @@ func (m *MDNS) Restart() error {
308304
return m.start(true)
309305
}
310306

311-
// Stop stops the mDNS server. It sends DNS-SD goodbye packets so
312-
// browsers remove the device promptly instead of waiting for TTL.
307+
// Stop stops the mDNS server by closing the multicast sockets.
308+
// Browsers age the device out via record TTL (we do not currently
309+
// emit explicit goodbye packets).
313310
func (m *MDNS) Stop() error {
314311
m.lock.Lock()
315312
defer m.lock.Unlock()

internal/mdns/server.go

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package mdns
2+
3+
import (
4+
"fmt"
5+
"net"
6+
"sync/atomic"
7+
8+
hashicorp_mdns "github.com/hashicorp/mdns"
9+
"github.com/miekg/dns"
10+
)
11+
12+
// jetkvmServer is a minimal mDNS responder. We use it instead of
13+
// hashicorp/mdns.NewServer because the upstream Server always binds
14+
// to BOTH 224.0.0.251:5353 and [ff02::fb]:5353 with no toggle, which
15+
// silently ignores the user's MDNSMode=ipv4_only / ipv6_only setting.
16+
// Record generation (PTR / SRV / TXT / A / AAAA) is still delegated
17+
// to a hashicorp/mdns.Zone (so we don't reinvent DNS-SD); we only
18+
// re-implement the transport + query loop.
19+
//
20+
// The query/response handling here mirrors hashicorp/mdns's
21+
// server.go closely so the on-the-wire behavior is the same. RFC
22+
// 6762 §18 is the relevant spec.
23+
type jetkvmServer struct {
24+
zone hashicorp_mdns.Zone
25+
26+
ipv4List *net.UDPConn
27+
ipv6List *net.UDPConn
28+
29+
shutdown int32
30+
shutdownCh chan struct{}
31+
}
32+
33+
var (
34+
multicastAddrIPv4 = &net.UDPAddr{IP: net.ParseIP("224.0.0.251"), Port: 5353}
35+
multicastAddrIPv6 = &net.UDPAddr{IP: net.ParseIP("ff02::fb"), Port: 5353}
36+
)
37+
38+
// newJetkvmServer starts an mDNS responder on the requested
39+
// transports. At least one of listenIPv4/listenIPv6 must be true.
40+
func newJetkvmServer(zone hashicorp_mdns.Zone, listenIPv4, listenIPv6 bool) (*jetkvmServer, error) {
41+
s := &jetkvmServer{zone: zone, shutdownCh: make(chan struct{})}
42+
43+
if listenIPv4 {
44+
c, err := net.ListenMulticastUDP("udp4", nil, multicastAddrIPv4)
45+
if err == nil {
46+
s.ipv4List = c
47+
go s.recv(c)
48+
}
49+
}
50+
if listenIPv6 {
51+
c, err := net.ListenMulticastUDP("udp6", nil, multicastAddrIPv6)
52+
if err == nil {
53+
s.ipv6List = c
54+
go s.recv(c)
55+
}
56+
}
57+
58+
if s.ipv4List == nil && s.ipv6List == nil {
59+
return nil, fmt.Errorf("no multicast listeners could be started")
60+
}
61+
return s, nil
62+
}
63+
64+
// Shutdown stops the server. Goroutines spawned by recv exit when
65+
// the underlying socket is closed.
66+
func (s *jetkvmServer) Shutdown() error {
67+
if !atomic.CompareAndSwapInt32(&s.shutdown, 0, 1) {
68+
return nil
69+
}
70+
close(s.shutdownCh)
71+
if s.ipv4List != nil {
72+
_ = s.ipv4List.Close()
73+
}
74+
if s.ipv6List != nil {
75+
_ = s.ipv6List.Close()
76+
}
77+
return nil
78+
}
79+
80+
func (s *jetkvmServer) recv(c *net.UDPConn) {
81+
buf := make([]byte, 65536)
82+
for {
83+
n, from, err := c.ReadFrom(buf)
84+
if err != nil {
85+
select {
86+
case <-s.shutdownCh:
87+
return
88+
default:
89+
continue
90+
}
91+
}
92+
s.handlePacket(buf[:n], from)
93+
}
94+
}
95+
96+
func (s *jetkvmServer) handlePacket(packet []byte, from net.Addr) {
97+
var msg dns.Msg
98+
if err := msg.Unpack(packet); err != nil {
99+
return
100+
}
101+
// RFC 6762: silently ignore non-zero opcode/rcode and truncated
102+
// queries (we don't implement the known-answer continuation).
103+
if msg.Opcode != dns.OpcodeQuery || msg.Rcode != 0 || msg.Truncated {
104+
return
105+
}
106+
107+
var unicastAnswer, multicastAnswer []dns.RR
108+
for _, q := range msg.Question {
109+
recs := s.zone.Records(q)
110+
if len(recs) == 0 {
111+
continue
112+
}
113+
// RFC 6762 §18.12: top bit of qclass = "unicast response preferred".
114+
if q.Qclass&(1<<15) != 0 {
115+
unicastAnswer = append(unicastAnswer, recs...)
116+
} else {
117+
multicastAnswer = append(multicastAnswer, recs...)
118+
}
119+
}
120+
121+
if len(multicastAnswer) > 0 {
122+
s.sendResponse(multicastAnswer, from, msg.Id, false)
123+
}
124+
if len(unicastAnswer) > 0 {
125+
s.sendResponse(unicastAnswer, from, msg.Id, true)
126+
}
127+
}
128+
129+
func (s *jetkvmServer) sendResponse(answers []dns.RR, from net.Addr, queryID uint16, unicast bool) {
130+
// 18.1: ID is 0 for multicast responses, query.Id for unicast.
131+
id := uint16(0)
132+
if unicast {
133+
id = queryID
134+
}
135+
resp := &dns.Msg{
136+
MsgHdr: dns.MsgHdr{
137+
Id: id,
138+
Response: true,
139+
Opcode: dns.OpcodeQuery,
140+
Authoritative: true,
141+
},
142+
Compress: true,
143+
Answer: answers,
144+
}
145+
buf, err := resp.Pack()
146+
if err != nil {
147+
return
148+
}
149+
150+
// Match hashicorp/mdns: send back via the same transport family
151+
// the query came in on. (Their TODO notes this isn't strictly
152+
// RFC 6762 §6 compliant — multicast responses should go to the
153+
// multicast group — but in practice clients re-query periodically
154+
// so the unicast-back behavior works.)
155+
addr, ok := from.(*net.UDPAddr)
156+
if !ok {
157+
return
158+
}
159+
if addr.IP.To4() != nil {
160+
if s.ipv4List == nil {
161+
return
162+
}
163+
_, _ = s.ipv4List.WriteToUDP(buf, addr)
164+
} else {
165+
if s.ipv6List == nil {
166+
return
167+
}
168+
_, _ = s.ipv6List.WriteToUDP(buf, addr)
169+
}
170+
}

main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ func Main() {
190190
logger.Log().Msg("JetKVM Shutting Down")
191191

192192
if mDNS != nil {
193-
// Send DNS-SD goodbye so Bonjour browsers drop the device
194-
// immediately instead of waiting for the record TTL.
193+
// Close the multicast sockets so we stop responding to
194+
// browses immediately. Browsers age the entry out by TTL.
195195
if err := mDNS.Stop(); err != nil {
196196
logger.Warn().Err(err).Msg("failed to stop mDNS server")
197197
}

0 commit comments

Comments
 (0)