diff --git a/pkg/kubeapiserver/authorizer/config.go b/pkg/kubeapiserver/authorizer/config.go index c33e2fa24d3..dee62636220 100644 --- a/pkg/kubeapiserver/authorizer/config.go +++ b/pkg/kubeapiserver/authorizer/config.go @@ -19,6 +19,7 @@ package authorizer import ( "errors" "fmt" + utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/wait" authzconfig "k8s.io/apiserver/pkg/apis/apiserver" @@ -122,6 +123,7 @@ func (config Config) New() (authorizer.Authorizer, authorizer.RuleResolver, erro configuredAuthorizer.Webhook.AuthorizedTTL.Duration, configuredAuthorizer.Webhook.UnauthorizedTTL.Duration, *config.WebhookRetryBackoff, + configuredAuthorizer.Webhook.MatchConditions, ) if err != nil { return nil, nil, err diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index 59d15fdc07d..ddf0b4b100d 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -17,8 +17,8 @@ limitations under the License. package validation import ( + "errors" "fmt" - utilvalidation "k8s.io/apimachinery/pkg/util/validation" "net/url" "os" "path/filepath" @@ -27,10 +27,15 @@ import ( v1 "k8s.io/api/authorization/v1" "k8s.io/api/authorization/v1beta1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" api "k8s.io/apiserver/pkg/apis/apiserver" + authorizationcel "k8s.io/apiserver/pkg/authorization/cel" + "k8s.io/apiserver/pkg/cel" + "k8s.io/apiserver/pkg/cel/environment" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/util/cert" ) @@ -334,24 +339,81 @@ func ValidateWebhookConfiguration(fldPath *field.Path, c *api.WebhookConfigurati allErrs = append(allErrs, field.NotSupported(fldPath.Child("connectionInfo", "type"), c.ConnectionInfo, []string{api.AuthorizationWebhookConnectionInfoTypeInCluster, api.AuthorizationWebhookConnectionInfoTypeKubeConfigFile})) } - // TODO: Remove this check and ensure that correct validations below for MatchConditions are added - // for i, condition := range c.MatchConditions { - // fldPath := fldPath.Child("matchConditions").Index(i).Child("expression") - // if len(strings.TrimSpace(condition.Expression)) == 0 { - // allErrs = append(allErrs, field.Required(fldPath, "")) - // } else { - // allErrs = append(allErrs, ValidateWebhookMatchCondition(fldPath, sampleSAR, condition.Expression)...) - // } - // } - if len(c.MatchConditions) != 0 { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("matchConditions"), c.MatchConditions, []string{})) + _, errs := compileMatchConditions(c.MatchConditions, fldPath, utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthorizationConfiguration)) + allErrs = append(allErrs, errs...) + + return allErrs +} + +// ValidateAndCompileMatchConditions validates a given webhook's matchConditions. +// This is exported for use in authz package. +func ValidateAndCompileMatchConditions(matchConditions []api.WebhookMatchCondition) (*authorizationcel.CELMatcher, field.ErrorList) { + return compileMatchConditions(matchConditions, nil, utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthorizationConfiguration)) +} + +func compileMatchConditions(matchConditions []api.WebhookMatchCondition, fldPath *field.Path, structuredAuthzFeatureEnabled bool) (*authorizationcel.CELMatcher, field.ErrorList) { + var allErrs field.ErrorList + // should fail when match conditions are used without feature enabled + if len(matchConditions) > 0 && !structuredAuthzFeatureEnabled { + allErrs = append(allErrs, field.Invalid(fldPath.Child("matchConditions"), "", "matchConditions are not supported when StructuredAuthorizationConfiguration feature gate is disabled")) + } + if len(matchConditions) > 64 { + allErrs = append(allErrs, field.TooMany(fldPath.Child("matchConditions"), len(matchConditions), 64)) + return nil, allErrs } - return allErrs + compiler := authorizationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + seenExpressions := sets.NewString() + var compilationResults []authorizationcel.CompilationResult + + for i, condition := range matchConditions { + fldPath := fldPath.Child("matchConditions").Index(i).Child("expression") + if len(strings.TrimSpace(condition.Expression)) == 0 { + allErrs = append(allErrs, field.Required(fldPath, "")) + continue + } + if seenExpressions.Has(condition.Expression) { + allErrs = append(allErrs, field.Duplicate(fldPath, condition.Expression)) + continue + } + seenExpressions.Insert(condition.Expression) + compilationResult, err := compileMatchConditionsExpression(fldPath, compiler, condition.Expression) + if err != nil { + allErrs = append(allErrs, err) + continue + } + compilationResults = append(compilationResults, compilationResult) + } + if len(compilationResults) == 0 { + return nil, allErrs + } + return &authorizationcel.CELMatcher{ + CompilationResults: compilationResults, + }, allErrs } -func ValidateWebhookMatchCondition(fldPath *field.Path, sampleSAR runtime.Object, expression string) field.ErrorList { - allErrs := field.ErrorList{} - // TODO: typecheck CEL expression - return allErrs +func compileMatchConditionsExpression(fldPath *field.Path, compiler authorizationcel.Compiler, expression string) (authorizationcel.CompilationResult, *field.Error) { + authzExpression := &authorizationcel.SubjectAccessReviewMatchCondition{ + Expression: expression, + } + compilationResult, err := compiler.CompileCELExpression(authzExpression) + if err != nil { + return compilationResult, convertCELErrorToValidationError(fldPath, authzExpression, err) + } + return compilationResult, nil +} + +func convertCELErrorToValidationError(fldPath *field.Path, expression authorizationcel.ExpressionAccessor, err error) *field.Error { + var celErr *cel.Error + if errors.As(err, &celErr) { + switch celErr.Type { + case cel.ErrorTypeRequired: + return field.Required(fldPath, celErr.Detail) + case cel.ErrorTypeInvalid: + return field.Invalid(fldPath, expression.GetExpression(), celErr.Detail) + default: + return field.InternalError(fldPath, celErr) + } + } + return field.InternalError(fldPath, fmt.Errorf("error is not cel error: %w", err)) } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index d2e5f28c9f0..22630309d28 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -32,7 +32,10 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" api "k8s.io/apiserver/pkg/apis/apiserver" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" certutil "k8s.io/client-go/util/cert" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -428,6 +431,8 @@ type ( ) func TestValidateAuthorizationConfiguration(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, true)() + badKubeConfigFile := "../some/relative/path/kubeconfig" tempKubeConfigFile, err := os.CreateTemp("/tmp", "kubeconfig") @@ -557,6 +562,39 @@ func TestValidateAuthorizationConfiguration(t *testing.T) { knownTypes: sets.NewString(string("Webhook")), repeatableTypes: sets.NewString(string("Webhook")), }, + { + name: "bare minimum configuration with Webhook and MatchConditions", + configuration: api.AuthorizationConfiguration{ + Authorizers: []api.AuthorizerConfiguration{ + { + Type: "Webhook", + Name: "default", + Webhook: &api.WebhookConfiguration{ + Timeout: metav1.Duration{Duration: 5 * time.Second}, + AuthorizedTTL: metav1.Duration{Duration: 5 * time.Minute}, + UnauthorizedTTL: metav1.Duration{Duration: 30 * time.Second}, + FailurePolicy: "NoOpinion", + SubjectAccessReviewVersion: "v1", + MatchConditionSubjectAccessReviewVersion: "v1", + ConnectionInfo: api.WebhookConnectionInfo{ + Type: "InClusterConfig", + }, + MatchConditions: []api.WebhookMatchCondition{ + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kube-system'", + }, + { + Expression: "request.user == 'admin'", + }, + }, + }, + }, + }, + }, + expectedErrList: field.ErrorList{}, + knownTypes: sets.NewString(string("Webhook")), + repeatableTypes: sets.NewString(string("Webhook")), + }, { name: "bare minimum configuration with multiple webhooks", configuration: api.AuthorizationConfiguration{ @@ -1156,8 +1194,6 @@ func TestValidateAuthorizationConfiguration(t *testing.T) { knownTypes: sets.NewString(string("Webhook")), repeatableTypes: sets.NewString(string("Webhook")), }, - - // TODO: When the CEL expression validator is implemented, add a few test cases to typecheck the expression } for _, test := range tests { @@ -1166,20 +1202,120 @@ func TestValidateAuthorizationConfiguration(t *testing.T) { if len(errList) != len(test.expectedErrList) { t.Errorf("expected %d errs, got %d, errors %v", len(test.expectedErrList), len(errList), errList) } - - for i, expected := range test.expectedErrList { - if expected.Type.String() != errList[i].Type.String() { - t.Errorf("expected err type %s, got %s", - expected.Type.String(), - errList[i].Type.String()) - } - if expected.BadValue != errList[i].BadValue { - t.Errorf("expected bad value '%s', got '%s'", - expected.BadValue, - errList[i].BadValue) + if len(errList) == len(test.expectedErrList) { + for i, expected := range test.expectedErrList { + if expected.Type.String() != errList[i].Type.String() { + t.Errorf("expected err type %s, got %s", + expected.Type.String(), + errList[i].Type.String()) + } + if expected.BadValue != errList[i].BadValue { + t.Errorf("expected bad value '%s', got '%s'", + expected.BadValue, + errList[i].BadValue) + } } } }) } } + +func TestValidateAndCompileMatchConditions(t *testing.T) { + testCases := []struct { + name string + matchConditions []api.WebhookMatchCondition + featureEnabled bool + expectedErr string + }{ + { + name: "match conditions are used With feature enabled", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kube-system'", + }, + { + Expression: "request.user == 'admin'", + }, + }, + featureEnabled: true, + expectedErr: "", + }, + { + name: "should fail when match conditions are used without feature enabled", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kube-system'", + }, + { + Expression: "request.user == 'admin'", + }, + }, + featureEnabled: false, + expectedErr: `matchConditions: Invalid value: "": matchConditions are not supported when StructuredAuthorizationConfiguration feature gate is disabled`, + }, + { + name: "no matchConditions should not require feature enablement", + matchConditions: []api.WebhookMatchCondition{}, + featureEnabled: false, + expectedErr: "", + }, + { + name: "match conditions with invalid expressions", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: " ", + }, + }, + featureEnabled: true, + expectedErr: "matchConditions[0].expression: Required value", + }, + { + name: "match conditions with duplicate expressions", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: "request.user == 'admin'", + }, + { + Expression: "request.user == 'admin'", + }, + }, + featureEnabled: true, + expectedErr: `matchConditions[1].expression: Duplicate value: "request.user == 'admin'"`, + }, + { + name: "match conditions with undeclared reference", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: "test", + }, + }, + featureEnabled: true, + expectedErr: "matchConditions[0].expression: Invalid value: \"test\": compilation failed: ERROR: :1:1: undeclared reference to 'test' (in container '')\n | test\n | ^", + }, + { + name: "match conditions with bad return type", + matchConditions: []api.WebhookMatchCondition{ + { + Expression: "request.user = 'test'", + }, + }, + featureEnabled: true, + expectedErr: "matchConditions[0].expression: Invalid value: \"request.user = 'test'\": compilation failed: ERROR: :1:14: Syntax error: token recognition error at: '= '\n | request.user = 'test'\n | .............^\nERROR: :1:16: Syntax error: extraneous input ''test'' expecting \n | request.user = 'test'\n | ...............^", + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, tt.featureEnabled)() + celMatcher, errList := ValidateAndCompileMatchConditions(tt.matchConditions) + if len(tt.expectedErr) == 0 && len(tt.matchConditions) > 0 && len(errList) == 0 && celMatcher == nil { + t.Errorf("celMatcher should not be nil when there are matchCondition and no error returned") + } + got := errList.ToAggregate() + if d := cmp.Diff(tt.expectedErr, errString(got)); d != "" { + t.Fatalf("ValidateAndCompileMatchConditions validation mismatch (-want +got):\n%s", d) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go new file mode 100644 index 00000000000..e441c347d07 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go @@ -0,0 +1,178 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "fmt" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types/ref" + + "k8s.io/apimachinery/pkg/util/version" + apiservercel "k8s.io/apiserver/pkg/cel" + "k8s.io/apiserver/pkg/cel/environment" +) + +const ( + subjectAccessReviewRequestVarName = "request" +) + +// CompilationResult represents a compiled authorization cel expression. +type CompilationResult struct { + Program cel.Program + ExpressionAccessor ExpressionAccessor +} + +// EvaluationResult contains the minimal required fields and metadata of a cel evaluation +type EvaluationResult struct { + EvalResult ref.Val + ExpressionAccessor ExpressionAccessor +} + +// Compiler is an interface for compiling CEL expressions with the desired environment mode. +type Compiler interface { + CompileCELExpression(expressionAccessor ExpressionAccessor) (CompilationResult, error) +} + +type compiler struct { + envSet *environment.EnvSet +} + +// NewCompiler returns a new Compiler. +func NewCompiler(env *environment.EnvSet) Compiler { + return &compiler{ + envSet: mustBuildEnv(env), + } +} + +func (c compiler) CompileCELExpression(expressionAccessor ExpressionAccessor) (CompilationResult, error) { + resultError := func(errorString string, errType apiservercel.ErrorType) (CompilationResult, error) { + err := &apiservercel.Error{ + Type: errType, + Detail: errorString, + } + return CompilationResult{ + ExpressionAccessor: expressionAccessor, + }, err + } + env, err := c.envSet.Env(environment.StoredExpressions) + if err != nil { + return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal) + } + ast, issues := env.Compile(expressionAccessor.GetExpression()) + if issues != nil { + return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid) + } + found := false + returnTypes := expressionAccessor.ReturnTypes() + for _, returnType := range returnTypes { + if ast.OutputType() == returnType { + found = true + break + } + } + if !found { + var reason string + if len(returnTypes) == 1 { + reason = fmt.Sprintf("must evaluate to %v but got %v", returnTypes[0].String(), ast.OutputType()) + } else { + reason = fmt.Sprintf("must evaluate to one of %v", returnTypes) + } + + return resultError(reason, apiservercel.ErrorTypeInvalid) + } + _, err = cel.AstToCheckedExpr(ast) + if err != nil { + // should be impossible since env.Compile returned no issues + return resultError("unexpected compilation error: "+err.Error(), apiservercel.ErrorTypeInternal) + } + prog, err := env.Program(ast) + if err != nil { + return resultError("program instantiation failed: "+err.Error(), apiservercel.ErrorTypeInternal) + } + return CompilationResult{ + Program: prog, + ExpressionAccessor: expressionAccessor, + }, nil +} + +func mustBuildEnv(baseEnv *environment.EnvSet) *environment.EnvSet { + field := func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField { + return apiservercel.NewDeclField(name, declType, required, nil, nil) + } + fields := func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField { + result := make(map[string]*apiservercel.DeclField, len(fields)) + for _, f := range fields { + result[f.Name] = f + } + return result + } + subjectAccessReviewSpecRequestType := buildRequestType(field, fields) + extended, err := baseEnv.Extend( + environment.VersionedOptions{ + // we record this as 1.0 since it was available in the + // first version that supported this feature + IntroducedVersion: version.MajorMinor(1, 0), + EnvOptions: []cel.EnvOption{ + cel.Variable(subjectAccessReviewRequestVarName, subjectAccessReviewSpecRequestType.CelType()), + }, + DeclTypes: []*apiservercel.DeclType{ + subjectAccessReviewSpecRequestType, + }, + }, + ) + if err != nil { + panic(fmt.Sprintf("environment misconfigured: %v", err)) + } + + return extended +} + +// buildRequestType generates a DeclType for SubjectAccessReviewSpec. +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) + return apiservercel.NewObjectType("kubernetes.SubjectAccessReviewSpec", fields( + field("resourceAttributes", resourceAttributesType, false), + field("nonResourceAttributes", nonResourceAttributesType, false), + field("user", apiservercel.StringType, false), + field("groups", apiservercel.NewListType(apiservercel.StringType, -1), false), + field("extra", apiservercel.NewMapType(apiservercel.StringType, apiservercel.NewListType(apiservercel.StringType, -1), -1), false), + field("uid", apiservercel.StringType, false), + )) +} + +// buildResourceAttributesType generates a DeclType for ResourceAttributes. +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), + field("verb", apiservercel.StringType, false), + field("group", apiservercel.StringType, false), + field("version", apiservercel.StringType, false), + field("resource", apiservercel.StringType, false), + field("subresource", apiservercel.StringType, false), + field("name", apiservercel.StringType, false), + )) +} + +// buildNonResourceAttributesType generates a DeclType for NonResourceAttributes. +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), + )) +} diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go new file mode 100644 index 00000000000..ec7975b90e0 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile_test.go @@ -0,0 +1,158 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "fmt" + "reflect" + "strings" + "testing" + + v1 "k8s.io/api/authorization/v1" + apiservercel "k8s.io/apiserver/pkg/cel" + "k8s.io/apiserver/pkg/cel/environment" +) + +func TestCompileCELExpression(t *testing.T) { + cases := []struct { + name string + expression string + expectedError string + }{ + { + name: "SubjectAccessReviewSpec user comparison", + expression: "request.user == 'bob'", + }, + { + name: "undefined fields", + expression: "request.time == 'now'", + expectedError: "undefined field", + }, + { + name: "Syntax errors", + expression: "request++'", + expectedError: "Syntax error", + }, + { + name: "bad return type", + expression: "request.user", + expectedError: "must evaluate to bool", + }, + { + name: "undeclared reference", + expression: "x.user", + expectedError: "undeclared reference", + }, + } + + compiler := NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion())) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := compiler.CompileCELExpression(&SubjectAccessReviewMatchCondition{ + Expression: tc.expression, + }) + if len(tc.expectedError) > 0 && (err == nil || !strings.Contains(err.Error(), tc.expectedError)) { + t.Fatalf("expected error: %s compiling expression %s, got: %v", tc.expectedError, tc.expression, err) + } + if len(tc.expectedError) == 0 && err != nil { + t.Fatalf("unexpected error %v compiling expression %s", err, tc.expression) + } + }) + } +} + +func TestBuildRequestType(t *testing.T) { + f := func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField { + return apiservercel.NewDeclField(name, declType, required, nil, nil) + } + fs := func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField { + result := make(map[string]*apiservercel.DeclField, len(fields)) + for _, f := range fields { + result[f.Name] = f + } + return result + } + + requestDeclType := buildRequestType(f, fs) + requestType := reflect.TypeOf(v1.SubjectAccessReviewSpec{}) + if len(requestDeclType.Fields) != requestType.NumField() { + t.Fatalf("expected %d fields for SubjectAccessReviewSpec, got %d", requestType.NumField(), len(requestDeclType.Fields)) + } + resourceAttributesDeclType := buildResourceAttributesType(f, fs) + resourceAttributeType := reflect.TypeOf(v1.ResourceAttributes{}) + if len(resourceAttributesDeclType.Fields) != resourceAttributeType.NumField() { + t.Fatalf("expected %d fields for ResourceAttributes, got %d", resourceAttributeType.NumField(), len(resourceAttributesDeclType.Fields)) + } + nonResourceAttributesDeclType := buildNonResourceAttributesType(f, fs) + nonResourceAttributeType := reflect.TypeOf(v1.NonResourceAttributes{}) + if len(nonResourceAttributesDeclType.Fields) != nonResourceAttributeType.NumField() { + t.Fatalf("expected %d fields for NonResourceAttributes, got %d", nonResourceAttributeType.NumField(), len(nonResourceAttributesDeclType.Fields)) + } + if err := compareFieldsForType(t, requestType, requestDeclType, f, fs); err != nil { + t.Error(err) + } +} + +func compareFieldsForType(t *testing.T, nativeType reflect.Type, declType *apiservercel.DeclType, field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) error { + for i := 0; i < nativeType.NumField(); i++ { + nativeField := nativeType.Field(i) + jsonTagParts := strings.Split(nativeField.Tag.Get("json"), ",") + if len(jsonTagParts) < 1 { + t.Fatal("expected json tag to be present") + } + fieldName := jsonTagParts[0] + + declField, ok := declType.Fields[fieldName] + if !ok { + t.Fatalf("expected field %q to be present", nativeField.Name) + } + declFieldType := nativeTypeToCELType(t, nativeField.Type, field, fields) + if declFieldType != nil && declFieldType.CelType().Equal(declField.Type.CelType()).Value() != true { + return fmt.Errorf("expected native field %q to have type %v, got %v", nativeField.Name, nativeField.Type, declField.Type) + } + } + return nil +} + +func nativeTypeToCELType(t *testing.T, nativeType reflect.Type, field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { + switch nativeType { + case reflect.TypeOf(""): + return apiservercel.StringType + case reflect.TypeOf([]string{}): + return apiservercel.NewListType(apiservercel.StringType, -1) + case reflect.TypeOf(map[string]v1.ExtraValue{}): + return apiservercel.NewMapType(apiservercel.StringType, apiservercel.NewListType(apiservercel.StringType, -1), -1) + case reflect.TypeOf(&v1.ResourceAttributes{}): + resourceAttributesDeclType := buildResourceAttributesType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(v1.ResourceAttributes{}), resourceAttributesDeclType, field, fields); err != nil { + t.Error(err) + return nil + } + return resourceAttributesDeclType + case reflect.TypeOf(&v1.NonResourceAttributes{}): + nonResourceAttributesDeclType := buildNonResourceAttributesType(field, fields) + if err := compareFieldsForType(t, reflect.TypeOf(v1.NonResourceAttributes{}), nonResourceAttributesDeclType, field, fields); err != nil { + t.Error(err) + return nil + } + return nonResourceAttributesDeclType + default: + t.Fatalf("unsupported type %v", nativeType) + } + return nil +} diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/interface.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/interface.go new file mode 100644 index 00000000000..82166830c87 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/interface.go @@ -0,0 +1,41 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + celgo "github.com/google/cel-go/cel" +) + +type ExpressionAccessor interface { + GetExpression() string + ReturnTypes() []*celgo.Type +} + +var _ ExpressionAccessor = &SubjectAccessReviewMatchCondition{} + +// SubjectAccessReviewMatchCondition is a CEL expression that maps a SubjectAccessReview request to a list of values. +type SubjectAccessReviewMatchCondition struct { + Expression string +} + +func (v *SubjectAccessReviewMatchCondition) GetExpression() string { + return v.Expression +} + +func (v *SubjectAccessReviewMatchCondition) ReturnTypes() []*celgo.Type { + return []*celgo.Type{celgo.BoolType} +} diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go new file mode 100644 index 00000000000..09a462f68d7 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go @@ -0,0 +1,81 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "context" + "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" +) + +type CELMatcher struct { + CompilationResults []CompilationResult +} + +// 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, + } + for _, compilationResult := range c.CompilationResults { + evalResult, _, err := compilationResult.Program.ContextEval(ctx, va) + if err != nil { + evalErrors = append(evalErrors, fmt.Errorf("cel evaluation error: expression '%v' resulted in error: %w", compilationResult.ExpressionAccessor.GetExpression(), err)) + continue + } + if evalResult.Type() != celgo.BoolType { + evalErrors = append(evalErrors, fmt.Errorf("cel evaluation error: expression '%v' eval result type should be bool but got %W", compilationResult.ExpressionAccessor.GetExpression(), evalResult.Type())) + continue + } + match, ok := evalResult.Value().(bool) + if !ok { + evalErrors = append(evalErrors, fmt.Errorf("cel evaluation error: expression '%v' eval result value should be bool but got %W", compilationResult.ExpressionAccessor.GetExpression(), evalResult.Value())) + continue + } + // If at least one matchCondition successfully evaluates to FALSE, + // return early + if !match { + return false, nil + } + } + // if there is any error, return + if len(evalErrors) > 0 { + return false, utilerrors.NewAggregate(evalErrors) + } + // 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 +} diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/metrics_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/metrics_test.go index 9dc9fb969f5..6cdeb60f3be 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/metrics_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/metrics_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "k8s.io/apiserver/pkg/apis/apiserver" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" ) @@ -79,7 +80,7 @@ func TestAuthorizerMetrics(t *testing.T) { RecordRequestTotal: fakeAuthzMetrics.RequestTotal, RecordRequestLatency: fakeAuthzMetrics.RequestLatency, } - wh, err := newV1Authorizer(server.URL, scenario.clientCert, scenario.clientKey, scenario.clientCA, 0, authzMetrics) + wh, err := newV1Authorizer(server.URL, scenario.clientCert, scenario.clientKey, scenario.clientCA, 0, authzMetrics, []apiserver.WebhookMatchCondition{}) if err != nil { t.Error("failed to create client") return diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go index 191b3731850..594fdca19d8 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go @@ -31,8 +31,13 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/cache" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/apis/apiserver" + apiservervalidation "k8s.io/apiserver/pkg/apis/apiserver/validation" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + authorizationcel "k8s.io/apiserver/pkg/authorization/cel" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/kubernetes/scheme" authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1" @@ -66,11 +71,12 @@ type WebhookAuthorizer struct { retryBackoff wait.Backoff decisionOnError authorizer.Decision metrics AuthorizerMetrics + celMatcher *authorizationcel.CELMatcher } // NewFromInterface creates a WebhookAuthorizer using the given subjectAccessReview client func NewFromInterface(subjectAccessReview authorizationv1client.AuthorizationV1Interface, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { - return newWithBackoff(&subjectAccessReviewV1Client{subjectAccessReview.RESTClient()}, authorizedTTL, unauthorizedTTL, retryBackoff, metrics) + return newWithBackoff(&subjectAccessReviewV1Client{subjectAccessReview.RESTClient()}, authorizedTTL, unauthorizedTTL, retryBackoff, nil, metrics) } // New creates a new WebhookAuthorizer from the provided kubeconfig file. @@ -92,19 +98,24 @@ func NewFromInterface(subjectAccessReview authorizationv1client.AuthorizationV1I // // For additional HTTP configuration, refer to the kubeconfig documentation // https://kubernetes.io/docs/user-guide/kubeconfig-file/. -func New(config *rest.Config, version string, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff) (*WebhookAuthorizer, error) { +func New(config *rest.Config, version string, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, matchConditions []apiserver.WebhookMatchCondition) (*WebhookAuthorizer, error) { subjectAccessReview, err := subjectAccessReviewInterfaceFromConfig(config, version, retryBackoff) if err != nil { return nil, err } - return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff, AuthorizerMetrics{ + return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff, matchConditions, AuthorizerMetrics{ RecordRequestTotal: noopMetrics{}.RecordRequestTotal, RecordRequestLatency: noopMetrics{}.RecordRequestLatency, }) } // newWithBackoff allows tests to skip the sleep. -func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { +func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, matchConditions []apiserver.WebhookMatchCondition, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { + // compile all expressions once in validation and save the results to be used for eval later + cm, fieldErr := apiservervalidation.ValidateAndCompileMatchConditions(matchConditions) + if err := fieldErr.ToAggregate(); err != nil { + return nil, err + } return &WebhookAuthorizer{ subjectAccessReview: subjectAccessReview, responseCache: cache.NewLRUExpireCache(8192), @@ -113,6 +124,7 @@ func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, un retryBackoff: retryBackoff, decisionOnError: authorizer.DecisionNoOpinion, metrics: metrics, + celMatcher: cm, }, nil } @@ -190,6 +202,24 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri Verb: attr.GetVerb(), } } + // skipping match when feature is not enabled + if utilfeature.DefaultFeatureGate.Enabled(features.StructuredAuthorizationConfiguration) { + // Process Match Conditions before calling the webhook + matches, err := w.match(ctx, r) + // If at least one matchCondition evaluates to an error (but none are FALSE): + // If failurePolicy=Deny, then the webhook rejects the request + // If failurePolicy=NoOpinion, then the error is ignored and the webhook is skipped + if err != nil { + return w.decisionOnError, "", err + } + // If at least one matchCondition successfully evaluates to FALSE, + // then the webhook is skipped. + if !matches { + return authorizer.DecisionNoOpinion, "", nil + } + } + // If all evaluated successfully and ALL matchConditions evaluate to TRUE, + // then the webhook is called. key, err := json.Marshal(r.Spec) if err != nil { return w.decisionOnError, "", err @@ -256,6 +286,18 @@ func (w *WebhookAuthorizer) RulesFor(user user.Info, namespace string) ([]author return resourceRules, nonResourceRules, incomplete, fmt.Errorf("webhook authorizer does not support user rule resolution") } +// Match is used to evaluate the SubjectAccessReviewSpec against +// the authorizer's matchConditions in the form of cel expressions +// to return match or no match found, which then is used to +// determine if the webhook should be skipped. +func (w *WebhookAuthorizer) match(ctx context.Context, r *authorizationv1.SubjectAccessReview) (bool, error) { + // A nil celMatcher or zero saved CompilationResults matches all requests. + if w.celMatcher == nil || w.celMatcher.CompilationResults == nil { + return true, nil + } + return w.celMatcher.Eval(ctx, r) +} + func convertToSARExtra(extra map[string][]string) map[string]authorizationv1.ExtraValue { if extra == nil { return nil diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go index 52fa60b9e8d..ab3d6f3296b 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go @@ -40,10 +40,14 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/apis/apiserver" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" webhookutil "k8s.io/apiserver/pkg/util/webhook" v1 "k8s.io/client-go/tools/clientcmd/api/v1" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) var testRetryBackoff = wait.Backoff{ @@ -205,7 +209,7 @@ current-context: default if err != nil { return fmt.Errorf("error building sar client: %v", err) } - _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, noopAuthorizerMetrics()) + _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, []apiserver.WebhookMatchCondition{}, noopAuthorizerMetrics()) return err }() if err != nil && !tt.wantErr { @@ -318,7 +322,7 @@ func (m *mockV1Service) HTTPStatusCode() int { return m.statusCode } // newV1Authorizer creates a temporary kubeconfig file from the provided arguments and attempts to load // a new WebhookAuthorizer from it. -func newV1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { +func newV1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, metrics AuthorizerMetrics, expressions []apiserver.WebhookMatchCondition) (*WebhookAuthorizer, error) { tempfile, err := ioutil.TempFile("", "") if err != nil { return nil, err @@ -348,7 +352,7 @@ func newV1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, cache if err != nil { return nil, fmt.Errorf("error building sar client: %v", err) } - return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, metrics) + return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, expressions, metrics) } func TestV1TLSConfig(t *testing.T) { @@ -407,7 +411,7 @@ func TestV1TLSConfig(t *testing.T) { } defer server.Close() - wh, err := newV1Authorizer(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, 0, noopAuthorizerMetrics()) + wh, err := newV1Authorizer(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, 0, noopAuthorizerMetrics(), []apiserver.WebhookMatchCondition{}) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -472,7 +476,7 @@ func TestV1Webhook(t *testing.T) { } defer s.Close() - wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics()) + wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics(), []apiserver.WebhookMatchCondition{}) if err != nil { t.Fatal(err) } @@ -572,15 +576,20 @@ func TestV1WebhookCache(t *testing.T) { t.Fatal(err) } defer s.Close() - + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, true)() + expressions := []apiserver.WebhookMatchCondition{ + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + } // Create an authorizer that caches successful responses "forever" (100 days). - wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 2400*time.Hour, noopAuthorizerMetrics()) + wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 2400*time.Hour, noopAuthorizerMetrics(), expressions) if err != nil { t.Fatal(err) } - aliceAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "alice"}} - bobAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "bob"}} + aliceAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "alice"}, ResourceRequest: true, Namespace: "kittensandponies"} + bobAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "bob"}, ResourceRequest: true, Namespace: "kittensandponies"} aliceRidiculousAttr := authorizer.AttributesRecord{ User: &user.DefaultInfo{Name: "alice"}, ResourceRequest: true, @@ -589,6 +598,7 @@ func TestV1WebhookCache(t *testing.T) { APIVersion: strings.Repeat("a", 2000), Resource: strings.Repeat("r", 2000), Name: strings.Repeat("n", 2000), + Namespace: "kittensandponies", } bobRidiculousAttr := authorizer.AttributesRecord{ User: &user.DefaultInfo{Name: "bob"}, @@ -598,6 +608,7 @@ func TestV1WebhookCache(t *testing.T) { APIVersion: strings.Repeat("a", 2000), Resource: strings.Repeat("r", 2000), Name: strings.Repeat("n", 2000), + Namespace: "kittensandponies", } type webhookCacheTestCase struct { @@ -665,6 +676,404 @@ func TestV1WebhookCache(t *testing.T) { } } +// TestStructuredAuthzConfigFeatureEnablement verifies cel expressions can only be used when feature is enabled +func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { + + service := new(mockV1Service) + service.statusCode = 200 + service.Allow() + s, err := NewV1TestServer(service, serverCert, serverKey, caCert) + if err != nil { + t.Fatal(err) + } + defer s.Close() + + type webhookMatchConditionsTestCase struct { + name string + attr authorizer.AttributesRecord + allow bool + expectedCompileErr bool + expectedEvalErr bool + expectedDecision authorizer.Decision + expressions []apiserver.WebhookMatchCondition + featureEnabled bool + } + aliceAttr := authorizer.AttributesRecord{ + User: &user.DefaultInfo{ + Name: "alice", + UID: "1", + Groups: []string{"group1", "group2"}, + }, + ResourceRequest: true, + Namespace: "kittensandponies", + Verb: "get", + } + tests := []webhookMatchConditionsTestCase{ + { + name: "no match condition does not require feature enablement", + attr: aliceAttr, + allow: true, + expectedCompileErr: false, + expectedDecision: authorizer.DecisionAllow, + expressions: []apiserver.WebhookMatchCondition{}, + featureEnabled: false, + }, + { + name: "should fail when match conditions are used without feature enabled", + attr: aliceAttr, + allow: false, + expectedCompileErr: true, + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + }, + featureEnabled: false, + }, + { + name: "feature enabled, match all against all expressions", + attr: aliceAttr, + allow: true, + expectedCompileErr: false, + expectedDecision: authorizer.DecisionAllow, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "request.uid == '1'", + }, + { + Expression: "('group1' in request.groups)", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + featureEnabled: true, + }, + } + + for i, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, test.featureEnabled)() + wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics(), test.expressions) + if test.expectedCompileErr && err == nil { + t.Fatalf("%d: Expected compile error", i) + } else if !test.expectedCompileErr && err != nil { + t.Fatalf("%d: unexpected error when creating a new WebhookAuthorizer: %v", i, err) + } + if err == nil { + authorized, _, err := wh.Authorize(context.Background(), test.attr) + if test.expectedEvalErr && err == nil { + t.Fatalf("%d: Expected eval error", i) + } else if !test.expectedEvalErr && err != nil { + t.Fatalf("%d: unexpected error when authorizing: %v", i, err) + } + + if test.expectedDecision != authorized { + t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedDecision, authorized) + } + } + }) + } +} + +// TestV1WebhookMatchConditions verifies cel expressions are compiled and evaluated correctly +func TestV1WebhookMatchConditions(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, true)() + service := new(mockV1Service) + service.statusCode = 200 + service.Allow() + s, err := NewV1TestServer(service, serverCert, serverKey, caCert) + if err != nil { + t.Fatal(err) + } + defer s.Close() + + aliceAttr := authorizer.AttributesRecord{ + User: &user.DefaultInfo{ + Name: "alice", + UID: "1", + Groups: []string{"group1", "group2"}, + }, + ResourceRequest: true, + Namespace: "kittensandponies", + Verb: "get", + } + bobAttr := authorizer.AttributesRecord{ + User: &user.DefaultInfo{ + Name: "bob", + }, + ResourceRequest: false, + Namespace: "kittensandponies", + Verb: "get", + } + alice2Attr := authorizer.AttributesRecord{ + User: &user.DefaultInfo{ + Name: "alice2", + }, + } + type webhookMatchConditionsTestCase struct { + name string + attr authorizer.AttributesRecord + expectedCompileErr string + expectedEvalErr string + expectedDecision authorizer.Decision + expressions []apiserver.WebhookMatchCondition + } + + tests := []webhookMatchConditionsTestCase{ + { + name: "match all with no expressions", + attr: aliceAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionAllow, + expressions: []apiserver.WebhookMatchCondition{}, + }, + { + name: "match all against all expressions", + attr: aliceAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionAllow, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "request.uid == '1'", + }, + { + Expression: "('group1' in request.groups)", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "match all except group, eval to one successful false, no error", + attr: aliceAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expectedEvalErr: "", + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "request.uid == '1'", + }, + { + Expression: "('group3' in request.groups)", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "match condition with one compilation error", + attr: aliceAttr, + expectedCompileErr: "matchConditions[2].expression: Invalid value: \"('group3' in request.group)\": compilation failed: ERROR: :1:21: undefined field 'group'\n | ('group3' in request.group)\n | ....................^", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "request.uid == '1'", + }, + { + Expression: "('group3' in request.group)", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "match all except uid", + attr: aliceAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "request.uid == '2'", + }, + { + Expression: "('group1' in request.groups)", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "match on user name but not namespace", + attr: aliceAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kube-system'", + }, + }, + }, + { + name: "mismatch on user name", + attr: bobAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice'", + }, + }, + }, + { + name: "match on user name but not resourceAttributes", + attr: bobAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'bob'", + }, + { + Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "expression failed to compile due to wrong return type", + attr: bobAttr, + expectedCompileErr: `matchConditions[0].expression: Invalid value: "request.user": must evaluate to bool but got string`, + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user", + }, + }, + }, + { + name: "eval failed due to errors, no successful fail", + attr: alice2Attr, + expectedCompileErr: "", + expectedEvalErr: "cel evaluation error: expression 'request.resourceAttributes.namespace == 'kittensandponies'' resulted in error: no such key: resourceAttributes", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'alice2'", + }, + { + Expression: "request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "at least one matchCondition successfully evaluates to FALSE, error ignored", + attr: alice2Attr, + expectedCompileErr: "", + expectedEvalErr: "", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user != 'alice2'", + }, + { + Expression: "request.resourceAttributes.namespace == 'kittensandponies'", + }, + }, + }, + { + name: "match on user name but failed to compile due to type check in nonResourceAttributes", + attr: bobAttr, + expectedCompileErr: "matchConditions[1].expression: Invalid value: \"request.nonResourceAttributes.verb == 2\": compilation failed: ERROR: :1:36: found no matching overload for '_==_' applied to '(string, int)'\n | request.nonResourceAttributes.verb == 2\n | ...................................^", + expectedDecision: authorizer.DecisionNoOpinion, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'bob'", + }, + { + Expression: "request.nonResourceAttributes.verb == 2", + }, + }, + }, + { + name: "match on user name and nonresourceAttributes", + attr: bobAttr, + expectedCompileErr: "", + expectedDecision: authorizer.DecisionAllow, + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'bob'", + }, + { + Expression: "has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'", + }, + }, + }, + { + name: "match eval failed with bad SubjectAccessReviewSpec", + attr: authorizer.AttributesRecord{}, + 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]", + expressions: []apiserver.WebhookMatchCondition{ + { + Expression: "request.user == 'bob'", + }, + { + Expression: "has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'", + }, + }, + }, + } + + for i, test := range tests { + t.Run(test.name, func(t *testing.T) { + wh, err := newV1Authorizer(s.URL, clientCert, clientKey, caCert, 0, noopAuthorizerMetrics(), test.expressions) + if len(test.expectedCompileErr) > 0 && err == nil { + t.Fatalf("%d: Expected compile error", i) + } else if len(test.expectedCompileErr) == 0 && err != nil { + t.Fatalf("%d: unexpected error when creating a new WebhookAuthorizer: %v", i, err) + } + if err != nil { + if d := cmp.Diff(test.expectedCompileErr, err.Error()); d != "" { + t.Fatalf("newV1Authorizer mismatch (-want +got):\n%s", d) + } + } + if err == nil { + authorized, _, err := wh.Authorize(context.Background(), test.attr) + if len(test.expectedEvalErr) > 0 && err == nil { + t.Fatalf("%d: Expected eval error", i) + } else if len(test.expectedEvalErr) == 0 && err != nil { + t.Fatalf("%d: unexpected error when authorizing: %v", i, err) + } + + if err != nil { + if d := cmp.Diff(test.expectedEvalErr, err.Error()); d != "" { + t.Fatalf("Authorize mismatch (-want +got):\n%s", d) + } + } + + if test.expectedDecision != authorized { + t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedDecision, authorized) + } + } + }) + } +} + func noopAuthorizerMetrics() AuthorizerMetrics { return AuthorizerMetrics{ RecordRequestTotal: noopMetrics{}.RecordRequestTotal, diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go index 8023a57afe9..8cb6e6716f4 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go @@ -37,6 +37,7 @@ import ( "github.com/google/go-cmp/cmp" authorizationv1beta1 "k8s.io/api/authorization/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + authzconfig "k8s.io/apiserver/pkg/apis/apiserver" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" webhookutil "k8s.io/apiserver/pkg/util/webhook" @@ -195,7 +196,7 @@ current-context: default if err != nil { return fmt.Errorf("error building sar client: %v", err) } - _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, noopAuthorizerMetrics()) + _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) return err }() if err != nil && !tt.wantErr { @@ -338,7 +339,7 @@ func newV1beta1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, if err != nil { return nil, fmt.Errorf("error building sar client: %v", err) } - return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, noopAuthorizerMetrics()) + return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) } func TestV1beta1TLSConfig(t *testing.T) { diff --git a/vendor/modules.txt b/vendor/modules.txt index 91a15692672..043b2e5bf00 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1515,6 +1515,7 @@ k8s.io/apiserver/pkg/authentication/token/union k8s.io/apiserver/pkg/authentication/user k8s.io/apiserver/pkg/authorization/authorizer k8s.io/apiserver/pkg/authorization/authorizerfactory +k8s.io/apiserver/pkg/authorization/cel k8s.io/apiserver/pkg/authorization/path k8s.io/apiserver/pkg/authorization/union k8s.io/apiserver/pkg/cel