* apply security fix for admin only folder migration (#482)
* Data source: prevent from using auth proxy header as custom data source header (#474)
* applying changes from 446/fix-user-escalation-through-auth-proxy
* linting
* only validate custom headers if auth proxy is enabled
* import ordering
* add links to CVE
* clean up
* remove typo
Co-authored-by: Karl Persson <kalle.persson@grafana.com>
(cherry picked from commit 0100a6aa9645313b25a79a17d947cbf936cc4e76)
(cherry picked from commit c048378ad5)
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)...,
|
||||
|
||||
@@ -97,6 +97,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
|
||||
|
||||
ualert.UpdateRuleGroupIndexMigration(mg)
|
||||
accesscontrol.AddManagedFolderAlertActionsRepeatMigration(mg)
|
||||
accesscontrol.AddAdminOnlyMigration(mg)
|
||||
}
|
||||
|
||||
func addMigrationLogMigrations(mg *Migrator) {
|
||||
|
||||
Reference in New Issue
Block a user