Skip to content

Preserve subresource granularity in CheckAccess DataAction mapping #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Artem Kolomeetc (arxhive) merged 1 commit into from
Aug 11, 2026
Merged

Preserve subresource granularity in CheckAccess DataAction mapping #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions authz/providers/azure/rbac/checkaccessreqhelper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ const (
NonAADUserNotAllowedVerdict = "Access denied by Azure RBAC for non AAD users. Configure --azure.skip-authz-for-non-aad-users to enable access. If you are an AAD user, please set Extra:oid parameter for impersonated user in the kubeconfig."
CheckAccessErrorVerdict = "Access denied due to Azure RBAC check failure. Please retry later."
PodsResource = "pods"
ServicesResource = "services"
NodesResource = "nodes"
CustomResources = "customresources"
ProxySubresource = "proxy"
AttachSubresource = "attach"
PortForwardSubresource = "portforward"
ExecSubresource = "exec"
ReadVerb = "read"
WriteVerb = "write"
DeleteVerb = "delete"
Expand Down Expand Up @@ -248,16 +254,40 @@ func getActionName(verb string) string {
}
}

func getResourceAndAction(resource string, subResource string, verb string) string {
var action string
// securitySensitiveSubresources lists resource/subresource pairs that upstream
// Kubernetes treats as distinct authorization targets. For these, the
// subresource is preserved in the DataAction string
// ("<resource>/<subresource>/action") rather than collapsed into the base
// resource action, so the authorization decision keeps the same granularity as
// the upstream Kubernetes RBAC model (see the bootstrappolicy view/edit
// ClusterRoles).
//
// The pods exec/attach/portforward/proxy, services/proxy and nodes/proxy
// subresources are granted separately from base read/write in the upstream
// roles, so they are mapped to their own DataAction rather than to
// <resource>/read or <resource>/write.
var securitySensitiveSubresources = map[string]map[string]struct{}{
PodsResource: {
ExecSubresource: {},
AttachSubresource: {},
PortForwardSubresource: {},
ProxySubresource: {},
},
ServicesResource: {
ProxySubresource: {},
},
NodesResource: {
ProxySubresource: {},
},
}

if resource == PodsResource && subResource == "exec" {
action = path.Join(resource, subResource, "action")
} else {
action = path.Join(resource, getActionName(verb))
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")
}
}

return action
return path.Join(resource, getActionName(verb))
Comment on lines +269 to +290

@enj Mo Khan (enj) Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right approach. It should be inverted to define what is safe (i.e. the status sub-resource), not what is unsafe.

@arxhive Artem Kolomeetc (arxhive) Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and the history of this list makes the point - it grew from exec to nodeclient to proxy to attach/portforward, each time after the gap was found rather than before. Inverted in #25.

The safe set is taken from the upstream view ClusterRole (bootstrappolicy.viewRules), which grants exactly these subresource kinds alongside their parent: status, scale, and pods/log. So safeSubresources = {status, scale, log, logs} and everything else gets //action. Wildcard requests and the special verbs (bind/escalate/use/impersonate) keep their current mapping, since there the verb rather than the subresource identifies the operation.

Two consequences worth your eye, both called out in the PR:

  1. The Microsoft.ContainerService registry currently defines only managedClusters/pods/{read,write,delete} and pods/exec/action, so anything off the safe list matches only wildcard-bearing roles and is denied for roles that enumerate leaf actions. Fail-closed as intended, but a behavioral change.
  2. It also re-gates the CSR subresources (approval/approvals, and the same rule would cover nodeclient). Revert "fix: CSR subresource action collapse allowing unauthorized kubelet certificate issuance" #23 reverted fix: CSR subresource action collapse allowing unauthorized kubelet certificate issuance #12 which had gated nodeclient, so this partially re-applies what that revert removed. If CSR should stay collapsed until those DataActions exist, I will add them to the safe set explicitly.

}

func getDataActions(ctx context.Context, subRevReq *authzv1.SubjectAccessReviewSpec, clusterType string, allowCustomResourceTypeCheck bool, allowSubresourceTypeCheck bool) ([]azureutils.AuthorizationActionInfo, error) {
Expand Down
110 changes: 110 additions & 0 deletions authz/providers/azure/rbac/checkaccessreqhelper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,116 @@ func Test_getDataActions(t *testing.T) {
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "fleet/pods/exec/action"}, IsDataAction: true}},
},

// pods/proxy, services/proxy and nodes/proxy must produce a distinct
// DataAction so that a GET on the proxy subresource does NOT collapse
// onto <resource>/read, keeping the same granularity as the upstream
// Kubernetes RBAC model.
{
"podsProxyGetAKS",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "proxy", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/pods/proxy/action"}, IsDataAction: true}},
},

{
"podsProxyGetFleet",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "proxy", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: "fleet",
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "fleet/pods/proxy/action"}, IsDataAction: true}},
},

// pods/attach and pods/portforward are distinct authorization targets
// in upstream Kubernetes (bootstrappolicy edit role) and must map to
// their own DataAction rather than collapsing onto pods/read.
{
"podsAttachGetAKS",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "attach", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/pods/attach/action"}, IsDataAction: true}},
},

{
"podsPortForwardGetAKS",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "portforward", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/pods/portforward/action"}, IsDataAction: true}},
},

{
"podsPortForwardGetFleet",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "portforward", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: "fleet",
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "fleet/pods/portforward/action"}, IsDataAction: true}},
},

{
"servicesProxyGetAKS",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "services", Subresource: "proxy", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/services/proxy/action"}, IsDataAction: true}},
},

{
"nodesProxyGetAKS",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "nodes", Subresource: "proxy", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/nodes/proxy/action"}, IsDataAction: true}},
},

// A non-sensitive pods subresource (e.g. status) must still collapse to
// the base read action so this fix does not over-restrict legitimate reads.
{
"podsStatusSubresourceStillCollapsed",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "", Resource: "pods", Subresource: "status", Version: "v1", Name: "test", Verb: "get"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/pods/read"}, IsDataAction: true}},
},

// CSR with a non-sensitive subresource (e.g. approval, status)
// should still collapse to the base action, not preserve the subresource.
{
"csrApprovalSubresourceStillCollapsed",
args{
isWildcardTest: false,
subRevReq: &authzv1.SubjectAccessReviewSpec{
ResourceAttributes: &authzv1.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Subresource: "approval", Version: "v1", Name: "test", Verb: "update"},
}, clusterType: aksClusterType,
},
[]azureutils.AuthorizationActionInfo{{AuthorizationEntity: azureutils.AuthorizationEntity{Id: "aks/certificates.k8s.io/certificatesigningrequests/write"}, IsDataAction: true}},
},

{
"allStar",
args{
Expand Down
Loading