local DNS: search-domain NXDOMAIN wins race, question not restored
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 38.2k
- Forks
- 4.6k
- Avg merge
- 19d 15h
- Merged PRs (30d)
- 1
Description
Operating system
macOS
System version
macOS 26.5.2 (25F84)
Installation type
sing-box for macOS Graphical Client
If you are using a graphical client, please provide the version of the client.
1.14.0-beta.7
Version
Reported by the running client: sing-box 1.14.0-beta.7
Source inspected: v1.14.0-beta.14 (both defects below are still present at that tag)
Description
The local DNS transport races search-domain candidates against the original FQDN and returns the winner verbatim. This has two consequences:
1. NXDOMAIN for a search-domain candidate can become the answer for the original name.
local_shared.go builds one exchanger per entry of NameList(domain) and, for A/AAAA, races them:
// dns/transport/local/local_shared.go
names := systemConfig.NameList(domain) // [fqdn., fqdn.+search., ...]
...
if systemConfig.SingleRequest || !(question.Qtype == mDNS.TypeA || question.Qtype == mDNS.TypeAAAA) {
transport.ExchangeSequential(ctx, nameExchangers, nil, callback)
} else {
transport.ExchangeRace(ctx, nameExchangers, callback)
}
raceState.complete accepts the first result with err == nil, and NXDOMAIN is a valid response, so it qualifies:
// dns/transport/exchange_strategy.go
s.done = true
s.access.Unlock()
s.cancel()
s.callback(response, nil)
A search-domain candidate under a non-existent suffix is denied authoritatively by the root in one round trip, while the real name may need full recursion. The bogus candidate therefore wins routinely. This differs from how stub resolvers normally treat a search list — glibc and Go's own net resolver walk NameList in order and only advance when a name yields no answer; the FQDN is never made to compete with its own fallbacks.
2. The response is delivered with the rewritten question, violating RFC 1035 §4.1.1.
newNameExchanger rewrites the question via NewFanOutRequest(message, fqdn, ...):
// dns/transport/exchange_strategy.go
func NewFanOutRequest(message *mDNS.Msg, fqdn string, authenticatedData bool) *mDNS.Msg {
question := message.Question[0]
question.Name = fqdn
...
}
Nothing restores it. protocol/openconnect, protocol/openvpn and protocol/tailscale each have a restoreOriginalQuestion-style helper for exactly this, with the reason spelled out in a comment:
// RFC 1035 §4.1.1 requires the response Question to match the request byte-for-byte,
// and stub resolvers discard Answer RRs whose owner name does not match the question.
The local transport has no equivalent — git grep 'Question = \[\]mDNS.Question' -- dns/ at v1.14.0-beta.14 only matches dns/transport/mdns/mdns.go. Clients consequently reject the reply outright; dig reports:
;; Question section mismatch: got example.com.invalid/A/IN
;; connection timed out; no servers could be reached
Impact. The bogus NXDOMAIN is then cached under the original name. Its negative TTL comes from the root SOA (86400 minimum, ~12 h remaining in practice), whereas the correct answer for the affected name in my case carried a 43 s TTL. So one lost race takes a hostname down for hours, and experimental.cache_file.store_dns persists it across restarts. Resolution recovers immediately after POST /cache/dns/flush.
Note that a search domain does not have to be configured explicitly: Config.Search falls back to defaultSearch(), which derives the suffix from the domain part of os.Hostname() (dns/transport/local/systemconfig/config.go, used at source_darwin.go). A DHCP-assigned hostname such as host.lan is therefore enough to enable this path.
I also note 0f79e1222 ("Partition local DNS caches by interface signature") includes c.Search in Config.Signature(), which stops an entry poisoned on one network from being served on another. That narrows the blast radius but does not address either defect above.
Aside: the race is gated on systemConfig.SingleRequest, but options single-request in resolv.conf means "issue the A and AAAA queries sequentially rather than in parallel", not "try the search list sequentially". These look like two different concerns sharing one flag.
Reproduction
Both defects are in dns/transport, so they reproduce with a self-contained unit test — no remote server, no TUN, no graphical client. Save as dns/transport/searchdomain_repro_test.go and run go test ./dns/transport/ -run TestSearchDomainRaceReturnsMismatchedQuestion -v.
package transport
import (
"context"
"testing"
"time"
mDNS "github.com/miekg/dns"
)
// newFakeNameExchanger mirrors dns/transport/local.newNameExchanger: it rewrites
// the question with NewFanOutRequest(message, fqdn, ...) and hands the upstream
// response back to the caller unchanged.
func newFakeNameExchanger(message *mDNS.Msg, fqdn string, delay time.Duration, build func(request *mDNS.Msg) *mDNS.Msg) AsyncExchanger {
return func(ctx context.Context, callback func(response *mDNS.Msg, err error)) {
request := NewFanOutRequest(message, fqdn, false)
go func() {
if delay > 0 {
select {
case <-time.After(delay):
case <-ctx.Done():
callback(nil, ctx.Err())
return
}
}
callback(build(request), nil)
}()
}
}
func answerNameError(request *mDNS.Msg) *mDNS.Msg {
response := new(mDNS.Msg)
response.SetRcode(request, mDNS.RcodeNameError)
return response
}
func answerAddress(request *mDNS.Msg) *mDNS.Msg {
response := new(mDNS.Msg)
response.SetReply(request)
response.Answer = []mDNS.RR{&mDNS.A{
Hdr: mDNS.RR_Header{
Name: request.Question[0].Name,
Rrtype: mDNS.TypeA,
Class: mDNS.ClassINET,
Ttl: 43,
},
A: []byte{192, 0, 2, 1},
}}
return response
}
func TestSearchDomainRaceReturnsMismatchedQuestion(t *testing.T) {
t.Parallel()
const realName = "example.com."
const searchName = "example.com.invalid."
original := new(mDNS.Msg)
original.SetQuestion(realName, mDNS.TypeA)
exchangers := []AsyncExchanger{
// Real name: needs recursion, so it is slow.
newFakeNameExchanger(original, realName, 50*time.Millisecond, answerAddress),
// Search-domain candidate: denied authoritatively and instantly.
newFakeNameExchanger(original, searchName, 0, answerNameError),
}
done := make(chan struct{})
var response *mDNS.Msg
var err error
ExchangeRace(context.Background(), exchangers, func(raceResponse *mDNS.Msg, raceErr error) {
response, err = raceResponse, raceErr
close(done)
})
<-done
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(response.Question) != 1 {
t.Fatalf("expected 1 question, got %d", len(response.Question))
}
t.Logf("rcode = %s, question = %s", mDNS.RcodeToString[response.Rcode], response.Question[0].Name)
if response.Question[0].Name != realName {
t.Fatalf("response question is %q, want %q (RFC 1035 4.1.1: the question section must match the request; stub resolvers discard the response otherwise)",
response.Question[0].Name, realName)
}
if response.Rcode == mDNS.RcodeNameError {
t.Fatalf("NXDOMAIN for the search-domain candidate was returned as the answer for %q", realName)
}
}
The NameList step that produces the second candidate is unconditional for a name with at least Ndots dots (Ndots defaults to 1):
// dns/transport/local/systemconfig/config.go
hasNdots := strings.Count(name, ".") >= c.Ndots
name += "."
names := make([]string, 0, 1+len(c.Search))
if hasNdots && !avoidDNS(name) {
names = append(names, name)
}
for _, suffix := range c.Search {
fqdn := name + suffix
if !avoidDNS(fqdn) && len(fqdn) <= 254 {
names = append(names, fqdn)
}
}
Logs
$ go test ./dns/transport/ -run TestSearchDomainRaceReturnsMismatchedQuestion -v
=== RUN TestSearchDomainRaceReturnsMismatchedQuestion
=== PAUSE TestSearchDomainRaceReturnsMismatchedQuestion
=== CONT TestSearchDomainRaceReturnsMismatchedQuestion
searchdomain_repro_test.go:97: rcode = NXDOMAIN, question = example.com.invalid.
searchdomain_repro_test.go:99: response question is "example.com.invalid.", want "example.com."
(RFC 1035 4.1.1: the question section must match the request; stub resolvers
discard the response otherwise)
--- FAIL: TestSearchDomainRaceReturnsMismatchedQuestion (0.00s)
FAIL
FAIL github.com/sagernet/sing-box/dns/transport 0.889s
# Observed on the client, against a DNS server of "type": "local".
# Name and suffix replaced with placeholders.
$ dig @<sing-box tun dns> example.com A
;; Question section mismatch: got example.com.invalid/A/IN
;; connection timed out; no servers could be reached
# 8 consecutive attempts, all identical, including with a trailing dot,
# so the search suffix is appended by sing-box and not by dig.
# The suffixed name is denied by the root with a 24h SOA minimum:
example.com.invalid. 44317 IN SOA a.root-servers.net. nstld.verisign-grs.com. ... 86400
# A name that is warm in the upstream cache wins the race and behaves correctly:
$ dig @<sing-box tun dns> other.example.com A
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 4993
;; Query time: 0 msec
# After flushing, the affected name resolves correctly again:
$ curl -X POST http://127.0.0.1:<clash api port>/cache/dns/flush
204
$ dig @<sing-box tun dns> example.com A
;; ->>HEADER<<- opcode: QUERY, status: NOERROR
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with dns/transport/local/local_shared.go and dns/transport/exchange_strategy.go, then inspect the related helpers in protocol/openconnect, protocol/openvpn, and protocol/tailscale. Run the proposed TestSearchDomainRaceReturnsMismatchedQuestion in dns/transport and review dns/transport/local/systemconfig/config.go for NameList behavior. Done means search candidates no longer incorrectly win with NXDOMAIN and responses preserve the original question.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100