Add safety logic for rejecting seal configuration changes (#22582)

* Fix clone method and add new validation for same gen

* Add safety logic for rejecting seal configuration changes

* Remove ent build req for test file
This commit is contained in:
Divya Pola
2023-08-28 11:58:16 -05:00
committed by GitHub
parent 36174bc913
commit e600d43690
4 changed files with 712 additions and 15 deletions

View File

@@ -2654,7 +2654,10 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Compute seal generation
sealGenerationInfo := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs)
sealGenerationInfo, err := c.computeSealGenerationInfo(existingSealGenerationInfo, allSealKmsConfigs)
if err != nil {
return nil, err
}
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Create the Seals
@@ -2680,7 +2683,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
return nil, errors.New("no enabled Seals in configuration")
case containsShamir(enabledSealInfos) && containsShamir(disabledSealInfos):
return nil, errors.New("cannot migrate from one Shamir seal to another Shamir seal")
return nil, errors.New("shamir seals cannot be set disabled (they should simply not be set)")
case len(enabledSealInfos) == 1 && containsShamir(enabledSealInfos):
// The barrier seal is Shamir. If there are any disabled seals, then we put them all in the same
@@ -2720,25 +2723,31 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
}, nil
}
func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS) *vaultseal.SealGenerationInfo {
if existingSealGenInfo == nil {
return &vaultseal.SealGenerationInfo{
Generation: 1,
Seals: sealConfigs,
}
}
if cmp.Equal(existingSealGenInfo.Seals, sealConfigs) {
return existingSealGenInfo
}
func (c *ServerCommand) computeSealGenerationInfo(existingSealGenInfo *vaultseal.SealGenerationInfo, sealConfigs []*configutil.KMS) (*vaultseal.SealGenerationInfo, error) {
var generation uint64
generation = 1
generation := existingSealGenInfo.Generation + 1
if existingSealGenInfo != nil {
if cmp.Equal(existingSealGenInfo.Seals, sealConfigs) {
return existingSealGenInfo, nil
}
generation = existingSealGenInfo.Generation + 1
}
c.logger.Info("incrementing seal config gen, new generation: ", "generation", generation)
// If the stored copy doesn't match the current configuration, we introduce a new generation which keeps track if a rewrap of all CSPs and seal wrapped values has completed (initially false).
return &vaultseal.SealGenerationInfo{
// If the stored copy doesn't match the current configuration, we introduce a new generation
// which keeps track if a rewrap of all CSPs and seal wrapped values has completed (initially false).
newSealGenInfo := &vaultseal.SealGenerationInfo{
Generation: generation,
Seals: sealConfigs,
}
err := newSealGenInfo.Validate(existingSealGenInfo)
if err != nil {
return nil, err
}
return newSealGenInfo, nil
}
func initHaBackend(c *ServerCommand, config *server.Config, coreConfig *vault.CoreConfig, backend physical.Backend) (bool, error) {

View File

@@ -0,0 +1,606 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
package command
import (
"os"
"testing"
"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
"github.com/hashicorp/vault/vault/seal"
"github.com/hashicorp/vault/internalshared/configutil"
"github.com/stretchr/testify/require"
)
func init() {
if signed := os.Getenv("VAULT_LICENSE_CI"); signed != "" {
os.Setenv(EnvVaultLicense, signed)
}
}
func TestMultiSealCases(t *testing.T) {
cases := []struct {
name string
existingSealGenInfo *seal.SealGenerationInfo
allSealKmsConfigs []*configutil.KMS
expectedSealGenInfo *seal.SealGenerationInfo
isErrorExpected bool
expectedErrorMsg string
}{
// none_to_shamir
{
name: "none_to_shamir",
existingSealGenInfo: nil,
allSealKmsConfigs: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
},
},
},
// none_to_auto
{
name: "none_to_auto",
existingSealGenInfo: nil,
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
},
// none_to_multi
{
name: "none_to_multi",
existingSealGenInfo: nil,
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
isErrorExpected: true,
expectedErrorMsg: "cannot add more than one seal",
},
// shamir_to_auto
{
name: "shamir_to_auto",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 3,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
},
// shamir_to_multi
{
name: "shamir_to_multi",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 2,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 3,
},
},
isErrorExpected: true,
expectedErrorMsg: "cannot add more than one seal",
},
// auto_to_shamir_no_common_seal
{
name: "auto_to_shamir_no_common_seal",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
},
isErrorExpected: true,
expectedErrorMsg: "must have at least one seal in common with the old generation",
},
// auto_to_shamir_with_common_seal
{
name: "auto_to_shamir_with_common_seal",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
Disabled: true,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "shamir",
Name: "shamirSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
Disabled: true,
},
},
},
},
// auto_to_auto_no_common_seal
{
name: "auto_to_auto_no_common_seal",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 1,
},
},
isErrorExpected: true,
expectedErrorMsg: "must have at least one seal in common with the old generation",
},
// auto_to_auto_with_common_seal
{
name: "auto_to_auto_with_common_seal",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
Disabled: true,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
Disabled: true,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
},
// auto_to_multi_add_one
{
name: "auto_to_multi_add_one",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
},
// auto_to_multi_add_two
{
name: "auto_to_multi_add_two",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 3,
},
},
isErrorExpected: true,
expectedErrorMsg: "cannot add more than one seal",
},
// multi_to_auto_delete_one
{
name: "multi_to_auto_delete_one",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
},
},
// multi_to_auto_delete_two
{
name: "multi_to_auto_delete_two",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 3,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
},
isErrorExpected: true,
expectedErrorMsg: "cannot delete more than one seal",
},
// disable_two_auto
{
name: "disable_two_auto",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 3,
},
},
},
allSealKmsConfigs: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
Disabled: true,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 3,
Disabled: true,
},
},
expectedSealGenInfo: &seal.SealGenerationInfo{
Generation: 2,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
Disabled: true,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 3,
Disabled: true,
},
},
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
cmd := &ServerCommand{}
cmd.logger = corehelpers.NewTestLogger(t)
sealGenInfo, err := cmd.computeSealGenerationInfo(tc.existingSealGenInfo, tc.allSealKmsConfigs)
switch {
case tc.isErrorExpected:
require.Error(t, err)
require.EqualError(t, err, tc.expectedErrorMsg)
require.Nil(t, sealGenInfo)
default:
require.NoError(t, err)
require.Equal(t, tc.expectedSealGenInfo, sealGenInfo)
}
})
}
cases2 := []struct {
name string
existingSealGenInfo *seal.SealGenerationInfo
newSealGenInfo *seal.SealGenerationInfo
isErrorExpected bool
expectedErrorMsg string
}{
// same_generation_different_seals
{
name: "same_generation_different_seals",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
newSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal3",
Priority: 2,
},
},
},
isErrorExpected: true,
expectedErrorMsg: "existing seal generation is the same, but the configured seals are different",
},
// same_generation_same_seals
{
name: "same_generation_same_seals",
existingSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
newSealGenInfo: &seal.SealGenerationInfo{
Generation: 1,
Seals: []*configutil.KMS{
{
Type: "pkcs11",
Name: "autoSeal1",
Priority: 1,
},
{
Type: "pkcs11",
Name: "autoSeal2",
Priority: 2,
},
},
},
isErrorExpected: false,
},
}
for _, tc := range cases2 {
t.Run(tc.name, func(t *testing.T) {
err := tc.newSealGenInfo.Validate(tc.existingSealGenInfo)
switch {
case tc.isErrorExpected:
require.Error(t, err)
require.EqualError(t, err, tc.expectedErrorMsg)
default:
require.NoError(t, err)
}
})
}
}

