diff --git a/internal/controller/tlsconfigutil/ca_bundle.go b/internal/controller/tlsconfigutil/ca_bundle.go index c2726f3f3..48df83c96 100644 --- a/internal/controller/tlsconfigutil/ca_bundle.go +++ b/internal/controller/tlsconfigutil/ca_bundle.go @@ -5,6 +5,9 @@ import ( "crypto/x509" ) +var sHA256OfEmptyData = sha256.Sum256(nil) +var zeroSHA256 = [32]byte{} + // CABundle abstracts the internal representation of CA certificate bundles. type CABundle struct { caBundle []byte @@ -22,31 +25,41 @@ func NewCABundle(caBundle []byte, certPool *x509.CertPool) *CABundle { // GetCABundle returns the CA certificate bundle PEM bytes. func (c *CABundle) GetCABundle() []byte { + if c == nil { + return nil + } return c.caBundle } // GetCABundlePemString returns the certificate bundle PEM formatted as a string. func (c *CABundle) GetCABundlePemString() string { + if c == nil { + return "" + } return string(c.caBundle) } // GetCertPool returns a X509 cert pool with the CA certificate bundle. func (c *CABundle) GetCertPool() *x509.CertPool { + if c == nil { + return nil + } return c.certPool } // GetCABundleHash returns a sha256 sum of the CA bundle bytes. func (c *CABundle) GetCABundleHash() [32]byte { - return sha256.Sum256(c.caBundle) // note that this will always return the same hash for nil input + if c == nil || len(c.caBundle) < 1 { + return sHA256OfEmptyData + } + // 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 } // IsEqual returns whether a CABundle has the same CA certificate bundle as another. func (c *CABundle) IsEqual(other *CABundle) bool { - if c == nil && other == nil { - return true - } - if c == nil || other == nil { - return false - } - return sha256.Sum256(c.caBundle) == sha256.Sum256(other.GetCABundle()) + return c.GetCABundleHash() == other.GetCABundleHash() } diff --git a/internal/controller/tlsconfigutil/ca_bundle_test.go b/internal/controller/tlsconfigutil/ca_bundle_test.go index 1265bbf37..942a01ef5 100644 --- a/internal/controller/tlsconfigutil/ca_bundle_test.go +++ b/internal/controller/tlsconfigutil/ca_bundle_test.go @@ -10,6 +10,84 @@ import ( "go.pinniped.dev/internal/certauthority" ) +func TestGetCABundle(t *testing.T) { + t.Run("returns the CA bundle", func(t *testing.T) { + caBundle := NewCABundle([]byte("here are some bytes"), nil) + + require.Equal(t, []byte("here are some bytes"), caBundle.GetCABundle()) + }) + + t.Run("handles nil receiver by returning nil", func(t *testing.T) { + var nilCABundle *CABundle + var expected []byte + require.Equal(t, expected, nilCABundle.GetCABundle()) + }) +} + +func TestGetCABundlePemString(t *testing.T) { + t.Run("returns the CA bundle PEM string", func(t *testing.T) { + caBundle := NewCABundle([]byte("here is a string"), nil) + + require.Equal(t, "here is a string", caBundle.GetCABundlePemString()) + }) + + t.Run("handles nil receiver by returning empty sstring", func(t *testing.T) { + var nilCABundle *CABundle + require.Equal(t, "", nilCABundle.GetCABundlePemString()) + }) +} + +func TestGetCertPool(t *testing.T) { + t.Run("returns the cert pool", func(t *testing.T) { + aCertPool := x509.NewCertPool() + caBundle := NewCABundle(nil, aCertPool) + + require.Equal(t, aCertPool, caBundle.GetCertPool()) + }) + + t.Run("handles nil receiver by returning nil", func(t *testing.T) { + var nilCABundle *CABundle + var expected *x509.CertPool + require.Equal(t, expected, nilCABundle.GetCertPool()) + }) +} + +func TestGetCABundleHash(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} + + // 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"), nil) + + require.Equal(t, sha256OfTest, caBundle.GetCABundleHash()) + }) + + t.Run("handles nil receiver by returning the hash of nil", func(t *testing.T) { + var nilCABundle *CABundle + + require.Equal(t, sha256OfNil, nilCABundle.GetCABundleHash()) + }) + + t.Run("handles improperly initialized receiver by returning the hash of nil", func(t *testing.T) { + caBundle := &CABundle{} + + require.Equal(t, sha256OfNil, caBundle.GetCABundleHash()) + }) + + t.Run("handles improperly initialized receiver by computing the hash", func(t *testing.T) { + caBundle := &CABundle{ + caBundle: []byte("test"), + } + + require.Equal(t, sha256OfTest, caBundle.GetCABundleHash()) + }) +} + func TestCABundleIsEqual(t *testing.T) { testCA, err := certauthority.New("Test CA", 1*time.Hour) require.NoError(t, err) @@ -29,39 +107,27 @@ func TestCABundleIsEqual(t *testing.T) { expected: true, }, { - name: "should return not equal when left is nil and right is not", + name: "should return equal when left is nil and right is empty", left: nil, right: &CABundle{}, - expected: false, - }, - { - name: "should return not equal when right is nil and left is not", - left: &CABundle{}, - right: nil, - expected: false, - }, - { - name: "should return equal when both left and right have same CA certificate bytes", - left: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, - right: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, expected: true, }, { - name: "should return not equal when both left and right do not have same CA certificate bytes", - left: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, - right: &CABundle{ - caBundle: []byte("something that is not a cert"), - certPool: nil, - }, + name: "should return equal when right is nil and left is empty", + left: &CABundle{}, + right: nil, + expected: true, + }, + { + name: "should return equal when both left and right have same CA certificate bytes", + left: NewCABundle(testCA.Bundle(), certPool), + right: NewCABundle(testCA.Bundle(), certPool), + expected: true, + }, + { + name: "should return not equal when both left and right do not have same CA certificate bytes", + left: NewCABundle(testCA.Bundle(), certPool), + right: NewCABundle([]byte("something that is not a cert"), certPool), expected: false, }, } @@ -69,8 +135,8 @@ func TestCABundleIsEqual(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - actual := tt.left.IsEqual(tt.right) - require.Equal(t, tt.expected, actual) + require.Equal(t, tt.expected, tt.left.IsEqual(tt.right)) + require.Equal(t, tt.expected, tt.right.IsEqual(tt.left)) }) } } diff --git a/internal/controller/tlsconfigutil/tls_config_util_test.go b/internal/controller/tlsconfigutil/tls_config_util_test.go index 9c38d7715..1ffe8b729 100644 --- a/internal/controller/tlsconfigutil/tls_config_util_test.go +++ b/internal/controller/tlsconfigutil/tls_config_util_test.go @@ -64,10 +64,7 @@ func TestValidateTLSConfig(t *testing.T) { tlsSpec: &TLSSpec{ CertificateAuthorityData: base64EncodedBundle, }, - expectedCABundle: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, + expectedCABundle: NewCABundle(testCA.Bundle(), certPool), expectedCondition: &metav1.Condition{ Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, @@ -138,10 +135,7 @@ func TestValidateTLSConfig(t *testing.T) { }, }, }, - expectedCABundle: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, + expectedCABundle: NewCABundle(testCA.Bundle(), certPool), expectedCondition: &metav1.Condition{ Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, @@ -171,10 +165,7 @@ func TestValidateTLSConfig(t *testing.T) { }, }, }, - expectedCABundle: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, + expectedCABundle: NewCABundle(testCA.Bundle(), certPool), expectedCondition: &metav1.Condition{ Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue, @@ -403,10 +394,7 @@ func TestValidateTLSConfig(t *testing.T) { }, }, }, - expectedCABundle: &CABundle{ - caBundle: testCA.Bundle(), - certPool: certPool, - }, + expectedCABundle: NewCABundle(testCA.Bundle(), certPool), expectedCondition: &metav1.Condition{ Type: typeTLSConfigurationValid, Status: metav1.ConditionTrue,