From ef1877210ed3fe106edf80b11d9b580d95f0b026 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Tue, 15 Sep 2026 04:50:44 +1000 Subject: [PATCH] Merge commit from fork * caddyhttp: canonicalise paths after prefix stripping * caddyhttp: cover suffix stripping in path canonicalisation --- .../integration/rewrite_security_test.go | 79 +++++++++++++++++++ modules/caddyhttp/rewrite/rewrite.go | 44 ++++++++++- modules/caddyhttp/rewrite/rewrite_test.go | 41 +++++++++- 3 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 caddytest/integration/rewrite_security_test.go diff --git a/caddytest/integration/rewrite_security_test.go b/caddytest/integration/rewrite_security_test.go new file mode 100644 index 000000000..94d6635f8 --- /dev/null +++ b/caddytest/integration/rewrite_security_test.go @@ -0,0 +1,79 @@ +package integration + +import ( + "fmt" + "net/http" + "os" + "path/filepath" + "testing" + + "github.com/caddyserver/caddy/v2/caddytest" +) + +func TestStripRewritePreservesPathAuthorization(t *testing.T) { + root := t.TempDir() + for name, content := range map[string]string{ + "index.html": "ROOT INDEX SECRET", + "bar": "INNER SECRET", + "secret": "DOT SEGMENT SECRET", + } { + if err := os.WriteFile(filepath.Join(root, name), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + + tester := caddytest.NewTester(t) + tester.InitServer(fmt.Sprintf(` + { + skip_install_trust + admin localhost:2999 + http_port 9080 + grace_period 1ns + } + http://localhost:9080 { + root * %q + + handle_path /foo* { + basic_auth /* { + alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q + } + file_server + } + + handle /api* { + uri strip_prefix /api + basic_auth /secret* { + alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q + } + file_server + } + + handle /suffix* { + uri strip_suffix /suffix + basic_auth /* { + alice $2a$14$DCYyqvUO/PWT.UifNF0rbuRkRfjrhHViOfVfJklmqrvWUoYcTiN2q + } + file_server + } + } + `, filepath.ToSlash(root)), "caddyfile") + + for _, requestPath := range []string{ + "/foo", + "/foo/", + "/foobar", + "/foo/index.html", + "/api/secret", + "/api../secret", + "/api..%2fsecret", + "/suffix", + } { + t.Run(requestPath, func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "http://localhost:9080"+requestPath, nil) + if err != nil { + t.Fatal(err) + } + tester.AssertResponseCode(req, http.StatusUnauthorized) + }) + } +} diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 23da4f9b2..868fdd165 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -286,18 +286,30 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { prefix = "/" + prefix } mergeSlashes := !strings.Contains(prefix, "//") + stripped := false changePath(r, func(escapedPath string) string { escapedPath = caddyhttp.CleanPath(escapedPath, mergeSlashes) - return trimPathPrefix(escapedPath, prefix) + trimmed := trimPathPrefix(escapedPath, prefix) + stripped = stripped || trimmed != escapedPath + return trimmed }) + if stripped { + canonicalizePath(r) + } } if rewr.StripPathSuffix != "" { suffix := repl.ReplaceAll(rewr.StripPathSuffix, "") mergeSlashes := !strings.Contains(suffix, "//") + stripped := false changePath(r, func(escapedPath string) string { escapedPath = caddyhttp.CleanPath(escapedPath, mergeSlashes) - return trimPathSuffix(escapedPath, suffix) + trimmed := trimPathSuffix(escapedPath, suffix) + stripped = stripped || trimmed != escapedPath + return trimmed }) + if stripped { + canonicalizePath(r) + } } // substring replacements in URI @@ -591,6 +603,34 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) { } } +// canonicalizePath anchors and cleans a stripped origin-form path while +// preserving an alternate RawPath encoding when it still represents the +// canonical Path. It is applied after prefix or suffix removal because stripping can +// expose a relative path or dot segments that downstream handlers interpret +// differently. +func canonicalizePath(req *http.Request) { + p := req.URL.Path + if !strings.HasPrefix(p, "/") { + p = "/" + p + } + cleaned := caddyhttp.CleanPath(defaultEscapedPath(p), false) + p, _ = url.PathUnescape(cleaned) // defaultEscapedPath always returns valid escapes + + rawPath := req.URL.RawPath + if rawPath != "" { + if !strings.HasPrefix(rawPath, "/") { + rawPath = "/" + rawPath + } + decoded, err := url.PathUnescape(rawPath) + if err != nil || decoded != p || rawPath == defaultEscapedPath(p) { + rawPath = "" + } + } + + req.URL.Path = p + req.URL.RawPath = rawPath +} + // defaultEscapedPath returns the canonical percent-encoding of p, matching // what net/url.URL.EscapedPath() produces when RawPath is empty. It mirrors // the comparison net/url.URL.setPath uses to decide whether RawPath is needed. diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index 0261acd35..67ab01652 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -250,7 +250,7 @@ func TestRewrite(t *testing.T) { { rule: Rewrite{StripPathPrefix: "/prefix"}, input: newRequest(t, "GET", "/prefix"), - expect: newRequest(t, "GET", ""), + expect: newRequest(t, "GET", "/"), }, { rule: Rewrite{StripPathPrefix: "/prefix"}, @@ -315,6 +315,11 @@ func TestRewrite(t *testing.T) { input: newRequest(t, "GET", "/foo/bar"), expect: newRequest(t, "GET", "/foo/bar"), }, + { + rule: Rewrite{StripPathSuffix: "/suffix"}, + input: newRequest(t, "GET", "/suffix"), + expect: newRequest(t, "GET", "/"), + }, { rule: Rewrite{StripPathSuffix: "suffix"}, input: newRequest(t, "GET", "/foo/bar/suffix"), @@ -673,6 +678,40 @@ func TestQueryOpsReplaceScopedToKey(t *testing.T) { } } +func TestStripPathPrefixCanonicalizesRemainder(t *testing.T) { + for _, tc := range []struct { + name string + target string + prefix string + path string + rawPath string + }{ + {name: "empty remainder", target: "/mount", prefix: "/mount", path: "/"}, + {name: "mid-segment remainder", target: "/mountain", prefix: "/mount", path: "/ain"}, + {name: "parent segment", target: "/mount../document", prefix: "/mount", path: "/document"}, + {name: "current segment", target: "/mount./document/", prefix: "/mount", path: "/document/"}, + {name: "encoded parent segment", target: "/mount%2e%2e/document", prefix: "/mount", path: "/document"}, + {name: "encoded leading separator", target: "/mount%2Fdocument", prefix: "/mount", path: "/document"}, + {name: "alternate separator encoding", target: "/mount/a%2Fb", prefix: "/mount", path: "/a/b", rawPath: "/a%2Fb"}, + {name: "double-encoded parent segment", target: "/mount/%252e%252e/document", prefix: "/mount", path: "/%2e%2e/document"}, + {name: "non-UTF-8 byte", target: "/mount/%ff", prefix: "/mount", path: "/\xff", rawPath: "/%ff"}, + {name: "repeated separators", target: "/mount//a//b/", prefix: "/mount//", path: "/a//b/"}, + {name: "ordinary remainder", target: "/mount/document", prefix: "/mount", path: "/document"}, + } { + t.Run(tc.name, func(t *testing.T) { + r := newRequest(t, http.MethodGet, tc.target+"?keep=yes") + Rewrite{StripPathPrefix: tc.prefix}.Rewrite(r, caddy.NewReplacer()) + + if r.URL.Path != tc.path || r.URL.RawPath != tc.rawPath { + t.Errorf("Path/RawPath = %q/%q; want %q/%q", r.URL.Path, r.URL.RawPath, tc.path, tc.rawPath) + } + if r.URL.RawQuery != "keep=yes" || r.RequestURI != r.URL.EscapedPath()+"?keep=yes" { + t.Errorf("query or request target changed inconsistently: %q", r.RequestURI) + } + }) + } +} + func newRequest(t *testing.T, method, uri string) *http.Request { req, err := http.NewRequest(method, uri, nil) if err != nil {