KEP-5681: Conditional Authorization #5684
Conversation
k8s-ci-robot
added
sig/auth
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: luxas 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 |
k8s-ci-robot
added
the
size/XXL
|
@luxas: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
everettraven
left a comment
everettraven
left a comment
There was a problem hiding this comment.
Awesome to see this KEP - thanks for digging into this @luxas !
I'll be at both Maintainer Summit and KubeCon, so if folks are planning to discuss this there I'd love to sit in on those discussions :).
Leaving a few thoughts/questions that came to mind as I was doing an initial read.
| 1. The API server enforces the conditions in the validating admission stage, | ||
| where access to the objects is available with the correct consistency | ||
| guarantees. |
There was a problem hiding this comment.
Would there need to be an ordering requirement in the validating admission stage here?
Presumably, a request should go through the entire "authorization stage" before continuing through the rest of the admission stages - which means this would always have to be the first validating admission call in the chain?
There was a problem hiding this comment.
I don't have a strong opinion on when in the validating admission chain this is put, but indeed, it could make sense to make this be the first one in validating admission, as then any extra normal validating webhooks are not called unnecessarily, if the request would end up being denied already by the conditional authorization stage.
| // If a NoOpinion condition evaluates to true, the given | ||
| // authorizer’s ConditionSet cannot evaluate to Allow anymore, but | ||
| // necessarily Deny or NoOpinion, depending on whether there are any | ||
| // true EffectDeny conditions. | ||
| // However, later authorizers in the chain can still Allow or Deny. | ||
| // It is effectively a softer deny that just overrides the | ||
| // authorizer's own allow policies. It can be used if an authorizer | ||
| // does not consider itself fully authoritative for a given request. | ||
| // TODO: Talk about error handling; what happens if any of these | ||
| // conditions fail to evaluate. | ||
| EffectNoOpinion ConditionEffect = "NoOpinion" |
There was a problem hiding this comment.
As a reader, it might be helpful to have some examples of when an authorizer presenting a condition to be evaluated may not consider itself authoritative.
Probably naively, my understanding of NoOpinion in the existing authorizer sense aligns with "I don't have an enforceable policy defined for this request, and thus have no opinion on the authorization of this request".
If an authorizer is returning a condition to be evaluated for a policy, I would assume that the authorizer inherently has an opinion on whether or not the request should be authorized based on the result of that condition evaluation?
There was a problem hiding this comment.
Great question! 👍 This one is a bit hard to model/reason about yeah. This is effectively a deny policy which can be thought of as appended to each allow condition, as follows, e.g.
- policy1: "never give lucas any privileges" ("NoOpinion condition")
principal.name == "lucas"
- policy2: "never assign privileges for a node identity" ("NoOpinion condition")
principal.groups.contains("system:nodes")
- policy3: "let group engineers create deployments when serviceAccountName is foo" ("Allow condition")
principal.groups.contains("engineers") && action == "create" && resource is core::deployments && resource.request.v1.spec.serviceAccountName == "foo"
- policy4: "let group readers list deployments" ("Allow condition")
principal.groups.contains("readers") && action == "list" && resource is core::deployments
Now, even though these are not all conditional in this example, in theory they could be. The logic of the weaker "NoOpinion deny" conditions is such that they are logically appended to each Allow policy, as follows:
<allow_policy> && !(<noopinion1_policy> || ... || <noopinionN_policy>)
<=>
<allow_policy> && !<noopinion1_policy> && ... && !<noopinionN_policy>
In other words, the allow policy matches only if no NoOpinion deny policy also simultaneously. If any of the NoOpinion policies did match (that is, evaluate to true), it means that all Allow conditions are automatically falsified, as there is ... && !true && ... somewhere in every Allow condition, which necessarily never can make the expression evaluate to true.
In the above example, it is e.g. possible that user lucas also is in the engineers and/or readers groups, and it would be inconvenient to have to "remember" to add && principal.username != "lucas" to every allow condition to remove this possibility.
The above 4 policies are thus logically equivalent to the following two Allow policies, which also could have been written directly, but could be more cumbersome:
- "let group engineers create deployments when serviceAccountName is foo, unless the principal is lucas or a node"
principal.groups.contains("engineers") && action == "create" && resource is core::deployments && resource.request.v1.spec.serviceAccountName == "foo" && principal.username != "lucas" && !principal.groups.contains("system:nodes")
- "let group readers list deployments, unless the principal is lucas or a node"
principal.groups.contains("readers") && action == "list" && resource is core::deployments && principal.username != "lucas" && !principal.groups.contains("system:nodes")
It's always harder to reason about deny policies, and I'm not advocating for their use, but they are such strong primitives that it would feel irresponsible not to take their characteristics into account and plan for their inevitable (?) addition at some point. Today, VAP forms our deny-policy layer, and this proposal (IMO) must thus plan for deny policies existing. VAP, however, always are equivalent to the stronger Deny conditions, as they never consider any other authorizer in the chain, but do exactly as they like 😄.
Whether NoOpinion conditions like these are useful or not, is entirely dependent on what kind of authorizer chain you have. I'm not sure upstream kube can mandate folks to have one type of chain or the other.
However, it would/could be fair to demand a NoOpinion-capable authorizer to do the above rewrite from Allow+NoOpinion conditions to only Allow conditions before sending the conditions to Kubernetes, and in that case Kube would not need to worry about this. However, this is at the cost of longer condition evaluation times (as the same NoOpinion condition would evaluate once for every allow condition, instead of just once), and less nice error messages, as the error message would not be "user lucas cannot get privileges from authorizer foo", but e.g. "let group readers list deployments, unless the principal is lucas or a node", using the example from above.
This is a great point worth discussing, and I'm interested in hearing other people's thoughts.
| // Optional human-friendly description that can be shown as an error | ||
| // message or for debugging. | ||
| // TODO: We might want to leave this out, in case it consumes too much | ||
| // resources to pass around. | ||
| Description string |
There was a problem hiding this comment.
I wonder if this should actually be required, but limited to some reasonable maximum length?
Thinking about it from a user perspective, if I were to do a kubectl auth can-i ..., a mandatory description means that, given a conditional authorization, it could always return a response in the format of:
yes, under the following conditions:
- ...
- ...
The alternative with this being optional is that it returns either some reference to a conditional policy or the exact condition being enforced which, IMO, is a poor UX for someone that just wants to know whether they can or cannot do X on the cluster.
There was a problem hiding this comment.
I think this would be nice, yes, but I'm not sure that authorizers maybe even always have a description available. The condition would/could be shown as fallback in any case.
For negative (NoOpinion/Deny) policies, a mandatory description models basically what we have in VAP today. However, messages in VAP are optional today as well, so we might want to not mandate this, but instead let it be up to the authorizer to choose how nice they want to make it for the user 😄
Another point is that one could want to flag to the authorizer that "this SAR is for displaying stuff to the user, please give a description that you would not otherwise necessarily give". For example, I don't see how description would make sense for Allow conditions for SARs executed during requests, as there might be many conditions, and if none of them matches, it'd be very lengthy to return:
decision: NoOpinion
reason: No policy in Authorizer foo allowed the request, of all the following conditional policies:
- <allow1_description>
- <allow2_description>
- ...
- <allowN_description>
err: nil
(IIRC, RBAC returns reason e.g. "RBAC: no rules authorize user foo with groups [bar] to get pods" instead of listing every possible allow policy that could have applied, but did not)
That is just to say that:
- yes, descriptions are nice for SelfSAR, but
- might have to be optional, as not all authorizers have the info, and
- it'd be nice for the authorizer to be able to distinguish between "is this a SelfSAR for display" or "is this for a live request", to avoid sending potentially expensive descriptions (if the authorizer does support) over the wire for allow policies unneccessarily.
Further thoughts on this? I'd be weary of adding another boolean to SAR though 😅 (for "is this selfsar or not")
lmktfy
left a comment
lmktfy
left a comment
There was a problem hiding this comment.
This PR doesn't follow th conventions we use for our PRs. It's so different that I found it hard to follow.
To help contributors to review it, I recommend aligning to the conventions much more.
| and use that for comms, similar to ServiceAccount issuing and KMS | ||
| implementations? | ||
|
|
||
| ## Alternatives Considered, Detailed Decision Log |
There was a problem hiding this comment.
I'd expect the normal Alternatives section here.
There was a problem hiding this comment.
If you think it's clearer, I can split the section into "Alternatives Considered" for features that are wide enough to be consider 1:1 alternatives, but I'd like to call out alternatives for sub-problems of the KEP as well that were considered, as the solution space is pretty large, and this proposal has already gone through a couple of revisions.
| implementations? | ||
|
|
||
| ## Alternatives Considered, Detailed Decision Log | ||
|
|
There was a problem hiding this comment.
None of the alternatives proposes implementing this out of tree.
We should either record that that's an alternative (that we don't recommend), or explain why it can't be.
There was a problem hiding this comment.
None of the alternatives proposes implementing this out of tree.
I started with this in the third paragraph of the summary (the screenshotted text), but I see that it was not clearly enough mentioned as an alternative
I'll add a mention of this as an alternative in the list, but I'd like to keep the reasoning up-front for the reader, and not delay this reasoning to the end. Thanks for the call-out, hopefully that makes sense to you.
| However, this evaluator could evolve with distinct maturity guarantees than the | ||
| core conditional authorization feature. | ||
|
|
||
| ## Open Questions |
There was a problem hiding this comment.
KEPs have a special syntsx for markimg unresolved topics; this PR should use that kind of marker.
There was a problem hiding this comment.
Thanks for the call-out, I'll add the markers.
Sorry you felt it is hard to follow. It'll get improved over time, with feedback and increased iteration. |
luxas
left a comment
luxas
left a comment
There was a problem hiding this comment.
Adding comments that were discussed offline at KubeCon. Will update the KEP to reflect these comments as soon as I have time.
|
|
||
| ### Let the API server indicate that it supports conditional authorization | ||
|
|
||
| In order for the API server to indicate to a webhook authorizer that it supports |
There was a problem hiding this comment.
When discussing with Mo at the maintainer summit, he suggested that we'd use this mechanism to not just assume that failing closed for an old API server is NoOpinion, but the authorizer might want to fail closed with Deny for old API servers. This is a good point 👍, will add this back to the main proposal.
@lmktfy suggested async to use an feature-negotiation.kubernetes.io annotation with a JSON object containing enabled features as a generic encoding mechanism, so a webhook can understand enabled features in the API server, without having to parse the label/annotation key for the feature name. I'm fine with any feature negotiation mechanism, as suggested I'll drop a message in SIG Architecture to ask their opinion as well.
| front, even if we know the authorization verb, as e.g. the “patch” | ||
| authorization verb using Server-Side Apply can become a CREATE operation or a | ||
| normal UPDATE operation. | ||
|
|
There was a problem hiding this comment.
for impersonation requests, we'd add a fifth set of data here, only available when impersonating, that'd include the metadata about the request performed (resource, verb, apiGroup, ns, name, etc.) and the full userinfo (name, groups, etc.) that the policy can condition the impersonation request being allowed on.
Mo asked whether it'd make sense to allow going even more fine-grained, to allow saying "user ai-agent-1 can impersonate user lucas for create pods only when Pod.spec.serviceAccountName == "foo"". That could be done, if:
- we have a method to make conditions flow from kube-apiserver to aggregated API servers (see my other review comment below in this batch), and
- the condition type itself supports partial evaluation, as one needs to make sure that "the impersonation request can become authorized, regardless of the object", just like conditional authorization in general. In practice, this probably means that we could only support this specific feature with CEL conditions, which the API server could in-process partially evaluate. (or we would need to extend
EvaluateConditionsto add the ability to perform partial evaluation too, not just concrete evaluation)
For the policy mentioned in this comment, the initially-returned condition would be of form:
request.verb == "create" && request.apiGroup == "" && request.resource == "pods" && object.spec.serviceAccountName == "foo"
And as the API server in fact knows what the request's metadata at this point is, it would need to partially evaluate the condition into one where only object is unknown, that is:
object.spec.serviceAccountName == "foo"
This specific feature can be discussed, but I'm fine with constrained impersonation only being able to constrain the impersonation step based on request metadata, not object data.
| specified? | ||
|
|
||
| - Allow Policy 1: “true && false” \=\> false | ||
| - Allow Policy 2: “true && true && true && true && object.spec.storageClassName \== “dev”” |
There was a problem hiding this comment.
TODO: Lucas or Micah to use code blocks for these instead.
| with existing RBAC mechanisms. The goal of this proposal is to make authorizers | ||
| able scope down their policies, and have Kubernetes enforce those scoped-down | ||
| policies *as if* the authorizer had access to the resource data directly. | ||
| Examples of policies that bind the authorization decision to the resource data: |
There was a problem hiding this comment.
Lucas to move the rest of this chapter to another chapter (called something along the lines of "background"), to keep the summary short.
| } | ||
| ``` | ||
|
|
||
| In pseudo-code, the WithAuthorization HTTP filter functions as follows: |
There was a problem hiding this comment.
The point of this code is not the pseudo-code itself, but the logic, but it could as well be described with a diagram.
| type AuthorizationConditionsRequest struct { | ||
| // Operation is the operation being performed. This may be different than the | ||
| // operation requested. e.g. a patch can result in either a CREATE or UPDATE. | ||
| Operation admissionv1.Operation `json:"operation"` |
There was a problem hiding this comment.
This might be omitempty, unless we set this to be IMPERSONATE for constrained impersonation requests.
Add an ImpersonatedRequest field here for constrained impersonation.
| return w.decisionOnError, "", fmt.Errorf("webhook authorizer does not support conditional responses") | ||
| } | ||
|
|
||
| if len(r.Status.ConditionsChain) > 1 { |
There was a problem hiding this comment.
Relax this to allow any amount of chained authorizers, this is needed when an aggregated API server asks kube-apiserver whether to authorize a request; kube-apiserver might return multiple ConditionSets, and the aggregated API server should evaluate and respect those condition sets in order.
| // the authorization conditions. | ||
| // Mutually exclusive with Denied. | ||
| // Allowed=false and Denied=false means that the authorizer has no NoOpinion on the request. | ||
| Allowed bool `json:"allowed" protobuf:"varint,2,opt,name=allowed"` |
There was a problem hiding this comment.
nit: remove protobufs here for easier readability.
| EvaluateConditions endpoint with the missing data (the information about the | ||
| impersonated request). | ||
|
|
||
| This approach avoids adding synthetic verbs like `impersonate-on:user-info:list` |
There was a problem hiding this comment.
Make the differences more concrete. For example, with this version, one can express policies such as "allow ServiceAccount ai-agent-a to impersonate user lucas, but ONLY when also impersonating group ai-agent"
=> now allow rules can be written such that "allow lucas to delete persistentvolumes, but only when lucas is not in the ai-agent group", to allow user identity to flow through, but without allowing the agent (or whoever is impersonating) to do dangerous actions. (This has nothing to do with AI agents per se, but it just serves as an example of a logical impersonator).
Also showcase the difference between the "union" semantics of the current KEP, and what can be done with this one, and maybe (?) how the existing "RBAC frontend" could be re-used if needed/wanted.
| | :---- | :---- | :---- | | ||
| | Condition Type Not Supported by Builtin Condition Evaluators | Authorize() \+ EvaluateConditions() | EvaluateConditions() | | ||
| | Condition Type Supported | Authorize() | Neither | | ||
|
|
There was a problem hiding this comment.
Add a section on aggregated API servers here.
When a client talks to an aggregated API through the kube-apiserver, authorization happens twice. First, the kube-apiserver authorizes the request, and only if authorized, forwards the request to the aggregated one. The aggregated API server must authorize the request also a second time (as there could be clients hitting the aggregated API servers' HTTPS endpoint directly, too).
We must make sure things fail closed especially when kube-apiserver is new (supports conditional authorization), but the aggregated API server is old (does not support conditional authorization). Notably, admission does not run in the kube-apiserver, but only in the aggregated API server, and thus cannot kube-apiserver evaluate the conditions. If conditionally authorized, should kube-apiserver trust that the aggregated API server will perform the conditions evaluation, or not?
Case 1: The aggregated API server is configured with an authorizer chain of only Webhook, which points to kube-apiserver
✅ Fails closed out of the box. This is the standard case, most setups should be like this. If this is the case, and kube-apiserver sees a conditional response in the first authorization phase, it can safely continue with the request flow. Then, the aggregated API server performs a webhook SAR to kube-apiserver without the kubernetes.io/ConditionalAuthorizationFeature=true "feature flag". kube-apiserver evaluates the authorizer chain again, and reaches the same conditionally authorized response (are we afraid of a race condition in which these two directly-after-each-other responses would not be the same?), but notices that the requestor is old, and thus returns e.g. a Deny response. The aggregated API server respects and enforces the deny response by responding with 403.
Case 2: The aggregated API server uses some other authorization mechanism than kube-apiserver, or has some other authorizer in the chain in front of the kube-apiserver Webhook
❌ Changes needed to fail closed. In this case, one would need to
- make sure that the aggregated API server supports conditional authorization before forwarding the request (as discussed with Mo and Jordan, e.g. through some capability field/map on the
APIService), and - find a mechanism to propagate the conditions from the first evaluation phase to the aggregated API server (through some headers similar to the front-proxy ones?)
|
|
||
|  | ||
|
|
||
| For a practical example of what this unified interface can look like, take a |
There was a problem hiding this comment.
I suggest: move this paragraph to an appendix.
| subject to through (Self)SubjectAccessReview | ||
| - Ensure that a request is evaluated against an atomic set of policies in the | ||
| authorizer, even in presence of cached authorization decisions | ||
| - Allow conditions to be expressed in both transparent, analyzable forms (e.g. |
There was a problem hiding this comment.
I wonder if we think sets of authz policies might exist as a. OCI artefact.
- good idea?
- bad idea?
- out of scope?
| - Designing or mandating use of a specific policy language as the user experience | ||
| - Designing or mandating use of a specific “official” condition syntax | ||
| - Expose the conditions to arbitrary admission controllers | ||
| - Support conditional authorization for requests other than write and connect verbs. |
There was a problem hiding this comment.
So I can't use this to restrict reads, or for custom verbs (eg approving certificate signing)? That's important and needs calling out much more visibly.
That restriction conflicts with the ambition to cover impersonation; impersonation is neither a write nor a read.
|
|
||
| - Must preserve order of authorizers; evaluation must not differ from how it has | ||
| been evaluated using separate authorization and admission phases. | ||
| - Works for connectible (CONNECT verb) resources; only when the accessing the |
There was a problem hiding this comment.
Is partial support for CONNECT authz a requirement?
| - It should still be the case that only if the request *can become authorized* | ||
| (depending only on the objects), the request payload should be decoded. We | ||
| must not decode the object before this, as that would be a DoS attack vector. | ||
| - The combined result of the partial authorization \+ later condition |
| - Only allow a node agent to handle resources where `.spec.nodeName=<node-name>` | ||
| - Only allow creating or updating a ResourceClaim when adminAccess=false | ||
| (generalization of DRA AdminAccess) | ||
| - Only allow a principal to create objects with certain names |
There was a problem hiding this comment.
A viable alternative for this is ValidatingAdmissionPolicy. We might want to explain why we don't consider the existing in-tree solution.
| [KEP-2862: Fine-grained Kubelet API Authorization](https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/2862-fine-grained-kubelet-authz/README.md)) | ||
| - Encode the logic of the NodeRestriction admission controller through a | ||
| condition returned by the Node authorizer | ||
| - Empower out-of-tree authorizers to perform finer-grained policies, e.g. with |
There was a problem hiding this comment.
I'd like to understand what this means.
| ABAC and/or label-based controls | ||
|
|
||
| ## Proposal | ||
|
|
There was a problem hiding this comment.
This section should describe the proposed enhancement as if we have not solved the problem in detail, but that we do have a high level plan for the work we'll do.
eg: "definitely an OCI image format for Windows images, and add support to the kubelet for Windows Pods" (and not 2000 words about Windows paravirtualization).
Go into detail only after presenting the high level / strategic approach.
|
|
||
| Now, in the introduction it was mentioned that with this proposal, two new | ||
| [authorizer.Decision](https://pkg.go.dev/k8s.io/apiserver/pkg/authorization/authorizer#Decision) | ||
| types are added, namely `ConditionalAllow` or `ConditionalDeny`. |
There was a problem hiding this comment.
If I do a SubjectAccessReview as an aggregated API server, is it possible that I see a verdict at to ConditionalAllow?
(if I might, what should the API server then do given that verdict?)
| second authorizer ends up evaluating to NoOpinion, the third authorizer is | ||
| evaluated (and in this example evaluates first ConditionalAllow, then Allow). | ||
|
|
||
| Initially, before the SIG Auth Deep Dive (Sept 4, 2025) on this topic, the idea |
There was a problem hiding this comment.
I would omit the decision making history from the KEP. We have it on record in other ways (eg YouTube recordings).
4 participants
/sig auth
Rendered
There are still minor things to sort out, but hopefully this proposal at a high level matches what is desired, as per the previous SIG Auth meeting discussions. Let's discuss this in more detail at KubeCon next week 👍