[v8.3.x] Sync security changes (#45067)

* "Release: Updated versions in package to 8.3.5"

* [v8.3.x] Fix for CVE-2022-21702 (#225)

Fix for CVE-2022-21702

* Update yarn.lock for 8.3.5

* resolve conflicts

(cherry picked from commit bb38cfcba4b4f824060ff385d858c63f50b72d74)

* csrf checks for v8.3.5 (#234)

* Fix lint

* Cherry pick e2e test server changes

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-authored-by: Kevin Minehart <kmineh0151@gmail.com>
Co-authored-by: Serge Zaitsev <serge.zaitsev@grafana.com>

pkg/api/api.go

@@ -29,7 +29,7 @@ func (hs *HTTPServer) registerRoutes() {
 	reqEditorRole := middleware.ReqEditorRole
 	reqOrgAdmin := middleware.ReqOrgAdmin
 	reqOrgAdminFolderAdminOrTeamAdmin := middleware.OrgAdminFolderAdminOrTeamAdmin
-	reqCanAccessTeams := middleware.AdminOrFeatureEnabled(hs.Cfg.EditorsCanAdmin)
+	reqCanAccessTeams := middleware.AdminOrEditorAndFeatureEnabled(hs.Cfg.EditorsCanAdmin)
 	reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
 	redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)
 	authorize := acmiddleware.Middleware(hs.AccessControl)

pkg/api/http_server.go

@@ -422,6 +422,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
 	}
 
 	m.Use(middleware.Recovery(hs.Cfg))
+	m.UseMiddleware(middleware.CSRF(hs.Cfg.LoginCookieName))
 
 	hs.mapStatic(m, hs.Cfg.StaticRootPath, "build", "public/build")
 	hs.mapStatic(m, hs.Cfg.StaticRootPath, "", "public")

pkg/api/pluginproxy/ds_proxy.go

@@ -54,6 +54,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
 }
 

pkg/api/pluginproxy/ds_proxy_test.go

@@ -663,6 +663,20 @@ 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)
+		var routes []*plugins.Route
+		secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
+		dsService := datasources.ProvideService(bus.New(), nil, secretsService)
+		proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService)
+		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) {

pkg/api/pluginproxy/pluginproxy.go

@@ -83,5 +83,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
 }

pkg/api/pluginproxy/pluginproxy_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"io/ioutil"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -13,6 +14,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets/fakes"
 	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/web"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/macaron.v1"
@@ -245,6 +247,41 @@ func TestPluginProxy(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, `{ "url": "https://dynamic.grafana.com", "secret": "123"	}`, string(content))
 	})
+
+	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
+		}))
+		t.Cleanup(backendServer.Close)
+
+		responseWriter := web.NewResponseWriter("GET", httptest.NewRecorder())
+
+		route := &plugins.Route{
+			Path: "/",
+			URL:  backendServer.URL,
+		}
+
+		ctx := &models.ReqContext{
+			SignedInUser: &models.SignedInUser{},
+			Context: &web.Context{
+				Req:  httptest.NewRequest("GET", "/", nil),
+				Resp: responseWriter,
+			},
+		}
+		proxy := NewApiPluginProxy(ctx, "", route, "", &setting.Cfg{}, secretsService)
+		proxy.ServeHTTP(ctx.Resp, ctx.Req)
+
+		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.

pkg/api/team.go

@@ -2,6 +2,8 @@ package api
 
 import (
 	"errors"
+	"net/http"
+	"strconv"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
@@ -10,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/teamguardian"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/web"
 )
 
 // POST /api/teams
@@ -95,16 +98,11 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response {
 		page = 1
 	}
 
-	var userIdFilter int64
-	if hs.Cfg.EditorsCanAdmin && c.OrgRole != models.ROLE_ADMIN {
-		userIdFilter = c.SignedInUser.UserId
-	}
-
 	query := models.SearchTeamsQuery{
 		OrgId:        c.OrgId,
 		Query:        c.Query("query"),
 		Name:         c.Query("name"),
-		UserIdFilter: userIdFilter,
+		UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c),
 		Page:         page,
 		Limit:        perPage,
 		SignedInUser: c.SignedInUser,
@@ -125,13 +123,32 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response {
 	return response.JSON(200, query.Result)
 }
 
