From 50964c6677b1c3d5668dcb6ee8dc40ebfdf9f962 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 17 Dec 2020 15:30:26 -0800 Subject: [PATCH] Supervisor CSRF Secret has unique Type Signed-off-by: aram price --- .../generator/federation_domain_secrets.go | 12 ++ .../supervisorconfig/generator/generator.go | 104 ------------------ .../generator/secret_helper.go | 3 + .../generator/supervisor_secrets.go | 79 +++++++++++++ .../generator/supervisor_secrets_test.go | 4 +- test/integration/supervisor_secrets_test.go | 22 ++-- 6 files changed, 108 insertions(+), 116 deletions(-) delete mode 100644 internal/controller/supervisorconfig/generator/generator.go diff --git a/internal/controller/supervisorconfig/generator/federation_domain_secrets.go b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go index e9e169d1..879e488a 100644 --- a/internal/controller/supervisorconfig/generator/federation_domain_secrets.go +++ b/internal/controller/supervisorconfig/generator/federation_domain_secrets.go @@ -24,6 +24,10 @@ import ( "go.pinniped.dev/internal/plog" ) +const ( + federationDomainKind = "FederationDomain" +) + type federationDomainSecretsController struct { secretHelper SecretHelper secretRefFunc func(domain *configv1alpha1.FederationDomain) *corev1.LocalObjectReference @@ -236,3 +240,11 @@ func (c *federationDomainSecretsController) updateFederationDomain( return err }) } + +// isFederationDomainControllee returns whether the provided obj is controlled by an FederationDomain. +func isFederationDomainControllee(obj metav1.Object) bool { + controller := metav1.GetControllerOf(obj) + return controller != nil && + controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && + controller.Kind == federationDomainKind +} diff --git a/internal/controller/supervisorconfig/generator/generator.go b/internal/controller/supervisorconfig/generator/generator.go deleted file mode 100644 index d37ec235..00000000 --- a/internal/controller/supervisorconfig/generator/generator.go +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package generator - -import ( - "crypto/rand" - - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - - configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" -) - -const ( - federationDomainKind = "FederationDomain" -) - -func generateSymmetricKey() ([]byte, error) { - b := make([]byte, symmetricKeySize) - if _, err := rand.Read(b); err != nil { - return nil, err - } - return b, nil -} - -func isValid(secret *corev1.Secret, labels map[string]string) bool { - if secret.Type != symmetricSecretType { - return false - } - - data, ok := secret.Data[symmetricSecretDataKey] - if !ok { - return false - } - if len(data) != symmetricKeySize { - return false - } - - for key, value := range labels { - if secret.Labels[key] != value { - return false - } - } - - return true -} - -func secretDataFunc() (map[string][]byte, error) { - symmetricKey, err := generateKey() - if err != nil { - return nil, err - } - - return map[string][]byte{ - symmetricSecretDataKey: symmetricKey, - }, nil -} - -func generateSecret(namespace, name string, labels map[string]string, secretDataFunc func() (map[string][]byte, error), owner metav1.Object) (*corev1.Secret, error) { - secretData, err := secretDataFunc() - if err != nil { - return nil, err - } - - deploymentGVK := schema.GroupVersionKind{ - Group: appsv1.SchemeGroupVersion.Group, - Version: appsv1.SchemeGroupVersion.Version, - Kind: "Deployment", - } - - blockOwnerDeletion := true - isController := false - - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: deploymentGVK.GroupVersion().String(), - Kind: deploymentGVK.Kind, - Name: owner.GetName(), - UID: owner.GetUID(), - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - }, - }, - Labels: labels, - }, - Type: symmetricSecretType, - Data: secretData, - }, nil -} - -// isFederationDomainControllee returns whether the provided obj is controlled by an FederationDomain. -func isFederationDomainControllee(obj metav1.Object) bool { - controller := metav1.GetControllerOf(obj) - return controller != nil && - controller.APIVersion == configv1alpha1.SchemeGroupVersion.String() && - controller.Kind == federationDomainKind -} diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index c0e66fb7..75bfd7ac 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/internal/controller/supervisorconfig/generator/secret_helper.go @@ -27,6 +27,9 @@ type SecretHelper interface { } const ( + // SupervisorCSRFSigningKeySecretType is corev1.Secret.Type for the Supervisor's CSRF signing key Secret. + SupervisorCSRFSigningKeySecretType = "secrets.pinniped.dev/supervisor-csrf-signing-key" + // symmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper. symmetricSecretType = "secrets.pinniped.dev/symmetric" // symmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets.go b/internal/controller/supervisorconfig/generator/supervisor_secrets.go index 9f5ca1c4..b7325648 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets.go @@ -6,12 +6,14 @@ package generator import ( "context" + "crypto/rand" "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" @@ -143,3 +145,80 @@ func (c *supervisorSecretsController) updateSecret(ctx context.Context, newSecre return err }) } + +func generateSymmetricKey() ([]byte, error) { + b := make([]byte, symmetricKeySize) + if _, err := rand.Read(b); err != nil { + return nil, err + } + return b, nil +} + +func isValid(secret *corev1.Secret, labels map[string]string) bool { + if secret.Type != SupervisorCSRFSigningKeySecretType { + return false + } + + data, ok := secret.Data[symmetricSecretDataKey] + if !ok { + return false + } + if len(data) != symmetricKeySize { + return false + } + + for key, value := range labels { + if secret.Labels[key] != value { + return false + } + } + + return true +} + +func secretDataFunc() (map[string][]byte, error) { + symmetricKey, err := generateKey() + if err != nil { + return nil, err + } + + return map[string][]byte{ + symmetricSecretDataKey: symmetricKey, + }, nil +} + +func generateSecret(namespace, name string, labels map[string]string, secretDataFunc func() (map[string][]byte, error), owner metav1.Object) (*corev1.Secret, error) { + secretData, err := secretDataFunc() + if err != nil { + return nil, err + } + + deploymentGVK := schema.GroupVersionKind{ + Group: appsv1.SchemeGroupVersion.Group, + Version: appsv1.SchemeGroupVersion.Version, + Kind: "Deployment", + } + + blockOwnerDeletion := true + isController := false + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: deploymentGVK.GroupVersion().String(), + Kind: deploymentGVK.Kind, + Name: owner.GetName(), + UID: owner.GetUID(), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, + }, + }, + Labels: labels, + }, + Type: SupervisorCSRFSigningKeySecretType, + Data: secretData, + }, nil +} diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go index 2522fe7d..f0b75f7c 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go @@ -258,7 +258,7 @@ func TestSupervisorSecretsControllerSync(t *testing.T) { }, Labels: labels, }, - Type: "secrets.pinniped.dev/symmetric", + Type: "secrets.pinniped.dev/supervisor-csrf-signing-key", Data: map[string][]byte{ "key": generatedSymmetricKey, }, @@ -280,7 +280,7 @@ func TestSupervisorSecretsControllerSync(t *testing.T) { }, Labels: labels, }, - Type: "secrets.pinniped.dev/symmetric", + Type: "secrets.pinniped.dev/supervisor-csrf-signing-key", Data: map[string][]byte{ "key": otherGeneratedSymmetricKey, }, diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 25ad71dc..2866b340 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -40,7 +40,7 @@ func TestSupervisorSecrets(t *testing.T) { secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return env.SupervisorAppName + "-key" }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/supervisor-csrf-signing-key"), }, { name: "jwks", @@ -54,21 +54,21 @@ func TestSupervisorSecrets(t *testing.T) { secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.TokenSigningKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, { name: "state signature secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateSigningKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, { name: "state encryption secret", secretName: func(federationDomain *configv1alpha1.FederationDomain) string { return federationDomain.Status.Secrets.StateEncryptionKey.Name }, - ensureValid: ensureValidSymmetricKey, + ensureValid: ensureValidSymmetricSecretOfTypeFunc("secrets.pinniped.dev/symmetric"), }, } for _, test := range tests { @@ -160,10 +160,12 @@ func ensureValidJWKS(t *testing.T, secret *corev1.Secret) { require.True(t, foundActiveJWK, "could not find active JWK in JWKS: %s", jwks) } -func ensureValidSymmetricKey(t *testing.T, secret *corev1.Secret) { - t.Helper() - require.Equal(t, corev1.SecretType("secrets.pinniped.dev/symmetric"), secret.Type) - key, ok := secret.Data["key"] - require.Truef(t, ok, "secret data does not contain 'key': %s", secret.Data) - require.Equal(t, 32, len(key)) +func ensureValidSymmetricSecretOfTypeFunc(secretTypeValue string) func(*testing.T, *corev1.Secret) { + return func(t *testing.T, secret *corev1.Secret) { + t.Helper() + require.Equal(t, corev1.SecretType(secretTypeValue), secret.Type) + key, ok := secret.Data["key"] + require.Truef(t, ok, "secret data does not contain 'key': %s", secret.Data) + require.Equal(t, 32, len(key)) + } }