Authorization bypass / Access control

Note: The exact Grafana API paths depend on the deployed version. The PoC below demonstrates the concept using the resource-permission endpoints that were amended by this patch. Adapt the endpoint paths to your deployment. Prerequisites: - A Grafana instance exposing OSS access control endpoints. - Two ServiceAccounts (SA1 and SA2) with distinct UIDs (sa-uid-1 and sa-uid-2), where the tester has permissions scoped to SA1 but not SA2. - An admin or appropriate RBAC role to attempt cross-resource permission changes for service accounts. Goal: - Demonstrate that, prior to this fix, an attacker could reference a ServiceAccount by its UID and influence authorization checks in a way that could bypass per-resource RBAC boundaries. The fix ensures UID is preserved in the K8s adapter while translating to internal IDs for authorization. PoC steps (conceptual, adapt to actual endpoints): 1) Obtain a valid auth token for a user with restricted permissions (only SA1 access). 2) Call the resource-permission API using SA2's UID in the path, attempting to view or modify permissions for SA2 while our user should be restricted to SA1. Example (pseudo-endpoint; replace with actual Grafana API paths in your env): curl -H "Authorization: Bearer $TOKEN" \ -X GET \ https://grafana.example/api/oss/accesscontrol/resourcepermissions/iam.grafana.app/serviceaccounts/sa-uid-2 Expected (pre-fix): Depending on translation handling, this could return data or perform an action if the internal ID routing bypasses per-UID scoping. Expected (post-fix): The request should be rejected if SA2 is not permitted for the caller, because the UID is translated to an internal ID for auth and the UID is preserved for Kubernetes adapter routing, preventing cross-resource permission leakage. 3) Optionally attempt to perform a permission update for SA2 using the same UID in the path and observe that unauthorized changes are denied after the fix. 4) Compare to a baseline where you reference SA2 by its numeric internal ID (if your environment exposes it) to verify that authorization decisions consistently rely on the translated internal ID with proper scoping, and that UID preservation prevents misrouting to the Kubernetes adapter. Note: This PoC is framework-level and depends on your Grafana deployment’s actual REST API paths for resource permissions. The important aspect is that, after the fix, the UID carried in the URL is preserved for the Kubernetes adapter while the internal authorization uses the translated numeric ID, reducing the risk of UID→ID translation bypass.

diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 67f31f06bb9fe..33edcb270d3b5 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -452,7 +452,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api return nil, err } retrieverService := retriever.ProvideService(sqlStore, apikeyService, kvStore, userimplService, orgService) - serviceAccountPermissionsService, err := ossaccesscontrol.ProvideServiceAccountPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, retrieverService, acimplService, teamimplService, userimplService, actionSetService) + serviceAccountPermissionsService, err := ossaccesscontrol.ProvideServiceAccountPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, retrieverService, acimplService, teamimplService, userimplService, actionSetService, eventualRestConfigProvider) if err != nil { return nil, err } @@ -1163,7 +1163,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac return nil, err } retrieverService := retriever.ProvideService(sqlStore, apikeyService, kvStore, userimplService, orgService) - serviceAccountPermissionsService, err := ossaccesscontrol.ProvideServiceAccountPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, retrieverService, acimplService, teamimplService, userimplService, actionSetService) + serviceAccountPermissionsService, err := ossaccesscontrol.ProvideServiceAccountPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, retrieverService, acimplService, teamimplService, userimplService, actionSetService, eventualRestConfigProvider) if err != nil { return nil, err } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/service_account.go b/pkg/services/accesscontrol/ossaccesscontrol/service_account.go index 1097f9b1794bb..a5652103893b2 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/service_account.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/service_account.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" + "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/serviceaccounts" @@ -39,10 +40,12 @@ func ProvideServiceAccountPermissions( cfg *setting.Cfg, features featuremgmt.FeatureToggles, router routing.RouteRegister, sql db.DB, ac accesscontrol.AccessControl, license licensing.Licensing, serviceAccountRetrieverService *retriever.Service, service accesscontrol.Service, teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, + restConfigProvider apiserver.DirectRestConfigProvider, ) (*ServiceAccountPermissionsService, error) { options := resourcepermissions.Options{ Resource: "serviceaccounts", ResourceAttribute: "id", + APIGroup: "iam.grafana.app", ResourceTranslator: serviceaccounts.UIDToIDHandler(serviceAccountRetrieverService), ResourceValidator: func(ctx context.Context, orgID int64, resourceID string) error { ctx, span := tracer.Start(ctx, "accesscontrol.ossaccesscontrol.ProvideServiceAccountPermissions.ResourceValidator") @@ -67,9 +70,10 @@ func ProvideServiceAccountPermissions( "Edit": ServiceAccountEditActions, "Admin": ServiceAccountAdminActions, }, - ReaderRoleName: "Permission reader", - WriterRoleName: "Permission writer", - RoleGroup: "Service accounts", + ReaderRoleName: "Permission reader", + WriterRoleName: "Permission writer", + RoleGroup: "Service accounts", + RestConfigProvider: restConfigProvider, } srv, err := resourcepermissions.New(cfg, options, features, router, license, ac, service, sql, teamService, userService, actionSetService) diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 640d3377836ba..58d411cad378c 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -87,10 +87,11 @@ func (a *api) registerEndpoints() { } gotParams := web.Params(c.Req) - resourceID := gotParams[":resourceID"] - resourceID, err := resTranslator(c.Req.Context(), c.OrgID, resourceID) + resourceUID := gotParams[":resourceID"] + translatedID, err := resTranslator(c.Req.Context(), c.OrgID, resourceUID) if err == nil { - gotParams[":resourceID"] = resourceID + gotParams[":resourceUID"] = resourceUID // preserve original UID for the K8s adapter + gotParams[":resourceID"] = translatedID // overwrite with numeric ID for auth middleware web.SetURLParams(c.Req, gotParams) } else { c.JsonApiErr(http.StatusNotFound, "Not found", nil) diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go index fa115e0b1aac0..108548fe9849a 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "slices" "strconv" "strings" @@ -29,6 +30,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/web" ) var ErrRestConfigNotAvailable = errors.New("k8s rest config provider not available") @@ -55,7 +57,7 @@ func (a *api) getResourcePermissionsFromK8s(c *contextmodel.ReqContext, namespac return nil, err } - resourcePermName := a.buildResourcePermissionName(resourceID) + resourcePermName := a.buildResourcePermissionName(c.Req, resourceID) resourcePermResource := dynamicClient.Resource(iamv0.ResourcePermissionInfo.GroupVersionResource()).Namespace(namespace) unstructuredObj, err := resourcePermResource.Get(ctx, resourcePermName, metav1.GetOptions{}) @@ -373,8 +375,18 @@ func (a *api) getProvisionedPermissions(ctx context.Context, namespace string, r return dto, nil } -func (a *api) buildResourcePermissionName(resourceID string) string { - return fmt.Sprintf("%s-%s-%s", a.getAPIGroup(), a.service.options.Resource, resourceID) +// resourceNameFromRequest returns the UID stored by the resource translator when present +// (":resourceUID"), falling back to the (possibly numeric) resourceID for resources +// that don't perform UID→ID translation. +func resourceNameFromRequest(r *http.Request, resourceID string) string { + if uid := web.Params(r)[":resourceUID"]; uid != "" { + return uid + } + return resourceID +} + +func (a *api) buildResourcePermissionName(r *http.Request, resourceID string) string { + return fmt.Sprintf("%s-%s-%s", a.getAPIGroup(), a.service.options.Resource, resourceNameFromRequest(r, resourceID)) } // Write operations @@ -386,7 +398,7 @@ func (a *api) setResourcePermissionsToK8s(c *contextmodel.ReqContext, namespace return err } - resourcePermName := a.buildResourcePermissionName(resourceID) + resourcePermName := a.buildResourcePermissionName(c.Req, resourceID) resourcePermResource := dynamicClient.Resource(iamv0.ResourcePermissionInfo.GroupVersionResource()).Namespace(namespace) _, existingResourceVersion, err := a.getExistingResourcePermission(ctx, resourcePermResource, resourcePermName) @@ -438,7 +450,7 @@ func (a *api) setResourcePermissionsToK8s(c *contextmodel.ReqContext, namespace Resource: iamv0.ResourcePermissionspecResource{ ApiGroup: a.getAPIGroup(), Resource: a.service.options.Resource, - Name: resourceID, + Name: resourceNameFromRequest(c.Req, resourceID), }, Permissions: k8sPermissions, }, @@ -478,7 +490,7 @@ func (a *api) setSinglePermissionToK8s(c *contextmodel.ReqContext, namespace str return err } - resourcePermName := a.buildResourcePermissionName(resourceID) + resourcePermName := a.buildResourcePermissionName(c.Req, resourceID) resourcePermResource := dynamicClient.Resource(iamv0.ResourcePermissionInfo.GroupVersionResource()).Namespace(namespace) existingResourcePerm, existingResourceVersion, err := a.getExistingResourcePermission(ctx, resourcePermResource, resourcePermName) @@ -526,7 +538,7 @@ func (a *api) setSinglePermissionToK8s(c *contextmodel.ReqContext, namespace str Resource: iamv0.ResourcePermissionspecResource{ ApiGroup: a.getAPIGroup(), Resource: a.service.options.Resource, - Name: resourceID, + Name: resourceNameFromRequest(c.Req, resourceID), }, Permissions: newPermissions, }, diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go index 766fff2ba9260..1cd66dabfa9df 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go @@ -111,6 +111,7 @@ func TestBuildResourcePermissionName(t *testing.T) { apiGroup string resource string resourceID string + resourceUID string // when set, overrides resourceID in the name expectedName string }{ { @@ -127,6 +128,14 @@ func TestBuildResourcePermissionName(t *testing.T) { resourceID: "folder-uid-456", expectedName: "folders.grafana.app-folders-folder-uid-456", }, + { + name: "resourceUID from request overrides numeric resourceID", + apiGroup: "iam.grafana.app", + resource: "serviceaccounts", + resourceID: "42", + resourceUID: "sa-uid-abc", + expectedName: "iam.grafana.app-serviceaccounts-sa-uid-abc", + }, } for _, tt := range tests { @@ -140,7 +149,11 @@ func TestBuildResourcePermissionName(t *testing.T) { }, } - name := api.buildResourcePermissionName(tt.resourceID) + req := httptest.NewRequest(http.MethodGet, "/", nil) + if tt.resourceUID != "" { + req = web.SetURLParams(req, map[string]string{":resourceUID": tt.resourceUID}) + } + name := api.buildResourcePermissionName(req, tt.resourceID) assert.Equal(t, tt.expectedName, name) }) }