From 9aa481db0ea2191a5833eeb3b2df34b2543a0421 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 14 Feb 2018 10:20:50 -0500 Subject: [PATCH 1/2] Re-add lost stored-shares parameter to operator rekey command. Also change the rekey API to not require explicitly setting values to 1. Fixes #3969 --- command/operator_init.go | 19 +++++++++---------- command/operator_rekey.go | 16 +++++++++++++--- http/sys_init.go | 24 ++++++++---------------- http/sys_rekey.go | 17 +++++++++++------ 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/command/operator_init.go b/command/operator_init.go index 07651148596b..8660b8327d29 100644 --- a/command/operator_init.go +++ b/command/operator_init.go @@ -196,15 +196,6 @@ func (c *OperatorInitCommand) Flags() *FlagSets { "is only used in HSM mode.", }) - f.IntVar(&IntVar{ - Name: "stored-shares", - Target: &c.flagStoredShares, - Default: 0, // No default, because we need to check if was supplied - Completion: complete.PredictAnything, - Usage: "Number of unseal keys to store on an HSM. This must be equal to " + - "-key-shares. This is only used in HSM mode.", - }) - // Deprecations // TODO: remove in 0.9.0 f.BoolVar(&BoolVar{ @@ -222,6 +213,14 @@ func (c *OperatorInitCommand) Flags() *FlagSets { Usage: "", }) + f.IntVar(&IntVar{ + Name: "stored-shares", + Target: &c.flagStoredShares, + Default: 0, // No default, because we need to check if was supplied + Hidden: true, + Usage: "", + }) + return set } @@ -456,7 +455,7 @@ func (c *OperatorInitCommand) init(client *api.Client, req *api.InitRequest) int c.UI.Output("") c.UI.Output(fmt.Sprintf("Initial Root Token: %s", resp.RootToken)) - if req.StoredShares < 1 { + if len(resp.Keys) > 0 { c.UI.Output("") c.UI.Output(wrapAtLength(fmt.Sprintf( "Vault initialized with %d key shares and a key threshold of %d. Please "+ diff --git a/command/operator_rekey.go b/command/operator_rekey.go index 972676fcf2bf..2f297439119f 100644 --- a/command/operator_rekey.go +++ b/command/operator_rekey.go @@ -37,9 +37,10 @@ type OperatorRekeyCommand struct { // Deprecations // TODO: remove in 0.9.0 - flagDelete bool - flagRecoveryKey bool - flagRetrieve bool + flagDelete bool + flagRecoveryKey bool + flagRetrieve bool + flagStoredShares int testStdin io.Reader // for tests } @@ -231,6 +232,14 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Usage: "", }) + f.IntVar(&IntVar{ + Name: "stored-shares", + Target: &c.flagStoredShares, + Default: 0, + Hidden: true, + Usage: "", + }) + return set } @@ -323,6 +332,7 @@ func (c *OperatorRekeyCommand) init(client *api.Client) int { status, err := fn(&api.RekeyInitRequest{ SecretShares: c.flagKeyShares, SecretThreshold: c.flagKeyThreshold, + StoredShares: c.flagStoredShares, PGPKeys: c.flagPGPKeys, Backup: c.flagBackup, }) diff --git a/http/sys_init.go b/http/sys_init.go index 908d719f683d..f13ff5d3b665 100644 --- a/http/sys_init.go +++ b/http/sys_init.go @@ -69,36 +69,28 @@ func handleSysInitPut(core *vault.Core, w http.ResponseWriter, r *http.Request) // need to be a way to actually allow fetching of the generated keys by // operators. if core.SealAccess().StoredKeysSupported() { - if barrierConfig.SecretShares != 1 { - respondError(w, http.StatusBadRequest, fmt.Errorf("secret shares must be 1")) - return - } - if barrierConfig.SecretThreshold != barrierConfig.SecretShares { - respondError(w, http.StatusBadRequest, fmt.Errorf("secret threshold must be same as secret shares")) - return - } - if barrierConfig.StoredShares != barrierConfig.SecretShares { - respondError(w, http.StatusBadRequest, fmt.Errorf("stored shares must be same as secret shares")) - return - } - if barrierConfig.PGPKeys != nil && len(barrierConfig.PGPKeys) > 0 { + 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("init: stored keys supported, forcing shares/threshold to 1") } else { if barrierConfig.StoredShares > 0 { - respondError(w, http.StatusBadRequest, fmt.Errorf("stored keys are not supported")) + 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-barrierConfig.StoredShares { + 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-recoveryConfig.StoredShares { + if len(recoveryConfig.PGPKeys) > 0 && len(recoveryConfig.PGPKeys) != recoveryConfig.SecretShares { respondError(w, http.StatusBadRequest, fmt.Errorf("incorrect number of PGP keys for recovery")) return } diff --git a/http/sys_rekey.go b/http/sys_rekey.go index 23caf2b25c50..e3637eb0de09 100644 --- a/http/sys_rekey.go +++ b/http/sys_rekey.go @@ -117,16 +117,21 @@ func handleSysRekeyInitPut(ctx context.Context, core *vault.Core, recovery bool, return } - // If the seal supports recovery keys and stored keys, then we allow rekeying the barrier key - // iff the secret shares, secret threshold, and stored shares are set to 1. - if !recovery && core.SealAccess().RecoveryKeySupported() && core.SealAccess().StoredKeysSupported() { - if req.SecretShares != 1 || req.SecretThreshold != 1 || req.StoredShares != 1 { - respondError(w, http.StatusBadRequest, fmt.Errorf("secret shares, secret threshold, and stored shares must be set to 1")) + // If the seal supports stored keys, and we are rekeying the barrier key, + // force the shares to 1 + if !recovery && core.SealAccess().StoredKeysSupported() { + req.SecretShares = 1 + req.SecretThreshold = 1 + req.StoredShares = 1 + core.Logger().Warn("rekey: stored keys supported, forcing shares/threshold to 1") + } else { + if req.StoredShares != 0 { + respondError(w, http.StatusBadRequest, fmt.Errorf("stored keys are not supported by the current seal type")) return } } - if len(req.PGPKeys) > 0 && len(req.PGPKeys) != req.SecretShares-req.StoredShares { + if len(req.PGPKeys) > 0 && len(req.PGPKeys) != req.SecretShares { respondError(w, http.StatusBadRequest, fmt.Errorf("incorrect number of PGP keys for rekey")) return } From 5e27e0c1b6257319c3271ca9a42ca16f0eeb8b22 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 14 Feb 2018 16:10:19 -0500 Subject: [PATCH 2/2] Add some commenting --- command/operator_init.go | 3 ++- command/operator_rekey.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/command/operator_init.go b/command/operator_init.go index 8660b8327d29..213702a1a4a8 100644 --- a/command/operator_init.go +++ b/command/operator_init.go @@ -213,10 +213,11 @@ func (c *OperatorInitCommand) Flags() *FlagSets { Usage: "", }) + // Kept to keep scripts passing the flag working, but not used f.IntVar(&IntVar{ Name: "stored-shares", Target: &c.flagStoredShares, - Default: 0, // No default, because we need to check if was supplied + Default: 0, Hidden: true, Usage: "", }) diff --git a/command/operator_rekey.go b/command/operator_rekey.go index 2f297439119f..4cc23fadc7ff 100644 --- a/command/operator_rekey.go +++ b/command/operator_rekey.go @@ -232,6 +232,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Usage: "", }) + // Kept to keep scripts passing the flag working, but not used f.IntVar(&IntVar{ Name: "stored-shares", Target: &c.flagStoredShares,