Authorization bypass

HIGH

grafana/grafana

Commit: 0dacf3519331

Affected: Versions prior to 12.4.0 (pre-12.4.0)

2026-05-04 17:10 UTC

Description

The commit implements a security fix for an authorization bypass related to mutating OwnerReferences on SecureValue resources. Prior to this change, non-AccessPolicy identities could mutate OwnerReferences during an Update, potentially allowing an attacker to reassign ownership metadata and bypass RBAC/ownership-based restrictions. The patch preserves existing OwnerReferences for non-AccessPolicy identities and permits mutation of OwnerReferences only for AccessPolicy identities, mitigating unauthorized ownership tampering. This addresses a class of authorization bypass risks where ownership metadata could be manipulated to gain or escalate access.

Proof of Concept

Proof-of-concept (conceptual):

Prerequisites:
- Grafana deployment with SecureValue API enabled, version prior to 12.4.0.
- An attacker authenticated as a non-AccessPolicy identity in the same namespace as a SecureValue resource.
- A SecureValue resource (sv1) exists in namespace ns1, initially with no OwnerReferences (or with some OwnerReferences that the attacker should not be able to fully own).

Steps:
1) Attacker attempts to Update the SecureValue sv1 by sending a payload that sets OwnerReferences to a resource owned by the attacker, e.g., a DataSource owned by attacker:
{
  "apiVersion": "v1",
  "kind": "SecureValue",
  "metadata": {"name": "sv1"},
  "spec": {
    "description": "updated",
    "ownerReferences": [
      {
        "apiVersion": "v0alpha1",
        "kind": "DataSource",
        "name": "attacker-ds",
        "namespace": "ns1"
      }
    ]
  }
}

2) Send Update request to Grafana’s SecureValue API using the attacker’s non-AccessPolicy identity (e.g., via REST call with the attacker’s token).

What would happen (pre-fix vs post-fix):
- Before this patch (v=pre-12.4.0): The service would apply newSecureValue.OwnerReferences from the request, effectively mutating ownership metadata and potentially granting the attacker ownership rights or bypassing existing restrictions.
- With the patch (12.4.0 and later): The service detects a non-AccessPolicy identity and preserves the current OwnerReferences from sv1, ignoring the attacker-provided OwnerReferences in the update while still applying other allowed updates (e.g., description).

Verification:
- After the update with non-AccessPolicy identity, sv1.OwnerReferences should remain unchanged (as originally set).
- With an AccessPolicy identity, updates to OwnerReferences should be allowed (i.e., the attacker could mutate OwnerReferences when using the AccessPolicy identity).

Note: This PoC assumes the system exposes an API for updating SecureValue resources and that ownership semantics rely on OwnerReferences for authorization decisions. The practical impact is that non-AccessPolicy users cannot tamper with ownership metadata via updates, preventing a potential authorization bypass.

Commit Details

Author: Matheus Macabu

Date: 2026-05-04 16:12 UTC

Message:

