From 384cb53c64af78af8e1ac7ef5b0f91bad530e989 Mon Sep 17 00:00:00 2001 From: Devon Steenberg Date: Fri, 30 May 2025 11:49:41 +1200 Subject: [PATCH] fix(proxy): whitelist headers for proxy to forward [BE-11819] (#760) --- api/http/proxy/factory/reverse_proxy.go | 29 ++- api/http/proxy/factory/reverse_proxy_test.go | 190 +++++++++++++++++++ 2 files changed, 217 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 93d22f94d..932065638 100644 --- a/api/http/proxy/factory/reverse_proxy.go +++ b/api/http/proxy/factory/reverse_proxy.go @@ -7,12 +7,31 @@ 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. func newSingleHostReverseProxyWithHostHeader(target *url.URL) *httputil.ReverseProxy { + return &httputil.ReverseProxy{Director: createDirector(target)} +} + +func createDirector(target *url.URL) func(*http.Request) { 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 +45,14 @@ 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 k := range req.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) + } + } } - 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..6f23d75ec --- /dev/null +++ b/api/http/proxy/factory/reverse_proxy_test.go @@ -0,0 +1,190 @@ +package factory + +import ( + "net/http" + "net/url" + "testing" + + "github.com/google/go-cmp/cmp" + portainer "github.com/portainer/portainer/api" +) + +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"}, + 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, + ), + }, + { + 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"}, + 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, + ), + }, + { + 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{ + "Authorization": "secret", + "Proxy-Authorization": "secret", + "Cookie": "secret", + "X-Csrf-Token": "secret", + "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": "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, + ), + }, + } + + 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, 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 { + if canonicalHeaders { + req.Header.Add(k, v) + } else { + req.Header[k] = []string{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 +}