Provide a better error message around initializing with multiple seals (#23210)

* Provide a better error message around initializing with multiple seals - Specifically callout during cluster initialization or initial beta seal migration that we can only have a single seal enabled with the following error message: `Initializing a cluster or enabling multi-seal on an existing cluster must occur with a single seal before adding additional seals` - Handle the use case that we have multiple seals configured, but some are disabled, leaving a single enabled seal. This is the legacy seal migratation case that works without the BETA flag set, so should work with it set as well. * Update the expected error messages within seal tests * Remove support for old style migration configurations in multi-seal
2026-05-05 12:26:34 +02:00 · 2023-09-21 12:32:44 -04:00 · 2023-09-21 12:32:44 -04:00 · 4389ee438d
commit 4389ee438d
parent 6ef2a60314
2 changed files with 104 additions and 25 deletions
--- a/command/server_sealgenerationinfo_test.go
+++ b/command/server_sealgenerationinfo_test.go
@ -96,7 +96,7 @@ func TestMultiSealCases(t *testing.T) {
 				},
 			},
 			isErrorExpected:   true,
-			expectedErrorMsg:  "cannot add more than one seal",
+			expectedErrorMsg:  "Initializing a cluster or enabling multi-seal on an existing cluster must occur with a single seal before adding additional seals",
 			sealHaBetaEnabled: true,
 		},
 		// none_to_multi_with_disabled_seals_with_beta
@ -117,7 +117,7 @@ func TestMultiSealCases(t *testing.T) {
 				},
 			},
 			isErrorExpected:   true,
-			expectedErrorMsg:  "cannot add more than one seal",
+			expectedErrorMsg:  "Initializing a cluster or enabling multi-seal on an existing cluster must occur with a single seal before adding additional seals",
 			sealHaBetaEnabled: true,
 		},
 		// none_to_multi_with_disabled_seals_no_beta
@ -759,6 +759,72 @@ func TestMultiSealCases(t *testing.T) {
 			hasPartiallyWrappedPaths: false,
 			isErrorExpected:          false,
 		},
+		// migrate from non-beta single seal to single seal
+		{
+			name:                "none_to_single_seal",
+			existingSealGenInfo: nil,
+			newSealGenInfo: &seal.SealGenerationInfo{
+				Generation: 1,
+				Seals: []*configutil.KMS{
+					{
+						Type:     "shamir",
+						Name:     "shamir",
+						Priority: 1,
+					},
+				},
+			},
+			isRewrapped:              true,
+			hasPartiallyWrappedPaths: false,
+			isErrorExpected:          false,
+		},
+		// migrate from non-beta single seal to multi seal, with one disabled, so perform an old style migration
+		// we do not support this use-case at this time so trap the error
+		{
+			name:                "none_to_multiple_seals_one_disabled",
+			existingSealGenInfo: nil,
+			newSealGenInfo: &seal.SealGenerationInfo{
+				Generation: 1,
+				Seals: []*configutil.KMS{
+					{
+						Type: "pkcs11",
+						Name: "autoSeal",
+					},
+					{
+						Type:     "pkcs11",
+						Name:     "autoSeal",
+						Disabled: true,
+					},
+				},
+			},
+			isRewrapped:              true,
+			hasPartiallyWrappedPaths: false,
+			isErrorExpected:          true,
+			expectedErrorMsg:         "Initializing a cluster or enabling multi-seal on an existing cluster must occur with a single seal before adding additional seals",
+		},
+		// migrate from non-beta single seal to multi seal
+		{
+			name:                "none_to_multiple_seals",
+			existingSealGenInfo: nil,
+			newSealGenInfo: &seal.SealGenerationInfo{
+				Generation: 1,
+				Seals: []*configutil.KMS{
+					{
+						Type:     "pkcs11",
+						Name:     "autoSeal1",
+						Priority: 1,
+					},
+					{
+						Type:     "pkcs11",
+						Name:     "autoSeal2",
+						Priority: 2,
+					},
+				},
+			},
+			isRewrapped:              true,
+			hasPartiallyWrappedPaths: false,
+			isErrorExpected:          true,
+			expectedErrorMsg:         "Initializing a cluster or enabling multi-seal on an existing cluster must occur with a single seal before adding additional seals",
+		},
 		// have partially wrapped paths
 		{
 			name: "have_partially_wrapped_paths",
--- a/vault/seal/seal.go
+++ b/vault/seal/seal.go
@ -62,32 +62,45 @@ type SealGenerationInfo struct {
 // Validate is used to sanity check the seal generation info being created
 func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPartiallyWrappedPaths bool) error {
 	existingSealsLen := 0
-	previousShamirConfigured := false
-	existingSealNameAndType := "[]"
+	numConfiguredSeals := len(sgi.Seals)
 	configuredSealNameAndType := sealNameAndTypeAsStr(sgi.Seals)

-	if existingSgi != nil {
-		existingSealNameAndType = sealNameAndTypeAsStr(existingSgi.Seals)
-		if sgi.Generation == existingSgi.Generation {
-			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
+	// If no previous generation info exists, make sure we perform the initial migration/setup
+	// check for enabled configured seals to allow an old style seal migration configuration
+	if existingSgi == nil {
+		if numConfiguredSeals > 1 {
+			return fmt.Errorf("Initializing a cluster or enabling multi-seal on an existing "+
+				"cluster must occur with a single seal before adding additional seals\n"+
+				"Configured seals: %v", configuredSealNameAndType)
 		}

-		existingSealsLen = len(existingSgi.Seals)
-		for _, sealKmsConfig := range existingSgi.Seals {
-			if sealKmsConfig.Type == wrapping.WrapperTypeShamir.String() {
-				previousShamirConfigured = true
-				break
-			}
-		}
+		// No point in comparing anything more as we don't have any information around the
+		// existing seal if any actually existed
+		return nil
+	}

-		if !previousShamirConfigured && (!existingSgi.IsRewrapped() || hasPartiallyWrappedPaths) {
-			return errors.New("cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes")
+	existingSealNameAndType := sealNameAndTypeAsStr(existingSgi.Seals)
+	previousShamirConfigured := false
+
+	if sgi.Generation == existingSgi.Generation {
+		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
+	}
+
+	existingSealsLen = len(existingSgi.Seals)
+	for _, sealKmsConfig := range existingSgi.Seals {
+		if sealKmsConfig.Type == wrapping.WrapperTypeShamir.String() {
+			previousShamirConfigured = true
+			break
+		}
+	}
+
+	if !previousShamirConfigured && (!existingSgi.IsRewrapped() || hasPartiallyWrappedPaths) {
+		return errors.New("cannot make seal config changes while seal re-wrap is in progress, please revert any seal configuration changes")
 	}

 	numSealsToAdd := 0
@ -97,12 +110,12 @@ func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo, hasPart
 	// be set disabled, so, the number of seals to add is always going to be the length
 	// of new seal configs.
 	if previousShamirConfigured {
-		numSealsToAdd = len(sgi.Seals)
+		numSealsToAdd = numConfiguredSeals
 	} else {
-		numSealsToAdd = len(sgi.Seals) - existingSealsLen
+		numSealsToAdd = numConfiguredSeals - existingSealsLen
 	}

-	numSealsToDelete := existingSealsLen - len(sgi.Seals)
+	numSealsToDelete := existingSealsLen - numConfiguredSeals
 	switch {
 	case numSealsToAdd > 1:
 		return fmt.Errorf("cannot add more than one seal\n"+