From 5d7a8aac2b9dc9ef3c1706ae5665498d7010ec28 Mon Sep 17 00:00:00 2001 From: miagilepner Date: Mon, 30 Jan 2023 17:51:01 +0100 Subject: [PATCH] VAULT-12833 Update prompts for the rekey command (#18892) * update prompts for rekey command * cleanup additional places with unseal/recovery keys --- changelog/18892.txt | 3 + command/command_test.go | 44 +++++++-- command/operator_rekey.go | 136 +++++++++++++++++---------- command/operator_rekey_test.go | 167 +++++++++++++++++++++++++++++++++ 4 files changed, 289 insertions(+), 61 deletions(-) create mode 100644 changelog/18892.txt diff --git a/changelog/18892.txt b/changelog/18892.txt new file mode 100644 index 000000000..65b6ebf24 --- /dev/null +++ b/changelog/18892.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cli: updated `vault operator rekey` prompts to describe recovery keys when `-target=recovery` +``` diff --git a/command/command_test.go b/command/command_test.go index f4b8c7fd2..34ef52495 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/hashicorp/vault/vault" + "github.com/hashicorp/vault/vault/seal" "github.com/mitchellh/cli" auditFile "github.com/hashicorp/vault/builtin/audit/file" @@ -70,7 +71,7 @@ func testVaultServer(tb testing.TB) (*api.Client, func()) { func testVaultServerWithKVVersion(tb testing.TB, kvVersion string) (*api.Client, func()) { tb.Helper() - client, _, closer := testVaultServerUnsealWithKVVersion(tb, kvVersion) + client, _, closer := testVaultServerUnsealWithKVVersionWithSeal(tb, kvVersion, nil) return client, closer } @@ -89,13 +90,24 @@ func testVaultServerAllBackends(tb testing.TB) (*api.Client, func()) { return client, closer } +// testVaultServerAutoUnseal creates a test vault cluster and sets it up with auto unseal +// the function returns a client, the recovery keys, and a closer function +func testVaultServerAutoUnseal(tb testing.TB) (*api.Client, []string, func()) { + testSeal := seal.NewTestSeal(nil) + autoSeal, err := vault.NewAutoSeal(testSeal) + if err != nil { + tb.Fatal("unable to create autoseal", err) + } + return testVaultServerUnsealWithKVVersionWithSeal(tb, "1", autoSeal) +} + // testVaultServerUnseal creates a test vault cluster and returns a configured // API client, list of unseal keys (as strings), and a closer function. func testVaultServerUnseal(tb testing.TB) (*api.Client, []string, func()) { - return testVaultServerUnsealWithKVVersion(tb, "1") + return testVaultServerUnsealWithKVVersionWithSeal(tb, "1", nil) } -func testVaultServerUnsealWithKVVersion(tb testing.TB, kvVersion string) (*api.Client, []string, func()) { +func testVaultServerUnsealWithKVVersionWithSeal(tb testing.TB, kvVersion string, seal vault.Seal) (*api.Client, []string, func()) { tb.Helper() logger := log.NewInterceptLogger(&log.LoggerOptions{ Output: log.DefaultOutput, @@ -111,6 +123,7 @@ func testVaultServerUnsealWithKVVersion(tb testing.TB, kvVersion string) (*api.C AuditBackends: defaultVaultAuditBackends, LogicalBackends: defaultVaultLogicalBackends, BuiltinRegistry: builtinplugins.Registry, + Seal: seal, }, &vault.TestClusterOptions{ HandlerFunc: vaulthttp.Handler, NumCores: 1, @@ -144,7 +157,8 @@ func testVaultServerCoreConfig(tb testing.TB, coreConfig *vault.CoreConfig) (*ap } // testVaultServerCoreConfig creates a new vault cluster with the given core -// configuration. This is a lower-level test helper. +// configuration. This is a lower-level test helper. If the seal config supports recovery keys, then +// recovery keys are returned. Otherwise, unseal keys are returned func testVaultServerCoreConfigWithOpts(tb testing.TB, coreConfig *vault.CoreConfig, opts *vault.TestClusterOptions) (*api.Client, []string, func()) { tb.Helper() @@ -159,14 +173,24 @@ func testVaultServerCoreConfigWithOpts(tb testing.TB, coreConfig *vault.CoreConf client := cluster.Cores[0].Client client.SetToken(cluster.RootToken) - // Convert the unseal keys to base64 encoded, since these are how the user - // will get them. - unsealKeys := make([]string, len(cluster.BarrierKeys)) - for i := range unsealKeys { - unsealKeys[i] = base64.StdEncoding.EncodeToString(cluster.BarrierKeys[i]) + var keys [][]byte + if coreConfig.Seal != nil && coreConfig.Seal.RecoveryKeySupported() { + keys = cluster.RecoveryKeys + } else { + keys = cluster.BarrierKeys } - return client, unsealKeys, func() { defer cluster.Cleanup() } + return client, encodeKeys(keys), cluster.Cleanup +} + +// Convert the unseal keys to base64 encoded, since these are how the user +// will get them. +func encodeKeys(rawKeys [][]byte) []string { + keys := make([]string, len(rawKeys)) + for i := range rawKeys { + keys[i] = base64.StdEncoding.EncodeToString(rawKeys[i]) + } + return keys } // testVaultServerUninit creates an uninitialized server. diff --git a/command/operator_rekey.go b/command/operator_rekey.go index b73349405..c15bff811 100644 --- a/command/operator_rekey.go +++ b/command/operator_rekey.go @@ -20,6 +20,11 @@ var ( _ cli.CommandAutocomplete = (*OperatorRekeyCommand)(nil) ) +const ( + keyTypeRecovery = "Recovery" + keyTypeUnseal = "Unseal" +) + type OperatorRekeyCommand struct { *BaseCommand @@ -58,6 +63,9 @@ Usage: vault operator rekey [options] [KEY] the command. If key is specified as "-", the command will read from stdin. If a TTY is available, the command will prompt for text. + If the flag -target=recovery is supplied, then this operation will require a + quorum of recovery keys in order to generate a new set of recovery keys. + Initialize a rekey: $ vault operator rekey \ @@ -112,7 +120,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Target: &c.flagCancel, Default: false, Usage: "Reset the rekeying progress. This will discard any submitted " + - "unseal keys or configuration.", + "unseal keys, recovery keys, or configuration.", }) f.BoolVar(&BoolVar{ @@ -120,7 +128,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Target: &c.flagStatus, Default: false, Usage: "Print the status of the current attempt without providing an " + - "unseal key.", + "unseal or recovery key.", }) f.IntVar(&IntVar{ @@ -130,7 +138,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Default: 5, Completion: complete.PredictAnything, Usage: "Number of key shares to split the generated root key into. " + - "This is the number of \"unseal keys\" to generate.", + "This is the number of \"unseal keys\" or \"recovery keys\" to generate.", }) f.IntVar(&IntVar{ @@ -150,7 +158,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { EnvVar: "", Completion: complete.PredictAnything, Usage: "Nonce value provided at initialization. The same nonce value " + - "must be provided with each unseal key.", + "must be provided with each unseal or recovery key.", }) f.StringVar(&StringVar{ @@ -179,7 +187,7 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Usage: "Comma-separated list of paths to files on disk containing " + "public PGP keys OR a comma-separated list of Keybase usernames using " + "the format \"keybase:\". When supplied, the generated " + - "unseal keys will be encrypted and base64-encoded in the order " + + "unseal or recovery keys will be encrypted and base64-encoded in the order " + "specified in this list.", }) @@ -189,25 +197,25 @@ func (c *OperatorRekeyCommand) Flags() *FlagSets { Name: "backup", Target: &c.flagBackup, Default: false, - Usage: "Store a backup of the current PGP encrypted unseal keys in " + + Usage: "Store a backup of the current PGP encrypted unseal or recovery keys in " + "Vault's core. The encrypted values can be recovered in the event of " + "failure or discarded after success. See the -backup-delete and " + "-backup-retrieve options for more information. This option only " + - "applies when the existing unseal keys were PGP encrypted.", + "applies when the existing unseal or recovery keys were PGP encrypted.", }) f.BoolVar(&BoolVar{ Name: "backup-delete", Target: &c.flagBackupDelete, Default: false, - Usage: "Delete any stored backup unseal keys.", + Usage: "Delete any stored backup unseal or recovery keys.", }) f.BoolVar(&BoolVar{ Name: "backup-retrieve", Target: &c.flagBackupRetrieve, Default: false, - Usage: "Retrieve the backed-up unseal keys. This option is only available " + + Usage: "Retrieve the backed-up unseal or recovery keys. This option is only available " + "if the PGP keys were provided and the backup has not been deleted.", }) @@ -268,10 +276,12 @@ func (c *OperatorRekeyCommand) Run(args []string) int { func (c *OperatorRekeyCommand) init(client *api.Client) int { // Handle the different API requests var fn func(*api.RekeyInitRequest) (*api.RekeyStatusResponse, error) + keyTypeRequired := keyTypeUnseal switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { case "barrier": fn = client.Sys().RekeyInit case "recovery", "hsm": + keyTypeRequired = keyTypeRecovery fn = client.Sys().RekeyRecoveryKeyInit default: c.UI.Error(fmt.Sprintf("Unknown target: %s", c.flagTarget)) @@ -295,25 +305,25 @@ func (c *OperatorRekeyCommand) init(client *api.Client) int { if len(c.flagPGPKeys) == 0 { if Format(c.UI) == "table" { c.UI.Warn(wrapAtLength( - "WARNING! If you lose the keys after they are returned, there is no " + - "recovery. Consider canceling this operation and re-initializing " + - "with the -pgp-keys flag to protect the returned unseal keys along " + - "with -backup to allow recovery of the encrypted keys in case of " + - "emergency. You can delete the stored keys later using the -delete " + - "flag.")) + fmt.Sprintf("WARNING! If you lose the keys after they are returned, there is no "+ + "recovery. Consider canceling this operation and re-initializing "+ + "with the -pgp-keys flag to protect the returned %s keys along "+ + "with -backup to allow recovery of the encrypted keys in case of "+ + "emergency. You can delete the stored keys later using the -delete "+ + "flag.", strings.ToLower(keyTypeRequired)))) c.UI.Output("") } } if len(c.flagPGPKeys) > 0 && !c.flagBackup { if Format(c.UI) == "table" { c.UI.Warn(wrapAtLength( - "WARNING! You are using PGP keys for encrypted the resulting unseal " + - "keys, but you did not enable the option to backup the keys to " + - "Vault's core. If you lose the encrypted keys after they are " + - "returned, you will not be able to recover them. Consider canceling " + - "this operation and re-running with -backup to allow recovery of the " + - "encrypted unseal keys in case of emergency. You can delete the " + - "stored keys later using the -delete flag.")) + fmt.Sprintf("WARNING! You are using PGP keys for encrypted the resulting %s "+ + "keys, but you did not enable the option to backup the keys to "+ + "Vault's core. If you lose the encrypted keys after they are "+ + "returned, you will not be able to recover them. Consider canceling "+ + "this operation and re-running with -backup to allow recovery of the "+ + "encrypted unseal keys in case of emergency. You can delete the "+ + "stored keys later using the -delete flag.", strings.ToLower(keyTypeRequired)))) c.UI.Output("") } } @@ -358,7 +368,7 @@ func (c *OperatorRekeyCommand) cancel(client *api.Client) int { func (c *OperatorRekeyCommand) provide(client *api.Client, key string) int { var statusFn func() (interface{}, error) var updateFn func(string, string) (interface{}, error) - + keyTypeRequired := keyTypeUnseal switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { case "barrier": statusFn = func() (interface{}, error) { @@ -376,6 +386,7 @@ func (c *OperatorRekeyCommand) provide(client *api.Client, key string) int { } } case "recovery", "hsm": + keyTypeRequired = keyTypeRecovery statusFn = func() (interface{}, error) { return client.Sys().RekeyRecoveryKeyStatus() } @@ -448,7 +459,7 @@ func (c *OperatorRekeyCommand) provide(client *api.Client, key string) int { // Nonce value is not required if we are prompting via the terminal w := getWriterFromUI(c.UI) fmt.Fprintf(w, "Rekey operation nonce: %s\n", nonce) - fmt.Fprintf(w, "Unseal Key (will be hidden): ") + fmt.Fprintf(w, "%s Key (will be hidden): ", keyTypeRequired) key, err = password.Read(os.Stdin) fmt.Fprintf(w, "\n") if err != nil { @@ -458,11 +469,11 @@ func (c *OperatorRekeyCommand) provide(client *api.Client, key string) int { } c.UI.Error(wrapAtLength(fmt.Sprintf("An error occurred attempting to "+ - "ask for the unseal key. The raw error message is shown below, but "+ + "ask for the %s key. The raw error message is shown below, but "+ "usually this is because you attempted to pipe a value into the "+ "command or you are executing outside of a terminal (tty). If you "+ "want to pipe the value, pass \"-\" as the argument to read from "+ - "stdin. The raw error was: %s", err))) + "stdin. The raw error was: %s", strings.ToLower(keyTypeRequired), err))) return 1 } default: // Supplied directly as an arg @@ -697,7 +708,7 @@ func (c *OperatorRekeyCommand) printUnsealKeys(client *api.Client, status *api.R ))) case "recovery", "hsm": c.UI.Output(wrapAtLength(fmt.Sprintf( - "The encrypted unseal keys are backed up to \"core/recovery-keys-backup\" " + + "The encrypted recovery keys are backed up to \"core/recovery-keys-backup\" " + "in the storage backend. Remove these keys at any time using " + "\"vault operator rekey -backup-delete -target=recovery\". Vault does not automatically " + "remove these keys.", @@ -708,33 +719,56 @@ func (c *OperatorRekeyCommand) printUnsealKeys(client *api.Client, status *api.R switch status.VerificationRequired { case false: c.UI.Output("") - c.UI.Output(wrapAtLength(fmt.Sprintf( - "Vault rekeyed with %d key shares and a key threshold of %d. Please "+ - "securely distribute the key shares printed above. When Vault is "+ - "re-sealed, restarted, or stopped, you must supply at least %d of "+ - "these keys to unseal it before it can start servicing requests.", - status.N, - status.T, - status.T))) + switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { + case "barrier": + c.UI.Output(wrapAtLength(fmt.Sprintf( + "Vault unseal keys rekeyed with %d key shares and a key threshold of %d. Please "+ + "securely distribute the key shares printed above. When Vault is "+ + "re-sealed, restarted, or stopped, you must supply at least %d of "+ + "these keys to unseal it before it can start servicing requests.", + status.N, + status.T, + status.T))) + case "recovery", "hsm": + c.UI.Output(wrapAtLength(fmt.Sprintf( + "Vault recovery keys rekeyed with %d key shares and a key threshold of %d. Please "+ + "securely distribute the key shares printed above.", + status.N, + status.T))) + } + default: c.UI.Output("") - c.UI.Output(wrapAtLength(fmt.Sprintf( - "Vault has created a new key, split into %d key shares and a key threshold "+ - "of %d. These will not be active until after verification is complete. "+ - "Please securely distribute the key shares printed above. When Vault "+ - "is re-sealed, restarted, or stopped, you must supply at least %d of "+ - "these keys to unseal it before it can start servicing requests.", - status.N, - status.T, - status.T))) + var warningText string + switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { + case "barrier": + c.UI.Output(wrapAtLength(fmt.Sprintf( + "Vault has created a new unseal key, split into %d key shares and a key threshold "+ + "of %d. These will not be active until after verification is complete. "+ + "Please securely distribute the key shares printed above. When Vault "+ + "is re-sealed, restarted, or stopped, you must supply at least %d of "+ + "these keys to unseal it before it can start servicing requests.", + status.N, + status.T, + status.T))) + warningText = "unseal" + case "recovery", "hsm": + c.UI.Output(wrapAtLength(fmt.Sprintf( + "Vault has created a new recovery key, split into %d key shares and a key threshold "+ + "of %d. These will not be active until after verification is complete. "+ + "Please securely distribute the key shares printed above.", + status.N, + status.T))) + warningText = "authenticate with" + + } c.UI.Output("") - c.UI.Warn(wrapAtLength( - "Again, these key shares are _not_ valid until verification is performed. " + - "Do not lose or discard your current key shares until after verification " + - "is complete or you will be unable to unseal Vault. If you cancel the " + - "rekey process or seal Vault before verification is complete the new " + - "shares will be discarded and the current shares will remain valid.", - )) + c.UI.Warn(wrapAtLength(fmt.Sprintf( + "Again, these key shares are _not_ valid until verification is performed. "+ + "Do not lose or discard your current key shares until after verification "+ + "is complete or you will be unable to %s Vault. If you cancel the "+ + "rekey process or seal Vault before verification is complete the new "+ + "shares will be discarded and the current shares will remain valid.", warningText))) c.UI.Output("") c.UI.Warn(wrapAtLength( "The current verification status, including initial nonce, is shown below.", diff --git a/command/operator_rekey_test.go b/command/operator_rekey_test.go index 31617e5ac..08285aa34 100644 --- a/command/operator_rekey_test.go +++ b/command/operator_rekey_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" + "github.com/hashicorp/vault/sdk/helper/roottoken" + "github.com/hashicorp/vault/api" "github.com/mitchellh/cli" ) @@ -254,6 +256,83 @@ func TestOperatorRekeyCommand_Run(t *testing.T) { } }) + t.Run("provide_arg_recovery_keys", func(t *testing.T) { + t.Parallel() + + client, keys, closer := testVaultServerAutoUnseal(t) + defer closer() + + // Initialize a rekey + status, err := client.Sys().RekeyRecoveryKeyInit(&api.RekeyInitRequest{ + SecretShares: 1, + SecretThreshold: 1, + }) + if err != nil { + t.Fatal(err) + } + nonce := status.Nonce + + // Supply the first n-1 recovery keys + for _, key := range keys[:len(keys)-1] { + ui, cmd := testOperatorRekeyCommand(t) + cmd.client = client + + code := cmd.Run([]string{ + "-nonce", nonce, + "-target", "recovery", + key, + }) + if exp := 0; code != exp { + t.Errorf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) + } + } + + ui, cmd := testOperatorRekeyCommand(t) + cmd.client = client + + code := cmd.Run([]string{ + "-nonce", nonce, + "-target", "recovery", + keys[len(keys)-1], // the last recovery key + }) + if exp := 0; code != exp { + t.Errorf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) + } + + re := regexp.MustCompile(`Key 1: (.+)`) + output := ui.OutputWriter.String() + match := re.FindAllStringSubmatch(output, -1) + if len(match) < 1 || len(match[0]) < 2 { + t.Fatalf("bad match: %#v", match) + } + recoveryKey := match[0][1] + + if strings.Contains(strings.ToLower(output), "unseal key") { + t.Fatalf(`output %s shouldn't contain "unseal key"`, output) + } + + // verify that we can perform operations with the recovery key + // below we generate a root token using the recovery key + rootStatus, err := client.Sys().GenerateRootStatus() + if err != nil { + t.Fatal(err) + } + otp, err := roottoken.GenerateOTP(rootStatus.OTPLength) + if err != nil { + t.Fatal(err) + } + genRoot, err := client.Sys().GenerateRootInit(otp, "") + if err != nil { + t.Fatal(err) + } + r, err := client.Sys().GenerateRootUpdate(recoveryKey, genRoot.Nonce) + if err != nil { + t.Fatal(err) + } + if !r.Complete { + t.Fatal("expected root update to be complete") + } + }) t.Run("provide_arg", func(t *testing.T) { t.Parallel() @@ -392,6 +471,94 @@ func TestOperatorRekeyCommand_Run(t *testing.T) { } }) + t.Run("provide_stdin_recovery_keys", func(t *testing.T) { + t.Parallel() + + client, keys, closer := testVaultServerAutoUnseal(t) + defer closer() + + // Initialize a rekey + status, err := client.Sys().RekeyRecoveryKeyInit(&api.RekeyInitRequest{ + SecretShares: 1, + SecretThreshold: 1, + }) + if err != nil { + t.Fatal(err) + } + nonce := status.Nonce + for _, key := range keys[:len(keys)-1] { + stdinR, stdinW := io.Pipe() + go func() { + _, _ = stdinW.Write([]byte(key)) + _ = stdinW.Close() + }() + + ui, cmd := testOperatorRekeyCommand(t) + cmd.client = client + cmd.testStdin = stdinR + + code := cmd.Run([]string{ + "-target", "recovery", + "-nonce", nonce, + "-", + }) + if exp := 0; code != exp { + t.Errorf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) + } + } + + stdinR, stdinW := io.Pipe() + go func() { + _, _ = stdinW.Write([]byte(keys[len(keys)-1])) // the last recovery key + _ = stdinW.Close() + }() + + ui, cmd := testOperatorRekeyCommand(t) + cmd.client = client + cmd.testStdin = stdinR + + code := cmd.Run([]string{ + "-nonce", nonce, + "-target", "recovery", + "-", + }) + if exp := 0; code != exp { + t.Errorf("expected %d to be %d: %s", code, exp, ui.ErrorWriter.String()) + } + + re := regexp.MustCompile(`Key 1: (.+)`) + output := ui.OutputWriter.String() + match := re.FindAllStringSubmatch(output, -1) + if len(match) < 1 || len(match[0]) < 2 { + t.Fatalf("bad match: %#v", match) + } + recoveryKey := match[0][1] + + if strings.Contains(strings.ToLower(output), "unseal key") { + t.Fatalf(`output %s shouldn't contain "unseal key"`, output) + } + // verify that we can perform operations with the recovery key + // below we generate a root token using the recovery key + rootStatus, err := client.Sys().GenerateRootStatus() + if err != nil { + t.Fatal(err) + } + otp, err := roottoken.GenerateOTP(rootStatus.OTPLength) + if err != nil { + t.Fatal(err) + } + genRoot, err := client.Sys().GenerateRootInit(otp, "") + if err != nil { + t.Fatal(err) + } + r, err := client.Sys().GenerateRootUpdate(recoveryKey, genRoot.Nonce) + if err != nil { + t.Fatal(err) + } + if !r.Complete { + t.Fatal("expected root update to be complete") + } + }) t.Run("backup", func(t *testing.T) { t.Parallel()