* plugin/autopath: Fixes a nil pointer dereference panic in autopath during search path walk
When a plugin later in the chain returns a ClientWrite rcode without
writing a response (for example acl's drop action, which returns
(dns.RcodeSuccess, nil) without calling WriteMsg), autopath dereferences
a nil nw.Msg at nw.Msg.Rcode and panics. The final fallback
w.WriteMsg(firstReply) can also receive a nil firstReply for the same
reason.
Skip search path elements that produced no message, and only write the
first reply when it is non-nil. This mirrors the nil guards recently
added in plugin/minimal (#8506), plugin/dns64 (#8511) and plugin/cache
(#8512).
Signed-off-by: zjncs <18910855655@163.com>
* plugin/autopath: silence unused-parameter lint and assert no client write
Address review feedback on #8517: rename the unused 'w' parameter in
TestAutoPathNilMsgFromNext to '_w' so the revive unused-parameter check
passes, and assert that the recorder receives no message so the intended
drop/no-client-write behavior is explicit.
Signed-off-by: zjncs <18910855655@163.com>
---------
Signed-off-by: zjncs <18910855655@163.com>
* plugin/k8s_external: Fixes a nil pointer dereference panic when upstream lookup returns no response
When a CNAME-hosted service is resolved, k8s_external performs internal
upstream lookups for the target name. Upstream.Lookup can return a nil
message with a nil error when the internal self-query's plugin chain
returns a ClientWrite rcode without writing a response (for example
acl's drop action), and the a/aaaa/srv handlers then dereference
resp.Answer on a nil resp and panic.
Guard all four lookups with err == nil && resp != nil, matching the nil
checks already used in plugin/backend_lookup.go and the guard recently
added to plugin/dns64 (#8511).
Signed-off-by: zjncs <18910855655@163.com>
* plugin/k8s_external: silence unused-parameter lint in nil upstream test
The CI lint flagged the test handler's unused 'w' parameter. Rename it
to '_' so golangci-lint (revive unused-parameter) passes. No behavior
change.
Signed-off-by: zjncs <18910855655@163.com>
---------
Signed-off-by: zjncs <18910855655@163.com>
ResponseReverter.WriteMsg calls res1.Copy() without checking res1 for
nil, so any plugin further down the chain that writes a nil response
(for example a handler returning (dns.RcodeSuccess, nil) after
w.WriteMsg(nil)) panics here.
Return an error instead, mirroring the nil guard recently added to
plugin/cache's ResponseWriter.WriteMsg (#8512).
Signed-off-by: zjncs <18910855655@163.com>
This PR fixes a nil pointer dereference panic in dns64 during response,
when the internal A-record upstream re-lookup returns a nil response.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
bufio.Scanner stops at the first line longer than its 64KiB default
buffer and reports bufio.ErrTooLong from Err(). parse() never checked
Err(), so that line and every entry after it were dropped silently: the
hosts file simply looked shorter than it is, with nothing in the log.
Raise the scanner's limit to 1MiB (the scanner still grows its buffer
lazily, so nothing is preallocated up front) and log an error if the
scan does stop early, so the truncation is at least visible.
Signed-off-by: Paco Cartones <pacocartones@users.noreply.github.com>
Co-authored-by: Paco Cartones <pacocartones@users.noreply.github.com>
* Group AWS SDK v2 modules so related updates land together in one PR.
This PR Group AWS SDK v2 modules so related updates land together in one PR.
Also rename the k8s/etcd group keys to valid Dependabot identifiers.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* Update
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
---------
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Store the normalized key name in the request context only after successful TSIG verification. This lets downstream plugins distinguish unsigned requests from authenticated requests and authorize by key without relying on the stripped TSIG RR or exposing secret material.
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
Keep miekg/dns's default request policy unless a plugin explicitly registers an additional opcode. Aggregate the policy at the listener, then enforce it again after zone routing so mixed server blocks on one socket remain isolated.
Apply the same policy to UDP, TCP, and DNS-over-TLS while preserving TSIG verification and the one-question requirement.
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
* plugin/cache: stop setting AA on answers served from cache
toMsg hardcoded m1.Authoritative = true, so a reply rebuilt from a cache entry claimed authority the answer that populated it never had.
The hardcoding was a workaround for legacy stub resolvers that dropped non-authoritative answers, but it only ever ran on the cache hit path: the same query still returned AA=0 on every miss and after every TTL expiry, so those clients were never actually protected.
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
* plugin/cache: pin the AA=1 side of the cache round trip
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
* plugin/cache: assert AA=0 on verified stale refresh and prove the cache hit
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
* plugin/cache: count backend calls in TestCachePreservesAA
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
---------
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
* perf(loadbalance): fast-path zero-allocation roundRobin for homogeneous record sets
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* plugin/loadbalance: copy before shuffling in the fast path
The fast path shuffled the caller's slice in place, which is not safe.
roundRobin must not modify its input: a backend may hand back a slice it
owns rather than one built for the response. plugin/file does exactly that
- Lookup returns elem.Type(qtype), which is the zone tree's own []dns.RR -
so an in-place shuffle reorders the zone itself, visible to every other
query and racing with the ones running concurrently.
Copy the records into a fresh slice and shuffle that instead. This is still
a single allocation rather than the four slices the partitioning path builds,
so most of the gain is kept:
name old time/op new time/op delta
RoundRobin 353 ns 244 ns -31%
name old alloc/op new alloc/op delta
RoundRobin 118 B 54 B -54%
name old allocs/op new allocs/op delta
RoundRobin 5 4 -20%
Also reorder the type check so a response led by a CNAME is rejected on the
first record instead of scanning the whole answer section first.
TestRoundRobinDoesNotMutateInput pins the contract; it fails against the
in-place version.
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
---------
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
remapStringRewriter matches a record name against orig and its sub domains. The
sub domain check was strings.HasSuffix(src, "."+r.orig), which built the
dot-prefixed string on every call and threw it away. Match the label boundary by
index instead: src is a sub domain of orig when orig sits at the end of src with
a "." immediately before it, which is exactly what the HasSuffix call tested.
Go concatenates short strings into a 32-byte stack buffer, so "."+orig only
reached the heap once orig passed 31 bytes. Below that the temporary was free
and this saves a few ns per record. Above it, 48 B was allocated per record.
Kubernetes service names are past the threshold -
my-service.my-namespace.svc.cluster.local. is 42 bytes - and those are the names
an auto rule rewrites to when a Corefile maps an external name onto an
in-cluster one. orig is the name the question was rewritten to, so whether a
deployment sees the allocation is a property of its Corefile, not its queries.
Caching "."+orig on the rewriter instead does not work: responseRuleFor
constructs a new rewriter for every request an auto name rule rewrites, so the
concatenation would run once per request rather than once per rule, and escapes
to the heap from there. That buys per-record work with a per-request allocation
and regresses every response short enough not to amortize it.
Per record, one rewriteString call on an existing rewriter:
name master this PR
RemapStringRewriter/short/match 186.6n 16 B/1 120.4n 16 B/1 -35%
RemapStringRewriter/short/nomatch 61.5n 0 B/0 16.5n 0 B/0 -73%
RemapStringRewriter/long/match 381.9n 72 B/2 165.3n 24 B/1 -57%
RemapStringRewriter/long/nomatch 219.1n 48 B/1 18.7n 0 B/0 -91%
Per request - rewrite the question, build the response rules, apply them to the
answer:
name master this PR
AutoNameRuleResponse/exact 935n 96 B/4 945n 96 B/4 ~
AutoNameRuleResponse/subdomain 1.248µ 112 B/5 1.242µ 112 B/5 ~
AutoNameRuleResponse/nomatch 1.016µ 96 B/4 945n 96 B/4 -7%
AutoNameRuleResponse/subdomain-8 3.376µ 224 B/12 2.891µ 224 B/12 -14%
AutoNameRuleResponse/k8s/subdomain 1.611µ 176 B/6 1.380µ 128 B/5 -14%
AutoNameRuleResponse/k8s/subdomain-8 5.144µ 672 B/20 3.305µ 288 B/12 -36%
benchstat over 8 runs, i7-1065G7. With short names this is flat at the request
level: one rewriteString call is small next to the four allocations that
building the rules costs. The saving is per record and per byte of name, so it
shows up where responses carry several records and the rewritten-to name is
long.
This applies to exact, prefix, substring and regex name rules with answer auto.
suffix rules build a suffixStringRewriter instead and are not affected.
TestRemapStringRewriter pins the label boundary semantics the index arithmetic
now carries, notably that notexample.com. is not a sub domain of example.com.
It passes against the previous implementation too.
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
