diff --git a/authz/providers/azure/README.md b/authz/providers/azure/README.md index 3fa9b4e09..fd44473c5 100644 --- a/authz/providers/azure/README.md +++ b/authz/providers/azure/README.md @@ -229,7 +229,7 @@ If the operation is absent: unregistered action, but `Writer` matches one whenever the parent resource is among its 24 wildcards. -Status of the actions used by the mapping, measured on 2026-08-18: +Status of the actions used by the mapping, measured on 2026-08-26: | Action | Registered | | ------------------------------------------------------------------------------------- | ---------- | @@ -280,6 +280,12 @@ Two traps make this easy to miss: node and controller identities stop there and never reach the mapping. Only a cluster whose kubelet uses an Entra ID identity sends these requests to CheckAccess. +The `--azure.enforce-csr-nodeclient-data-action` rollout flag controls the +`certificatesigningrequests/nodeclient` mapping. It defaults to `false`, which preserves +the legacy `certificatesigningrequests/write` action. Set it to `true` only after the +dedicated operation is published and affected custom roles have migrated. Turning it +back off restores the legacy mapping without rolling back the Guard image. + ### Two different subresource mechanisms Guard handles subresources in two separate places. They use separate lists and serve @@ -615,6 +621,7 @@ AKS_AUTHZ_TOKEN_URL="https://${FQDN}:443/authz/token" | `--azure.discover-resources-frequency` | Discovery refresh interval | 5m | | `--azure.allow-custom-resource-type-check` | Enable CRD support | false | | `--azure.allow-subresource-type-check` | Enable subresource perms | false | +| `--azure.enforce-csr-nodeclient-data-action` | Enforce CSR nodeclient action | false | | `--azure.audit-sar` | Log all SAR requests | false | ### Prometheus Metrics diff --git a/authz/providers/azure/options/fleet_test.go b/authz/providers/azure/options/fleet_test.go index 3c80cc3c6..54cfa48df 100644 --- a/authz/providers/azure/options/fleet_test.go +++ b/authz/providers/azure/options/fleet_test.go @@ -19,9 +19,20 @@ package options import ( "testing" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" ) +func TestEnforceCSRNodeClientDataActionFlag(t *testing.T) { + opts := NewOptions() + assert.False(t, opts.EnforceCSRNodeClientDataAction) + + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + opts.AddFlags(flags) + assert.NoError(t, flags.Parse([]string{"--azure.enforce-csr-nodeclient-data-action=true"})) + assert.True(t, opts.EnforceCSRNodeClientDataAction) +} + func TestValidateFleetID(t *testing.T) { tests := []struct { name string diff --git a/authz/providers/azure/options/options.go b/authz/providers/azure/options/options.go index 63cc4c4d9..366cfee46 100644 --- a/authz/providers/azure/options/options.go +++ b/authz/providers/azure/options/options.go @@ -54,6 +54,7 @@ type Options struct { AllowNonResDiscoveryPathAccess bool AllowCustomResourceTypeCheck bool AllowSubresourceTypeCheck bool + EnforceCSRNodeClientDataAction bool UseNamespaceResourceScopeFormat bool DiscoverResources bool UseManagedNamespaceResourceScopeFormat bool @@ -85,6 +86,7 @@ func NewOptions() Options { AllowNonResDiscoveryPathAccess: true, AllowCustomResourceTypeCheck: false, AllowSubresourceTypeCheck: false, + EnforceCSRNodeClientDataAction: false, UseNamespaceResourceScopeFormat: false, DiscoverResources: false, ReconcileDiscoverResourcesFrequency: 5 * time.Minute, @@ -109,6 +111,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { fs.BoolVar(&o.AllowNonResDiscoveryPathAccess, "azure.allow-nonres-discovery-path-access", o.AllowNonResDiscoveryPathAccess, "allow access on Non Resource paths required for discovery, setting it false will require explicit non resource path role assignment for all users in Azure RBAC") fs.BoolVar(&o.AllowCustomResourceTypeCheck, "azure.allow-custom-resource-type-check", o.AllowCustomResourceTypeCheck, "allow custom resource type checks for authorization") fs.BoolVar(&o.AllowSubresourceTypeCheck, "azure.allow-subresource-type-check", o.AllowSubresourceTypeCheck, "allow subresource type checks for authorization") + fs.BoolVar(&o.EnforceCSRNodeClientDataAction, "azure.enforce-csr-nodeclient-data-action", o.EnforceCSRNodeClientDataAction, "map CSR nodeclient authorization checks to the dedicated nodeclient DataAction") fs.BoolVar(&o.UseNamespaceResourceScopeFormat, "azure.use-ns-resource-scope-format", o.UseNamespaceResourceScopeFormat, "use namespace as resource scope format for making rbac checkaccess calls at namespace scope") fs.StringVar(&o.KubeConfigFile, "azure.kubeconfig-file", "", "path to the kubeconfig of cluster.") fs.BoolVar(&o.UseManagedNamespaceResourceScopeFormat, "azure.use-managed-namespace-resource-scope-format", o.UseManagedNamespaceResourceScopeFormat, "enable managed namespace RBAC for azure authz mode") @@ -218,6 +221,8 @@ func (o Options) Apply(d *apps.Deployment) (extraObjs []runtime.Object, err erro args = append(args, fmt.Sprintf("--azure.allow-subresource-type-check=%t", o.AllowSubresourceTypeCheck)) + args = append(args, fmt.Sprintf("--azure.enforce-csr-nodeclient-data-action=%t", o.EnforceCSRNodeClientDataAction)) + d.Spec.Template.Spec.Containers[0].Args = args return extraObjs, nil } diff --git a/authz/providers/azure/rbac/checkaccess_v2.go b/authz/providers/azure/rbac/checkaccess_v2.go index b72c64faf..d26dcc6d8 100644 --- a/authz/providers/azure/rbac/checkaccess_v2.go +++ b/authz/providers/azure/rbac/checkaccess_v2.go @@ -278,7 +278,7 @@ func (a *AccessInfo) checkAccessV2(ctx context.Context, request *authzv1.Subject log.V(7).Info("Extracted user identity for v2", "userOid", userOid, "groupsCount", len(groups)) // Prepare actions list from request (same logic as v1 but get action IDs) - actions, err := getDataActionsV2(ctx, request, a.clusterType, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck) + actions, err := getDataActionsV2(ctx, request, a.clusterType, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) if err != nil { return nil, fmt.Errorf("error preparing v2 actions list: %w", err) } @@ -328,7 +328,7 @@ func (a *AccessInfo) checkAccessV2(ctx context.Context, request *authzv1.Subject // Generate fleet-specific actions using fleetMembers cluster type // This ensures actions like "Microsoft.ContainerService/fleets/members/pods/read" // are used instead of "Microsoft.ContainerService/managedClusters/pods/read" - fleetMemberActions, err := getDataActionsV2(ctx, request, fleetMembers, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck) + fleetMemberActions, err := getDataActionsV2(ctx, request, fleetMembers, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) if err != nil { return nil, fmt.Errorf("error preparing fleet actions list: %w", err) } @@ -372,8 +372,8 @@ func (a *AccessInfo) checkAccessV2(ctx context.Context, request *authzv1.Subject // IsDataAction and any subresource attributes are preserved for the PDP request // (see buildAuthorizationRequestV2 / toActionInfos). Dropping IsDataAction makes // PDP evaluate Kubernetes RBAC actions as management actions and deny every check. -func getDataActionsV2(ctx context.Context, request *authzv1.SubjectAccessReviewSpec, clusterType string, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool) ([]azureutils.AuthorizationActionInfo, error) { - return getDataActions(ctx, request, clusterType, allowCustomResourceTypeCheck, allowSubresourceTypeCheck) +func getDataActionsV2(ctx context.Context, request *authzv1.SubjectAccessReviewSpec, clusterType string, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool, enforceCSRNodeClientDataAction bool) ([]azureutils.AuthorizationActionInfo, error) { + return getDataActions(ctx, request, clusterType, allowCustomResourceTypeCheck, allowSubresourceTypeCheck, enforceCSRNodeClientDataAction) } // buildResourceIDForV2 constructs and validates a resource ID for CheckAccess v2 API. diff --git a/authz/providers/azure/rbac/checkaccess_v2_test.go b/authz/providers/azure/rbac/checkaccess_v2_test.go index e8b04d239..cb23b8589 100644 --- a/authz/providers/azure/rbac/checkaccess_v2_test.go +++ b/authz/providers/azure/rbac/checkaccess_v2_test.go @@ -251,7 +251,7 @@ func TestGetDataActionsV2_Success(t *testing.T) { }, } - actions, err := getDataActionsV2(ctx, request, managedClusters, false, false) + actions, err := getDataActionsV2(ctx, request, managedClusters, false, false, false) assert.NoError(t, err) assert.NotEmpty(t, actions) @@ -260,6 +260,25 @@ func TestGetDataActionsV2_Success(t *testing.T) { assert.True(t, actions[0].IsDataAction) } +func TestGetDataActionsV2_CSRNodeClientEnforced(t *testing.T) { + request := &authzv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: "certificates.k8s.io", + Verb: "create", + Resource: "certificatesigningrequests", + Subresource: "nodeclient", + }, + } + + actions, err := getDataActionsV2(context.Background(), request, managedClusters, false, false, true) + + assert.NoError(t, err) + if assert.Len(t, actions, 1) { + assert.Equal(t, "Microsoft.ContainerService/managedClusters/certificates.k8s.io/certificatesigningrequests/nodeclient/action", actions[0].Id) + assert.True(t, actions[0].IsDataAction) + } +} + func TestPerformCheckAccessV2_Success(t *testing.T) { mockClient := &mockPDPClient{ checkAccessFunc: func(ctx context.Context, authzReq checkaccess.AuthorizationRequest) (*checkaccess.AuthorizationDecisionResponse, error) { diff --git a/authz/providers/azure/rbac/checkaccessreqhelper.go b/authz/providers/azure/rbac/checkaccessreqhelper.go index 3d99f7222..e538326b5 100644 --- a/authz/providers/azure/rbac/checkaccessreqhelper.go +++ b/authz/providers/azure/rbac/checkaccessreqhelper.go @@ -52,12 +52,14 @@ const ( ServicesResource = "services" NodesResource = "nodes" ServiceAccountsResource = "serviceaccounts" + CSRResource = "certificatesigningrequests" CustomResources = "customresources" ProxySubresource = "proxy" AttachSubresource = "attach" PortForwardSubresource = "portforward" ExecSubresource = "exec" TokenSubresource = "token" + NodeClientSubresource = "nodeclient" ReadVerb = "read" WriteVerb = "write" DeleteVerb = "delete" @@ -277,6 +279,11 @@ func getActionName(verb string) string { // object. Collapsing it into serviceaccounts/write would grant token issuance // to every principal that can create or update ServiceAccount objects, which // upstream Kubernetes RBAC does not do. +// +// certificatesigningrequests/nodeclient is the authorization gate used by the +// upstream CSR approver before issuing a kubelet client certificate. Collapsing +// it into certificatesigningrequests/write would let general CSR writers pass +// this credential-issuance check. var securitySensitiveSubresources = map[string]map[string]struct{}{ PodsResource: { ExecSubresource: {}, @@ -293,18 +300,33 @@ var securitySensitiveSubresources = map[string]map[string]struct{}{ ServiceAccountsResource: { TokenSubresource: {}, }, + CSRResource: { + NodeClientSubresource: {}, + }, } -func getResourceAndAction(resource string, subResource string, verb string) string { - if subs, ok := securitySensitiveSubresources[resource]; ok && subResource != "" { - if _, sensitive := subs[subResource]; sensitive { - return path.Join(resource, subResource, "action") - } +func getResourceAndAction(resource string, subResource string, verb string, enforceCSRNodeClientDataAction bool) string { + if shouldPreserveSubresource(resource, subResource, enforceCSRNodeClientDataAction) { + return path.Join(resource, subResource, "action") } return path.Join(resource, getActionName(verb)) } -func getDataActions(ctx context.Context, subRevReq *authzv1.SubjectAccessReviewSpec, clusterType string, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool) ([]azureutils.AuthorizationActionInfo, error) { +func shouldPreserveSubresource(resource string, subResource string, enforceCSRNodeClientDataAction bool) bool { + if resource == CSRResource && subResource == NodeClientSubresource { + return enforceCSRNodeClientDataAction + } + + subResources, ok := securitySensitiveSubresources[resource] + if !ok { + return false + } + + _, ok = subResources[subResource] + return ok +} + +func getDataActions(ctx context.Context, subRevReq *authzv1.SubjectAccessReviewSpec, clusterType string, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool, enforceCSRNodeClientDataAction bool) ([]azureutils.AuthorizationActionInfo, error) { var authInfoList []azureutils.AuthorizationActionInfo var err error log := klog.FromContext(ctx) @@ -345,7 +367,7 @@ func getDataActions(ctx context.Context, subRevReq *authzv1.SubjectAccessReviewS authInfoSingle.AuthorizationEntity.Id = path.Join(authInfoSingle.AuthorizationEntity.Id, subRevReq.ResourceAttributes.Group) } - action := getResourceAndAction(subRevReq.ResourceAttributes.Resource, subRevReq.ResourceAttributes.Subresource, subRevReq.ResourceAttributes.Verb) + action := getResourceAndAction(subRevReq.ResourceAttributes.Resource, subRevReq.ResourceAttributes.Subresource, subRevReq.ResourceAttributes.Verb, enforceCSRNodeClientDataAction) authInfoSingle.AuthorizationEntity.Id = path.Join(authInfoSingle.AuthorizationEntity.Id, action) if allowSubresourceTypeCheck { err = setAuthInfoSubresourceAttributes(&authInfoSingle, subRevReq) @@ -648,13 +670,13 @@ func defaultDir(s string) string { return "-" // invalid for a namespace } -func getResultCacheKey(subRevReq *authzv1.SubjectAccessReviewSpec, allowSubresourceTypeCheck bool) string { +func getResultCacheKey(subRevReq *authzv1.SubjectAccessReviewSpec, allowSubresourceTypeCheck bool, enforceCSRNodeClientDataAction bool) string { cacheKey := subRevReq.User if subRevReq.ResourceAttributes != nil { cacheKey = path.Join(cacheKey, defaultDir(subRevReq.ResourceAttributes.Namespace)) cacheKey = path.Join(cacheKey, defaultDir(subRevReq.ResourceAttributes.Group)) - action := getResourceAndAction(subRevReq.ResourceAttributes.Resource, subRevReq.ResourceAttributes.Subresource, subRevReq.ResourceAttributes.Verb) + action := getResourceAndAction(subRevReq.ResourceAttributes.Resource, subRevReq.ResourceAttributes.Subresource, subRevReq.ResourceAttributes.Verb, enforceCSRNodeClientDataAction) cacheKey = path.Join(cacheKey, action) // Cache results for subresources of interest separately @@ -670,7 +692,7 @@ func getResultCacheKey(subRevReq *authzv1.SubjectAccessReviewSpec, allowSubresou return cacheKey } -func prepareCheckAccessRequestBody(ctx context.Context, req *authzv1.SubjectAccessReviewSpec, clusterType string, resourceId string, useNamespaceResourceScopeFormat bool, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool) ([]*CheckAccessRequest, error) { +func prepareCheckAccessRequestBody(ctx context.Context, req *authzv1.SubjectAccessReviewSpec, clusterType string, resourceId string, useNamespaceResourceScopeFormat bool, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool, enforceCSRNodeClientDataAction bool) ([]*CheckAccessRequest, error) { /* This is how sample SubjectAccessReview request will look like { "kind": "SubjectAccessReview", @@ -733,7 +755,7 @@ func prepareCheckAccessRequestBody(ctx context.Context, req *authzv1.SubjectAcce return nil, errutils.WithCode(fmt.Errorf("oid info not sent from authentication module"), http.StatusBadRequest) } groups := getValidSecurityGroups(req.Groups) - actions, err := getDataActions(ctx, req, clusterType, allowCustomResourceTypeCheck, allowSubresourceTypeCheck) + actions, err := getDataActions(ctx, req, clusterType, allowCustomResourceTypeCheck, allowSubresourceTypeCheck, enforceCSRNodeClientDataAction) if err != nil { return nil, errutils.WithCode(fmt.Errorf("Error while creating list of dataactions for check access call: %w", err), http.StatusInternalServerError) } diff --git a/authz/providers/azure/rbac/checkaccessreqhelper_test.go b/authz/providers/azure/rbac/checkaccessreqhelper_test.go index 88fb41f6d..de6eb4df6 100644 --- a/authz/providers/azure/rbac/checkaccessreqhelper_test.go +++ b/authz/providers/azure/rbac/checkaccessreqhelper_test.go @@ -175,11 +175,12 @@ func Test_getValidSecurityGroups(t *testing.T) { func Test_getDataActions(t *testing.T) { type args struct { - isCrTest bool - isSubresTest bool - isWildcardTest bool - subRevReq *authzv1.SubjectAccessReviewSpec - clusterType string + isCrTest bool + isSubresTest bool + isWildcardTest bool + enforceCSRNodeClientDataAction bool + subRevReq *authzv1.SubjectAccessReviewSpec + clusterType string } tests := []struct { name string @@ -568,6 +569,41 @@ func Test_getDataActions(t *testing.T) { []azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/pods/read"}, IsDataAction: true}}, }, + { + "csrNodeclientAKS", + args{ + isWildcardTest: false, + enforceCSRNodeClientDataAction: true, + subRevReq: &authzv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authzv1.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Subresource: "nodeclient", Version: "v1", Verb: "create"}, + }, clusterType: aksClusterType, + }, + []azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/certificates.k8s.io/certificatesigningrequests/nodeclient/action"}, IsDataAction: true}}, + }, + + { + "csrNodeclientFleet", + args{ + isWildcardTest: false, + enforceCSRNodeClientDataAction: true, + subRevReq: &authzv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authzv1.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Subresource: "nodeclient", Version: "v1", Verb: "create"}, + }, clusterType: "fleet", + }, + []azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "fleet/certificates.k8s.io/certificatesigningrequests/nodeclient/action"}, IsDataAction: true}}, + }, + + { + "csrNodeclientLegacyMapping", + args{ + isWildcardTest: false, + subRevReq: &authzv1.SubjectAccessReviewSpec{ + ResourceAttributes: &authzv1.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Subresource: "nodeclient", Version: "v1", Verb: "create"}, + }, clusterType: aksClusterType, + }, + []azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/certificates.k8s.io/certificatesigningrequests/write"}, IsDataAction: true}}, + }, + // CSR with a non-sensitive subresource (e.g. approval, status) // should still collapse to the base action, not preserve the subresource. { @@ -837,7 +873,7 @@ func Test_getDataActions(t *testing.T) { setStoredOperationsMap(t, createOperationsMap(tt.args.clusterType)) ctx := context.Background() - got, _ := getDataActions(ctx, tt.args.subRevReq, tt.args.clusterType, tt.args.isCrTest, tt.args.isSubresTest) + got, _ := getDataActions(ctx, tt.args.subRevReq, tt.args.clusterType, tt.args.isCrTest, tt.args.isSubresTest, tt.args.enforceCSRNodeClientDataAction) if !tt.args.isWildcardTest { if !reflect.DeepEqual(got[0].AuthorizationEntity, tt.want[0].AuthorizationEntity) { t.Errorf("getDataActions() = %v, want %v", got, tt.want) @@ -916,7 +952,7 @@ func Test_getDataActions_wildcardWithEmptyOperationsMap(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - got, err := getDataActions(ctx, tt.spec, "Microsoft.ContainerService/managedClusters", false, false) + got, err := getDataActions(ctx, tt.spec, "Microsoft.ContainerService/managedClusters", false, false, false) assert.Nil(t, got, "expected nil actions for wildcard with empty operations map") assert.Error(t, err) @@ -1000,7 +1036,7 @@ func Test_prepareCheckAccessRequestBody(t *testing.T) { wantErr := errors.New("oid info not sent from authenticatoin module") ctx := context.Background() - got, gotErr := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, gotErr := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got != nil && gotErr != wantErr { t.Errorf("Want:%v WantErr:%v, got:%v, gotErr:%v", nil, wantErr, got, gotErr) @@ -1010,7 +1046,7 @@ func Test_prepareCheckAccessRequestBody(t *testing.T) { clusterType = "arc" wantErr = errors.New("oid info sent from authenticatoin module is not valid") - got, gotErr = prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, gotErr = prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got != nil && gotErr != wantErr { t.Errorf("Want:%v WantErr:%v, got:%v, gotErr:%v", nil, wantErr, got, gotErr) @@ -1027,7 +1063,7 @@ func Test_prepareCheckAccessRequestBodyWithNamespace(t *testing.T) { var want string = "resourceId/providers/Microsoft.KubernetesConfiguration/namespaces/dev" ctx := context.Background() - got, gotErr := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, true, true, false) + got, gotErr := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, true, true, false, false) if got == nil { t.Errorf("Want: not nil Got: nil, gotErr:%v", gotErr) @@ -1040,7 +1076,7 @@ func Test_prepareCheckAccessRequestBodyWithNamespace(t *testing.T) { // testing with the old namespace format want = "resourceId/namespaces/dev" - got, gotErr = prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, gotErr = prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got == nil { t.Errorf("Want: not nil Got: nil, gotErr:%v", gotErr) } @@ -1070,7 +1106,7 @@ func Test_prepareCheckAccessRequestBodyWithCustomResource(t *testing.T) { setStoredOperationsMap(t, createOperationsMap(clusterType)) ctx := context.Background() - got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1110,7 +1146,7 @@ func Test_prepareCheckAccessRequestBodyWithCustomResourceOperationsMapEmpty(t *t setStoredOperationsMap(t, azureutils.OperationsMap{}) ctx := context.Background() - got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1142,7 +1178,7 @@ func Test_prepareCheckAccessRequestBodyWithCustomResourceTypeCheckDisabled(t *te setStoredOperationsMap(t, operationsMap) ctx := context.Background() - got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, false, false) + got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, false, false, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1174,7 +1210,7 @@ func Test_prepareCheckAccessRequestBodyWithCustomResourceAndStars(t *testing.T) setStoredOperationsMap(t, operationsMap) ctx := context.Background() - got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false) + got, _ := prepareCheckAccessRequestBody(ctx, req, clusterType, resourceId, false, true, false, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1272,7 +1308,7 @@ func Test_prepareCheckAccessRequestBodyWithFleetMembers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - got, gotErr := prepareCheckAccessRequestBody(ctx, tt.req, tt.clusterType, tt.resourceID, false, false, false) + got, gotErr := prepareCheckAccessRequestBody(ctx, tt.req, tt.clusterType, tt.resourceID, false, false, false, false) if gotErr != nil { t.Errorf("Unexpected error: %v", gotErr) @@ -1334,7 +1370,7 @@ func Test_prepareCheckAccessRequestBodyWithSubresource(t *testing.T) { clusterType := aksClusterType createOperationsMap(clusterType) - got, _ := prepareCheckAccessRequestBody(context.Background(), req, clusterType, resourceId, false, false, true) + got, _ := prepareCheckAccessRequestBody(context.Background(), req, clusterType, resourceId, false, false, true, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1374,7 +1410,7 @@ func Test_prepareCheckAccessRequestBodyWithSubresourceDisabled(t *testing.T) { clusterType := aksClusterType createOperationsMap(clusterType) - got, _ := prepareCheckAccessRequestBody(context.Background(), req, clusterType, resourceId, false, false, false) + got, _ := prepareCheckAccessRequestBody(context.Background(), req, clusterType, resourceId, false, false, false, false) if got == nil { t.Errorf("Want: not nil Got: nil") @@ -1391,8 +1427,9 @@ func Test_prepareCheckAccessRequestBodyWithSubresourceDisabled(t *testing.T) { func Test_getResultCacheKey(t *testing.T) { type args struct { - subRevReq *authzv1.SubjectAccessReviewSpec - allowSubresourceTypeCheck bool + subRevReq *authzv1.SubjectAccessReviewSpec + allowSubresourceTypeCheck bool + enforceCSRNodeClientDataAction bool } tests := []struct { name string @@ -1514,6 +1551,21 @@ func Test_getResultCacheKey(t *testing.T) { "beta@msn.com/-/-/nodes/read", }, + { + "csrNodeclientEnforced", + args{ + subRevReq: &authzv1.SubjectAccessReviewSpec{ + User: "node@example.com", + ResourceAttributes: &authzv1.ResourceAttributes{ + Group: "certificates.k8s.io", Resource: "certificatesigningrequests", + Subresource: "nodeclient", Version: "v1", Verb: "create", + }, + }, + enforceCSRNodeClientDataAction: true, + }, + "node@example.com/-/certificates.k8s.io/certificatesigningrequests/nodeclient/action", + }, + { "allStar", args{ @@ -1546,7 +1598,7 @@ func Test_getResultCacheKey(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := getResultCacheKey(tt.args.subRevReq, tt.args.allowSubresourceTypeCheck); got != tt.want { + if got := getResultCacheKey(tt.args.subRevReq, tt.args.allowSubresourceTypeCheck, tt.args.enforceCSRNodeClientDataAction); got != tt.want { t.Errorf("getResultCacheKey() = %v, want %v", got, tt.want) } }) diff --git a/authz/providers/azure/rbac/rbac.go b/authz/providers/azure/rbac/rbac.go index 78fd4709d..0e78e5dd4 100644 --- a/authz/providers/azure/rbac/rbac.go +++ b/authz/providers/azure/rbac/rbac.go @@ -108,6 +108,7 @@ type AccessInfo struct { allowNonResDiscoveryPathAccess bool allowCustomResourceTypeCheck bool allowSubresourceTypeCheck bool + enforceCSRNodeClientDataAction bool useManagedNamespaceResourceScopeFormat bool useNamespaceResourceScopeFormat bool httpClientRetryCount int @@ -200,6 +201,7 @@ func newAccessInfo(tokenProvider graph.TokenProvider, rbacURL *url.URL, opts aut allowNonResDiscoveryPathAccess: opts.AllowNonResDiscoveryPathAccess, allowCustomResourceTypeCheck: opts.AllowCustomResourceTypeCheck, allowSubresourceTypeCheck: opts.AllowSubresourceTypeCheck, + enforceCSRNodeClientDataAction: opts.EnforceCSRNodeClientDataAction, useManagedNamespaceResourceScopeFormat: opts.UseManagedNamespaceResourceScopeFormat, useNamespaceResourceScopeFormat: opts.UseNamespaceResourceScopeFormat, httpClientRetryCount: authopts.HttpClientRetryCount, @@ -315,7 +317,7 @@ func (a *AccessInfo) ShouldSkipAuthzCheckForNonAADUsers() bool { func (a *AccessInfo) GetResultFromCache(ctx context.Context, request *authzv1.SubjectAccessReviewSpec, store authz.Store) (bool, CacheResult) { log := klog.FromContext(ctx) var result CacheResult - key := getResultCacheKey(request, a.allowSubresourceTypeCheck) + key := getResultCacheKey(request, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) log.V(10).Info("Cache search", "key", key) found, err := store.Get(key, &result) if err != nil { @@ -348,7 +350,7 @@ func (a *AccessInfo) SkipAuthzCheck(request *authzv1.SubjectAccessReviewSpec) bo func (a *AccessInfo) SetResultInCache(ctx context.Context, request *authzv1.SubjectAccessReviewSpec, result CacheResult, store authz.Store) error { log := klog.FromContext(ctx) - key := getResultCacheKey(request, a.allowSubresourceTypeCheck) + key := getResultCacheKey(request, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) log.V(10).Info("Cache set", "key", key, "allowed", result.Allowed, "reason", result.Reason) return store.Set(key, result) } @@ -491,7 +493,7 @@ func (a *AccessInfo) CheckAccess(ctx context.Context, request *authzv1.SubjectAc // V1 API path (legacy) log.Info("Using CheckAccess v1 API") - checkAccessBodies, err := prepareCheckAccessRequestBody(ctx, request, a.clusterType, a.azureResourceId, a.useNamespaceResourceScopeFormat, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck) + checkAccessBodies, err := prepareCheckAccessRequestBody(ctx, request, a.clusterType, a.azureResourceId, a.useNamespaceResourceScopeFormat, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) if err != nil { return nil, fmt.Errorf("error in preparing check access request: %w", err) } @@ -550,7 +552,7 @@ func (a *AccessInfo) CheckAccess(ctx context.Context, request *authzv1.SubjectAc return nil, fmt.Errorf("Failed to build fleet manager check access URL: %w", err) } - bodiesForFleetRBAC, err := prepareCheckAccessRequestBody(ctx, request, fleetMembers, a.fleetManagerResourceId, false, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck) + bodiesForFleetRBAC, err := prepareCheckAccessRequestBody(ctx, request, fleetMembers, a.fleetManagerResourceId, false, a.allowCustomResourceTypeCheck, a.allowSubresourceTypeCheck, a.enforceCSRNodeClientDataAction) if err != nil { return nil, fmt.Errorf("Failed to prepare check access request for fleet manager: %w", err) }