From 7ae2163be7b2cfb9ce7959c7bf165cf144434651 Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Fri, 27 Mar 2026 09:45:26 -0400 Subject: [PATCH] fixing scoped kube token validation of service accounts (#64952) --- api/types/provisioning.go | 3 + api/types/provisioning_test.go | 20 ++++ lib/scopes/joining/token.go | 7 +- lib/scopes/joining/token_test.go | 36 ++++++ lib/services/provisioning.go | 37 +++++- lib/services/provisioning_test.go | 192 ++++++++++++++++++++++++++++++ 6 files changed, 291 insertions(+), 4 deletions(-) create mode 100644 lib/services/provisioning_test.go diff --git a/api/types/provisioning.go b/api/types/provisioning.go index 54c378c8e14..b58c5d5b0cf 100644 --- a/api/types/provisioning.go +++ b/api/types/provisioning.go @@ -847,6 +847,9 @@ func (a *ProvisionTokenSpecV2CircleCI) checkAndSetDefaults() error { return nil } +// validates the given Kubernetes configuration and sets defaults if necessary. Additional validations applied +// during marshal/write can be found in lib/services/provisioning.go:strongValidateProvisionTokenWithDefaults(). +// Scoped variants of these validations are found in lib/scopes/joining/token.go:validateKubernetes() func (a *ProvisionTokenSpecV2Kubernetes) checkAndSetDefaults() error { if len(a.Allow) == 0 { return trace.BadParameter("allow: at least one rule must be set") diff --git a/api/types/provisioning_test.go b/api/types/provisioning_test.go index 17d1ec4bfea..a3711589caf 100644 --- a/api/types/provisioning_test.go +++ b/api/types/provisioning_test.go @@ -580,6 +580,26 @@ func TestProvisionTokenV2_CheckAndSetDefaults(t *testing.T) { }, wantErr: true, }, + { + desc: "kubernetes: too many parts in service account name", + token: &ProvisionTokenV2{ + Metadata: Metadata{ + Name: "test", + }, + Spec: ProvisionTokenSpecV2{ + Roles: []SystemRole{RoleNode}, + JoinMethod: JoinMethodKubernetes, + Kubernetes: &ProvisionTokenSpecV2Kubernetes{ + Allow: []*ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: "too:many:parts", + }, + }, + }, + }, + }, + wantErr: true, + }, { desc: "kubernetes: allow rule blank", token: &ProvisionTokenV2{ diff --git a/lib/scopes/joining/token.go b/lib/scopes/joining/token.go index f23500612b7..984abb36a9b 100644 --- a/lib/scopes/joining/token.go +++ b/lib/scopes/joining/token.go @@ -51,6 +51,8 @@ const ( TokenUsageModeUnlimited = "unlimited" ) +// validates the given Kubernetes configuration. Also implemented by +// lib/services/provisioning.go:strongValidateProvisionTokenWithDefaults() for unscoped tokens func validateKubernetes(kube *joiningv1.Kubernetes) error { if kube == nil || len(kube.GetAllow()) == 0 { return trace.BadParameter("at least one allow rule must be set") @@ -61,11 +63,10 @@ func validateKubernetes(kube *joiningv1.Kubernetes) error { return trace.BadParameter("allow[%d].service_account must be set", i) } - namespace, name, found := strings.Cut(rule.ServiceAccount, ":") - if !found || namespace == "" || name == "" { + parts := strings.Split(rule.ServiceAccount, ":") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { return trace.BadParameter("allow[%d].service_account should be in format \"namespace:service_account\", got %q instead", i, rule.ServiceAccount) } - } switch types.KubernetesJoinType(kube.GetType()) { diff --git a/lib/scopes/joining/token_test.go b/lib/scopes/joining/token_test.go index fe69a5ecd88..19f9b2e4560 100644 --- a/lib/scopes/joining/token_test.go +++ b/lib/scopes/joining/token_test.go @@ -332,6 +332,42 @@ func TestValidateScopedToken(t *testing.T) { expectedStrongErr: "allow[0].service_account should be in format \"namespace:service_account\"", expectedWeakErr: "allow[0].service_account should be in format \"namespace:service_account\"", }, + { + name: "kubernetes token with service account allow rule made up of too many parts", + modFn: func(tok *joiningv1.ScopedToken) { + tok.Spec.JoinMethod = string(types.JoinMethodKubernetes) + tok.Spec.Kubernetes = &joiningv1.Kubernetes{ + Allow: []*joiningv1.Kubernetes_Rule{{ServiceAccount: "too:many:parts"}}, + Type: string(types.KubernetesJoinTypeInCluster), + } + }, + expectedStrongErr: "allow[0].service_account should be in format \"namespace:service_account\"", + expectedWeakErr: "allow[0].service_account should be in format \"namespace:service_account\"", + }, + { + name: "kubernetes token with service account allow rule with empty account name", + modFn: func(tok *joiningv1.ScopedToken) { + tok.Spec.JoinMethod = string(types.JoinMethodKubernetes) + tok.Spec.Kubernetes = &joiningv1.Kubernetes{ + Allow: []*joiningv1.Kubernetes_Rule{{ServiceAccount: "namespace:"}}, + Type: string(types.KubernetesJoinTypeInCluster), + } + }, + expectedStrongErr: "allow[0].service_account should be in format \"namespace:service_account\"", + expectedWeakErr: "allow[0].service_account should be in format \"namespace:service_account\"", + }, + { + name: "kubernetes token with service account allow rule with empty namespace", + modFn: func(tok *joiningv1.ScopedToken) { + tok.Spec.JoinMethod = string(types.JoinMethodKubernetes) + tok.Spec.Kubernetes = &joiningv1.Kubernetes{ + Allow: []*joiningv1.Kubernetes_Rule{{ServiceAccount: ":service_account"}}, + Type: string(types.KubernetesJoinTypeInCluster), + } + }, + expectedStrongErr: "allow[0].service_account should be in format \"namespace:service_account\"", + expectedWeakErr: "allow[0].service_account should be in format \"namespace:service_account\"", + }, { name: "kubernetes token with unrecognized join type", modFn: func(tok *joiningv1.ScopedToken) { diff --git a/lib/services/provisioning.go b/lib/services/provisioning.go index 839a946fd5f..d6845bb027f 100644 --- a/lib/services/provisioning.go +++ b/lib/services/provisioning.go @@ -20,6 +20,7 @@ package services import ( "context" + "strings" "time" "github.com/gravitational/trace" @@ -116,6 +117,40 @@ func UnmarshalProvisionToken(data []byte, opts ...MarshalOption) (types.Provisio return nil, trace.BadParameter("server resource version %v is not supported", h.Version) } +// strongValidateProvisionTokenWithDefaults checks if the provision token is valid and sets defaults if necessary.. +func strongValidateProvisionTokenWithDefaults(token *types.ProvisionTokenV2) error { + if err := token.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + // for now there are no additional, on-write validations for token types other than kubernetes + if token.GetJoinMethod() != types.JoinMethodKubernetes { + return nil + } + + kube := token.GetKubernetes() + if kube == nil { + // technically should never happen since CheckAndSetDefaults() performs a similar check, + // but we'll be defensive just in case + return trace.BadParameter("allow: at least one rule must be set") + } + + for i, rule := range kube.Allow { + // validation for empty namespace and account was added much later than the rest of the validations + // in CheckAndSetDefaults(), so we only enforce them when marshaling a token rather than when unmarshaling + namespace, account, _ := strings.Cut(rule.ServiceAccount, ":") + if namespace == "" || account == "" { + return trace.BadParameter( + `allow[%d].service_account: name of service account should be in format "namespace:service_account", got %q instead`, + i, + rule.ServiceAccount, + ) + } + } + + return nil +} + // MarshalProvisionToken marshals the ProvisionToken resource to JSON. func MarshalProvisionToken(provisionToken types.ProvisionToken, opts ...MarshalOption) ([]byte, error) { cfg, err := CollectOptions(opts) @@ -125,7 +160,7 @@ func MarshalProvisionToken(provisionToken types.ProvisionToken, opts ...MarshalO switch provisionToken := provisionToken.(type) { case *types.ProvisionTokenV2: - if err := provisionToken.CheckAndSetDefaults(); err != nil { + if err := strongValidateProvisionTokenWithDefaults(provisionToken); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/services/provisioning_test.go b/lib/services/provisioning_test.go new file mode 100644 index 00000000000..8d0196ecb23 --- /dev/null +++ b/lib/services/provisioning_test.go @@ -0,0 +1,192 @@ +/* + * Teleport + * Copyright (C) 2026 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package services_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/utils" +) + +// TestMarshalingProvisionTokenKube tests validation cases specific to kubernetes provision tokens. +func TestMarshalingProvisionTokenKube(t *testing.T) { + t.Parallel() + cases := []struct { + name string + token types.ProvisionToken + wantMarshalErr bool + wantUnmarshalErr bool + }{ + { + name: "valid kube token", + token: &types.ProvisionTokenV2{ + Version: types.V2, + Metadata: types.Metadata{ + Name: "kube-token", + }, + Spec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + JoinMethod: types.JoinMethodKubernetes, + Kubernetes: &types.ProvisionTokenSpecV2Kubernetes{ + Allow: []*types.ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: "namespace:service_account", + }, + }, + }, + }, + }, + wantMarshalErr: false, + wantUnmarshalErr: false, + }, + { + name: "too many parts in service account name", + token: &types.ProvisionTokenV2{ + Version: types.V2, + Metadata: types.Metadata{ + Name: "kube-token", + }, + Spec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + JoinMethod: types.JoinMethodKubernetes, + Kubernetes: &types.ProvisionTokenSpecV2Kubernetes{ + Allow: []*types.ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: "too:many:parts", + }, + }, + }, + }, + }, + wantMarshalErr: true, + wantUnmarshalErr: true, + }, + { + name: "missing account name", + token: &types.ProvisionTokenV2{ + Version: types.V2, + Metadata: types.Metadata{ + Name: "kube-token", + }, + Spec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + JoinMethod: types.JoinMethodKubernetes, + Kubernetes: &types.ProvisionTokenSpecV2Kubernetes{ + Allow: []*types.ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: "namespace:", + }, + }, + }, + }, + }, + // this is a newer validation, so we should expect unmarshaling to succeed in order + // to prevent existing tokens that are now malformed from causing issues + wantUnmarshalErr: false, + wantMarshalErr: true, + }, + { + name: "missing namespace", + token: &types.ProvisionTokenV2{ + Version: types.V2, + Metadata: types.Metadata{ + Name: "kube-token", + }, + Spec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + JoinMethod: types.JoinMethodKubernetes, + Kubernetes: &types.ProvisionTokenSpecV2Kubernetes{ + Allow: []*types.ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: ":service_account", + }, + }, + }, + }, + }, + // this is a newer validation, so we should expect unmarshaling to succeed in order + // to prevent existing tokens that are now malformed from causing issues + wantUnmarshalErr: false, + wantMarshalErr: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + // since there are test cases where MarshalProvisionToken would fail but UnmarshalProvisionToken + // would not, we use utils.FastMarshal directly to ensure we can always properly assert for both + // paths + data, err := utils.FastMarshal(c.token) + require.NoError(t, err) + + _, err = services.UnmarshalProvisionToken(data) + if c.wantUnmarshalErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + _, err = services.MarshalProvisionToken(c.token) + if c.wantMarshalErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestDefaultsAppliedDuringMarshal(t *testing.T) { + t.Parallel() + + token := &types.ProvisionTokenV2{ + Version: types.V2, + Metadata: types.Metadata{ + Name: "kube-token", + }, + Spec: types.ProvisionTokenSpecV2{ + Roles: []types.SystemRole{types.RoleNode}, + JoinMethod: types.JoinMethodKubernetes, + Kubernetes: &types.ProvisionTokenSpecV2Kubernetes{ + // a default value of "in_cluster" should be applied during + // marshaling when type is left unspecified + Type: types.KubernetesJoinTypeUnspecified, + Allow: []*types.ProvisionTokenSpecV2Kubernetes_Rule{ + { + ServiceAccount: "namespace:service_account", + }, + }, + }, + }, + } + + marshaled, err := services.MarshalProvisionToken(token) + require.NoError(t, err) + + unmarshaled, err := services.UnmarshalProvisionToken(marshaled) + require.NoError(t, err) + + require.Equal(t, types.KubernetesJoinTypeInCluster, unmarshaled.GetKubernetes().Type) +}