From 27726868b3d7c613844b55cd209ca93645c99b85 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 21 Jan 2022 16:43:04 +0100 Subject: [PATCH] [v7.5.x] Fix for CVE-2022-21702 (#226) Fix for CVE-2022-21702 --- pkg/api/pluginproxy/ds_proxy.go | 1 + pkg/api/pluginproxy/ds_proxy_test.go | 12 +++++++ pkg/api/pluginproxy/pluginproxy.go | 8 ++++- pkg/api/pluginproxy/pluginproxy_test.go | 44 +++++++++++++++++++++++ pkg/plugins/backendplugin/manager.go | 1 + pkg/plugins/backendplugin/manager_test.go | 9 ++++- pkg/util/proxyutil/proxyutil.go | 6 ++++ 7 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index e5cf53dacc958..b5537ba6eb941 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -49,6 +49,7 @@ func (t *handleResponseTransport) RoundTrip(req *http.Request) (*http.Response, return nil, err } res.Header.Del("Set-Cookie") + proxyutil.SetProxyResponseHeaders(res.Header) return res, nil } diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 4f61e2055c471..be4bd8b38e7db 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -605,6 +605,18 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { assert.Equal(t, "important_cookie=important_value", proxy.ctx.Resp.Header().Get("Set-Cookie")) }) + t.Run("When response should set Content-Security-Policy header", func(t *testing.T) { + ctx, ds := setUp(t) + dsPlugin := &plugins.DataSourcePlugin{} + proxy, err := NewDataSourceProxy(ds, dsPlugin, ctx, "/render", &setting.Cfg{}) + require.NoError(t, err) + + proxy.HandleRequest() + + require.NoError(t, writeErr) + assert.Equal(t, "sandbox", proxy.ctx.Resp.Header().Get("Content-Security-Policy")) + }) + t.Run("Data source returns status code 401", func(t *testing.T) { ctx, ds := setUp(t, setUpCfg{ writeCb: func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/api/pluginproxy/pluginproxy.go b/pkg/api/pluginproxy/pluginproxy.go index 34e88459ef37b..5b0c3e436c25d 100644 --- a/pkg/api/pluginproxy/pluginproxy.go +++ b/pkg/api/pluginproxy/pluginproxy.go @@ -71,5 +71,11 @@ func NewApiPluginProxy(ctx *models.ReqContext, proxyPath string, route *plugins. } } - return &httputil.ReverseProxy{Director: director} + return &httputil.ReverseProxy{Director: director, ModifyResponse: modifyResponse} +} + +func modifyResponse(resp *http.Response) error { + proxyutil.SetProxyResponseHeaders(resp.Header) + + return nil } diff --git a/pkg/api/pluginproxy/pluginproxy_test.go b/pkg/api/pluginproxy/pluginproxy_test.go index 7b538139bbc14..b9873ef4f3973 100644 --- a/pkg/api/pluginproxy/pluginproxy_test.go +++ b/pkg/api/pluginproxy/pluginproxy_test.go @@ -2,6 +2,7 @@ package pluginproxy import ( "net/http" + "net/http/httptest" "testing" "github.com/grafana/grafana/pkg/bus" @@ -11,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + macaron "gopkg.in/macaron.v1" ) func TestPluginProxy(t *testing.T) { @@ -148,6 +150,48 @@ func TestPluginProxy(t *testing.T) { ) assert.Equal(t, "https://example.com", req.URL.String()) }) + + t.Run("When proxying a request should set expected response headers", func(t *testing.T) { + requestHandled := false + backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + _, _ = w.Write([]byte("I am the backend")) + requestHandled = true + })) + + responseRecorder := &closeNotifierResponseRecorder{ + ResponseRecorder: httptest.NewRecorder(), + } + responseWriter := macaron.NewResponseWriter("GET", responseRecorder) + + t.Cleanup(responseRecorder.Close) + t.Cleanup(backendServer.Close) + + route := &plugins.AppPluginRoute{ + Path: "/", + URL: backendServer.URL, + } + + ctx := &models.ReqContext{ + SignedInUser: &models.SignedInUser{}, + Context: &macaron.Context{ + Req: macaron.Request{ + Request: httptest.NewRequest("GET", "/", nil), + }, + Resp: responseWriter, + }, + } + proxy := NewApiPluginProxy(ctx, "", route, "", &setting.Cfg{}) + proxy.ServeHTTP(ctx.Resp, ctx.Req.Request) + + for { + if requestHandled { + break + } + } + + require.Equal(t, "sandbox", ctx.Resp.Header().Get("Content-Security-Policy")) + }) } // getPluginProxiedRequest is a helper for easier setup of tests based on global config and ReqContext. diff --git a/pkg/plugins/backendplugin/manager.go b/pkg/plugins/backendplugin/manager.go index 600e5c40acb2b..cc0c902fb6cdf 100644 --- a/pkg/plugins/backendplugin/manager.go +++ b/pkg/plugins/backendplugin/manager.go @@ -406,6 +406,7 @@ func flushStream(plugin Plugin, stream CallResourceClientResponseStream, w http. } } + proxyutil.SetProxyResponseHeaders(w.Header()) w.WriteHeader(resp.Status) } diff --git a/pkg/plugins/backendplugin/manager_test.go b/pkg/plugins/backendplugin/manager_test.go index daa648e156c7a..e6155a00d949b 100644 --- a/pkg/plugins/backendplugin/manager_test.go +++ b/pkg/plugins/backendplugin/manager_test.go @@ -177,7 +177,8 @@ func TestManager(t *testing.T) { t.Run("Call resource should return expected response", func(t *testing.T) { ctx.plugin.CallResourceHandlerFunc = backend.CallResourceHandlerFunc(func(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { return sender.Send(&backend.CallResourceResponse{ - Status: http.StatusOK, + Status: http.StatusOK, + Headers: map[string][]string{}, }) }) @@ -186,7 +187,13 @@ func TestManager(t *testing.T) { w := httptest.NewRecorder() err = ctx.manager.callResourceInternal(w, req, backend.PluginContext{PluginID: testPluginID}) require.NoError(t, err) + for { + if w.Flushed { + break + } + } require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, "sandbox", w.Header().Get("Content-Security-Policy")) }) }) }) diff --git a/pkg/util/proxyutil/proxyutil.go b/pkg/util/proxyutil/proxyutil.go index d6e3572133370..3db22a1426ed1 100644 --- a/pkg/util/proxyutil/proxyutil.go +++ b/pkg/util/proxyutil/proxyutil.go @@ -42,3 +42,9 @@ func ClearCookieHeader(req *http.Request, keepCookiesNames []string) { req.AddCookie(c) } } + +// SetProxyResponseHeaders sets proxy response headers. +// Sets Content-Security-Policy: sandbox +func SetProxyResponseHeaders(header http.Header) { + header.Set("Content-Security-Policy", "sandbox") +}