Skip to content

KEP-5284: Constrained Impersonation #5285

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

qiujian16
Copy link
Contributor

  • One-line PR description: Introduce authorization rules to restrict impersonating on specified resource with specified actions.
  • Other comments:

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 7, 2025
@k8s-ci-robot k8s-ci-robot requested review from deads2k and enj May 7, 2025 02:49
@k8s-ci-robot
Copy link
Contributor

Hi @qiujian16. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 7, 2025
@enj enj added this to SIG Auth May 7, 2025
@enj enj moved this to Needs Triage in SIG Auth May 7, 2025
@liggitt liggitt mentioned this pull request May 7, 2025
4 tasks
@liggitt
Copy link
Member

liggitt commented May 7, 2025

cc @ahmedtd @vinayakankugoyal

@aramase aramase moved this from Needs Triage to In Review in SIG Auth May 12, 2025
@aramase
Copy link
Member

aramase commented May 12, 2025

/assign enj

should be approved by the remaining approvers and/or the owning SIG (or
SIG Architecture for cross-cutting KEPs).
-->
# KEP-NNNN: Limit Impersonate Permission
Copy link

Choose a reason for hiding this comment

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

nit: use the actual number here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


## Summary

This is to add additional access control over the existing impersonation action. An impersonator
Copy link

Choose a reason for hiding this comment

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

Is this a change to RBAC? If so, the summary should mention RBAC.

Copy link
Member

Choose a reason for hiding this comment

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

This does not change RBAC in any way.

-->

## Alternatives

Copy link

Choose a reason for hiding this comment

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

A couple of alternative ideas come to mind:

• permission boundaries, AWS style

In that story, a principal is constrained and cannot perform some actions. Done right - which isn't trivial - the principal can create new principals, grant permission to those principals, and impersonate (act as) those principals, but only within a bounding set of allowed actions.


CEL. The constraint on impersonation could be done through an access-granting mechanism that evaluates an expression before permitting it. Clients would explicitly declare which access rule they intend to rely on.

For example, the SubjectAccessReview might look like:

{
  "apiVersion": "authorization.k8s.io/v1beta1",
  "kind": "SubjectAccessReview",
  "spec": {
    "resourceAttributes": {
      "namespace": "default",
      "verb": "get",
      "group": "example.org",
      "resource": "something"
    },
    "accessRule": {
      "kind": "ClusterAccessRule",
      "group version": "example.org/v1",
      "name": "elevation-of-privilege"
    },
    "user": "lmktfy",
    "group": []
    ]
  }
}

In this story, the ClusterRoleBinding allows impersonate only with a particular Access Rule name (similar, or identical to, the existing resourceNames mechanism).

However an access rule might day you can impersonate that principal but only to perform deletes.

Copy link

Choose a reason for hiding this comment

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

We file alternatives we think of, so long as they are reasonable, even when we plan to pick the original proposal.

Copy link
Member

Choose a reason for hiding this comment

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

I do not see us expanding SAR / authz for this KEP. The approach described in the KEP is far simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added this as an alternative.

Copy link
Member

@enj enj left a comment

Choose a reason for hiding this comment

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

First pass.

Copy link
Member

Choose a reason for hiding this comment

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

Please complete this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

# The following PRR answers are required at alpha release
# List the feature gate name and the components for which it must be enabled
feature-gates:
- name: ImpersonateAccessControl
Copy link
Member

Choose a reason for hiding this comment

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

This feature gate name feels too vague. Some ideas:

  • ScopedImpersonation
  • ConstrainedImpersonation
  • LimitedImpersonation
  • ConditionalImpersonation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer ConstrainedImpersonation :)

Copy link
Member

Choose a reason for hiding this comment

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

Please update all examples to include binding(s) in addition to the role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

- `impersonate:scheduled-node` that limits the impersonator to impersonate the node the
impersonator is running on. The resource must be `nodes`.

For the imperonsonator, two permissions will be required:
Copy link
Member

Choose a reason for hiding this comment

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

Include the exact YAML of the subject access review that will be sent to the authorizer chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Comment on lines 173 to 174
This proposal is to introduce additional permissions for impersonation, so that any
impersonator can impersonate in a more restricted way.
Copy link
Member

Choose a reason for hiding this comment

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

I assume this would be an opt-in feature such that existing impersonation flows keep working as-is. If so, highlight that in the Goals section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Comment on lines 966 to 969
Some concerns on using this approch:
- impersonating on any APIGroup is hard to describe.
- core APIGroup has to be specially treated.
- the APIGroup might be too long for a CRD, e.g. cluster.x-k8s.io.imperonsation.k8s.io
Copy link
Member

Choose a reason for hiding this comment

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

While I am fine with the verb based approach described in this KEP, I don't really find any of these concerns to be compelling. At the end of the day we are just talking about different string based options here.

I would say the most compelling reason is that the existing impersonation flows are verb based, so making the new flow verb based as well feels more consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense, updated

- `impersonate:user` limits the impersonator to impersonate users with
certain names/groups/userextras. The resources must be `users`/`groups`/`userextras`.
The resource names must be user names, group names or values in the user extras accoringly.
- `impersonate:scheduled-node` that limits the impersonator to impersonate the node the
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to describe how the workload learns the node that it is scheduled on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added more details in Design Details section

