Fix block-answer TTLs that were nanoseconds, and stop counting NXDOMAIN as an error
Both found by running the resolver locally against real upstreams. response_ttl is a duration (60s) and a DNS TTL is seconds, but eight places converted with a bare uint32(), which keeps the low 32 bits of the nanosecond count. A blocked name's negative answer carried an SOA TTL and minimum of 997195776 (about 31 years), so any client or downstream cache that honoured it would remember the block, and keep remembering it after the name was unblocked. blocklist.TTLSeconds converts properly (whole seconds, clamped to 2^31-1) and is used by the rule parser and the block, sinkhole and redirect answers. The tests read the TTL off the wire; with the old conversion they fail with 4165425152. An ordinary upstream NXDOMAIN was classified as an error. A name that does not exist is the normal result of a typo, a search-domain expansion or a leaked mDNS query, so on a real network the error metric counted mostly those and, with the access log's errors-only default, every one wrote a line. Only the upstream failing or refusing (SERVFAIL, REFUSED, FORMERR, NOTIMP) is an error now. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
04ebf68c0f
commit
b513501773
6 files changed
+146
-13
No files matched your search
+12
-10
@@ -467,11 +467,13 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response {
|
||||
res.Msg = reply
|
||||
res.Upstream = addr
|
||||
res.Action = accesslog.ActionAllow
|
||||
// A non-success RCODE is reported as an error through finish's metrics
|
||||
// call, so the counters stay in one place.
|
||||
if reply.Rcode != dns.RcodeServerFailure && reply.Rcode != dns.RcodeSuccess {
|
||||
res.Action = accesslog.ActionError
|
||||
} else if reply.Rcode == dns.RcodeServerFailure {
|
||||
// NXDOMAIN is an answer, not a failure: a name that does not exist is the
|
||||
// normal result of a typo, a search-domain expansion or a leaked mDNS
|
||||
// query, and on a real network there are a great many of them. Counting
|
||||
// them as errors buried the real ones in the metrics and, with the access
|
||||
// log's errors-only default, wrote a line for each. What is left is the
|
||||
// upstream failing or refusing: SERVFAIL, REFUSED, FORMERR, NOTIMP.
|
||||
if reply.Rcode != dns.RcodeSuccess && reply.Rcode != dns.RcodeNameError {
|
||||
res.Action = accesslog.ActionError
|
||||
}
|
||||
if e.cache != nil && !decision.SkipCache {
|
||||
@@ -701,7 +703,7 @@ func (e *Engine) blockedMsg(q *dns.Msg, entry *blocklist.Entry) *dns.Msg {
|
||||
if entry != nil {
|
||||
switch entry.Action {
|
||||
case blocklist.ActionNXDomain:
|
||||
return nxDomain(q, uint32(cfg.ResponseTTL))
|
||||
return nxDomain(q, blocklist.TTLSeconds(cfg.ResponseTTL.D()))
|
||||
case blocklist.ActionZero:
|
||||
c := cfg
|
||||
c.DefaultAction = "zero"
|
||||
@@ -710,9 +712,9 @@ func (e *Engine) blockedMsg(q *dns.Msg, entry *blocklist.Entry) *dns.Msg {
|
||||
}
|
||||
return sinkhole(q, c)
|
||||
case blocklist.ActionRedirect:
|
||||
return redirect(q, entry.Response, uint32(cfg.ResponseTTL))
|
||||
return redirect(q, entry.Response, blocklist.TTLSeconds(cfg.ResponseTTL.D()))
|
||||
case blocklist.ActionCNAME:
|
||||
return redirect(q, entry.Response, uint32(cfg.ResponseTTL))
|
||||
return redirect(q, entry.Response, blocklist.TTLSeconds(cfg.ResponseTTL.D()))
|
||||
}
|
||||
}
|
||||
return blockedResponse(q, cfg, entry)
|
||||
@@ -1020,7 +1022,7 @@ func syntheticSOA(q *dns.Msg, ttl uint32) dns.RR {
|
||||
|
||||
// blockedResponse builds the configured blocklist answer.
|
||||
func blockedResponse(q *dns.Msg, cfg config.Block, entry *blocklist.Entry) *dns.Msg {
|
||||
ttl := uint32(cfg.ResponseTTL.D())
|
||||
ttl := blocklist.TTLSeconds(cfg.ResponseTTL.D())
|
||||
if ttl == 0 {
|
||||
ttl = 60
|
||||
}
|
||||
@@ -1057,7 +1059,7 @@ func sinkhole(q *dns.Msg, cfg config.Block) *dns.Msg {
|
||||
m := new(dns.Msg)
|
||||
m.SetReply(q)
|
||||
m.Authoritative = true
|
||||
ttl := uint32(cfg.ResponseTTL.D())
|
||||
ttl := blocklist.TTLSeconds(cfg.ResponseTTL.D())
|
||||
if ttl == 0 {
|
||||
ttl = 60
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user