mirror of
https://github.com/coredns/coredns.git
synced 2026-07-10 17:50:12 -04:00
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 <wangzongqi@msn.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user