Authorization bypass / Impersonation

PoC (exploit path before the fix): - Prerequisites: Grafana provisioning with Git-backed provisioning enabled, running on a vulnerable version prior to 12.4.0 where the patch is not yet applied. - Attacker session: Obtain a valid session/token for a legitimate user (the target victim is not required to be authenticated by the attacker). - Craft a provisioning Job manifest that includes author annotations set to a different user than the requester, e.g.: { "metadata": {"name": "poctest"}, "annotations": { "provisioning.grafana.app/author": "Victim User", "provisioning.grafana.app/authorEmail": "victim@example.com", "provisioning.grafana.app/authorID": "user:victim" }, "spec": { /* provisioning job spec */ } } - Submit the manifest to the Grafana provisioning API using the attacker’s token (e.g., via Kubernetes/REST API – exact endpoint depends on deployment). - Expected (pre-fix) behavior: The server would accept the annotations and create the job, associating the commit and related actions to the Victim User as indicated in the annotations, thereby enabling an authorization bypass/impersonation via forged author fields. - Post-fix expectation: The server validates the annotations against the actual requester and would reject the create request with an error like: "annotation provisioning.grafana.app/author must match the requesting user" (or similar for the email/ID fields). No impersonation occurs and the commit signature reflects the acting user. - Verify via Git repo: In a vulnerable setup, inspecting the generated commit would show the Victim as the author. In a patched setup, the commit author would reflect the actual requester or the request would be rejected. Concrete commands (illustrative; adapt endpoints to your Grafana deployment): - Acquire token for attacker (or victim) and prepare JSON payload as above. - curl -X POST \ -H "Authorization: Bearer <token>" \ -H "Content-Type: application/json" \ --data @poctest.json \ https://grafana.example.com/apis/provisioning/v0alpha1/namespaces/default/jobs - Check response: in a vulnerable version, 201 Created with commit authored as Victim. In the fixed version, you should receive 400 Bad Request with validation error mentioning the mismatched annotation. - Inspect commit in Git repo to confirm author on commit (pre-fix): git log -1 --pretty=format:"%h %an <%ae>" - After fix, this command should not return a commit with the forged author or the request should be rejected altogether.

