Skip to content
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

Preserve roles from user cert when forwarding k8s requests #3624

Merged
merged 3 commits into from
May 5, 2020

Conversation

awly
Copy link
Contributor

@awly awly commented Apr 23, 2020

When a proxy forwards a k8s request to another proxy in a trusted leaf
cluster, it dynamically generates a new client key/cert to impersonate
the user. This key/cert is presented to the leaf proxy as if a user was
directly making a request to it.

Auth server, when processing the CSR, would load user identity from the
backend. If this user had temporary role grants via workflow API, they
would not be loaded from the backend.

This means that if a user accesses k8s through:

kubectl -1-> root proxy -2-> leaf proxy -3-> k8s

the 2nd hop would drop their temporary role grants.

To fix this, preserve full user identity from the original client cert
presented to root proxy, as encoded in CSR Subject. The auth server
implicitly trusts the CSR Subject that the proxy presents.
This also requires fixing the Subject encoding on the proxy side, which
was inconsistent with how auth server does it.

One possible downside here is: a compromised proxy can mint k8s client certs for
arbitrary users with arbitrary (but existing) roles.

Fixes #3593

@awly awly requested review from klizhentas and russjones April 23, 2020 18:01
@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch from cd3521e to a34965e Compare April 23, 2020 18:02
Copy link
Contributor

@webvictim webvictim left a comment

Choose a reason for hiding this comment

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

This is great. Please wait for others before merging though.

@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch 2 times, most recently from 674ae8a to 6bae48e Compare April 23, 2020 21:59
@russjones
Copy link
Contributor

@awly I don't suppose kubeconfig supports configuration which would allow us to use SNI to solve this problem? If it did, we could just connect the client and target proxy directly instead of going through certificate re-issue.

@awly
Copy link
Contributor Author

awly commented Apr 24, 2020

@russjones interesting idea!
I believe kubectl uses the standard crypto/tls under the hood. crypto/tls always sends SNI unless InsecureSkipVerify is set or if ServerName contains an IP.

If we used SNI, it wouldn't work with:

  • proxies sharing the same hostname (using different ports)
  • root proxies that only have a public IP and not a hostname

which could be fine, but it's an awkward caveat to document

@webvictim
Copy link
Contributor

A lot of people's Teleport PoCs/trials don't use domains set up with proper TLS at all, they're using IP addresses and --insecure. It would almost certainly make that sort of use case harder.

@fspmarshall
Copy link
Contributor

Since this fix relies on updated proxy behavior, version compatibility is a concern. The auth server should fallback to the old strategy if it doesn't receive any roles from the proxy. You can add a // DELETE IN: 5.0 directive to the fallback logic.

One possible downside here is: a compromised proxy can mint k8s client certs for
arbitrary users with arbitrary (but existing) roles.

If a certificate was constructed in part based on access requests, the request IDs are preserved under the teleport-active-requests extension. The extension is currently only used by tsh, but it exists and could technically be used to solve this problem. Doesn't prevent a malicious proxy from deciding to impersonate a different user though, so its an open question whether the extra complexity is worth it. Also, keeping the user's cert as the source of truth is more consistent with how teleport operates everywhere else.

@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch from 6bae48e to 4242478 Compare April 24, 2020 20:12
@awly
Copy link
Contributor Author

awly commented Apr 24, 2020

A lot of people's Teleport PoCs/trials don't use domains set up with proper TLS at all, they're using IP addresses and --insecure. It would almost certainly make that sort of use case harder.

@webvictim would or wouldn't make it harder? I imagine we want to make trying the product easy but ensure that prod setups use good security practices.

Since this fix relies on updated proxy behavior, version compatibility is a concern. The auth server should fallback to the old strategy if it doesn't receive any roles from the proxy.

@fspmarshall ahh, good catch. I'll add some heuristic in auth server to detect a CSR from older proxy and fall back to manually producing the Subject. Stand by.

@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch from 4242478 to d11728a Compare April 28, 2020 17:00
@webvictim
Copy link
Contributor

@webvictim would or wouldn't make it harder? I imagine we want to make trying the product easy but ensure that prod setups use good security practices.

I suppose it wouldn't really make it harder. I agree with your overall assessment - an easyish trial but great production setup is exactly we want to do. Trialling the product without using DNS and valid TLS certs is already a tricky experience so it's unlikely to make a lot of difference.

@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch 2 times, most recently from d4c9fc7 to 302184c Compare April 29, 2020 18:55
@awly
Copy link
Contributor Author

awly commented Apr 29, 2020

@klizhentas @russjones added UsageKubeOnly enforcement as we discussed.

@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch 5 times, most recently from 96c5c4a to c196219 Compare April 30, 2020 21:05
Andrew Lytvynov added 2 commits May 5, 2020 14:44
When a proxy forwards a k8s request to another proxy in a trusted leaf
cluster, it dynamically generates a new client key/cert to impersonate
the user. This key/cert is presented to the leaf proxy as if a user was
directly making a request to it.

Auth server, when processing the CSR, would load user identity from the
backend. If this user had temporary role grants via workflow API, they
would not be loaded from the backend.

This means that if a user accesses k8s through:
```
kubectl -> root proxy -> leaf proxy -> k8s
```
the second hop would drop their temporary role grants.

To fix this, preserve full user identity from the original client cert
presented to root proxy, as encoded in CSR Subject. The auth server
implicitly trusts the CSR Subject that the proxy presents.
This also requires fixing the Subject encoding on the proxy side, which
was inconsistent with how auth server does it.

One downside here is: a compromised proxy can mint k8s client certs for
known users with arbitrary roles.
Auth server has to trust the proxy with all k8s CSR fields. Usage is the
only field it can enforce, to somewhat limit the blast radius of
compromising this cert flow.
@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch 2 times, most recently from c5981bb to 70442a1 Compare May 5, 2020 22:04
There seem to be some test stubs in `lib/auth` that can generate TLS
key/cert but without supporting k8s users/groups.
Rewrite cert generation in `kube_integration_test.go` to use the same
logic as `auth.Server.ProcessKubeCSR`.
@awly awly force-pushed the andrew/k8s-forwarder-preserve-roles branch from 70442a1 to f41714f Compare May 5, 2020 23:36
@awly awly merged commit 1a09ce4 into master May 5, 2020
@awly awly deleted the andrew/k8s-forwarder-preserve-roles branch May 5, 2020 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leaf Cluster not Mapping Approved Roles
4 participants