Secrets: Preserve owner reference in updates for non-access policies (#124029)

Triage Assessment

Vulnerability Type: Authorization bypass

Confidence: HIGH

Reasoning:

The patch changes how owner references are updated on Secret values by restricting mutations to only 'AccessPolicy' identities. For non-AccessPolicy identities, it preserves existing OwnerReferences, preventing unauthorized tampering with ownership metadata which could affect RBAC and resource authorization. This addresses authorization bypass risks where a user could modify ownership to gain or escalate access.

Verification Assessment

Vulnerability Type: Authorization bypass

Confidence: HIGH

Affected Versions: Versions prior to 12.4.0 (pre-12.4.0)

Code Diff

diff --git a/pkg/registry/apis/secret/service/secure_value.go b/pkg/registry/apis/secret/service/secure_value.go
index a348ee9b2b288..a9a103d481f47 100644
--- a/pkg/registry/apis/secret/service/secure_value.go
+++ b/pkg/registry/apis/secret/service/secure_value.go
@@ -146,6 +146,10 @@ func (s *SecureValueService) Update(ctx context.Context, newSecureValue *secretv
 		return nil, false, fmt.Errorf("reading secure value secret: %+w", err)
 	}
 
+	// only service identities are allowed to mutate owner references after a secure value is created.
+	// for any other identity, preserve the existing ones to prevent unauthorized changes.
+	s.preserveOwnerReferencesForNonAccessPolicy(ctx, currentVersion, newSecureValue)
+
 	keeperCfg, err := s.keeperMetadataStorage.GetKeeperConfig(ctx, currentVersion.Namespace, currentVersion.Status.Keeper, contracts.ReadOpts{})
 	if err != nil {
 		return nil, false, fmt.Errorf("fetching keeper config: namespace=%+v keeper: %q %w", newSecureValue.Namespace, currentVersion.Status.Keeper, err)
@@ -443,3 +447,12 @@ func (s *SecureValueService) SetKeeperAsActive(ctx context.Context, namespace xk
 	}
 	return nil
 }
+
+func (s *SecureValueService) preserveOwnerReferencesForNonAccessPolicy(ctx context.Context, currentSecureValue, newSecureValue *secretv1beta1.SecureValue) {
+	authInfo, ok := claims.AuthInfoFrom(ctx)
+	if ok && authInfo.GetIdentityType() == claims.TypeAccessPolicy {
+		return
+	}
+
+	newSecureValue.OwnerReferences = currentSecureValue.OwnerReferences
+}

diff --git a/pkg/registry/apis/secret/service/secure_value_test.go b/pkg/registry/apis/secret/service/secure_value_test.go
index 33c30a77b3ab9..ec3fc27ba0ecf 100644
--- a/pkg/registry/apis/secret/service/secure_value_test.go
+++ b/pkg/registry/apis/secret/service/secure_value_test.go
@@ -282,6 +282,210 @@ func TestCrud(t *testing.T) {
 	})
 }
 
