KEP-4633: Only allow anonymous auth for configured endpoints

[vinayakankugoyal]

• One-line PR description: Adding new KEP.

• Issue link: Only allow anonymous auth for configured endpoints. #4633

• Here is the implementation of the Alpha phase of this KEP: KEP-4633: Only allow anonymous auth for configured endpoints. kubernetes#124917

[vinayakankugoyal]

/assign @liggitt

[vinayakankugoyal]

/un-assign @liggitt

[vinayakankugoyal]

/unassign @liggitt

[vinayakankugoyal]

/unassign @liggitt

[vinayakankugoyal]

/cc @liggitt

[liggitt]

This lgtm. Will give @enj time to look over it as well, and @jpbetz for PRR (alpha) review.

I'd like to see this merged by the end of the week if possible.

[enj]

I'd like to see this merged by the end of the week if possible.

I plan on looking at this today. If that doesn't happen, I'll take care of getting it merged/marked implementable next week.

[vinayakankugoyal]

I'd like to see this merged by the end of the week if possible.

I plan on looking at this today. If that doesn't happen, I'll take care of getting it merged/marked implementable next week.

Hi @enj! gentle ping on this. We are getting close to the KEP freeze timelines. Thanks!

[enj]

At a high level, I am in favor of this KEP. I believe it will let certain environments harden themselves.

Assuming Jordan is back from PTO tomorrow, I think my comments below would serve as good topics for the API review meeting.

1. As far as path based logic goes, both RBAC and the path authz support globs. Why does this feature not support globs as well?

2. Does the path matching require an exact match or does it perform any kind of trimming on leading or trailing slashes?

3. By the time the authentication layer runs, we have already paid the cost to of parsing the request info data. Thus why are we using paths to represent resources, when we have the proper GVR available to us?

4. I disagree with the structure of this API. It seems to assume that the existing flag based config is 'good' when we clearly believe that it is not. I doubt we would design it today in the same way. So instead of having two fields will nuanced interactions with each other, I would expect an API that has two mutually exclusive positive fields. For example:

anonymous:
  enabledForAllRequests: bool
  enabledForSelectedPaths: [ ... ]

This makes it obvious from the single field exactly what the config is doing.

5. I would also prefer an API that leaves enough space for future extension instead of assuming we will only have path based restrictions:

anonymous:
  enabledForAllRequests: bool
  enabledWithConditions:
    - path: ...
    - resource: ...

6. Should we allow for IP based allow lists for anonymous auth?

7. This entire feature and much more could be accomplished with an L7 proxy in front of the API server (over localhost). Many OSS proxies exist that can do this while allowing the cluster operator to disable anonymous authentication on the cluster. Why is that approach not recommended since it requires no changes to core Kube?

8. No other existing authenticator makes decisions based on the request metadata today. Why is okay for the anonymous authenticator to do that? Are we okay in the future allowing other authenticators to do that? Both request signing and IP based conditional authentication come to mind as use cases.

9. @stlaz made an interesting comment that this design felt like it belonged in authorization instead of authentication. The path based restriction does feel like a deny authorizer. If an existing authenticator (like JWT) asserts the anonymous user (or the anonymous user is impersonated), are we okay with that anonymous user not having any of these restrictions? If the restriction was implemented in authorization, it would be uniform regardless of how the anonymous user was asserted.

[liggitt]

responding to #4634 (comment)

1. As far as path based logic goes, both RBAC and the path authz support globs. Why does this feature not support globs as well?

I want to keep the surface area here as small as possible, globbing wasn't required for the use cases presented so far, and the intent of this feature is to constrain anonymous auth to a well-known set of endpoints. That points to constraining this to exact paths in my opinion.

AI: call out that this is more constrained than RBAC, etc, on purpose, and why.

2. Does the path matching require an exact match or does it perform any kind of trimming on leading or trailing slashes?

AI: clarify this is an exact case-sensitive match, if you want to allow with/without slash, add both to the list (just like RBAC).

3. By the time the authentication layer runs, we have already paid the cost of parsing the request info data. Thus why are we using paths to represent resources, when we have the proper GVR available to us?

