RFD0185: Kubernetes Access without Certificate-based Routing #47436

strideynet · 2024-10-10T11:37:46Z

Proposal to replace Certificate-attribute based routing in Kubernetes Access for performance and usability improvements. There's a very rough patch of what this change would look like at 47b55a8 courtesy of @tigrato

strideynet · 2024-10-11T09:38:33Z

rfd/0185-k8s-access-non-cert-routing.md

+
+## Security Considerations
+
+### Per-session MFA


My outstanding concern here relates to whether or not Per-Session MFA is meant to be locked to a specific target.

Today, the KubernetesCluster attribute seemingly has the side-effect of locking Per-Session MFA certificates to a specific target cluster. Looking back to the original Per-Session MFA RFD, the design does state that that should be the case, but says that a "TargetName" attribute should be used to enforce this - which I can't see any evidence of within the codebase today.

If locking Per-Session MFA certs to a specific cluster is intended/required, then we can retain the KubernetesCluster attribute, but make this intention more explicit by moving the code that enforces this into the authorizer rather than as part of the router/forwarder.

To close the loop here with #50567 now merged, after some discussion we decided to keep this guarantee in place with:

returning an error if URL routing parameters attempt to override certificate parameters, to ensure session MFA certs remain bound to the cluster for which they were issued

explicitly requiring non-empty identity routing parameters to use the path routing endpoint if an MFAVerified value is present on the cert. Path routing is functionally no-op due in this case to (1), but this won't force e.g. tsh to stop using the path route in kubeconfig.yaml

tigrato · 2024-10-11T09:48:57Z

rfd/0185-k8s-access-non-cert-routing.md

+encode attributes of an identity - information about which cluster you wish to
+connect to is not an attribute of identity!


I will approve the RFD just because of this comment

tigrato · 2024-10-11T10:06:37Z

rfd/0185-k8s-access-non-cert-routing.md