diff --git a/apps/provisioning/pkg/jobs/validator.go b/apps/provisioning/pkg/jobs/validator.go index 7e81c7ab5518e..fa2f952094de7 100644 --- a/apps/provisioning/pkg/jobs/validator.go +++ b/apps/provisioning/pkg/jobs/validator.go @@ -13,9 +13,19 @@ import ( "github.com/grafana/grafana/apps/provisioning/pkg/repository/git" "github.com/grafana/grafana/apps/provisioning/pkg/resources" "github.com/grafana/grafana/apps/provisioning/pkg/safepath" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" ) +// AnnoAuthor, AnnoAuthorEmail and AnnoAuthorID carry the display name, email +// and stable identity of the user that triggered the job. They are set by the +// server at creation time and are immutable. +const ( + AnnoAuthor = "provisioning.grafana.app/author" + AnnoAuthorEmail = "provisioning.grafana.app/authorEmail" + AnnoAuthorID = "provisioning.grafana.app/authorID" +) + // ValidateJob performs validation on the Job specification and returns an error if validation fails. // supportedResources is the configured set of resource types provisioning can manage; export-style // job options (push and migrate) are validated against it. @@ -316,9 +326,56 @@ func (v *AdmissionValidator) Validate(ctx context.Context, a admission.Attribute return fmt.Errorf("expected job, got %T", obj) } + if err := validateAuthor(ctx, a, job); err != nil { + return err + } + return ValidateJob(job, v.supportedResources) } +func validateAuthor(ctx context.Context, a admission.Attributes, job *provisioning.Job) error { + name := job.Annotations[AnnoAuthor] + email := job.Annotations[AnnoAuthorEmail] + authorID := job.Annotations[AnnoAuthorID] + + switch a.GetOperation() { + case admission.Create: + if (name == "" && email == "" && authorID == "") || identity.IsServiceIdentity(ctx) { + return nil + } + id, err := identity.GetRequester(ctx) + if err != nil { + return apierrors.NewBadRequest("job author annotations must match the requesting user") + } + if name != "" && name != id.GetName() { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s must match the requesting user", AnnoAuthor)) + } + if email != "" && email != id.GetEmail() { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s must match the requesting user", AnnoAuthorEmail)) + } + if authorID != "" && authorID != id.GetUID() { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s must match the requesting user", AnnoAuthorID)) + } + case admission.Update: + old, ok := a.GetOldObject().(*provisioning.Job) + if !ok { + return nil + } + if old.Annotations[AnnoAuthor] != name { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s is immutable", AnnoAuthor)) + } + if old.Annotations[AnnoAuthorEmail] != email { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s is immutable", AnnoAuthorEmail)) + } + if old.Annotations[AnnoAuthorID] != authorID { + return apierrors.NewBadRequest(fmt.Sprintf("annotation %s is immutable", AnnoAuthorID)) + } + case admission.Delete, admission.Connect: + } + + return nil +} + // HistoricJobAdmissionValidator handles validation for HistoricJob resources during admission. // HistoricJobs are read-only records of completed jobs, so validation is minimal. type HistoricJobAdmissionValidator struct{} diff --git a/apps/provisioning/pkg/jobs/validator_test.go b/apps/provisioning/pkg/jobs/validator_test.go index c61d5e4fa3d2d..b779a34244a20 100644 --- a/apps/provisioning/pkg/jobs/validator_test.go +++ b/apps/provisioning/pkg/jobs/validator_test.go @@ -12,7 +12,10 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" + authlib "github.com/grafana/authlib/types" + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/identity" ) func TestValidateJob(t *testing.T) { @@ -1201,3 +1204,144 @@ func newHistoricJobAdmissionTestAttributes(obj runtime.Object, op admission.Oper &user.DefaultInfo{}, ) } + +func TestValidateAuthor(t *testing.T) { + requester := &identity.StaticRequester{ + Type: authlib.TypeUser, + Name: "Test User", + Email: "test@example.com", + UserUID: "abc123", + } + userCtx := identity.WithRequester(t.Context(), requester) + serviceCtx, _, err := identity.WithProvisioningIdentity(t.Context(), "default") + require.NoError(t, err) + + annotations := map[string]string{ + AnnoAuthor: requester.GetName(), + AnnoAuthorEmail: requester.GetEmail(), + AnnoAuthorID: requester.GetUID(), + } + + tests := []struct { + name string + ctx context.Context + operation admission.Operation + annotations map[string]string + oldAnnotations map[string]string + wantErrContains string + }{ + { + name: "create without annotations", + ctx: userCtx, + operation: admission.Create, + annotations: nil, + }, + { + name: "create by service identity", + ctx: serviceCtx, + operation: admission.Create, + annotations: map[string]string{AnnoAuthor: "someone else"}, + }, + { + name: "create with matching annotations", + ctx: userCtx, + operation: admission.Create, + annotations: annotations, + }, + { + name: "create with mismatched name", + ctx: userCtx, + operation: admission.Create, + annotations: map[string]string{AnnoAuthor: "someone else"}, + wantErrContains: AnnoAuthor + " must match", + }, + { + name: "create with mismatched email", + ctx: userCtx, + operation: admission.Create, + annotations: map[string]string{AnnoAuthorEmail: "other@example.com"}, + wantErrContains: AnnoAuthorEmail + " must match", + }, + { + name: "create with mismatched author ID", + ctx: userCtx, + operation: admission.Create, + annotations: map[string]string{AnnoAuthorID: "user:other"}, + wantErrContains: AnnoAuthorID + " must match", + }, + { + name: "create without requester", + ctx: t.Context(), + operation: admission.Create, + annotations: map[string]string{AnnoAuthor: "Test User"}, + wantErrContains: "job author annotations must match the requesting user", + }, + { + name: "update with unchanged annotations", + ctx: userCtx, + operation: admission.Update, + annotations: annotations, + oldAnnotations: annotations, + }, + { + name: "update changing name", + ctx: userCtx, + operation: admission.Update, + annotations: map[string]string{AnnoAuthor: "someone else"}, + oldAnnotations: annotations, + wantErrContains: AnnoAuthor + " is immutable", + }, + { + name: "update changing email", + ctx: userCtx, + operation: admission.Update, + annotations: map[string]string{AnnoAuthor: requester.GetName(), AnnoAuthorEmail: "other@example.com"}, + oldAnnotations: annotations, + wantErrContains: AnnoAuthorEmail + " is immutable", + }, + { + name: "update removing author ID", + ctx: userCtx, + operation: admission.Update, + annotations: map[string]string{AnnoAuthor: requester.GetName(), AnnoAuthorEmail: requester.GetEmail()}, + oldAnnotations: annotations, + wantErrContains: AnnoAuthorID + " is immutable", + }, + { + name: "delete is ignored", + ctx: userCtx, + operation: admission.Delete, + annotations: map[string]string{AnnoAuthor: "someone else"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + job := &provisioning.Job{ObjectMeta: metav1.ObjectMeta{Name: "test-job", Annotations: tt.annotations}} + var oldObj runtime.Object + if tt.oldAnnotations != nil { + oldObj = &provisioning.Job{ObjectMeta: metav1.ObjectMeta{Name: "test-job", Annotations: tt.oldAnnotations}} + } + attr := admission.NewAttributesRecord( + job, + oldObj, + provisioning.JobResourceInfo.GroupVersionKind(), + "default", + "test-job", + provisioning.JobResourceInfo.GroupVersionResource(), + "", + tt.operation, + nil, + false, + &user.DefaultInfo{}, + ) + + err := validateAuthor(tt.ctx, attr, job) + if tt.wantErrContains != "" { + require.ErrorContains(t, err, tt.wantErrContains) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index e2e9720ee8bc4..3586bd3c39c52 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/apps/provisioning/pkg/safepath" "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) @@ -88,6 +89,9 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. // handleRequest processes the HTTP request for files operations. func (c *filesConnector) handleRequest(ctx context.Context, name string, r *http.Request, responder rest.Responder, logger logging.Logger) { + if author, ok := jobs.UserAttribution(ctx); ok { + ctx = repository.WithAuthorSignature(ctx, repository.CommitSignature{Name: author.Name, Email: author.Email}) + } repo, err := c.getRepo(ctx, r.Method, name) if err != nil { logger.Debug("failed to find repository", "error", err) diff --git a/pkg/registry/apis/provisioning/jobs/author.go b/pkg/registry/apis/provisioning/jobs/author.go new file mode 100644 index 0000000000000..f61a6bbd27d97 --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/author.go @@ -0,0 +1,31 @@ +package jobs + +import ( + "context" + + authlib "github.com/grafana/authlib/types" + "github.com/open-feature/go-sdk/openfeature" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/services/featuremgmt" +) + +// Author identifies the user behind a request. +type Author struct { + Name string + Email string + ID string +} + +// UserAttribution returns the author for the user in ctx, or false when user +// attribution is disabled or the request is not made by a user. +func UserAttribution(ctx context.Context) (Author, bool) { + if !openfeature.NewDefaultClient().Boolean(ctx, featuremgmt.FlagProvisioningUserAttribution, false, openfeature.TransactionContext(ctx)) { + return Author{}, false + } + id, err := identity.GetRequester(ctx) + if err != nil || !id.IsIdentityType(authlib.TypeUser) { + return Author{}, false + } + return Author{Name: id.GetName(), Email: id.GetEmail(), ID: id.GetUID()}, true +} diff --git a/pkg/registry/apis/provisioning/jobs/author_test.go b/pkg/registry/apis/provisioning/jobs/author_test.go new file mode 100644 index 0000000000000..8b92b1704053d --- /dev/null +++ b/pkg/registry/apis/provisioning/jobs/author_test.go @@ -0,0 +1,93 @@ +package jobs + +import ( + "testing" + + authlib "github.com/grafana/authlib/types" + "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/services/featuremgmt" +) + +func TestUserAttribution(t *testing.T) { + tests := []struct { + name string + requester identity.Requester + flag bool + expected *Author + }{ + { + name: "user identity returns signature", + requester: &identity.StaticRequester{ + Type: authlib.TypeUser, + Name: "Test User", + Email: "test@example.com", + UserUID: "abc123", + }, + flag: true, + expected: &Author{Name: "Test User", Email: "test@example.com", ID: "user:abc123"}, + }, + { + name: "flag disabled returns nothing", + requester: &identity.StaticRequester{ + Type: authlib.TypeUser, + Name: "Test User", + Email: "test@example.com", + }, + flag: false, + expected: nil, + }, + { + name: "service identity returns nothing", + requester: &identity.StaticRequester{ + Type: authlib.TypeAccessPolicy, + Name: "provisioning", + }, + flag: true, + expected: nil, + }, + { + name: "missing requester returns nothing", + requester: nil, + flag: true, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := t.Context() + if tt.requester != nil { + ctx = identity.WithRequester(ctx, tt.requester) + } + setUserAttributionFlag(t, tt.flag) + + author, ok := UserAttribution(ctx) + if tt.expected == nil { + assert.False(t, ok) + } else { + require.True(t, ok) + assert.Equal(t, *tt.expected, author) + } + }) + } +} + +func setUserAttributionFlag(t *testing.T, value bool) { + t.Helper() + provider, err := featuremgmt.CreateStaticProviderWithStandardFlags(map[string]memprovider.InMemoryFlag{ + featuremgmt.FlagProvisioningUserAttribution: { + Key: featuremgmt.FlagProvisioningUserAttribution, + Variants: map[string]any{"": value}, + }, + }) + require.NoError(t, err) + require.NoError(t, openfeature.SetProviderAndWait(provider)) + t.Cleanup(func() { + _ = openfeature.SetProviderAndWait(openfeature.NoopProvider{}) + }) +} diff --git a/pkg/registry/apis/provisioning/jobs/driver.go b/pkg/registry/apis/provisioning/jobs/driver.go index 286a5350d60e3..4da4c3ada999c 100644 --- a/pkg/registry/apis/provisioning/jobs/driver.go +++ b/pkg/registry/apis/provisioning/jobs/driver.go @@ -7,6 +7,7 @@ import ( "sync" "time" + "github.com/open-feature/go-sdk/openfeature" "go.opentelemetry.io/otel/attribute" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apiserver/pkg/endpoints/request" @@ -15,8 +16,11 @@ import ( "github.com/grafana/grafana/apps/provisioning/pkg/apis/apifmt" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller" + appjobs "github.com/grafana/grafana/apps/provisioning/pkg/jobs" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/featuremgmt" ) // Store is an abstraction for the storage API. @@ -389,6 +393,12 @@ func (d *jobDriver) processJob(ctx context.Context, recorder JobProgressRecorder attribute.String("job.action", string(job.Spec.Action)), ) + name, email := job.Annotations[appjobs.AnnoAuthor], job.Annotations[appjobs.AnnoAuthorEmail] + if (name != "" || email != "") && + openfeature ... [truncated]