A flat list of paths is way simpler to reason about than parallel lists of non-resource paths and structured resource tuples, in terms of what it is allowing. I expect anyone using this feature to have ~1-3 entries and being very explicit about the paths the anonymous authenticator can be used for.

AI: explain why this surface area is so constrained, and indicate that people who want anonymous access to a wider or more complicated set of endpoints should probably just leave it globally enabled and lean on authorization policy.

4. I disagree with the structure of this API. It seems to assume that the existing flag based config is 'good' when we clearly believe that it is not. I doubt we would design it today in the same way. So instead of having two fields will nuanced interactions with each other, I would expect an API that has two mutually exclusive positive fields. For example:

anonymous:
  enabledForAllRequests: bool
  enabledForSelectedPaths: [ ... ]

This makes it obvious from the single field exactly what the config is doing.

I want to be able to use the config file to disable anonymous auth as well, so I think the single coarse-grained explicit on / off boolean field is useful.

Because we load the config file strictly, a typoed restrictToPaths will error. I don't think making the admin repeat themselves to enable for all paths is useful.

AI: explicitly describe how to disable anonymous auth via the config file

5. I would also prefer an API that leaves enough space for future extension instead of assuming we will only have path based restrictions:

anonymous:
  enabledForAllRequests: bool
  enabledWithConditions:
    - path: ...
    - resource: ...

AI: switch to a list of struct conditions with only path as a condition today, which makes it possible to extend in the future if needed.

conditions []AnonymousAuthCondition

type AnonymousAuthCondition struct {
  Path string
}

6. Should we allow for IP based allow lists for anonymous auth?

I don't think we should include this now, but if we structured as mentioned for 5, this could be added in the future:

conditions:
- path: /healthz
  ip: 127.0.0.1
- path: /readyz
  ip: 127.0.0.1
- path: /livez
  ip: 127.0.0.1
- path: /api/v1/namespaces/kube-public/configmaps/public-info
- path: /.well-known/openid-configuration

7. This entire feature and much more could be accomplished with an L7 proxy in front of the API server (over localhost). Many OSS proxies exist that can do this while allowing the cluster operator to disable anonymous authentication on the cluster. Why is that approach not recommended since it requires no changes to core Kube?

If someone is going to set up a sidecar load balancer container for kube-apiserver anyway, then sure, that could handle this scenario. However, many deployments do not run sidecar load balancer containers (see every kubeadm deployment) and the complexity to recommend they start doing so is way higher than allowing scoping down anonymous auth this way.

AI: add an alternative considered sentence that a sidecar proxy could have handled this but would push complexity into all consumers who are not running side car proxies today, and the complexity of allowing the restriction in-tree is minimal.

8. No other existing authenticator makes decisions based on the request metadata today. Why is okay for the anonymous authenticator to do that? Are we okay in the future allowing other authenticators to do that? Both request signing and IP based conditional authentication come to mind as use cases.

AuthenticateRequest already takes the full request, TokenReview is a subset of that which only looks at the bearer token. I'd actually be ok with resurrecting #2843 to forward some info about the request to the token authenticators if we can constrain it sufficiently

Determining if a given authenticator is active or not is a much smaller surface area than modifying the resulting user info based on request attributes. It's worth noting that this doesn't change the authenticator go interface at all, AuthenticateRequest already has the go request. Starting with a very constrained in-tree use of this information which we know doesn't modify resulting user info, before considering surfacing it to our external APIs, seems ok.

9. @stlaz made an interesting comment that this design felt like it belonged in authorization instead of authentication. The path based restriction does feel like a deny authorizer.

I'm not in favor of splitting this between authn and authz for a couple reasons:

1. a deny authorizer is a lot more complex to implement than restricting the anonymous authenticator
2. having two decoupled subsystems where a later phase is responsible for locking down over-granted stuff from the first phase is not ideal... we already have this with authz/admission, and I don't want to repeat that pattern if we don't have to

AI: add an alternative considered about split authn / deny authorizer and why we don't want to do that.

If an existing authenticator (like JWT) asserts the anonymous user (or the anonymous user is impersonated), are we okay with that anonymous user not having any of these restrictions? If the restriction was implemented in authorization, it would be uniform regardless of how the anonymous user was asserted.

