Merge 7c53bb23c55bf273894273add863eb1085638aa7 into b3455fa99a5e8d07133d5140017ec7c49f032a07

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)

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 {