diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 241ef4fe7..df6fa5e3c 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -111,9 +111,9 @@ type tokenAuthenticatorCloser interface { type cachedJWTAuthenticator struct { authenticator.Token - spec *authenticationv1alpha1.JWTAuthenticatorSpec - caBundlePEMSHA256 [32]byte - cancel context.CancelFunc + spec *authenticationv1alpha1.JWTAuthenticatorSpec + caBundleHash tlsconfigutil.CABundleHash + cancel context.CancelFunc } func (c *cachedJWTAuthenticator) Close() { @@ -237,7 +237,7 @@ func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Co if jwtAuthenticatorFromCache != nil && reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &jwtAuthenticator.Spec) && tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status - jwtAuthenticatorFromCache.caBundlePEMSHA256 == caBundle.Hash() { + jwtAuthenticatorFromCache.caBundleHash.Equal(caBundle.Hash()) { c.log.WithValues("jwtAuthenticator", klog.KObj(jwtAuthenticator), "issuer", jwtAuthenticator.Spec.Issuer). Info("actual jwt authenticator and desired jwt authenticator are the same") // Stop, no more work to be done. This authenticator is already validated and cached. @@ -562,7 +562,7 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator( client *http.Client, spec *authenticationv1alpha1.JWTAuthenticatorSpec, keySet *coreosoidc.RemoteKeySet, - caBundlePEMSHA256 [32]byte, + caBundleHash tlsconfigutil.CABundleHash, conditions []*metav1.Condition, prereqOk bool, ) (*cachedJWTAuthenticator, []*metav1.Condition, error) { @@ -633,10 +633,10 @@ func (c *jwtCacheFillerController) newCachedJWTAuthenticator( Message: msg, }) return &cachedJWTAuthenticator{ - Token: oidcAuthenticator, - spec: spec, - caBundlePEMSHA256: caBundlePEMSHA256, - cancel: cancel, + Token: oidcAuthenticator, + spec: spec, + caBundleHash: caBundleHash, + cancel: cancel, }, conditions, nil } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 01489c69f..c6aaa0111 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -10,7 +10,6 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" - "crypto/sha256" "crypto/x509" _ "embed" "encoding/base64" @@ -45,6 +44,7 @@ import ( conciergefake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" conciergeinformers "go.pinniped.dev/generated/latest/client/concierge/informers/externalversions" "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/plog" @@ -2631,8 +2631,8 @@ func newCacheValue(t *testing.T, spec authenticationv1alpha1.JWTAuthenticatorSpe }) return &cachedJWTAuthenticator{ - spec: &spec, - caBundlePEMSHA256: sha256.Sum256([]byte(caBundle)), + spec: &spec, + caBundleHash: tlsconfigutil.NewCABundleHash([]byte(caBundle)), cancel: func() { wasClosed = true }, diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index 4da1c09f6..9a62122d3 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -61,8 +61,8 @@ const ( type cachedWebhookAuthenticator struct { authenticator.Token - spec *authenticationv1alpha1.WebhookAuthenticatorSpec - caBundlePEMSHA256 [32]byte + spec *authenticationv1alpha1.WebhookAuthenticatorSpec + caBundleHash tlsconfigutil.CABundleHash } // New instantiates a new controllerlib.Controller which will populate the provided authncache.Cache. @@ -162,8 +162,6 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co conditions := make([]*metav1.Condition, 0) caBundle, conditions, tlsBundleOk := c.validateTLSBundle(webhookAuthenticator.Spec.TLS, conditions) - caBundlePEMSHA256 := caBundle.Hash() - // Only revalidate and update the cache if the cached authenticator is different from the desired authenticator. // There is no need to repeat validations for a spec that was already successfully validated. We are making a // design decision to avoid repeating the validation which dials the server, even though the server's TLS @@ -177,7 +175,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co if webhookAuthenticatorFromCache != nil && reflect.DeepEqual(webhookAuthenticatorFromCache.spec, &webhookAuthenticator.Spec) && tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status - webhookAuthenticatorFromCache.caBundlePEMSHA256 == caBundlePEMSHA256 { + webhookAuthenticatorFromCache.caBundleHash.Equal(caBundle.Hash()) { c.log.WithValues("webhookAuthenticator", klog.KObj(webhookAuthenticator), "endpoint", webhookAuthenticator.Spec.Endpoint). Info("actual webhook authenticator and desired webhook authenticator are the same") // Stop, no more work to be done. This authenticator is already validated and cached. @@ -210,9 +208,9 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co c.cache.Delete(cacheKey) } else { c.cache.Store(cacheKey, &cachedWebhookAuthenticator{ - Token: newWebhookAuthenticatorForCache, - spec: webhookAuthenticator.Spec.DeepCopy(), // deep copy to avoid caching original object - caBundlePEMSHA256: caBundlePEMSHA256, + Token: newWebhookAuthenticatorForCache, + spec: webhookAuthenticator.Spec.DeepCopy(), // deep copy to avoid caching original object + caBundleHash: caBundle.Hash(), }) c.log.WithValues("webhook", klog.KObj(webhookAuthenticator), "endpoint", webhookAuthenticator.Spec.Endpoint). Info("added new webhook authenticator") diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 5b4990201..89f2b01b0 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -6,7 +6,6 @@ package webhookcachefiller import ( "bytes" "context" - "crypto/sha256" "crypto/tls" "encoding/base64" "encoding/json" @@ -39,6 +38,7 @@ import ( conciergeinformers "go.pinniped.dev/generated/latest/client/concierge/informers/externalversions" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controller/authenticator/authncache" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/plog" @@ -2030,8 +2030,8 @@ func newCacheValue(t *testing.T, spec authenticationv1alpha1.WebhookAuthenticato t.Helper() return &cachedWebhookAuthenticator{ - spec: &spec, - caBundlePEMSHA256: sha256.Sum256([]byte(caBundle)), + spec: &spec, + caBundleHash: tlsconfigutil.NewCABundleHash([]byte(caBundle)), } } diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index e4ff38e32..04501bd19 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -5,7 +5,6 @@ package activedirectoryupstreamwatcher import ( "context" - "crypto/sha256" "encoding/base64" "errors" "fmt" @@ -28,6 +27,7 @@ import ( supervisorinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" @@ -237,7 +237,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { testGroupSearchNameAttrName = "test-group-name-attr" caBundleConfigMapName = "test-ca-bundle-cm" - caBundleSecretName = "test-ca-bundle-secret" + caBundleSecretName = "test-ca-bundle-secret" //nolint:gosec // this is not a credential ) testValidSecretData := map[string][]byte{"username": []byte(testBindUsername), "password": []byte(testBindPassword)} @@ -517,7 +517,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -544,7 +544,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -571,7 +571,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -599,7 +599,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -800,7 +800,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(nil), + CABundleHash: tlsconfigutil.NewCABundleHash(nil), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -871,7 +871,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(nil), + CABundleHash: tlsconfigutil.NewCABundleHash(nil), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -948,7 +948,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, IDPSpecGeneration: 1234, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), ConnectionValidCondition: &metav1.Condition{ Type: "LDAPConnectionValid", Status: "True", @@ -1083,7 +1083,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(nil), + CABundleHash: tlsconfigutil.NewCABundleHash(nil), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1136,7 +1136,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1298,7 +1298,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1319,7 +1319,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1391,7 +1391,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), GroupSearchBase: testGroupSearchBase, IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), @@ -1414,7 +1414,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -1467,7 +1467,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -1487,7 +1487,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.StartTLS, IDPSpecGeneration: 1234, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithStartTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithStartTLS.CABundle), UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), @@ -1509,7 +1509,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithStartTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithStartTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1529,7 +1529,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1233, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1552,7 +1552,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1573,7 +1573,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { IDPSpecGeneration: 1234, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), // already previously validated with version 4242 SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), }}, @@ -1594,7 +1594,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1634,7 +1634,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1654,7 +1654,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4241")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1677,7 +1677,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1739,7 +1739,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), @@ -1805,7 +1805,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: exampleDefaultNamingContext, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -1870,7 +1870,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: exampleDefaultNamingContext, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -1935,7 +1935,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: exampleDefaultNamingContext, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -2092,7 +2092,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4241")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -2149,7 +2149,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, GroupSearchBase: exampleDefaultNamingContext, UserSearchBase: testUserSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInRootDSECondition(0))), @@ -2220,7 +2220,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(activeDirectoryConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), SearchBaseFoundCondition: condPtr(withoutTime(searchBaseFoundInConfigCondition(0))), diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index 47057c97b..762737235 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -6,9 +6,7 @@ package githubupstreamwatcher import ( "context" - "crypto/sha256" "crypto/tls" - "crypto/x509" "errors" "fmt" "net" @@ -74,8 +72,8 @@ type UpstreamGitHubIdentityProviderICache interface { } type GitHubValidatedAPICacheI interface { - MarkAsValidated(address string, caBundlePEM []byte) - IsValid(address string, caBundlePEM []byte) bool + MarkAsValidated(address string, caBundleHash tlsconfigutil.CABundleHash) + IsValid(address string, caBundleHash tlsconfigutil.CABundleHash) bool } type GitHubValidatedAPICache struct { @@ -83,24 +81,24 @@ type GitHubValidatedAPICache struct { } type GitHubValidatedAPICacheKey struct { - address string - caBundlePEMSHA256 [32]byte + address string + caBundleHash tlsconfigutil.CABundleHash } -func (g *GitHubValidatedAPICache) MarkAsValidated(address string, caBundlePEM []byte) { +func (g *GitHubValidatedAPICache) MarkAsValidated(address string, caBundleHash tlsconfigutil.CABundleHash) { key := GitHubValidatedAPICacheKey{ - address: address, - caBundlePEMSHA256: sha256.Sum256(caBundlePEM), + address: address, + caBundleHash: caBundleHash, } // Existence in the cache means it has been validated. // The TTL in the cache is not important, it's just a "really long time". g.cache.Set(key, nil, 365*24*time.Hour) } -func (g *GitHubValidatedAPICache) IsValid(address string, caBundlePEM []byte) bool { +func (g *GitHubValidatedAPICache) IsValid(address string, caBundleHash tlsconfigutil.CABundleHash) bool { key := GitHubValidatedAPICacheKey{ - address: address, - caBundlePEMSHA256: sha256.Sum256(caBundlePEM), + address: address, + caBundleHash: caBundleHash, } _, ok := g.cache.Get(key) return ok @@ -335,8 +333,7 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro githubConnectionCondition, hostURL, httpClient, githubConnectionErr := c.validateGitHubConnection( hostPort, - caBundle.PEMBytes(), - caBundle.CertPool(), + caBundle, hostCondition.Status == metav1.ConditionTrue, tlsConfigCondition.Status == metav1.ConditionTrue, ) @@ -425,11 +422,9 @@ func validateHost(gitHubAPIConfig idpv1alpha1.GitHubAPIConfig) (*metav1.Conditio }, &hostPort } -// TODO: this should take in a tlsconfigutil.CABundle func (c *gitHubWatcherController) validateGitHubConnection( hostPort *endpointaddr.HostPort, - caBundlePEM []byte, - certPool *x509.CertPool, + caBundle *tlsconfigutil.CABundle, hostConditionOk, tlsConfigConditionOk bool, ) (*metav1.Condition, string, *http.Client, error) { if !hostConditionOk || !tlsConfigConditionOk { @@ -443,8 +438,8 @@ func (c *gitHubWatcherController) validateGitHubConnection( address := hostPort.Endpoint() - if !c.validatedCache.IsValid(address, caBundlePEM) { - conn, tlsDialErr := c.dialFunc("tcp", address, ptls.Default(certPool)) + if !c.validatedCache.IsValid(address, caBundle.Hash()) { + conn, tlsDialErr := c.dialFunc("tcp", address, ptls.Default(caBundle.CertPool())) if tlsDialErr != nil { return &metav1.Condition{ Type: GitHubConnectionValid, @@ -457,14 +452,14 @@ func (c *gitHubWatcherController) validateGitHubConnection( _ = conn.Close() } - c.validatedCache.MarkAsValidated(address, caBundlePEM) + c.validatedCache.MarkAsValidated(address, caBundle.Hash()) return &metav1.Condition{ Type: GitHubConnectionValid, Status: metav1.ConditionTrue, Reason: conditionsutil.ReasonSuccess, Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", address), - }, fmt.Sprintf("https://%s", address), phttp.Default(certPool), nil + }, fmt.Sprintf("https://%s", address), phttp.Default(caBundle.CertPool()), nil } // buildDialErrorMessage standardizes DNS error messages that appear differently on different platforms, so that tests and log grepping is uniform. diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index 1486e4cfd..c1fb81400 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -6,7 +6,6 @@ package githubupstreamwatcher import ( "bytes" "context" - "crypto/sha256" "crypto/tls" "crypto/x509" "encoding/base64" @@ -40,6 +39,7 @@ import ( "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" "go.pinniped.dev/internal/federationdomain/upstreamprovider" @@ -451,8 +451,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -513,8 +513,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -591,8 +591,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: "github.com:443", - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: "github.com:443", + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -665,8 +665,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerIPv6Domain, - caBundlePEMSHA256: sha256.Sum256(goodServerIPv6CA), + address: goodServerIPv6Domain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerIPv6CA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -780,8 +780,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -961,8 +961,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ @@ -1058,8 +1058,8 @@ func TestController(t *testing.T) { }, preexistingValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingCache: []*upstreamgithub.ProviderConfig{ @@ -1087,8 +1087,8 @@ func TestController(t *testing.T) { }, wantValidatedCache: []GitHubValidatedAPICacheKey{ { - address: goodServerDomain, - caBundlePEMSHA256: sha256.Sum256(goodServerCA), + address: goodServerDomain, + caBundleHash: tlsconfigutil.NewCABundleHash(goodServerCA), }, }, wantResultingUpstreams: []idpv1alpha1.GitHubIdentityProvider{ diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go index ed3e6f6d5..653fce090 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher_test.go @@ -5,7 +5,6 @@ package ldapupstreamwatcher import ( "context" - "crypto/sha256" "encoding/base64" "errors" "fmt" @@ -27,6 +26,7 @@ import ( supervisorinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" @@ -236,7 +236,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { testGroupSearchNameAttrName = "test-group-name-attr" caBundleConfigMapName = "test-ca-bundle-cm" - caBundleSecretName = "test-ca-bundle-secret" + caBundleSecretName = "test-ca-bundle-secret" //nolint:gosec // this is not a credential ) testValidSecretData := map[string][]byte{"username": []byte(testBindUsername), "password": []byte(testBindPassword)} @@ -447,7 +447,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -474,7 +474,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -501,7 +501,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -528,7 +528,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -721,7 +721,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(nil), + CABundleHash: tlsconfigutil.NewCABundleHash(nil), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -789,7 +789,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: &metav1.Condition{ Type: "LDAPConnectionValid", @@ -910,7 +910,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(nil), + CABundleHash: tlsconfigutil.NewCABundleHash(nil), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -962,7 +962,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1015,7 +1015,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1035,7 +1035,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1054,7 +1054,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithStartTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithStartTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1074,7 +1074,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.StartTLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithStartTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithStartTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1092,7 +1092,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { BindSecretResourceVersion: "4242", LDAPConnectionProtocol: upstreamldap.TLS, IDPSpecGeneration: 1233, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, }}, @@ -1114,7 +1114,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1134,7 +1134,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { IDPSpecGeneration: 1234, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), // already previously validated with version 4242 }}, setupMocks: func(conn *mockldapconn.MockConn) { @@ -1154,7 +1154,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1193,7 +1193,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1237,7 +1237,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, @@ -1276,7 +1276,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(providerConfigForValidUpstreamWithTLS.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(providerConfigForValidUpstreamWithTLS.CABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}}, @@ -1338,7 +1338,7 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { LDAPConnectionProtocol: upstreamldap.TLS, UserSearchBase: testUserSearchBase, GroupSearchBase: testGroupSearchBase, - CABundlePEMSHA256: sha256.Sum256(testCABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(testCABundle), IDPSpecGeneration: 1234, ConnectionValidCondition: condPtr(ldapConnectionValidTrueConditionWithoutTimeOrGeneration("4242")), }}, diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 32cc333f1..02ef033e2 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -96,7 +96,7 @@ type UpstreamOIDCIdentityProviderICache interface { // oidcDiscoveryCacheKey is the type of keys in an oidcDiscoveryCache. type oidcDiscoveryCacheKey struct { issuer string - caBundleHash [32]byte + caBundleHash tlsconfigutil.CABundleHash } // oidcDiscoveryCacheValue is the type of cache entries in an oidcDiscoveryCache. @@ -359,7 +359,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id // Get the discovered provider and HTTP client from cache, if they are found in the cache. cacheKey := oidcDiscoveryCacheKey{ issuer: upstream.Spec.Issuer, - caBundleHash: caBundle.Hash(), // note that this will always return the same hash for nil input + caBundleHash: caBundle.Hash(), } if cacheEntry := c.validatorCache.getProvider(cacheKey); cacheEntry != nil { discoveredProvider = cacheEntry.provider diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index f63d01e28..baccac721 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -6,7 +6,6 @@ package oidcupstreamwatcher import ( "bytes" "context" - "crypto/sha256" "crypto/x509" "encoding/base64" "encoding/json" @@ -32,6 +31,7 @@ import ( supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" supervisorinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/controller/tlsconfigutil" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" "go.pinniped.dev/internal/federationdomain/upstreamprovider" @@ -1114,7 +1114,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { // without encountering any errors. cacheKey := oidcDiscoveryCacheKey{ issuer: testIssuerURL + "/this-path-does-not-exist", - caBundleHash: sha256.Sum256([]byte(testIssuerCA)), + caBundleHash: tlsconfigutil.NewCABundleHash([]byte(testIssuerCA)), } // Put it into the initial cache for this test. return map[oidcDiscoveryCacheKey]*oidcDiscoveryCacheValue{ diff --git a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go index bb55be45c..ac1ed370e 100644 --- a/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go +++ b/internal/controller/supervisorconfig/upstreamwatchers/upstream_watchers.go @@ -5,7 +5,6 @@ package upstreamwatchers import ( "context" - "crypto/sha256" "fmt" "time" @@ -41,9 +40,9 @@ const ( // ValidatedSettings is the struct which is cached by the ValidatedSettingsCacheI interface. type ValidatedSettings struct { - IDPSpecGeneration int64 // which IDP spec was used during the validation - BindSecretResourceVersion string // which bind secret was used during the validation - CABundlePEMSHA256 [32]byte // hash of the CA bundle used during the validation + IDPSpecGeneration int64 // which IDP spec was used during the validation + BindSecretResourceVersion string // which bind secret was used during the validation + CABundleHash tlsconfigutil.CABundleHash // hash of the CA bundle used during the validation // Cache the setting for TLS vs StartTLS. This is always auto-discovered by probing the server. LDAPConnectionProtocol upstreamldap.LDAPConnectionProtocol @@ -285,7 +284,7 @@ func validateAndSetLDAPServerConnectivityAndSearchBase( if hasPreviousValidatedSettings && validatedSettings.UserSearchBase != "" && validatedSettings.GroupSearchBase != "" && - validatedSettings.CABundlePEMSHA256 == sha256.Sum256(config.CABundle) { + validatedSettings.CABundleHash.Equal(tlsconfigutil.NewCABundleHash(config.CABundle)) { // Found previously validated settings in the cache (which is also not missing search base fields), so use them. config.ConnectionProtocol = validatedSettings.LDAPConnectionProtocol config.UserSearch.Base = validatedSettings.UserSearchBase @@ -313,7 +312,7 @@ func validateAndSetLDAPServerConnectivityAndSearchBase( validatedSettingsCache.Set(upstream.Name(), ValidatedSettings{ IDPSpecGeneration: upstream.Generation(), BindSecretResourceVersion: currentSecretVersion, - CABundlePEMSHA256: sha256.Sum256(config.CABundle), + CABundleHash: tlsconfigutil.NewCABundleHash(config.CABundle), LDAPConnectionProtocol: config.ConnectionProtocol, UserSearchBase: config.UserSearch.Base, GroupSearchBase: config.GroupSearch.Base, diff --git a/internal/controller/tlsconfigutil/ca_bundle.go b/internal/controller/tlsconfigutil/ca_bundle.go index 7ac7dd412..e4c878cc0 100644 --- a/internal/controller/tlsconfigutil/ca_bundle.go +++ b/internal/controller/tlsconfigutil/ca_bundle.go @@ -1,3 +1,6 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package tlsconfigutil import ( @@ -5,13 +8,24 @@ import ( "crypto/x509" ) -var sHA256OfEmptyData = sha256.Sum256(nil) -var zeroSHA256 = [32]byte{} +type CABundleHash struct { + hash [32]byte +} + +func NewCABundleHash(bundle []byte) CABundleHash { + return CABundleHash{ + hash: sha256.Sum256(bundle), + } +} + +func (a CABundleHash) Equal(b CABundleHash) bool { + return a == b +} // CABundle abstracts the internal representation of CA certificate bundles. type CABundle struct { caBundle []byte - sha256 [32]byte + sha256 CABundleHash certPool *x509.CertPool } @@ -26,7 +40,7 @@ func NewCABundle(caBundle []byte) (*CABundle, bool) { return &CABundle{ caBundle: caBundle, - sha256: sha256.Sum256(caBundle), + sha256: NewCABundleHash(caBundle), certPool: certPool, }, ok } @@ -56,13 +70,9 @@ func (c *CABundle) CertPool() *x509.CertPool { } // Hash returns a sha256 sum of the CA bundle bytes. -func (c *CABundle) Hash() [32]byte { - if c == nil || len(c.caBundle) < 1 { - return sHA256OfEmptyData +func (c *CABundle) Hash() CABundleHash { + if c == nil { + return NewCABundleHash(nil) } - // This handles improperly initialized receivers - if c.sha256 == zeroSHA256 { - c.sha256 = sha256.Sum256(c.caBundle) - } - return c.sha256 // note that this will always return the same hash for nil input + return c.sha256 } diff --git a/internal/controller/tlsconfigutil/ca_bundle_test.go b/internal/controller/tlsconfigutil/ca_bundle_test.go index d49f9b156..c52d7d932 100644 --- a/internal/controller/tlsconfigutil/ca_bundle_test.go +++ b/internal/controller/tlsconfigutil/ca_bundle_test.go @@ -1,7 +1,9 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package tlsconfigutil import ( - "crypto/sha256" "crypto/x509" "testing" "time" @@ -11,6 +13,37 @@ import ( "go.pinniped.dev/internal/certauthority" ) +func TestNewCABundleHash(t *testing.T) { + sha256OfNil := CABundleHash{hash: [32]byte{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}} + + // On the command line, `echo "test" | shasum -a 256` yields "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + // which is 32 bytes of data encoded as 64 characters. + // https://stackoverflow.com/a/70565837 + // This is the actual binary data: + sha256OfTest := CABundleHash{hash: [32]byte{159, 134, 208, 129, 136, 76, 125, 101, 154, 47, 234, 160, 197, 90, 208, 21, 163, 191, 79, 27, 43, 11, 130, 44, 209, 93, 108, 21, 176, 240, 10, 8}} + + t.Run("will hash the data given", func(t *testing.T) { + caBundleHash := NewCABundleHash([]byte("test")) + + require.True(t, sha256OfTest.Equal(caBundleHash)) + require.Equal(t, sha256OfTest, caBundleHash) + }) + + t.Run("will return the hash of nil input", func(t *testing.T) { + caBundleHash := NewCABundleHash(nil) + + require.True(t, sha256OfNil.Equal(caBundleHash)) + require.Equal(t, sha256OfNil, caBundleHash) + }) + + t.Run("will return the hash of empty input", func(t *testing.T) { + caBundleHash := NewCABundleHash([]byte{}) + + require.True(t, sha256OfNil.Equal(caBundleHash)) + require.Equal(t, sha256OfNil, caBundleHash) + }) +} + func TestNewCABundle(t *testing.T) { testCA, err := certauthority.New("Test CA", 1*time.Hour) require.NoError(t, err) @@ -20,7 +53,7 @@ func TestNewCABundle(t *testing.T) { require.True(t, ok) require.Equal(t, testCA.Bundle(), caBundle.PEMBytes()) - require.Equal(t, sha256.Sum256(testCA.Bundle()), caBundle.Hash()) + require.Equal(t, NewCABundleHash(testCA.Bundle()), caBundle.Hash()) require.Equal(t, string(testCA.Bundle()), caBundle.PEMString()) require.True(t, testCA.Pool().Equal(caBundle.CertPool()), "should be the cert pool of the testCA") }) @@ -30,7 +63,7 @@ func TestNewCABundle(t *testing.T) { require.False(t, ok) require.Equal(t, []byte("here are some bytes"), caBundle.PEMBytes()) - require.Equal(t, sha256.Sum256([]byte("here are some bytes")), caBundle.Hash()) + require.Equal(t, NewCABundleHash([]byte("here are some bytes")), caBundle.Hash()) require.Equal(t, "here are some bytes", caBundle.PEMString()) require.True(t, x509.NewCertPool().Equal(caBundle.CertPool()), "should be an empty cert pool") }) @@ -113,49 +146,27 @@ func TestCertPool(t *testing.T) { } func TestHash(t *testing.T) { - sha256OfNil := [32]uint8{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55} + t.Run("returns the Hash of the given CA bundle", func(t *testing.T) { + caBundle, _ := NewCABundle([]byte("this is a CA bundle")) - // On the command line, `echo "test" | shasum -a 256` yields "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", - // which is 32 bytes of data encoded as 64 characters. - // https://stackoverflow.com/a/70565837 - // This is the actual binary data: - sha256OfTest := [32]byte{159, 134, 208, 129, 136, 76, 125, 101, 154, 47, 234, 160, 197, 90, 208, 21, 163, 191, 79, 27, 43, 11, 130, 44, 209, 93, 108, 21, 176, 240, 10, 8} - - t.Run("returns the SHA256", func(t *testing.T) { - caBundle, _ := NewCABundle([]byte("test")) - - require.Equal(t, sha256OfTest, caBundle.Hash()) + require.True(t, NewCABundleHash([]byte("this is a CA bundle")).Equal(caBundle.Hash())) }) - t.Run("returns the SHA256 when the PEM is nil", func(t *testing.T) { + t.Run("returns the Hash of nil when the CA bundle is nil", func(t *testing.T) { caBundle, _ := NewCABundle(nil) - require.Equal(t, sha256OfNil, caBundle.Hash()) + require.True(t, NewCABundleHash(nil).Equal(caBundle.Hash())) }) - t.Run("returns the SHA256 when the PEM is empty", func(t *testing.T) { + t.Run("returns the Hash of nil when the CA bundle is empty", func(t *testing.T) { caBundle, _ := NewCABundle([]byte{}) - require.Equal(t, sha256OfNil, caBundle.Hash()) + require.True(t, NewCABundleHash(nil).Equal(caBundle.Hash())) }) - t.Run("handles nil receiver by returning the hash of nil", func(t *testing.T) { + t.Run("returns the Hash of nil when the receiver is nil", func(t *testing.T) { var nilCABundle *CABundle - require.Equal(t, sha256OfNil, nilCABundle.Hash()) - }) - - t.Run("handles improperly initialized receiver by returning the hash of nil", func(t *testing.T) { - caBundle := &CABundle{} - - require.Equal(t, sha256OfNil, caBundle.Hash()) - }) - - t.Run("handles improperly initialized receiver by computing the hash", func(t *testing.T) { - caBundle := &CABundle{ - caBundle: []byte("test"), - } - - require.Equal(t, sha256OfTest, caBundle.Hash()) + require.True(t, NewCABundleHash(nil).Equal(nilCABundle.Hash())) }) } diff --git a/internal/controller/tlsconfigutil/tls_config_util.go b/internal/controller/tlsconfigutil/tls_config_util.go index 986cc237b..42116a288 100644 --- a/internal/controller/tlsconfigutil/tls_config_util.go +++ b/internal/controller/tlsconfigutil/tls_config_util.go @@ -90,12 +90,6 @@ func ValidateTLSConfig( secretInformer corev1informers.SecretInformer, configMapInformer corev1informers.ConfigMapInformer, ) (*metav1.Condition, *CABundle) { - // TODO: This func should return a struct that abstracts away the internals of how a CA bundle is held in memory - // and can return the CA bundle as string PEM, []byte base64-encoded, CertPool, hash, etc, as well as compare itself - // to either a different struct instance or a hash. - // - // TODO: There could easily be a hash type struct alias for the specific hash value (e.g. "[32]byte") with an Equality function. - caBundle, err := buildCABundle(tlsSpec, conditionPrefix, namespace, secretInformer, configMapInformer) if err != nil { return invalidTLSCondition(err.Error()), nil diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 8828ccd9f..9beee329c 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -92,7 +92,6 @@ type ProviderConfig struct { ConnectionProtocol LDAPConnectionProtocol // PEM-encoded CA cert bundle to trust when connecting to the LDAP server. Can be nil. - // TODO: should this be a tlsconfigutil.CABundle? CABundle []byte // BindUsername is the username to use when performing a bind with the upstream LDAP IDP. @@ -373,7 +372,7 @@ func (p *Provider) tlsConfig() (*tls.Config, error) { return ptls.DefaultLDAP(rootCAs), nil } -// GetName returns a name for this upstream provider. +// GetResourceName returns a name for this upstream provider. func (p *Provider) GetResourceName() string { return p.c.Name }