proposal: external client-go auth providers #1503

ericchiang · 2017-12-14T02:00:28Z

Updates kubernetes/kubernetes#55968
Updates kubernetes/kubernetes#14362

cc @kubernetes/sig-auth-proposals
cc @mattmoyer
cc @ericavonb

liggitt · 2017-12-15T05:24:57Z

@kubernetes/sig-cli-proposals

liggitt · 2017-12-15T05:25:24Z

@kubernetes/sig-api-machinery-proposals for client-go

liggitt · 2017-12-15T05:33:26Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+    // UpdateTransportConfig updates a config by adding a transport wrapper,
+    // setting a bearer token (should ignore if one is already set), or adding
+    // TLS client certificate credentials.
+    UpdateTransportConfig(c *transport.Config)


is this called just once at process start?

Yes, but it can still instrument or wrap the transport. Will add a comment.

ericavonb · 2017-12-15T18:14:04Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+
+# Proposal
+
+This proposal builds off of earlier requests to [support exec-based plugins](


In terms of encouraging good behaviors, does having an exec option encourage providers to bypass greater user transparency, cross-compatibility and standardization via plugging in some executable? As opposed to making their auth compatible with OIDC or something (assuming client-go/kubectl handles tokens and refreshing them properly)

I've seen more people willing to wrap kubectl then implement OIDC (or ask their IdP to implement OIDC). The goal of this feature is to ensure users use kubectl the same way, which I think is worth the trade-off.

I still think there's still incentive to implement an open standard to not require an exec plugin. I even floated the idea of a "generic oauth2" provider a while back kubernetes/kubernetes#35530 (comment)

Long story short, for EKS we would very much appreciate not having to conform to OIDC and being able to exec a binary (specifically heptio authenticator).

ericavonb · 2017-12-15T18:23:26Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+    Name   string            `json:"name"`
+    Config map[string]string `json:"config"`
+
+    // Exec should only be provided if Name and Config aren't.


If Exec and (Name,Config) are mutually exclusive, would it make more sense to move the Exec config up a level to AuthInfo instead? It makes some sense to me to add a "dynamic" token (or cert) option along side Token and TokenFile.

I don't mean to over complicate the issue, it just seems a bit complicated from a user's perspective. It would make more sense to me if either this were its own named auth provider (e.g. Name: "exec"), moved out of the auth provider section, or allowed to specify a named auth provider as well with some sort of defined login on how that case is handled.

Agreed this is an awkward place. I'll move it a level up.

ericavonb · 2017-12-15T18:42:32Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+        args: ["token", "-i", "(CLUSTER_ID)", "-r", "(ROLE_ARN)"]
+```
+
+## TLS client certificate support


This seems like a related but separate change from the executable plugin. It is not necessary to break it out into its own proposal but I wanted to note that one does not block the other from moving forward.

Thanks, added a comment.

nckturner · 2017-12-15T19:30:01Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+
+# Objective
+
+This document describes a credential rotation strategy for kubectl and client-go


Just wanted to call out that we would like to be able to use this for kube-proxy and kubelet as well, to authenticate nodes with AWS IAM credentials.

Can you expand a little on that? Why would you want to give kube-proxy IAM credentials to talk to the API server instead of a regular service account?

if it's supported by client-go, then kubeproxy and kubelet would pick it up automatically (though kubelet would not be able to take advantage of the nodeauthorizer/noderestriction components unless an IAM-to-node-name mapping could be determined)

The stress on kubectl in this proposal is a bit misleading. The change would necessarily be in client-go, thus supported by any component that installs the plugin. Why call out kubectl in the title? Why not just discuss kubectl specifically in the login section?

Why call out kubectl?

Most of the use cases I was thinking about were about kubectl users. But you're right this should be more general to client-go.

@ericchiang, our node identity is essentially IAM credentials that are present on the node, so it makes sense to us to use those directly for authentication. If its already in client-go, why not use it?

@liggitt Right, the heptio authenticator provides the ability to map IAM role to node name.

@liggitt Right, the heptio authenticator provides the ability to map IAM role to node name.

ah, great

ccojocar

Please define the format (including version) of the output generated by the authenticator tool.

ccojocar · 2017-12-18T08:59:29Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+
+This document describes a credential rotation strategy for kubectl and client-go
+using an exec-based plugin mechanism.
+


I am a bit concern about the complexity which this approach will bring, that I my opinion shades the benefits. The kubectl is currently self-contained and can be easily installed. One will need to install and to configure an additional authenticator tool in multiple machines maybe with different operating systems. Also there are case when people uses parallel cloud providers, they will need to install an additional tool for each of them. I imagine that the authenticator will require its own configuration separated from kubectl.

From security perspective, it will get more complicated to secure the entire setup. An attacker can leverage in certain scenarios (e.g wrong permissions + client credential flow) the authenticator tool to get access to the entire cluster.

I don't think having every IdP integration compiled into kubectl is going to scale. This is in-line with other efforts around kubernetes/kubernetes to move cloud provider, or other kind of external project specific integrations, out of core in favor of extension points.

Additionally, sig-auth often doesn't have the expertise to maintain or comment on the security soundness of integrations with system's we're not familiar with. There are trade-offs, but I don't think today's situation is a good place for these plugins.

There's a bigger conversation here about what to do with the existing in-tree plugins, but I think that's going to have to come after we introduce an extension point.

It sounds good. Hopefully the use will not be overwhelm by the number of external authenticators.

ccojocar · 2017-12-18T09:06:50Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+proposes that we should add this as a first-class feature of kubectl. Specifically,
+kubectl (and client-go) should be able to receive credentials by executing a
+command and reading that command's stdout.
+


The format of the output must be defined to ensure interoperability between kubectl/client-go and the external authenticator. I guess a JSON schema with a version would be sufficient. This could prevent cases when the authentication could be broken by a change on the kubectl/client-go side.

Alternatively we support jsonpath and integrator define their own scheme. I'm not sure if it's better but we do this in the gcp plugin today: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go#L77-L84

I think @cosmincojocar is talking about using a structured, versioned API so we can ask for additional data beside the credentials in the future.

Do we really need versioned output? Do we expect the tool to pass things like expiry to us? I expected just printing the bearer token to be sufficient. We expect some of these tools to not know anything about kubectl.

If we don't go with a structured stdout API, I'd be in favor of some way of parsing the output like jsonpath.

Or maybe get directly in the output a base64 encoded JWT.

Do you plan to cache the token on kubectl/client-go side and invoke the external authenticator only when the token is about to expire? Or do you want to invoke the external authenticator every time a token is required?

I think, the authenticator can persist its tokens in a file after the initial authentication, especially in the case when the refresh token is supported. It can execute later on only the refresh flow when the token is close to the expiration.

I suspect that there are authenticators which do not support the refresh flow (e.g. client credentials). In that case it will request a new token from IdP instead of refresh it. Also in this case a sort of caching will be helpful in order to avoid overwhelming the IdP.

I know that these details are up to the authenticator implementation, but they will have an impact in the kubectl/client-go performance and reliability.

For kubectl, I'd rather leave it up to the external authenticator to handle persisting tokens in a file. Some might not want the token to be saved to disk or need other special handling for their credentials.

There's room for better cache/refresh handling in general but I don't think it needs to be directly coupled to this change.

Added some comments on why we went for an unversioned output.

ccojocar · 2017-12-18T09:16:59Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+specification supported but a subset of vendors (OpenID Connect).
+
+External tools that do rotation generally have to be called on a regular bases in
+case the current credentials have expired, which leads to wrapping `kubectl` with


This is not required if the refresh token login is built into the plugin. For instance Azure Active Directory plugin does this without any external wrapping see: https://github.com/kubernetes/client-go/blob/1b75876d45719a84085528ee621939b0c68a4b85/plugin/pkg/client/auth/azure/azure.go#L276

We would like to be able to support refreshing credentials for additional providers who do not have a plugin compiled in. It's not feasible to require every new provider or enterprise with custom tooling to submit a PR to kubernetes and we would like to discourage forking or wrapping kubectl in an alias as is often currently done. (not to say this proposal is not the only solution to this problem)

ccojocar · 2017-12-18T09:24:28Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+either by opening a browser, or performing a text based prompt.
+
+We should take care that interactive stderr and stdin are correctly
+inherited by the sub-process to enable this kind of interaction.


How would you distinguish between the cases when the authenticator process hangs or waits for the user input (e.g. interactive flow)? I guess a sort of execution timeout of the subprocess is required. On the other hand, kubectl has more control over this in case of a plugin.

I'd like to start with the subprocess implementing this logic and present appropriate feedback.

If this is interactive, the user will likely CTRL+c the execution anyway.

Made this clear.

perotinus · 2018-01-04T20:41:25Z

/cc @perotinus

I'm interested in following along with this for the cluster registry. We had discussed using the cluster registry to provide a way to bootstrap kubectl with credentials, so I'd like to make sure we can integrate with this proposal in a reasonable way.

perotinus · 2018-01-04T21:02:22Z

/uncc @perotinus

ericchiang · 2018-01-10T00:15:42Z

Proposal updated to address feedback.

ericavonb · 2018-01-10T16:18:05Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+
+# Motivation
+
+Kubernetes clients can provide three kind of credentials: bearer tokens, TLS


nit: s/kind/kinds/

ericavonb

my review probably doesn't mean anything formally, but lgtm! 😄

ericchiang · 2018-01-10T17:07:19Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+https://github.com/kubernetes/client-go/blob/kubernetes-1.9.0/plugin/pkg/client/auth/gcp/gcp.go#L230)
+and parses the output using JSONPath.
+
+In the future, client-go may write versioned data to the plugin's stdin to pass


Just realized that this section conflicts with the login section. They both need to read from stdin. I need to remove it.

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. client-go: add an exec-based client auth provider Updates kubernetes/enhancements#541 Implements kubernetes/community#1503 Closes kubernetes/kubernetes#57164 ```release-note client-go: alpha support for exec-based credential providers ``` /sig auth /kind feature Kubernetes-commit: cb9d6b51556a1677f262e35e4aded0051c424818

Frusty · 2018-04-24T16:00:54Z

contributors/design-proposals/auth/kubectl-exec-plugins.md

+makes it hard to express things like a list of arguments or list key/value environment
+variables. As such, `AuthInfo` should add another field which expresses the `exec`
+config. This has the benefit of a more natural structure, but the trade-off of not being
+compatible with the existing `kubectl config set-credentials` implementation.


Is there any way today to use kubectl to write below config into ~/.kube/config?

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. client-go: add an exec-based client auth provider Updates kubernetes/enhancements#541 Implements kubernetes/community#1503 Closes kubernetes/kubernetes#57164 ```release-note client-go: alpha support for exec-based credential providers ``` /sig auth /kind feature Kubernetes-commit: cb9d6b51556a1677f262e35e4aded0051c424818

…iders proposal: external client-go auth providers

ericchiang mentioned this pull request Dec 14, 2017

Support out-of-tree authentication providers kubernetes/kubernetes#55968

Closed

k8s-github-robot assigned liggitt and deads2k Dec 14, 2017

k8s-ci-robot added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Dec 15, 2017

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 15, 2017

liggitt reviewed Dec 15, 2017

View reviewed changes

ericavonb reviewed Dec 15, 2017

View reviewed changes

ericchiang mentioned this pull request Dec 15, 2017

OIDC User Login flow kubernetes/kubernetes#55514

Closed

nckturner reviewed Dec 15, 2017

View reviewed changes

ccojocar suggested changes Dec 18, 2017

View reviewed changes

k8s-ci-robot requested a review from perotinus January 4, 2018 20:41

k8s-ci-robot removed the request for review from perotinus January 4, 2018 21:02

ericchiang force-pushed the external-auth-providers branch from 76938f5 to 2a2f8f7 Compare January 10, 2018 00:12

ericavonb reviewed Jan 10, 2018

View reviewed changes

ericavonb approved these changes Jan 10, 2018

View reviewed changes

ericchiang commented Jan 10, 2018

View reviewed changes

ericchiang changed the title ~~proposal: external kubectl auth providers~~ proposal: external client-go auth providers Jan 10, 2018

ericchiang force-pushed the external-auth-providers branch from 2a2f8f7 to c2da875 Compare January 10, 2018 18:09

Frusty reviewed Apr 24, 2018

View reviewed changes

sczizzo mentioned this pull request May 11, 2020

idea: Credential plugin for kubectl gravitational/teleport#3690

Closed

MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021

Merge pull request kubernetes#1503 from ericchiang/external-auth-prov…

503c6db

…iders proposal: external client-go auth providers


		# Proposal

		This proposal builds off of earlier requests to [support exec-based plugins](


		# Objective

		This document describes a credential rotation strategy for kubectl and client-go


		This document describes a credential rotation strategy for kubectl and client-go
		using an exec-based plugin mechanism.


		# Motivation

		Kubernetes clients can provide three kind of credentials: bearer tokens, TLS

proposal: external client-go auth providers #1503

proposal: external client-go auth providers #1503

Conversation

ericchiang commented Dec 14, 2017 • edited Loading

liggitt commented Dec 15, 2017

liggitt commented Dec 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedanese Dec 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccojocar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perotinus commented Jan 4, 2018

perotinus commented Jan 4, 2018

ericchiang commented Jan 10, 2018

Choose a reason for hiding this comment

ericavonb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericchiang commented Dec 14, 2017 •

edited

Loading

mikedanese Dec 15, 2017 •

edited

Loading