listener.Dnstap holds clientsMu.RLock() while iterating connected sink
clients. In the flush-error branch it called removeClient(c) synchronously,
but removeClient takes clientsMu.Lock(). A sync.RWMutex is not reentrant, so
the goroutine blocks forever waiting to acquire the write lock it can never
get while holding the read lock. The queued Lock() then blocks every
subsequent Dnstap broadcast and close(), and the goroutine leaks.
A flush error is the normal failure mode for a slow or disconnected sink
client (writeMsg buffers into framestream and succeeds; flush does the real
socket write and fails), so a single misbehaving client wedged the whole
listen path. Because Dnstap runs inline in the request-serving goroutine via
TapMessageWithMetadata, this could cascade into stalled request handling.
The write-error branch one line up already offloaded with `go removeClient(c)`.
Do the same in the flush-error branch and drop the early return so the
broadcast still reaches the remaining clients.
Assisted-by: Claude Opus 4.8
Signed-off-by: Pavel Lazureykis <pavel@lazureykis.dev>
serveQUICStream acquires a worker from streamProcessPool and then calls
readDOQMessage with no read deadline. A client that opens a QUIC stream
but never (or only slowly) sends its DoQ query blocks that worker
indefinitely. With enough such streams this starves the worker pool and
stalls DoQ service for other clients, a remotely triggerable denial of
service that needs no authentication.
Set a per-stream read deadline before readDOQMessage, bounded by the
server's existing ReadTimeout (the same deadline used to read a query on
TCP, default 3s), so a stalled stream cannot hold a worker forever. A
deadline hit surfaces as a read error handled by the existing error path,
which closes the connection and frees the worker; the normal fast path is
unchanged.
Adds a regression test proving a stalled stream no longer starves the
single-worker pool while a well-behaved query is still served.
Signed-off-by: Omkhar Arasaratnam <omkhar@linkedin.com>
Co-authored-by: Omkhar Arasaratnam <omkhar@linkedin.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* core/dnsserver: add Config.UDPDecorateWriterFunc for external plugins
Allow external plugins to install a dns.DecorateWriter (an existing
miekg/dns Server field CoreDNS never sets) on plain-UDP servers, e.g.
for per-socket write disciplines or overload protection; UDP is the
only transport without a Max*-style concurrency setting.
The field is a factory called once per Server in ServePacket, so each
socket gets its own decorator under multisocket; the *Server argument
is the same value handlers see via ctx.Value(dnsserver.Key{}). Follows
the Config.HTTPRequestValidateFunc precedent, including server-block
propagation and the ServePacket-time lookup over s.zones. Nil (the
default) changes nothing.
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
* core/dnsserver: select UDP writer decorator deterministically in NewServer
Review feedback: s.zones is a map, so choosing the decorator in
ServePacket was non-deterministic when several server blocks share a
listener. Select it in NewServer from the group slice instead (stable
config order, last one set wins) and document the resolution order.
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
* core/dnsserver: group decorator selection with the other last-writer-wins fields
Pure move: place it next to ProxyProtoConnPolicy, the existing field
resolved the same way at the same point.
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
---------
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
LookupStaticHostV4/V6 dereferenced h.hmap and h.inline as call arguments,
which are evaluated before lookupStaticHostFamily takes the read lock. The
reload path (readHosts) swaps h.hmap under h.Lock() on every reload, so the
field read raced the swap for every A/AAAA lookup. This was introduced when
wildcard support (#8185) refactored these methods to pass the maps as
parameters; LookupStaticAddr still reads the fields inside the lock and was
unaffected.
Read h.hmap/h.inline inside the RLock by selecting the address family with a
bool instead of passing pre-dereferenced maps.
Also read h.mtime under the existing RLock in readHosts: it was read without a
lock while the reload writes it under h.Lock(), and readHosts runs from both
the OnStartup handler and the reload ticker goroutine.
Both races are confirmed by go test -race.
Signed-off-by: Pavel Lazureykis <pavel@lazureykis.dev>
The DoH (HTTP/2) and DoH3 (HTTP/3) handlers wrote the raw parse error
returned by doh.RequestToMsgWire directly into the HTTP 400 response
body via http.Error(w, err.Error(), ...).
When the request body read fails mid-stream (for example the server's
ReadTimeout firing on a slow client, or a connection reset),
io.ReadAll(http.MaxBytesReader(...)) returns a *net.OpError whose text
embeds the server's own listen socket, e.g.
read tcp 10.0.0.1:5443->10.0.0.2:48418: i/o timeout
That discloses the server's internal bind address and port to any
unauthenticated remote caller who can trigger a body-read error.
Log the underlying error at debug level and return a generic
"invalid request" body to the client instead, matching the fixed-string
responses already used for the 404 and 500 cases in the same handlers.
Add tests asserting the response body no longer contains the raw read
error or an internal address.
Signed-off-by: zongqi-wang <wangzongqi@msn.com>
* plugin/file: run additional processing for wildcard answers
The wildcard branch of Zone.Lookup returned a nil additional section, so a
wildcard-synthesized MX/SRV/SVCB/HTTPS answer with an in-bailiwick target
did not include the target's A/AAAA glue. The non-wildcard path already
does this, so call additionalProcessing in the wildcard branch as well.
Fixes#6629
Signed-off-by: Salih Muhammed <root@lr0.org>
* plugin/file: move wildcard additional test into wildcard_test.go
Requested in review: keep the wildcard tests in one file.
Signed-off-by: Salih Muhammed <root@lr0.org>
---------
Signed-off-by: Salih Muhammed <root@lr0.org>
Remove fixed TLS 1.2 cipher suite list and maximum TLS version so
crypto/tls can use its maintained defaults. Keep TLS 1.2 as the
minimum supported version.
Document the shared TLS default behavior for plugins that expose TLS
configuration and add coverage to ensure CoreDNS leaves Go-managed
TLS fields unset.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
The cache key is hashed on qname, qtype, DO and CD, but deliberately not on the AD bit, so a single entry serves both AD- and non-AD-requesting clients. The AD bit returned must therefore be derived per request from the cached answer's authentication status, not frozen from the query that populated the entry.
This pins both orderings (noad-then-ad and ad-then-noad) so the historical asymmetry reported in #6642 cannot regress.
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
