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) + } +}