+func TestUpdateOwnerReferences(t *testing.T) {
+	t.Parallel()
+
+	namespace := "ns1"
+	owner := common.ObjectReference{
+		APIGroup:   "prometheus.datasource.grafana.app",
+		APIVersion: "v0alpha1",
+		Kind:       "DataSource",
+		Name:       "test-ds",
+		Namespace:  namespace,
+	}
+
+	t.Run("update without owner references on either side does not require AccessPolicy identity", func(t *testing.T) {
+		t.Parallel()
+		sut := testutils.Setup(t)
+
+		sv1, err := sut.CreateSv(t.Context())
+		require.NoError(t, err)
+		require.Empty(t, sv1.OwnerReferences)
+
+		input := sv1.DeepCopy()
+		input.Spec.Description = "updated"
+
+		sv2, err := sut.UpdateSv(t.Context(), input)
+		require.NoError(t, err)
+		require.Equal(t, "updated", sv2.Spec.Description)
+	})
+
+	t.Run("update keeping the same owner reference does not require AccessPolicy identity", func(t *testing.T) {
+		t.Parallel()
+		sut := testutils.Setup(t)
+
+		sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+			cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+		})
+		require.NoError(t, err)
+		require.Len(t, sv1.OwnerReferences, 1)
+
+		input := sv1.DeepCopy()
+		input.Spec.Description = "updated"
+
+		sv2, err := sut.UpdateSv(t.Context(), input)
+		require.NoError(t, err)
+		require.Equal(t, "updated", sv2.Spec.Description)
+		require.Len(t, sv2.OwnerReferences, 1)
+	})
+
+	t.Run("adding an owner reference", func(t *testing.T) {
+		t.Parallel()
+
+		t.Run("applied for AccessPolicy identity", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context())
+			require.NoError(t, err)
+			require.Empty(t, sv1.OwnerReferences)
+
+			input := sv1.DeepCopy()
+			input.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+
+			ctx := testutils.CreateServiceAuthContext(t.Context(), "service-identity", namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Len(t, sv2.OwnerReferences, 1)
+			require.Equal(t, owner.Name, sv2.OwnerReferences[0].Name)
+		})
+
+		t.Run("ignored for any other identity, owner references stay empty", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context())
+			require.NoError(t, err)
+			require.Empty(t, sv1.OwnerReferences)
+
+			input := sv1.DeepCopy()
+			input.Spec.Description = "updated"
+			input.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+
+			ctx := testutils.CreateUserAuthContext(t.Context(), namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Empty(t, sv2.OwnerReferences)
+			// Other fields are still updated.
+			require.Equal(t, "updated", sv2.Spec.Description)
+		})
+	})
+
+	t.Run("removing an owner reference", func(t *testing.T) {
+		t.Parallel()
+
+		t.Run("applied for AccessPolicy identity", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+				cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+			})
+			require.NoError(t, err)
+			require.Len(t, sv1.OwnerReferences, 1)
+
+			input := sv1.DeepCopy()
+			input.OwnerReferences = nil
+
+			ctx := testutils.CreateServiceAuthContext(t.Context(), "service-identity", namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Empty(t, sv2.OwnerReferences)
+		})
+
+		t.Run("ignored for any other identity, owner references are preserved", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+				cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+			})
+			require.NoError(t, err)
+			require.Len(t, sv1.OwnerReferences, 1)
+
+			input := sv1.DeepCopy()
+			input.Spec.Description = "updated"
+			input.OwnerReferences = nil
+
+			ctx := testutils.CreateUserAuthContext(t.Context(), namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Len(t, sv2.OwnerReferences, 1)
+			require.Equal(t, owner.Name, sv2.OwnerReferences[0].Name)
+			require.Equal(t, "updated", sv2.Spec.Description)
+		})
+	})
+
+	t.Run("changing an owner reference", func(t *testing.T) {
+		t.Parallel()
+
+		differentOwner := common.ObjectReference{
+			APIGroup:   "prometheus.datasource.grafana.app",
+			APIVersion: "v0alpha1",
+			Kind:       "DataSource",
+			Name:       "other-ds",
+			Namespace:  namespace,
+		}
+
+		t.Run("applied for AccessPolicy identity", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+				cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+			})
+			require.NoError(t, err)
+			require.Len(t, sv1.OwnerReferences, 1)
+
+			input := sv1.DeepCopy()
+			input.OwnerReferences = []metav1.OwnerReference{differentOwner.ToOwnerReference()}
+
+			ctx := testutils.CreateServiceAuthContext(t.Context(), "service-identity", namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Len(t, sv2.OwnerReferences, 1)
+			require.Equal(t, differentOwner.Name, sv2.OwnerReferences[0].Name)
+		})
+
+		t.Run("ignored for any other identity, owner references are preserved", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+				cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+			})
+			require.NoError(t, err)
+
+			input := sv1.DeepCopy()
+			input.OwnerReferences = []metav1.OwnerReference{differentOwner.ToOwnerReference()}
+
+			ctx := testutils.CreateUserAuthContext(t.Context(), namespace, nil)
+			sv2, err := sut.UpdateSv(ctx, input)
+			require.NoError(t, err)
+			require.Len(t, sv2.OwnerReferences, 1)
+			require.Equal(t, owner.Name, sv2.OwnerReferences[0].Name)
+		})
+
+		t.Run("ignored when auth info is missing, owner references are preserved", func(t *testing.T) {
+			t.Parallel()
+			sut := testutils.Setup(t)
+
+			sv1, err := sut.CreateSv(t.Context(), func(cfg *testutils.CreateSvConfig) {
+				cfg.Sv.OwnerReferences = []metav1.OwnerReference{owner.ToOwnerReference()}
+			})
+			require.NoError(t, err)
+
+			input := sv1.DeepCopy()
+			input.OwnerReferences = []metav1.OwnerReference{differentOwner.ToOwnerReference()}
+
+			sv2, err := sut.UpdateSv(t.Context(), input)
+			require.NoError(t, err)
+			require.Len(t, sv2.OwnerReferences, 1)
+			require.Equal(t, owner.Name, sv2.OwnerReferences[0].Name)
+		})
+	})
+}
+
 func Test_SetAsActive(t *testing.T) {
 	t.Parallel()
 

← Back to Alerts View on GitHub →