A path holding a character Go escapes reaches the app as the client sent it (closes #126)

ReverseProxy sends the path URL.EscapedPath gives, which decodes and
escapes again a path holding a character such as |, turning %2F into /.
The path as sent now goes in URL.Opaque, which Go sends as it is; the
rate limit and WAF exempt paths and the log line's path use the same
path. A path starting with // is left as before: Go would send it in
Opaque as a scheme and host.

Model: opus-5-5

README.md

@@ -96,11 +96,14 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set, and the default rule file of
 ## What it does so far
 
 - Passes each request to the app and the app's answer back unchanged: method,
-  path, query, headers, body and status. Bodies stream through in both
+  path, query, headers, body and status. The one exception is a path starting
-  directions and are never held whole in memory, but for the part of a request
+  with `//` that holds a character Go escapes, such as `|`: Go cannot send it as
-  body the Core Rule Set reads, at most `SWWAF_WAF_BODY_LIMIT` and one byte
+  it is, and the app receives it decoded and escaped again, `%2F` as `/`. Bodies
-  more, which is held until the app is sent it. A WebSocket, or any other
+  stream through in both directions and are never held whole in memory, but for
-  upgraded connection, passes through, and the timeouts do not cut it.
+  the part of a request body the Core Rule Set reads, at most
+  `SWWAF_WAF_BODY_LIMIT` and one byte more, which is held until the app is sent
+  it. A WebSocket, or any other upgraded connection, passes through, and the
+  timeouts do not cut it.
 - Works out the client's address. A TCP peer outside `SWWAF_TRUSTED_PROXIES` is
   the client, and the forwarded headers it sends are replaced, not passed on.
   For a peer inside it, `X-Forwarded-For` is read from the right, and the first

internal/proxy/coreruleset.go

@@ -23,7 +23,7 @@ import (
 // limit while the Core Rule Set reads it, which it notes nothing of.
 func (rq *request) checkCoreRuleSet() string {
 	cfg := rq.h.config
-	if cfg.WAFMode == config.WAFModeOff || pathExempt(rq.in.URL, cfg.WAFExemptPaths) {
+	if cfg.WAFMode == config.WAFModeOff || pathExempt(rq.in, cfg.WAFExemptPaths) {
 		return ""
 	}
 

internal/proxy/passthrough_test.go

@@ -74,6 +74,47 @@ func TestPassesRequestAndAnswerUnchanged(t *testing.T) {
 	wantRequestFields(t, line, addr, len(requestBody), len(answerBody))
 }
 
+func TestPassesThePathAsSent(t *testing.T) {
+	t.Parallel()
+
+	for _, path := range []string{
+		// An encoded slash and a character Go escapes: |, ^, and é sent
+		// as its UTF-8 bytes.
+		"/a%2Fb|c", "/feat%2Fx/file^1", "/x%2Fy/café",
+		// A path starting with //, which Go would send as a scheme and
+		// host if it were put in URL.Opaque.
+		"//a%2Fb",
+	} {
+		t.Run(path, func(t *testing.T) {
+			t.Parallel()
+
+			saw := make(chan string, 1)
+			app := startApp(t, func(_ http.ResponseWriter, r *http.Request) {
+				saw <- r.RequestURI
+			})
+			addr, out := startProxy(t, app.URL, nil)
+
+			conn := dial(t, addr)
+			send(t, conn, "GET "+path+"?q=1 HTTP/1.1\r\nHost: "+appHost+"\r\n\r\n")
+
+			got := readResponse(t, conn)
+			if got.status != http.StatusOK {
+				t.Fatalf("status %d, want %d", got.status, http.StatusOK)
+			}
+
+			target := <-saw
+			if target != path+"?q=1" {
+				t.Errorf("app saw %s, want %s", target, path+"?q=1")
+			}
+
+			line := out.requestLine(t)
+			if line.Path != path {
+				t.Errorf("log line has path %s, want %s", line.Path, path)
+			}
+		})
+	}
+}
+
 // wantAppSaw checks that the app received the test's request unchanged.
 func wantAppSaw(t *testing.T, saw appSaw, body []byte) {
 	t.Helper()

internal/proxy/ratelimits_test.go

@@ -185,6 +185,9 @@ func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) {
 		"/assets/x%2Fy",
 		"/assets/x%2fy",
 		`/assets/x\y`,
+		// The same with a character Go escapes, |, for which Go's
+		// URL.EscapedPath gives /assets/x/y%7Cz, with no encoded slash.
+		"/assets/x%2Fy|z",
 	} {
 		t.Run(sent, func(t *testing.T) {
 			t.Parallel()

internal/proxy/request.go

@@ -8,7 +8,6 @@ import (
 	"net/http/httptrace"
 	"net/http/httputil"
 	"net/netip"
-	"net/url"
 	"os"
 	"slices"
 	"strings"
@@ -22,6 +21,7 @@ import (
 	"sneak.berlin/go/smallwebwaf/internal/lookup"
 	"sneak.berlin/go/smallwebwaf/internal/ratelimit"
 	"sneak.berlin/go/smallwebwaf/internal/requestlog"
+	"sneak.berlin/go/smallwebwaf/internal/rules"
 )
 
 // flushAfterEachWrite has ReverseProxy pass on each part of the app's
@@ -137,7 +137,7 @@ func (h *handler) newRequest(w http.ResponseWriter, r *http.Request) *request {
 			Method:           r.Method,
 			Scheme:           scheme(r, peerTrusted),
 			Host:             r.Host,
-			Path:             r.URL.EscapedPath(),
+			Path:             rules.Path(r),
 			Query:            r.URL.RawQuery,
 			Protocol:         r.Proto,
 			Referer:          r.Referer(),
@@ -284,7 +284,7 @@ func (rq *request) checkClient(ctx context.Context) string {
 	}
 
 	rq.counted = !isInside(rq.client, cfg.RateLimitExemptNets) &&
-		!pathExempt(rq.in.URL, cfg.RateLimitExemptPaths)
+		!pathExempt(rq.in, cfg.RateLimitExemptPaths)
 	if rq.counted {
 		rq.limitPercent, rq.bytesPercent = rq.limitPercentages()
 		rq.line.LimitPercent, rq.line.LimitPercentSetting = rq.limitPercent.logged()
@@ -307,7 +307,7 @@ func (rq *request) checkClient(ctx context.Context) string {
 	return rq.checkCoreRuleSet()
 }
 
-// pathExempt reports whether a request for u is exempt under prefixes,
+// pathExempt reports whether r is exempt under prefixes,
 // SWWAF_RATE_LIMIT_EXEMPT_PATHS or SWWAF_WAF_EXEMPT_PATHS: whether its
 // path as sent, the path the app receives, not percent-decoded, starts
 // with one of prefixes, so that /%61ssets/x is not under /assets/ for an
@@ -316,10 +316,9 @@ func (rq *request) checkClient(ctx context.Context) string {
 // encoded slash (%2F or %2f), never is, since an app may act on it as a
 // path outside every prefix: /assets/..%2Flogin as /login, or /assets%2Fx
 // as one path segment, as Go's router does.
-func pathExempt(u *url.URL, prefixes []string) bool {
+func pathExempt(r *http.Request, prefixes []string) bool {
-	decoded := u.Path
+	decoded := r.URL.Path
-	// EscapedPath is the path as the app receives it, not decoded.
+	sent := rules.Path(r)
-	sent := u.EscapedPath()
 
 	if strings.Contains(decoded, "..") || strings.Contains(decoded, `\`) ||
 		strings.Contains(strings.ToLower(sent), "%2f") {
@@ -375,6 +374,17 @@ func (rq *request) rewrite(pr *httputil.ProxyRequest) {
 	// ReverseProxy drops query parameters it cannot parse; the app gets
 	// the query as the client sent it.
 	pr.Out.URL.RawQuery = pr.In.URL.RawQuery
+	// ReverseProxy sends the path URL.EscapedPath gives, which, for a path
+	// holding a character Go escapes, such as |, is the whole path decoded
+	// and escaped again, %2F turned into /. Opaque is sent as it is, so the
+	// app gets the path as the client sent it. Left out are a target that
+	// is no path, such as *, and a path starting with //, which Go would
+	// send in Opaque as a scheme and host.
+	path := rules.Path(pr.In)
+	if strings.HasPrefix(path, "/") && !strings.HasPrefix(path, "//") {
+		pr.Out.URL.Opaque = path
+	}
+
 	setForwardedHeaders(pr.In, pr.Out, rq.peer, rq.peerTrusted)
 	pr.Out.Header.Set(requestIDHeader, rq.line.RequestID)
 	pr.Out.Header.Del(asnHeader)