+// UserFilter returns the user ID used in a filter when querying a team
+// 1. If the user is a viewer or editor, this will return the user's ID.
+// 2. If EditorsCanAdmin is enabled and the user is an editor, this will return models.FilterIgnoreUser (0)
+// 3. If the user is an admin, this will return models.FilterIgnoreUser (0)
+func userFilter(editorsCanAdmin bool, c *models.ReqContext) int64 {
+	userIdFilter := c.SignedInUser.UserId
+	if (editorsCanAdmin && c.OrgRole == models.ROLE_EDITOR) || c.OrgRole == models.ROLE_ADMIN {
+		userIdFilter = models.FilterIgnoreUser
+	}
+
+	return userIdFilter
+}
+
 // GET /api/teams/:teamId
 func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response {
+	teamId, err := strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64)
+	if err != nil {
+		return response.Error(http.StatusBadRequest, "teamId is invalid", err)
+	}
+
 	query := models.GetTeamByIdQuery{
 		OrgId:        c.OrgId,
-		Id:           c.ParamsInt64(":teamId"),
+		Id:           teamId,
 		SignedInUser: c.SignedInUser,
 		HiddenUsers:  hs.Cfg.HiddenUsers,
+		UserIdFilter: userFilter(hs.Cfg.EditorsCanAdmin, c),
 	}
 
 	if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil {

pkg/api/team_members.go

@@ -16,6 +16,10 @@ import (
 func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response {
 	query := models.GetTeamMembersQuery{OrgId: c.OrgId, TeamId: c.ParamsInt64(":teamId")}
 
+	if err := teamguardian.CanAdmin(hs.Bus, query.OrgId, query.TeamId, c.SignedInUser); err != nil {
+		return response.Error(403, "Not allowed to list team members", err)
+	}
+
 	if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil {
 		return response.Error(500, "Failed to get Team Members", err)
 	}

pkg/macaron/binding.go

@@ -2,8 +2,10 @@ package macaron
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"mime"
 	"net/http"
 	"reflect"
 )
@@ -11,9 +13,16 @@ import (
 // Bind deserializes JSON payload from the request
 func Bind(req *http.Request, v interface{}) error {
 	if req.Body != nil {
-		defer req.Body.Close()
-		err := json.NewDecoder(req.Body).Decode(v)
-		if err != nil && err != io.EOF {
+		m, _, err := mime.ParseMediaType(req.Header.Get("Content-type"))
+		if err != nil {
+			return err
+		}
+		if m != "application/json" {
+			return errors.New("bad content type")
+		}
+		defer func() { _ = req.Body.Close() }()
+		err = json.NewDecoder(req.Body).Decode(v)
+		if err != nil && !errors.Is(err, io.EOF) {
 			return err
 		}
 	}
@@ -29,7 +38,7 @@ func validate(obj interface{}) error {
 	if validator, ok := obj.(Validator); ok {
 		return validator.Validate()
 	}
-	// Otherwise, use relfection to match `binding:"Required"` struct field tags.
+	// Otherwise, use reflection to match `binding:"Required"` struct field tags.
 	// Resolve all pointers and interfaces, until we get a concrete type.
 	t := reflect.TypeOf(obj)
 	v := reflect.ValueOf(obj)
@@ -69,6 +78,7 @@ func validate(obj interface{}) error {
 				return err
 			}
 		}
+	default: // ignore
 	}
 	return nil
 }

pkg/middleware/auth.go

@@ -128,20 +128,22 @@ func Auth(options *AuthOptions) web.Handler {
 	}
 }
 
-// AdminOrFeatureEnabled creates a middleware that allows access
-// if the signed in user is either an Org Admin or if the
-// feature flag is enabled.
+// AdminOrEditorAndFeatureEnabled creates a middleware that allows
+// access if the signed in user is either an Org Admin or if they
+// are an Org Editor and the feature flag is enabled.
 // Intended for when feature flags open up access to APIs that
 // are otherwise only available to admins.
-func AdminOrFeatureEnabled(enabled bool) web.Handler {
+func AdminOrEditorAndFeatureEnabled(enabled bool) web.Handler {
 	return func(c *models.ReqContext) {
 		if c.OrgRole == models.ROLE_ADMIN {
 			return
 		}
 
-		if !enabled {
-			accessForbidden(c)
+		if c.OrgRole == models.ROLE_EDITOR && enabled {
+			return
 		}
+
+		accessForbidden(c)
 	}
 }
 

pkg/middleware/csrf.go

@@ -0,0 +1,39 @@
+package middleware
+
+import (
+	"errors"
+	"net/http"
+	"net/url"
+	"strings"
+)
+
+func CSRF(loginCookieName string) func(http.Handler) http.Handler {
+	// As per RFC 7231/4.2.2 these methods are idempotent:
+	// (GET is excluded because it may have side effects in some APIs)
+	safeMethods := []string{"HEAD", "OPTIONS", "TRACE"}
+
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// If request has no login cookie - skip CSRF checks
+			if _, err := r.Cookie(loginCookieName); errors.Is(err, http.ErrNoCookie) {
+				next.ServeHTTP(w, r)
+				return
+			}
+			// Skip CSRF checks for "safe" methods
+			for _, method := range safeMethods {
+				if r.Method == method {
+					next.ServeHTTP(w, r)
+					return
+				}
+			}
+			// Otherwise - verify that Origin matches the server origin
+			host := strings.Split(r.Host, ":")[0]
+			origin, err := url.Parse(r.Header.Get("Origin"))
+			if err != nil || (origin.String() != "" && origin.Hostname() != host) {
+				http.Error(w, "origin not allowed", http.StatusForbidden)
+				return
+			}
+			next.ServeHTTP(w, r)
+		})
+	}
+}

pkg/models/team.go

@@ -55,8 +55,12 @@ type GetTeamByIdQuery struct {
 	SignedInUser *SignedInUser
 	HiddenUsers  map[string]struct{}
 	Result       *TeamDTO
+	UserIdFilter int64
 }
 
+// FilterIgnoreUser is used in a get / search teams query when the caller does not want to filter teams by user ID / membership
+const FilterIgnoreUser int64 = 0
+
 type GetTeamsByUserQuery struct {
 	OrgId  int64
 	UserId int64      `json:"userId"`

pkg/plugins/manager/manager.go

@@ -374,6 +374,8 @@ func flushStream(plugin backendplugin.Plugin, stream callResourceClientResponseS
 					w.Header().Add(k, v)
 				}
 			}
+
+			proxyutil.SetProxyResponseHeaders(w.Header())
 			w.WriteHeader(resp.Status)
 		}
 

