Allow custom audience for kubernetes in-cluster joining (#49528)

gravitational · Nov 28, 2024 · 53cba46 · 53cba46
1 parent 98c53d5
commit 53cba46
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 9 deletions.
diff --git a/lib/auth/join_kubernetes.go b/lib/auth/join_kubernetes.go
@@ -31,7 +31,7 @@ import (
 )
 
 type k8sTokenReviewValidator interface {
-	Validate(context.Context, string) (*kubernetestoken.ValidationResult, error)
+	Validate(ctx context.Context, token, clusterName string) (*kubernetestoken.ValidationResult, error)
 }
 
 type k8sJWKSValidator func(now time.Time, jwksData []byte, clusterName string, token string) (*kubernetestoken.ValidationResult, error)
@@ -52,14 +52,15 @@ func (a *Server) checkKubernetesJoinRequest(ctx context.Context, req *types.Regi
 		)
 	}
 
+	clusterName, err := a.GetDomainName()
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
 	// Switch to join method subtype token validation.
 	var result *kubernetestoken.ValidationResult
 	switch token.Spec.Kubernetes.Type {
 	case types.KubernetesJoinTypeStaticJWKS:
-		clusterName, err := a.GetDomainName()
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
 		result, err = a.k8sJWKSValidator(
 			a.clock.Now(),
 			[]byte(token.Spec.Kubernetes.StaticJWKS.JWKS),
@@ -70,7 +71,7 @@ func (a *Server) checkKubernetesJoinRequest(ctx context.Context, req *types.Regi
 			return nil, trace.WrapWithMessage(err, "reviewing kubernetes token with static_jwks")
 		}
 	case types.KubernetesJoinTypeInCluster, types.KubernetesJoinTypeUnspecified:
-		result, err = a.k8sTokenReviewValidator.Validate(ctx, req.IDToken)
+		result, err = a.k8sTokenReviewValidator.Validate(ctx, req.IDToken, clusterName)
 		if err != nil {
 			return nil, trace.WrapWithMessage(err, "reviewing kubernetes token with in_cluster")
 		}

diff --git a/lib/auth/join_kubernetes_test.go b/lib/auth/join_kubernetes_test.go
@@ -35,7 +35,7 @@ type mockK8STokenReviewValidator struct {
 	tokens map[string]*kubernetestoken.ValidationResult
 }
 
-func (m *mockK8STokenReviewValidator) Validate(_ context.Context, token string) (*kubernetestoken.ValidationResult, error) {
+func (m *mockK8STokenReviewValidator) Validate(_ context.Context, token, _ string) (*kubernetestoken.ValidationResult, error) {
 	result, ok := m.tokens[token]
 	if !ok {
 		return nil, errMockInvalidToken

diff --git a/lib/kubernetestoken/token_validator.go b/lib/kubernetestoken/token_validator.go
@@ -46,6 +46,8 @@ const (
 	// Kubernetes should support bound tokens on 1.20 and 1.21,
 	// but we can have an apiserver running 1.21 and kubelets running 1.19.
 	kubernetesBoundTokenSupportVersion = "1.22.0"
+	// kubernetesAudience is the Kubernetes default audience put on SA tokens if we don't specify one.
+	kubernetesAudience = "https://kubernetes.default.svc"
 )
 
 type ValidationResult struct {
@@ -110,7 +112,7 @@ func (v *TokenReviewValidator) getClient() (kubernetes.Interface, error) {
 }
 
 // Validate uses the Kubernetes TokenReview API to validate a token and return its UserInfo
-func (v *TokenReviewValidator) Validate(ctx context.Context, token string) (*ValidationResult, error) {
+func (v *TokenReviewValidator) Validate(ctx context.Context, token, clusterName string) (*ValidationResult, error) {
 	client, err := v.getClient()
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -119,6 +121,11 @@ func (v *TokenReviewValidator) Validate(ctx context.Context, token string) (*Val
 	review := &v1.TokenReview{
 		Spec: v1.TokenReviewSpec{
 			Token: token,
+			// In-cluster used to only allow tokens with the kubernetes audience
+			// But people kept confusing it with JWKS and set the cluster name
+			// as the audience/. To avoid his common footgun we now allow tokens
+			// whose audience is the teleport cluster name.
+			Audiences: []string{kubernetesAudience, clusterName},
 		},
 	}
 	options := metav1.CreateOptions{}

diff --git a/lib/kubernetestoken/token_validator_test.go b/lib/kubernetestoken/token_validator_test.go
@@ -41,6 +41,8 @@ import (
 	"github.com/gravitational/teleport/lib/cryptosuites"
 )
 
+const testClusterName = "teleport.example.com"
+
 var userGroups = []string{"system:serviceaccounts", "system:serviceaccounts:namespace", "system:authenticated"}
 
 var boundTokenKubernetesVersion = version.Info{
@@ -65,6 +67,7 @@ func tokenReviewMock(t *testing.T, reviewResult *v1.TokenReview) func(ctest.Acti
 		require.True(t, ok)
 
 		require.Equal(t, reviewResult.Spec.Token, reviewRequest.Spec.Token)
+		require.ElementsMatch(t, reviewRequest.Spec.Audiences, []string{kubernetesAudience, testClusterName})
 		return true, reviewResult, nil
 	}
 }
@@ -238,7 +241,7 @@ func TestIDTokenValidator_Validate(t *testing.T) {
 			v := TokenReviewValidator{
 				client: client,
 			}
-			result, err := v.Validate(context.Background(), tt.token)
+			result, err := v.Validate(context.Background(), tt.token, testClusterName)
 			if tt.expectedError != nil {
 				require.ErrorIs(t, err, tt.expectedError)
 				return