From 83b1824d06e54000fc988aea0ae2facf5fb34dea Mon Sep 17 00:00:00 2001
From: Victor Song <vsong1618@gmail.com>
Date: Mon, 19 Aug 2024 02:01:41 -0500
Subject: [PATCH 1/2] ipn/ipnlocal/serve: copy over raw query params if invalid

By default, `httputil.ReverseProxy` strips out invalid query
params for safety purposes (e.g. if the proxy and downstream
hand query parameters differently there could be an issue).
To allow users of `tailscale serve` or `tailscale funnel`
to deal with invalid query parameters themselves, we copy
the raw query parameters over if `httputil/ReverseProxy`
modifies them.

Fixes #13058

Signed-off-by: Victor Song <vsong1618@gmail.com>
---
 ipn/ipnlocal/serve.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go
index 638b26a36..a220447c8 100644
--- a/ipn/ipnlocal/serve.go
+++ b/ipn/ipnlocal/serve.go
@@ -746,6 +746,14 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			r.Out.URL.RawPath = rp.url.RawPath
 		}
 
+		// ReverseProxy.ServeHTTP strips out unparseable query parameters
+		// as it could be a security concern. To preserve downstream use
+		// of query parameters, we copy them back if the input and output
+		// do not match
+		if r.In.URL.RawQuery != r.Out.URL.RawQuery {
+			r.Out.URL.RawQuery = r.In.URL.RawQuery
+		}
+
 		r.Out.Host = r.In.Host
 		addProxyForwardedHeaders(r)
 		rp.lb.addTailscaleIdentityHeaders(r)

From 7c53bb23c55bf273894273add863eb1085638aa7 Mon Sep 17 00:00:00 2001
From: Victor Song <vsong1618@gmail.com>
Date: Sat, 24 Aug 2024 12:57:35 -0500
Subject: [PATCH 2/2] ipn/ipnlocal/serve: add tests for query parameter
 passthrough

Signed-off-by: Victor Song <vsong1618@gmail.com>
---
 ipn/ipnlocal/serve_test.go | 80 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go
index 3c028c65e..5c86ca015 100644
--- a/ipn/ipnlocal/serve_test.go
+++ b/ipn/ipnlocal/serve_test.go
@@ -757,6 +757,86 @@ func TestServeHTTPProxyHeaders(t *testing.T) {
 	}
 }
 
+func TestServeHTTPProxyQuery(t *testing.T) {
+	b := newTestBackend(t)
+
+	// Start test serve endpoint.
+	testServ := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			// Set the raw request query parameter to a response header, so
+			// the final output can can be checked in tests.
+			t.Logf("adding query parameters %s", r.URL.RawQuery)
+			w.Header().Add("QueryParam", r.URL.RawQuery)
+		},
+	))
+	defer testServ.Close()
+
+	// The input query parameters should always be the same as the output query parameters.
+	// For this reason, the tests only include the input query parameters
+	// (rather than both the input and expected query parameters).
+	tests := []struct {
+		name string
+		in   string
+	}{
+		{
+			name: "single well-formed query parameter",
+			in:   "key=value",
+		},
+		{
+			name: "single malformed query parameter",
+			in:   "key=value;nonsense",
+		},
+		{
+			name: "one malformed and one well-formed query parameter",
+			in:   "key1=value1;nonsense&key2=value2",
+		},
+		{
+			name: "one well-formed, one malformed, and another well-formed query parameter",
+			in:   "key1=value1&key2=value2;nonsense&key3=value3",
+		},
+		{
+			name: "one malformed and one well-formed query parameter (this time with percent sign)",
+			in:   "key1=value1%yes%%&key2=value2",
+		},
+	}
+
+	const testPath = "/foo"
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			conf := &ipn.ServeConfig{
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"example.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						testPath: {Proxy: testServ.URL + testPath},
+					}},
+				},
+			}
+
+			if err := b.SetServeConfig(conf, ""); err != nil {
+				t.Fatal(err)
+			}
+
+			req := &http.Request{
+				URL: &url.URL{Path: testPath, RawQuery: tt.in},
+				TLS: &tls.ConnectionState{ServerName: "example.ts.net"},
+			}
+			req = req.WithContext(serveHTTPContextKey.WithValue(req.Context(),
+				&serveHTTPContext{
+					DestPort: 443,
+					SrcAddr:  netip.MustParseAddrPort("1.2.3.4:1234"),
+				}))
+
+			w := httptest.NewRecorder()
+			b.serveWebHandler(w, req)
+
+			q := w.Result().Header.Get("QueryParam")
+			if q != tt.in {
+				t.Errorf("wanted query params %q got %q", tt.in, q)
+			}
+		})
+	}
+}
+
 func Test_reverseProxyConfiguration(t *testing.T) {
 	b := newTestBackend(t)
 	type test struct {