diff --git a/api/sys_rekey.go b/api/sys_rekey.go index 573201751c..95cb27ff0d 100644 --- a/api/sys_rekey.go +++ b/api/sys_rekey.go @@ -147,11 +147,29 @@ func (c *Sys) RekeyCancel() error { return c.RekeyCancelWithContext(context.Background()) } +func (c *Sys) RekeyCancelWithNonce(nonce string) error { + return c.RekeyCancelWithContextWithNonce(context.Background(), nonce) +} + func (c *Sys) RekeyCancelWithContext(ctx context.Context) error { + return c.RekeyCancelWithContextWithNonce(ctx, "") +} + +func (c *Sys) RekeyCancelWithContextWithNonce(ctx context.Context, nonce string) error { ctx, cancelFunc := c.c.withConfiguredTimeout(ctx) defer cancelFunc() r := c.c.NewRequest(http.MethodDelete, "/v1/sys/rekey/init") + if nonce != "" { + body := map[string]interface{}{ + "nonce": nonce, + } + + if err := r.SetJSONBody(body); err != nil { + return err + } + + } resp, err := c.c.rawRequestWithContext(ctx, r) if err == nil { @@ -164,12 +182,28 @@ func (c *Sys) RekeyRecoveryKeyCancel() error { return c.RekeyRecoveryKeyCancelWithContext(context.Background()) } +func (c *Sys) RekeyRecoveryKeyCancelWithNonce(nonce string) error { + return c.RekeyRecoveryKeyCancelWithContextWithNonce(context.Background(), nonce) +} + func (c *Sys) RekeyRecoveryKeyCancelWithContext(ctx context.Context) error { + return c.RekeyCancelWithContextWithNonce(ctx, "") +} + +func (c *Sys) RekeyRecoveryKeyCancelWithContextWithNonce(ctx context.Context, nonce string) error { ctx, cancelFunc := c.c.withConfiguredTimeout(ctx) defer cancelFunc() - r := c.c.NewRequest(http.MethodDelete, "/v1/sys/rekey-recovery-key/init") + if nonce != "" { + body := map[string]interface{}{ + "nonce": nonce, + } + + if err := r.SetJSONBody(body); err != nil { + return err + } + } resp, err := c.c.rawRequestWithContext(ctx, r) if err == nil { defer resp.Body.Close() diff --git a/changelog/30794.txt b/changelog/30794.txt new file mode 100644 index 0000000000..5109a56002 --- /dev/null +++ b/changelog/30794.txt @@ -0,0 +1,3 @@ +```release-note:security +core: require a nonce when cancelling a rekey operation that was initiated within the last 10 minutes. +``` \ No newline at end of file diff --git a/command/operator_rekey.go b/command/operator_rekey.go index 9b48415682..1b2f0f6f1f 100644 --- a/command/operator_rekey.go +++ b/command/operator_rekey.go @@ -337,6 +337,15 @@ func (c *OperatorRekeyCommand) init(client *api.Client) int { // cancel is used to abort the rekey process. func (c *OperatorRekeyCommand) cancel(client *api.Client) int { + if c.flagNonce != "" && c.flagVerify { + c.UI.Error("The -nonce flag is not valid with the -verify flag") + return 1 + } + + if c.flagNonce != "" { + return c.cancelWithNonce(client) + } + // Handle the different API requests var fn func() error switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { @@ -366,6 +375,29 @@ func (c *OperatorRekeyCommand) cancel(client *api.Client) int { return 0 } +func (c *OperatorRekeyCommand) cancelWithNonce(client *api.Client) int { + var fn func(nonce string) error + switch strings.ToLower(strings.TrimSpace(c.flagTarget)) { + case "barrier": + fn = client.Sys().RekeyCancelWithNonce + case "recovery", "hsm": + fn = client.Sys().RekeyRecoveryKeyCancelWithNonce + + default: + c.UI.Error(fmt.Sprintf("Unknown target: %s", c.flagTarget)) + return 1 + } + + // Make the request + if err := fn(c.flagNonce); err != nil { + c.UI.Error(fmt.Sprintf("Error canceling rekey: %s", err)) + return 2 + } + + c.UI.Output("Success! Canceled rekeying (if it was started)") + return 0 +} + // provide prompts the user for the seal key and posts it to the update root // endpoint. If this is the last unseal, this function outputs it. func (c *OperatorRekeyCommand) provide(client *api.Client, key string) int { diff --git a/command/operator_rekey_test.go b/command/operator_rekey_test.go index d8a4ee2537..821f2bcaab 100644 --- a/command/operator_rekey_test.go +++ b/command/operator_rekey_test.go @@ -67,6 +67,16 @@ func TestOperatorRekeyCommand_Run(t *testing.T) { "incorrect number", 2, }, + { + "cancel_verify_nonce", + []string{ + "-cancel", + "-verify", + "-nonce", "abcd", + }, + "The -nonce flag is not valid with the -verify flag", + 1, + }, } t.Run("validations", func(t *testing.T) { @@ -152,10 +162,11 @@ func TestOperatorRekeyCommand_Run(t *testing.T) { defer closer() // Initialize a rekey - if _, err := client.Sys().RekeyInit(&api.RekeyInitRequest{ + init, err := client.Sys().RekeyInit(&api.RekeyInitRequest{ SecretShares: 1, SecretThreshold: 1, - }); err != nil { + }) + if err != nil { t.Fatal(err) } @@ -163,7 +174,7 @@ func TestOperatorRekeyCommand_Run(t *testing.T) { cmd.client = client code := cmd.Run([]string{ - "-cancel", + "-cancel", "-nonce", init.Nonce, }) if exp := 0; code != exp { t.Errorf("expected %d to be %d", code, exp) diff --git a/http/http_test.go b/http/http_test.go index addd423b61..df02e9978a 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -32,6 +32,10 @@ func testHttpDelete(t *testing.T, token string, addr string) *http.Response { return testHttpData(t, "DELETE", token, addr, "", nil, false, 0, false) } +func testHttpDeleteData(t *testing.T, token string, addr string, body interface{}) *http.Response { + return testHttpData(t, "DELETE", token, addr, "", body, false, 0, false) +} + // Go 1.8+ clients redirect automatically which breaks our 307 standby testing func testHttpDeleteDisableRedirect(t *testing.T, token string, addr string) *http.Response { return testHttpData(t, "DELETE", token, addr, "", nil, true, 0, false) diff --git a/http/sys_rekey.go b/http/sys_rekey.go index a43da4f1df..5d344b808c 100644 --- a/http/sys_rekey.go +++ b/http/sys_rekey.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "net/http" + "time" "github.com/hashicorp/vault/helper/pgpkeys" "github.com/hashicorp/vault/sdk/helper/consts" @@ -134,6 +135,7 @@ func handleSysRekeyInitPut(ctx context.Context, core *vault.Core, recovery bool, PGPKeys: req.PGPKeys, Backup: req.Backup, VerificationRequired: req.RequireVerification, + Created: time.Now().UTC(), }, recovery) if err != nil { respondError(w, err.Code(), err) @@ -144,7 +146,13 @@ func handleSysRekeyInitPut(ctx context.Context, core *vault.Core, recovery bool, } func handleSysRekeyInitDelete(ctx context.Context, core *vault.Core, recovery bool, w http.ResponseWriter, r *http.Request) { - if err := core.RekeyCancel(recovery); err != nil { + var req RekeyDeleteRequest + if _, err := parseJSONRequest(core.PerfStandby(), r, w, &req); err != nil { + respondError(w, http.StatusBadRequest, err) + return + } + + if err := core.RekeyCancel(recovery, req.Nonce, 10*time.Minute); err != nil { respondError(w, err.Code(), err) return } @@ -412,3 +420,8 @@ type RekeyVerificationUpdateResponse struct { Nonce string `json:"nonce"` Complete bool `json:"complete"` } + +type RekeyDeleteRequest struct { + Nonce string `json:"nonce"` + Key string `json:"key"` +} diff --git a/http/sys_rekey_test.go b/http/sys_rekey_test.go index e396644476..a185c48640 100644 --- a/http/sys_rekey_test.go +++ b/http/sys_rekey_test.go @@ -149,7 +149,7 @@ func TestSysRekey_Init_Cancel(t *testing.T) { defer cluster.Cleanup() cl := cluster.Cores[0].Client - _, err := cl.Logical().Write("sys/rekey/init", map[string]interface{}{ + initResp, err := cl.Logical().Write("sys/rekey/init", map[string]interface{}{ "secret_shares": 5, "secret_threshold": 3, }) @@ -157,7 +157,7 @@ func TestSysRekey_Init_Cancel(t *testing.T) { t.Fatalf("err: %s", err) } - _, err = cl.Logical().Delete("sys/rekey/init") + err = cl.Sys().RekeyCancelWithNonce(initResp.Data["nonce"].(string)) if err != nil { t.Fatalf("err: %s", err) } @@ -278,8 +278,12 @@ func TestSysRekey_ReInitUpdate(t *testing.T) { "secret_threshold": 3, }) testResponseStatus(t, resp, 200) + var initResp map[string]interface{} + testResponseBody(t, resp, &initResp) - resp = testHttpDelete(t, token, addr+"/v1/sys/rekey/init") + resp = testHttpDeleteData(t, token, addr+"/v1/sys/rekey/init", map[string]interface{}{ + "nonce": initResp["nonce"].(string), + }) testResponseStatus(t, resp, 204) resp = testHttpPut(t, token, addr+"/v1/sys/rekey/init", map[string]interface{}{ diff --git a/vault/rekey.go b/vault/rekey.go index 1f254eb0d7..62ce50e90b 100644 --- a/vault/rekey.go +++ b/vault/rekey.go @@ -11,6 +11,7 @@ import ( "encoding/json" "fmt" "net/http" + "time" aeadwrapper "github.com/hashicorp/go-kms-wrapping/wrappers/aead/v2" "github.com/hashicorp/go-uuid" @@ -236,6 +237,7 @@ func (c *Core) BarrierRekeyInit(config *SealConfig) logical.HTTPCodedError { return logical.CodedError(http.StatusInternalServerError, fmt.Errorf("error generating nonce for procedure: %w", err).Error()) } c.barrierRekeyConfig.Nonce = nonce + c.barrierRekeyConfig.Created = time.Now().UTC() if c.logger.IsInfo() { c.logger.Info("rekey initialized", "nonce", c.barrierRekeyConfig.Nonce, "shares", c.barrierRekeyConfig.SecretShares, "threshold", c.barrierRekeyConfig.SecretThreshold, "validation_required", c.barrierRekeyConfig.VerificationRequired) @@ -286,6 +288,7 @@ func (c *Core) RecoveryRekeyInit(config *SealConfig) logical.HTTPCodedError { return logical.CodedError(http.StatusInternalServerError, fmt.Errorf("error generating nonce for procedure: %w", err).Error()) } c.recoveryRekeyConfig.Nonce = nonce + c.recoveryRekeyConfig.Created = time.Now().UTC() if c.logger.IsInfo() { c.logger.Info("rekey initialized", "nonce", c.recoveryRekeyConfig.Nonce, "shares", c.recoveryRekeyConfig.SecretShares, "threshold", c.recoveryRekeyConfig.SecretThreshold, "validation_required", c.recoveryRekeyConfig.VerificationRequired) @@ -910,7 +913,7 @@ func (c *Core) RekeyVerify(ctx context.Context, key []byte, nonce string, recove } // RekeyCancel is used to cancel an in-progress rekey -func (c *Core) RekeyCancel(recovery bool) logical.HTTPCodedError { +func (c *Core) RekeyCancel(recovery bool, nonce string, requiresNonceDeadline time.Duration) logical.HTTPCodedError { c.stateLock.RLock() defer c.stateLock.RUnlock() if c.Sealed() { @@ -923,10 +926,26 @@ func (c *Core) RekeyCancel(recovery bool) logical.HTTPCodedError { c.rekeyLock.Lock() defer c.rekeyLock.Unlock() + validBarrierReq := func() bool { + return c.barrierRekeyConfig.Nonce == nonce || + rekeyCancelDeadlineIsMet(c.barrierRekeyConfig.Created, requiresNonceDeadline) + } + + validRecoveryReq := func() bool { + return c.recoveryRekeyConfig.Nonce == nonce || + rekeyCancelDeadlineIsMet(c.recoveryRekeyConfig.Created, requiresNonceDeadline) + } + // Clear any progress or config if recovery { + if c.recoveryRekeyConfig != nil && !validRecoveryReq() { + return logical.CodedError(http.StatusBadRequest, "invalid request") + } c.recoveryRekeyConfig = nil } else { + if c.barrierRekeyConfig != nil && !validBarrierReq() { + return logical.CodedError(http.StatusBadRequest, "invalid request") + } c.barrierRekeyConfig = nil } return nil @@ -1031,3 +1050,11 @@ func (c *Core) RekeyDeleteBackup(ctx context.Context, recovery bool) logical.HTT } return nil } + +func rekeyCancelDeadlineIsMet(created time.Time, deadline time.Duration) bool { + if created.IsZero() { + return false + } + passed := time.Now().UTC().Sub(created) >= deadline + return passed +} diff --git a/vault/rekey_test.go b/vault/rekey_test.go index 246042ea89..b00d0d4547 100644 --- a/vault/rekey_test.go +++ b/vault/rekey_test.go @@ -8,13 +8,16 @@ import ( "fmt" "reflect" "strings" + "sync" "testing" + "time" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/sdk/physical" "github.com/hashicorp/vault/sdk/physical/inmem" "github.com/hashicorp/vault/vault/seal" + "github.com/stretchr/testify/require" ) func TestCore_Rekey_Lifecycle(t *testing.T) { @@ -57,7 +60,7 @@ func testCore_Rekey_Lifecycle_Common(t *testing.T, c *Core, recovery bool) { } // Cancel should be idempotent - err = c.RekeyCancel(false) + err = c.RekeyCancel(false, "", 10*time.Minute) if err != nil { t.Fatalf("err: %v", err) } @@ -83,7 +86,7 @@ func testCore_Rekey_Lifecycle_Common(t *testing.T, c *Core, recovery bool) { } // Cancel should be clear - err = c.RekeyCancel(recovery) + err = c.RekeyCancel(recovery, conf.Nonce, 10*time.Minute) if err != nil { t.Fatalf("err: %v", err) } @@ -548,3 +551,195 @@ func TestSysRekey_Verification_Invalid(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +// TestCancelRekey_Nonce verifies that cancelling a rekey operation requires a +// nonce +func TestCancelRekey_Nonce(t *testing.T) { + t.Parallel() + tests := []struct { + recovery bool + config *SealConfig + core func(t *testing.T) *Core + }{ + { + recovery: true, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + Type: string(SealConfigTypeMultiseal), + }, + core: func(t *testing.T) *Core { + c, _, _, _ := TestCoreUnsealedWithConfigSealOpts(t, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &seal.TestSealOpts{StoredKeys: seal.StoredKeysSupportedGeneric}) + return c + }, + }, + { + recovery: false, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + StoredShares: 1, + Type: string(SealConfigTypeShamir), + }, + core: func(t *testing.T) *Core { + c, _, _ := TestCoreUnsealed(t) + return c + }, + }, + } + + for _, tc := range tests { + t.Run(tc.config.Type, func(t *testing.T) { + c := tc.core(t) + // bail if recovery rekey is not supported + if tc.recovery && !c.seal.RecoveryKeySupported() { + t.Skip(t, "recovery rekey not supported") + } + + err := c.RekeyInit(tc.config, tc.recovery) + require.NoError(t, err, "rekey init failed") + + // try to cancel without the nonce + err = c.RekeyCancel(tc.recovery, "", 10*time.Minute) + require.Error(t, err, "cancel should have errored") + + // retrieve the nonce + var nonce string + c.stateLock.RLock() + c.rekeyLock.RLock() + if tc.recovery { + nonce = c.recoveryRekeyConfig.Nonce + } else { + nonce = c.barrierRekeyConfig.Nonce + } + c.rekeyLock.RUnlock() + c.stateLock.RUnlock() + + require.NotEmpty(t, nonce, "nonce missing") + + // cancel successfully + err = c.RekeyCancel(tc.recovery, nonce, 10*time.Minute) + require.NoError(t, err, "error on rekey cancel") + }) + } +} + +// TestCancelRekey_Regression creates 50 cancel requests in parallel and then +// starts a rekey operation. The test verifies that the spammed cancel requests +// are not able to cancel the rekey, because they do not provide a nonce. +func TestCancelRekey_Regression(t *testing.T) { + t.Parallel() + testCases := []struct { + recovery bool + config *SealConfig + core func(t *testing.T) *Core + }{ + { + recovery: true, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + Type: string(SealConfigTypeMultiseal), + }, + core: func(t *testing.T) *Core { + c, _, _, _ := TestCoreUnsealedWithConfigSealOpts(t, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &seal.TestSealOpts{StoredKeys: seal.StoredKeysSupportedGeneric}) + return c + }, + }, + { + recovery: false, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + StoredShares: 1, + Type: string(SealConfigTypeShamir), + }, + core: func(t *testing.T) *Core { + c, _, _ := TestCoreUnsealed(t) + return c + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.config.Type, func(t *testing.T) { + c := tc.core(t) + wg := sync.WaitGroup{} + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + c.RekeyCancel(tc.recovery, "", 10*time.Minute) + }() + } + err := c.RekeyInit(tc.config, tc.recovery) + require.NoError(t, err) + + wg.Wait() + happening, keys, err := c.RekeyProgress(tc.recovery, false) + require.NoError(t, err) + require.True(t, happening) + require.Equal(t, 0, keys) + }) + } +} + +// TestCancelRekey_AfterDeadline verifies that cancelling a rekey after the deadline +// does not require a nonce. +func TestCancelRekey_AfterDeadline(t *testing.T) { + testCases := []struct { + recovery bool + config *SealConfig + core func(t *testing.T) *Core + }{ + { + recovery: true, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + Type: string(SealConfigTypeMultiseal), + }, + core: func(t *testing.T) *Core { + c, _, _, _ := TestCoreUnsealedWithConfigSealOpts(t, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &SealConfig{StoredShares: 1, SecretShares: 1, SecretThreshold: 1}, + &seal.TestSealOpts{StoredKeys: seal.StoredKeysSupportedGeneric}) + return c + }, + }, + { + recovery: false, + config: &SealConfig{ + SecretShares: 1, + SecretThreshold: 1, + StoredShares: 1, + Type: string(SealConfigTypeShamir), + }, + core: func(t *testing.T) *Core { + c, _, _ := TestCoreUnsealed(t) + return c + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.config.Type, func(t *testing.T) { + c := tc.core(t) + err := c.RekeyInit(tc.config, tc.recovery) + require.NoError(t, err) + + // ensure that that 10 ms have passed before we cancel + time.Sleep(10 * time.Millisecond) + // set the deadline to a microsecond, which means we won't need a + // nonce to cancel the rekey + err = c.RekeyCancel(tc.recovery, "", time.Microsecond) + require.NoError(t, err) + }) + } +} diff --git a/vault/seal_config.go b/vault/seal_config.go index cc81a983c7..0f01546c65 100644 --- a/vault/seal_config.go +++ b/vault/seal_config.go @@ -7,6 +7,7 @@ import ( "bytes" "encoding/base64" "fmt" + "time" "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/packet" @@ -65,6 +66,9 @@ type SealConfig struct { // Name is the name provided in the seal configuration to identify the seal Name string `json:"name" mapstructure:"name"` + + // Created is the time of creation in UTC + Created time.Time `json:"-"` } // Validate is used to sanity check the seal configuration diff --git a/website/content/api-docs/system/rekey-recovery-key.mdx b/website/content/api-docs/system/rekey-recovery-key.mdx index cca8dc3103..13021049e6 100644 --- a/website/content/api-docs/system/rekey-recovery-key.mdx +++ b/website/content/api-docs/system/rekey-recovery-key.mdx @@ -122,16 +122,35 @@ well as any progress made. This must be called to change the parameters of the rekey. Note: verification is still a part of a rekey. If rekeying is canceled during the verification flow, the current unseal keys remain valid. + + Clients can call the endpoint without authenticating to Vault. + + | Method | Path | | :------- | :----------------------------- | | `DELETE` | `/sys/rekey-recovery-key/init` | +### Parameters + +- `nonce` `(string: )` – Specifies the nonce of the rekey operation. If + the rekey was initialized within the last 10 minutes, you must provide the + nonce to cancel the operation. + +### Sample payload + +```json +{ + "nonce": "abcd1234..." +} +``` + ### Sample request ```shell-session $ curl \ --header "X-Vault-Token: ..." \ --request DELETE \ + --data @payload.json \ http://127.0.0.1:8200/v1/sys/rekey-recovery-key/init ``` diff --git a/website/content/api-docs/system/rekey.mdx b/website/content/api-docs/system/rekey.mdx index fc95fddc98..fe7225b645 100644 --- a/website/content/api-docs/system/rekey.mdx +++ b/website/content/api-docs/system/rekey.mdx @@ -122,16 +122,35 @@ well as any progress made. This must be called to change the parameters of the rekey. Note: verification is still a part of a rekey. If rekeying is canceled during the verification flow, the current unseal keys remain valid. + + Clients can call the endpoint without authenticating to Vault. + + | Method | Path | | :------- | :---------------- | | `DELETE` | `/sys/rekey/init` | +### Parameters + +- `nonce` `(string: )` – Specifies the nonce of the rekey operation. If + the rekey was initialized within the last 10 minutes, you must provide the + nonce to cancel the operation. + +### Sample payload + +```json +{ + "nonce": "abcd1234..." +} +``` + ### Sample request ```shell-session $ curl \ --header "X-Vault-Token: ..." \ --request DELETE \ + --data @payload.json \ http://127.0.0.1:8200/v1/sys/rekey/init ``` diff --git a/website/content/docs/updates/important-changes.mdx b/website/content/docs/updates/important-changes.mdx index 43337db26a..8750d9c6ee 100644 --- a/website/content/docs/updates/important-changes.mdx +++ b/website/content/docs/updates/important-changes.mdx @@ -68,6 +68,20 @@ it must have a value for `disable_mlock`. | Performance Secondary | Yes | value depends on cluster specifics. [See docs](/vault/docs/configuration#disable_mlock) | DR Secondary | Yes | value depends on cluster specifics. [See docs](/vault/docs/configuration#disable_mlock) +## Rekey cancellations use a nonce ((#rekey-cancel-nonce)) +| Change | Affected version | Affected deployments +| ------------ | ---------------- | -------------------- +| Breaking | 1.20.0, 1.19.6, 1.18.11, 1.17.18, 1.16.21 | Any + +Vault 1.20.0, 1.19.6, 1.18.11, 1.17.18, and 1.16.21 require a nonce to cancel +[rekey](/vault/api-docs/system/rekey) and +[rekey recovery key](/vault/api-docs/system/rekey-recovery-key) operations +within 10 minutes of initializing a rekey request. Cancellation requests after +the 10 minute window do not require a nonce and succeed as expected. + +### Recommendation +To cancel a rekey operation, provide the nonce value from the +`/sys/rekey/init` or `sys/rekey-recovery-key/init` response. ## Transit support for Ed25519ph and Ed25519ctx signatures ((#ed25519))