fix(proxy): whitelist headers for proxy to forward [BE-11819] (#665)

api/http/proxy/factory/reverse_proxy.go

@@ -7,6 +7,21 @@ import (
 	"strings"
 )
 
+// Note that we discard any non-canonical headers by design
+var allowedHeaders = map[string]struct{}{
+	"Accept":                  {},
+	"Accept-Encoding":         {},
+	"Accept-Language":         {},
+	"Cache-Control":           {},
+	"Content-Length":          {},
+	"Content-Type":            {},
+	"Private-Token":           {},
+	"User-Agent":              {},
+	"X-Portaineragent-Target": {},
+	"X-Portainer-Volumename":  {},
+	"X-Registry-Auth":         {},
+}
+
 // newSingleHostReverseProxyWithHostHeader is based on NewSingleHostReverseProxy
 // from golang.org/src/net/http/httputil/reverseproxy.go and merely sets the Host
 // HTTP header, which NewSingleHostReverseProxy deliberately preserves.
@@ -15,7 +30,6 @@ func NewSingleHostReverseProxyWithHostHeader(target *url.URL) *httputil.ReverseP
 }
 
 func createDirector(target *url.URL) func(*http.Request) {
-	sensitiveHeaders := []string{"Cookie", "X-Csrf-Token"}
 	targetQuery := target.RawQuery
 	return func(req *http.Request) {
 		req.URL.Scheme = target.Scheme
@@ -32,8 +46,11 @@ func createDirector(target *url.URL) func(*http.Request) {
 			req.Header.Set("User-Agent", "")
 		}
 
-		for _, header := range sensitiveHeaders {
+		for k := range req.Header {
-			delete(req.Header, header)
+			if _, ok := allowedHeaders[k]; !ok {
+				// We use delete here instead of req.Header.Del because we want to delete non canonical headers.
+				delete(req.Header, k)
+			}
 		}
 	}
 }

api/http/proxy/factory/reverse_proxy_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	portainer "github.com/portainer/portainer/api"
 )
 
 func Test_createDirector(t *testing.T) {
@@ -23,12 +24,14 @@ func Test_createDirector(t *testing.T) {
 				"GET",
 				"https://agent-portainer.io/test?c=7",
 				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json", "User-Agent": "something"},
+				true,
 			),
 			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"},
+				true,
 			),
 		},
 		{
@@ -39,12 +42,14 @@ func Test_createDirector(t *testing.T) {
 				"GET",
 				"https://agent-portainer.io/test?c=7",
 				map[string]string{"Accept-Encoding": "gzip", "Accept": "application/json"},
+				true,
 			),
 			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": ""},
+				true,
 			),
 		},
 		{
@@ -55,18 +60,83 @@ func Test_createDirector(t *testing.T) {
 				"GET",
 				"https://agent-portainer.io/test?c=7",
 				map[string]string{
-					"Accept-Encoding": "gzip",
+					"Authorization":           "secret",
-					"Accept":          "application/json",
+					"Proxy-Authorization":     "secret",
-					"User-Agent":      "something",
+					"Cookie":                  "secret",
-					"Cookie":          "junk",
+					"X-Csrf-Token":            "secret",
-					"X-Csrf-Token":    "junk",
+					"X-Api-Key":               "secret",
+					"Accept":                  "application/json",
+					"Accept-Encoding":         "gzip",
+					"Accept-Language":         "en-GB",
+					"Cache-Control":           "None",
+					"Content-Length":          "100",
+					"Content-Type":            "application/json",
+					"Private-Token":           "test-private-token",
+					"User-Agent":              "test-user-agent",
+					"X-Portaineragent-Target": "test-agent-1",
+					"X-Portainer-Volumename":  "test-volume-1",
+					"X-Registry-Auth":         "test-registry-auth",
 				},
+				true,
 			),
 			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"},
+				map[string]string{
+					"Accept":                  "application/json",
+					"Accept-Encoding":         "gzip",
+					"Accept-Language":         "en-GB",
+					"Cache-Control":           "None",
+					"Content-Length":          "100",
+					"Content-Type":            "application/json",
+					"Private-Token":           "test-private-token",
+					"User-Agent":              "test-user-agent",
+					"X-Portaineragent-Target": "test-agent-1",
+					"X-Portainer-Volumename":  "test-volume-1",
+					"X-Registry-Auth":         "test-registry-auth",
+				},
+				true,
+			),
+		},
+		{
+			name:   "Non canonical 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":                             "application/json",
+					"Accept-Encoding":                    "gzip",
+					"Accept-Language":                    "en-GB",
+					"Cache-Control":                      "None",
+					"Content-Length":                     "100",
+					"Content-Type":                       "application/json",
+					"Private-Token":                      "test-private-token",
+					"User-Agent":                         "test-user-agent",
+					portainer.PortainerAgentTargetHeader: "test-agent-1",
+					"X-Portainer-VolumeName":             "test-volume-1",
+					"X-Registry-Auth":                    "test-registry-auth",
+				},
+				false,
+			),
+			expectedReq: createRequest(
+				t,
+				"GET",
+				"https://portainer.io/api/docker/test?a=5&b=6&c=7",
+				map[string]string{
+					"Accept":          "application/json",
+					"Accept-Encoding": "gzip",
+					"Accept-Language": "en-GB",
+					"Cache-Control":   "None",
+					"Content-Length":  "100",
+					"Content-Type":    "application/json",
+					"Private-Token":   "test-private-token",
+					"User-Agent":      "test-user-agent",
+					"X-Registry-Auth": "test-registry-auth",
+				},
+				true,
 			),
 		},
 	}
@@ -92,13 +162,17 @@ func createURL(t *testing.T, urlString string) *url.URL {
 	return parsedURL
 }
 
-func createRequest(t *testing.T, method, url string, headers map[string]string) *http.Request {
+func createRequest(t *testing.T, method, url string, headers map[string]string, canonicalHeaders bool) *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)
+			if canonicalHeaders {
+				req.Header.Add(k, v)
+			} else {
+				req.Header[k] = []string{v}
+			}
 		}
 	}