From 4d30c43494edc876cc1a31433306e6e75385eaf1 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 6 Mar 2023 12:08:53 -0500 Subject: [PATCH] Add integration tests for secondary authz --- .../cel/validatingadmissionpolicy_test.go | 190 +++++++++++++++++- test/integration/authutil/authutil.go | 25 ++- 2 files changed, 204 insertions(+), 11 deletions(-) diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go index 37b027964bc..0620f927e50 100644 --- a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go +++ b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go @@ -27,8 +27,11 @@ import ( "k8s.io/apiextensions-apiserver/test/integration/fixtures" genericfeatures "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/rest" featuregatetesting "k8s.io/component-base/featuregate/testing" + apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/authutil" "k8s.io/kubernetes/test/integration/etcd" "k8s.io/kubernetes/test/integration/framework" @@ -41,6 +44,7 @@ import ( clientset "k8s.io/client-go/kubernetes" v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1" @@ -2050,6 +2054,177 @@ func TestBindingRemoval(t *testing.T) { } } +// Test_ValidateSecondaryAuthorization tests a ValidatingAdmissionPolicy that performs secondary authorization checks +// for both users and service accounts. +func Test_ValidateSecondaryAuthorization(t *testing.T) { + testcases := []struct { + name string + rbac *rbacv1.PolicyRule + expression string + allowed bool + extraAccountFn func(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset + extraAccountRbac *rbacv1.PolicyRule + }{ + { + name: "principal is allowed to create a specific deployment", + rbac: &rbacv1.PolicyRule{ + Verbs: []string{"create"}, + APIGroups: []string{"apps"}, + Resources: []string{"deployments/status"}, + ResourceNames: []string{"charmander"}, + }, + expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('default').namespace('default').name('charmander').check('create').allowed()", + allowed: true, + }, + { + name: "principal is not allowed to create a specific deployment", + expression: "authorizer.group('apps').resource('deployments').subresource('status').namespace('default').name('charmander').check('create').allowed()", + allowed: false, + }, + { + name: "principal is authorized for custom verb on current resource", + rbac: &rbacv1.PolicyRule{ + Verbs: []string{"anthropomorphize"}, + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + }, + expression: "authorizer.requestResource.check('anthropomorphize').allowed()", + allowed: true, + }, + { + name: "principal is not authorized for custom verb on current resource", + expression: "authorizer.requestResource.check('anthropomorphize').allowed()", + allowed: false, + }, + { + name: "serviceaccount is authorized for custom verb on current resource", + extraAccountFn: serviceAccountClient("default", "extra-acct"), + extraAccountRbac: &rbacv1.PolicyRule{ + Verbs: []string{"anthropomorphize"}, + APIGroups: []string{""}, + Resources: []string{"pods"}, + }, + expression: "authorizer.serviceAccount('default', 'extra-acct').group('').resource('pods').check('anthropomorphize').allowed()", + allowed: true, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + clients := map[string]func(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset{ + "user": secondaryAuthorizationUserClient, + "serviceaccount": secondaryAuthorizationServiceAccountClient, + } + + for clientName, clientFn := range clients { + t.Run(clientName, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true)() + server, err := apiservertesting.StartTestServer(t, nil, []string{ + "--enable-admission-plugins", "ValidatingAdmissionPolicy", + "--authorization-mode=RBAC", + "--anonymous-auth", + }, framework.SharedEtcd()) + if err != nil { + t.Fatal(err) + } + defer server.TearDownFn() + + // For test set up such as creating policies, bindings and RBAC rules. + adminClient := clientset.NewForConfigOrDie(server.ClientConfig) + + // Principal is always allowed to create and update namespaces so that the admission requests to test + // authorization expressions can be sent by the principal. + rules := []rbacv1.PolicyRule{{ + Verbs: []string{"create", "update"}, + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + }} + if testcase.rbac != nil { + rules = append(rules, *testcase.rbac) + } + + client := clientFn(t, adminClient, server.ClientConfig, rules) + + if testcase.extraAccountFn != nil { + var extraRules []rbacv1.PolicyRule + if testcase.extraAccountRbac != nil { + extraRules = append(rules, *testcase.extraAccountRbac) + } + testcase.extraAccountFn(t, adminClient, server.ClientConfig, extraRules) + } + + policy := withWaitReadyConstraintAndExpression(withValidations([]admissionregistrationv1alpha1.Validation{ + { + Expression: testcase.expression, + }, + }, withFailurePolicy(admissionregistrationv1alpha1.Fail, withNamespaceMatch(makePolicy("validate-authz"))))) + if _, err := adminClient.AdmissionregistrationV1alpha1().ValidatingAdmissionPolicies().Create(context.TODO(), policy, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + if err := createAndWaitReady(t, adminClient, makeBinding("validate-authz-binding", "validate-authz", ""), nil); err != nil { + t.Fatal(err) + } + + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-authz", + }, + } + _, err = client.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{}) + + var expected metav1.StatusReason = "" + if !testcase.allowed { + expected = metav1.StatusReasonInvalid + } + checkFailureReason(t, err, expected) + }) + } + }) + } +} + +type clientFn func(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset + +func secondaryAuthorizationUserClient(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset { + clientConfig = rest.CopyConfig(clientConfig) + clientConfig.Impersonate = rest.ImpersonationConfig{ + UserName: "alice", + UID: "1234", + } + client := clientset.NewForConfigOrDie(clientConfig) + + for _, rule := range rules { + authutil.GrantUserAuthorization(t, context.TODO(), adminClient, "alice", rule) + } + return client +} + +func secondaryAuthorizationServiceAccountClient(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset { + return serviceAccountClient("default", "test-service-acct")(t, adminClient, clientConfig, rules) +} + +func serviceAccountClient(namespace, name string) clientFn { + return func(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset { + clientConfig = rest.CopyConfig(clientConfig) + sa, err := adminClient.CoreV1().ServiceAccounts(namespace).Create(context.TODO(), &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: name}}, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + uid := sa.UID + + clientConfig.Impersonate = rest.ImpersonationConfig{ + UserName: "system:serviceaccount:" + namespace + ":" + name, + UID: string(uid), + } + client := clientset.NewForConfigOrDie(clientConfig) + + for _, rule := range rules { + authutil.GrantServiceAccountAuthorization(t, context.TODO(), adminClient, name, namespace, rule) + } + return client + } +} + func withWaitReadyConstraintAndExpression(policy *admissionregistrationv1alpha1.ValidatingAdmissionPolicy) *admissionregistrationv1alpha1.ValidatingAdmissionPolicy { policy = policy.DeepCopy() policy.Spec.MatchConstraints.ResourceRules = append(policy.Spec.MatchConstraints.ResourceRules, admissionregistrationv1alpha1.NamedRuleWithOperations{ @@ -2280,11 +2455,16 @@ func checkFailureReason(t *testing.T, err error, expectedReason metav1.StatusRea // no reason was given, no error was passed - early exit return } - reason := err.(apierrors.APIStatus).Status().Reason - if reason != expectedReason { - t.Logf("actual error reason: %v", reason) - t.Logf("expected failure reason: %v", expectedReason) - t.Error("unexpected error reason") + switch e := err.(type) { + case apierrors.APIStatus: + reason := e.Status().Reason + if reason != expectedReason { + t.Logf("actual error reason: %v", reason) + t.Logf("expected failure reason: %v", expectedReason) + t.Error("Unexpected error reason") + } + default: + t.Errorf("Unexpected error: %v", err) } } diff --git a/test/integration/authutil/authutil.go b/test/integration/authutil/authutil.go index e605ab201d1..8373de2360f 100644 --- a/test/integration/authutil/authutil.go +++ b/test/integration/authutil/authutil.go @@ -64,6 +64,14 @@ func WaitForNamedAuthorizationUpdate(t *testing.T, ctx context.Context, c author } func GrantUserAuthorization(t *testing.T, ctx context.Context, adminClient clientset.Interface, username string, rule rbacv1.PolicyRule) { + grantAuthorization(t, ctx, adminClient, username, "", rbacv1.UserKind, rule) +} + +func GrantServiceAccountAuthorization(t *testing.T, ctx context.Context, adminClient clientset.Interface, serviceAccountName, serviceAccountNamespace string, rule rbacv1.PolicyRule) { + grantAuthorization(t, ctx, adminClient, serviceAccountName, serviceAccountNamespace, rbacv1.ServiceAccountKind, rule) +} + +func grantAuthorization(t *testing.T, ctx context.Context, adminClient clientset.Interface, name, namespace, accountKind string, rule rbacv1.PolicyRule) { t.Helper() cr, err := adminClient.RbacV1().ClusterRoles().Create(ctx, &rbacv1.ClusterRole{ @@ -83,10 +91,10 @@ func GrantUserAuthorization(t *testing.T, ctx context.Context, adminClient clien ObjectMeta: metav1.ObjectMeta{GenerateName: strings.Replace(t.Name(), "/", "--", -1)}, Subjects: []rbacv1.Subject{ { - Kind: rbacv1.UserKind, - APIGroup: rbacv1.GroupName, - Name: username, - Namespace: "", + Kind: accountKind, + // APIGroup defaults to the appropriate value for both users and service accounts + Name: name, + Namespace: namespace, }, }, RoleRef: rbacv1.RoleRef{ @@ -107,12 +115,17 @@ func GrantUserAuthorization(t *testing.T, ctx context.Context, adminClient clien resourceName = rule.ResourceNames[0] } + subjectName := name + if accountKind == rbacv1.ServiceAccountKind { + subjectName = "system:serviceaccount:" + namespace + ":" + name + } + WaitForNamedAuthorizationUpdate( t, ctx, adminClient.AuthorizationV1(), - username, - "", + subjectName, + namespace, rule.Verbs[0], resourceName, schema.GroupResource{Group: rule.APIGroups[0], Resource: rule.Resources[0]},