diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 9b364799678..8f181f32dcf 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -8,17 +8,20 @@ import ( "net/http" "sort" "strconv" + "strings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/api/datasource" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins/adapters" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/datasources/permissions" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util/proxyutil" "github.com/grafana/grafana/pkg/web" @@ -325,6 +328,26 @@ func validateURL(cmdType string, url string) response.Response { return nil } +// validateJSONData prevents the user from adding a custom header with name that matches the auth proxy header name. +// This is done to prevent data source proxy from being used to circumvent auth proxy. +// For more context take a look at CVE-2022-35957 +func validateJSONData(jsonData *simplejson.Json, cfg *setting.Cfg) error { + if jsonData == nil || !cfg.AuthProxyEnabled { + return nil + } + + for key, value := range jsonData.MustMap() { + if strings.HasPrefix(key, "httpHeaderName") { + header := fmt.Sprint(value) + if http.CanonicalHeaderKey(header) == http.CanonicalHeaderKey(cfg.AuthProxyHeaderName) { + datasourcesLogger.Error("Forbidden to add a data source header with a name equal to auth proxy header name", "headerName", key) + return errors.New("validation error, invalid header name specified") + } + } + } + return nil +} + // swagger:route POST /datasources datasources addDataSource // // Create a data source. @@ -356,6 +379,9 @@ func (hs *HTTPServer) AddDataSource(c *models.ReqContext) response.Response { return resp } } + if err := validateJSONData(cmd.JsonData, hs.Cfg); err != nil { + return response.Error(http.StatusBadRequest, "Failed to add datasource", err) + } if err := hs.DataSourcesService.AddDataSource(c.Req.Context(), &cmd); err != nil { if errors.Is(err, datasources.ErrDataSourceNameExists) || errors.Is(err, datasources.ErrDataSourceUidExists) { @@ -413,6 +439,9 @@ func (hs *HTTPServer) UpdateDataSourceByID(c *models.ReqContext) response.Respon if resp := validateURL(cmd.Type, cmd.Url); resp != nil { return resp } + if err := validateJSONData(cmd.JsonData, hs.Cfg); err != nil { + return response.Error(http.StatusBadRequest, "Failed to update datasource", err) + } ds, err := hs.getRawDataSourceById(c.Req.Context(), cmd.Id, cmd.OrgId) if err != nil { @@ -450,6 +479,9 @@ func (hs *HTTPServer) UpdateDataSourceByUID(c *models.ReqContext) response.Respo if resp := validateURL(cmd.Type, cmd.Url); resp != nil { return resp } + if err := validateJSONData(cmd.JsonData, hs.Cfg); err != nil { + return response.Error(http.StatusBadRequest, "Failed to update datasource", err) + } ds, err := hs.getRawDataSourceByUID(c.Req.Context(), web.Params(c.Req)[":uid"], c.OrgId) if err != nil { diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index bcbc9e33809..b3503bd056f 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/datasources" @@ -82,6 +83,7 @@ func TestAddDataSource_InvalidURL(t *testing.T) { sc := setupScenarioContext(t, "/api/datasources") hs := &HTTPServer{ DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), } sc.m.Post(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { @@ -108,6 +110,7 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &datasources.DataSource{}, }, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources") @@ -127,10 +130,42 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { assert.Equal(t, 200, sc.resp.Code) } +// Using a custom header whose name matches the name specified for auth proxy header should fail +func TestAddDataSource_InvalidJSONData(t *testing.T) { + hs := &HTTPServer{ + DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), + } + + sc := setupScenarioContext(t, "/api/datasources") + + hs.Cfg = setting.NewCfg() + hs.Cfg.AuthProxyEnabled = true + hs.Cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", hs.Cfg.AuthProxyHeaderName) + + sc.m.Post(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { + c.Req.Body = mockRequestBody(datasources.AddDataSourceCommand{ + Name: "Test", + Url: "localhost:5432", + Access: "direct", + Type: "test", + JsonData: jsonData, + }) + return hs.AddDataSource(c) + })) + + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + + assert.Equal(t, 400, sc.resp.Code) +} + // Updating data sources with invalid URLs should lead to an error. func TestUpdateDataSource_InvalidURL(t *testing.T) { hs := &HTTPServer{ DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources/1234") @@ -149,6 +184,35 @@ func TestUpdateDataSource_InvalidURL(t *testing.T) { assert.Equal(t, 400, sc.resp.Code) } +// Using a custom header whose name matches the name specified for auth proxy header should fail +func TestUpdateDataSource_InvalidJSONData(t *testing.T) { + hs := &HTTPServer{ + DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), + } + sc := setupScenarioContext(t, "/api/datasources/1234") + + hs.Cfg.AuthProxyEnabled = true + hs.Cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", hs.Cfg.AuthProxyHeaderName) + + sc.m.Put(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { + c.Req.Body = mockRequestBody(datasources.AddDataSourceCommand{ + Name: "Test", + Url: "localhost:5432", + Access: "direct", + Type: "test", + JsonData: jsonData, + }) + return hs.AddDataSource(c) + })) + + sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() + + assert.Equal(t, 400, sc.resp.Code) +} + // Updating data sources with URLs not specifying protocol should work. func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { const name = "Test" @@ -158,6 +222,7 @@ func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &datasources.DataSource{}, }, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources/1234") diff --git a/pkg/services/datasources/service/datasource_service.go b/pkg/services/datasources/service/datasource_service.go index 35ee939321a..bb464179c6a 100644 --- a/pkg/services/datasources/service/datasource_service.go +++ b/pkg/services/datasources/service/datasource_service.go @@ -525,8 +525,9 @@ func (s *Service) getCustomHeaders(jsonData *simplejson.Json, decryptedValues ma return headers } - index := 1 + index := 0 for { + index++ headerNameSuffix := fmt.Sprintf("httpHeaderName%d", index) headerValueSuffix := fmt.Sprintf("httpHeaderValue%d", index) @@ -536,10 +537,16 @@ func (s *Service) getCustomHeaders(jsonData *simplejson.Json, decryptedValues ma break } + // skip a header with name that corresponds to auth proxy header's name + // to make sure that data source proxy isn't used to circumvent auth proxy. + // For more context take a look at CVE-2022-35957 + if s.cfg.AuthProxyEnabled && http.CanonicalHeaderKey(key) == http.CanonicalHeaderKey(s.cfg.AuthProxyHeaderName) { + continue + } + if val, ok := decryptedValues[headerValueSuffix]; ok { headers[key] = val } - index++ } return headers diff --git a/pkg/services/datasources/service/datasource_service_test.go b/pkg/services/datasources/service/datasource_service_test.go index 7685e5df0f2..70b2fce9a7e 100644 --- a/pkg/services/datasources/service/datasource_service_test.go +++ b/pkg/services/datasources/service/datasource_service_test.go @@ -511,6 +511,84 @@ func TestService_GetHttpTransport(t *testing.T) { require.NotNil(t, configuredOpts.SigV4) require.Equal(t, "es", configuredOpts.SigV4.Service) }) + + t.Run("Should not add a header with key that corresponds to auth proxies key name", func(t *testing.T) { + provider := httpclient.NewProvider() + + cfg.AuthProxyEnabled = true + cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + + allowedHeaderName := "X-ALLOWED-HEADER" + allowedHeaderValue := "X-ALLOWED-HEADER_value" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", allowedHeaderName) + jsonData.Set("httpHeaderName2", cfg.AuthProxyHeaderName) + + ds := datasources.DataSource{ + Id: 1, + Url: "http://k8s:8001", + Type: "Kubernetes", + JsonData: jsonData, + } + + secretsStore := kvstore.SetupTestService(t) + secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) + + secureJsonData, err := json.Marshal(map[string]string{ + "httpHeaderValue1": allowedHeaderValue, + "httpHeaderValue2": "admin", + }) + require.NoError(t, err) + + err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(secureJsonData)) + require.NoError(t, err) + + headers := dsService.getCustomHeaders(jsonData, map[string]string{"httpHeaderValue2": "admin", "httpHeaderValue1": "X-ALLOWED-HEADER_value"}) + require.Equal(t, "", headers[cfg.AuthProxyHeaderName], "header with auth proxy header name should have been stripped") + require.Equal(t, allowedHeaderValue, headers[allowedHeaderName], "other headers should stay intact") + + // 1. Start HTTP test server which checks the request headers + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(cfg.AuthProxyHeaderName) != "" { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte("Did not expect the header to be set")) + require.NoError(t, err) + return + } + + if r.Header.Get(allowedHeaderName) != allowedHeaderValue { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte("Expected the allowed header to be set")) + require.NoError(t, err) + return + } + + w.WriteHeader(200) + _, err := w.Write([]byte("Ok")) + require.NoError(t, err) + })) + defer backend.Close() + + // 2. Get HTTP transport from datasource which uses the test server as backend + ds.Url = backend.URL + rt, err := dsService.GetHTTPTransport(context.Background(), &ds, provider) + require.NoError(t, err) + require.NotNil(t, rt) + + // 3. Send test request which should not have a header with name that matches auth proxy header + req := httptest.NewRequest("GET", backend.URL+"/test-headers", nil) + res, err := rt.RoundTrip(req) + require.NoError(t, err) + t.Cleanup(func() { + err := res.Body.Close() + require.NoError(t, err) + }) + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + bodyStr := string(body) + require.Equal(t, "Ok", bodyStr) + }) } func TestService_getTimeout(t *testing.T) { diff --git a/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go b/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go new file mode 100644 index 00000000000..41a30a529f3 --- /dev/null +++ b/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go @@ -0,0 +1,100 @@ +package accesscontrol + +import ( + "strings" + + "xorm.io/xorm" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +) + +func AddAdminOnlyMigration(mg *migrator.Migrator) { + mg.AddMigration("admin only folder/dashboard permission", &adminOnlyMigrator{}) +} + +type adminOnlyMigrator struct { + migrator.MigrationBase +} + +func (m *adminOnlyMigrator) SQL(dialect migrator.Dialect) string { + return CodeMigrationSQL +} + +func (m *adminOnlyMigrator) Exec(sess *xorm.Session, mg *migrator.Migrator) error { + logger := log.New("admin-permissions-only-migrator") + type model struct { + UID string `xorm:"uid"` + OrgID int64 `xorm:"org_id"` + IsFolder bool `xorm:"is_folder"` + } + var models []model + + // Find all dashboards and folders that should have only admin permission in acl + // When a dashboard or folder only has admin permission the acl table should be empty and the has_acl set to true + sql := ` + SELECT res.uid, res.is_folder, res.org_id + FROM (SELECT dashboard.id, dashboard.uid, dashboard.is_folder, dashboard.org_id, count(dashboard_acl.id) as count + FROM dashboard + LEFT JOIN dashboard_acl ON dashboard.id = dashboard_acl.dashboard_id + WHERE dashboard.has_acl IS TRUE + GROUP BY dashboard.id) as res + WHERE res.count = 0 + ` + + if err := sess.SQL(sql).Find(&models); err != nil { + return err + } + + for _, model := range models { + var scope string + + // set scope based on type + if model.IsFolder { + scope = "folders:uid:" + model.UID + } else { + scope = "dashboards:uid:" + model.UID + } + + // Find all managed editor and viewer permissions with scopes to folder or dashboard + sql = ` + SELECT r.id + FROM role r + LEFT JOIN permission p on r.id = p.role_id + WHERE p.scope = ? + AND r.org_id = ? + AND r.name IN ('managed:builtins:editor:permissions', 'managed:builtins:viewer:permissions') + GROUP BY r.id + ` + + var roleIDS []int64 + if err := sess.SQL(sql, scope, model.OrgID).Find(&roleIDS); err != nil { + return err + } + + if len(roleIDS) == 0 { + continue + } + + msg := "removing viewer and editor permissions on " + if model.IsFolder { + msg += "folder" + } else { + msg += "dashboard" + } + + logger.Info(msg, "uid", model.UID) + + // Remove managed permission for editors and viewers if there was any + removeSQL := `DELETE FROM permission WHERE scope = ? AND role_id IN(?` + strings.Repeat(", ?", len(roleIDS)-1) + `) ` + params := []interface{}{removeSQL, scope} + for _, id := range roleIDS { + params = append(params, id) + } + if _, err := sess.Exec(params...); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go b/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go index d734e3cd295..5b8f18eeb57 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go @@ -69,6 +69,7 @@ type dashboard struct { FolderID int64 `xorm:"folder_id"` OrgID int64 `xorm:"org_id"` IsFolder bool + HasAcl bool `xorm:"has_acl"` } func (m dashboardPermissionsMigrator) Exec(sess *xorm.Session, migrator *migrator.Migrator) error { @@ -76,7 +77,7 @@ func (m dashboardPermissionsMigrator) Exec(sess *xorm.Session, migrator *migrato m.dialect = migrator.Dialect var dashboards []dashboard - if err := m.sess.SQL("SELECT id, is_folder, folder_id, org_id FROM dashboard").Find(&dashboards); err != nil { + if err := m.sess.SQL("SELECT id, is_folder, folder_id, org_id, has_acl FROM dashboard").Find(&dashboards); err != nil { return fmt.Errorf("failed to list dashboards: %w", err) } @@ -108,7 +109,7 @@ func (m dashboardPermissionsMigrator) migratePermissions(dashboards []dashboard, permissionMap[d.OrgID] = map[string][]*ac.Permission{} } - if (d.IsFolder || d.FolderID == 0) && len(acls) == 0 { + if (d.IsFolder || d.FolderID == 0) && len(acls) == 0 && !d.HasAcl { permissionMap[d.OrgID]["managed:builtins:editor:permissions"] = append( permissionMap[d.OrgID]["managed:builtins:editor:permissions"], m.mapPermission(d.ID, models.PERMISSION_EDIT, d.IsFolder)..., diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index ba5126e4d1b..55d68687bdf 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -97,6 +97,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) { ualert.UpdateRuleGroupIndexMigration(mg) accesscontrol.AddManagedFolderAlertActionsRepeatMigration(mg) + accesscontrol.AddAdminOnlyMigration(mg) } func addMigrationLogMigrations(mg *Migrator) {