Vault Community Edition rekey and recovery key operations can cause denial of service in github.com/hashicorp/vault

@@ -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()

@@ -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 {

@@ -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)

@@ -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)

@@ -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"`
+}

@@ -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{}{

@@ -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
+}

@@ -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)
+		})
+	}
+}