Browse Source

fix: prevent creation of specific type of secrets (#6280)

Gergely Bräutigam 1 month ago
parent
commit
4ddd240af7

+ 30 - 0
apis/externalsecrets/v1/externalsecret_validator.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
+	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -59,6 +61,10 @@ func validateExternalSecret(es *ExternalSecret) (admission.Warnings, error) {
 		errs = errors.Join(errs, errors.New("either data or dataFrom should be specified"))
 	}
 
+	if err := validatePrivilegedTemplate(es); err != nil {
+		errs = errors.Join(errs, err)
+	}
+
 	for _, ref := range es.Spec.DataFrom {
 		if err := validateExtractFindGenerator(ref); err != nil {
 			errs = errors.Join(errs, err)
@@ -116,6 +122,30 @@ func validatePolicies(es *ExternalSecret) error {
 	return errs
 }
 
+// validatePrivilegedTemplate rejects templates with specific types and annotations combinations
+// to prevent users from creating long-lived tokens beyond the scope of the defined RBAC.
+func validatePrivilegedTemplate(es *ExternalSecret) error {
+	tpl := es.Spec.Target.Template
+	if tpl == nil {
+		return nil
+	}
+	//nolint:exhaustive // don't need exhaustive
+	switch tpl.Type {
+	case corev1.SecretTypeServiceAccountToken:
+		if _, ok := tpl.Metadata.Annotations[corev1.ServiceAccountNameKey]; ok {
+			return fmt.Errorf("template.type=%q with annotation %q is not allowed", corev1.SecretTypeServiceAccountToken, corev1.ServiceAccountNameKey)
+		}
+		for _, tf := range tpl.TemplateFrom {
+			if strings.EqualFold(tf.Target, TemplateTargetAnnotations) {
+				return fmt.Errorf("template.type=%q with templateFrom target=%q is not allowed", corev1.SecretTypeServiceAccountToken, TemplateTargetAnnotations)
+			}
+		}
+	case corev1.SecretTypeBootstrapToken:
+		return fmt.Errorf("template.type=%q is not allowed", corev1.SecretTypeBootstrapToken)
+	}
+	return nil
+}
+
 func validateDuplicateKeys(es *ExternalSecret, errs error) error {
 	if es.Spec.Target.DeletionPolicy == DeletionPolicyRetain {
 		seenKeys := make(map[string]struct{})

+ 158 - 0
apis/externalsecrets/v1/externalsecret_validator_test.go

@@ -18,6 +18,8 @@ package v1
 
 import (
 	"testing"
+
+	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -202,6 +204,162 @@ either data or dataFrom should be specified`,
 			},
 			expectedErr: "duplicate secretKey found: SERVICE_NAME",
 		},
+		{
+			name: "service account token template with name annotation is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							Metadata: ExternalSecretTemplateMetadata{
+								Annotations: map[string]string{
+									corev1.ServiceAccountNameKey: "external-secrets",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="kubernetes.io/service-account-token" with annotation "kubernetes.io/service-account.name" is not allowed`,
+		},
+		{
+			name: "service account token template without name annotation is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "service account token template with templateFrom annotations target is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							TemplateFrom: []TemplateFrom{
+								{Target: TemplateTargetAnnotations},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="kubernetes.io/service-account-token" with templateFrom target="Annotations" is not allowed`,
+		},
+		{
+			name: "service account token template with lowercase templateFrom annotations target is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							TemplateFrom: []TemplateFrom{
+								{Target: "annotations"},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="kubernetes.io/service-account-token" with templateFrom target="Annotations" is not allowed`,
+		},
+		{
+			name: "service account token template with templateFrom data target is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							TemplateFrom: []TemplateFrom{
+								{Target: TemplateTargetData},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "bootstrap token template is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeBootstrapToken,
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="bootstrap.kubernetes.io/token" is not allowed`,
+		},
+		{
+			name: "service account name annotation without service account token type is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeOpaque,
+							Metadata: ExternalSecretTemplateMetadata{
+								Annotations: map[string]string{
+									corev1.ServiceAccountNameKey: "external-secrets",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

+ 29 - 0
apis/externalsecrets/v1beta1/externalsecret_validator.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 
+	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -59,6 +60,10 @@ func validateExternalSecret(es *ExternalSecret) (admission.Warnings, error) {
 		errs = errors.Join(errs, errors.New("either data or dataFrom should be specified"))
 	}
 
+	if err := validatePrivilegedTemplate(es); err != nil {
+		errs = errors.Join(errs, err)
+	}
+
 	for _, ref := range es.Spec.DataFrom {
 		if err := validateExtractFindGenerator(ref); err != nil {
 			errs = errors.Join(errs, err)
@@ -116,6 +121,30 @@ func validatePolicies(es *ExternalSecret) error {
 	return errs
 }
 
+// validatePrivilegedTemplate rejects templates with specific types and annotations combinations
+// to prevent users from creating long-lived tokens beyond the scope of the defined RBAC.
+func validatePrivilegedTemplate(es *ExternalSecret) error {
+	tpl := es.Spec.Target.Template
+	if tpl == nil {
+		return nil
+	}
+	//nolint:exhaustive // don't need exhaustive
+	switch tpl.Type {
+	case corev1.SecretTypeServiceAccountToken:
+		if _, ok := tpl.Metadata.Annotations[corev1.ServiceAccountNameKey]; ok {
+			return fmt.Errorf("template.type=%q with annotation %q is not allowed", corev1.SecretTypeServiceAccountToken, corev1.ServiceAccountNameKey)
+		}
+		for _, tf := range tpl.TemplateFrom {
+			if tf.Target == TemplateTargetAnnotations {
+				return fmt.Errorf("template.type=%q with templateFrom target=%q is not allowed", corev1.SecretTypeServiceAccountToken, TemplateTargetAnnotations)
+			}
+		}
+	case corev1.SecretTypeBootstrapToken:
+		return fmt.Errorf("template.type=%q is not allowed", corev1.SecretTypeBootstrapToken)
+	}
+	return nil
+}
+
 func validateDuplicateKeys(es *ExternalSecret, errs error) error {
 	if es.Spec.Target.DeletionPolicy == DeletionPolicyRetain {
 		seenKeys := make(map[string]struct{})

+ 135 - 0
apis/externalsecrets/v1beta1/externalsecret_validator_test.go

@@ -18,6 +18,8 @@ package v1beta1
 
 import (
 	"testing"
+
+	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -202,6 +204,139 @@ either data or dataFrom should be specified`,
 			},
 			expectedErr: "duplicate secretKey found: SERVICE_NAME",
 		},
+		{
+			name: "service account token template with name annotation is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							Metadata: ExternalSecretTemplateMetadata{
+								Annotations: map[string]string{
+									corev1.ServiceAccountNameKey: "external-secrets",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="kubernetes.io/service-account-token" with annotation "kubernetes.io/service-account.name" is not allowed`,
+		},
+		{
+			name: "service account token template without name annotation is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "service account token template with templateFrom annotations target is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							TemplateFrom: []TemplateFrom{
+								{Target: TemplateTargetAnnotations},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="kubernetes.io/service-account-token" with templateFrom target="Annotations" is not allowed`,
+		},
+		{
+			name: "service account token template with templateFrom data target is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeServiceAccountToken,
+							TemplateFrom: []TemplateFrom{
+								{Target: TemplateTargetData},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "bootstrap token template is rejected",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeBootstrapToken,
+						},
+					},
+				},
+			},
+			expectedErr: `template.type="bootstrap.kubernetes.io/token" is not allowed`,
+		},
+		{
+			name: "service account name annotation without service account token type is allowed",
+			obj: &ExternalSecret{
+				Spec: ExternalSecretSpec{
+					DataFrom: []ExternalSecretDataFromRemoteRef{
+						{
+							SourceRef: &StoreGeneratorSourceRef{
+								GeneratorRef: &GeneratorRef{},
+							},
+						},
+					},
+					Target: ExternalSecretTarget{
+						Template: &ExternalSecretTemplate{
+							Type: corev1.SecretTypeOpaque,
+							Metadata: ExternalSecretTemplateMetadata{
+								Annotations: map[string]string{
+									corev1.ServiceAccountNameKey: "external-secrets",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

+ 13 - 0
runtime/template/v2/template_test.go

@@ -837,6 +837,19 @@ func TestExecuteInvalidTemplateScope(t *testing.T) {
 	assert.ErrorContains(t, err, "expected 'Values' or 'KeysAndValues'")
 }
 
+// Target dispatch is case-insensitive; validators rely on this.
+func TestExecuteTargetCaseInsensitive(t *testing.T) {
+	for _, target := range []string{"Annotations", "annotations", "ANNOTATIONS", "AnNoTaTiOnS"} {
+		t.Run(target, func(t *testing.T) {
+			sec := &corev1.Secret{}
+			require.NoError(t, Execute(map[string][]byte{"foo": []byte("bar")}, nil, esapi.TemplateScopeValues, target, sec))
+			assert.Equal(t, "bar", sec.Annotations["foo"])
+			assert.Empty(t, sec.Labels)
+			assert.Empty(t, sec.Data)
+		})
+	}
+}
+
 func TestScopeKeysAndValues(t *testing.T) {
 	tbl := []struct {
 		name               string