From 534f668ac98c6e68f87ed46f2ba49924a96dc411 Mon Sep 17 00:00:00 2001 From: Cedric Wang Date: Wed, 8 Jul 2026 18:07:01 -0700 Subject: [PATCH] core/dnsserver: sanitize DoH/DoH3 request parse errors (#8254) 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 --- core/dnsserver/server_https.go | 3 +- core/dnsserver/server_https3.go | 4 ++- core/dnsserver/server_https3_test.go | 35 +++++++++++++++++++++++ core/dnsserver/server_https_test.go | 42 ++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/core/dnsserver/server_https.go b/core/dnsserver/server_https.go index 334d4cffb..8beab0e9e 100644 --- a/core/dnsserver/server_https.go +++ b/core/dnsserver/server_https.go @@ -189,7 +189,8 @@ func (s *ServerHTTPS) ServeHTTP(w http.ResponseWriter, r *http.Request) { msg, raw, err := doh.RequestToMsgWire(r) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + clog.Debugf("DoH request could not be parsed: %v", err) + http.Error(w, "invalid request", http.StatusBadRequest) s.countResponse(http.StatusBadRequest) return } diff --git a/core/dnsserver/server_https3.go b/core/dnsserver/server_https3.go index b43fc7cd8..d283a3cc7 100644 --- a/core/dnsserver/server_https3.go +++ b/core/dnsserver/server_https3.go @@ -13,6 +13,7 @@ import ( "github.com/coredns/coredns/plugin/metrics/vars" "github.com/coredns/coredns/plugin/pkg/dnsutil" "github.com/coredns/coredns/plugin/pkg/doh" + clog "github.com/coredns/coredns/plugin/pkg/log" cproxyproto "github.com/coredns/coredns/plugin/pkg/proxyproto" "github.com/coredns/coredns/plugin/pkg/response" "github.com/coredns/coredns/plugin/pkg/reuseport" @@ -178,7 +179,8 @@ func (s *ServerHTTPS3) ServeHTTP(w http.ResponseWriter, r *http.Request) { msg, raw, err := doh.RequestToMsgWire(r) if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + clog.Debugf("DoH3 request could not be parsed: %v", err) + http.Error(w, "invalid request", http.StatusBadRequest) s.countResponse(http.StatusBadRequest) return } diff --git a/core/dnsserver/server_https3_test.go b/core/dnsserver/server_https3_test.go index 8025c8ce1..182e2d995 100644 --- a/core/dnsserver/server_https3_test.go +++ b/core/dnsserver/server_https3_test.go @@ -347,3 +347,38 @@ func TestServeHTTP3AcceptsValidTSIG(t *testing.T) { t.Fatalf("expected NOERROR response from plugin, got %s", dns.RcodeToString[resp.Rcode]) } } + +func TestServeHTTP3DoesNotLeakBodyReadError(t *testing.T) { + c := Config{ + Zone: "example.com.", + Transport: "https", + TLSConfig: &tls.Config{}, + ListenHosts: []string{"127.0.0.1"}, + Port: "443", + } + s, err := NewServerHTTPS3("127.0.0.1:443", []*Config{&c}) + if err != nil { + t.Fatal("could not create HTTPS3 server:", err) + } + + r := httptest.NewRequest(http.MethodPost, "/dns-query", errReader{}) + r.RemoteAddr = "127.0.0.1:12345" + w := httptest.NewRecorder() + + s.ServeHTTP(w, r) + + res := w.Result() + defer res.Body.Close() + + if res.StatusCode != http.StatusBadRequest { + t.Fatalf("expected status %d, got %d", http.StatusBadRequest, res.StatusCode) + } + + body, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(string(body)); got != "invalid request" { + t.Fatalf("expected sanitized body %q, got %q", "invalid request", got) + } +} diff --git a/core/dnsserver/server_https_test.go b/core/dnsserver/server_https_test.go index 525348aa9..a27e4642b 100644 --- a/core/dnsserver/server_https_test.go +++ b/core/dnsserver/server_https_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto/tls" + "errors" "io" "net" "net/http" @@ -583,3 +584,44 @@ func TestDoHWriterTsigStatusReturnsStoredStatus(t *testing.T) { t.Fatal("expected TsigStatus to return stored tsigStatus") } } + +type errReader struct{} + +const leakyBodyReadError = "read tcp 10.0.0.1:5443->10.0.0.2:48418: i/o timeout" + +func (errReader) Read([]byte) (int, error) { return 0, errors.New(leakyBodyReadError) } + +func TestServeHTTPDoesNotLeakBodyReadError(t *testing.T) { + c := Config{ + Zone: "example.com.", + Transport: "https", + TLSConfig: &tls.Config{}, + ListenHosts: []string{"127.0.0.1"}, + Port: "443", + } + s, err := NewServerHTTPS("127.0.0.1:443", []*Config{&c}) + if err != nil { + t.Fatal("could not create HTTPS server:", err) + } + + r := httptest.NewRequest(http.MethodPost, "/dns-query", errReader{}) + r.RemoteAddr = "127.0.0.1:12345" + w := httptest.NewRecorder() + + s.ServeHTTP(w, r) + + res := w.Result() + defer res.Body.Close() + + if res.StatusCode != http.StatusBadRequest { + t.Fatalf("expected status %d, got %d", http.StatusBadRequest, res.StatusCode) + } + + body, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(string(body)); got != "invalid request" { + t.Fatalf("expected sanitized body %q, got %q", "invalid request", got) + } +}