From 706f1f8b7b2aa46d29996fe24e5ad9bd5420975f Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 10 Nov 2026 22:37:35 +0800 Subject: [PATCH] outbound: a disarmed DNS door writes no reply produced after the disarm Disarm cancels the resolver, and context.AfterFunc closes each open stream connection from another goroutine. A query read before that close was still answered: a cancel before its exchange wrote a 29-byte SERVFAIL and recorded a DNSFailed event, and a cancel after the upstream answered wrote the real reply. serveConn now drops a query read after the cancel and drops any reply once the resolver is canceled before the write, and answer records no failure for an exchange the cancel ended. --- sandboxd/outbound/dns.go | 9 ++++- sandboxd/outbound/dns_test.go | 73 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/sandboxd/outbound/dns.go b/sandboxd/outbound/dns.go index a566735e6..dde6f374d 100644 --- a/sandboxd/outbound/dns.go +++ b/sandboxd/outbound/dns.go @@ -111,7 +111,7 @@ func (r *resolver) serveConn(conn net.Conn, datagram bool) { r.h.dnsMalformed.Add(1) return } - if err != nil { + if err != nil || r.ctx.Err() != nil { return } select { @@ -121,6 +121,11 @@ func (r *resolver) serveConn(conn net.Conn, datagram bool) { } buf := udpBufs.Get().(*[dnsMsgMax]byte) answer, ok := r.answer(query, buf[:], datagram) + if r.ctx.Err() != nil { + udpBufs.Put(buf) + <-r.inflight + return + } if ok { _ = conn.SetWriteDeadline(time.Now().Add(dnsIdle)) err = writeDNSMsg(conn, answer) @@ -156,6 +161,8 @@ func (r *resolver) answer(query, buf []byte, datagram bool) ([]byte, bool) { ev.Answers, drop, err = r.scan(resp, hdr.ID) } switch { + case err != nil && r.ctx.Err() != nil: + return nil, true case err != nil: ev.Decision = DNSFailed r.record(ev) diff --git a/sandboxd/outbound/dns_test.go b/sandboxd/outbound/dns_test.go index 751aa99e7..8088f914a 100644 --- a/sandboxd/outbound/dns_test.go +++ b/sandboxd/outbound/dns_test.go @@ -157,6 +157,75 @@ func TestDNSDoorDisarmEndsAnOpenConnection(t *testing.T) { } } +func TestADisarmedResolverDropsAQueryItReadsAfterTheDisarm(t *testing.T) { + upstream := newFakeNameserver(t) + h := newHost(t, &fakeEngine{}, poolConfig(testKey, dnsPolicy), func(o *Options) { + o.Nameservers = func() []string { return []string{upstream} } + }) + for range 20 { + ctx, cancel := context.WithCancel(t.Context()) + r := newResolver(ctx, h, func(DNSEvent) {}) + guest, door := net.Pipe() + cancel() + served := make(chan struct{}) + go func() { + r.serveConn(unclosable{door}, false) + close(served) + }() + _ = guest.SetDeadline(time.Now().Add(time.Second)) + _ = writeDNSMsg(guest, packQuery(t, dnsmessage.Header{ID: 1}, "public.test.")) + _ = guest.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) + if answer, err := readDNSMsg(guest, dnsMsgMax); err == nil { + t.Fatalf("a resolver canceled by a disarm answered %d bytes to a query it read afterwards", len(answer)) + } + _ = guest.Close() + <-served + _ = door.Close() + } +} + +func TestADisarmLandingWhileAQueryIsAnsweredWritesNoReplyAndRecordsNoFailure(t *testing.T) { + for name, during := range map[string]string{"before the exchange": "nameservers", "after the answer": "record"} { + t.Run(name, func(t *testing.T) { + upstream := newFakeNameserver(t) + ctx, cancel := context.WithCancel(t.Context()) + h := newHost(t, &fakeEngine{}, poolConfig(testKey, dnsPolicy), func(o *Options) { + o.Nameservers = func() []string { + if during == "nameservers" { + cancel() + } + return []string{upstream} + } + }) + var events []DNSEvent + r := newResolver(ctx, h, func(ev DNSEvent) { + events = append(events, ev) + if during == "record" { + cancel() + } + }) + guest, door := net.Pipe() + served := make(chan struct{}) + go func() { + r.serveConn(unclosable{door}, false) + close(served) + }() + _ = guest.SetDeadline(time.Now().Add(time.Second)) + _ = writeDNSMsg(guest, packQuery(t, dnsmessage.Header{ID: 1}, "public.test.")) + _ = guest.SetReadDeadline(time.Now().Add(200 * time.Millisecond)) + if answer, err := readDNSMsg(guest, dnsMsgMax); err == nil { + t.Errorf("a disarm landing %s let %d bytes out", name, len(answer)) + } + _ = guest.Close() + <-served + _ = door.Close() + if slices.ContainsFunc(events, func(ev DNSEvent) bool { return ev.Decision == DNSFailed }) { + t.Errorf("events %+v, want no DNSFailed for a query the disarm dropped", events) + } + }) + } +} + func TestDNSDoorsHandATruncatedAnswerToEachTransport(t *testing.T) { upstream := newFakeNameserver(t) h := newHost(t, &fakeEngine{}, poolConfig(testKey, dnsPolicy), func(o *Options) { @@ -668,3 +737,7 @@ func (c *fuzzConn) RemoteAddr() net.Addr { return nil } func (c *fuzzConn) SetDeadline(time.Time) error { return nil } func (c *fuzzConn) SetReadDeadline(time.Time) error { return nil } func (c *fuzzConn) SetWriteDeadline(time.Time) error { return nil } + +type unclosable struct{ net.Conn } + +func (unclosable) Close() error { return nil }