- `impersonate:user` limits the impersonator to impersonate users with
certain names/groups/userextras. The resources must be `users`/`groups`/`userextras`.
The resource names must be user names, group names or values in the user extras accoringly.
- `impersonate:scheduled-node` that limits the impersonator to impersonate the node the
Copy link
Member

Choose a reason for hiding this comment

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

Include the exact set of impersonation headers the workload must set for the node case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added more details in Design Details section


## Summary

This is to add additional access control over the existing impersonation action. An impersonator
Copy link
Member

Choose a reason for hiding this comment

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

This does not change RBAC in any way.

-->

## Alternatives

Copy link
Member

Choose a reason for hiding this comment

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

I do not see us expanding SAR / authz for this KEP. The approach described in the KEP is far simpler.

@github-project-automation github-project-automation bot moved this from In Review to Changes Requested in SIG Auth May 14, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiujian16
Once this PR has been reviewed and has the lgtm label, please ask for approval from enj. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


## Proposal

Introduce a set of verbs with prefix of `impersonate-on:`, e.g. `impersonate-on:create` and
Copy link

Choose a reason for hiding this comment

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

Overall, I am wary of making verbs that you have to parse.

For example, the impersonate:user verb sets the scene for other verbs, such as list:metadata-only or proxy:port:443.

We should we sure what direction of travel we want if we are to establish this kind of precedent, and clearly indicate why the alternatives are overall less viable.

@qiujian16 qiujian16 changed the title KEP-5284: Limit impersonate permissions KEP-5284: Constrained Impersonation May 16, 2025
- `impersonate:scheduled-node` that limits the impersonator to impersonate the node the
impersonator is running on. The resources must be `nodes`.

For the imperonsonator, two permissions will be required:
Copy link

Choose a reason for hiding this comment

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

Suggested change
For the imperonsonator, two permissions will be required:
For clusters that use RBAC authz mode, two permissions will be required for impersonation. For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

- a search in the Kubernetes bug triage tool (https://storage.googleapis.com/k8s-triage/index.html)
-->

- SAR check on impersonate user with permission:
Copy link

Choose a reason for hiding this comment

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

Suggested change
- SAR check on impersonate user with permission:
- SubjectAccessReview check on impersonating user. For RBAC authz mode, this might look like:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

name: impersonate-user
rules:
- apiGroups:
- auhtentications.k8s.io
Copy link

Choose a reason for hiding this comment

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

(typo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

- The impersonator cannot impersonate on updating nodes

- SAR check on impersonate scheduled node with permissions. The impersonator has the
user extra info of `"authentication.kubernetes.io/node-name": "node1"`
Copy link

Choose a reason for hiding this comment

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

Can any client provide this? What protects against abuse?

The important examples here are actually:
• a compromised node
• an attacker aiming to elevate privileges by impersonating a node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If an attacker intends to elevate privileges by impersonating a node, it needs to have an user extra "authentication.kubernetes.io/node-name": "someNode" in the userInfo, and it needs to have both permissions with verbs "impersonated:scheduled-nodes" and "impersonated:someVerb".

-->

On upgrade to a version that enables the feature
* the previous inpersonator with impersonate permission will still work, but it is highly
Copy link

Choose a reason for hiding this comment

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

(typo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

It is almost the same as the verb based approach in the proposal. However, since existing impersonation flows are
verb based, so making the new flow verb based as well feels more consistent.

### Check permission intersaction of impersonator and target user
Copy link

Choose a reason for hiding this comment

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

(typo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

in the proposed apporch, the impersonator's permission is clearer that it can only perform
the action when impersonating.

### Expand RBAC/SAR
Copy link

Choose a reason for hiding this comment

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

If the cluster does not use RBAC, is this alternative still feasible?

"name": "elevation-of-privilege"
},
"user": "lmktfy",
"group": []
Copy link

Choose a reason for hiding this comment

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

Try not to use real usernames in examples (people sometimes copy them verbatim!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


# The following PRR answers are required at beta release
metrics:
- my_feature_metric
Copy link

Choose a reason for hiding this comment

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

(placeholder - please change)

of impersonated user and the requester.

## Proposal

Copy link

Choose a reason for hiding this comment

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

If a cluster had its own authn and splits principals into ServiceAccounts, ProjectRoles (not K8s RBAC) and ProjectUsers: does this mechanism prevent things working for that design?

We should not (almost must not) assume that clusters rely on K8s RBAC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think it prevent things working. This still work if it is not k8s RBAC, the external authorizer will receive subject access review call with the specified verb.

Comment on lines +196 to +200
- `impersonate:user-info` limits the impersonator to impersonate users with
certain names/groups/userextras. The resources must be `users`/`groups`/`userextras`/`uids`.
The resource names must be user names, group names or values in the user extras accoringly.
- `impersonate:serviceaccount` that limits the impersonator to impersonate the serviceaccount with
the certain name/namespace. The resources must be `serviceaccounts`.
Copy link
Member

Choose a reason for hiding this comment

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

Does impersonate:serviceaccount for some resourceName imply access to the the system:serviceaccounts: groups associated with those serviceaccounts as well?
Does this verb behave differently than the current impersonate verb when serviceaccounts is supplied as the resource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the same as impersonate verb with serviceaccount as the resource, but it will require additional impersonate-on:{action} verb. The reason that not to reuse the original impersonate verb is to make it unchanged for backward compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Status: Changes Requested
Development

Successfully merging this pull request may close these issues.

8 participants