Having other actual authenticators identify the incoming request as anonymous is ... sort of insane. I think we should start by just configuring / restricting the actual anonymous authenticator.

AI: add an open question to resolve before beta about also applying any restrictions here to anonymous userInfo that comes back after all authenticators and impersonation have run.

[jpbetz]

/approve
For PRR

[vinayakankugoyal]

@liggitt and @enj - I've updated the KEP based on the discussion and AI in #4634 (comment). Please take another look! Thanks!

[liggitt]

thanks

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, liggitt, vinayakankugoyal

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

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/prod-readiness/OWNERS [jpbetz]
• keps/sig-auth/OWNERS [liggitt]

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

[cfryanr]

How would this new feature impact aggregated APIs added later to the cluster by installing apps which include a new APIService? I didn't see mention of that in the KEP. If some of those new aggregated APIs require anonymous auth, would anonymous auth be rejected if those paths were not included in this new configuration (assuming this new feature is configured with paths)?

[liggitt]

would anonymous auth be rejected if those paths were not included in this new configuration (assuming this new feature is configured with paths)?

Yes. This is putting control about which endpoints allow anonymous auth in the hands of the kube-apiserver administrator. If they constrain anonymous auth to a specific set of paths, that precludes anonymous access through the kube-apiserver to aggregated API server endpoints not included in that set of paths.

[neolit123]

i think i agree with some of the points @enj brough up earlier especially around extensibility of the feature.
on a UX tangent, it seems like the configuration becomes "well-guarded" by the administrator as it's a file on disk, however it also becomes not well extensible.

example:

• kubeadm defaults some anonymous auth endpoint config so that it can bootstrap clusters with BS tokens.
• a tool is deployed on the kubeadm cluster that needs to change the kubeadm defaults so that it can support additional annonymous paths. i believe such a tool today is Pinniped that @enj also worked on.

and as another crude example, if anonymous paths were guarded by authz with something like RBAC then that means they could be additive as resources are. however a single centralized config is a different story and also it means the admin has to keep it in sync on HA setups. so maybe a controller is needed.

this makes deployment well guarded but more challenging.

[enj]

if anonymous paths were guarded by authz with something like RBAC

One of the goals of the feature is to disallow misconfiguration of clusters via RBAC, especially in cloud envs where the control plane config is owned by the cloud provider. Any tool has to the deal with anonymous auth being disabled. For example, AKS always has it disabled and there is no way to enable it. I don't see that ever changing.

If kubeadm adopts this functionality (therefore changing the default of those environments), they should make it easy to configure since there is no distinction between the Kube API admin and the cluster operator in self hosted clusters. The anonymous config needs to be coordinated with the runtime config of the cluster such as aggregated APIs.

[neolit123]

if anonymous paths were guarded by authz with something like RBAC

One of the goals of the feature is to disallow misconfiguration of clusters via RBAC, especially in cloud envs where the control plane config is owned by the cloud provider.

here i mean the way RBAC is implemented as more of an example. it's an additive permission mechanism, via API resources, vs the static config which this feature utilizes. if the cluster-admin role could add permissions for new paths additively we shift the responsibility from the kube-apiserver administrator to the cluster administrator, which may not be the same people...it makes this feature N times more complex and less secure supposedly, but more flexible.

If kubeadm adopts this functionality (therefore changing the default of those environments), they should make it easy to configure since there is no distinction between the Kube API admin and the cluster operator in self-hosted clusters. The anonymous config needs to be coordinated with the runtime config of the cluster such as aggregated APIs.

also i would like to be clear that i don't disagree with the static config approach, and yes kubeadm can enable its internal patches mechanism to support extensibility on the cluster creation phase, but that would be a blocker for any tool deployed on top of kubeadm only via kubectl and YAML that wishes to enable additional anonymous auth endpoints. it has no way to modify the static config. (unless it also deploys a high-proviliged controller that manipulates files on control plane disk)

for managed solutions up the stack like GKE, AKS, TKG, we could reason about this feature enablement, but i don't think i would give a +1 to enable it by default in OSS kubeadm. we could write a hardening guide. but i'm just one of the votes, so maybe the rest of the kubeadm maintainers will disagree with me.