pkg/plugins/manager/manager_test.go

@@ -447,7 +447,8 @@ func TestPluginManager_lifecycle_managed(t *testing.T) {
 						ctx.pluginClient.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{},
 							})
 						}
 
@@ -456,7 +457,13 @@ func TestPluginManager_lifecycle_managed(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"))
 					})
 				})
 			})

pkg/services/sqlstore/team.go

@@ -52,18 +52,6 @@ func getTeamMemberCount(filteredUsers []string) string {
 	return "(SELECT COUNT(*) FROM team_member WHERE team_member.team_id = team.id) AS member_count "
 }
 
-func getTeamSearchSQLBase(filteredUsers []string) string {
-	return `SELECT
-		team.id AS id,
-		team.org_id,
-		team.name AS name,
-		team.email AS email,
-		team_member.permission, ` +
-		getTeamMemberCount(filteredUsers) +
-		` FROM team AS team
-		INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ? `
-}
-
 func getTeamSelectSQLBase(filteredUsers []string) string {
 	return `SELECT
 		team.id as id,
@@ -182,17 +170,15 @@ func SearchTeams(ctx context.Context, query *models.SearchTeamsQuery) error {
 	params := make([]interface{}, 0)
 
 	filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers)
-	if query.UserIdFilter > 0 {
-		sql.WriteString(getTeamSearchSQLBase(filteredUsers))
-		for _, user := range filteredUsers {
-			params = append(params, user)
-		}
+	sql.WriteString(getTeamSelectSQLBase(filteredUsers))
+
+	for _, user := range filteredUsers {
+		params = append(params, user)
+	}
+
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`)
 		params = append(params, query.UserIdFilter)
-	} else {
-		sql.WriteString(getTeamSelectSQLBase(filteredUsers))
-		for _, user := range filteredUsers {
-			params = append(params, user)
-		}
 	}
 
 	sql.WriteString(` WHERE team.org_id = ?`)
@@ -221,6 +207,8 @@ func SearchTeams(ctx context.Context, query *models.SearchTeamsQuery) error {
 
 	team := models.Team{}
 	countSess := x.Table("team")
+	countSess.Where("team.org_id=?", query.OrgId)
+
 	if query.Query != "" {
 		countSess.Where(`name `+dialect.LikeStr()+` ?`, queryWithWildcards)
 	}
@@ -229,6 +217,18 @@ func SearchTeams(ctx context.Context, query *models.SearchTeamsQuery) error {
 		countSess.Where("name=?", query.Name)
 	}
 
+	// If we're not retrieving all results, then only search for teams that this user has access to
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		countSess.
+			Where(`
+			team.id IN (
+				SELECT
+				team_id
+				FROM team_member
+				WHERE team_member.user_id = ?
+			)`, query.UserIdFilter)
+	}
+
 	count, err := countSess.Count(&team)
 	query.Result.TotalCount = count
 
@@ -245,6 +245,11 @@ func GetTeamById(ctx context.Context, query *models.GetTeamByIdQuery) error {
 		params = append(params, user)
 	}
 
+	if query.UserIdFilter != models.FilterIgnoreUser {
+		sql.WriteString(` INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ?`)
+		params = append(params, query.UserIdFilter)
+	}
+
 	sql.WriteString(` WHERE team.org_id = ? and team.id = ?`)
 	params = append(params, query.OrgId, query.Id)
 

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")
+}