connect/ca: ensure edits to the key type/bits for the connect builtin…

… CA will regenerate the roots progress on #9572
hashicorp · Jul 13, 2021 · 9950662 · 9950662
1 parent dc15e5e
commit 9950662
Show file tree

Hide file tree

Showing 5 changed files with 401 additions and 183 deletions.
diff --git a/.changelog/10330.txt b/.changelog/10330.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+connect/ca: ensure edits to the key type/bits for the connect builtin CA will regenerate the roots
+```
diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go
@@ -60,6 +60,11 @@ type ConsulProviderStateDelegate interface {
 	ApplyCARequest(*structs.CARequest) (interface{}, error)
 }
 
+func hexStringHash(input string) string {
+	hash := sha256.Sum256([]byte(input))
+	return connect.HexString(hash[:])
+}
+
 // Configure sets up the provider using the given configuration.
 func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
 	// Parse the raw config and update our ID.
@@ -68,8 +73,7 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
 		return err
 	}
 	c.config = config
-	hash := sha256.Sum256([]byte(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, cfg.IsPrimary)))
-	c.id = connect.HexString(hash[:])
+	c.id = hexStringHash(fmt.Sprintf("%s,%s,%s,%d,%v", config.PrivateKey, config.RootCert, config.PrivateKeyType, config.PrivateKeyBits, cfg.IsPrimary))
 	c.clusterID = cfg.ClusterID
 	c.isPrimary = cfg.IsPrimary
 	c.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: c.clusterID})
@@ -87,35 +91,41 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error {
 		return nil
 	}
 
-	// Check if there's an entry with the old ID scheme.
-	oldID := fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert)
-	_, providerState, err = c.Delegate.State().CAProviderState(oldID)
-	if err != nil {
-		return err
+	oldIDs := []string{
+		hexStringHash(fmt.Sprintf("%s,%s,%v", config.PrivateKey, config.RootCert, cfg.IsPrimary)),
+		fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert),
 	}
 
-	// Found an entry with the old ID, so update it to the new ID and
-	// delete the old entry.
-	if providerState != nil {
-		newState := *providerState
-		newState.ID = c.id
-		createReq := &structs.CARequest{
-			Op:            structs.CAOpSetProviderState,
-			ProviderState: &newState,
-		}
-		if _, err := c.Delegate.ApplyCARequest(createReq); err != nil {
+	// Check if there any entries with old ID schemes.
+	for _, oldID := range oldIDs {
+		_, providerState, err = c.Delegate.State().CAProviderState(oldID)
+		if err != nil {
 			return err
 		}
 
-		deleteReq := &structs.CARequest{
-			Op:            structs.CAOpDeleteProviderState,
-			ProviderState: providerState,
-		}
-		if _, err := c.Delegate.ApplyCARequest(deleteReq); err != nil {
-			return err
-		}
+		// Found an entry with the old ID, so update it to the new ID and
+		// delete the old entry.
+		if providerState != nil {
+			newState := *providerState
+			newState.ID = c.id
+			createReq := &structs.CARequest{
+				Op:            structs.CAOpSetProviderState,
+				ProviderState: &newState,
+			}
+			if _, err := c.Delegate.ApplyCARequest(createReq); err != nil {
+				return err
+			}
 
-		return nil
+			deleteReq := &structs.CARequest{
+				Op:            structs.CAOpDeleteProviderState,
+				ProviderState: providerState,
+			}
+			if _, err := c.Delegate.ApplyCARequest(deleteReq); err != nil {
+				return err
+			}
+
+			return nil
+		}
 	}
 
 	args := &structs.CARequest{

diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go
@@ -438,30 +438,45 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
 }
 
 func TestConsulCAProvider_MigrateOldID(t *testing.T) {
-	t.Parallel()
+	cases := []struct {
+		name  string
+		oldID string
+	}{
+		{
+			name:  "original-unhashed",
+			oldID: ",",
+		},
+		{
+			name:  "hash-v1",
+			oldID: hexStringHash(",,true"),
+		},
+	}
 
-	require := require.New(t)
-	conf := testConsulCAConfig()
-	delegate := newMockDelegate(t, conf)
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			conf := testConsulCAConfig()
+			delegate := newMockDelegate(t, conf)
 
-	// Create an entry with an old-style ID.
-	_, err := delegate.ApplyCARequest(&structs.CARequest{
-		Op: structs.CAOpSetProviderState,
-		ProviderState: &structs.CAConsulProviderState{
-			ID: ",",
-		},
-	})
-	require.NoError(err)
-	_, providerState, err := delegate.state.CAProviderState(",")
-	require.NoError(err)
-	require.NotNil(providerState)
+			// Create an entry with an old-style ID.
+			_, err := delegate.ApplyCARequest(&structs.CARequest{
+				Op: structs.CAOpSetProviderState,
+				ProviderState: &structs.CAConsulProviderState{
+					ID: tc.oldID,
+				},
+			})
+			require.NoError(t, err)
+			_, providerState, err := delegate.state.CAProviderState(tc.oldID)
+			require.NoError(t, err)
+			require.NotNil(t, providerState)
 
-	provider := TestConsulProvider(t, delegate)
-	require.NoError(provider.Configure(testProviderConfig(conf)))
-	require.NoError(provider.GenerateRoot())
+			provider := TestConsulProvider(t, delegate)
+			require.NoError(t, provider.Configure(testProviderConfig(conf)))
+			require.NoError(t, provider.GenerateRoot())
 
-	// After running Configure, the old ID entry should be gone.
-	_, providerState, err = delegate.state.CAProviderState(",")
-	require.NoError(err)
-	require.Nil(providerState)
+			// After running Configure, the old ID entry should be gone.
+			_, providerState, err = delegate.state.CAProviderState(tc.oldID)
+			require.NoError(t, err)
+			require.Nil(t, providerState)
+		})
+	}
 }