Include empty string attributes for CEL authz evaluation

Author: liggitt

staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go

@@ -22,6 +22,7 @@ import (
 	"github.com/google/cel-go/cel"
 	"github.com/google/cel-go/common/types/ref"
 
+	authorizationv1 "k8s.io/api/authorization/v1"
 	"k8s.io/apimachinery/pkg/util/version"
 	apiservercel "k8s.io/apiserver/pkg/cel"
 	"k8s.io/apiserver/pkg/cel/environment"
@@ -143,6 +144,7 @@ func mustBuildEnv(baseEnv *environment.EnvSet) *environment.EnvSet {
 }
 
 // buildRequestType generates a DeclType for SubjectAccessReviewSpec.
+// if attributes are added here, also add to convertObjectToUnstructured.
 func buildRequestType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType {
 	resourceAttributesType := buildResourceAttributesType(field, fields)
 	nonResourceAttributesType := buildNonResourceAttributesType(field, fields)
@@ -157,6 +159,7 @@ func buildRequestType(field func(name string, declType *apiservercel.DeclType, r
 }
 
 // buildResourceAttributesType generates a DeclType for ResourceAttributes.
+// if attributes are added here, also add to convertObjectToUnstructured.
 func buildResourceAttributesType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType {
 	return apiservercel.NewObjectType("kubernetes.ResourceAttributes", fields(
 		field("namespace", apiservercel.StringType, false),
@@ -170,9 +173,42 @@ func buildResourceAttributesType(field func(name string, declType *apiservercel.
 }
 
 // buildNonResourceAttributesType generates a DeclType for NonResourceAttributes.
+// if attributes are added here, also add to convertObjectToUnstructured.
 func buildNonResourceAttributesType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType {
 	return apiservercel.NewObjectType("kubernetes.NonResourceAttributes", fields(
 		field("path", apiservercel.StringType, false),
 		field("verb", apiservercel.StringType, false),
 	))
 }
+
+func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) map[string]interface{} {
+	// Construct version containing every SubjectAccessReview user and string attribute field, even omitempty ones, for evaluation by CEL
+	extra := obj.Extra
+	if extra == nil {
+		extra = map[string]authorizationv1.ExtraValue{}
+	}
+	ret := map[string]interface{}{
+		"user":   obj.User,
+		"groups": obj.Groups,
+		"uid":    string(obj.UID),
+		"extra":  extra,
+	}
+	if obj.ResourceAttributes != nil {
+		ret["resourceAttributes"] = map[string]string{
+			"namespace":   obj.ResourceAttributes.Namespace,
+			"verb":        obj.ResourceAttributes.Verb,
+			"group":       obj.ResourceAttributes.Group,
+			"version":     obj.ResourceAttributes.Version,
+			"resource":    obj.ResourceAttributes.Resource,
+			"subresource": obj.ResourceAttributes.Subresource,
+			"name":        obj.ResourceAttributes.Name,
+		}
+	}
+	if obj.NonResourceAttributes != nil {
+		ret["nonResourceAttributes"] = map[string]string{
+			"verb": obj.NonResourceAttributes.Verb,
+			"path": obj.NonResourceAttributes.Path,
+		}
+	}
+	return ret
+}

staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go

@@ -21,8 +21,8 @@ import (
 	"fmt"
 
 	celgo "github.com/google/cel-go/cel"
+
 	authorizationv1 "k8s.io/api/authorization/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
 
@@ -33,12 +33,8 @@ type CELMatcher struct {
 // eval evaluates the given SubjectAccessReview against all cel matchCondition expression
 func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessReview) (bool, error) {
 	var evalErrors []error
-	specValObject, err := convertObjectToUnstructured(&r.Spec)
-	if err != nil {
-		return false, fmt.Errorf("authz celMatcher eval error: convert SubjectAccessReviewSpec object to unstructured failed: %w", err)
-	}
 	va := map[string]interface{}{
-		"request": specValObject,
+		"request": convertObjectToUnstructured(&r.Spec),
 	}
 	for _, compilationResult := range c.CompilationResults {
 		evalResult, _, err := compilationResult.Program.ContextEval(ctx, va)
@@ -68,14 +64,3 @@ func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessR
 	// return ALL matchConditions evaluate to TRUE successfully without error
 	return true, nil
 }
-
-func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) (map[string]interface{}, error) {
-	if obj == nil {
-		return nil, nil
-	}
-	ret, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
-	if err != nil {
-		return nil, err
-	}
-	return ret, nil
-}

staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go

@@ -704,6 +704,7 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) {
 			Name:   "alice",
 			UID:    "1",
 			Groups: []string{"group1", "group2"},
+			Extra:  map[string][]string{"key1": {"a", "b", "c"}},
 		},
 		ResourceRequest: true,
 		Namespace:       "kittensandponies",
@@ -798,6 +799,7 @@ func TestV1WebhookMatchConditions(t *testing.T) {
 			Name:   "alice",
 			UID:    "1",
 			Groups: []string{"group1", "group2"},
+			Extra:  map[string][]string{"key1": {"a", "b", "c"}},
 		},
 		ResourceRequest: true,
 		Namespace:       "kittensandponies",
@@ -848,6 +850,18 @@ func TestV1WebhookMatchConditions(t *testing.T) {
 				{
 					Expression: "('group1' in request.groups)",
 				},
+				{
+					Expression: "('key1' in request.extra)",
+				},
+				{
+					Expression: "!('key2' in request.extra)",
+				},
+				{
+					Expression: "('a' in request.extra['key1'])",
+				},
+				{
+					Expression: "!('z' in request.extra['key1'])",
+				},
 				{
 					Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'",
 				},
@@ -1028,13 +1042,10 @@ func TestV1WebhookMatchConditions(t *testing.T) {
 			expectedCompileErr: "",
 			// default decisionOnError in newWithBackoff to skip
 			expectedDecision: authorizer.DecisionNoOpinion,
-			expectedEvalErr:  "[cel evaluation error: expression 'request.user == 'bob'' resulted in error: no such key: user, cel evaluation error: expression 'has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'' resulted in error: no such key: verb]",
+			expectedEvalErr:  "cel evaluation error: expression 'request.resourceAttributes.verb == 'get'' resulted in error: no such key: resourceAttributes",
 			expressions: []apiserver.WebhookMatchCondition{
 				{
-					Expression: "request.user == 'bob'",
-				},
-				{
-					Expression: "has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'",
+					Expression: "request.resourceAttributes.verb == 'get'",
 				},
 			},
 		},