connect/ca: require new vault mount points when updating the key type/bits for the vault connect CA provider (#10331)

progress on #9572

.changelog/10331.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect/ca: require new vault mount points when updating the key type/bits for the vault connect CA provider
+```

GNUmakefile

@@ -360,14 +360,14 @@ ifeq ("$(CIRCLECI)","true")
 # Run in CI
 	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report.xml" -- -cover -coverprofile=coverage.txt ./agent/connect/ca
 # Run leader tests that require Vault
-	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report-leader.xml" -- -cover -coverprofile=coverage-leader.txt -run TestLeader_Vault_ ./agent/consul
+	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report-leader.xml" -- -cover -coverprofile=coverage-leader.txt -run '.*_Vault_' ./agent/consul
 # Run agent tests that require Vault
 	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report-agent.xml" -- -cover -coverprofile=coverage-agent.txt -run '.*_Vault_' ./agent
 else
 # Run locally
 	@echo "Running /agent/connect/ca tests in verbose mode"
 	@go test -v ./agent/connect/ca
-	@go test -v ./agent/consul -run 'TestLeader_Vault_'
+	@go test -v ./agent/consul -run '.*_Vault_'
 	@go test -v ./agent -run '.*_Vault_'
 endif
 

agent/connect/ca/provider_vault.go

@@ -163,7 +163,7 @@ func (v *VaultProvider) GenerateRoot() error {
 	}
 
 	// Set up the root PKI backend if necessary.
-	_, err := v.ActiveRoot()
+	rootPEM, err := v.ActiveRoot()
 	switch err {
 	case ErrBackendNotMounted:
 		err := v.client.Sys().Mount(v.config.RootPKIPath, &vaultapi.MountInput{
@@ -197,6 +197,31 @@ func (v *VaultProvider) GenerateRoot() error {
 		if err != nil {
 			return err
 		}
+
+		if rootPEM != "" {
+			rootCert, err := connect.ParseCert(rootPEM)
+			if err != nil {
+				return err
+			}
+
+			// Vault PKI doesn't allow in-place cert/key regeneration. That
+			// means if you need to change either the key type or key bits then
+			// you also need to provide new mount points.
+			// https://www.vaultproject.io/api-docs/secret/pki#generate-root
+			//
+			// A separate bug in vault likely also requires that you use the
+			// ForceWithoutCrossSigning option when changing key types.
+			foundKeyType, foundKeyBits, err := connect.KeyInfoFromCert(rootCert)
+			if err != nil {
+				return err
+			}
+			if v.config.PrivateKeyType != foundKeyType {
+				return fmt.Errorf("cannot update the PrivateKeyType field without choosing a new PKI mount for the root CA")
+			}
+			if v.config.PrivateKeyBits != foundKeyBits {
+				return fmt.Errorf("cannot update the PrivateKeyBits field without choosing a new PKI mount for the root CA")
+			}
+		}
 	}
 
 	return nil

agent/consul/connect_ca_endpoint_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/hashicorp/consul/agent/connect"
 	ca "github.com/hashicorp/consul/agent/connect/ca"
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
 )
@@ -511,6 +512,111 @@ func TestConnectCAConfig_TriggerRotation(t *testing.T) {
 	}
 }
 
+func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	ca.SkipIfVaultNotPresent(t)
+
+	t.Parallel()
+
+	testVault := ca.NewTestVaultServer(t)
+	defer testVault.Stop()
+
+	_, s1 := testServerWithConfig(t, func(c *Config) {
+		c.Build = "1.6.0"
+		c.PrimaryDatacenter = "dc1"
+		c.CAConfig = &structs.CAConfiguration{
+			Provider: "vault",
+			Config: map[string]interface{}{
+				"Address":             testVault.Addr,
+				"Token":               testVault.RootToken,
+				"RootPKIPath":         "pki-root/",
+				"IntermediatePKIPath": "pki-intermediate/",
+			},
+		}
+	})
+	defer s1.Shutdown()
+
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
+
+	// Capture the current root.
+	{
+		rootList, _, err := getTestRoots(s1, "dc1")
+		require.NoError(t, err)
+		require.Len(t, rootList.Roots, 1)
+	}
+
+	cases := []struct {
+		name      string
+		configFn  func() (*structs.CAConfiguration, error)
+		expectErr string
+	}{
+		{
+			name: "cannot edit key bits",
+			configFn: func() (*structs.CAConfiguration, error) {
+				return &structs.CAConfiguration{
+					Provider: "vault",
+					Config: map[string]interface{}{
+						"Address":             testVault.Addr,
+						"Token":               testVault.RootToken,
+						"RootPKIPath":         "pki-root/",
+						"IntermediatePKIPath": "pki-intermediate/",
+						//
+						"PrivateKeyType": "ec",
+						"PrivateKeyBits": 384,
+					},
+					ForceWithoutCrossSigning: true,
+				}, nil
+			},
+			expectErr: `error generating CA root certificate: cannot update the PrivateKeyBits field without choosing a new PKI mount for the root CA`,
+		},
+		{
+			name: "cannot edit key type",
+			configFn: func() (*structs.CAConfiguration, error) {
+				return &structs.CAConfiguration{
+					Provider: "vault",
+					Config: map[string]interface{}{
+						"Address":             testVault.Addr,
+						"Token":               testVault.RootToken,
+						"RootPKIPath":         "pki-root/",
+						"IntermediatePKIPath": "pki-intermediate/",
+						//
+						"PrivateKeyType": "rsa",
+						"PrivateKeyBits": 4096,
+					},
+					ForceWithoutCrossSigning: true,
+				}, nil
+			},
+			expectErr: `error generating CA root certificate: cannot update the PrivateKeyType field without choosing a new PKI mount for the root CA`,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			newConfig, err := tc.configFn()
+			require.NoError(t, err)
+
+			args := &structs.CARequest{
+				Datacenter: "dc1",
+				Config:     newConfig,
+			}
+			var reply interface{}
+
+			err = msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)
+			if tc.expectErr == "" {
+				require.NoError(t, err)
+			} else {
+				testutil.RequireErrorContains(t, err, tc.expectErr)
+			}
+		})
+	}
+}
+
 func TestConnectCAConfig_UpdateSecondary(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")