[cherry-pick: release-v0.79.x] fix(openshift): block SCC escalation via annotations

docs/SCCConfig.md

@@ -65,8 +65,20 @@ spec:
 - `spec.platforms.openshift.scc.default` specifies the default SCC that
 will be attached to the service account used for workloads (`pipeline` SA by
 default)
-- `spec.platforms.openshift.scc.maxAllowed` specifies the highest SCC that can 
-be requested for in any namespace
+- `spec.platforms.openshift.scc.maxAllowed` specifies the highest (least
+restrictive) SCC that can be requested via a namespace annotation
+
+If `maxAllowed` is not set, the operator defaults it to the value of `default`.
+This means that, out of the box, a namespace can only request an SCC that is
+equally or more restrictive than the default SCC. To allow a namespace to
+request a less restrictive SCC (for example `anyuid`), you must explicitly set
+`maxAllowed` to that SCC (or a less restrictive one) in `TektonConfig`.
+
+> **Security note:** In earlier releases, leaving `maxAllowed` empty allowed a
+> namespace to request *any* SCC — including `privileged` — via the
+> `operator.tekton.dev/scc` annotation. This allowed privilege escalation and
+> has been fixed: an empty `maxAllowed` is now treated as "only the default SCC
+> is allowed".
 
 Note that the SCC specified in `default` field cannot be of a higher priority 
 than the one specified in `maxAllowed` field.
@@ -102,6 +114,14 @@ Tekton needs elevated privileges like running as root and elevated Linux
 capabilities. Users can request for `anyuid` SCC in one namespace without
 impacting permissions of Tekton workloads running in other namespaces.
 
-**Note: The SCC requested by the `operator.tekton.dev/scc` can not have a 
-higher priority than the one specified in `TektonConfig.Spec.Platforms.OpenShift.
-SCC.MaxAllowed` field.**
+Requesting a less restrictive SCC such as `anyuid` this way requires
+`maxAllowed` to permit it. Because `maxAllowed` defaults to the `default` SCC
+(`pipelines-scc` by default), you must first set
+`spec.platforms.openshift.scc.maxAllowed` to `anyuid` (or a less restrictive
+SCC) in `TektonConfig`; otherwise the namespace annotation is rejected.
+
+**Note: The SCC requested by the `operator.tekton.dev/scc` annotation can not
+have a higher priority (be less restrictive) than the one specified in
+`TektonConfig.Spec.Platforms.OpenShift.SCC.MaxAllowed`. If `maxAllowed` is not
+set, it defaults to the `default` SCC, so only the default SCC (or a more
+restrictive one) can be requested via the annotation.**

pkg/apis/operator/v1alpha1/tektonconfig_default_test.go

@@ -193,14 +193,16 @@ func Test_SetDefaults_SCC(t *testing.T) {
 			name:     "default SCC is set to 'pipelines-scc' when nothing is set",
 			inputSCC: nil,
 			expectedSCC: &SCC{
-				Default: PipelinesSCC,
+				Default:    PipelinesSCC,
+				MaxAllowed: PipelinesSCC,
 			},
 		},
 		{
 			name:     "defaulting works when default SCC is empty",
 			inputSCC: &SCC{},
 			expectedSCC: &SCC{
-				Default: PipelinesSCC,
+				Default:    PipelinesSCC,
+				MaxAllowed: PipelinesSCC,
 			},
 		},
 		{
@@ -219,7 +221,41 @@ func Test_SetDefaults_SCC(t *testing.T) {
 				Default: "alreadyExistingSCC",
 			},
 			expectedSCC: &SCC{
-				Default: "alreadyExistingSCC",
+				Default:    "alreadyExistingSCC",
+				MaxAllowed: "alreadyExistingSCC",
+			},
+		},
+		{
+			name: "maxAllowed defaults to default SCC when only default is set",
+			inputSCC: &SCC{
+				Default:    "custom-scc",
+				MaxAllowed: "",
+			},
+			expectedSCC: &SCC{
+				Default:    "custom-scc",
+				MaxAllowed: "custom-scc",
+			},
+		},
+		{
+			name: "maxAllowed is not overridden when explicitly set",
+			inputSCC: &SCC{
+				Default:    PipelinesSCC,
+				MaxAllowed: "privileged",
+			},
+			expectedSCC: &SCC{
+				Default:    PipelinesSCC,
+				MaxAllowed: "privileged",
+			},
+		},
+		{
+			name: "empty maxAllowed gets defaulted to prevent escalation",
+			inputSCC: &SCC{
+				Default:    PipelinesSCC,
+				MaxAllowed: "",
+			},
+			expectedSCC: &SCC{
+				Default:    PipelinesSCC,
+				MaxAllowed: PipelinesSCC,
 			},
 		},
 	}

