make kubernetes_ca_cert optional

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

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")
 	}

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) {

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 trust store will be used.`,
 				DisplayAttrs: &framework.DisplayAttributes{
 					Name: "Kubernetes CA Certificate",
 				},
@@ -159,16 +159,40 @@ 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)
 	disableIssValidation := data.Get("disable_iss_validation").(bool)
 	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 != "" {
+		if !hasCerts(caCert) {
+			return logical.ErrorResponse(
+				"Configured CA PEM data contains no valid certificates, TLS verification will fail",
+			), nil
+		}
 	}
 
 	config := &kubeConfig{
@@ -179,7 +203,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ
 		TokenReviewerJWT:              tokenReviewer,
 		Issuer:                        issuer,
 		DisableISSValidation:          disableIssValidation,
-		DisableLocalCAJwt:             disableLocalJWT,
+		DisableLocalCAJwt:             disableLocalCAJwt,
 		UseAnnotationsAsAliasMetadata: useAnnotationsAsAliasMetadata,
 	}
 

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",
@@ -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 {