diff --git a/CHANGELOG.md b/CHANGELOG.md index a69ef337..26555ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ * `k8s.io/api` v0.29.1 -> v0.29.2 * `k8s.io/apimachinery` v0.29.1 -> v0.29.2 +### Improvements + +* Allow TLS client to use the host's root CA set when no CA certificates are provided and +`disable_local_ca_jwt` is true if running Vault in a Kubernetes pod. Additionally, validate the +configuration's provided CA PEM bundle. [GH-238](https://github.com/hashicorp/vault-plugin-auth-kubernetes/pull/238) + ## 0.18.0 (Feb 2, 2024) ### Changes diff --git a/backend.go b/backend.go index bb2a58ad..ea0d8b64 100644 --- a/backend.go +++ b/backend.go @@ -128,7 +128,16 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, return b, nil } -var getDefaultHTTPClient = cleanhttp.DefaultPooledClient +// getDefaultTLSConfig returns a http.Client with the supported, default +// tls.Config from getDefaultTLSConfig() set on the +// cleanhttp.DefaultPooledTransport() http.Transport. +func getDefaultHTTPClient() *http.Client { + transport := cleanhttp.DefaultPooledTransport() + transport.TLSClientConfig = getDefaultTLSConfig() + return &http.Client{ + Transport: transport, + } +} func getDefaultTLSConfig() *tls.Config { return &tls.Config{ @@ -433,21 +442,24 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { caCertBytes = []byte(data) } - certPool := x509.NewCertPool() - if len(caCertBytes) > 0 { + var certPool *x509.CertPool + if len(caCertBytes) == 0 { + // since the CA chain is not configured, we use the system's cert pool. + var err error + certPool, err = x509.SystemCertPool() + if err != nil { + return err + } + } else { + // since we have a CA chain configured, we create a new x509.CertPool with its + // contents. + certPool = x509.NewCertPool() if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok { b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail") } - } else { - // provide an empty certPool - b.Logger().Warn("No CA certificates configured, TLS verification will fail") - // TODO: think about supporting host root CA certificates via a configuration toggle, - // in which case RootCAs should be set to nil } - // only refresh the Root CAs if they have changed since the last full update. - if !b.tlsConfig.RootCAs.Equal(certPool) { - b.Logger().Trace("Root CA certificate pool has changed, updating the client's transport") + setTLSClientConfig := func() error { transport, ok := b.httpClient.Transport.(*http.Transport) if !ok { // should never happen @@ -456,6 +468,15 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { b.tlsConfig.RootCAs = certPool transport.TLSClientConfig = b.tlsConfig + return nil + } + + // only refresh the Root CAs if they have changed since the last full update. + if b.tlsConfig.RootCAs == nil { + return setTLSClientConfig() + } else if !b.tlsConfig.RootCAs.Equal(certPool) { + b.Logger().Trace("Root CA certificate pool has changed, updating the client's transport") + return setTLSClientConfig() } else { b.Logger().Trace("Root CA certificate pool is unchanged, no update required") } diff --git a/backend_test.go b/backend_test.go index b44fec34..5b3209db 100644 --- a/backend_test.go +++ b/backend_test.go @@ -25,6 +25,11 @@ func Test_kubeAuthBackend_updateTLSConfig(t *testing.T) { localCertPool := getTestCertPool(t, testLocalCACert) otherCertPool := getTestCertPool(t, testOtherCACert) + systemCertPool, err := x509.SystemCertPool() + if err != nil { + t.Fatal(err) + } + type testConfig struct { config *kubeConfig expectTLSConfig *tls.Config @@ -173,6 +178,23 @@ func Test_kubeAuthBackend_updateTLSConfig(t *testing.T) { }, wantErr: false, }, + { + name: "ca-certs-not-set", + httpClient: getDefaultHTTPClient(), + tlsConfig: getDefaultTLSConfig(), + configs: []testConfig{ + { + config: &kubeConfig{ + DisableLocalCAJwt: true, + }, + expectTLSConfig: &tls.Config{ + MinVersion: minTLSVersion, + RootCAs: systemCertPool, + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/common_test.go b/common_test.go index 90cb638c..fd589b12 100644 --- a/common_test.go +++ b/common_test.go @@ -104,5 +104,19 @@ pEDcN/HcXM47TP7XgWW0rfQli3RucuqMV7LHvvpiGIWwfutrK9g7Py91W2JbQCA0 D14XDzgsruCwlWAP1FMvLMIPhSknpIJd9Xql+0/Ae1yl9f3Uamj3mDtBKg8/U5nJ 0wU= -----END CERTIFICATE----- +` + + testInvalidCACert = ` +-----BEGIN CERTIFICATE----- +MIIC5zCCAc+gAwIBAgIBATANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwptaW5p +a3ViZUNBMB4XDTE3MDgxMDIzMTQ1NVoXDTI3MDgwODIzMTQ1NVowFTETMBEGA1UE +AxMKbWluaWt1YmVDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAN8d +w2p/KXRkm+vzOO0eT1vYBWP7fKsnng9/g5nnXAJlt9NxpOSolRcyItm/04R0E1jx +jpgsdzkybc+QU5ZiszOYN833/D5hCNVAABVivpDd2P8wVKXN46cB99e24etUVBqG +5aR0Ku3IBsJjCN9efhF+XRCA2gy/KaXMdKJhHfdtc8hCr7G9+2wO2G58FLmIfEyH +owviOGt0BSnCtMpsA8ZgGQyfqgSd5u466aCv6oj0MyzsMnfS38niM53Rlv4IY6ol +taYbWXtCNndQ2S687qE0qTCxhE95Bm2Nfkqct4R1798sJz83xNv8hALvxr/vPK/J +2XkIm3oo3YKG4n/CHXcCAwEAAaNCMEAwDgYDVR0PAQH/BAQDAgKkMB0GA1UdJQQW +MBQGCCsGAQUFBwMCBggrBgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3 ` ) diff --git a/path_config.go b/path_config.go index bd2d3bc4..c4155288 100644 --- a/path_config.go +++ b/path_config.go @@ -34,10 +34,10 @@ func pathConfig(b *kubeAuthBackend) *framework.Path { Type: framework.TypeString, Description: "Host must be a host string, a host:port pair, or a URL to the base of the Kubernetes API server.", }, - "kubernetes_ca_cert": { - Type: framework.TypeString, - Description: "PEM encoded CA cert for use by the TLS client used to talk with the API.", + Type: framework.TypeString, + Description: `Optional PEM encoded CA cert for use by the TLS client used to talk with the API. +If it is not set and disable_local_ca_jwt is true, the system's trusted CA certificate pool will be used.`, DisplayAttrs: &framework.DisplayAttributes{ Name: "Kubernetes CA Certificate", }, @@ -159,7 +159,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ return logical.ErrorResponse("no host provided"), nil } - disableLocalJWT := data.Get("disable_local_ca_jwt").(bool) + disableLocalCAJwt := data.Get("disable_local_ca_jwt").(bool) pemList := data.Get("pem_keys").([]string) caCert := data.Get("kubernetes_ca_cert").(string) issuer := data.Get("issuer").(string) @@ -167,8 +167,30 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ tokenReviewer := data.Get("token_reviewer_jwt").(string) useAnnotationsAsAliasMetadata := data.Get("use_annotations_as_alias_metadata").(bool) - if disableLocalJWT && caCert == "" { - return logical.ErrorResponse("kubernetes_ca_cert must be given when disable_local_ca_jwt is true"), nil + // hasCerts returns true if caCert contains at least one valid certificate. It + // does not check if any of the certificates from caCert are CAs, although that + // might be something that we want in the future. + hasCerts := func(certBundle string) bool { + var b *pem.Block + rest := []byte(certBundle) + for { + b, rest = pem.Decode(rest) + if b == nil { + break + } + + if pem.EncodeToMemory(b) != nil { + return true + } + } + + return false + } + + if caCert != "" && !hasCerts(caCert) { + return logical.ErrorResponse( + "The provided CA PEM data contains no valid certificates", + ), nil } config := &kubeConfig{ @@ -179,7 +201,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ TokenReviewerJWT: tokenReviewer, Issuer: issuer, DisableISSValidation: disableIssValidation, - DisableLocalCAJwt: disableLocalJWT, + DisableLocalCAJwt: disableLocalCAJwt, UseAnnotationsAsAliasMetadata: useAnnotationsAsAliasMetadata, } diff --git a/path_config_test.go b/path_config_test.go index 388fe0dc..34807bc5 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -174,7 +174,7 @@ func TestConfig(t *testing.T) { t.Fatalf("got unexpected error: %v", resp.Error()) } - // test invalid cert + // test invalid pem_keys data = map[string]interface{}{ "pem_keys": "bad", "kubernetes_host": "host", @@ -195,6 +195,27 @@ func TestConfig(t *testing.T) { t.Fatalf("got unexpected error: %v", resp.Error()) } + // test invalid kubernetes_ca_cert + data = map[string]interface{}{ + "kubernetes_ca_cert": testInvalidCACert, + "kubernetes_host": "host", + } + + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if resp == nil || !resp.IsError() { + t.Fatal("expected error") + } + if resp.Error().Error() != "The provided CA PEM data contains no valid certificates" { + t.Fatalf("got unexpected error: %v", resp.Error()) + } + // Test success with no certs data = map[string]interface{}{ "kubernetes_host": "host", @@ -458,7 +479,7 @@ func TestConfig_LocalCaJWT(t *testing.T) { DisableLocalCAJwt: false, }, }, - "CA and disable local default": { + "disable local default, CA set": { config: map[string]interface{}{ "kubernetes_host": "host", "kubernetes_ca_cert": testCACert, @@ -474,6 +495,39 @@ func TestConfig_LocalCaJWT(t *testing.T) { DisableLocalCAJwt: true, }, }, + "disable local default, JWT set": { + config: map[string]interface{}{ + "kubernetes_host": "host", + "token_reviewer_jwt": jwtGoodDataToken, + "disable_local_ca_jwt": true, + }, + setupInClusterFiles: true, + expected: &kubeConfig{ + PublicKeys: []crypto.PublicKey{}, + PEMKeys: []string{}, + Host: "host", + CACert: "", + TokenReviewerJWT: jwtGoodDataToken, + DisableISSValidation: true, + DisableLocalCAJwt: true, + }, + }, + "disable local default, no CA or JWT": { + config: map[string]interface{}{ + "kubernetes_host": "host", + "disable_local_ca_jwt": true, + }, + setupInClusterFiles: true, + expected: &kubeConfig{ + PublicKeys: []crypto.PublicKey{}, + PEMKeys: []string{}, + Host: "host", + CACert: "", + TokenReviewerJWT: "", + DisableISSValidation: true, + DisableLocalCAJwt: true, + }, + }, } for name, tc := range testCases {