From bb7e7fe81eea81081d08eb478fc6b350e97e4f6e Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 28 Apr 2021 13:49:42 -0400 Subject: [PATCH] webhookcachefiller: be stricter about CA bundle validation Signed-off-by: Monis Khan --- .../controller/authenticator/authenticator.go | 16 ++++++++++++++-- .../webhookcachefiller_test.go | 9 +++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/internal/controller/authenticator/authenticator.go b/internal/controller/authenticator/authenticator.go index 14af741d..2ac68500 100644 --- a/internal/controller/authenticator/authenticator.go +++ b/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 } diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 4d6a0744..aae172d9 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/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",