View File

@@ -437,3 +437,16 @@ func getEnvConfig(kms *KMS) map[string]string {
return envValues
}
func (k *KMS) Clone() *KMS {
ret := &KMS{
UnusedKeys: k.UnusedKeys,
Type: k.Type,
Purpose: k.Purpose,
Config: k.Config,
Name: k.Name,
Disabled: k.Disabled,
Priority: k.Priority,
}
return ret
}

View File

@@ -13,6 +13,8 @@ import (
"sync/atomic"
"time"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/internalshared/configutil"
@@ -51,6 +53,73 @@ type SealGenerationInfo struct {
rewrapped atomic.Bool
}
// Validate is used to sanity check the seal generation info being created
func (sgi *SealGenerationInfo) Validate(existingSgi *SealGenerationInfo) error {
existingSealsLen := 0
previousShamirConfigured := false
if existingSgi != nil {
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")
}
return nil
}
existingSealsLen = len(existingSgi.Seals)
for _, sealKmsConfig := range existingSgi.Seals {
if sealKmsConfig.Type == wrapping.WrapperTypeShamir.String() {
previousShamirConfigured = true
break
}
}
}
numSealsToAdd := 0
// With a previously configured shamir seal, we are either going from [shamir]->[auto]
// or [shamir]->[another shamir] (since we do not allow multiple shamir
// seals, and, mixed shamir and auto seals). Also, we do not allow shamir seals to
// 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)
} else {
numSealsToAdd = len(sgi.Seals) - existingSealsLen
}
numSealsToDelete := existingSealsLen - len(sgi.Seals)
switch {
case numSealsToAdd > 1:
return errors.New("cannot add more than one seal")
case numSealsToDelete > 1:
return errors.New("cannot delete more than one seal")
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 nil
}
func haveCommonSeal(existingSealKmsConfigs, newSealKmsConfigs []*configutil.KMS) (result 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()) {
return true
}
}
}
return false
}
// SetRewrapped updates the SealGenerationInfo's rewrapped status to the provided value.
func (sgi *SealGenerationInfo) SetRewrapped(value bool) {
sgi.rewrapped.Store(value)