From ee49a2e4495eb8b026a9d0418d4832dbdba3b79e Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 12 Mar 2024 18:28:28 -0700 Subject: [PATCH 01/18] make kubernetes-ca-cert optional and default to local CA cert --- backend.go | 23 +++++++++++------------ path_config.go | 10 +++------- path_config_test.go | 18 +++++++++++++++++- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/backend.go b/backend.go index bb2a58ad..f5bab5f6 100644 --- a/backend.go +++ b/backend.go @@ -102,7 +102,6 @@ type kubeAuthBackend struct { // localCACertReader contains the local CA certificate. Local CA certificate is // used when running in a pod with following configuration // - kubernetes_ca_cert is not set - // - disable_local_ca_jwt is false localCACertReader *cachingFileReader // tlsConfigUpdaterRunning is used to signal the current state of the tlsConfig updater routine. @@ -334,6 +333,16 @@ func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*k if config == nil { return config, nil } + + // Read local CA cert unless it was stored in config. + // Else build the TLSConfig with the trusted CA cert and load into client + if config.CACert == "" { + config.CACert, err = b.localCACertReader.ReadFile() + if err != nil { + return nil, err + } + } + // Nothing more to do if loading local CA cert and JWT token is disabled. if config.DisableLocalCAJwt { return config, nil @@ -348,16 +357,6 @@ func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*k b.Logger().Debug("failed to read local service account token, will use client token", "error", err) } } - - // Read local CA cert unless it was stored in config. - // Else build the TLSConfig with the trusted CA cert and load into client - if config.CACert == "" { - config.CACert, err = b.localCACertReader.ReadFile() - if err != nil { - return nil, err - } - } - return config, nil } @@ -425,7 +424,7 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { var caCertBytes []byte if config.CACert != "" { caCertBytes = []byte(config.CACert) - } else if !config.DisableLocalCAJwt && b.localCACertReader != nil { + } else if b.localCACertReader != nil { data, err := b.localCACertReader.ReadFile() if err != nil { return err diff --git a/path_config.go b/path_config.go index bd2d3bc4..8fc01d38 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 not set, the local CA cert will be used.`, DisplayAttrs: &framework.DisplayAttributes{ Name: "Kubernetes CA Certificate", }, @@ -167,10 +167,6 @@ 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 - } - config := &kubeConfig{ PublicKeys: make([]crypto.PublicKey, len(pemList)), PEMKeys: pemList, diff --git a/path_config_test.go b/path_config_test.go index 388fe0dc..d4623ecf 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -458,7 +458,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 +474,22 @@ func TestConfig_LocalCaJWT(t *testing.T) { DisableLocalCAJwt: true, }, }, + "disable local default, no CA or JWT, default to local CA": { + config: map[string]interface{}{ + "kubernetes_host": "host", + "disable_local_ca_jwt": true, + }, + setupInClusterFiles: true, + expected: &kubeConfig{ + PublicKeys: []crypto.PublicKey{}, + PEMKeys: []string{}, + Host: "host", + CACert: testLocalCACert, + TokenReviewerJWT: "", + DisableISSValidation: true, + DisableLocalCAJwt: true, + }, + }, } for name, tc := range testCases { From 875494f4bc18bd26718a4cb705e178771d1bbb0d Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 12 Mar 2024 23:19:05 -0700 Subject: [PATCH 02/18] update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a69ef337..d081b879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ * `k8s.io/api` v0.29.1 -> v0.29.2 * `k8s.io/apimachinery` v0.29.1 -> v0.29.2 +### Improvements + +* Make kubernetes_ca_cert optional [GH-238](https://github.com/hashicorp/vault-plugin-auth-kubernetes/pull/238) + ## 0.18.0 (Feb 2, 2024) ### Changes From cac82477abf2fd5deabaef4f18eaf44053493c46 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Thu, 14 Mar 2024 14:47:32 -0700 Subject: [PATCH 03/18] do nothing to TSL config when true disable_local_ca_jwt & unset kubernetes_ca_cert --- backend.go | 25 +++++++++++++------------ path_config.go | 6 +++--- path_config_test.go | 21 +++++++++++++++++++-- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/backend.go b/backend.go index f5bab5f6..f7c7fd25 100644 --- a/backend.go +++ b/backend.go @@ -102,6 +102,7 @@ type kubeAuthBackend struct { // localCACertReader contains the local CA certificate. Local CA certificate is // used when running in a pod with following configuration // - kubernetes_ca_cert is not set + // - disable_local_ca_jwt is false localCACertReader *cachingFileReader // tlsConfigUpdaterRunning is used to signal the current state of the tlsConfig updater routine. @@ -333,16 +334,6 @@ func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*k if config == nil { return config, nil } - - // Read local CA cert unless it was stored in config. - // Else build the TLSConfig with the trusted CA cert and load into client - if config.CACert == "" { - config.CACert, err = b.localCACertReader.ReadFile() - if err != nil { - return nil, err - } - } - // Nothing more to do if loading local CA cert and JWT token is disabled. if config.DisableLocalCAJwt { return config, nil @@ -357,6 +348,16 @@ func (b *kubeAuthBackend) loadConfig(ctx context.Context, s logical.Storage) (*k b.Logger().Debug("failed to read local service account token, will use client token", "error", err) } } + + // Read local CA cert unless it was stored in config. + // Else build the TLSConfig with the trusted CA cert and load into client + if config.CACert == "" { + config.CACert, err = b.localCACertReader.ReadFile() + if err != nil { + return nil, err + } + } + return config, nil } @@ -424,7 +425,7 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { var caCertBytes []byte if config.CACert != "" { caCertBytes = []byte(config.CACert) - } else if b.localCACertReader != nil { + } else if !config.DisableLocalCAJwt && b.localCACertReader != nil { data, err := b.localCACertReader.ReadFile() if err != nil { return err @@ -439,7 +440,7 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { } } else { // provide an empty certPool - b.Logger().Warn("No CA certificates configured, TLS verification will fail") + b.Logger().Warn("No CA certificates configured, TLS verification will use the system's trust store") // TODO: think about supporting host root CA certificates via a configuration toggle, // in which case RootCAs should be set to nil } diff --git a/path_config.go b/path_config.go index 8fc01d38..1f79d3fa 100644 --- a/path_config.go +++ b/path_config.go @@ -37,7 +37,7 @@ func pathConfig(b *kubeAuthBackend) *framework.Path { "kubernetes_ca_cert": { Type: framework.TypeString, Description: `Optional PEM encoded CA cert for use by the TLS client used to talk with the API. -If not set, the local CA cert will be used.`, +If it is not set and disable_local_ca_jwt is true, the system's trust store 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) @@ -175,7 +175,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 d4623ecf..51bb09e0 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -474,9 +474,10 @@ func TestConfig_LocalCaJWT(t *testing.T) { DisableLocalCAJwt: true, }, }, - "disable local default, no CA or JWT, default to local CA": { + "disable local default, JWT set": { config: map[string]interface{}{ "kubernetes_host": "host", + "token_reviewer_jwt": jwtGoodDataToken, "disable_local_ca_jwt": true, }, setupInClusterFiles: true, @@ -484,7 +485,23 @@ func TestConfig_LocalCaJWT(t *testing.T) { PublicKeys: []crypto.PublicKey{}, PEMKeys: []string{}, Host: "host", - CACert: testLocalCACert, + 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, From a3f2e8679cb133e4dd2d6a5dc9ae54cfef7f6c0a Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Mon, 18 Mar 2024 13:27:05 -0700 Subject: [PATCH 04/18] skip updating root CAs --- backend.go | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/backend.go b/backend.go index f7c7fd25..9208454a 100644 --- a/backend.go +++ b/backend.go @@ -421,28 +421,27 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { return err } - // attempt to read the CA certificates from the config directly or from the filesystem. - var caCertBytes []byte - if config.CACert != "" { - caCertBytes = []byte(config.CACert) - } else if !config.DisableLocalCAJwt && b.localCACertReader != nil { - data, err := b.localCACertReader.ReadFile() - if err != nil { - return err + var certPool *x509.CertPool + if config.CACert == "" && config.DisableLocalCAJwt { + b.Logger().Warn("No CA certificates configured and defaulting to using the local CA cert disabled, " + + "TLS verification will use the system's trust store") + } else { + // attempt to read the CA certificates from the config directly or from the filesystem. + var caCertBytes []byte + if config.CACert != "" { + caCertBytes = []byte(config.CACert) + } else if b.localCACertReader != nil { + data, err := b.localCACertReader.ReadFile() + if err != nil { + return err + } + caCertBytes = []byte(data) } - caCertBytes = []byte(data) - } - certPool := x509.NewCertPool() - if len(caCertBytes) > 0 { + 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 use the system's trust store") - // 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. From ef0454e3306b751412b71da8dc62ad5d89b3af02 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Mon, 18 Mar 2024 15:57:21 -0700 Subject: [PATCH 05/18] set transport.TLSClientConfig to getDefaultTLSConfig() when falling back to the system's trust store --- backend.go | 41 +++++++++++++++++++++++------------------ backend_test.go | 17 +++++++++++++++++ 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/backend.go b/backend.go index 9208454a..1f9ac445 100644 --- a/backend.go +++ b/backend.go @@ -421,27 +421,32 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { return err } - var certPool *x509.CertPool - if config.CACert == "" && config.DisableLocalCAJwt { - b.Logger().Warn("No CA certificates configured and defaulting to using the local CA cert disabled, " + - "TLS verification will use the system's trust store") - } else { - // attempt to read the CA certificates from the config directly or from the filesystem. - var caCertBytes []byte - if config.CACert != "" { - caCertBytes = []byte(config.CACert) - } else if b.localCACertReader != nil { - data, err := b.localCACertReader.ReadFile() - if err != nil { - return err - } - caCertBytes = []byte(data) + // attempt to read the CA certificates from the config directly or from the filesystem. + var caCertBytes []byte + if config.CACert != "" { + caCertBytes = []byte(config.CACert) + } else if !config.DisableLocalCAJwt && b.localCACertReader != nil { + data, err := b.localCACertReader.ReadFile() + if err != nil { + return err } + caCertBytes = []byte(data) + } - certPool = x509.NewCertPool() - if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok { - b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail") + if len(caCertBytes) == 0 { + b.Logger().Warn("No CA certificates configured, TLS verification will use the system's trust store") + transport, ok := b.httpClient.Transport.(*http.Transport) + if !ok { + // should never happen + return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport) } + transport.TLSClientConfig = b.tlsConfig + return nil + } + + certPool := x509.NewCertPool() + if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok { + b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail") } // only refresh the Root CAs if they have changed since the last full update. diff --git a/backend_test.go b/backend_test.go index b44fec34..88f84068 100644 --- a/backend_test.go +++ b/backend_test.go @@ -173,6 +173,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: nil, + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From e77f4e6bce68b82b13c82139f4dcff67b8dc296f Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Mon, 18 Mar 2024 15:59:01 -0700 Subject: [PATCH 06/18] forgot to stage transport.TLSClientConfig = getDefaultTLSConfig() change --- backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend.go b/backend.go index 1f9ac445..ff3326dd 100644 --- a/backend.go +++ b/backend.go @@ -440,7 +440,7 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { // should never happen return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport) } - transport.TLSClientConfig = b.tlsConfig + transport.TLSClientConfig = getDefaultTLSConfig() return nil } From 27b393394c2b95cededc3d30744da03644ea7d07 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 19 Mar 2024 10:07:15 -0700 Subject: [PATCH 07/18] only update transport.TLSClientConfig when transport.TLSClientConfig.RootCAs has become nil or at the initial update --- backend.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/backend.go b/backend.go index ff3326dd..655b274b 100644 --- a/backend.go +++ b/backend.go @@ -434,13 +434,17 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { } if len(caCertBytes) == 0 { - b.Logger().Warn("No CA certificates configured, TLS verification will use the system's trust store") + // no CA certificates configured, TLS verification will use the system's trust store transport, ok := b.httpClient.Transport.(*http.Transport) if !ok { // should never happen return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport) } - transport.TLSClientConfig = getDefaultTLSConfig() + if transport.TLSClientConfig == nil || transport.TLSClientConfig.RootCAs != nil { + b.Logger().Trace("Root CA certificate pool has become nil, updating the client's transport") + b.tlsConfig.RootCAs = nil + transport.TLSClientConfig = getDefaultTLSConfig() + } return nil } From 0af6e9431c2a2d36fd12ac4af7cb55b272043b92 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 19 Mar 2024 19:39:59 +0000 Subject: [PATCH 08/18] Use the SystemCertPool() when no CACert chain is provided - check that the CA cert chain contains at least one valid certificate. --- backend.go | 41 ++++++++++++++++++++++++----------------- backend_test.go | 7 ++++++- path_config.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/backend.go b/backend.go index 655b274b..eb5d5229 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,28 +442,26 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { caCertBytes = []byte(data) } + var certPool *x509.CertPool if len(caCertBytes) == 0 { - // no CA certificates configured, TLS verification will use the system's trust store - transport, ok := b.httpClient.Transport.(*http.Transport) - if !ok { - // should never happen - return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport) - } - if transport.TLSClientConfig == nil || transport.TLSClientConfig.RootCAs != nil { - b.Logger().Trace("Root CA certificate pool has become nil, updating the client's transport") - b.tlsConfig.RootCAs = nil - transport.TLSClientConfig = getDefaultTLSConfig() + // since the CA chain is not configured, we use the system's cert pool. + sysRootPool, err := x509.SystemCertPool() + if err != nil { + return err } - return nil - } - certPool := x509.NewCertPool() - if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok { - b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail") + certPool = sysRootPool + } 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") + } } // only refresh the Root CAs if they have changed since the last full update. - if !b.tlsConfig.RootCAs.Equal(certPool) { + if b.tlsConfig.RootCAs == nil || !b.tlsConfig.RootCAs.Equal(certPool) { b.Logger().Trace("Root CA certificate pool has changed, updating the client's transport") transport, ok := b.httpClient.Transport.(*http.Transport) if !ok { diff --git a/backend_test.go b/backend_test.go index 88f84068..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 @@ -184,7 +189,7 @@ func Test_kubeAuthBackend_updateTLSConfig(t *testing.T) { }, expectTLSConfig: &tls.Config{ MinVersion: minTLSVersion, - RootCAs: nil, + RootCAs: systemCertPool, }, }, }, diff --git a/path_config.go b/path_config.go index 1f79d3fa..9732aea6 100644 --- a/path_config.go +++ b/path_config.go @@ -167,6 +167,34 @@ 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) + // validCACert 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. + validCACert := func() bool { + var b *pem.Block + rest := []byte(caCert) + for { + b, rest = pem.Decode(rest) + if b == nil { + break + } + + if pem.EncodeToMemory(b) != nil { + return true + } + } + + return false + } + + if caCert != "" { + if !validCACert() { + return logical.ErrorResponse( + "Configured CA PEM data contains no valid certificates, TLS verification will fail", + ), nil + } + } + config := &kubeConfig{ PublicKeys: make([]crypto.PublicKey, len(pemList)), PEMKeys: pemList, From f03381964bcf91ff9efde048d7b33e9ba7a2ed2e Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 19 Mar 2024 16:17:41 -0400 Subject: [PATCH 09/18] Minor fix ups --- backend.go | 4 ++-- path_config.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backend.go b/backend.go index eb5d5229..4a30a201 100644 --- a/backend.go +++ b/backend.go @@ -445,12 +445,12 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { var certPool *x509.CertPool if len(caCertBytes) == 0 { // since the CA chain is not configured, we use the system's cert pool. - sysRootPool, err := x509.SystemCertPool() + sysCertPool, err := x509.SystemCertPool() if err != nil { return err } - certPool = sysRootPool + certPool = sysCertPool } else { // since we have a CA chain configured, we create a new x509.CertPool with its // contents. diff --git a/path_config.go b/path_config.go index 9732aea6..033255d1 100644 --- a/path_config.go +++ b/path_config.go @@ -167,12 +167,12 @@ 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) - // validCACert returns true if caCert contains at least one valid certificate. It + // 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. - validCACert := func() bool { + hasCerts := func(certBundle string) bool { var b *pem.Block - rest := []byte(caCert) + rest := []byte(certBundle) for { b, rest = pem.Decode(rest) if b == nil { @@ -188,7 +188,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ } if caCert != "" { - if !validCACert() { + if !hasCerts(caCert) { return logical.ErrorResponse( "Configured CA PEM data contains no valid certificates, TLS verification will fail", ), nil From 2a8f1d31affdc1cec8cd3456f285249cbf9be1a5 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 19 Mar 2024 17:24:42 -0400 Subject: [PATCH 10/18] Guard against nil RootCAs --- backend.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/backend.go b/backend.go index 4a30a201..ea0d8b64 100644 --- a/backend.go +++ b/backend.go @@ -445,12 +445,11 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { var certPool *x509.CertPool if len(caCertBytes) == 0 { // since the CA chain is not configured, we use the system's cert pool. - sysCertPool, err := x509.SystemCertPool() + var err error + certPool, err = x509.SystemCertPool() if err != nil { return err } - - certPool = sysCertPool } else { // since we have a CA chain configured, we create a new x509.CertPool with its // contents. @@ -460,9 +459,7 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error { } } - // only refresh the Root CAs if they have changed since the last full update. - if b.tlsConfig.RootCAs == nil || !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 @@ -471,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") } From e6e5bd82173383c2d0c9935b151a12108b34a95d Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 19 Mar 2024 19:04:11 -0700 Subject: [PATCH 11/18] add test for invalid CA cert --- path_config_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/path_config_test.go b/path_config_test.go index 51bb09e0..4cbce708 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 CA cert + data = map[string]interface{}{ + "kubernetes_ca_cert": "bad", + "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() != "Configured CA PEM data contains no valid certificates, TLS verification will fail" { + t.Fatalf("got unexpected error: %v", resp.Error()) + } + // Test success with no certs data = map[string]interface{}{ "kubernetes_host": "host", From 82ec9fba3cad6c81677748f0f427be68b5b228de Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 09:34:37 -0700 Subject: [PATCH 12/18] Update path_config_test.go Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com> --- path_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_config_test.go b/path_config_test.go index 4cbce708..b3572439 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 PEM keys + // test invalid pem_keys data = map[string]interface{}{ "pem_keys": "bad", "kubernetes_host": "host", From 698a21b30de465506a8f115508a76fc9e9dfb8d3 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 09:35:05 -0700 Subject: [PATCH 13/18] Update path_config.go Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com> --- path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_config.go b/path_config.go index 033255d1..0c073d4a 100644 --- a/path_config.go +++ b/path_config.go @@ -190,7 +190,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ if caCert != "" { if !hasCerts(caCert) { return logical.ErrorResponse( - "Configured CA PEM data contains no valid certificates, TLS verification will fail", + "The provided CA PEM data contains no valid certificates." ), nil } } From 6b84d973d600304075b5cba867ccefc226f14ea6 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 09:35:32 -0700 Subject: [PATCH 14/18] Update path_config.go Co-authored-by: Ben Ash <32777270+benashz@users.noreply.github.com> --- path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_config.go b/path_config.go index 0c073d4a..a862f6d4 100644 --- a/path_config.go +++ b/path_config.go @@ -37,7 +37,7 @@ func pathConfig(b *kubeAuthBackend) *framework.Path { "kubernetes_ca_cert": { 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 trust store will be used.`, +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", }, From 3a5f5d7a35266bcad3908d4279670702b40f23f1 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 10:15:51 -0700 Subject: [PATCH 15/18] update CHANGELOG entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d081b879..461250de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ ### Improvements -* Make kubernetes_ca_cert optional [GH-238](https://github.com/hashicorp/vault-plugin-auth-kubernetes/pull/238) +* 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) From 5025f0347fc703697ea6479c3c52d04c5f55b7df Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 10:21:51 -0700 Subject: [PATCH 16/18] use truncated CA cert for invalid kubernetes_ca_cert test --- common_test.go | 13 +++++++++++++ path_config.go | 8 ++------ path_config_test.go | 6 +++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/common_test.go b/common_test.go index 90cb638c..9bd0d16f 100644 --- a/common_test.go +++ b/common_test.go @@ -104,5 +104,18 @@ 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 a862f6d4..7c67cfac 100644 --- a/path_config.go +++ b/path_config.go @@ -187,12 +187,8 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ return false } - if caCert != "" { - if !hasCerts(caCert) { - return logical.ErrorResponse( - "The provided CA PEM data contains no valid certificates." - ), nil - } + if caCert != "" && !hasCerts(caCert) { + return logical.ErrorResponse("The provided CA PEM data contains no valid certificates"), nil } config := &kubeConfig{ diff --git a/path_config_test.go b/path_config_test.go index b3572439..34807bc5 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -195,9 +195,9 @@ func TestConfig(t *testing.T) { t.Fatalf("got unexpected error: %v", resp.Error()) } - // test invalid CA cert + // test invalid kubernetes_ca_cert data = map[string]interface{}{ - "kubernetes_ca_cert": "bad", + "kubernetes_ca_cert": testInvalidCACert, "kubernetes_host": "host", } @@ -212,7 +212,7 @@ func TestConfig(t *testing.T) { if resp == nil || !resp.IsError() { t.Fatal("expected error") } - if resp.Error().Error() != "Configured CA PEM data contains no valid certificates, TLS verification will fail" { + if resp.Error().Error() != "The provided CA PEM data contains no valid certificates" { t.Fatalf("got unexpected error: %v", resp.Error()) } From f1c5acf3c2ef434bedaf45013b3cc4bb30d32ecd Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Tue, 26 Mar 2024 10:27:24 -0700 Subject: [PATCH 17/18] add new line --- common_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/common_test.go b/common_test.go index 9bd0d16f..fd589b12 100644 --- a/common_test.go +++ b/common_test.go @@ -105,6 +105,7 @@ D14XDzgsruCwlWAP1FMvLMIPhSknpIJd9Xql+0/Ae1yl9f3Uamj3mDtBKg8/U5nJ 0wU= -----END CERTIFICATE----- ` + testInvalidCACert = ` -----BEGIN CERTIFICATE----- MIIC5zCCAc+gAwIBAgIBATANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwptaW5p From f916f7e761547f1b6e91470e6c98844099f2da57 Mon Sep 17 00:00:00 2001 From: Thy Ton Date: Wed, 27 Mar 2024 15:56:06 -0700 Subject: [PATCH 18/18] fold long lines --- CHANGELOG.md | 4 +++- path_config.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 461250de..26555ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,9 @@ ### 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) +* 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) diff --git a/path_config.go b/path_config.go index 7c67cfac..c4155288 100644 --- a/path_config.go +++ b/path_config.go @@ -188,7 +188,9 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ } if caCert != "" && !hasCerts(caCert) { - return logical.ErrorResponse("The provided CA PEM data contains no valid certificates"), nil + return logical.ErrorResponse( + "The provided CA PEM data contains no valid certificates", + ), nil } config := &kubeConfig{