core/dnsserver: bound DoQ stream read with the server read timeout (#8231)

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/server_quic.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"time"
 
 	"github.com/coredns/coredns/plugin/metrics/vars"
 	clog "github.com/coredns/coredns/plugin/pkg/log"
@@ -186,6 +187,19 @@ func (s *ServerQUIC) serveQUICStream(stream *quic.Stream, conn *quic.Conn) {
 		s.closeQUICConn(conn, DoQCodeInternalError)
 		return
 	}
+
+	// A stream is served by a worker acquired from s.streamProcessPool. A
+	// client that opens a stream but never (or only slowly) sends its DoQ
+	// query would otherwise block readDOQMessage indefinitely, holding that
+	// worker and eventually starving the pool. Bound the wait with the
+	// server's read timeout (the same deadline used for reading a query on
+	// TCP), so a stalled stream cannot hold a worker forever. A deadline
+	// hit surfaces as a read error handled by the existing error path below,
+	// which closes the connection and frees the worker.
+	if s.ReadTimeout != 0 {
+		_ = stream.SetReadDeadline(time.Now().Add(s.ReadTimeout))
+	}
+
 	buf, err := readDOQMessage(stream)
 
 	// io.EOF does not really mean that there's any error, it is just

core/dnsserver/server_quic_test.go

@@ -553,6 +553,131 @@ func TestServerQUIC_ServeQUIC_TSIGBadSigSetsTsigStatus(t *testing.T) {
 	}
 }
 
+// echoPlugin answers every query with a minimal reply. It is used as a
+// negative control to prove a normal DoQ query is still served after the
+// per-stream read deadline was introduced.
+type echoPlugin struct{}
+
+func (echoPlugin) Name() string { return "echo" }
+
+func (echoPlugin) ServeDNS(_ context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	m := new(dns.Msg)
+	m.SetReply(r)
+	if err := w.WriteMsg(m); err != nil {
+		return dns.RcodeServerFailure, err
+	}
+	return dns.RcodeSuccess, nil
+}
+
+// TestServerQUIC_ServeQUIC_StalledStreamDoesNotStarveWorkerPool is a
+// regression test for the DoQ stream/worker-pool starvation DoS
+// (GHSA-f2c9-fp4w-rhw6): a client that opens a stream but never finishes
+// sending its query used to block a worker from streamProcessPool forever,
+// because readDOQMessage was called with no read deadline. With the worker
+// pool shrunk to a single slot, a stalled stream would previously prevent
+// any other query from ever being served. The per-stream read deadline must
+// free the worker so an unrelated, well-behaved client is still served.
+func TestServerQUIC_ServeQUIC_StalledStreamDoesNotStarveWorkerPool(t *testing.T) {
+	config := testConfig("quic", echoPlugin{})
+	config.TLSConfig = mustMakeQUICServerTLSConfig(t)
+	// A single worker means a stalled stream, absent a read deadline, would
+	// hold the only worker and starve every subsequent connection.
+	workerPoolSize := 1
+	config.MaxQUICWorkerPoolSize = &workerPoolSize
+
+	server, err := NewServerQUIC(transport.QUIC+"://127.0.0.1:0", []*Config{config})
+	if err != nil {
+		t.Fatalf("NewServerQUIC() failed: %v", err)
+	}
+	// Keep the test fast: the stalled stream must time out quickly.
+	server.ReadTimeout = 250 * time.Millisecond
+
+	pc, err := server.ListenPacket()
+	if err != nil {
+		t.Fatalf("ListenPacket() failed: %v", err)
+	}
+	defer pc.Close()
+
+	serveErrCh := make(chan error, 1)
+	go func() {
+		serveErrCh <- server.ServeQUIC()
+	}()
+
+	defer func() {
+		_ = server.Stop()
+		select {
+		case <-serveErrCh:
+		case <-time.After(2 * time.Second):
+		}
+	}()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	// Connection 1: open a stream and send a length prefix but never the
+	// message body, stalling the server's readDOQMessage. This grabs the
+	// only worker.
+	stallConn, err := quic.DialAddr(ctx, pc.LocalAddr().String(), mustMakeQUICClientTLSConfig(), &quic.Config{})
+	if err != nil {
+		t.Fatalf("quic.DialAddr() for stalled conn failed: %v", err)
+	}
+	defer stallConn.CloseWithError(DoQCodeNoError, "")
+
+	stallStream, err := stallConn.OpenStreamSync(ctx)
+	if err != nil {
+		t.Fatalf("OpenStreamSync() for stalled stream failed: %v", err)
+	}
+	// Announce a 100-byte message but send nothing more, so the server
+	// blocks reading the body until the read deadline fires.
+	if _, err := stallStream.Write([]byte{0x00, 0x64}); err != nil {
+		t.Fatalf("stalled stream.Write() failed: %v", err)
+	}
+
+	// Connection 2: a well-behaved client. Before the fix this query would
+	// never be answered because the single worker is stuck on the stalled
+	// stream; after the fix the stalled worker is freed once the read
+	// deadline fires and this query succeeds.
+	normalConn, err := quic.DialAddr(ctx, pc.LocalAddr().String(), mustMakeQUICClientTLSConfig(), &quic.Config{})
+	if err != nil {
+		t.Fatalf("quic.DialAddr() for normal conn failed: %v", err)
+	}
+	defer normalConn.CloseWithError(DoQCodeNoError, "")
+
+	normalStream, err := normalConn.OpenStreamSync(ctx)
+	if err != nil {
+		t.Fatalf("OpenStreamSync() for normal stream failed: %v", err)
+	}
+
+	q := new(dns.Msg)
+	q.SetQuestion("example.com.", dns.TypeA)
+	q.Id = 0
+	wire, err := q.Pack()
+	if err != nil {
+		t.Fatalf("dns.Msg.Pack() failed: %v", err)
+	}
+	if _, err := normalStream.Write(AddPrefix(wire)); err != nil {
+		t.Fatalf("normal stream.Write() failed: %v", err)
+	}
+	if err := normalStream.Close(); err != nil {
+		t.Fatalf("normal stream.Close() failed: %v", err)
+	}
+
+	respCh := make(chan error, 1)
+	go func() {
+		_, rerr := readDOQMessage(normalStream)
+		respCh <- rerr
+	}()
+
+	select {
+	case rerr := <-respCh:
+		if rerr != nil {
+			t.Fatalf("normal query was not served: readDOQMessage() error = %v", rerr)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("normal query was not served within 5s: stalled stream starved the worker pool")
+	}
+}
+
 func mustMakeQUICServerTLSConfig(t *testing.T) *tls.Config {
 	t.Helper()