From cbec85fc031aa495c7f1041a24119e90cd750e9e Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Thu, 17 Oct 2019 21:01:12 -0400 Subject: [PATCH 1/5] Shamir seals now come in two varieties: legacy and new-style. Legacy Shamir is automatically converted to new-style when a rekey operation is performed. All new Vault initializations using Shamir are new-style. New-style Shamir writes an encrypted master key to storage, just like AutoUnseal. The stored master key is encrypted using the shared key that is split via Shamir's algorithm. Thus when unsealing, we take the key fragments given, combine them into a Key-Encryption-Key, and use that to decrypt the master key on disk. Then the master key is used to read the keyring that decrypts the barrier. --- command/operator_generate_root_test.go | 2 +- command/operator_init_test.go | 2 +- command/server.go | 4 +- command/server_util.go | 2 +- helper/testhelpers/testhelpers.go | 68 ++++-- http/sys_init.go | 37 ---- vault/barrier.go | 12 +- vault/barrier_aes_gcm.go | 29 ++- vault/barrier_aes_gcm_test.go | 24 +-- vault/barrier_test.go | 12 +- vault/core.go | 69 ++++-- .../external_tests/api/sys_rekey_ext_test.go | 129 +++++++----- vault/external_tests/raft/raft_test.go | 8 +- vault/generate_root.go | 45 ++-- vault/ha.go | 45 ++-- vault/init.go | 94 +++++++-- vault/init_test.go | 3 +- vault/rekey.go | 162 +++++++++------ vault/rekey_test.go | 76 +++++-- vault/seal.go | 196 +++++++++++++----- vault/seal/seal.go | 8 +- vault/seal/shamir/shamir.go | 29 +-- vault/seal_access.go | 22 +- vault/seal_autoseal.go | 72 +------ vault/seal_test.go | 15 +- vault/seal_testing.go | 63 ++---- vault/seal_testing_util.go | 36 +++- vault/testing.go | 27 ++- 28 files changed, 781 insertions(+), 510 deletions(-) diff --git a/command/operator_generate_root_test.go b/command/operator_generate_root_test.go index 4dc5c77630e7..e63a80781558 100644 --- a/command/operator_generate_root_test.go +++ b/command/operator_generate_root_test.go @@ -325,7 +325,7 @@ func TestOperatorGenerateRootCommand_Run(t *testing.T) { keys[len(keys)-1], // the last unseal key }) if exp := 0; code != exp { - t.Errorf("expected %d to be %d", code, exp) + t.Fatalf("expected %d to be %d, out=%q, err=%q", code, exp, ui.OutputWriter, ui.ErrorWriter) } reToken := regexp.MustCompile(`Encoded Token\s+(.+)`) diff --git a/command/operator_init_test.go b/command/operator_init_test.go index dea22d534eb1..99937808e7da 100644 --- a/command/operator_init_test.go +++ b/command/operator_init_test.go @@ -303,7 +303,7 @@ func TestOperatorInitCommand_Run(t *testing.T) { "-root-token-pgp-key", pubFiles[0], }) if exp := 0; code != exp { - t.Errorf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) + t.Fatalf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) } re := regexp.MustCompile(`Unseal Key \d+: (.+)`) diff --git a/command/server.go b/command/server.go index 928526b49add..aad9ef02ef84 100644 --- a/command/server.go +++ b/command/server.go @@ -1822,7 +1822,7 @@ func (c *ServerCommand) enableDev(core *vault.Core, coreConfig *vault.CoreConfig } } - if core.SealAccess().StoredKeysSupported() { + if core.SealAccess().StoredKeysSupported() != vault.StoredKeysNotSupported { barrierConfig.StoredShares = 1 } @@ -1836,7 +1836,7 @@ func (c *ServerCommand) enableDev(core *vault.Core, coreConfig *vault.CoreConfig } // Handle unseal with stored keys - if core.SealAccess().StoredKeysSupported() { + if core.SealAccess().StoredKeysSupported() == vault.StoredKeysSupportedGeneric { err := core.UnsealWithStoredKeys(ctx) if err != nil { return nil, err diff --git a/command/server_util.go b/command/server_util.go index 87e1e22b85d7..ba0d746224cd 100644 --- a/command/server_util.go +++ b/command/server_util.go @@ -87,7 +87,7 @@ func adjustCoreForSealMigration(logger log.Logger, core *vault.Core, barrierSeal return errors.New("Migrating from autoseal to Shamir seal is not currently supported on Vault Enterprise") } - // If we're not cominng from Shamir we expect the previous seal to be + // If we're not coming from Shamir we expect the previous seal to be // in the config and disabled. existSeal = unwrapSeal newSeal = barrierSeal diff --git a/helper/testhelpers/testhelpers.go b/helper/testhelpers/testhelpers.go index 945671d13acb..d6cf76c60932 100644 --- a/helper/testhelpers/testhelpers.go +++ b/helper/testhelpers/testhelpers.go @@ -131,18 +131,32 @@ func EnsureCoreSealed(t testing.T, core *vault.TestClusterCore) { func EnsureCoresUnsealed(t testing.T, c *vault.TestCluster) { t.Helper() - for _, core := range c.Cores { - EnsureCoreUnsealed(t, c, core) + for i, core := range c.Cores { + err := AttemptUnsealCore(c, core) + if err != nil { + t.Fatalf("failed to unseal core %d: %v", i, err) + } + } +} + +func AttemptUnsealCores(c *vault.TestCluster) error { + for i, core := range c.Cores { + err := AttemptUnsealCore(c, core) + if err != nil { + return fmt.Errorf("failed to unseal core %d: %v", i, err) + } } + return nil } -func EnsureCoreUnsealed(t testing.T, c *vault.TestCluster, core *vault.TestClusterCore) { + +func AttemptUnsealCore(c *vault.TestCluster, core *vault.TestClusterCore) error { if !core.Sealed() { - return + return nil } core.SealAccess().ClearCaches(context.Background()) if err := core.UnsealWithStoredKeys(context.Background()); err != nil { - t.Fatal(err) + return err } client := core.Client @@ -153,20 +167,22 @@ func EnsureCoreUnsealed(t testing.T, c *vault.TestCluster, core *vault.TestClust // Sometimes when we get here it's already unsealed on its own // and then this fails for DR secondaries so check again if core.Sealed() { - t.Fatal(err) + return err + } else { + return nil } - break } if statusResp == nil { - t.Fatal("nil status response during unseal") + return fmt.Errorf("nil status response during unseal") } if !statusResp.Sealed { break } } if core.Sealed() { - t.Fatal("core is still sealed") + return fmt.Errorf("core is still sealed") } + return nil } func EnsureStableActiveNode(t testing.T, cluster *vault.TestCluster) { @@ -270,10 +286,16 @@ func WaitForActiveNode(t testing.T, cluster *vault.TestCluster) *vault.TestClust return nil } -func RekeyCluster(t testing.T, cluster *vault.TestCluster) { +func RekeyCluster(t testing.T, cluster *vault.TestCluster, recovery bool) [][]byte { + t.Helper() + cluster.Logger.Info("rekeying cluster", "recovery", recovery) client := cluster.Cores[0].Client - init, err := client.Sys().RekeyInit(&api.RekeyInitRequest{ + initFunc := client.Sys().RekeyInit + if recovery { + initFunc = client.Sys().RekeyRecoveryKeyInit + } + init, err := initFunc(&api.RekeyInitRequest{ SecretShares: 5, SecretThreshold: 3, }) @@ -282,8 +304,17 @@ func RekeyCluster(t testing.T, cluster *vault.TestCluster) { } var statusResp *api.RekeyUpdateResponse - for j := 0; j < len(cluster.BarrierKeys); j++ { - statusResp, err = client.Sys().RekeyUpdate(base64.StdEncoding.EncodeToString(cluster.BarrierKeys[j]), init.Nonce) + var keys = cluster.BarrierKeys + if cluster.Cores[0].Core.SealAccess().RecoveryKeySupported() { + keys = cluster.RecoveryKeys + } + + updateFunc := client.Sys().RekeyUpdate + if recovery { + updateFunc = client.Sys().RekeyRecoveryKeyUpdate + } + for j := 0; j < len(keys); j++ { + statusResp, err = updateFunc(base64.StdEncoding.EncodeToString(keys[j]), init.Nonce) if err != nil { t.Fatal(err) } @@ -294,20 +325,23 @@ func RekeyCluster(t testing.T, cluster *vault.TestCluster) { break } } + cluster.Logger.Info("cluster rekeyed", "recovery", recovery) + if cluster.Cores[0].Core.SealAccess().RecoveryKeySupported() && !recovery { + return nil + } if len(statusResp.KeysB64) != 5 { t.Fatal("wrong number of keys") } - newBarrierKeys := make([][]byte, 5) + newKeys := make([][]byte, 5) for i, key := range statusResp.KeysB64 { - newBarrierKeys[i], err = base64.StdEncoding.DecodeString(key) + newKeys[i], err = base64.StdEncoding.DecodeString(key) if err != nil { t.Fatal(err) } } - - cluster.BarrierKeys = newBarrierKeys + return newKeys } type TestRaftServerAddressProvider struct { diff --git a/http/sys_init.go b/http/sys_init.go index 0e43d4248ac1..ca77ada91101 100644 --- a/http/sys_init.go +++ b/http/sys_init.go @@ -4,7 +4,6 @@ import ( "context" "encoding/base64" "encoding/hex" - "fmt" "net/http" "github.com/hashicorp/vault/vault" @@ -59,42 +58,6 @@ func handleSysInitPut(core *vault.Core, w http.ResponseWriter, r *http.Request) PGPKeys: req.RecoveryPGPKeys, } - // N.B. Although the core is capable of handling situations where some keys - // are stored and some aren't, in practice, replication + HSMs makes this - // extremely hard to reason about, to the point that it will probably never - // be supported. The reason is that each HSM needs to encode the master key - // separately, which means the shares must be generated independently, - // which means both that the shares will be different *AND* there would - // need to be a way to actually allow fetching of the generated keys by - // operators. - if core.SealAccess().StoredKeysSupported() { - if len(barrierConfig.PGPKeys) > 0 { - respondError(w, http.StatusBadRequest, fmt.Errorf("PGP keys not supported when storing shares")) - return - } - barrierConfig.SecretShares = 1 - barrierConfig.SecretThreshold = 1 - barrierConfig.StoredShares = 1 - core.Logger().Warn("stored keys supported on init, forcing shares/threshold to 1") - } else { - if barrierConfig.StoredShares > 0 { - respondError(w, http.StatusBadRequest, fmt.Errorf("stored keys are not supported by the current seal type")) - return - } - } - - if len(barrierConfig.PGPKeys) > 0 && len(barrierConfig.PGPKeys) != barrierConfig.SecretShares { - respondError(w, http.StatusBadRequest, fmt.Errorf("incorrect number of PGP keys")) - return - } - - if core.SealAccess().RecoveryKeySupported() { - if len(recoveryConfig.PGPKeys) > 0 && len(recoveryConfig.PGPKeys) != recoveryConfig.SecretShares { - respondError(w, http.StatusBadRequest, fmt.Errorf("incorrect number of PGP keys for recovery")) - return - } - } - initParams := &vault.InitParams{ BarrierConfig: barrierConfig, RecoveryConfig: recoveryConfig, diff --git a/vault/barrier.go b/vault/barrier.go index 862a8ca11537..92491f7635b4 100644 --- a/vault/barrier.go +++ b/vault/barrier.go @@ -55,6 +55,12 @@ const ( // keyring to discover the new master key. The new master key is then // used to reload the keyring itself. masterKeyPath = "core/master" + + // shamirKekPath is used with Shamir in v1.3+ to store a copy of the + // unseal key behind the barrier. As with masterKeyPath this is primarily + // used by standbys to handle rekeys. It also comes into play when restoring + // raft snapshots. + shamirKekPath = "core/shamir-kek" ) // SecurityBarrier is a critical component of Vault. It is used to wrap @@ -69,8 +75,10 @@ type SecurityBarrier interface { Initialized(ctx context.Context) (bool, error) // Initialize works only if the barrier has not been initialized - // and makes use of the given master key. - Initialize(context.Context, []byte, io.Reader) error + // and makes use of the given master key. When sealKey is provided + // it's because we're using a new-style Shamir seal, and masterKey + // is to be stored using sealKey to encrypt it. + Initialize(ctx context.Context, masterKey []byte, sealKey []byte, random io.Reader) error // GenerateKey is used to generate a new key GenerateKey(io.Reader) ([]byte, error) diff --git a/vault/barrier_aes_gcm.go b/vault/barrier_aes_gcm.go index 314ca9187158..fa2fc22a7946 100644 --- a/vault/barrier_aes_gcm.go +++ b/vault/barrier_aes_gcm.go @@ -115,7 +115,7 @@ func (b *AESGCMBarrier) Initialized(ctx context.Context) (bool, error) { // Initialize works only if the barrier has not been initialized // and makes use of the given master key. -func (b *AESGCMBarrier) Initialize(ctx context.Context, key []byte, reader io.Reader) error { +func (b *AESGCMBarrier) Initialize(ctx context.Context, key, sealKey []byte, reader io.Reader) error { // Verify the key size min, max := b.KeyLength() if len(key) < min || len(key) > max { @@ -146,7 +146,28 @@ func (b *AESGCMBarrier) Initialize(ctx context.Context, key []byte, reader io.Re if err != nil { return errwrap.Wrapf("failed to create keyring: {{err}}", err) } - return b.persistKeyring(ctx, keyring) + + err = b.persistKeyring(ctx, keyring) + if err != nil { + return err + } + + if len(sealKey) > 0 { + primary, err := b.aeadFromKey(encrypt) + if err != nil { + return err + } + + err = b.putInternal(ctx, 1, primary, &logical.StorageEntry{ + Key: shamirKekPath, + Value: sealKey, + }) + if err != nil { + return errwrap.Wrapf("failed to store new seal key: {{err}}", err) + } + } + + return nil } // persistKeyring is used to write out the keyring using the @@ -720,6 +741,10 @@ func (b *AESGCMBarrier) Put(ctx context.Context, entry *logical.StorageEntry) er return err } + return b.putInternal(ctx, term, primary, entry) +} + +func (b *AESGCMBarrier) putInternal(ctx context.Context, term uint32, primary cipher.AEAD, entry *logical.StorageEntry) error { value, err := b.encrypt(entry.Key, term, primary, entry.Value) if err != nil { return err diff --git a/vault/barrier_aes_gcm_test.go b/vault/barrier_aes_gcm_test.go index b9dd2bfc3e30..b63f15b53db6 100644 --- a/vault/barrier_aes_gcm_test.go +++ b/vault/barrier_aes_gcm_test.go @@ -31,7 +31,7 @@ func mockBarrier(t testing.TB) (physical.Backend, SecurityBarrier, []byte) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) return inm, b, key } @@ -207,7 +207,7 @@ func TestAESGCMBarrier_Confidential(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) // Put a logical entry @@ -247,7 +247,7 @@ func TestAESGCMBarrier_Integrity(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) // Put a logical entry @@ -286,7 +286,7 @@ func TestAESGCMBarrier_MoveIntegrityV1(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - err = b.Initialize(context.Background(), key, rand.Reader) + err = b.Initialize(context.Background(), key, nil, rand.Reader) if err != nil { t.Fatalf("err: %v", err) } @@ -330,7 +330,7 @@ func TestAESGCMBarrier_MoveIntegrityV2(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - err = b.Initialize(context.Background(), key, rand.Reader) + err = b.Initialize(context.Background(), key, nil, rand.Reader) if err != nil { t.Fatalf("err: %v", err) } @@ -374,7 +374,7 @@ func TestAESGCMBarrier_UpgradeV1toV2(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - err = b.Initialize(context.Background(), key, rand.Reader) + err = b.Initialize(context.Background(), key, nil, rand.Reader) if err != nil { t.Fatalf("err: %v", err) } @@ -427,7 +427,7 @@ func TestEncrypt_Unique(t *testing.T) { } key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) if b.keyring == nil { @@ -466,19 +466,19 @@ func TestInitialize_KeyLength(t *testing.T) { middle := []byte("ThisIsASecretKeyAndMore") short := []byte("Key") - err = b.Initialize(context.Background(), long, rand.Reader) + err = b.Initialize(context.Background(), long, nil, rand.Reader) if err == nil { t.Fatalf("key length protection failed") } - err = b.Initialize(context.Background(), middle, rand.Reader) + err = b.Initialize(context.Background(), middle, nil, rand.Reader) if err == nil { t.Fatalf("key length protection failed") } - err = b.Initialize(context.Background(), short, rand.Reader) + err = b.Initialize(context.Background(), short, nil, rand.Reader) if err == nil { t.Fatalf("key length protection failed") @@ -500,7 +500,7 @@ func TestEncrypt_BarrierEncryptor(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) cipher, err := b.Encrypt(context.Background(), "foo", []byte("quick brown fox")) @@ -530,7 +530,7 @@ func TestAESGCMBarrier_ReloadKeyring(t *testing.T) { // Initialize and unseal key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) b.Unseal(context.Background(), key) keyringRaw, err := inm.Get(context.Background(), keyringPath) diff --git a/vault/barrier_test.go b/vault/barrier_test.go index 8a2f2289aece..5468fce3eefe 100644 --- a/vault/barrier_test.go +++ b/vault/barrier_test.go @@ -70,12 +70,12 @@ func testBarrier(t *testing.T, b SecurityBarrier) { } // Initialize the vault - if err := b.Initialize(context.Background(), key, rand.Reader); err != nil { + if err := b.Initialize(context.Background(), key, nil, rand.Reader); err != nil { t.Fatalf("err: %v", err) } // Double Initialize should fail - if err := b.Initialize(context.Background(), key, rand.Reader); err != ErrBarrierAlreadyInit { + if err := b.Initialize(context.Background(), key, nil, rand.Reader); err != ErrBarrierAlreadyInit { t.Fatalf("err: %v", err) } @@ -247,7 +247,7 @@ func testBarrier(t *testing.T, b SecurityBarrier) { func testBarrier_Rotate(t *testing.T, b SecurityBarrier) { // Initialize the barrier key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) err := b.Unseal(context.Background(), key) if err != nil { t.Fatalf("err: %v", err) @@ -353,7 +353,7 @@ func testBarrier_Rotate(t *testing.T, b SecurityBarrier) { func testBarrier_Rekey(t *testing.T, b SecurityBarrier) { // Initialize the barrier key, _ := b.GenerateKey(rand.Reader) - b.Initialize(context.Background(), key, rand.Reader) + b.Initialize(context.Background(), key, nil, rand.Reader) err := b.Unseal(context.Background(), key) if err != nil { t.Fatalf("err: %v", err) @@ -433,7 +433,7 @@ func testBarrier_Rekey(t *testing.T, b SecurityBarrier) { func testBarrier_Upgrade(t *testing.T, b1, b2 SecurityBarrier) { // Initialize the barrier key, _ := b1.GenerateKey(rand.Reader) - b1.Initialize(context.Background(), key, rand.Reader) + b1.Initialize(context.Background(), key, nil, rand.Reader) err := b1.Unseal(context.Background(), key) if err != nil { t.Fatalf("err: %v", err) @@ -504,7 +504,7 @@ func testBarrier_Upgrade(t *testing.T, b1, b2 SecurityBarrier) { func testBarrier_Upgrade_Rekey(t *testing.T, b1, b2 SecurityBarrier) { // Initialize the barrier key, _ := b1.GenerateKey(rand.Reader) - b1.Initialize(context.Background(), key, rand.Reader) + b1.Initialize(context.Background(), key, nil, rand.Reader) err := b1.Unseal(context.Background(), key) if err != nil { t.Fatalf("err: %v", err) diff --git a/vault/core.go b/vault/core.go index c04aebf5f3e2..2899e664e854 100644 --- a/vault/core.go +++ b/vault/core.go @@ -6,7 +6,6 @@ import ( "crypto/rand" "crypto/subtle" "crypto/x509" - "encoding/base64" "errors" "fmt" "io" @@ -935,6 +934,8 @@ func (c *Core) unseal(key []byte, useRecoveryKeys bool) (bool, error) { sealToUse = c.migrationSeal } + // unsealPart returns either a master key (legacy shamir) or an unseal + // key (new-style shamir). masterKey, err := c.unsealPart(ctx, sealToUse, key, useRecoveryKeys) if err != nil { return false, err @@ -942,15 +943,39 @@ func (c *Core) unseal(key []byte, useRecoveryKeys bool) (bool, error) { if masterKey != nil { if c.seal.BarrierType() == seal.Shamir { - _, err := c.seal.GetAccess().(*shamirseal.ShamirSeal).SetConfig(map[string]string{ - "key": base64.StdEncoding.EncodeToString(masterKey), - }) + // If this is a legacy shamir seal this serves no purpose but it + // doesn't hurt. + err = c.seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(masterKey) if err != nil { return false, err } } if !c.isRaftUnseal() { + if c.seal.BarrierType() == seal.Shamir { + cfg, err := c.seal.BarrierConfig(ctx) + if err != nil { + return false, err + } + + // If there is a stored key, retrieve it. + if cfg.StoredShares > 0 { + if err != nil { + return false, err + } + // Here's where we actually test that the provided unseal + // key is valid: can it decrypt the stored master key? + storedKeys, err := c.seal.GetStoredKeys(ctx) + if err != nil { + return false, err + } + if len(storedKeys) == 0 { + return false, fmt.Errorf("shamir seal with stored keys configured but no stored keys found") + } + masterKey = storedKeys[0] + } + } + return c.unsealInternal(ctx, masterKey) } @@ -964,8 +989,9 @@ func (c *Core) unseal(key []byte, useRecoveryKeys bool) (bool, error) { go func() { keyringFound := false + haveMasterKey := c.seal.StoredKeysSupported() != StoredKeysSupportedShamirMaster defer func() { - if keyringFound { + if keyringFound && haveMasterKey { _, err := c.unsealInternal(ctx, masterKey) if err != nil { c.logger.Error("failed to unseal", "error", err) @@ -976,18 +1002,31 @@ func (c *Core) unseal(key []byte, useRecoveryKeys bool) (bool, error) { // Wait until we at least have the keyring before we attempt to // unseal the node. for { - keys, err := c.underlyingPhysical.List(ctx, keyringPrefix) - if err != nil { - c.logger.Error("failed to list physical keys", "error", err) - return + if !keyringFound { + keys, err := c.underlyingPhysical.List(ctx, keyringPrefix) + if err != nil { + c.logger.Error("failed to list physical keys", "error", err) + return + } + if strutil.StrListContains(keys, "keyring") { + keyringFound = true + } } - if strutil.StrListContains(keys, "keyring") { - keyringFound = true - return + if !haveMasterKey { + keys, err := c.seal.GetStoredKeys(ctx) + if err != nil { + c.logger.Error("failed to read master key", "error", err) + return + } + if len(keys) > 0 { + haveMasterKey = true + masterKey = keys[0] + } } - select { - case <-time.After(1 * time.Second): + if keyringFound && haveMasterKey { + return } + time.Sleep(1 * time.Second) } }() @@ -1084,7 +1123,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover // keys setup, nor 2) seals that support recovery keys but not stored keys. // If insufficient shares are provided, shamir.Combine will error, and if // no stored keys are found it will return masterKey as nil. - if seal.StoredKeysSupported() { + if seal.StoredKeysSupported() == StoredKeysSupportedGeneric { masterKeyShares, err := seal.GetStoredKeys(ctx) if err != nil { return nil, errwrap.Wrapf("unable to retrieve stored keys: {{err}}", err) diff --git a/vault/external_tests/api/sys_rekey_ext_test.go b/vault/external_tests/api/sys_rekey_ext_test.go index b3d7ca182983..c84d802dc3f8 100644 --- a/vault/external_tests/api/sys_rekey_ext_test.go +++ b/vault/external_tests/api/sys_rekey_ext_test.go @@ -2,25 +2,66 @@ package api import ( "encoding/base64" - "strings" + "fmt" "testing" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/testhelpers" vaulthttp "github.com/hashicorp/vault/http" - "github.com/hashicorp/vault/shamir" + "github.com/hashicorp/vault/sdk/helper/logging" + "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/hashicorp/vault/vault" ) func TestSysRekey_Verification(t *testing.T) { - testSysRekey_Verification(t, false) - testSysRekey_Verification(t, true) + testcases := []struct { + recovery bool + legacyShamir bool + }{ + {recovery: true, legacyShamir: false}, + {recovery: false, legacyShamir: false}, + {recovery: false, legacyShamir: true}, + } + + for _, tc := range testcases { + recovery, legacy := tc.recovery, tc.legacyShamir + t.Run(fmt.Sprintf("recovery=%v,legacyShamir=%v", recovery, legacy), func(t *testing.T) { + t.Parallel() + testSysRekey_Verification(t, recovery, legacy) + }) + } } -func testSysRekey_Verification(t *testing.T, recovery bool) { - cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ +func testSysRekey_Verification(t *testing.T, recovery bool, legacyShamir bool) { + opts := &vault.TestClusterOptions{ HandlerFunc: vaulthttp.Handler, - }) + } + switch { + case recovery: + if legacyShamir { + panic("invalid case") + } + opts.SealFunc = func() vault.Seal { + return vault.NewTestSeal(t, &vault.TestSealOpts{ + StoredKeys: vault.StoredKeysSupportedGeneric, + }) + } + case legacyShamir: + opts.SealFunc = func() vault.Seal { + return vault.NewTestSeal(t, &vault.TestSealOpts{ + StoredKeys: vault.StoredKeysNotSupported, + }) + } + } + inm, err := inmem.NewInmemHA(nil, logging.NewVaultLogger(hclog.Debug)) + if err != nil { + t.Fatal(err) + } + conf := vault.CoreConfig{ + Physical: inm, + } + cluster := vault.NewTestCluster(t, &conf, opts) cluster.Start() defer cluster.Cleanup() @@ -41,48 +82,6 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { verificationCancelFunc = client.Sys().RekeyRecoveryKeyVerificationCancel } - sealAccess := cluster.Cores[0].Core.SealAccess() - sealTestingParams := &vault.SealAccessTestingParams{} - - // This first block verifies that if we are using recovery keys to force a - // rekey of a stored-shares barrier that verification is not allowed since - // the keys aren't returned - if !recovery { - sealTestingParams.PretendToAllowRecoveryKeys = true - sealTestingParams.PretendToAllowStoredShares = true - if err := sealAccess.SetTestingParams(sealTestingParams); err != nil { - t.Fatal(err) - } - - _, err := initFunc(&api.RekeyInitRequest{ - StoredShares: 1, - RequireVerification: true, - }) - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "requiring verification not supported") { - t.Fatalf("unexpected error: %v", err) - } - // Now we set things back and start a normal rekey with the verification process - sealTestingParams.PretendToAllowRecoveryKeys = false - sealTestingParams.PretendToAllowStoredShares = false - if err := sealAccess.SetTestingParams(sealTestingParams); err != nil { - t.Fatal(err) - } - } else { - cluster.RecoveryKeys = cluster.BarrierKeys - sealTestingParams.PretendToAllowRecoveryKeys = true - recoveryKey, err := shamir.Combine(cluster.BarrierKeys) - if err != nil { - t.Fatal(err) - } - sealTestingParams.PretendRecoveryKey = recoveryKey - if err := sealAccess.SetTestingParams(sealTestingParams); err != nil { - t.Fatal(err) - } - } - var verificationNonce string var newKeys []string doRekeyInitialSteps := func() { @@ -101,9 +100,13 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { t.Fatal("expected verification required") } + keys := cluster.BarrierKeys + if recovery { + keys = cluster.RecoveryKeys + } var resp *api.RekeyUpdateResponse for i := 0; i < 3; i++ { - resp, err = updateFunc(base64.StdEncoding.EncodeToString(cluster.BarrierKeys[i]), status.Nonce) + resp, err = updateFunc(base64.StdEncoding.EncodeToString(keys[i]), status.Nonce) if err != nil { t.Fatal(err) } @@ -124,7 +127,7 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { doRekeyInitialSteps() // We are still going, so should not be able to init again - _, err := initFunc(&api.RekeyInitRequest{ + _, err = initFunc(&api.RekeyInitRequest{ SecretShares: 5, SecretThreshold: 3, RequireVerification: true, @@ -136,7 +139,12 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { // Sealing should clear state, so after this we should be able to perform // the above again cluster.EnsureCoresSealed(t) - cluster.UnsealCores(t) + if recovery { + cluster.UnsealWithStoredKeys(t) + } else { + cluster.UnsealCores(t) + } + vault.TestWaitActive(t, cluster.Cores[0].Core) doRekeyInitialSteps() doStartVerify := func() { @@ -211,6 +219,7 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { // still be the old keys (which are still currently set) cluster.EnsureCoresSealed(t) cluster.UnsealCores(t) + vault.TestWaitActive(t, cluster.Cores[0].Core) // Should be able to init again and get back to where we were doRekeyInitialSteps() @@ -237,6 +246,18 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { // Seal and unseal -- it should fail to unseal because the key has now been // rotated cluster.EnsureCoresSealed(t) + + // Simulate restarting Vault rather than just a seal/unseal, because + // the standbys may not have had time to learn about the new key before + // we sealed them. We could sleep, but that's unreliable. + oldKeys := cluster.BarrierKeys + opts.SkipInit = true + opts.SealFunc = nil // post rekey we should use the barrier config on disk + cluster = vault.NewTestCluster(t, &conf, opts) + cluster.BarrierKeys = oldKeys + cluster.Start() + defer cluster.Cleanup() + if err := cluster.UnsealCoresWithError(); err == nil { t.Fatal("expected error") } @@ -252,7 +273,7 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { } cluster.BarrierKeys = newKeyBytes if err := cluster.UnsealCoresWithError(); err != nil { - t.Fatal("expected error") + t.Fatal(err) } } else { // The old keys should no longer work @@ -261,7 +282,7 @@ func testSysRekey_Verification(t *testing.T, recovery bool) { t.Fatal("expected error") } - // Put tne new keys in place and run again + // Put the new keys in place and run again cluster.RecoveryKeys = nil for _, key := range newKeys { dec, err := base64.StdEncoding.DecodeString(key) diff --git a/vault/external_tests/raft/raft_test.go b/vault/external_tests/raft/raft_test.go index de3713d7b52f..2d47dbd7c09a 100644 --- a/vault/external_tests/raft/raft_test.go +++ b/vault/external_tests/raft/raft_test.go @@ -373,7 +373,7 @@ func TestRaft_SnapshotAPI_RekeyRotate_Backward(t *testing.T) { if tCaseLocal.Rekey { // Rekey - testhelpers.RekeyCluster(t, cluster) + cluster.BarrierKeys = testhelpers.RekeyCluster(t, cluster, false) } if tCaseLocal.Rekey { @@ -520,7 +520,7 @@ func TestRaft_SnapshotAPI_RekeyRotate_Forward(t *testing.T) { if tCaseLocal.Rekey { // Rekey - testhelpers.RekeyCluster(t, cluster) + cluster.BarrierKeys = testhelpers.RekeyCluster(t, cluster, false) } if tCaseLocal.Rotate { // Set the key clean up to 0 so it's cleaned immediately. This @@ -588,8 +588,8 @@ func TestRaft_SnapshotAPI_RekeyRotate_Forward(t *testing.T) { } // Parse Response apiResp := api.Response{Response: resp} - if !strings.Contains(apiResp.Error().Error(), "could not verify hash file, possibly the snapshot is using a different set of unseal keys") { - t.Fatal(apiResp.Error()) + if apiResp.Error() == nil || !strings.Contains(apiResp.Error().Error(), "could not verify hash file, possibly the snapshot is using a different set of unseal keys") { + t.Fatalf("expected error verifying hash file, got %v", apiResp.Error()) } } diff --git a/vault/generate_root.go b/vault/generate_root.go index 9708877709d0..56033fdb4df3 100644 --- a/vault/generate_root.go +++ b/vault/generate_root.go @@ -6,12 +6,14 @@ import ( "encoding/base64" "errors" "fmt" + "github.com/hashicorp/errwrap" "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/pgpkeys" "github.com/hashicorp/vault/helper/xor" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/shamir" + shamirseal "github.com/hashicorp/vault/vault/seal/shamir" ) const coreDROperationTokenPath = "core/dr-operation-token" @@ -304,44 +306,47 @@ func (c *Core) GenerateRootUpdate(ctx context.Context, key []byte, nonce string, // If we are in recovery mode, then retrieve // the stored keys and unseal the barrier if c.recoveryMode { - if !c.seal.StoredKeysSupported() { - c.logger.Error("root generation aborted, recovery key verified but stored keys unsupported") - return nil, errors.New("recovery key verified but stored keys unsupported") - } - masterKeyShares, err := c.seal.GetStoredKeys(ctx) + storedKeys, err := c.seal.GetStoredKeys(ctx) if err != nil { return nil, errwrap.Wrapf("unable to retrieve stored keys in recovery mode: {{err}}", err) } - switch len(masterKeyShares) { - case 0: - return nil, errors.New("seal returned no master key shares in recovery mode") - case 1: - combinedKey = masterKeyShares[0] - default: - combinedKey, err = shamir.Combine(masterKeyShares) - if err != nil { - return nil, errwrap.Wrapf("failed to compute master key in recovery mode: {{err}}", err) - } - } - // Use the retrieved master key to unseal the barrier - if err := c.barrier.Unseal(ctx, combinedKey); err != nil { + if err := c.barrier.Unseal(ctx, storedKeys[0]); err != nil { c.logger.Error("root generation aborted, recovery operation token verification failed", "error", err) return nil, err } } default: + masterKey := combinedKey + if c.seal.StoredKeysSupported() == StoredKeysSupportedShamirMaster { + testseal := NewDefaultSeal(shamirseal.NewSeal(c.logger.Named("testseal"))) + testseal.SetCore(c) + cfg, err := c.seal.BarrierConfig(ctx) + if err != nil { + return nil, errwrap.Wrapf("failed to setup test barrier config: {{err}}", err) + } + testseal.SetCachedBarrierConfig(cfg) + err = testseal.GetAccess().(*shamirseal.ShamirSeal).SetKey(combinedKey) + if err != nil { + return nil, errwrap.Wrapf("failed to setup unseal key: {{err}}", err) + } + stored, err := testseal.GetStoredKeys(ctx) + if err != nil { + return nil, errwrap.Wrapf("failed to read master key: {{err}}", err) + } + masterKey = stored[0] + } switch { case c.recoveryMode: // If we are in recovery mode, being able to unseal // the barrier is how we establish authentication - if err := c.barrier.Unseal(ctx, combinedKey); err != nil { + if err := c.barrier.Unseal(ctx, masterKey); err != nil { c.logger.Error("root generation aborted, recovery operation token verification failed", "error", err) return nil, err } default: - if err := c.barrier.VerifyMaster(combinedKey); err != nil { + if err := c.barrier.VerifyMaster(masterKey); err != nil { c.logger.Error("root generation aborted, master key verification failed", "error", err) return nil, err } diff --git a/vault/ha.go b/vault/ha.go index 6e31b78a52f8..ad6b28ecc599 100644 --- a/vault/ha.go +++ b/vault/ha.go @@ -4,17 +4,17 @@ import ( "context" "crypto/ecdsa" "crypto/x509" - "encoding/base64" "errors" "fmt" + "github.com/hashicorp/vault/vault/seal/shamir" "strings" "sync/atomic" "time" - metrics "github.com/armon/go-metrics" + "github.com/armon/go-metrics" "github.com/hashicorp/errwrap" - multierror "github.com/hashicorp/go-multierror" - uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/physical/raft" "github.com/hashicorp/vault/sdk/helper/certutil" @@ -22,8 +22,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/physical" - "github.com/hashicorp/vault/vault/seal" - shamirseal "github.com/hashicorp/vault/vault/seal/shamir" "github.com/oklog/run" ) @@ -772,22 +770,35 @@ func (c *Core) reloadMasterKey(ctx context.Context) error { if err := c.barrier.ReloadMasterKey(ctx); err != nil { return errwrap.Wrapf("error reloading master key: {{err}}", err) } + return nil +} - if c.seal.BarrierType() == seal.Shamir { - keyring, err := c.barrier.Keyring() +func (c *Core) reloadShamirKey(ctx context.Context) error { + _ = c.seal.SetBarrierConfig(ctx, nil) + if cfg, _ := c.seal.BarrierConfig(ctx); cfg == nil { + return nil + } + var shamirKey []byte + switch c.seal.StoredKeysSupported() { + case StoredKeysSupportedGeneric: + return nil + case StoredKeysSupportedShamirMaster: + entry, err := c.barrier.Get(ctx, shamirKekPath) if err != nil { - return errwrap.Wrapf("failed to update seal access: {{err}}", err) + return err } - - _, err = c.seal.GetAccess().(*shamirseal.ShamirSeal).SetConfig(map[string]string{ - "key": base64.StdEncoding.EncodeToString(keyring.MasterKey()), - }) + if entry == nil { + return nil + } + shamirKey = entry.Value + case StoredKeysNotSupported: + keyring, err := c.barrier.Keyring() if err != nil { return errwrap.Wrapf("failed to update seal access: {{err}}", err) } + shamirKey = keyring.masterKey } - - return nil + return c.seal.GetAccess().(*shamir.ShamirSeal).SetKey(shamirKey) } func (c *Core) performKeyUpgrades(ctx context.Context) error { @@ -803,6 +814,10 @@ func (c *Core) performKeyUpgrades(ctx context.Context) error { return errwrap.Wrapf("error reloading keyring: {{err}}", err) } + if err := c.reloadShamirKey(ctx); err != nil { + return errwrap.Wrapf("error reloading shamir kek key: {{err}}", err) + } + if err := c.scheduleUpgradeCleanup(ctx); err != nil { return errwrap.Wrapf("error scheduling upgrade cleanup: {{err}}", err) } diff --git a/vault/init.go b/vault/init.go index 7800b433f3d3..fe0207e89b3a 100644 --- a/vault/init.go +++ b/vault/init.go @@ -15,6 +15,8 @@ import ( "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/pgpkeys" "github.com/hashicorp/vault/shamir" + "github.com/hashicorp/vault/vault/seal" + shamirseal "github.com/hashicorp/vault/vault/seal/shamir" ) // InitParams keeps the init function from being littered with too many @@ -23,6 +25,9 @@ type InitParams struct { BarrierConfig *SealConfig RecoveryConfig *SealConfig RootTokenPGPKey string + // LegacyShamirSeal should only be used in test code, we don't want to + // give the user a way to create legacy shamir seals. + LegacyShamirSeal bool } // InitResult is used to provide the key parts back after @@ -132,6 +137,41 @@ func (c *Core) Initialize(ctx context.Context, initParams *InitParams) (*InitRes barrierConfig := initParams.BarrierConfig recoveryConfig := initParams.RecoveryConfig + // N.B. Although the core is capable of handling situations where some keys + // are stored and some aren't, in practice, replication + HSMs makes this + // extremely hard to reason about, to the point that it will probably never + // be supported. The reason is that each HSM needs to encode the master key + // separately, which means the shares must be generated independently, + // which means both that the shares will be different *AND* there would + // need to be a way to actually allow fetching of the generated keys by + // operators. + if c.SealAccess().StoredKeysSupported() == StoredKeysSupportedGeneric { + if len(barrierConfig.PGPKeys) > 0 { + return nil, fmt.Errorf("PGP keys not supported when storing shares") + } + barrierConfig.SecretShares = 1 + barrierConfig.SecretThreshold = 1 + if barrierConfig.StoredShares != 1 { + c.Logger().Warn("stored keys supported on init, forcing shares/threshold to 1") + } + } + + if initParams.LegacyShamirSeal { + barrierConfig.StoredShares = 0 + } else { + barrierConfig.StoredShares = 1 + } + + if len(barrierConfig.PGPKeys) > 0 && len(barrierConfig.PGPKeys) != barrierConfig.SecretShares { + return nil, fmt.Errorf("incorrect number of PGP keys") + } + + if c.SealAccess().RecoveryKeySupported() { + if len(recoveryConfig.PGPKeys) > 0 && len(recoveryConfig.PGPKeys) != recoveryConfig.SecretShares { + return nil, fmt.Errorf("incorrect number of PGP keys for recovery") + } + } + if c.seal.RecoveryKeySupported() { if recoveryConfig == nil { return nil, fmt.Errorf("recovery configuration must be supplied") @@ -201,24 +241,34 @@ func (c *Core) Initialize(ctx context.Context, initParams *InitParams) (*InitRes return nil, errwrap.Wrapf("error initializing seal: {{err}}", err) } - barrierKey, barrierUnsealKeys, err := c.generateShares(barrierConfig) + initPTCleanup := initPTFunc(c) + if initPTCleanup != nil { + defer initPTCleanup() + } + + barrierKey, barrierKeyShares, err := c.generateShares(barrierConfig) if err != nil { c.logger.Error("error generating shares", "error", err) return nil, err } - initPTCleanup := initPTFunc(c) - if initPTCleanup != nil { - defer initPTCleanup() + var sealKey []byte + var sealKeyShares [][]byte + if barrierConfig.StoredShares == 1 && c.seal.BarrierType() == seal.Shamir { + sealKey, sealKeyShares, err = c.generateShares(barrierConfig) + if err != nil { + c.logger.Error("error generating shares", "error", err) + return nil, err + } } // Initialize the barrier - if err := c.barrier.Initialize(ctx, barrierKey, c.secureRandomReader); err != nil { + if err := c.barrier.Initialize(ctx, barrierKey, sealKey, c.secureRandomReader); err != nil { c.logger.Error("failed to initialize barrier", "error", err) return nil, errwrap.Wrapf("failed to initialize barrier: {{err}}", err) } if c.logger.IsInfo() { - c.logger.Info("security barrier initialized", "shares", barrierConfig.SecretShares, "threshold", barrierConfig.SecretThreshold) + c.logger.Info("security barrier initialized", "stored", barrierConfig.StoredShares, "shares", barrierConfig.SecretShares, "threshold", barrierConfig.SecretThreshold) } // Unseal the barrier @@ -243,22 +293,34 @@ func (c *Core) Initialize(ctx context.Context, initParams *InitParams) (*InitRes return nil, errwrap.Wrapf("barrier configuration saving failed: {{err}}", err) } + results := &InitResult{ + SecretShares: [][]byte{}, + } + // If we are storing shares, pop them out of the returned results and push // them through the seal - if barrierConfig.StoredShares > 0 { - var keysToStore [][]byte - for i := 0; i < barrierConfig.StoredShares; i++ { - keysToStore = append(keysToStore, barrierUnsealKeys[0]) - barrierUnsealKeys = barrierUnsealKeys[1:] + switch c.seal.StoredKeysSupported() { + case StoredKeysSupportedShamirMaster: + keysToStore := [][]byte{barrierKey} + if err := c.seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(sealKey); err != nil { + c.logger.Error("failed to set seal key", "error", err) + return nil, errwrap.Wrapf("failed to set seal key: {{err}}", err) } if err := c.seal.SetStoredKeys(ctx, keysToStore); err != nil { c.logger.Error("failed to store keys", "error", err) return nil, errwrap.Wrapf("failed to store keys: {{err}}", err) } - } - - results := &InitResult{ - SecretShares: barrierUnsealKeys, + results.SecretShares = sealKeyShares + case StoredKeysSupportedGeneric: + keysToStore := [][]byte{barrierKey} + if err := c.seal.SetStoredKeys(ctx, keysToStore); err != nil { + c.logger.Error("failed to store keys", "error", err) + return nil, errwrap.Wrapf("failed to store keys: {{err}}", err) + } + default: + // We don't support initializing an old-style Shamir seal anymore, so + // this case is only reachable by tests. + results.SecretShares = barrierKeyShares } // Perform initial setup @@ -345,7 +407,7 @@ func (c *Core) UnsealWithStoredKeys(ctx context.Context) error { c.unsealWithStoredKeysLock.Lock() defer c.unsealWithStoredKeysLock.Unlock() - if !c.seal.StoredKeysSupported() { + if c.seal.BarrierType() == seal.Shamir { return nil } diff --git a/vault/init_test.go b/vault/init_test.go index 91673993be2a..9135aa81ff44 100644 --- a/vault/init_test.go +++ b/vault/init_test.go @@ -2,6 +2,7 @@ package vault import ( "context" + "github.com/hashicorp/vault/vault/seal" "reflect" "testing" @@ -80,7 +81,7 @@ func testCore_Init_Common(t *testing.T, c *Core, conf *CoreConfig, barrierConf, t.Fatalf("err: %v", err) } - if len(res.SecretShares) != (barrierConf.SecretShares - barrierConf.StoredShares) { + if c.seal.BarrierType() == seal.Shamir && len(res.SecretShares) != barrierConf.SecretShares { t.Fatalf("Bad: got\n%#v\nexpected conf matching\n%#v\n", *res, *barrierConf) } if recoveryConf != nil { diff --git a/vault/rekey.go b/vault/rekey.go index 4af057ceabd1..f6505a471241 100644 --- a/vault/rekey.go +++ b/vault/rekey.go @@ -4,14 +4,13 @@ import ( "bytes" "context" "crypto/subtle" - "encoding/base64" "encoding/hex" "encoding/json" "fmt" "net/http" "github.com/hashicorp/errwrap" - uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/go-uuid" "github.com/hashicorp/vault/helper/pgpkeys" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" @@ -169,30 +168,37 @@ func (c *Core) RekeyInit(config *SealConfig, recovery bool) logical.HTTPCodedErr // BarrierRekeyInit is used to initialize the rekey settings for the barrier key func (c *Core) BarrierRekeyInit(config *SealConfig) logical.HTTPCodedError { - if c.seal.StoredKeysSupported() { - c.logger.Warn("stored keys supported, forcing rekey shares/threshold to 1") + switch c.seal.BarrierType() { + case seal.Shamir: + // As of Vault 1.3 all seals use StoredShares==1. The one exception is + // legacy shamir seals, which we can read but not write (by design). + // So if someone does a rekey, regardless of their intention, we're going + // to migrate them to a non-legacy Shamir seal. + if config.StoredShares != 1 { + c.logger.Warn("shamir stored keys supported, forcing rekey shares/threshold to 1") + config.StoredShares = 1 + } + default: + if config.StoredShares != 1 { + c.logger.Warn("stored keys supported, forcing rekey shares/threshold to 1") + config.StoredShares = 1 + } config.SecretShares = 1 config.SecretThreshold = 1 - config.StoredShares = 1 - } - if config.StoredShares > 0 { - if !c.seal.StoredKeysSupported() { - return logical.CodedError(http.StatusBadRequest, "storing keys not supported by barrier seal") - } if len(config.PGPKeys) > 0 { return logical.CodedError(http.StatusBadRequest, "PGP key encryption not supported when using stored keys") } if config.Backup { return logical.CodedError(http.StatusBadRequest, "key backup not supported when using stored keys") } + } - if c.seal.RecoveryKeySupported() { - if config.VerificationRequired { - return logical.CodedError(http.StatusBadRequest, "requiring verification not supported when rekeying the barrier key with recovery keys") - } - c.logger.Debug("using recovery seal configuration to rekey barrier key") + if c.seal.RecoveryKeySupported() { + if config.VerificationRequired { + return logical.CodedError(http.StatusBadRequest, "requiring verification not supported when rekeying the barrier key with recovery keys") } + c.logger.Debug("using recovery seal configuration to rekey barrier key") } // Check if the seal configuration is valid @@ -326,7 +332,7 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) var existingConfig *SealConfig var err error var useRecovery bool // Determines whether recovery key is being used to rekey the master key - if c.seal.StoredKeysSupported() && c.seal.RecoveryKeySupported() { + if c.seal.StoredKeysSupported() == StoredKeysSupportedGeneric && c.seal.RecoveryKeySupported() { existingConfig, err = c.seal.RecoveryConfig(ctx) useRecovery = true } else { @@ -384,20 +390,41 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) } } - if useRecovery { + switch { + case useRecovery: if err := c.seal.VerifyRecoveryKey(ctx, recoveredKey); err != nil { c.logger.Error("rekey recovery key verification failed", "error", err) return nil, logical.CodedError(http.StatusBadRequest, errwrap.Wrapf("recovery key verification failed: {{err}}", err).Error()) } - } else { + case c.seal.BarrierType() == seal.Shamir: + if c.seal.StoredKeysSupported() == StoredKeysSupportedShamirMaster { + testseal := NewDefaultSeal(shamirseal.NewSeal(c.logger.Named("testseal"))) + testseal.SetCore(c) + err = testseal.GetAccess().(*shamirseal.ShamirSeal).SetKey(recoveredKey) + if err != nil { + return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to setup unseal key: {{err}}", err).Error()) + } + cfg, err := c.seal.BarrierConfig(ctx) + if err != nil { + return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to setup test barrier config: {{err}}", err).Error()) + } + testseal.SetCachedBarrierConfig(cfg) + stored, err := testseal.GetStoredKeys(ctx) + if err != nil { + return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to read master key: {{err}}", err).Error()) + } + recoveredKey = stored[0] + } if err := c.barrier.VerifyMaster(recoveredKey); err != nil { c.logger.Error("master key verification failed", "error", err) return nil, logical.CodedError(http.StatusBadRequest, errwrap.Wrapf("master key verification failed: {{err}}", err).Error()) } } - // Generate a new master key - newMasterKey, err := c.barrier.GenerateKey(c.secureRandomReader) + // Generate a new key: for AutoUnseal, this is a new master key; for Shamir, + // this is a new unseal key, and performBarrierRekey will also generate a + // new master key. + newKey, err := c.barrier.GenerateKey(c.secureRandomReader) if err != nil { c.logger.Error("failed to generate master key", "error", err) return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("master key generation failed: {{err}}", err).Error()) @@ -406,27 +433,19 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) results := &RekeyResult{ Backup: c.barrierRekeyConfig.Backup, } - // Set result.SecretShares to the master key if only a single key - // part is used -- no Shamir split required. - if c.barrierRekeyConfig.SecretShares == 1 { - results.SecretShares = append(results.SecretShares, newMasterKey) - } else { - // Split the master key using the Shamir algorithm - shares, err := shamir.Split(newMasterKey, c.barrierRekeyConfig.SecretShares, c.barrierRekeyConfig.SecretThreshold) - if err != nil { - c.logger.Error("failed to generate shares", "error", err) - return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to generate shares: {{err}}", err).Error()) - } - results.SecretShares = shares - } - - // If we are storing any shares, add them to the shares to store and remove - // from the returned keys - var keysToStore [][]byte - if c.seal.StoredKeysSupported() && c.barrierRekeyConfig.StoredShares > 0 { - for i := 0; i < c.barrierRekeyConfig.StoredShares; i++ { - keysToStore = append(keysToStore, results.SecretShares[0]) - results.SecretShares = results.SecretShares[1:] + if c.seal.StoredKeysSupported() != StoredKeysSupportedGeneric { + // Set result.SecretShares to the new key itself if only a single key + // part is used -- no Shamir split required. + if c.barrierRekeyConfig.SecretShares == 1 { + results.SecretShares = append(results.SecretShares, newKey) + } else { + // Split the new key using the Shamir algorithm + shares, err := shamir.Split(newKey, c.barrierRekeyConfig.SecretShares, c.barrierRekeyConfig.SecretThreshold) + if err != nil { + c.logger.Error("failed to generate shares", "error", err) + return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to generate shares: {{err}}", err).Error()) + } + results.SecretShares = shares } } @@ -473,13 +492,6 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) } } - if keysToStore != nil { - if err := c.seal.SetStoredKeys(ctx, keysToStore); err != nil { - c.logger.Error("failed to store keys", "error", err) - return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to store keys: {{err}}", err).Error()) - } - } - // If we are requiring validation, return now; otherwise rekey the barrier if c.barrierRekeyConfig.VerificationRequired { nonce, err := uuid.GenerateUUID() @@ -488,14 +500,14 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to generate verification nonce: {{err}}", err).Error()) } c.barrierRekeyConfig.VerificationNonce = nonce - c.barrierRekeyConfig.VerificationKey = newMasterKey + c.barrierRekeyConfig.VerificationKey = newKey results.VerificationRequired = true results.VerificationNonce = nonce return results, nil } - if err := c.performBarrierRekey(ctx, newMasterKey); err != nil { + if err := c.performBarrierRekey(ctx, newKey); err != nil { return nil, logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to perform barrier rekey: {{err}}", err).Error()) } @@ -503,14 +515,52 @@ func (c *Core) BarrierRekeyUpdate(ctx context.Context, key []byte, nonce string) return results, nil } -func (c *Core) performBarrierRekey(ctx context.Context, newMasterKey []byte) logical.HTTPCodedError { +func (c *Core) performBarrierRekey(ctx context.Context, newSealKey []byte) logical.HTTPCodedError { + legacyUpgrade := c.seal.StoredKeysSupported() == StoredKeysNotSupported + if legacyUpgrade { + // We won't be able to call SetStoredKeys without setting StoredShares=1. + existingConfig, err := c.seal.BarrierConfig(ctx) + if err != nil { + return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to fetch existing config: {{err}}", err).Error()) + } + existingConfig.StoredShares = 1 + c.seal.SetCachedBarrierConfig(existingConfig) + } + + if c.seal.StoredKeysSupported() != StoredKeysSupportedGeneric { + err := c.seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(newSealKey) + if err != nil { + return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to update barrier seal key: {{err}}", err).Error()) + } + } + + newMasterKey, err := c.barrier.GenerateKey(c.secureRandomReader) + if err != nil { + return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to perform rekey: {{err}}", err).Error()) + } + if err := c.seal.SetStoredKeys(ctx, [][]byte{newMasterKey}); err != nil { + c.logger.Error("failed to store keys", "error", err) + return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to store keys: {{err}}", err).Error()) + } + // Rekey the barrier if err := c.barrier.Rekey(ctx, newMasterKey); err != nil { c.logger.Error("failed to rekey barrier", "error", err) return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to rekey barrier: {{err}}", err).Error()) } if c.logger.IsInfo() { - c.logger.Info("security barrier rekeyed", "shares", c.barrierRekeyConfig.SecretShares, "threshold", c.barrierRekeyConfig.SecretThreshold) + c.logger.Info("security barrier rekeyed", "stored", c.barrierRekeyConfig.StoredShares, "shares", c.barrierRekeyConfig.SecretShares, "threshold", c.barrierRekeyConfig.SecretThreshold) + } + + if len(newSealKey) > 0 { + err := c.barrier.Put(ctx, &logical.StorageEntry{ + Key: shamirKekPath, + Value: newSealKey, + }) + if err != nil { + c.logger.Error("failed to store new seal key", "error", err) + return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to store new seal key: {{err}}", err).Error()) + } } c.barrierRekeyConfig.VerificationKey = nil @@ -531,14 +581,6 @@ func (c *Core) performBarrierRekey(ctx context.Context, newMasterKey []byte) log } c.barrierRekeyConfig.RekeyProgress = nil - if c.seal.BarrierType() == seal.Shamir { - _, err := c.seal.GetAccess().(*shamirseal.ShamirSeal).SetConfig(map[string]string{ - "key": base64.StdEncoding.EncodeToString(newMasterKey), - }) - if err != nil { - return logical.CodedError(http.StatusInternalServerError, errwrap.Wrapf("failed to update seal access: {{err}}", err).Error()) - } - } return nil } diff --git a/vault/rekey_test.go b/vault/rekey_test.go index 643b587e3841..fd3b53efd43c 100644 --- a/vault/rekey_test.go +++ b/vault/rekey_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "testing" log "github.com/hashicorp/go-hclog" @@ -14,21 +15,28 @@ import ( ) func TestCore_Rekey_Lifecycle(t *testing.T) { - bc, _ := TestSealDefConfigs() - bc.SecretShares = 1 - bc.SecretThreshold = 1 - bc.StoredShares = 0 + bc := &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + StoredShares: 1, + } c, masterKeys, _, _ := TestCoreUnsealedWithConfigs(t, bc, nil) if len(masterKeys) != 1 { t.Fatalf("expected %d keys, got %d", bc.SecretShares-bc.StoredShares, len(masterKeys)) } - testCore_Rekey_Lifecycle_Common(t, c, masterKeys, false) + testCore_Rekey_Lifecycle_Common(t, c, false) } -func testCore_Rekey_Lifecycle_Common(t *testing.T, c *Core, masterKeys [][]byte, recovery bool) { +func testCore_Rekey_Lifecycle_Common(t *testing.T, c *Core, recovery bool) { + min, _ := c.barrier.KeyLength() // Verify update not allowed - if _, err := c.RekeyUpdate(context.Background(), masterKeys[0], "", recovery); err == nil { - t.Fatalf("no rekey should be in progress") + _, err := c.RekeyUpdate(context.Background(), make([]byte, min), "", recovery) + expected := "no barrier rekey in progress" + if recovery { + expected = "no recovery rekey in progress" + } + if err == nil || !strings.Contains(err.Error(), expected) { + t.Fatalf("no rekey should be in progress, err: %v", err) } // Should be no progress @@ -130,10 +138,10 @@ func testCore_Rekey_Init_Common(t *testing.T, c *Core, recovery bool) { } func TestCore_Rekey_Update(t *testing.T) { - bc, _ := TestSealDefConfigs() - bc.SecretShares = 1 - bc.SecretThreshold = 1 - bc.StoredShares = 0 + bc := &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + } c, masterKeys, _, root := TestCoreUnsealedWithConfigs(t, bc, nil) testCore_Rekey_Update_Common(t, c, masterKeys, root, false) } @@ -181,13 +189,6 @@ func testCore_Rekey_Update_Common(t *testing.T, c *Core, keys [][]byte, root str if result == nil { t.Fatal("nil result after update") } - if newConf.StoredShares > 0 { - if len(result.SecretShares) > 0 { - t.Fatal("got secret shares when should have been storing") - } - } else if len(result.SecretShares) != newConf.SecretShares { - t.Fatalf("rekey update error: %#v", result) - } // Should be no progress if _, _, err := c.RekeyProgress(recovery, false); err == nil { @@ -321,8 +322,21 @@ func testCore_Rekey_Update_Common(t *testing.T, c *Core, keys [][]byte, root str } } +func TestCore_Rekey_Legacy(t *testing.T) { + bc := &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + } + c, masterKeys, _, root := TestCoreUnsealedWithConfigSealOpts(t, bc, nil, + &TestSealOpts{StoredKeys: StoredKeysNotSupported}) + testCore_Rekey_Update_Common(t, c, masterKeys, root, false) +} + func TestCore_Rekey_Invalid(t *testing.T) { - bc, _ := TestSealDefConfigs() + bc := &SealConfig{ + SecretShares: 5, + SecretThreshold: 3, + } bc.StoredShares = 0 bc.SecretShares = 1 bc.SecretThreshold = 1 @@ -496,3 +510,25 @@ func TestCore_Rekey_Standby(t *testing.T) { t.Fatalf("rekey failed") } } + +// verifies that if we are using recovery keys to force a +// rekey of a stored-shares barrier that verification is not allowed since +// the keys aren't returned +func TestSysRekey_Verification_Invalid(t *testing.T) { + core, _, _, _ := TestCoreUnsealedWithConfigSealOpts(t, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &TestSealOpts{StoredKeys: StoredKeysSupportedGeneric}) + + err := core.BarrierRekeyInit(&SealConfig{ + VerificationRequired: true, + StoredShares: 1, + }) + + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "requiring verification not supported") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/vault/seal.go b/vault/seal.go index 4f859ae47696..8583b2aecf73 100644 --- a/vault/seal.go +++ b/vault/seal.go @@ -3,17 +3,16 @@ package vault import ( "bytes" "context" - "crypto/subtle" "encoding/base64" "encoding/json" "fmt" "sync/atomic" + "github.com/golang/protobuf/proto" "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/vault/seal" - "github.com/keybase/go-crypto/openpgp" "github.com/keybase/go-crypto/openpgp/packet" ) @@ -50,12 +49,36 @@ const ( RecoveryTypeShamir = "shamir" ) +type StoredKeysSupport int + +const ( + // The 0 value of StoredKeysSupport is an invalid option + StoredKeysInvalid StoredKeysSupport = iota + StoredKeysNotSupported + StoredKeysSupportedGeneric + StoredKeysSupportedShamirMaster +) + +func (s StoredKeysSupport) String() string { + switch s { + case StoredKeysNotSupported: + return "Old-style Shamir" + case StoredKeysSupportedGeneric: + return "AutoUnseal" + case StoredKeysSupportedShamirMaster: + return "New-style Shamir" + default: + return "Invalid StoredKeys type" + } +} + type Seal interface { SetCore(*Core) Init(context.Context) error Finalize(context.Context) error - StoredKeysSupported() bool + StoredKeysSupported() StoredKeysSupport + SealWrapable() bool SetStoredKeys(context.Context, [][]byte) error GetStoredKeys(context.Context) ([][]byte, error) @@ -77,12 +100,9 @@ type Seal interface { } type defaultSeal struct { - access seal.Access - config atomic.Value - core *Core - PretendToAllowStoredShares bool - PretendToAllowRecoveryKeys bool - PretendRecoveryKey []byte + access seal.Access + config atomic.Value + core *Core } func NewDefaultSeal(lowLevel seal.Access) Seal { @@ -93,6 +113,10 @@ func NewDefaultSeal(lowLevel seal.Access) Seal { return ret } +func (d *defaultSeal) SealWrapable() bool { + return false +} + func (d *defaultSeal) checkCore() error { if d.core == nil { return fmt.Errorf("seal does not have a core set") @@ -124,25 +148,61 @@ func (d *defaultSeal) BarrierType() string { return seal.Shamir } -func (d *defaultSeal) StoredKeysSupported() bool { - return d.PretendToAllowStoredShares +func (d *defaultSeal) StoredKeysSupported() StoredKeysSupport { + isLegacy, err := d.LegacySeal() + if err != nil { + if d.core != nil && d.core.logger != nil { + d.core.logger.Error("no seal config found, can't determine if legacy or new-style shamir") + } + return StoredKeysInvalid + } + switch { + case isLegacy: + return StoredKeysNotSupported + default: + return StoredKeysSupportedShamirMaster + } } func (d *defaultSeal) RecoveryKeySupported() bool { - return d.PretendToAllowRecoveryKeys + return false } func (d *defaultSeal) SetStoredKeys(ctx context.Context, keys [][]byte) error { - return fmt.Errorf("stored keys are not supported") + isLegacy, err := d.LegacySeal() + if err != nil { + return err + } + if isLegacy { + return fmt.Errorf("stored keys are not supported") + } + return writeStoredKeys(ctx, d.core.physical, d.access, keys) +} + +func (d *defaultSeal) LegacySeal() (bool, error) { + cfg := d.config.Load().(*SealConfig) + if cfg == nil { + return false, fmt.Errorf("no seal config found, can't determine if legacy or new-style shamir") + } + return cfg.StoredShares == 0, nil } func (d *defaultSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { - return nil, fmt.Errorf("stored keys are not supported") + isLegacy, err := d.LegacySeal() + if err != nil { + return nil, err + } + if isLegacy { + return nil, fmt.Errorf("stored keys are not supported") + } + keys, err := readStoredKeys(ctx, d.core.physical, d.access) + return keys, err } func (d *defaultSeal) BarrierConfig(ctx context.Context) (*SealConfig, error) { - if d.config.Load().(*SealConfig) != nil { - return d.config.Load().(*SealConfig).Clone(), nil + cfg := d.config.Load().(*SealConfig) + if cfg != nil { + return cfg.Clone(), nil } if err := d.checkCore(); err != nil { @@ -204,7 +264,7 @@ func (d *defaultSeal) SetBarrierConfig(ctx context.Context, config *SealConfig) config.Type = d.BarrierType() - // If we are doing a raft unseal we do not want to persit the barrier config + // If we are doing a raft unseal we do not want to persist the barrier config // because storage isn't setup yet. if d.core.isRaftUnseal() { d.config.Store(config.Clone()) @@ -238,34 +298,18 @@ func (d *defaultSeal) SetCachedBarrierConfig(config *SealConfig) { } func (d *defaultSeal) RecoveryType() string { - if d.PretendToAllowRecoveryKeys { - return RecoveryTypeShamir - } return RecoveryTypeUnsupported } func (d *defaultSeal) RecoveryConfig(ctx context.Context) (*SealConfig, error) { - if d.PretendToAllowRecoveryKeys { - return &SealConfig{ - SecretShares: 5, - SecretThreshold: 3, - }, nil - } return nil, fmt.Errorf("recovery not supported") } func (d *defaultSeal) RecoveryKey(ctx context.Context) ([]byte, error) { - if d.PretendToAllowRecoveryKeys { - return d.PretendRecoveryKey, nil - } - return nil, fmt.Errorf("recovery not supported") } func (d *defaultSeal) SetRecoveryConfig(ctx context.Context, config *SealConfig) error { - if d.PretendToAllowRecoveryKeys { - return nil - } return fmt.Errorf("recovery not supported") } @@ -273,20 +317,10 @@ func (d *defaultSeal) SetCachedRecoveryConfig(config *SealConfig) { } func (d *defaultSeal) VerifyRecoveryKey(ctx context.Context, key []byte) error { - if d.PretendToAllowRecoveryKeys { - if subtle.ConstantTimeCompare(key, d.PretendRecoveryKey) == 1 { - return nil - } - return fmt.Errorf("mismatch") - } return fmt.Errorf("recovery not supported") } func (d *defaultSeal) SetRecoveryKey(ctx context.Context, key []byte) error { - if d.PretendToAllowRecoveryKeys { - d.PretendRecoveryKey = key - return nil - } return fmt.Errorf("recovery not supported") } @@ -318,7 +352,7 @@ type SealConfig struct { // should be stored at coreUnsealKeysBackupPath after successful rekeying. Backup bool `json:"backup" mapstructure:"backup"` - // How many keys to store, for seals that support storage. + // How many keys to store, for seals that support storage. Always 0 or 1. StoredShares int `json:"stored_shares" mapstructure:"stored_shares"` // Stores the progress of the rekey operation (key shares) @@ -361,10 +395,10 @@ func (s *SealConfig) Validate() error { if s.SecretThreshold > s.SecretShares { return fmt.Errorf("threshold cannot be larger than shares") } - if s.StoredShares > s.SecretShares { - return fmt.Errorf("stored keys cannot be larger than shares") + if s.StoredShares > 1 { + return fmt.Errorf("stored keys cannot be larger than 1") } - if len(s.PGPKeys) > 0 && len(s.PGPKeys) != s.SecretShares-s.StoredShares { + if len(s.PGPKeys) > 0 && len(s.PGPKeys) != s.SecretShares { return fmt.Errorf("count mismatch between number of provided PGP keys and number of shares") } if len(s.PGPKeys) > 0 { @@ -403,3 +437,71 @@ func (s *SealConfig) Clone() *SealConfig { } return ret } + +func writeStoredKeys(ctx context.Context, storage physical.Backend, encryptor seal.Encryptor, keys [][]byte) error { + if keys == nil { + return fmt.Errorf("keys were nil") + } + if len(keys) == 0 { + return fmt.Errorf("no keys provided") + } + + buf, err := json.Marshal(keys) + if err != nil { + return errwrap.Wrapf("failed to encode keys for storage: {{err}}", err) + } + + // Encrypt and marshal the keys + blobInfo, err := encryptor.Encrypt(ctx, buf) + if err != nil { + return errwrap.Wrapf("failed to encrypt keys for storage: {{err}}", err) + } + + value, err := proto.Marshal(blobInfo) + if err != nil { + return errwrap.Wrapf("failed to marshal value for storage: {{err}}", err) + } + + // Store the seal configuration. + pe := &physical.Entry{ + Key: StoredBarrierKeysPath, + Value: value, + } + + if err := storage.Put(ctx, pe); err != nil { + return errwrap.Wrapf("failed to write keys to storage: {{err}}", err) + } + + return nil +} + +func readStoredKeys(ctx context.Context, storage physical.Backend, encryptor seal.Encryptor) ([][]byte, error) { + pe, err := storage.Get(ctx, StoredBarrierKeysPath) + if err != nil { + return nil, 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, nil + } + + blobInfo := &physical.EncryptedBlobInfo{} + if err := proto.Unmarshal(pe.Value, blobInfo); err != nil { + return nil, errwrap.Wrapf("failed to proto decode stored keys: {{err}}", err) + } + + pt, err := encryptor.Decrypt(ctx, blobInfo) + if err != nil { + return nil, 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 nil, fmt.Errorf("failed to decode stored keys: %v", err) + } + + return keys, nil +} diff --git a/vault/seal/seal.go b/vault/seal/seal.go index 44eab206ecdf..fd1deb3f539d 100644 --- a/vault/seal/seal.go +++ b/vault/seal/seal.go @@ -22,6 +22,11 @@ const ( HSMAutoDeprecated = "hsm-auto" ) +type Encryptor interface { + Encrypt(context.Context, []byte) (*physical.EncryptedBlobInfo, error) + Decrypt(context.Context, *physical.EncryptedBlobInfo) ([]byte, error) +} + // Access is the embedded implementation of autoSeal that contains logic // specific to encrypting and decrypting data, or in this case keys. type Access interface { @@ -31,6 +36,5 @@ type Access interface { Init(context.Context) error Finalize(context.Context) error - Encrypt(context.Context, []byte) (*physical.EncryptedBlobInfo, error) - Decrypt(context.Context, *physical.EncryptedBlobInfo) ([]byte, error) + Encryptor } diff --git a/vault/seal/shamir/shamir.go b/vault/seal/shamir/shamir.go index 9916263d2c20..5054cc1d34c9 100644 --- a/vault/seal/shamir/shamir.go +++ b/vault/seal/shamir/shamir.go @@ -4,7 +4,6 @@ import ( "context" "crypto/aes" "crypto/cipher" - "encoding/base64" "errors" "fmt" @@ -17,6 +16,7 @@ import ( // ShamirSeal implements the seal.Access interface for Shamir unseal type ShamirSeal struct { logger log.Logger + key []byte aead cipher.AEAD } @@ -31,35 +31,24 @@ func NewSeal(logger log.Logger) *ShamirSeal { return seal } -// SetConfig sets the fields on the ShamirSeal object based on -// values from the config parameter. -func (s *ShamirSeal) SetConfig(config map[string]string) (map[string]string, error) { - // Map that holds non-sensitive configuration info - sealInfo := make(map[string]string) - - if config == nil || config["key"] == "" { - return sealInfo, nil - } - - keyB64 := config["key"] - key, err := base64.StdEncoding.DecodeString(keyB64) - if err != nil { - return sealInfo, err - } +func (s *ShamirSeal) GetKey() []byte { + return s.key +} +func (s *ShamirSeal) SetKey(key []byte) error { aesCipher, err := aes.NewCipher(key) if err != nil { - return sealInfo, err + return err } aead, err := cipher.NewGCM(aesCipher) if err != nil { - return sealInfo, err + return err } + s.key = key s.aead = aead - - return sealInfo, nil + return nil } // Init is called during core.Initialize. No-op at the moment. diff --git a/vault/seal_access.go b/vault/seal_access.go index f4a31dc90824..5f44433c3462 100644 --- a/vault/seal_access.go +++ b/vault/seal_access.go @@ -2,7 +2,6 @@ package vault import ( "context" - "fmt" ) // SealAccess is a wrapper around Seal that exposes accessor methods @@ -16,7 +15,7 @@ func NewSealAccess(seal Seal) *SealAccess { return &SealAccess{seal: seal} } -func (s *SealAccess) StoredKeysSupported() bool { +func (s *SealAccess) StoredKeysSupported() StoredKeysSupport { return s.seal.StoredKeysSupported() } @@ -46,22 +45,3 @@ func (s *SealAccess) ClearCaches(ctx context.Context) { s.seal.SetRecoveryConfig(ctx, nil) } } - -type SealAccessTestingParams struct { - PretendToAllowStoredShares bool - PretendToAllowRecoveryKeys bool - PretendRecoveryKey []byte -} - -func (s *SealAccess) SetTestingParams(params *SealAccessTestingParams) error { - d, ok := s.seal.(*defaultSeal) - if !ok { - return fmt.Errorf("not a defaultseal") - } - d.PretendToAllowRecoveryKeys = params.PretendToAllowRecoveryKeys - d.PretendToAllowStoredShares = params.PretendToAllowStoredShares - if params.PretendRecoveryKey != nil { - d.PretendRecoveryKey = params.PretendRecoveryKey - } - return nil -} diff --git a/vault/seal_autoseal.go b/vault/seal_autoseal.go index 854840844892..cb8af8408646 100644 --- a/vault/seal_autoseal.go +++ b/vault/seal_autoseal.go @@ -42,6 +42,10 @@ func NewAutoSeal(lowLevel seal.Access) *autoSeal { return ret } +func (d *autoSeal) SealWrapable() bool { + return true +} + func (d *autoSeal) GetAccess() seal.Access { return d.Access } @@ -73,8 +77,8 @@ func (d *autoSeal) BarrierType() string { return d.SealType() } -func (d *autoSeal) StoredKeysSupported() bool { - return true +func (d *autoSeal) StoredKeysSupported() StoredKeysSupport { + return StoredKeysSupportedGeneric } func (d *autoSeal) RecoveryKeySupported() bool { @@ -84,73 +88,13 @@ func (d *autoSeal) RecoveryKeySupported() bool { // SetStoredKeys uses the autoSeal.Access.Encrypts method to wrap the keys. The stored entry // does not need to be seal wrapped in this case. func (d *autoSeal) SetStoredKeys(ctx context.Context, keys [][]byte) error { - if keys == nil { - return fmt.Errorf("keys were nil") - } - if len(keys) == 0 { - return fmt.Errorf("no keys provided") - } - - buf, err := json.Marshal(keys) - if err != nil { - return errwrap.Wrapf("failed to encode keys for storage: {{err}}", err) - } - - // Encrypt and marshal the keys - blobInfo, err := d.Encrypt(ctx, buf) - if err != nil { - return errwrap.Wrapf("failed to encrypt keys for storage: {{err}}", err) - } - - value, err := proto.Marshal(blobInfo) - if err != nil { - return errwrap.Wrapf("failed to marshal value for storage: {{err}}", err) - } - - // Store the seal configuration. - pe := &physical.Entry{ - Key: StoredBarrierKeysPath, - Value: value, - } - - if err := d.core.physical.Put(ctx, pe); err != nil { - return errwrap.Wrapf("failed to write keys to storage: {{err}}", err) - } - - return nil + return writeStoredKeys(ctx, d.core.physical, d, keys) } // GetStoredKeys retrieves the key shares by unwrapping the encrypted key using the // autoseal. func (d *autoSeal) GetStoredKeys(ctx context.Context) ([][]byte, error) { - pe, err := d.core.physical.Get(ctx, StoredBarrierKeysPath) - if err != nil { - return nil, 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, nil - } - - blobInfo := &physical.EncryptedBlobInfo{} - if err := proto.Unmarshal(pe.Value, blobInfo); err != nil { - return nil, errwrap.Wrapf("failed to proto decode stored keys: {{err}}", err) - } - - pt, err := d.Decrypt(ctx, blobInfo) - if err != nil { - return nil, 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 nil, fmt.Errorf("failed to decode stored keys: %v", err) - } - - return keys, nil + return readStoredKeys(ctx, d.core.physical, d) } func (d *autoSeal) upgradeStoredKeys(ctx context.Context) error { diff --git a/vault/seal_test.go b/vault/seal_test.go index a31c0b981a9e..a174289b2015 100644 --- a/vault/seal_test.go +++ b/vault/seal_test.go @@ -6,13 +6,16 @@ import ( "testing" ) +// TestDefaultSeal_Config exercises Shamir SetBarrierConfig and BarrierConfig. +// Note that this is a little questionable, because we're doing an init and +// unseal, then changing the barrier config using an internal function instead +// of an API. In other words if your change break this test, it might be more +// the test's fault than your changes. func TestDefaultSeal_Config(t *testing.T) { - bc, _ := TestSealDefConfigs() - // Change these to non-default values to ensure we are seeing the real - // config we set - bc.SecretShares = 4 - bc.SecretThreshold = 2 - + bc := &SealConfig{ + SecretShares: 4, + SecretThreshold: 2, + } core, _, _ := TestCoreUnsealed(t) defSeal := NewDefaultSeal(nil) diff --git a/vault/seal_testing.go b/vault/seal_testing.go index 58e04f7b3a87..1437914631fa 100644 --- a/vault/seal_testing.go +++ b/vault/seal_testing.go @@ -7,74 +7,35 @@ import ( testing "github.com/mitchellh/go-testing-interface" ) -var ( - TestCoreUnsealedWithConfigs = testCoreUnsealedWithConfigs - TestSealDefConfigs = testSealDefConfigs -) - type TestSealOpts struct { - StoredKeysDisabled bool - RecoveryKeysDisabled bool - Secret []byte - Logger log.Logger + Logger log.Logger + StoredKeys StoredKeysSupport + Secret []byte } -func testCoreUnsealedWithConfigs(t testing.T, barrierConf, recoveryConf *SealConfig) (*Core, [][]byte, [][]byte, string) { +func TestCoreUnsealedWithConfigs(t testing.T, barrierConf, recoveryConf *SealConfig) (*Core, [][]byte, [][]byte, string) { t.Helper() - var opts *TestSealOpts + opts := &TestSealOpts{} if recoveryConf == nil { - opts = &TestSealOpts{ - StoredKeysDisabled: true, - RecoveryKeysDisabled: true, - } - } - seal := NewTestSeal(t, opts) - core := TestCoreWithSeal(t, seal, false) - result, err := core.Initialize(context.Background(), &InitParams{ - BarrierConfig: barrierConf, - RecoveryConfig: recoveryConf, - }) - if err != nil { - t.Fatalf("err: %s", err) - } - err = core.UnsealWithStoredKeys(context.Background()) - if err != nil && IsFatalError(err) { - t.Fatalf("err: %s", err) + opts.StoredKeys = StoredKeysSupportedShamirMaster } - if core.Sealed() { - for _, key := range result.SecretShares { - if _, err := core.Unseal(TestKeyCopy(key)); err != nil { - t.Fatalf("unseal err: %s", err) - } - } - - if core.Sealed() { - t.Fatal("should not be sealed") - } - } - - return core, result.SecretShares, result.RecoveryShares, result.RootToken -} - -func testSealDefConfigs() (*SealConfig, *SealConfig) { - return &SealConfig{ - SecretShares: 5, - SecretThreshold: 3, - }, nil + return TestCoreUnsealedWithConfigSealOpts(t, barrierConf, recoveryConf, opts) } func TestCoreUnsealedWithConfigSealOpts(t testing.T, barrierConf, recoveryConf *SealConfig, sealOpts *TestSealOpts) (*Core, [][]byte, [][]byte, string) { + t.Helper() seal := NewTestSeal(t, sealOpts) core := TestCoreWithSeal(t, seal, false) result, err := core.Initialize(context.Background(), &InitParams{ - BarrierConfig: barrierConf, - RecoveryConfig: recoveryConf, + BarrierConfig: barrierConf, + RecoveryConfig: recoveryConf, + LegacyShamirSeal: sealOpts.StoredKeys == StoredKeysNotSupported, }) if err != nil { t.Fatalf("err: %s", err) } err = core.UnsealWithStoredKeys(context.Background()) - if err != nil { + if err != nil && IsFatalError(err) { t.Fatalf("err: %s", err) } if core.Sealed() { diff --git a/vault/seal_testing_util.go b/vault/seal_testing_util.go index 3c65e1176731..b919f4a0d5c3 100644 --- a/vault/seal_testing_util.go +++ b/vault/seal_testing_util.go @@ -1,17 +1,41 @@ -// +build !enterprise - package vault import ( "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault/sdk/helper/logging" + "github.com/hashicorp/vault/vault/seal" shamirseal "github.com/hashicorp/vault/vault/seal/shamir" testing "github.com/mitchellh/go-testing-interface" ) func NewTestSeal(t testing.T, opts *TestSealOpts) Seal { - var logger hclog.Logger - if opts != nil { - logger = opts.Logger + t.Helper() + if opts == nil { + opts = &TestSealOpts{} + } + if opts.Logger == nil { + opts.Logger = logging.NewVaultLogger(hclog.Debug) + } + + switch opts.StoredKeys { + case StoredKeysSupportedShamirMaster: + newSeal := NewDefaultSeal(shamirseal.NewSeal(opts.Logger)) + // Need StoredShares set or this will look like a legacy shamir seal. + newSeal.SetCachedBarrierConfig(&SealConfig{ + StoredShares: 1, + SecretThreshold: 1, + SecretShares: 1, + }) + return newSeal + case StoredKeysNotSupported: + newSeal := NewDefaultSeal(shamirseal.NewSeal(opts.Logger)) + newSeal.SetCachedBarrierConfig(&SealConfig{ + StoredShares: 0, + SecretThreshold: 1, + SecretShares: 1, + }) + return newSeal + default: + return NewAutoSeal(seal.NewTestSeal(opts.Secret)) } - return NewDefaultSeal(shamirseal.NewSeal(logger)) } diff --git a/vault/testing.go b/vault/testing.go index 335acde8bb44..ba6e554dd54a 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -266,9 +266,11 @@ func TestCoreInitClusterWrapperSetup(t testing.T, core *Core, handler http.Handl SecretThreshold: 3, } - // If we support storing barrier keys, then set that to equal the min threshold to unseal - if core.seal.StoredKeysSupported() { - barrierConfig.StoredShares = barrierConfig.SecretThreshold + switch core.seal.StoredKeysSupported() { + case StoredKeysNotSupported: + barrierConfig.StoredShares = 0 + default: + barrierConfig.StoredShares = 1 } recoveryConfig := &SealConfig{ @@ -276,10 +278,14 @@ func TestCoreInitClusterWrapperSetup(t testing.T, core *Core, handler http.Handl SecretThreshold: 3, } - result, err := core.Initialize(context.Background(), &InitParams{ + initParams := &InitParams{ BarrierConfig: barrierConfig, RecoveryConfig: recoveryConfig, - }) + } + if core.seal.StoredKeysSupported() == StoredKeysNotSupported { + initParams.LegacyShamirSeal = true + } + result, err := core.Initialize(context.Background(), initParams) if err != nil { t.Fatalf("err: %s", err) } @@ -822,6 +828,7 @@ func (c *TestCluster) Start() { // UnsealCores uses the cluster barrier keys to unseal the test cluster cores func (c *TestCluster) UnsealCores(t testing.T) { + t.Helper() if err := c.UnsealCoresWithError(); err != nil { t.Fatal(err) } @@ -833,7 +840,7 @@ func (c *TestCluster) UnsealCoresWithError() error { // Unseal first core for _, key := range c.BarrierKeys { if _, err := c.Cores[0].Unseal(TestKeyCopy(key)); err != nil { - return fmt.Errorf("unseal err: %s", err) + return fmt.Errorf("unseal core %d err: %s", 0, err) } } @@ -850,7 +857,7 @@ func (c *TestCluster) UnsealCoresWithError() error { for i := 1; i < numCores; i++ { for _, key := range c.BarrierKeys { if _, err := c.Cores[i].Core.Unseal(TestKeyCopy(key)); err != nil { - return fmt.Errorf("unseal err: %s", err) + return fmt.Errorf("unseal core %d err: %s", i, err) } } } @@ -1630,9 +1637,15 @@ func NewTestCluster(t testing.T, base *CoreConfig, opts *TestClusterOptions) *Te t.Fatal(err) } + cfg, err := cores[0].seal.BarrierConfig(ctx) + if err != nil { + t.Fatal(err) + } + // Unseal other cores unless otherwise specified if (opts == nil || !opts.KeepStandbysSealed) && numCores > 1 { for i := 1; i < numCores; i++ { + cores[i].seal.SetCachedBarrierConfig(cfg) for _, key := range bKeys { if _, err := cores[i].Unseal(TestKeyCopy(key)); err != nil { t.Fatalf("unseal err: %s", err) From 8f109b5782354d94b623af0c029072bc6d795a7a Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Fri, 18 Oct 2019 09:11:54 -0400 Subject: [PATCH 2/5] Move to 1.12.12. --- .circleci/config.yml | 8 ++++---- .circleci/config/@config.yml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4ce52b9170c4..2b85d20d6b74 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -54,7 +54,7 @@ jobs: - ui/node_modules go-mod-download: docker: - - image: golang:1.12.11-buster + - image: golang:1.12.12-buster shell: /usr/bin/env bash -euo pipefail -c working_directory: /src steps: @@ -76,7 +76,7 @@ jobs: - /go/pkg/mod build-go-dev: docker: - - image: golang:1.12.11-buster + - image: golang:1.12.12-buster shell: /usr/bin/env bash -euo pipefail -c working_directory: /src steps: @@ -407,7 +407,7 @@ workflows: # executors: # go: # docker: -# - image: golang:1.12.11-buster +# - image: golang:1.12.12-buster # shell: /usr/bin/env bash -euo pipefail -c # working_directory: /src # go-machine: @@ -586,7 +586,7 @@ workflows: # go-sum: go-sum-v1-{{ checksum \"go.sum\" }} # yarn-lock: yarn-lock-v1-{{ checksum \"ui/yarn.lock\" }} # images: -# go: golang:1.12.11-buster +# go: golang:1.12.12-buster # node: node:10-buster # version: 2.1 # workflows: diff --git a/.circleci/config/@config.yml b/.circleci/config/@config.yml index c3fcebea90ff..2ead2ef76246 100644 --- a/.circleci/config/@config.yml +++ b/.circleci/config/@config.yml @@ -3,7 +3,7 @@ version: 2.1 references: images: - go: &GOLANG_IMAGE golang:1.12.11-buster # Pin Go to patch version (ex: 1.2.3) + go: &GOLANG_IMAGE golang:1.12.12-buster # Pin Go to patch version (ex: 1.2.3) node: &NODE_IMAGE node:10-buster # Pin Node.js to major version (ex: 10) cache: From 2109ff9fe6680bc067ebddc3a83fda4808f4b522 Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Fri, 18 Oct 2019 12:14:13 -0400 Subject: [PATCH 3/5] Update seal migration code and test to reflect new shamir seal behaviour. --- command/seal_migration_test.go | 28 ++++++++++++++-------------- vault/core.go | 30 +++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/command/seal_migration_test.go b/command/seal_migration_test.go index 373b6dd7e773..008783d8ba1c 100644 --- a/command/seal_migration_test.go +++ b/command/seal_migration_test.go @@ -20,7 +20,7 @@ import ( ) func TestSealMigration(t *testing.T) { - logger := logging.NewVaultLogger(hclog.Trace) + logger := logging.NewVaultLogger(hclog.Trace).Named(t.Name()) phys, err := physInmem.NewInmem(nil, logger) if err != nil { t.Fatal(err) @@ -47,8 +47,8 @@ func TestSealMigration(t *testing.T) { var keys []string var rootToken string - // First: start up as normal with shamir seal, init it { + logger.Info("integ: start up as normal with shamir seal, init it") cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() defer cluster.Cleanup() @@ -73,9 +73,9 @@ func TestSealMigration(t *testing.T) { cluster.Cores = nil } - // Second: start up as normal with shamir seal and unseal, make sure - // everything is normal { + logger.SetLevel(hclog.Trace) + logger.Info("integ: start up as normal with shamir seal and unseal, make sure everything is normal") cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() defer cluster.Cleanup() @@ -103,8 +103,9 @@ func TestSealMigration(t *testing.T) { var autoSeal vault.Seal - // Third: create an autoseal and activate migration { + logger.SetLevel(hclog.Trace) + logger.Info("integ: creating an autoseal and activating migration") cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() defer cluster.Cleanup() @@ -147,8 +148,9 @@ func TestSealMigration(t *testing.T) { cluster.Cores = nil } - // Fourth: verify autoseal and recovery key usage { + logger.SetLevel(hclog.Trace) + logger.Info("integ: verify autoseal and recovery key usage") coreConfig.Seal = autoSeal cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() @@ -202,8 +204,9 @@ func TestSealMigration(t *testing.T) { altTestSeal.Type = "test-alternate" altSeal := vault.NewAutoSeal(altTestSeal) - // Fifth: migrate from auto-seal to auto-seal { + logger.SetLevel(hclog.Trace) + logger.Info("integ: migrate from auto-seal to auto-seal") coreConfig.Seal = autoSeal cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() @@ -239,9 +242,9 @@ func TestSealMigration(t *testing.T) { cluster.Cores = nil } - // Sixth: create an Shamir seal and activate migration. Verify it doesn't work - // if disabled isn't set. { + logger.SetLevel(hclog.Trace) + logger.Info("integ: create a Shamir seal and activate migration; verify it doesn't work if disabled isn't set.") coreConfig.Seal = altSeal cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() @@ -282,12 +285,9 @@ func TestSealMigration(t *testing.T) { cluster.Cores = nil } - if entry, err := phys.Get(ctx, vault.StoredBarrierKeysPath); err != nil || entry != nil { - t.Fatalf("expected nil error and nil entry, got error %#v and entry %#v", err, entry) - } - - // Seventh: verify autoseal is off and the expected key shares work { + logger.SetLevel(hclog.Trace) + logger.Info("integ: verify autoseal is off and the expected key shares work") coreConfig.Seal = shamirSeal cluster := vault.NewTestCluster(t, coreConfig, clusterConfig) cluster.Start() diff --git a/vault/core.go b/vault/core.go index 2899e664e854..6f3f4c602aec 100644 --- a/vault/core.go +++ b/vault/core.go @@ -931,6 +931,7 @@ func (c *Core) unseal(key []byte, useRecoveryKeys bool) (bool, error) { sealToUse := c.seal if c.migrationSeal != nil { + c.logger.Info("unsealing using migration seal") sealToUse = c.migrationSeal } @@ -1112,7 +1113,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover } if seal.RecoveryKeySupported() && (useRecoveryKeys || c.migrationSeal != nil) { - // Verify recovery key + // Verify recovery key. if err := seal.VerifyRecoveryKey(ctx, recoveredKey); err != nil { return nil, err } @@ -1123,7 +1124,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover // keys setup, nor 2) seals that support recovery keys but not stored keys. // If insufficient shares are provided, shamir.Combine will error, and if // no stored keys are found it will return masterKey as nil. - if seal.StoredKeysSupported() == StoredKeysSupportedGeneric { + if seal.StoredKeysSupported() != StoredKeysNotSupported { masterKeyShares, err := seal.GetStoredKeys(ctx) if err != nil { return nil, errwrap.Wrapf("unable to retrieve stored keys: {{err}}", err) @@ -1144,9 +1145,21 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover } else { masterKey = recoveredKey } + newRecoveryKey := masterKey // If we have a migration seal, now's the time! if c.migrationSeal != nil { + if seal.StoredKeysSupported() == StoredKeysSupportedShamirMaster { + err = seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(masterKey) + if err != nil { + return nil, errwrap.Wrapf("failed to set master key in seal: {{err}}", err) + } + storedKeys, err := seal.GetStoredKeys(ctx) + if err != nil { + return nil, errwrap.Wrapf("unable to retrieve stored keys: {{err}}", err) + } + masterKey = storedKeys[0] + } // Unseal the barrier so we can rekey if err := c.barrier.Unseal(ctx, masterKey); err != nil { return nil, errwrap.Wrapf("error unsealing barrier with constructed master key: {{err}}", err) @@ -1185,13 +1198,12 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover // We have recovery keys; we're going to use them as the new // barrier key. - if err := c.barrier.Rekey(ctx, recoveryKey); err != nil { - return nil, errwrap.Wrapf("error rekeying barrier during migration: {{err}}", err) + err = c.seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(recoveryKey) + if err != nil { + return nil, errwrap.Wrapf("failed to set master key in seal: {{err}}", err) } - - if err := c.barrier.Delete(ctx, StoredBarrierKeysPath); err != nil { - // Don't actually exit here as successful deletion isn't critical - c.logger.Error("error deleting stored barrier keys after migration; continuing anyways", "error", err) + if err := c.seal.SetStoredKeys(ctx, [][]byte{masterKey}); err != nil { + return nil, errwrap.Wrapf("error setting new barrier key information during migrate: {{err}}", err) } masterKey = recoveryKey @@ -1199,7 +1211,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover case c.seal.RecoveryKeySupported(): // The new seal will have recovery keys; we set it to the existing // master key, so barrier key shares -> recovery key shares - if err := c.seal.SetRecoveryKey(ctx, masterKey); err != nil { + if err := c.seal.SetRecoveryKey(ctx, newRecoveryKey); err != nil { return nil, errwrap.Wrapf("error setting new recovery key information: {{err}}", err) } From f1c6494a004ba9a8d590d9edb10cf9f0ae35ecfd Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Fri, 18 Oct 2019 12:28:10 -0400 Subject: [PATCH 4/5] Fix test complication error. --- vault/generate_root_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vault/generate_root_test.go b/vault/generate_root_test.go index cd4c44be0c43..f57cfa75db69 100644 --- a/vault/generate_root_test.go +++ b/vault/generate_root_test.go @@ -82,7 +82,8 @@ func TestCore_GenerateRoot_Init(t *testing.T) { c, _, _ := TestCoreUnsealed(t) testCore_GenerateRoot_Init_Common(t, c) - bc, rc := TestSealDefConfigs() + bc := &SealConfig{SecretShares: 5, SecretThreshold: 3, StoredShares: 1} + rc := &SealConfig{SecretShares: 5, SecretThreshold: 3} c, _, _, _ = TestCoreUnsealedWithConfigs(t, bc, rc) testCore_GenerateRoot_Init_Common(t, c) } From 38cf4cfd92a759d01e31164102306107971fd31f Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Fri, 18 Oct 2019 14:24:17 -0400 Subject: [PATCH 5/5] Revert an unnecessary change, fix comment. --- vault/core.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/core.go b/vault/core.go index 6f3f4c602aec..9d9fc0029514 100644 --- a/vault/core.go +++ b/vault/core.go @@ -1124,7 +1124,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover // keys setup, nor 2) seals that support recovery keys but not stored keys. // If insufficient shares are provided, shamir.Combine will error, and if // no stored keys are found it will return masterKey as nil. - if seal.StoredKeysSupported() != StoredKeysNotSupported { + if seal.StoredKeysSupported() == StoredKeysSupportedGeneric { masterKeyShares, err := seal.GetStoredKeys(ctx) if err != nil { return nil, errwrap.Wrapf("unable to retrieve stored keys: {{err}}", err) @@ -1197,7 +1197,7 @@ func (c *Core) unsealPart(ctx context.Context, seal Seal, key []byte, useRecover } // We have recovery keys; we're going to use them as the new - // barrier key. + // shamir KeK. err = c.seal.GetAccess().(*shamirseal.ShamirSeal).SetKey(recoveryKey) if err != nil { return nil, errwrap.Wrapf("failed to set master key in seal: {{err}}", err)