pkg/apis/operator/v1alpha1/tektonconfig_defaults.go

@@ -66,6 +66,11 @@ func (tc *TektonConfig) SetDefaults(ctx context.Context) {
 		if tc.Spec.Platforms.OpenShift.SCC.Default == "" {
 			tc.Spec.Platforms.OpenShift.SCC.Default = PipelinesSCC
 		}
+		//Security: Default maxAllowed to match default SCC to prevent privilege escalation
+		// via namespace annotations. Empty maxAllowed previously allowed ANY SCC.
+		if tc.Spec.Platforms.OpenShift.SCC.MaxAllowed == "" {
+			tc.Spec.Platforms.OpenShift.SCC.MaxAllowed = tc.Spec.Platforms.OpenShift.SCC.Default
+		}
 
 		setAddonDefaults(&tc.Spec.Addon)
 	} else {

pkg/apis/operator/v1alpha1/tektonconfig_validation.go

@@ -81,24 +81,34 @@ func (tc *TektonConfig) Validate(ctx context.Context) (errs *apis.FieldError) {
 		maxAllowedSCC := tc.Spec.Platforms.OpenShift.SCC.MaxAllowed
 		if maxAllowedSCC != "" {
 			// verify maxAllowed SCC exists on the cluster
-			if err := verifySCCExists(ctx, maxAllowedSCC); err != nil {
-				errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error verifying SCC exists: %s - %v", maxAllowedSCC, err), "spec.platforms.openshift.scc.maxAllowed"))
+			// we don't want to verify pipelines-scc here as it will be created
+			// later when the RBAC reconciler will be run
+			if maxAllowedSCC != PipelinesSCC {
+				if err := verifySCCExists(ctx, maxAllowedSCC); err != nil {
+					errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error verifying SCC exists: %s - %v", maxAllowedSCC, err), "spec.platforms.openshift.scc.maxAllowed"))
+				}
 			}
 
 			// Check that maxAllowed SCC and default SCC are compatible wrt priority
-			hasPriority, err := compareSCCAMoreRestrictiveThanB(ctx, defaultSCC, maxAllowedSCC)
-			if err != nil {
-				errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error comparing priority between maxAllowed and default SCC in TektonConfig: %v", err), "spec.platforms.openshift.scc.maxAllowed"))
-			} else if !hasPriority {
-				errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("maxAllowed SCC (%s) must be less restrictive than the default SCC (%s)", maxAllowedSCC, defaultSCC), "spec.platforms.openshift.scc.maxAllowed"))
+			// Skip this check if either is pipelines-scc (will be created later)
+			if defaultSCC != PipelinesSCC && maxAllowedSCC != PipelinesSCC {
+				hasPriority, err := compareSCCAMoreRestrictiveThanB(ctx, defaultSCC, maxAllowedSCC)
+				if err != nil {
+					errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error comparing priority between maxAllowed and default SCC in TektonConfig: %v", err), "spec.platforms.openshift.scc.maxAllowed"))
+				} else if !hasPriority {
+					errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("maxAllowed SCC (%s) must be less restrictive than the default SCC (%s)", maxAllowedSCC, defaultSCC), "spec.platforms.openshift.scc.maxAllowed"))
+				}
 			}
 
 			// Now validate maxAllowed SCC config with namespaces
-			sccErrors, err := compareSCCsWithAllNamespaces(ctx, maxAllowedSCC)
-			if err != nil {
-				errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error comparing priority between maxAllowed and SCCs requested in all namespaces: %v", err), "spec.platforms.openshift.scc.maxAllowed"))
+			// Skip this check if maxAllowed is pipelines-scc (will be created later)
+			if maxAllowedSCC != PipelinesSCC {
+				sccErrors, err := compareSCCsWithAllNamespaces(ctx, maxAllowedSCC)
+				if err != nil {
+					errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("error comparing priority between maxAllowed and SCCs requested in all namespaces: %v", err), "spec.platforms.openshift.scc.maxAllowed"))
+				}
+				errs = errs.Also(sccErrors)
 			}
-			errs = errs.Also(sccErrors)
 		}
 	}
 

pkg/reconciler/openshift/namespace/namespace.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"github.com/markbates/inflect"
+	security "github.com/openshift/client-go/security/clientset/versioned"
 	"github.com/tektoncd/operator/pkg/client/listers/operator/v1alpha1"
 	"github.com/tektoncd/operator/pkg/common"
 	"github.com/tektoncd/operator/pkg/reconciler/openshift"
