Protect SealWrapper health fields (#22740)

* Create accessors for SealWrapper fields protecteb by the lock.

* Use NewSealWrapper constructor to create all seal wrappers.
This commit is contained in:
Victor Rodriguez
2023-09-01 14:38:11 -04:00
committed by GitHub
parent 07e76196ba
commit 5dc85c58c1
12 changed files with 163 additions and 118 deletions

View File

@@ -2599,8 +2599,8 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
recordSealConfigError := func(err error) {
sealConfigError = errors.Join(sealConfigError, err)
}
enabledSealWrappers := make([]vaultseal.SealWrapper, 0)
disabledSealWrappers := make([]vaultseal.SealWrapper, 0)
enabledSealWrappers := make([]*vaultseal.SealWrapper, 0)
disabledSealWrappers := make([]*vaultseal.SealWrapper, 0)
allSealKmsConfigs := make([]*configutil.KMS, 0)
type infoKeysAndMap struct {
@@ -2642,13 +2642,13 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
wrapper = aeadwrapper.NewShamirWrapper()
}
sealWrapper := vaultseal.SealWrapper{
Wrapper: wrapper,
Priority: configSeal.Priority,
Name: configSeal.Name,
SealConfigType: configSeal.Type,
Disabled: configSeal.Disabled,
}
sealWrapper := vaultseal.NewSealWrapper(
wrapper,
configSeal.Priority,
configSeal.Name,
configSeal.Type,
configSeal.Disabled,
)
if configSeal.Disabled {
disabledSealWrappers = append(disabledSealWrappers, sealWrapper)
@@ -2666,8 +2666,10 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Set the info keys, this modifies the function arguments `info` and `infoKeys`
// TODO(SEALHA): Why are we doing this? What is its use?
appendWrapperInfoKeys := func(prefix string, sealWrappers []vaultseal.SealWrapper) {
if len(sealWrappers) > 0 {
appendWrapperInfoKeys := func(prefix string, sealWrappers []*vaultseal.SealWrapper) {
if len(sealWrappers) == 0 {
return
}
useName := false
if len(sealWrappers) > 1 {
useName = true
@@ -2682,7 +2684,6 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
}
}
}
}
appendWrapperInfoKeys("", enabledSealWrappers)
appendWrapperInfoKeys("Old", disabledSealWrappers)
@@ -2697,7 +2698,7 @@ func setSeal(c *ServerCommand, config *server.Config, infoKeys []string, info ma
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Create the Seals
containsShamir := func(sealWrappers []vaultseal.SealWrapper) bool {
containsShamir := func(sealWrappers []*vaultseal.SealWrapper) bool {
for _, si := range sealWrappers {
if vault.SealConfigTypeShamir.IsSameAs(si.SealConfigType) {
return true

View File

@@ -73,13 +73,7 @@ func (tss *TransitSealServer) MakeSeal(t testing.T, key string) (vault.Seal, err
t.Fatalf("error setting wrapper config: %v", err)
}
access, err := seal.NewAccessFromSealWrappers(tss.Logger, 1, true, []seal.SealWrapper{
{
Wrapper: transitSealWrapper,
Priority: 1,
Name: "transit",
},
})
access, err := seal.NewAccessFromWrapper(tss.Logger, transitSealWrapper, vault.SealConfigTypeTransit.String())
if err != nil {
return nil, err
}

View File

@@ -1113,13 +1113,7 @@ func CreateCore(conf *CoreConfig) (*Core, error) {
wrapper := aeadwrapper.NewShamirWrapper()
wrapper.SetConfig(context.Background(), awskms.WithLogger(c.logger.Named("shamir")))
access, err := vaultseal.NewAccessFromSealWrappers(c.logger, 1, true, []vaultseal.SealWrapper{
{
Wrapper: wrapper,
Priority: 1,
Name: "shamir",
},
})
access, err := vaultseal.NewAccessFromWrapper(c.logger, wrapper, SealConfigTypeShamir.String())
if err != nil {
return nil, err
}
@@ -2885,13 +2879,7 @@ func (c *Core) adjustForSealMigration(unwrapSeal Seal) error {
// See note about creating a SealGenerationInfo for the unwrap seal in
// function setSeal in server.go.
sealAccess, err := vaultseal.NewAccessFromSealWrappers(c.logger, 1, true, []vaultseal.SealWrapper{
{
Wrapper: aeadwrapper.NewShamirWrapper(),
Priority: 1,
Name: "shamir",
},
})
sealAccess, err := vaultseal.NewAccessFromWrapper(c.logger, aeadwrapper.NewShamirWrapper(), SealConfigTypeShamir.String())
if err != nil {
return err
}
@@ -3113,13 +3101,7 @@ func (c *Core) unsealKeyToRootKey(ctx context.Context, seal Seal, combinedKey []
if useTestSeal {
// Note that the seal generation should not matter, since the only thing we are doing with
// this seal is calling GetStoredKeys (i.e. we are not encrypting anything).
sealAccess, err := vaultseal.NewAccessFromSealWrappers(c.logger, 1, true, []vaultseal.SealWrapper{
{
Wrapper: aeadwrapper.NewShamirWrapper(),
Priority: 1,
Name: "shamir",
},
})
sealAccess, err := vaultseal.NewAccessFromWrapper(c.logger, aeadwrapper.NewShamirWrapper(), SealConfigTypeShamir.String())
if err != nil {
return nil, fmt.Errorf("failed to setup seal wrapper for test barrier config: %w", err)
}

View File

@@ -4998,14 +4998,15 @@ func (c *Core) GetSealBackendStatus(ctx context.Context) (*SealBackendStatusResp
for _, sealWrapper := range a.GetAllSealWrappersByPriority() {
b := SealBackendStatus{
Name: sealWrapper.Name,
Healthy: sealWrapper.Healthy,
Healthy: sealWrapper.IsHealthy(),
}
if !sealWrapper.Healthy {
if !sealWrapper.LastSeenHealthy.IsZero() {
b.UnhealthySince = sealWrapper.LastSeenHealthy.String()
if !sealWrapper.IsHealthy() {
lastSeenHealthy := sealWrapper.LastSeenHealthy()
if !lastSeenHealthy.IsZero() {
b.UnhealthySince = lastSeenHealthy.String()
}
if uhMin.IsZero() || uhMin.After(sealWrapper.LastSeenHealthy) {
uhMin = sealWrapper.LastSeenHealthy
if uhMin.IsZero() || uhMin.After(lastSeenHealthy) {
uhMin = lastSeenHealthy
}
}
r.Backends = append(r.Backends, b)

View File

@@ -13,6 +13,7 @@ import (
"net/http"
aeadwrapper "github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/vault/helper/pgpkeys"
"github.com/hashicorp/vault/sdk/helper/consts"
@@ -400,16 +401,12 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string)
}
case c.seal.BarrierSealConfigType() == SealConfigTypeShamir:
if c.seal.StoredKeysSupported() == seal.StoredKeysSupportedShamirRoot {
access, err := seal.NewAccessFromSealWrappers(c.logger, c.seal.GetAccess().Generation(), true, []seal.SealWrapper{
{
Wrapper: aeadwrapper.NewShamirWrapper(),
Priority: 1,
Name: existingConfig.Name,
},
})
access, err := seal.NewAccessFromWrapper(c.logger, aeadwrapper.NewShamirWrapper(), SealConfigTypeShamir.String())
if err != nil {
return nil, logical.CodedError(http.StatusInternalServerError, fmt.Errorf("failed to setup test seal: %w", err).Error())
}
access.GetAllSealWrappersByPriority()[0].Name = existingConfig.Name
testseal := NewDefaultSeal(access)
testseal.SetCore(c)
err = testseal.GetAccess().SetShamirSealKey(recoveredKey)

View File

@@ -215,7 +215,7 @@ type access struct {
var _ Access = (*access)(nil)
func NewAccess(logger hclog.Logger, sealGenerationInfo *SealGenerationInfo, sealWrappers []SealWrapper) Access {
func NewAccess(logger hclog.Logger, sealGenerationInfo *SealGenerationInfo, sealWrappers []*SealWrapper) Access {
if logger == nil {
logger = hclog.NewNullLogger()
}
@@ -231,10 +231,7 @@ func NewAccess(logger hclog.Logger, sealGenerationInfo *SealGenerationInfo, seal
}
a.wrappersByPriority = make([]*SealWrapper, len(sealWrappers))
for i, sw := range sealWrappers {
v := sw
a.wrappersByPriority[i] = &v
v.Healthy = true
v.LastSeenHealthy = time.Now()
a.wrappersByPriority[i] = sw
}
sort.Slice(a.wrappersByPriority, func(i int, j int) bool { return a.wrappersByPriority[i].Priority < a.wrappersByPriority[j].Priority })
@@ -242,7 +239,7 @@ func NewAccess(logger hclog.Logger, sealGenerationInfo *SealGenerationInfo, seal
return a
}
func NewAccessFromSealWrappers(logger hclog.Logger, generation uint64, rewrapped bool, sealWrappers []SealWrapper) (Access, error) {
func NewAccessFromSealWrappers(logger hclog.Logger, generation uint64, rewrapped bool, sealWrappers []*SealWrapper) (Access, error) {
sealGenerationInfo := &SealGenerationInfo{
Generation: generation,
}
@@ -262,6 +259,16 @@ func NewAccessFromSealWrappers(logger hclog.Logger, generation uint64, rewrapped
return NewAccess(logger, sealGenerationInfo, sealWrappers), nil
}
// NewAccessFromWrapper creates an enabled Access for a single wrapping.Wrapper.
// The Access has generation set to 1 and the rewrapped flag set to true.
// The SealWrapper created uses the seal config type as the name, has priority set to 1 and the
// disabled flag set to false.
func NewAccessFromWrapper(logger hclog.Logger, wrapper wrapping.Wrapper, sealConfigType string) (Access, error) {
sealWrapper := NewSealWrapper(wrapper, 1, sealConfigType, sealConfigType, false)
return NewAccessFromSealWrappers(logger, 1, true, []*SealWrapper{sealWrapper})
}
func (a *access) GetAllSealWrappersByPriority() []*SealWrapper {
return copySealWrappers(a.wrappersByPriority, false)
}
@@ -276,9 +283,7 @@ func (a *access) AllSealWrappersHealthy() bool {
if sw.Disabled {
continue
}
sw.HcLock.RLock()
defer sw.HcLock.RUnlock()
if !sw.Healthy {
if !sw.IsHealthy() {
return false
}
}
@@ -361,6 +366,7 @@ func (a *access) Encrypt(ctx context.Context, plaintext []byte, options ...wrapp
errs := make(map[string]error)
for _, sealWrapper := range a.GetEnabledSealWrappersByPriority() {
now := time.Now()
var encryptErr error
defer func(now time.Time) {
metrics.MeasureSince([]string{"seal", "encrypt", "time"}, now)
@@ -370,7 +376,7 @@ func (a *access) Encrypt(ctx context.Context, plaintext []byte, options ...wrapp
metrics.IncrCounter([]string{"seal", "encrypt", "error"}, 1)
metrics.IncrCounter([]string{"seal", sealWrapper.Name, "encrypt", "error"}, 1)
}
}(time.Now())
}(now)
metrics.IncrCounter([]string{"seal", "encrypt"}, 1)
metrics.IncrCounter([]string{"seal", sealWrapper.Name, "encrypt"}, 1)
@@ -381,7 +387,7 @@ func (a *access) Encrypt(ctx context.Context, plaintext []byte, options ...wrapp
a.logger.Trace("error encrypting with seal", "seal", sealWrapper.Name, "err", encryptErr)
errs[sealWrapper.Name] = encryptErr
sealWrapper.Healthy = false
sealWrapper.SetHealthy(false, now)
} else {
a.logger.Trace("encrypted value using seal", "seal", sealWrapper.Name, "keyId", ciphertext.KeyInfo.KeyId)

View File

@@ -43,14 +43,21 @@ func NewTestSealOpts(opts *TestSealOpts) *TestSealOpts {
func NewTestSeal(opts *TestSealOpts) (Access, []*ToggleableWrapper) {
opts = NewTestSealOpts(opts)
wrappers := make([]*ToggleableWrapper, opts.WrapperCount)
sealWrappers := make([]SealWrapper, opts.WrapperCount)
sealWrappers := make([]*SealWrapper, opts.WrapperCount)
ctx := context.Background()
for i := 0; i < opts.WrapperCount; i++ {
wrappers[i] = &ToggleableWrapper{Wrapper: wrapping.NewTestWrapper(opts.Secret)}
sealWrappers[i] = SealWrapper{
Wrapper: wrappers[i],
Priority: i + 1,
Name: fmt.Sprintf("%s-%d", opts.Name, i+1),
wrapperType, err := wrappers[i].Type(ctx)
if err != nil {
panic(err)
}
sealWrappers[i] = NewSealWrapper(
wrappers[i],
i+1,
fmt.Sprintf("%s-%d", opts.Name, i+1),
wrapperType.String(),
false,
)
}
sealAccess, err := NewAccessFromSealWrappers(nil, opts.Generation, true, sealWrappers)
@@ -64,16 +71,24 @@ func NewToggleableTestSeal(opts *TestSealOpts) (Access, []func(error)) {
opts = NewTestSealOpts(opts)
wrappers := make([]*ToggleableWrapper, opts.WrapperCount)
sealWrappers := make([]SealWrapper, opts.WrapperCount)
sealWrappers := make([]*SealWrapper, opts.WrapperCount)
funcs := make([]func(error), opts.WrapperCount)
ctx := context.Background()
for i := 0; i < opts.WrapperCount; i++ {
w := &ToggleableWrapper{Wrapper: wrapping.NewTestWrapper(opts.Secret)}
wrappers[i] = w
sealWrappers[i] = SealWrapper{
Wrapper: wrappers[i],
Priority: i + 1,
Name: fmt.Sprintf("%s-%d", opts.Name, i+1),
wrapperType, err := w.Type(ctx)
if err != nil {
panic(err)
}
wrappers[i] = w
sealWrappers[i] = NewSealWrapper(
wrappers[i],
i+1,
fmt.Sprintf("%s-%d", opts.Name, i+1),
wrapperType.String(),
false,
)
funcs[i] = w.SetError
}

View File

@@ -13,6 +13,7 @@ import (
)
// SealWrapper contains a Wrapper and related information needed by the seal that uses it.
// Use NewSealWrapper to construct new instances, do not do it directly.
type SealWrapper struct {
Wrapper wrapping.Wrapper
Priority int
@@ -25,17 +26,69 @@ type SealWrapper struct {
// Disabled indicates, when true indicates that this wrapper should only be used for decryption.
Disabled bool
HcLock sync.RWMutex
LastHealthCheck time.Time
LastSeenHealthy time.Time
Healthy bool
// hcLock protects lastHealthy, lastSeenHealthy, and healthy. Do not modify those fields directly, use setHealth instead.
hcLock sync.RWMutex
lastHealthCheck time.Time
lastSeenHealthy time.Time
healthy bool
}
func NewSealWrapper(wrapper wrapping.Wrapper, priority int, name string, sealConfigType string, disabled bool) *SealWrapper {
ret := &SealWrapper{
Wrapper: wrapper,
Priority: priority,
Name: name,
SealConfigType: sealConfigType,
Disabled: disabled,
}
ret.setHealth(true, time.Now(), ret.lastHealthCheck)
return ret
}
func (sw *SealWrapper) rlock() func() {
sw.hcLock.RLock()
return sw.hcLock.RUnlock
}
func (sw *SealWrapper) lock() func() {
sw.hcLock.Lock()
return sw.hcLock.Unlock
}
func (sw *SealWrapper) SetHealthy(healthy bool, checkTime time.Time) {
unlock := sw.lock()
defer unlock()
wasHealthy := sw.healthy
lastHealthy := sw.lastSeenHealthy
if !wasHealthy && healthy {
lastHealthy = checkTime
}
sw.setHealth(healthy, lastHealthy, checkTime)
}
func (sw *SealWrapper) IsHealthy() bool {
sw.HcLock.RLock()
defer sw.HcLock.RUnlock()
unlock := sw.rlock()
defer unlock()
return sw.Healthy
return sw.healthy
}
func (sw *SealWrapper) LastSeenHealthy() time.Time {
unlock := sw.rlock()
defer unlock()
return sw.lastSeenHealthy
}
func (sw *SealWrapper) LastHealthCheck() time.Time {
unlock := sw.rlock()
defer unlock()
return sw.lastHealthCheck
}
var (
@@ -46,13 +99,11 @@ var (
)
func (sw *SealWrapper) CheckHealth(ctx context.Context, checkTime time.Time) error {
sw.HcLock.Lock()
defer sw.HcLock.Unlock()
sw.LastHealthCheck = checkTime
unlock := sw.lock()
defer unlock()
// Assume the wrapper is unhealthy, if we make it to the end we'll set it to true
sw.Healthy = false
sw.setHealth(false, sw.lastSeenHealthy, checkTime)
testVal := fmt.Sprintf("Heartbeat %d", mathrand.Intn(1000))
ciphertext, err := sw.Wrapper.Encrypt(ctx, []byte(testVal), nil)
@@ -70,8 +121,14 @@ func (sw *SealWrapper) CheckHealth(ctx context.Context, checkTime time.Time) err
return errors.New("failed to decrypt health test value to expected result")
}
sw.LastSeenHealthy = checkTime
sw.Healthy = true
sw.setHealth(true, checkTime, checkTime)
return nil
}
// setHealth sets the fields protected by sw.hcLock, callers *must* hold the write lock.
func (sw *SealWrapper) setHealth(healthy bool, lastSeenHealthy, lastHealthCheck time.Time) {
sw.healthy = healthy
sw.lastSeenHealthy = lastSeenHealthy
sw.lastHealthCheck = lastHealthCheck
}

View File

@@ -480,7 +480,7 @@ func (d *autoSeal) StartHealthCheck() {
// Seal wrapper is unhealthy
d.logger.Warn("seal wrapper health check failed", "seal_name", sealWrapper.Name, "err", err)
d.core.MetricSink().SetGaugeWithLabels(autoSealUnavailableDuration,
float32(time.Since(sealWrapper.LastSeenHealthy).Milliseconds()), mLabels)
float32(time.Since(sealWrapper.LastSeenHealthy()).Milliseconds()), mLabels)
allHealthy = false
} else {
// Seal wrapper is healthy
@@ -488,7 +488,7 @@ func (d *autoSeal) StartHealthCheck() {
d.logger.Debug("seal wrapper health test passed", "seal_name", sealWrapper.Name)
} else {
d.logger.Info("seal wrapper is now healthy again", "downtime", "seal_name", sealWrapper.Name,
now.Sub(sealWrapper.LastSeenHealthy).String())
now.Sub(sealWrapper.LastSeenHealthy()).String())
}
allUnhealthy = false
}

View File

@@ -181,7 +181,7 @@ func TestAutoSeal_HealthCheck(t *testing.T) {
metrics.NewGlobal(metricsConf, inmemSink)
pBackend := newTestBackend(t)
testSealAccess, setErrs := seal.NewToggleableTestSeal(nil)
testSealAccess, setErrs := seal.NewToggleableTestSeal(&seal.TestSealOpts{Name: "health-test"})
core, _, _ := TestCoreUnsealedWithConfig(t, &CoreConfig{
MetricSink: metricsutil.NewClusterMetricSink("", inmemSink),
Physical: pBackend,

View File

@@ -144,6 +144,8 @@ const (
SealConfigTypePkcs11 = SealConfigType(wrapping.WrapperTypePkcs11)
SealConfigTypeAwsKms = SealConfigType(wrapping.WrapperTypeAwsKms)
SealConfigTypeHsmAutoDeprecated = SealConfigType(wrapping.WrapperTypeHsmAuto)
SealConfigTypeTransit = SealConfigType(wrapping.WrapperTypeTransit)
SealConfigTypeGcpCkms = SealConfigType(wrapping.WrapperTypeGcpCkms)
// SealConfigTypeRecovery is an alias for SealConfigTypeShamir since all recovery seals are
// defaultSeals using shamir wrappers.

View File

@@ -4,7 +4,7 @@
package vault
import (
aeadwrapper "github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2"
"github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2"
"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
"github.com/hashicorp/vault/vault/seal"
testing "github.com/mitchellh/go-testing-interface"
@@ -17,13 +17,8 @@ func NewTestSeal(t testing.T, opts *seal.TestSealOpts) Seal {
switch opts.StoredKeys {
case seal.StoredKeysSupportedShamirRoot:
w := aeadwrapper.NewShamirWrapper()
sealAccess, err := seal.NewAccessFromSealWrappers(logger, opts.Generation, true, []seal.SealWrapper{
{
Wrapper: w,
Priority: 1,
Name: "shamir",
},
sealAccess, err := seal.NewAccessFromSealWrappers(logger, opts.Generation, true, []*seal.SealWrapper{
seal.NewSealWrapper(aead.NewShamirWrapper(), 1, "shamir", "shamir", false),
})
if err != nil {
t.Fatal("error creating test seal", err)
@@ -37,13 +32,8 @@ func NewTestSeal(t testing.T, opts *seal.TestSealOpts) Seal {
})
return newSeal
case seal.StoredKeysNotSupported:
w := aeadwrapper.NewShamirWrapper()
sealAccess, err := seal.NewAccessFromSealWrappers(logger, opts.Generation, true, []seal.SealWrapper{
{
Wrapper: w,
Priority: 1,
Name: "shamir",
},
sealAccess, err := seal.NewAccessFromSealWrappers(logger, opts.Generation, true, []*seal.SealWrapper{
seal.NewSealWrapper(aead.NewShamirWrapper(), 1, "shamir", "shamir", false),
})
if err != nil {
t.Fatal("error creating test seal", err)