[v9.0.x] CSRF: Fix additional headers option (#52347)

* merged and backport to 9.0.x * merged http_server * fix for provider being interface * fix for provider maybe * wire inject * wire inject * wire inject Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com>
2022-07-19 07:50:31 +01:00
parent 198f690667
commit e4c221afc4
4 changed files with 289 additions and 3 deletions
@@ -14,6 +14,7 @@ import (
 	"sync"

 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/middleware/csrf"

 	"github.com/grafana/grafana/pkg/api/avatar"
 	"github.com/grafana/grafana/pkg/api/routing"
@@ -159,6 +160,7 @@ type HTTPServer struct {
 	dashboardPermissionsService  accesscontrol.DashboardPermissionsService
 	starService                  star.Service
 	CoremodelRegistry            *coremodel.Registry
+	Csrf                         csrf.Service
 }

 type ServerOptions struct {
@@ -193,6 +195,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	avatarCacheServer *avatar.AvatarCacheServer, preferenceService pref.Service, entityEventsService store.EntityEventsService,
 	teamsPermissionsService accesscontrol.TeamPermissionsService, folderPermissionsService accesscontrol.FolderPermissionsService,
 	dashboardPermissionsService accesscontrol.DashboardPermissionsService, starService star.Service, coremodelRegistry *coremodel.Registry,
+	csrf csrf.Service,
 ) (*HTTPServer, error) {
 	web.Env = cfg.Env
 	m := web.New()
@@ -272,6 +275,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		dashboardPermissionsService:  dashboardPermissionsService,
 		starService:                  starService,
 		CoremodelRegistry:            coremodelRegistry,
+		Csrf:                         csrf,
 	}
 	if hs.Listener != nil {
 		hs.log.Debug("Using provided listener")
@@ -499,7 +503,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
 	}

 	m.Use(middleware.Recovery(hs.Cfg))
-	m.UseMiddleware(middleware.CSRF(hs.Cfg.LoginCookieName, hs.log))
+	m.UseMiddleware(hs.Csrf.Middleware())

 	hs.mapStatic(m, hs.Cfg.StaticRootPath, "build", "public/build")
 	hs.mapStatic(m, hs.Cfg.StaticRootPath, "", "public", "/public/views/swagger.html")
@@ -0,0 +1,161 @@
+package csrf
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+	"reflect"
+
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util"
+)
+
+type Service interface {
+	Middleware() func(http.Handler) http.Handler
+	TrustOrigin(origin string)
+	AddAdditionalHeaders(headerName string)
+	AddSafeEndpoint(endpoint string)
+}
+
+type CSRF struct {
+	Cfg *setting.Cfg
+
+	TrustedOrigins map[string]struct{}
+	Headers        map[string]struct{}
+	SafeEndpoints  map[string]struct{}
+}
+
+func ProvideCSRFFilter(cfg *setting.Cfg) Service {
+	c := &CSRF{
+		Cfg:            cfg,
+		TrustedOrigins: map[string]struct{}{},
+		Headers:        map[string]struct{}{},
+		SafeEndpoints:  map[string]struct{}{},
+	}
+
+	additionalHeaders := cfg.SectionWithEnvOverrides("security").Key("csrf_additional_headers").Strings(" ")
+	trustedOrigins := cfg.SectionWithEnvOverrides("security").Key("csrf_trusted_origins").Strings(" ")
+
+	for _, header := range additionalHeaders {
+		c.Headers[header] = struct{}{}
+	}
+	for _, origin := range trustedOrigins {
+		c.TrustedOrigins[origin] = struct{}{}
+	}
+
+	return c
+}
+
+func (c *CSRF) Middleware() func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			e := &ErrorWithStatus{}
+
+			err := c.Check(r)
+			if err != nil {
+				if !errors.As(err, &e) {
+					http.Error(w, fmt.Sprintf("internal server error: expected error type errorWithStatus, got %s. Error: %v", reflect.TypeOf(err), err), http.StatusInternalServerError)
+				}
+				http.Error(w, err.Error(), e.HTTPStatus)
+				return
+			}
+
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
+func (c *CSRF) Check(r *http.Request) error {
+	// 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"}
+
+	// If request has no login cookie - skip CSRF checks
+	if _, err := r.Cookie(c.Cfg.LoginCookieName); errors.Is(err, http.ErrNoCookie) {
+		return nil
+	}
+	// Skip CSRF checks for "safe" methods
+	for _, method := range safeMethods {
+		if r.Method == method {
+			return nil
+		}
+	}
+	// Skip CSRF checks for "safe" endpoints
+	for safeEndpoint := range c.SafeEndpoints {
+		if r.URL.Path == safeEndpoint {
+			return nil
+		}
+	}
+	// Otherwise - verify that Origin matches the server origin
+	netAddr, err := util.SplitHostPortDefault(r.Host, "", "0") // we ignore the port
+	if err != nil {
+		return &ErrorWithStatus{Underlying: err, HTTPStatus: http.StatusBadRequest}
+	}
+
+	o := r.Header.Get("Origin")
+
+	// No Origin header sent, skip CSRF check.
+	if o == "" {
+		return nil
+	}
+
+	originURL, err := url.Parse(o)
+	if err != nil {
+		return &ErrorWithStatus{Underlying: err, HTTPStatus: http.StatusBadRequest}
+	}
+	origin := originURL.Hostname()
+
+	trustedOrigin := false
+	for h := range c.Headers {
+		customHost := r.Header.Get(h)
+		addr, err := util.SplitHostPortDefault(customHost, "", "0") // we ignore the port
+		if err != nil {
+			return &ErrorWithStatus{Underlying: err, HTTPStatus: http.StatusBadRequest}
+		}
+		if addr.Host == origin {
+			trustedOrigin = true
+			break
+		}
+	}
+
+	for o := range c.TrustedOrigins {
+		if o == origin {
+			trustedOrigin = true
+			break
+		}
+	}
+
+	hostnameMatches := origin == netAddr.Host
+	if netAddr.Host == "" || !trustedOrigin && !hostnameMatches {
+		return &ErrorWithStatus{Underlying: errors.New("origin not allowed"), HTTPStatus: http.StatusForbidden}
+	}
+
+	return nil
+}
+
+func (c *CSRF) TrustOrigin(origin string) {
+	c.TrustedOrigins[origin] = struct{}{}
+}
+
+func (c *CSRF) AddAdditionalHeaders(headerName string) {
+	c.Headers[headerName] = struct{}{}
+}
+
+// AddSafeEndpoint is used for endpoints requests to skip CSRF check
+func (c *CSRF) AddSafeEndpoint(endpoint string) {
+	c.SafeEndpoints[endpoint] = struct{}{}
+}
+
+type ErrorWithStatus struct {
+	Underlying error
+	HTTPStatus int
+}
+
+func (e ErrorWithStatus) Error() string {
+	return e.Underlying.Error()
+}
+
+func (e ErrorWithStatus) Unwrap() error {
+	return e.Underlying
+}
@@ -1,12 +1,17 @@
 package middleware

 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"

-	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/middleware/csrf"
+	"github.com/grafana/grafana/pkg/setting"
 )

 func TestMiddlewareCSRF(t *testing.T) {
@@ -98,6 +103,117 @@ func TestMiddlewareCSRF(t *testing.T) {
 	}
 }

