From 23b0fb62de76721fb6ea68630afb428bfe8592a0 Mon Sep 17 00:00:00 2001 From: Vishal Nayak Date: Wed, 23 Oct 2019 12:52:28 -0400 Subject: [PATCH] Abstract generate-root authentication into the strategy interface (#7698) * Abstract generate-root authentication into the strategy interface * Generate root strategy ncabatoff (#7700) * Adapt to new shamir-as-kek reality. * Don't try to verify the master key when we might still be sealed (in recovery mode). Instead, verify it in the authenticate methods. --- vault/core.go | 47 +++++++++++++++++++ vault/generate_root.go | 82 +++++++-------------------------- vault/generate_root_recovery.go | 21 ++++++++- 3 files changed, 83 insertions(+), 67 deletions(-) diff --git a/vault/core.go b/vault/core.go index 9d9fc0029..644c6243c 100644 --- a/vault/core.go +++ b/vault/core.go @@ -2041,6 +2041,53 @@ func (c *Core) SetSealsForMigration(migrationSeal, newSeal, unwrapSeal Seal) { } } +// unsealKeyToMasterKey takes a key provided by the user, either a recovery key +// if using an autoseal or an unseal key with Shamir. It returns a nil error +// if the key is valid and an error otherwise. It also returns the master key +// that can be used to unseal the barrier. +func (c *Core) unsealKeyToMasterKey(ctx context.Context, combinedKey []byte) ([]byte, error) { + switch c.seal.StoredKeysSupported() { + case StoredKeysSupportedGeneric: + if err := c.seal.VerifyRecoveryKey(ctx, combinedKey); err != nil { + return nil, errwrap.Wrapf("recovery key verification failed: {{err}}", err) + } + + storedKeys, err := c.seal.GetStoredKeys(ctx) + if err == nil && len(storedKeys) != 1 { + err = fmt.Errorf("expected exactly one stored key, got %d", len(storedKeys)) + } + if err != nil { + return nil, errwrap.Wrapf("unable to retrieve stored keys", err) + } + return storedKeys[0], nil + + case 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) + } + storedKeys, err := testseal.GetStoredKeys(ctx) + if err == nil && len(storedKeys) != 1 { + err = fmt.Errorf("expected exactly one stored key, got %d", len(storedKeys)) + } + if err != nil { + return nil, errwrap.Wrapf("unable to retrieve stored keys", err) + } + return storedKeys[0], nil + + case StoredKeysNotSupported: + return combinedKey, nil + } + return nil, fmt.Errorf("invalid seal") +} + func (c *Core) IsInSealMigration() bool { c.stateLock.RLock() defer c.stateLock.RUnlock() diff --git a/vault/generate_root.go b/vault/generate_root.go index 56033fdb4..c0521d30e 100644 --- a/vault/generate_root.go +++ b/vault/generate_root.go @@ -13,7 +13,6 @@ import ( "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" @@ -32,12 +31,25 @@ var ( // create a token upon completion of the generate root process. type GenerateRootStrategy interface { generate(context.Context, *Core) (string, func(), error) + authenticate(context.Context, *Core, []byte) error } // generateStandardRootToken implements the GenerateRootStrategy and is in // charge of creating standard root tokens. type generateStandardRootToken struct{} +func (g generateStandardRootToken) authenticate(ctx context.Context, c *Core, combinedKey []byte) error { + _, err := c.unsealKeyToMasterKey(ctx, combinedKey) + if err != nil { + return errwrap.Wrapf("unable to authenticate: {{err}}", err) + } + if err := c.barrier.VerifyMaster(combinedKey); err != nil { + return errwrap.Wrapf("master key verification failed: {{err}}", err) + } + + return nil +} + func (g generateStandardRootToken) generate(ctx context.Context, c *Core) (string, func(), error) { te, err := c.tokenStore.rootToken(ctx) if err != nil { @@ -296,71 +308,9 @@ func (c *Core) GenerateRootUpdate(ctx context.Context, key []byte, nonce string, } } - switch { - case c.seal.RecoveryKeySupported(): - // Ensure that the combined recovery key is valid - if err := c.seal.VerifyRecoveryKey(ctx, combinedKey); err != nil { - c.logger.Error("root generation aborted, recovery key verification failed", "error", err) - return nil, err - } - // If we are in recovery mode, then retrieve - // the stored keys and unseal the barrier - if c.recoveryMode { - storedKeys, err := c.seal.GetStoredKeys(ctx) - if err != nil { - return nil, errwrap.Wrapf("unable to retrieve stored keys in recovery mode: {{err}}", err) - } - - // Use the retrieved master key to unseal the barrier - 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, 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(masterKey); err != nil { - c.logger.Error("root generation aborted, master key verification failed", "error", err) - return nil, err - } - } - } - - // Authentication in recovery mode is successful - if c.recoveryMode { - // Run any post unseal functions that are set - for _, v := range c.postRecoveryUnsealFuncs { - if err := v(); err != nil { - return nil, errwrap.Wrapf("failed to run post unseal func: {{err}}", err) - } - } + if err := strategy.authenticate(ctx, c, combinedKey); err != nil { + c.logger.Error("root generation aborted", "error", err.Error()) + return nil, errwrap.Wrapf("root generation aborted: {{err}}", err) } // Run the generate strategy diff --git a/vault/generate_root_recovery.go b/vault/generate_root_recovery.go index 73b9c2c33..e19c12ca0 100644 --- a/vault/generate_root_recovery.go +++ b/vault/generate_root_recovery.go @@ -2,7 +2,7 @@ package vault import ( "context" - + "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/sdk/helper/base62" "go.uber.org/atomic" ) @@ -19,6 +19,25 @@ type generateRecoveryToken struct { token *atomic.String } +func (g *generateRecoveryToken) authenticate(ctx context.Context, c *Core, combinedKey []byte) error { + key, err := c.unsealKeyToMasterKey(ctx, combinedKey) + if err != nil { + return errwrap.Wrapf("unable to authenticate: {{err}}", err) + } + + // Use the retrieved master key to unseal the barrier + if err := c.barrier.Unseal(ctx, key); err != nil { + return errwrap.Wrapf("recovery operation token generation failed, cannot unseal barrier: {{err}}", err) + } + + for _, v := range c.postRecoveryUnsealFuncs { + if err := v(); err != nil { + return errwrap.Wrapf("failed to run post unseal func: {{err}}", err) + } + } + return nil +} + func (g *generateRecoveryToken) generate(ctx context.Context, c *Core) (string, func(), error) { id, err := base62.Random(TokenLength) if err != nil {