From d787d4ff9544eadb7f6642f817a03cfca934847f Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Mon, 7 Sep 2026 23:09:56 +1000 Subject: [PATCH] Match backend addresses case-insensitively when redacting DNS names are case-insensitive, so a backend named back in a different case escaped redaction and reached the client. - Fold ASCII case when replacing a backend's URL, host and hostname - Pin the redaction vectors, the same-status rule and the bare-GET premise behind treating 406 and 415 as replayable --- upstream.go | 40 ++++++++++++++++++++++++++- upstream_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/upstream.go b/upstream.go index 8324976..eb2488e 100644 --- a/upstream.go +++ b/upstream.go @@ -45,6 +45,12 @@ func (e *upstreamError) Error() string { // backends are meant to disagree about; 408 and 429 report a backend's timing // and capacity. A backend that times out mid-query answers 200 with a truncated // body rather than any 4xx (query_eng.clj:463-483), so no timeout reaches here. +// +// Every 4xx left over blames the request and so is safe to replay, though none +// is reachable while the fan-out forwards no client header: 406 needs an Accept +// the query app refuses (http/server.clj:72; must-accept-type in +// http.clj:136-145 is unwired), and 415 a Content-Encoding on a POST to +// /commands (middleware.clj:165-178), which pdbmux never proxies. func clientShaped(status int) bool { switch status { case http.StatusForbidden, http.StatusNotFound, @@ -107,12 +113,44 @@ func (s *Server) redactBackends(body []byte) []byte { out := string(body) for _, b := range s.cfg.Backends { for _, term := range backendTerms(b) { - out = strings.ReplaceAll(out, term, redactedBackend) + out = replaceFold(out, term, redactedBackend) } } return []byte(out) } +// replaceFold is strings.ReplaceAll ignoring ASCII case, because DNS names are +// case-insensitive and a backend shouted back in capitals is still named. +func replaceFold(s, old, repl string) string { + if old == "" { + return s + } + hay, needle := foldASCII(s), foldASCII(old) + var out strings.Builder + for { + i := strings.Index(hay, needle) + if i < 0 { + out.WriteString(s) + return out.String() + } + out.WriteString(s[:i]) + out.WriteString(repl) + s, hay = s[i+len(needle):], hay[i+len(needle):] + } +} + +// foldASCII lowercases ASCII only, so byte offsets into the result also index +// the input — which Unicode-aware folding does not guarantee. +func foldASCII(s string) string { + b := []byte(s) + for i, c := range b { + if c >= 'A' && c <= 'Z' { + b[i] = c + 'a' - 'A' + } + } + return string(b) +} + // backendTerms is what identifies a backend in text: its URL, that URL's host // and the bare hostname, longest first so replacing one cannot leave a fragment // of another behind. The configured name is deliberately absent — no backend diff --git a/upstream_test.go b/upstream_test.go index 8e8f97f..58ecd74 100644 --- a/upstream_test.go +++ b/upstream_test.go @@ -4,8 +4,10 @@ import ( "errors" "fmt" "net/http" + "net/http/httptest" "net/url" "strings" + "sync" "testing" "time" ) @@ -27,6 +29,10 @@ func TestUnanimousClientError_AgreementRule(t *testing.T) { {"all agree on 415", []error{upErr(415, "bad media"), upErr(415, "bad media")}, 415}, {"differing bodies still agree", []error{upErr(400, "one"), upErr(400, "two")}, 400}, {"disagreeing 4xx", []error{upErr(400, "bad"), upErr(404, "gone")}, 0}, + // Both client-shaped, so only the same-status rule refuses these. + {"agree on shape, disagree on code", []error{upErr(400, "x"), upErr(415, "y")}, 0}, + {"a majority agrees", []error{upErr(400, "x"), upErr(400, "y"), upErr(422, "z")}, 0}, + {"the first differs", []error{upErr(422, "z"), upErr(400, "x"), upErr(400, "y")}, 0}, {"4xx with a 5xx", []error{upErr(400, "bad"), upErr(500, "boom")}, 0}, {"4xx with a transport failure", []error{upErr(400, "bad"), refused}, 0}, {"all 403", []error{upErr(403, "denied"), upErr(403, "denied")}, 0}, @@ -415,6 +421,70 @@ func TestWriteUpstreamError_EmptyBodyGetsAMessage(t *testing.T) { } } +// clientShaped calls 406 and 415 replayable on the grounds that neither is +// reachable while the fan-out sends a bare GET. Forwarding a client's +// negotiation headers would make them reachable, so pin the premise. +func TestFanOut_ForwardsNoNegotiationHeaders(t *testing.T) { + var mu sync.Mutex + var seen []http.Header + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + seen = append(seen, r.Header.Clone()) + mu.Unlock() + setContentType(w, "application/json") + _, _ = w.Write([]byte(`[]`)) + })) + defer up.Close() + + h := newTestServer(testConfig(up.URL, up.URL, mergeStatic)).Handler() + for _, path := range []string{nodesPath, resourcesPath} { + req := httptest.NewRequest(http.MethodGet, path, nil) + req.Header.Set("Accept", "application/xml") + req.Header.Set("Content-Encoding", "br") + h.ServeHTTP(httptest.NewRecorder(), req) + } + + mu.Lock() + defer mu.Unlock() + if len(seen) == 0 { + t.Fatal("no backend request observed") + } + for _, hdr := range seen { + for _, name := range []string{"Accept", "Content-Encoding", "Content-Type"} { + if v := hdr.Get(name); v != "" { + t.Errorf("fan-out forwarded %s: %q", name, v) + } + } + } +} + +// Redaction is fail-safe: it over-matches rather than let an address through, +// and DNS case must not be a way past it. +func TestRedactBackends_Vectors(t *testing.T) { + srv := newTestServer(testConfig("http://pdb1.ops.unkin.net:8080", "https://10.20.30.40:8081", mergeStatic)) + for _, tc := range []struct { + name, in, want string + }{ + {"url", "GET http://pdb1.ops.unkin.net:8080/pdb failed", "GET /pdb failed"}, + {"uppercase hostname", "PDB1.OPS.UNKIN.NET refused", " refused"}, + {"mixed-case url", "HTTP://PDB1.Ops.Unkin.Net:8080/pdb", "/pdb"}, + {"ip backend", "dial https://10.20.30.40:8081/pdb", "dial /pdb"}, + {"json-escaped slashes", `{"at":"http:\/\/pdb1.ops.unkin.net:8080\/pdb"}`, `{"at":"http:\/\/\/pdb"}`}, + {"percent-encoded colon", "pdb1.ops.unkin.net%3A8080", "%3A8080"}, + {"trailing-dot fqdn", "pdb1.ops.unkin.net. timed out", ". timed out"}, + {"hostname as substring", "peer-pdb1.ops.unkin.net-alt", "peer--alt"}, + {"repeated", "pdb1.ops.unkin.net and PDB1.ops.unkin.net", " and "}, + {"non-ascii body survives folding", "Ünïcode PDB1.OPS.UNKIN.NET ✓", "Ünïcode ✓"}, + {"names nothing", "Unrecognized column 'bogus'", "Unrecognized column 'bogus'"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := string(srv.redactBackends([]byte(tc.in))); got != tc.want { + t.Errorf("redactBackends(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + func hostOf(t *testing.T, raw string) string { t.Helper() u, err := url.Parse(raw) -- 2.47.3