+func TestCSRF_Check(t *testing.T) {
+	tests := []struct {
+		name           string
+		request        *http.Request
+		addtHeader     map[string]struct{}
+		trustedOrigins map[string]struct{}
+		safeEndpoints  map[string]struct{}
+		expectedOK     bool
+		expectedStatus int
+	}{
+		{
+			name:       "base case",
+			request:    postRequest(t, "", nil),
+			expectedOK: true,
+		},
+		{
+			name:           "base with null origin header",
+			request:        postRequest(t, "", map[string]string{"Origin": "null"}),
+			expectedStatus: http.StatusForbidden,
+		},
+		{
+			name:       "grafana.org",
+			request:    postRequest(t, "grafana.org", map[string]string{"Origin": "https://grafana.org"}),
+			expectedOK: true,
+		},
+		{
+			name:           "grafana.org with X-Forwarded-Host",
+			request:        postRequest(t, "grafana.localhost", map[string]string{"X-Forwarded-Host": "grafana.org", "Origin": "https://grafana.org"}),
+			expectedStatus: http.StatusForbidden,
+		},
+		{
+			name:       "grafana.org with X-Forwarded-Host and header trusted",
+			request:    postRequest(t, "grafana.localhost", map[string]string{"X-Forwarded-Host": "grafana.org", "Origin": "https://grafana.org"}),
+			addtHeader: map[string]struct{}{"X-Forwarded-Host": {}},
+			expectedOK: true,
+		},
+		{
+			name:           "grafana.org from grafana.com",
+			request:        postRequest(t, "grafana.org", map[string]string{"Origin": "https://grafana.com"}),
+			expectedStatus: http.StatusForbidden,
+		},
+		{
+			name:           "grafana.org from grafana.com explicit trust for grafana.com",
+			request:        postRequest(t, "grafana.org", map[string]string{"Origin": "https://grafana.com"}),
+			trustedOrigins: map[string]struct{}{"grafana.com": {}},
+			expectedOK:     true,
+		},
+		{
+			name:           "grafana.org from grafana.com with X-Forwarded-Host and header trusted",
+			request:        postRequest(t, "grafana.localhost", map[string]string{"X-Forwarded-Host": "grafana.org", "Origin": "https://grafana.com"}),
+			addtHeader:     map[string]struct{}{"X-Forwarded-Host": {}},
+			trustedOrigins: map[string]struct{}{"grafana.com": {}},
+			expectedOK:     true,
+		},
+		{
+			name:          "safe endpoint",
+			request:       postRequest(t, "example.org/foo/bar", map[string]string{"Origin": "null"}),
+			safeEndpoints: map[string]struct{}{"foo/bar": {}},
+			expectedOK:    true,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			c := csrf.CSRF{
+				Cfg:            setting.NewCfg(),
+				TrustedOrigins: tc.trustedOrigins,
+				Headers:        tc.addtHeader,
+				SafeEndpoints:  tc.safeEndpoints,
+			}
+			c.Cfg.LoginCookieName = "LoginCookie"
+
+			err := c.Check(tc.request)
+			if tc.expectedOK {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				var actual *csrf.ErrorWithStatus
+				require.True(t, errors.As(err, &actual))
+				assert.EqualValues(t, tc.expectedStatus, actual.HTTPStatus)
+			}
+		})
+	}
+}
+
+func postRequest(t testing.TB, hostname string, headers map[string]string) *http.Request {
+	t.Helper()
+	urlParts := strings.SplitN(hostname, "/", 2)
+
+	path := "/"
+	if len(urlParts) == 2 {
+		path = urlParts[1]
+	}
+	r, err := http.NewRequest(http.MethodPost, path, nil)
+	require.NoError(t, err)
+
+	r.Host = urlParts[0]
+
+	r.AddCookie(&http.Cookie{
+		Name:  "LoginCookie",
+		Value: "this should not be important",
+	})
+
+	for k, v := range headers {
+		r.Header.Set(k, v)
+	}
+	return r
+}
+
 func csrfScenario(t *testing.T, cookieName, method, origin, host string) *httptest.ResponseRecorder {
 	req, err := http.NewRequest(method, "/", nil)
 	if err != nil {
@@ -118,7 +234,10 @@ func csrfScenario(t *testing.T, cookieName, method, origin, host string) *httpte
 	})

 	rr := httptest.NewRecorder()
-	handler := CSRF(cookieName, log.New())(testHandler)
+	cfg := setting.NewCfg()
+	cfg.LoginCookieName = cookieName
+	service := csrf.ProvideCSRFFilter(cfg)
+	handler := service.Middleware()(testHandler)
 	handler.ServeHTTP(rr, req)
 	return rr
 }
@@ -28,6 +28,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/usagestats/statscollector"
 	loginpkg "github.com/grafana/grafana/pkg/login"
 	"github.com/grafana/grafana/pkg/login/social"
+	"github.com/grafana/grafana/pkg/middleware/csrf"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin"
@@ -260,6 +261,7 @@ var wireBasicSet = wire.NewSet(
 	ossaccesscontrol.ProvideDashboardPermissions,
 	wire.Bind(new(accesscontrol.DashboardPermissionsService), new(*ossaccesscontrol.DashboardPermissionsService)),
 	starimpl.ProvideService,
+	csrf.ProvideCSRFFilter,
 )

 var wireSet = wire.NewSet(