Merge pull request #588 from enj/enj/i/webhookcachefiller_ca

webhookcachefiller: be stricter about CA bundle validation

internal/controller/authenticator/authenticator.go

@@ -5,7 +5,9 @@
 package authenticator
 
 import (
+	"crypto/x509"
 	"encoding/base64"
+	"fmt"
 
 	auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
 )
@@ -22,8 +24,18 @@ type Closer interface {
 // nil CA bundle will be returned. If the provided spec contains a CA bundle that is not properly
 // encoded, an error will be returned.
 func CABundle(spec *auth1alpha1.TLSSpec) ([]byte, error) {
-	if spec == nil {
+	if spec == nil || len(spec.CertificateAuthorityData) == 0 {
 		return nil, nil
 	}
-	return base64.StdEncoding.DecodeString(spec.CertificateAuthorityData)
+
+	pem, err := base64.StdEncoding.DecodeString(spec.CertificateAuthorityData)
+	if err != nil {
+		return nil, err
+	}
+
+	if ok := x509.NewCertPool().AppendCertsFromPEM(pem); !ok {
+		return nil, fmt.Errorf("certificateAuthorityData is not valid PEM")
+	}
+
+	return pem, nil
 }

internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go

@@ -135,6 +135,15 @@ func TestNewWebhookAuthenticator(t *testing.T) {
 		require.EqualError(t, err, "invalid TLS configuration: illegal base64 data at input byte 7")
 	})
 
+	t.Run("invalid pem data", func(t *testing.T) {
+		res, err := newWebhookAuthenticator(&auth1alpha1.WebhookAuthenticatorSpec{
+			Endpoint: "https://example.com",
+			TLS:      &auth1alpha1.TLSSpec{CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("bad data"))},
+		}, ioutil.TempFile, clientcmd.WriteToFile)
+		require.Nil(t, res)
+		require.EqualError(t, err, "invalid TLS configuration: certificateAuthorityData is not valid PEM")
+	})
+
 	t.Run("valid config with no TLS spec", func(t *testing.T) {
 		res, err := newWebhookAuthenticator(&auth1alpha1.WebhookAuthenticatorSpec{
 			Endpoint: "https://example.com",