From 0b11c5e33bd2483e5bcdc3fe8d0d11eaad4aa599 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 19 Sep 2019 10:52:21 -0400 Subject: [PATCH 1/9] core: re-encrypt barrier and recovery keys if the unseal key is updated Seal keys can be rotated. When this happens, the barrier and recovery keys should be re-encrypted with the new seal key. This change automatically re-encrypts the barrier and recovery keys with the latest seal key on the active node during the 'postUnseal' phase. --- vault/core.go | 6 +++ vault/seal_autoseal.go | 92 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/vault/core.go b/vault/core.go index 7979361585e4..24a3a7fdf2e2 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1692,6 +1692,12 @@ func (c *Core) postUnseal(ctx context.Context, ctxCancelFunc context.CancelFunc, return err } + if seal, ok := c.seal.(*autoSeal); ok { + if err := seal.UpgradeKeys(c.activeContext); err != nil { + return err + } + } + c.metricsCh = make(chan struct{}) go c.emitMetrics(c.metricsCh) diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index ef8a7f5e51a1..e6059ca77920 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -147,6 +147,69 @@ func (d *autoSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { return keys, nil } +func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { + pe, err := d.core.physical.Get(ctx, StoredBarrierKeysPath) + if err != nil { + return errwrap.Wrapf("failed to fetch stored keys: {{err}}", err) + } + + // This is not strictly an error; we may not have any stored keys, for + // instance, if we're not initialized + if pe == nil { + return nil + } + + blobInfo := &physical.EncryptedBlobInfo{} + if err := proto.Unmarshal(pe.Value, blobInfo); err != nil { + return errwrap.Wrapf("failed to proto decode stored keys: {{err}}", err) + } + + if blobInfo.KeyInfo != nil && blobInfo.KeyInfo.KeyID != d.Access.KeyID() { + d.core.logger.Info("autoseal: upgrading stored keys") + + pt, err := d.Decrypt(ctx, blobInfo) + if err != nil { + return errwrap.Wrapf("failed to decrypt encrypted stored keys: {{err}}", err) + } + + // Decode the barrier entry + var keys [][]byte + if err := json.Unmarshal(pt, &keys); err != nil { + return errwrap.Wrapf("failed to decode stored keys: {{err}}", err) + } + + if err := d.SetStoredKeys(ctx, keys); err != nil { + return errwrap.Wrapf("failed to save upgraded stored keys: {{err}}", err) + } + } + return nil +} + +// UpgradeKeys re-encrypts and saves the stored keys and the recovery key +// with the current key if the current KeyID is different from the KeyID +// the stored keys and the recovery key are encrypted with. The provided +// Context must be non-nil. +func (d *autoSeal) UpgradeKeys(ctx context.Context) error { + // Exit if the context has been canceled + if ctx.Err() != nil { + return ctx.Err() + } + + // Many of the seals update their keys to the latest KeyID when Encrypt + // is called. + if _, err := d.Encrypt(ctx, []byte("a")); err != nil { + return err + } + + if err := d.upgradeRecoveryKey(ctx); err != nil { + return err + } + if err := d.upgradeStoredKeys(ctx); err != nil { + return err + } + return nil +} + func (d *autoSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { if d.barrierConfig.Load().(*SealConfig) != nil { return d.barrierConfig.Load().(*SealConfig).Clone(), nil @@ -430,6 +493,35 @@ func (d *autoSeal) getRecoveryKeyInternal(ctx context.Context) ([]byte, error) { return pt, nil } +func (d *autoSeal) upgradeRecoveryKey(ctx context.Context) error { + pe, err := d.core.physical.Get(ctx, recoveryKeyPath) + if err != nil { + return errwrap.Wrapf("failed to fetch recovery key: {{err}}", err) + } + if pe == nil { + return fmt.Errorf("no recovery key found") + } + + blobInfo := &physical.EncryptedBlobInfo{} + if err := proto.Unmarshal(pe.Value, blobInfo); err != nil { + return errwrap.Wrapf("failed to proto decode recovery key: {{err}}", err) + } + + if blobInfo.KeyInfo != nil && blobInfo.KeyInfo.KeyID != d.Access.KeyID() { + d.core.logger.Info("autoseal: upgrading recovery key") + + pt, err := d.Decrypt(ctx, blobInfo) + if err != nil { + return errwrap.Wrapf("failed to decrypt encrypted recovery key: {{err}}", err) + + } + if err := d.SetRecoveryKey(ctx, pt); err != nil { + return errwrap.Wrapf("failed to save upgraded recovery key: {{err}}", err) + } + } + return nil +} + // migrateRecoveryConfig is a helper func to migrate the recovery config to // live outside the barrier. This is called from SetRecoveryConfig which is // always called with the stateLock. From 166479f4758bcbc6e8321f776fd6be22253bfc76 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 26 Sep 2019 11:05:47 -0400 Subject: [PATCH 2/9] Update comment on vault.Core.postUnseal --- vault/core.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/core.go b/vault/core.go index 24a3a7fdf2e2..17d997cbc6b9 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1654,7 +1654,7 @@ func (s standardUnsealStrategy) unseal(ctx context.Context, logger log.Logger, c return nil } -// postUnseal is invoked after the barrier is unsealed, but before +// postUnseal is invoked on the active node after the barrier is unsealed, but before // allowing any user operations. This allows us to setup any state that // requires the Vault to be unsealed such as mount tables, logical backends, // credential stores, etc. From ae996a521d171427c7d3bf823aeed341839ed6d8 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 26 Sep 2019 16:44:03 -0400 Subject: [PATCH 3/9] Add comment to explain the call to autoSeal.UpgradeKeys --- vault/core.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vault/core.go b/vault/core.go index 17d997cbc6b9..30173a94fc94 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1692,6 +1692,12 @@ func (c *Core) postUnseal(ctx context.Context, ctxCancelFunc context.CancelFunc, return err } + // Automatically re-encrypt the keys used for auto unsealing when the + // seal's encryption key changes. The regular rotation of cryptographic + // keys is a NIST recommendation. Access to prior keys for decryption + // is normally supported for a configurable time period. Re-encrypting + // the keys used for auto unsealing ensures Vault and its data will + // continue to be accessible even after prior seal keys are destroyed. if seal, ok := c.seal.(*autoSeal); ok { if err := seal.UpgradeKeys(c.activeContext); err != nil { return err From ac1e6236cc1981198def8b3f5a9fa9e5a723fd65 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 26 Sep 2019 16:48:31 -0400 Subject: [PATCH 4/9] Return error if no stored keys are found during key upgrade --- vault/seal_autoseal.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index e6059ca77920..3d2f4340612f 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -152,11 +152,8 @@ func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { if err != nil { return errwrap.Wrapf("failed to fetch stored keys: {{err}}", err) } - - // This is not strictly an error; we may not have any stored keys, for - // instance, if we're not initialized if pe == nil { - return nil + return fmt.Errorf("no stored keys found") } blobInfo := &physical.EncryptedBlobInfo{} From 87b309c85213a9502b1f3ba4b2db9bb2d6a45a00 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 26 Sep 2019 16:51:02 -0400 Subject: [PATCH 5/9] Remove unnecessary check for a canceled context --- vault/seal_autoseal.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index 3d2f4340612f..020d4faf6677 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -187,11 +187,6 @@ func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { // the stored keys and the recovery key are encrypted with. The provided // Context must be non-nil. func (d *autoSeal) UpgradeKeys(ctx context.Context) error { - // Exit if the context has been canceled - if ctx.Err() != nil { - return ctx.Err() - } - // Many of the seals update their keys to the latest KeyID when Encrypt // is called. if _, err := d.Encrypt(ctx, []byte("a")); err != nil { From 05bfc8c1871703be50357e375dbfd47d2333f3dd Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 26 Sep 2019 19:20:52 -0400 Subject: [PATCH 6/9] Use a named logger in autoseal --- vault/seal_autoseal.go | 50 +++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index 020d4faf6677..e07ebd75b6ef 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -9,6 +9,7 @@ import ( proto "github.com/golang/protobuf/proto" "github.com/hashicorp/errwrap" + log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/vault/seal" ) @@ -26,6 +27,7 @@ type autoSeal struct { barrierConfig atomic.Value recoveryConfig atomic.Value core *Core + logger log.Logger } // Ensure we are implementing the Seal interface @@ -53,6 +55,10 @@ func (d *autoSeal) checkCore() error { func (d *autoSeal) SetCore(core *Core) { d.core = core + if d.logger == nil { + d.logger = d.core.Logger().Named("autoseal") + d.core.AddLogger(d.logger) + } } func (d *autoSeal) Init(ctx context.Context) error { @@ -162,7 +168,7 @@ func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { } if blobInfo.KeyInfo != nil && blobInfo.KeyInfo.KeyID != d.Access.KeyID() { - d.core.logger.Info("autoseal: upgrading stored keys") + d.logger.Info("upgrading stored keys") pt, err := d.Decrypt(ctx, blobInfo) if err != nil { @@ -215,14 +221,14 @@ func (d *autoSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { entry, err := d.core.physical.Get(ctx, barrierSealConfigPath) if err != nil { - d.core.logger.Error("autoseal: failed to read seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("failed to read seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("failed to read %q seal configuration: {{err}}", sealType), err) } // If the seal configuration is missing, we are not initialized if entry == nil { - if d.core.logger.IsInfo() { - d.core.logger.Info("autoseal: seal configuration missing, not initialized", "seal_type", sealType) + if d.logger.IsInfo() { + d.logger.Info("seal configuration missing, not initialized", "seal_type", sealType) } return nil, nil } @@ -230,20 +236,20 @@ func (d *autoSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { conf := &SealConfig{} err = json.Unmarshal(entry.Value, conf) if err != nil { - d.core.logger.Error("autoseal: failed to decode seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("failed to decode seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("failed to decode %q seal configuration: {{err}}", sealType), err) } // Check for a valid seal configuration if err := conf.Validate(); err != nil { - d.core.logger.Error("autoseal: invalid seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("invalid seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("%q seal validation failed: {{err}}", sealType), err) } barrierTypeUpgradeCheck(d.BarrierType(), conf) if conf.Type != d.BarrierType() { - d.core.logger.Error("autoseal: barrier seal type does not match loaded type", "seal_type", conf.Type, "loaded_type", d.BarrierType()) + d.logger.Error("barrier seal type does not match loaded type", "seal_type", conf.Type, "loaded_type", d.BarrierType()) return nil, fmt.Errorf("barrier seal type of %q does not match loaded type of %q", conf.Type, d.BarrierType()) } @@ -276,7 +282,7 @@ func (d *autoSeal) SetBarrierConfig(ctx context.Context, conf *SealConfig) error } if err := d.core.physical.Put(ctx, pe); err != nil { - d.core.logger.Error("autoseal: failed to write barrier seal configuration", "error", err) + d.logger.Error("failed to write barrier seal configuration", "error", err) return errwrap.Wrapf("failed to write barrier seal configuration: {{err}}", err) } @@ -309,13 +315,13 @@ func (d *autoSeal) RecoveryConfig(ctx context.Context) (*SealConfig, error) { var err error entry, err = d.core.physical.Get(ctx, recoverySealConfigPlaintextPath) if err != nil { - d.core.logger.Error("autoseal: failed to read seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("failed to read seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("failed to read %q seal configuration: {{err}}", sealType), err) } if entry == nil { if d.core.Sealed() { - d.core.logger.Info("autoseal: seal configuration missing, but cannot check old path as core is sealed", "seal_type", sealType) + d.logger.Info("seal configuration missing, but cannot check old path as core is sealed", "seal_type", sealType) return nil, nil } @@ -328,8 +334,8 @@ func (d *autoSeal) RecoveryConfig(ctx context.Context) (*SealConfig, error) { // If the seal configuration is missing, then we are not initialized. if be == nil { - if d.core.logger.IsInfo() { - d.core.logger.Info("autoseal: seal configuration missing, not initialized", "seal_type", sealType) + if d.logger.IsInfo() { + d.logger.Info("seal configuration missing, not initialized", "seal_type", sealType) } return nil, nil } @@ -343,18 +349,18 @@ func (d *autoSeal) RecoveryConfig(ctx context.Context) (*SealConfig, error) { conf := &SealConfig{} if err := json.Unmarshal(entry.Value, conf); err != nil { - d.core.logger.Error("autoseal: failed to decode seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("failed to decode seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("failed to decode %q seal configuration: {{err}}", sealType), err) } // Check for a valid seal configuration if err := conf.Validate(); err != nil { - d.core.logger.Error("autoseal: invalid seal configuration", "seal_type", sealType, "error", err) + d.logger.Error("invalid seal configuration", "seal_type", sealType, "error", err) return nil, errwrap.Wrapf(fmt.Sprintf("%q seal validation failed: {{err}}", sealType), err) } if conf.Type != d.RecoveryType() { - d.core.logger.Error("autoseal: recovery seal type does not match loaded type", "seal_type", conf.Type, "loaded_type", d.RecoveryType()) + d.logger.Error("recovery seal type does not match loaded type", "seal_type", conf.Type, "loaded_type", d.RecoveryType()) return nil, fmt.Errorf("recovery seal type of %q does not match loaded type of %q", conf.Type, d.RecoveryType()) } @@ -394,7 +400,7 @@ func (d *autoSeal) SetRecoveryConfig(ctx context.Context, conf *SealConfig) erro } if err := d.core.physical.Put(ctx, pe); err != nil { - d.core.logger.Error("autoseal: failed to write recovery seal configuration", "error", err) + d.logger.Error("failed to write recovery seal configuration", "error", err) return errwrap.Wrapf("failed to write recovery seal configuration: {{err}}", err) } @@ -450,7 +456,7 @@ func (d *autoSeal) SetRecoveryKey(ctx context.Context, key []byte) error { } if err := d.core.physical.Put(ctx, be); err != nil { - d.core.logger.Error("autoseal: failed to write recovery key", "error", err) + d.logger.Error("failed to write recovery key", "error", err) return errwrap.Wrapf("failed to write recovery key: {{err}}", err) } @@ -464,11 +470,11 @@ func (d *autoSeal) RecoveryKey(ctx context.Context) ([]byte, error) { func (d *autoSeal) getRecoveryKeyInternal(ctx context.Context) ([]byte, error) { pe, err := d.core.physical.Get(ctx, recoveryKeyPath) if err != nil { - d.core.logger.Error("autoseal: failed to read recovery key", "error", err) + d.logger.Error("failed to read recovery key", "error", err) return nil, errwrap.Wrapf("failed to read recovery key: {{err}}", err) } if pe == nil { - d.core.logger.Warn("autoseal: no recovery key found") + d.logger.Warn("no recovery key found") return nil, fmt.Errorf("no recovery key found") } @@ -500,7 +506,7 @@ func (d *autoSeal) upgradeRecoveryKey(ctx context.Context) error { } if blobInfo.KeyInfo != nil && blobInfo.KeyInfo.KeyID != d.Access.KeyID() { - d.core.logger.Info("autoseal: upgrading recovery key") + d.logger.Info("upgrading recovery key") pt, err := d.Decrypt(ctx, blobInfo) if err != nil { @@ -530,8 +536,8 @@ func (d *autoSeal) migrateRecoveryConfig(ctx context.Context) error { } // Only log if we are performing the migration - d.core.logger.Debug("migrating recovery seal configuration") - defer d.core.logger.Debug("done migrating recovery seal configuration") + d.logger.Debug("migrating recovery seal configuration") + defer d.logger.Debug("done migrating recovery seal configuration") // Perform migration pe := &physical.Entry{ From bfb7f680266b0011c487ef3d2661f628d2ad8c02 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Wed, 2 Oct 2019 11:48:39 -0400 Subject: [PATCH 7/9] Add unit tests --- vault/seal/seal_testing.go | 8 +- vault/seal_autoseal.go | 2 +- vault/seal_autoseal_test.go | 151 ++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 vault/seal_autoseal_test.go diff --git a/vault/seal/seal_testing.go b/vault/seal/seal_testing.go index 52bdb51409fb..1beecff8c85f 100644 --- a/vault/seal/seal_testing.go +++ b/vault/seal/seal_testing.go @@ -10,6 +10,7 @@ import ( type TestSeal struct { Type string secret []byte + keyId string } var _ Access = (*TestSeal)(nil) @@ -18,6 +19,7 @@ func NewTestSeal(secret []byte) *TestSeal { return &TestSeal{ Type: Test, secret: secret, + keyId: "static-key", } } @@ -34,7 +36,11 @@ func (t *TestSeal) SealType() string { } func (t *TestSeal) KeyID() string { - return "static-key" + return t.keyId +} + +func (t *TestSeal) SetKeyID(k string) { + t.keyId = k } func (t *TestSeal) Encrypt(_ context.Context, plaintext []byte) (*physical.EncryptedBlobInfo, error) { diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index e07ebd75b6ef..854840844892 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -33,7 +33,7 @@ type autoSeal struct { // Ensure we are implementing the Seal interface var _ Seal = (*autoSeal)(nil) -func NewAutoSeal(lowLevel seal.Access) Seal { +func NewAutoSeal(lowLevel seal.Access) *autoSeal { ret := &autoSeal{ Access: lowLevel, } diff --git a/vault/seal_autoseal_test.go b/vault/seal_autoseal_test.go new file mode 100644 index 000000000000..0184161be178 --- /dev/null +++ b/vault/seal_autoseal_test.go @@ -0,0 +1,151 @@ +package vault + +import ( + "bytes" + "context" + "reflect" + "testing" + + proto "github.com/golang/protobuf/proto" + "github.com/hashicorp/vault/sdk/physical" + "github.com/hashicorp/vault/vault/seal" +) + +type phy struct { + t *testing.T + entries map[string][]*physical.Entry +} + +func newTestBackend(t *testing.T) *phy { + return &phy{ + t: t, + entries: make(map[string][]*physical.Entry), + } +} + +func (p *phy) Put(_ context.Context, entry *physical.Entry) error { + p.entries[entry.Key] = append(p.entries[entry.Key], entry) + return nil +} + +func (p *phy) Get(_ context.Context, key string) (*physical.Entry, error) { + entries := p.entries[key] + if entries == nil { + return nil, nil + } + return entries[len(entries)-1], nil +} + +func (p *phy) Delete(_ context.Context, key string) error { + p.t.Errorf("Delete called on phy: key: %v", key) + return nil +} + +func (p *phy) List(_ context.Context, prefix string) ([]string, error) { + p.t.Errorf("List called on phy: prefix: %v", prefix) + return []string{}, nil +} + +func (p *phy) Len() int { + return len(p.entries) +} + +func TestAutoSeal_UpgradeKeys(t *testing.T) { + core, _, _ := TestCoreUnsealed(t) + testSeal := seal.NewTestSeal(nil) + + var encKeys []string + changeKey := func(key string) { + encKeys = append(encKeys, key) + testSeal.SetKeyID(key) + } + + // Set initial encryption key. + changeKey("kaz") + + autoSeal := NewAutoSeal(testSeal) + autoSeal.SetCore(core) + pBackend := newTestBackend(t) + core.physical = pBackend + + ctx := context.Background() + + inkeys := [][]byte{[]byte("grist"), []byte("house")} + if err := autoSeal.SetStoredKeys(ctx, inkeys); err != nil { + t.Fatalf("SetStoredKeys: want no error, got %v", err) + } + + inRecoveryKey := []byte("falernum") + if err := autoSeal.SetRecoveryKey(ctx, inRecoveryKey); err != nil { + t.Fatalf("SetRecoveryKey: want no error, got %v", err) + } + + check := func() { + // The values of the stored keys should never change. + outkeys, err := autoSeal.GetStoredKeys(ctx) + if err != nil { + t.Fatalf("GetStoredKeys: want no error, got %v", err) + } + if !reflect.DeepEqual(inkeys, outkeys) { + t.Errorf("incorrect stored keys: want %v, got %v", inkeys, outkeys) + } + + // The value of the recovery key should also never change. + outRecoveryKey, err := autoSeal.RecoveryKey(ctx) + if err != nil { + t.Fatalf("RecoveryKey: want no error, got %v", err) + } + if !bytes.Equal(inRecoveryKey, outRecoveryKey) { + t.Errorf("incorrect recovery key: want %q, got %q", inRecoveryKey, outRecoveryKey) + } + + // There should only be 2 entries in the physical backend. One for + // the stored keys and one for the recovery key. + if want, got := 2, pBackend.Len(); want != got { + t.Errorf("backend unexpected Len: want %d, got %d", want, got) + } + + for phyKey, phyEntries := range pBackend.entries { + // Calling UpgradeKeys should only add an entry if the key has + // changed. + if keyCount, entryCount := len(encKeys), len(phyEntries); keyCount != entryCount { + t.Errorf("phyKey = %s: encryption key count not equal to entry count: keys=%d, entries=%d", phyKey, keyCount, entryCount) + } + + // Each phyEntry should correspond to a key at the same index + // in encKeys. Iterate over each phyEntry and verify it was + // encrypted with its corresponding key in encKeys. + for i, phyEntry := range phyEntries { + blobInfo := &physical.EncryptedBlobInfo{} + if err := proto.Unmarshal(phyEntry.Value, blobInfo); err != nil { + t.Errorf("phyKey = %s: failed to proto decode stored keys: %s", phyKey, err) + } + if blobInfo.KeyInfo == nil { + t.Errorf("phyKey = %s: KeyInfo missing: %+v", phyKey, blobInfo) + } + if want, got := encKeys[i], blobInfo.KeyInfo.KeyID; want != got { + t.Errorf("phyKey = %s: Incorrect encryption key: want %s, got %s", phyKey, want, got) + } + } + } + } + + // Verify the current state is correct before calling UpgradeKeys. + check() + + // Call UpgradeKeys before changing the encryption key and verify + // nothing has changed. + if err := autoSeal.UpgradeKeys(ctx); err != nil { + t.Fatalf("UpgradeKeys: want no error, got %v", err) + } + check() + + // Change the encryption key, call UpgradeKeys, then verify the stored + // keys and recovery key has been re-encrypted with the new encryption + // key. + changeKey("primanti") + if err := autoSeal.UpgradeKeys(ctx); err != nil { + t.Fatalf("UpgradeKeys: want no error, got %v", err) + } + check() +} From fb0d0cd89c56937ccffea62edef82e57f9263062 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 3 Oct 2019 12:12:01 -0400 Subject: [PATCH 8/9] Add comment to phy test structure --- vault/seal_autoseal_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/vault/seal_autoseal_test.go b/vault/seal_autoseal_test.go index 0184161be178..5352eec3eaa6 100644 --- a/vault/seal_autoseal_test.go +++ b/vault/seal_autoseal_test.go @@ -11,11 +11,18 @@ import ( "github.com/hashicorp/vault/vault/seal" ) +// phy implements physical.Backend. It maps keys to a slice of entries. +// Each call to Put appends the entry to the slice of entries for that +// key. No deduplication is done. This allows the test for UpgradeKeys to +// verify entries are only being updated when the underlying encryption key +// has been updated. type phy struct { t *testing.T entries map[string][]*physical.Entry } +var _ physical.Backend = (*phy)(nil) + func newTestBackend(t *testing.T) *phy { return &phy{ t: t, From 329c14af93d3948e3b86f9660cd03419a4cdc3d6 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Thu, 3 Oct 2019 13:29:37 -0400 Subject: [PATCH 9/9] Continue postUnseal if seal UpgradeKeys returns error --- vault/core.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/core.go b/vault/core.go index 30173a94fc94..17f8c9dc8154 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1700,7 +1700,7 @@ func (c *Core) postUnseal(ctx context.Context, ctxCancelFunc context.CancelFunc, // continue to be accessible even after prior seal keys are destroyed. if seal, ok := c.seal.(*autoSeal); ok { if err := seal.UpgradeKeys(c.activeContext); err != nil { - return err + c.logger.Warn("post-unseal upgrade seal keys failed", "error", err) } }