@@ -64,6 +65,9 @@ type reconciler struct {
 	secretlister       corelisters.SecretLister
 	tektonConfigLister v1alpha1.TektonConfigLister
 
+	// securityClient is optional and only used for testing to inject a fake client
+	securityClient security.Interface
+
 	disallowUnknownFields bool
 	secretName            string
 }
@@ -176,7 +180,10 @@ func (ac *reconciler) admissionAllowed(ctx context.Context, req *admissionv1.Adm
 
 	logger.Infof("Trying to admit namespace: %s with SCC: %s", namespaceObject.Name, nsSCC)
 
-	securityClient := common.GetSecurityClient(ctx)
+	securityClient := ac.securityClient
+	if securityClient == nil {
+		securityClient = common.GetSecurityClient(ctx)
+	}
 
 	// verify SCC exists on the cluster
 	_, err := securityClient.SecurityV1().SecurityContextConstraints().Get(ctx, nsSCC, metav1.GetOptions{})
@@ -191,10 +198,21 @@ func (ac *reconciler) admissionAllowed(ctx context.Context, req *admissionv1.Adm
 
 	// Check if the SCC requested in namespace is in line with the maxAllowed SCC in TektonConfig
 	maxAllowedSCC := tc.Spec.Platforms.OpenShift.SCC.MaxAllowed
+	defaultSCC := tc.Spec.Platforms.OpenShift.SCC.Default
 
-	// If no maxAllowed is set, no problem
+	// Security: Treat empty maxAllowed as "only default SCC allowed" to prevent privilege
+	// escalation. Empty maxAllowed previously allowed ANY SCC.
 	if maxAllowedSCC == "" {
-		logger.Infof("Namespace %s validation: no maxAllowed SCC set in TektonConfig", namespaceObject.Name)
+		if nsSCC != defaultSCC {
+			prioErr := fmt.Sprintf("namespace %s requested SCC %s, but maxAllowed is not configured. Only the default SCC %s is permitted", namespaceObject.Name, nsSCC, defaultSCC)
+			logger.Warnf("Namespace %s validation failed: %s", namespaceObject.Name, prioErr)
+			return false, &metav1.Status{
+				Status:  "Failure",
+				Message: prioErr,
+			}, nil
+		}
+		// Requesting default SCC when maxAllowed is empty is allowed
+		logger.Infof("Namespace %s validation: maxAllowed not set, allowing default SCC %s", namespaceObject.Name, defaultSCC)
 		return true, nil, nil
 	}
 

pkg/reconciler/openshift/namespace/namespace_test.go

@@ -21,6 +21,8 @@
 	"encoding/json"
 	"testing"
 
+	securityv1 "github.com/openshift/api/security/v1"
+	fakesecurity "github.com/openshift/client-go/security/clientset/versioned/fake"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	operatorfake "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
 	operatorinformers "github.com/tektoncd/operator/pkg/client/informers/externalversions"
@@ -166,3 +168,170 @@
 	assert.Equal(t, false, allowed)
 	assert.Assert(t, status == nil, "status should be nil for error")
 }
+
+// TestReconciler_admissionAllowed_SCCEscalationPrevention tests the security fix
+// for prevents privilege escalation via namespace annotations whenmaxAllowed is empty.
+func TestReconciler_admissionAllowed_SCCEscalationPrevention(t *testing.T) {
+	tests := []struct {
+		name           string
+		sccAnnotation  string
+		defaultSCC     string
+		maxAllowedSCC  string
+		wantAllowed    bool
+		wantStatusNil  bool
+		wantErrMessage string
+	}{
+		{
+			name:           "empty maxAllowed allows default SCC",
+			sccAnnotation:  "pipelines-scc",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "",
+			wantAllowed:    true,
+			wantStatusNil:  true,
+			wantErrMessage: "",
+		},
+		{
+			name:           "empty maxAllowed with custom default allows that default",
+			sccAnnotation:  "custom-scc",
+			defaultSCC:     "custom-scc",
+			maxAllowedSCC:  "",
+			wantAllowed:    true,
+			wantStatusNil:  true,
+			wantErrMessage: "",
+		},
+		{
+			name:           "empty maxAllowed blocks privileged SCC escalation",
+			sccAnnotation:  "privileged",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "",
+			wantAllowed:    false,
+			wantStatusNil:  false,
+			wantErrMessage: "namespace test-namespace requested SCC privileged, but maxAllowed is not configured. Only the default SCC pipelines-scc is permitted",
+		},
+		{
+			name:           "empty maxAllowed blocks anyuid SCC escalation",
+			sccAnnotation:  "anyuid",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "",
+			wantAllowed:    false,
+			wantStatusNil:  false,
+			wantErrMessage: "namespace test-namespace requested SCC anyuid, but maxAllowed is not configured. Only the default SCC pipelines-scc is permitted",
+		},
+		// Bug fix verification: maxAllowed should allow same or more restrictive SCCs
+		{
+			name:           "maxAllowed equals default allows same SCC (after defaulting)",
+			sccAnnotation:  "pipelines-scc",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "pipelines-scc",
+			wantAllowed:    true,
+			wantStatusNil:  true,
+			wantErrMessage: "",
+		},
+		{
+			name:           "explicit maxAllowed allows same SCC",
+			sccAnnotation:  "anyuid",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "anyuid",
+			wantAllowed:    true,
+			wantStatusNil:  true,
+			wantErrMessage: "",
+		},
+		{
+			name:           "maxAllowed correctly blocks less restrictive SCC",
+			sccAnnotation:  "privileged",
+			defaultSCC:     "pipelines-scc",
+			maxAllowedSCC:  "anyuid",
+			wantAllowed:    false,
+			wantStatusNil:  false,
+			wantErrMessage: "namespace: test-namespace has requested SCC: privileged, but it is less restrictive than 'maxAllowed' SCC: anyuid",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := logging.WithLogger(context.Background(), logtesting.TestLogger(t))
+
+			// Create namespace with SCC annotation
+			namespace := &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-namespace",
+					Annotations: map[string]string{
+						"operator.tekton.dev/scc": tt.sccAnnotation,
+					},
+				},
+			}
+
+			// Create TektonConfig with specified SCC settings
+			tektonConfig := &v1alpha1.TektonConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "config",
+				},
+				Spec: v1alpha1.TektonConfigSpec{
+					Platforms: v1alpha1.Platforms{
+						OpenShift: v1alpha1.OpenShift{
+							SCC: &v1alpha1.SCC{
+								Default:    tt.defaultSCC,
+								MaxAllowed: tt.maxAllowedSCC,
+							},
+						},
+					},
+				},
+			}
+
+			// Setup fake client and informer
+			operatorClient := operatorfake.NewSimpleClientset(tektonConfig)
+			operatorInformerFactory := operatorinformers.NewSharedInformerFactory(operatorClient, 0)
+			tektonConfigInformer := operatorInformerFactory.Operator().V1alpha1().TektonConfigs()
+
+			err := tektonConfigInformer.Informer().GetStore().Add(tektonConfig)
+			assert.NilError(t, err)
+
+			// Create fake security client and add common SCCs
+			securityClient := fakesecurity.NewClientset()
+			commonSCCs := []securityv1.SecurityContextConstraints{
+				{ObjectMeta: metav1.ObjectMeta{Name: "pipelines-scc"}},
+				{ObjectMeta: metav1.ObjectMeta{Name: "custom-scc"}},
+				{ObjectMeta: metav1.ObjectMeta{Name: "privileged"}},
+				{ObjectMeta: metav1.ObjectMeta{Name: "anyuid"}},
+				{ObjectMeta: metav1.ObjectMeta{Name: "restricted"}},
+			}
+			for _, scc := range commonSCCs {
+				_, err := securityClient.SecurityV1().SecurityContextConstraints().Create(ctx, &scc, metav1.CreateOptions{})
+				assert.NilError(t, err)
+			}
+
+			r := &reconciler{
+				tektonConfigLister: tektonConfigInformer.Lister(),
+				securityClient:     securityClient,
+			}
+
+			// Create admission request
+			namespaceBytes, err := json.Marshal(namespace)
+			assert.NilError(t, err)
+
+			req := &admissionv1.AdmissionRequest{
+				Kind: metav1.GroupVersionKind{
+					Group:   "",
+					Version: "v1",
+					Kind:    "Namespace",
+				},
+				Object: runtime.RawExtension{
+					Raw: namespaceBytes,
+				},
+				Operation: admissionv1.Create,
+			}
+
+			allowed, status, err := r.admissionAllowed(ctx, req)
+
+			// Verify results
+			assert.NilError(t, err, "should not error during validation")
+			assert.Equal(t, tt.wantAllowed, allowed, "allowed mismatch")
+			assert.Equal(t, tt.wantStatusNil, status == nil, "status nil mismatch")
+
+			if !tt.wantStatusNil && status != nil {
+				assert.Equal(t, status.Message, tt.wantErrMessage,
+					"expected message %q, got %q", tt.wantErrMessage, status.Message)
+			}
+		})
+	}
+}