backport of commit bc42d56c7a (#23204)

Co-authored-by: Steven Clark <steven.clark@hashicorp.com>

command/server.go

@@ -2784,6 +2784,10 @@ func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal
 	generation := uint64(1)
 
 	if existingSealGenInfo != nil {
+		// This forces a seal re-wrap on all seal related config changes, as we can't
+		// be sure what effect the config change might do. This is purposefully different
+		// from within the Validate call below that just matches on seal configs based
+		// on name/type.
 		if cmp.Equal(existingSealGenInfo.Seals, sealConfigs) {
 			return existingSealGenInfo, nil
 		}

command/server_sealgenerationinfo_test.go

@@ -599,7 +599,7 @@ func TestMultiSealCases(t *testing.T) {
 			switch {
 			case tc.isErrorExpected:
 				require.Error(t, err)
-				require.EqualError(t, err, tc.expectedErrorMsg)
+				require.ErrorContains(t, err, tc.expectedErrorMsg)
 				require.Nil(t, sealGenInfo)
 			default:
 				require.NoError(t, err)
@@ -726,6 +726,39 @@ func TestMultiSealCases(t *testing.T) {
 			isErrorExpected:          true,
 			expectedErrorMsg:         "cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes",
 		},
+		// single seal migration use-case
+		{
+			name: "single_seal_migration",
+			existingSealGenInfo: &seal.SealGenerationInfo{
+				Generation: 2,
+				Seals: []*configutil.KMS{
+					{
+						Type:     "transit",
+						Name:     "transit",
+						Priority: 1,
+					},
+				},
+			},
+			newSealGenInfo: &seal.SealGenerationInfo{
+				Generation: 1,
+				Seals: []*configutil.KMS{
+					{
+						Type:     "transit",
+						Name:     "transit-disabled",
+						Priority: 1,
+						Disabled: true,
+					},
+					{
+						Type:     "shamir",
+						Name:     "shamir",
+						Priority: 1,
+					},
+				},
+			},
+			isRewrapped:              true,
+			hasPartiallyWrappedPaths: false,
+			isErrorExpected:          false,
+		},
 		// have partially wrapped paths
 		{
 			name: "have_partially_wrapped_paths",
@@ -801,7 +834,7 @@ func TestMultiSealCases(t *testing.T) {
 			switch {
 			case tc.isErrorExpected:
 				require.Error(t, err)
-				require.EqualError(t, err, tc.expectedErrorMsg)
+				require.ErrorContains(t, err, tc.expectedErrorMsg)
 			default:
 				require.NoError(t, err)
 			}

internalshared/configutil/kms.go

@@ -44,6 +44,8 @@ type EntropyMode int
 const (
 	EntropyUnknown EntropyMode = iota
 	EntropyAugmentation
+
+	KmsRenameDisabledSuffix = "-disabled"
 )
 
 type Entropy struct {
@@ -133,7 +135,7 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int
 		name := strings.ToLower(key)
 		// ensure that seals of the same type will have unique names for seal migration
 		if disabled {
-			name += "-disabled"
+			name += KmsRenameDisabledSuffix
 		}
 		if v, ok := m["name"]; ok {
 			name, ok = v.(string)

vault/seal/seal.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
+	"strings"
 	"sync/atomic"
 	"time"
 
@@ -62,10 +63,16 @@ type SealGenerationInfo struct {
 func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPartiallyWrappedPaths bool) error {
 	existingSealsLen := 0
 	previousShamirConfigured := false
+	existingSealNameAndType := "[]"
+	configuredSealNameAndType := sealNameAndTypeAsStr(sgi.Seals)
+
 	if existingSgi != nil {
+		existingSealNameAndType = sealNameAndTypeAsStr(existingSgi.Seals)
 		if sgi.Generation == existingSgi.Generation {
-			if !cmp.Equal(sgi.Seals, existingSgi.Seals) {
-				return errors.New("existing seal generation is the same, but the configured seals are different")
+			if !haveMatchingSeals(sgi.Seals, existingSgi.Seals) {
+				return fmt.Errorf("existing seal generation is the same, but the configured seals are different\n"+
+					"Existing seals: %v\n"+
+					"Configured seals: %v", existingSealNameAndType, configuredSealNameAndType)
 			}
 			return nil
 		}
@@ -98,37 +105,106 @@ func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPart
 	numSealsToDelete := existingSealsLen - len(sgi.Seals)
 	switch {
 	case numSealsToAdd > 1:
-		return errors.New("cannot add more than one seal")
+		return fmt.Errorf("cannot add more than one seal\n"+
+			"Existing seals: %v\n"+
+			"Configured seals: %v", existingSealNameAndType, configuredSealNameAndType)
 
 	case numSealsToDelete > 1:
-		return errors.New("cannot delete more than one seal")
+		return fmt.Errorf("cannot delete more than one seal\n"+
+			"Existing seals: %v\n"+
+			"Configured seals: %v", existingSealNameAndType, configuredSealNameAndType)
 
 	case !previousShamirConfigured && existingSgi != nil && !haveCommonSeal(existingSgi.Seals, sgi.Seals):
 		// With a previously configured shamir seal, we are either going from [shamir]->[auto] or [shamir]->[another shamir],
 		// in which case we cannot have a common seal because shamir seals cannot be set to disabled, they can only be deleted.
-		return errors.New("must have at least one seal in common with the old generation")
+		return fmt.Errorf("must have at least one seal in common with the old generation\n"+
+			"Existing seals: %v\n"+
+			"Configured seals: %v", existingSealNameAndType, configuredSealNameAndType)
 	}
 	return nil
 }
 
-func haveCommonSeal(existingSealKmsConfigs, newSealKmsConfigs []*configutil.KMS) (result bool) {
+func sealNameAndTypeAsStr(seals []*configutil.KMS) string {
+	info := []string{}
+	for _, seal := range seals {
+		info = append(info, fmt.Sprintf("Name: %s Type: %s", seal.Name, seal.Type))
+	}
+	return fmt.Sprintf("[%s]", strings.Join(info, ", "))
+}
+
+// haveMatchingSeals verifies that we have the corresponding matching seals by name and type, config and other
+// properties are ignored in the comparison
+func haveMatchingSeals(existingSealKmsConfigs, newSealKmsConfigs []*configutil.KMS) bool {
+	if len(existingSealKmsConfigs) != len(newSealKmsConfigs) {
+		return false
+	}
+
+	for _, existingSealKmsConfig := range existingSealKmsConfigs {
+		found := false
+		for _, newSealKmsConfig := range newSealKmsConfigs {
+			if cmp.Equal(existingSealKmsConfig, newSealKmsConfig, compareKMSConfigByNameAndType()) {
+				found = true
+				break
+			}
+		}
+
+		if !found {
+			return false
+		}
+	}
+	return true
+}
+
+// haveCommonSeal verifies that we have at least one matching seal across
+// the inputs by name and type, config and other properties are ignored in
+// the comparison
+func haveCommonSeal(existingSealKmsConfigs, newSealKmsConfigs []*configutil.KMS) bool {
 	for _, existingSealKmsConfig := range existingSealKmsConfigs {
 		for _, newSealKmsConfig := range newSealKmsConfigs {
-			// Clone the existing seal config and set 'Disabled' and 'Priority' fields same as the
-			// new seal config, because there might be a case where a seal might be disabled in
-			// current config, but might be stored as enabled previously, and this still needs to
-			// be considered as a common seal.
-			clonedSgi := existingSealKmsConfig.Clone()
-			clonedSgi.Disabled = newSealKmsConfig.Disabled
-			clonedSgi.Priority = newSealKmsConfig.Priority
-			if cmp.Equal(clonedSgi, newSealKmsConfig.Clone()) {
+			// Technically we might be matching the "wrong" seal if the old seal was renamed to
+			// "transit-disabled" and we have a new seal named transit. There isn't any way for
+			// us to properly distinguish between them
+			if cmp.Equal(existingSealKmsConfig, newSealKmsConfig, compareKMSConfigByNameAndType()) {
 				return true
 			}
 		}
 	}
+
+	// We might have renamed a disabled seal that was previously used so attempt to match by
+	// removing the "-disabled" suffix
+	for _, seal := range findRenamedDisabledSeals(newSealKmsConfigs) {
+		clonedSeal := seal.Clone()
+		clonedSeal.Name = strings.TrimSuffix(clonedSeal.Name, configutil.KmsRenameDisabledSuffix)
+
+		for _, existingSealKmsConfig := range existingSealKmsConfigs {
+			if cmp.Equal(existingSealKmsConfig, clonedSeal, compareKMSConfigByNameAndType()) {
+				return true
+			}
+		}
+	}
+
 	return false
 }
 
+func findRenamedDisabledSeals(configs []*configutil.KMS) []*configutil.KMS {
+	diabledSeals := []*configutil.KMS{}
+	for _, seal := range configs {
+		if seal.Disabled && strings.HasSuffix(seal.Name, configutil.KmsRenameDisabledSuffix) {
+			diabledSeals = append(diabledSeals, seal)
+		}
+	}
+	return diabledSeals
+}
+
+func compareKMSConfigByNameAndType() cmp.Option {
+	// We only match based on name and type to avoid configuration changes such
+	// as a Vault token change in the config map from eliminating the match and
+	// preventing startup on a matching seal.
+	return cmp.Comparer(func(a, b *configutil.KMS) bool {
+		return a.Name == b.Name && a.Type == b.Type
+	})
+}
+
 // SetRewrapped updates the SealGenerationInfo's rewrapped status to the provided value.
 func (sgi *SealGenerationInfo) SetRewrapped(value bool) {
 	sgi.rewrapped.Store(value)