+```yaml
+apiVersion: v1
+clusters:
+- cluster:


It's important to note that this setup requires a separate cluster entry for each Kubernetes cluster. Currently, our implementation assigns one cluster per Teleport cluster rather than per Kubernetes cluster. This approach has a significant drawback: when two different Kubernetes clusters are part of the same Teleport cluster, kubectl caches objects in the same directory .kube/cache/discovery/teleport_proxy_port. This can lead to major issues due to version mismatches or incompatible CRDs.

The proposed solution addresses all these problems because kubectl will start caching data in .kube/cache/discovery/teleport_proxy_port/v1/teleport/base64_teleport_cluster_name/base64_kube_cluster_name

Check #12731

tigrato · 2024-10-11T10:08:10Z

rfd/0185-k8s-access-non-cert-routing.md

+Kubernetes clients:
+
+- `kubectl`
+- Python Kubernetes SDK


It's worth checking javascript client as we have some that also use it for automation.

tigrato · 2024-10-11T10:11:26Z

rfd/0185-k8s-access-non-cert-routing.md

+It would no longer be possible to track usage of Kubernetes Access for analytics
+purposes via the certificate generation event. However, we have other events
+that indicate Kubernetes Access usage (e.g an event per request), that are
+better suited for this purpose - so this seems acceptable.


Note that when kubectl performs discovery, it requests a single certificate but retrieves all API resource definitions. In theory, this could generate duplicate events, but Teleport disregards discovery requests for usage purposes, so nothing will change neither the metrics will be duplicated.

As for billing, since we count Kubernetes requests, this behavior won’t affect anything.

tigrato · 2024-10-11T10:15:31Z

rfd/0185-k8s-access-non-cert-routing.md

+  # are provided, then the output will contain the union of the results of the
+  # selectors.
+  selectors:
+    - name: my-cluster


Will this generate a separate kubeconfig for each cluster, or a single configuration that includes all clusters?

I prefer the latter, but it might be worth mentioning or offering both options. We should also pay attention to context and file naming. By default, tsh creates context entries using the format {{TeleportCluster}}-{{KubeCluster}}, but this can be customized by providing a template with the context override flag. It would be useful to allow users to adjust these settings to customize their outputs.

Yeah - following discussion with [CUSTOMER] - I figure it's easier to give an option between a single kubeconfig and a kubeconfig per cluster. I'm sure one you get into the thousands of clusters range, certain implementations of the Kube SDK will probably dislike the size of the config file.

By default, tsh creates context entries using the format {{TeleportCluster}}-{{KubeCluster}}, but this can be customized by providing a template with the context override flag. It would be useful to allow users to adjust these settings to customize their outputs.

This seems reasonable - we use the same context structure in tbot as well atm fwiw.

tigrato · 2024-10-11T10:18:32Z

rfd/0185-k8s-access-non-cert-routing.md

+
+This has the following limitations:
+
+- We've already witnessed limited support for specifying SNIs in Kubernetes


The main challenge is that by using SNI for routing and supporting kubectl, Teleport would need to generate certificates for *.rootCluster.kube.teleportproxy.com, as well as for leaf clusters *.leafCluster.kube.teleportproxy.com. Over time, this approach could introduce significant issues as the system evolves. Since we must support kubectl, the only viable solution is to send the identification via tls-server-name.

timothyb89 · 2024-12-03T03:31:07Z

rfd/0185-k8s-access-non-cert-routing.md

+Teleport clients would no longer need to call `GenerateUserCerts` with a
+`KubernetesCluster` to produce a certificate explicitly for access to
+Kubernetes. When producing a kubeconfig, the client should use the user's 
+standard identity.


Beyond just routing info (which this will be superseding) we also have a certificate usage restriction for Kubernetes, which the auth middleware currently requires for anything that touches the forwarder. Do you see the usage:kube restriction going away with this?

The usage was only configured when using KubeCSR, which was deprecated several versions ago.

Furthermore, if the identity does not have a usage specified, it will bypass any usage validation checks.

teleport/lib/auth/middleware.go

Lines 664 to 673 in b71f017

// If there is any restriction on the certificate usage

// reject the API server request. This is done so some classes

// of certificates issued for kubernetes usage by proxy, can not be used

// against auth server. Later on we can extend more

// advanced cert usage, but for now this is the safest option.

if len(identity.Usage) != 0 && !slices.Equal(a.AcceptedUsage, identity.Usage) {

log.Warningf("Restricted certificate of user %q with usage %v rejected while accessing the auth endpoint with acceptable usage %v.",

identity.Username, identity.Usage, a.AcceptedUsage)

return nil, trace.AccessDenied("access denied: invalid client certificate")

}

rfd/0185-k8s-access-non-cert-routing.md

This implements path-based routing for Kubernetes clusters as described by [RFD0185]. A new prefixed path handler is added that accepts base64-encoded Teleport and Kubernetes cluster names. The request is routed to the destination Teleport cluster using these parameters instead of those embedded in the session TLS identity, and then the preexisting handlers check authorization and complete the request as usual. This removes the need for certificates to be issued per Kubernetes cluster: so long as the incoming identity is granted access to the cluster via its roles, access can succeed, and no `KubernetesCluster` attribute or cert usage restrictions are needed. [RFD0185]: #47436

* Support routing to Kubernetes clusters by request path This implements path-based routing for Kubernetes clusters as described by [RFD0185]. A new prefixed path handler is added that accepts base64-encoded Teleport and Kubernetes cluster names. The request is routed to the destination Teleport cluster using these parameters instead of those embedded in the session TLS identity, and then the preexisting handlers check authorization and complete the request as usual. This removes the need for certificates to be issued per Kubernetes cluster: so long as the incoming identity is granted access to the cluster via its roles, access can succeed, and no `KubernetesCluster` attribute or cert usage restrictions are needed. [RFD0185]: #47436 * Remove routeSourcer in favor of identity values Also: fixes various other review comments, adds comment explaining auth strategy. * Revert more dead code * Revert more unnecessary changes; validate utf8 parameters * Build fixes after merge with slog transition * Add basic tests for Kubernetes path routing * Fix imports * Check identity parameters when per session MFA is enabled Also, don't allow path parameters to override the identity, if the contains nonempty routing params. * Fix failing TestSingleCertRouting due to improper reuse of rest config * Session MFA includes role based enforcement; added more test cases * Don't allow route params to be overwritten if MFAVerified is set * Code review feedback Deep clone the request and explicitly clear RawPath.

ravicious · 2025-01-28T09:43:50Z

Just read this following Tim's status update and it all sounds really nice. Being able to weave this into kube APIs is *chef's kiss*. If only this was possible for TCP apps. 🥲

tigrato · 2025-01-28T09:45:48Z

Just read this following Tim's status update and it all sounds really nice. Being able to weave this into kube APIs is *chef's kiss*. If only this was possible for TCP apps. 🥲

This is possible to expand to tcp apps using SNI

…#51534) * Support routing to Kubernetes clusters by request path (#50567) * Support routing to Kubernetes clusters by request path This implements path-based routing for Kubernetes clusters as described by [RFD0185]. A new prefixed path handler is added that accepts base64-encoded Teleport and Kubernetes cluster names. The request is routed to the destination Teleport cluster using these parameters instead of those embedded in the session TLS identity, and then the preexisting handlers check authorization and complete the request as usual. This removes the need for certificates to be issued per Kubernetes cluster: so long as the incoming identity is granted access to the cluster via its roles, access can succeed, and no `KubernetesCluster` attribute or cert usage restrictions are needed. [RFD0185]: #47436 * Remove routeSourcer in favor of identity values Also: fixes various other review comments, adds comment explaining auth strategy. * Revert more dead code * Revert more unnecessary changes; validate utf8 parameters * Build fixes after merge with slog transition * Add basic tests for Kubernetes path routing * Fix imports * Check identity parameters when per session MFA is enabled Also, don't allow path parameters to override the identity, if the contains nonempty routing params. * Fix failing TestSingleCertRouting due to improper reuse of rest config * Session MFA includes role based enforcement; added more test cases * Don't allow route params to be overwritten if MFAVerified is set * Code review feedback Deep clone the request and explicitly clear RawPath. * Undo slog for v17 compatibility

tigrato

@strideynet can we merge this?

strideynet · 2025-02-06T10:31:16Z

@strideynet can we merge this?

Needs one more review by the looks of it, then I'll merge.

strideynet added 8 commits October 10, 2024 10:11

Add blank rfd to reserve number

eccdfa8

Scratch out rough overview of the RFD

1bafba4

Add section on client compat

ad7e8a0

Add notes on back/fore compat

ff2ddba

Add section on audit and analytics

59a13f3

Add notes on SNI

c6c7766

Add test results for kubectl/python sdk

18c88d4

Reorg

5fce33b

strideynet commented Oct 11, 2024

View reviewed changes

strideynet requested a review from tigrato October 11, 2024 09:40

tigrato reviewed Oct 11, 2024

View reviewed changes

strideynet added the no-changelog Indicates that a PR does not require a changelog entry label Oct 11, 2024

strideynet marked this pull request as ready for review October 14, 2024 11:00

github-actions bot added rfd Request for Discussion size/md labels Oct 14, 2024

timothyb89 reviewed Dec 3, 2024

View reviewed changes

timothyb89 reviewed Dec 6, 2024

View reviewed changes

rfd/0185-k8s-access-non-cert-routing.md Show resolved Hide resolved

timothyb89 mentioned this pull request Dec 24, 2024

Support routing to Kubernetes clusters by request path #50567

Merged

rosstimothy requested a review from creack January 13, 2025 22:01

timothyb89 mentioned this pull request Jan 28, 2025

[v17] Support routing to Kubernetes clusters by request path (#50567) #51534

Merged

tigrato approved these changes Feb 6, 2025

View reviewed changes

ryanclark approved these changes Feb 6, 2025

View reviewed changes

public-teleport-github-review-bot bot removed the request for review from creack February 6, 2025 10:37

strideynet added this pull request to the merge queue Feb 6, 2025

Merged via the queue into master with commit b7afa6a Feb 6, 2025
43 of 44 checks passed

strideynet deleted the rfd/185-kubernetes-access-cert-attribute-less-routing branch February 6, 2025 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFD0185: Kubernetes Access without Certificate-based Routing #47436

RFD0185: Kubernetes Access without Certificate-based Routing #47436

strideynet commented Oct 10, 2024 •

edited

Loading

strideynet Oct 11, 2024

timothyb89 Jan 28, 2025

tigrato Oct 11, 2024

tigrato Oct 11, 2024

tigrato Oct 11, 2024

tigrato Oct 11, 2024

tigrato Oct 11, 2024

strideynet Oct 11, 2024

tigrato Oct 11, 2024

timothyb89 Dec 3, 2024

tigrato Dec 3, 2024

ravicious commented Jan 28, 2025

tigrato commented Jan 28, 2025

tigrato left a comment

strideynet commented Feb 6, 2025

		encode attributes of an identity - information about which cluster you wish to
		connect to is not an attribute of identity!


		This has the following limitations:

		- We've already witnessed limited support for specifying SNIs in Kubernetes

	// If there is any restriction on the certificate usage
	// reject the API server request. This is done so some classes
	// of certificates issued for kubernetes usage by proxy, can not be used
	// against auth server. Later on we can extend more
	// advanced cert usage, but for now this is the safest option.
	if len(identity.Usage) != 0 && !slices.Equal(a.AcceptedUsage, identity.Usage) {
	log.Warningf("Restricted certificate of user %q with usage %v rejected while accessing the auth endpoint with acceptable usage %v.",
	identity.Username, identity.Usage, a.AcceptedUsage)
	return nil, trace.AccessDenied("access denied: invalid client certificate")
	}

RFD0185: Kubernetes Access without Certificate-based Routing #47436

RFD0185: Kubernetes Access without Certificate-based Routing #47436

Conversation

strideynet commented Oct 10, 2024 • 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

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

ravicious commented Jan 28, 2025

tigrato commented Jan 28, 2025

tigrato left a comment

Choose a reason for hiding this comment

strideynet commented Feb 6, 2025

strideynet commented Oct 10, 2024 •

edited

Loading