From be3e8e3332010ac411433fff15c9d8dbd3501b7f Mon Sep 17 00:00:00 2001
From: Devon Steenberg <devon.steenberg@portainer.io>
Date: Wed, 16 Apr 2025 15:30:56 +1200
Subject: [PATCH] fix(proxy): don't forward sensitive headers [BE-11819] (#654)

---
 api/http/proxy/factory/reverse_proxy.go      |  12 +-
 api/http/proxy/factory/reverse_proxy_test.go | 116 +++++++++++++++++++
 2 files changed, 126 insertions(+), 2 deletions(-)
 create mode 100644 api/http/proxy/factory/reverse_proxy_test.go

diff --git a/api/http/proxy/factory/reverse_proxy.go b/api/http/proxy/factory/reverse_proxy.go
index fdee70c4d..d583d75fe 100644
--- a/api/http/proxy/factory/reverse_proxy.go
+++ b/api/http/proxy/factory/reverse_proxy.go
@@ -11,8 +11,13 @@ import (
 // from golang.org/src/net/http/httputil/reverseproxy.go and merely sets the Host
 // HTTP header, which NewSingleHostReverseProxy deliberately preserves.
 func NewSingleHostReverseProxyWithHostHeader(target *url.URL) *httputil.ReverseProxy {
+	return &httputil.ReverseProxy{Director: createDirector(target)}
+}
+
+func createDirector(target *url.URL) func(*http.Request) {
+	sensitiveHeaders := []string{"Cookie", "X-Csrf-Token"}
 	targetQuery := target.RawQuery
-	director := func(req *http.Request) {
+	return func(req *http.Request) {
 		req.URL.Scheme = target.Scheme
 		req.URL.Host = target.Host
 		req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
@@ -26,8 +31,11 @@ func NewSingleHostReverseProxyWithHostHeader(target *url.URL) *httputil.ReverseP
 			// explicitly disable User-Agent so it's not set to default value
 			req.Header.Set("User-Agent", "")
 		}
+
+		for _, header := range sensitiveHeaders {
+			delete(req.Header, header)
+		}
 	}
-	return &httputil.ReverseProxy{Director: director}
 }
 
 // singleJoiningSlash from golang.org/src/net/http/httputil/reverseproxy.go
diff --git a/api/http/proxy/factory/reverse_proxy_test.go b/api/http/proxy/factory/reverse_proxy_test.go
new file mode 100644
index 000000000..1a3d88ba0
--- /dev/null
+++ b/api/http/proxy/factory/reverse_proxy_test.go
@@ -0,0 +1,116 @@
+package factory
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func Test_createDirector(t *testing.T) {
+	testCases := []struct {
+		name        string
+		target      *url.URL
+		req         *http.Request
+		expectedReq *http.Request
+	}{
+		{
+			name:   "base case",
+			target: createURL(t, "https://portainer.io/api/docker?a=5&b=6"),
+			req: createRequest(
+				t,
+				"GET",
+				"https://agent-portainer.io/test?c=7",
+				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json", "User-Agent": "something"},
+			),
+			expectedReq: createRequest(
+				t,
+				"GET",
+				"https://portainer.io/api/docker/test?a=5&b=6&c=7",
+				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json", "User-Agent": "something"},
+			),
+		},
+		{
+			name:   "no User-Agent",
+			target: createURL(t, "https://portainer.io/api/docker?a=5&b=6"),
+			req: createRequest(
+				t,
+				"GET",
+				"https://agent-portainer.io/test?c=7",
+				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json"},
+			),
+			expectedReq: createRequest(
+				t,
+				"GET",
+				"https://portainer.io/api/docker/test?a=5&b=6&c=7",
+				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json", "User-Agent": ""},
+			),
+		},
+		{
+			name:   "Sensitive Headers",
+			target: createURL(t, "https://portainer.io/api/docker?a=5&b=6"),
+			req: createRequest(
+				t,
+				"GET",
+				"https://agent-portainer.io/test?c=7",
+				map[string]string{
+					"Accept-Encoding": "gzip",
+					"Accept":          "application/json",
+					"User-Agent":      "something",
+					"Cookie":          "junk",
+					"X-Csrf-Token":    "junk",
+				},
+			),
+			expectedReq: createRequest(
+				t,
+				"GET",
+				"https://portainer.io/api/docker/test?a=5&b=6&c=7",
+				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json", "User-Agent": "something"},
+			),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			director := createDirector(tc.target)
+			director(tc.req)
+
+			if diff := cmp.Diff(tc.req, tc.expectedReq, cmp.Comparer(compareRequests)); diff != "" {
+				t.Fatalf("requests are different: \n%s", diff)
+			}
+		})
+	}
+}
+
+func createURL(t *testing.T, urlString string) *url.URL {
+	parsedURL, err := url.Parse(urlString)
+	if err != nil {
+		t.Fatalf("Failed to create url: %s", err)
+	}
+
+	return parsedURL
+}
+
+func createRequest(t *testing.T, method, url string, headers map[string]string) *http.Request {
+	req, err := http.NewRequest(method, url, nil)
+	if err != nil {
+		t.Fatalf("Failed to create http request: %s", err)
+	} else {
+		for k, v := range headers {
+			req.Header.Add(k, v)
+		}
+	}
+
+	return req
+}
+
+func compareRequests(a, b *http.Request) bool {
+	methodEqual := a.Method == b.Method
+	urlEqual := cmp.Diff(a.URL, b.URL) == ""
+	hostEqual := a.Host == b.Host
+	protoEqual := a.Proto == b.Proto && a.ProtoMajor == b.ProtoMajor && a.ProtoMinor == b.ProtoMinor
+	headersEqual := cmp.Diff(a.Header, b.Header) == ""
+
+	return methodEqual && urlEqual && hostEqual && protoEqual && headersEqual
+}