keyring: warn if removing a key that was used for encrypting variables (#24766)

Adds an additional check in the Keyring.Delete RPC to make sure we're not
trying to delete a key that's been used to encrypt a variable. It also adds a
-force flag for the CLI/API to sidestep that check.

.changelog/24766.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+keyring: Warn if deleting a key previously used to encrypt an existing variable
+```

api/keyring.go

@@ -6,6 +6,7 @@ package api
 import (
 	"fmt"
 	"net/url"
+	"strconv"
 )
 
 // Keyring is used to access the Variables keyring.
@@ -60,14 +61,17 @@ func (k *Keyring) List(q *QueryOptions) ([]*RootKeyMeta, *QueryMeta, error) {
 
 // Delete deletes a specific inactive key from the keyring
 func (k *Keyring) Delete(opts *KeyringDeleteOptions, w *WriteOptions) (*WriteMeta, error) {
-	wm, err := k.client.delete(fmt.Sprintf("/v1/operator/keyring/key/%v",
-		url.PathEscape(opts.KeyID)), nil, nil, w)
+	wm, err := k.client.delete(fmt.Sprintf("/v1/operator/keyring/key/%v?force=%v",
+		url.PathEscape(opts.KeyID), strconv.FormatBool(opts.Force)), nil, nil, w)
 	return wm, err
 }
 
 // KeyringDeleteOptions are parameters for the Delete API
 type KeyringDeleteOptions struct {
 	KeyID string // UUID
+	// Force can be used to force deletion of a root keyring that was used to encrypt
+	// an existing variable or to sign a workload identity
+	Force bool
 }
 
 // Rotate requests a key rotation

api/keyring_test.go

@@ -37,8 +37,8 @@ func TestKeyring_CRUD(t *testing.T) {
 	assertQueryMeta(t, qm)
 	must.Len(t, 2, keys)
 
-	// Delete the old key
-	wm, err = kr.Delete(&KeyringDeleteOptions{KeyID: oldKeyID}, nil)
+	// Delete the old key with force
+	wm, err = kr.Delete(&KeyringDeleteOptions{KeyID: oldKeyID, Force: true}, nil)
 	must.NoError(t, err)
 	assertWriteMeta(t, wm)
 

command/agent/keyring_endpoint.go

@@ -116,9 +116,20 @@ func (s *HTTPServer) KeyringRequest(resp http.ResponseWriter, req *http.Request)
 		return s.keyringListRequest(resp, req)
 	case strings.HasPrefix(path, "key"):
 		keyID := strings.TrimPrefix(req.URL.Path, "/v1/operator/keyring/key/")
+
+		var forceBool bool
+		var err error
+		forceQuery, ok := req.URL.Query()["force"]
+		if ok {
+			forceBool, err = strconv.ParseBool(forceQuery[0])
+		}
+
+		if err != nil {
+			return nil, CodedError(422, "invalid force parameter")
+		}
 		switch req.Method {
 		case http.MethodDelete:
-			return s.keyringDeleteRequest(resp, req, keyID)
+			return s.keyringDeleteRequest(resp, req, keyID, forceBool)
 		default:
 			return nil, CodedError(405, ErrInvalidMethod)
 		}
@@ -185,9 +196,9 @@ func (s *HTTPServer) keyringRotateRequest(resp http.ResponseWriter, req *http.Re
 	return out, nil
 }
 
-func (s *HTTPServer) keyringDeleteRequest(resp http.ResponseWriter, req *http.Request, keyID string) (interface{}, error) {
+func (s *HTTPServer) keyringDeleteRequest(resp http.ResponseWriter, req *http.Request, keyID string, force bool) (interface{}, error) {
 
-	args := structs.KeyringDeleteRootKeyRequest{KeyID: keyID}
+	args := structs.KeyringDeleteRootKeyRequest{KeyID: keyID, Force: force}
 	s.parseWriteRequest(req, &args.WriteRequest)
 
 	var out structs.KeyringDeleteRootKeyResponse

command/agent/keyring_endpoint_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/go-jose/go-jose/v3"
+	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/shoenig/test/must"
 
 	"github.com/hashicorp/nomad/ci"
@@ -36,6 +37,13 @@ func TestHTTP_Keyring_CRUD(t *testing.T) {
 		must.Len(t, 1, listResp)
 		key0 := listResp[0].KeyID
 
+		// Create a variable to test force key deletion
+		state := s.server.State()
+		encryptedVar := mock.VariableEncrypted()
+		encryptedVar.KeyID = key0
+		varSetResp := state.VarSet(0, &structs.VarApplyStateRequest{Var: encryptedVar})
+		must.NoError(t, varSetResp.Error)
+
 		// Rotate
 
 		req, err = http.NewRequest(http.MethodPut, "/v1/operator/keyring/rotate", nil)
@@ -87,6 +95,12 @@ func TestHTTP_Keyring_CRUD(t *testing.T) {
 		req, err = http.NewRequest(http.MethodDelete, "/v1/operator/keyring/key/"+key0, nil)
 		must.NoError(t, err)
 		obj, err = s.Server.KeyringRequest(respW, req)
+		must.Error(t, err)
+		must.EqError(t, err, "root key in use, cannot delete")
+
+		req, err = http.NewRequest(http.MethodDelete, "/v1/operator/keyring/key/"+key0+"?force=true", nil)
+		must.NoError(t, err)
+		obj, err = s.Server.KeyringRequest(respW, req)
 		must.NoError(t, err)
 
 		req, err = http.NewRequest(http.MethodGet, "/v1/operator/keyring/keys", nil)

command/operator_root_keyring_remove.go

@@ -29,7 +29,14 @@ Usage: nomad operator root keyring remove [options] <key ID>
 
 General Options:
 
-  ` + generalOptionsUsage(usageOptsDefault|usageOptsNoNamespace)
+  ` + generalOptionsUsage(usageOptsDefault|usageOptsNoNamespace) + `
+
+Remove Options:
+
+  -force
+    Remove the key even if it was used to sign an existing variable
+    or workload identity.
+`
 
 	return strings.TrimSpace(helpText)
 }
@@ -51,11 +58,12 @@ func (c *OperatorRootKeyringRemoveCommand) Name() string {
 }
 
 func (c *OperatorRootKeyringRemoveCommand) Run(args []string) int {
-	var verbose bool
+	var force, verbose bool
 
 	flags := c.Meta.FlagSet("root keyring remove", FlagSetClient)
 	flags.Usage = func() { c.Ui.Output(c.Help()) }
 	flags.BoolVar(&verbose, "verbose", false, "")
+	flags.BoolVar(&force, "force", false, "Forces deletion of the root keyring even if it's in use.")
 
 	if err := flags.Parse(args); err != nil {
 		return 1
@@ -76,6 +84,7 @@ func (c *OperatorRootKeyringRemoveCommand) Run(args []string) int {
 	}
 	_, err = client.Keyring().Delete(&api.KeyringDeleteOptions{
 		KeyID: removeKey,
+		Force: force,
 	}, nil)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("error: %s", err))

nomad/keyring_endpoint.go

@@ -4,6 +4,7 @@
 package nomad
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -344,10 +345,24 @@ func (k *Keyring) Delete(args *structs.KeyringDeleteRootKeyRequest, reply *struc
 	if err != nil {
 		return err
 	}
+
+	if rootKey == nil {
+		return errors.New("root key not found")
+	}
+
 	if rootKey != nil && rootKey.IsActive() {
 		return fmt.Errorf("active root key cannot be deleted - call rotate first")
 	}
 
+	// make sure the key was used to encrypt an existing variable
+	rootKeyInUse, err := snap.IsRootKeyInUse(args.KeyID)
+	if err != nil {
+		return err
+	}
+	if rootKeyInUse && !args.Force {
+		return errors.New("root key in use, cannot delete")
+	}
+
 	_, index, err = k.srv.raftApply(structs.WrappedRootKeysDeleteRequestType, args)
 	if err != nil {
 		return err

nomad/keyring_endpoint_test.go

@@ -8,12 +8,11 @@ import (
 	"time"
 
 	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc/v2"
-	"github.com/shoenig/test/must"
-	"github.com/stretchr/testify/require"
-
 	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/shoenig/test/must"
 )
 
 // TestKeyringEndpoint_CRUD exercises the basic keyring operations
@@ -26,11 +25,11 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 	defer shutdown()
 	testutil.WaitForKeyring(t, srv.RPC, "global")
 	codec := rpcClient(t, srv)
+	state := srv.fsm.State()
 
 	// Upsert a new key
-
 	key, err := structs.NewUnwrappedRootKey(structs.EncryptionAlgorithmAES256GCM)
-	require.NoError(t, err)
+	must.NoError(t, err)
 	id := key.Meta.KeyID
 	key = key.MakeActive()
 
@@ -41,12 +40,19 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 	var updateResp structs.KeyringUpdateRootKeyResponse
 
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-	require.EqualError(t, err, structs.ErrPermissionDenied.Error())
+	must.EqError(t, err, structs.ErrPermissionDenied.Error())
 
 	updateReq.AuthToken = rootToken.SecretID
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-	require.NoError(t, err)
-	require.NotEqual(t, uint64(0), updateResp.Index)
+	must.NoError(t, err)
+	must.NotEq(t, uint64(0), updateResp.Index)
+
+	// Upsert a variable and encrypt it with that key (used for key deletion test
+	// below)
+	encryptedVar := mock.VariableEncrypted()
+	encryptedVar.KeyID = key.Meta.KeyID
+	varSetResp := state.VarSet(0, &structs.VarApplyStateRequest{Var: encryptedVar})
+	must.NoError(t, varSetResp.Error)
 
 	// Get doesn't need a token here because it uses mTLS role verification
 	getReq := &structs.KeyringGetRootKeyRequest{
@@ -56,9 +62,9 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 	var getResp structs.KeyringGetRootKeyResponse
 
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Get", getReq, &getResp)
-	require.NoError(t, err)
-	require.Equal(t, updateResp.Index, getResp.Index)
-	require.Equal(t, structs.EncryptionAlgorithmAES256GCM, getResp.Key.Meta.Algorithm)
+	must.NoError(t, err)
+	must.Eq(t, updateResp.Index, getResp.Index)
+	must.Eq(t, structs.EncryptionAlgorithmAES256GCM, getResp.Key.Meta.Algorithm)
 
 	// Make a blocking query for List and wait for an Update. Note
 	// that Get queries don't need ACL tokens in the test server
@@ -82,13 +88,13 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 
 	updateReq.RootKey.Meta.CreateTime = time.Now().UTC().UnixNano()
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-	require.NoError(t, err)
-	require.NotEqual(t, uint64(0), updateResp.Index)
+	must.NoError(t, err)
+	must.NotEq(t, uint64(0), updateResp.Index)
 
 	// wait for the blocking query to complete and check the response
-	require.NoError(t, <-errCh)
-	require.Equal(t, listResp.Index, updateResp.Index)
-	require.Len(t, listResp.Keys, 2) // bootstrap + new one
+	must.NoError(t, <-errCh)
+	must.Eq(t, listResp.Index, updateResp.Index)
+	must.SliceLen(t, 2, listResp.Keys) /// bootstrap + new one
 
 	// Delete the key and verify that it's gone
 
@@ -99,20 +105,25 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 	var delResp structs.KeyringDeleteRootKeyResponse
 
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Delete", delReq, &delResp)
-	require.EqualError(t, err, structs.ErrPermissionDenied.Error())
+	must.EqError(t, err, structs.ErrPermissionDenied.Error())
 
 	delReq.AuthToken = rootToken.SecretID
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Delete", delReq, &delResp)
-	require.EqualError(t, err, "active root key cannot be deleted - call rotate first")
+	must.EqError(t, err, "active root key cannot be deleted - call rotate first")
 
 	// set inactive
 	updateReq.RootKey = updateReq.RootKey.MakeInactive()
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Delete", delReq, &delResp)
-	require.NoError(t, err)
-	require.Greater(t, delResp.Index, getResp.Index)
+	must.EqError(t, err, "root key in use, cannot delete")
+
+	// delete with force
+	delReq.Force = true
+	err = msgpackrpc.CallWithCodec(codec, "Keyring.Delete", delReq, &delResp)
+	must.NoError(t, err)
+	must.Greater(t, getResp.Index, delResp.Index)
 
 	listReq := &structs.KeyringListRootKeyMetaRequest{
 		QueryOptions: structs.QueryOptions{
@@ -121,9 +132,9 @@ func TestKeyringEndpoint_CRUD(t *testing.T) {
 		},
 	}
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.List", listReq, &listResp)
-	require.NoError(t, err)
-	require.Greater(t, listResp.Index, getResp.Index)
-	require.Len(t, listResp.Keys, 1) // just the bootstrap key
+	must.NoError(t, err)
+	must.Greater(t, getResp.Index, listResp.Index)
+	must.SliceLen(t, 1, listResp.Keys) // just the bootstrap key
 }
 
 // TestKeyringEndpoint_validateUpdate exercises all the various
@@ -140,7 +151,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) {
 
 	// Setup an existing key
 	key, err := structs.NewUnwrappedRootKey(structs.EncryptionAlgorithmAES256GCM)
-	require.NoError(t, err)
+	must.NoError(t, err)
 	id := key.Meta.KeyID
 	key = key.MakeActive()
 
@@ -153,7 +164,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) {
 	}
 	var updateResp structs.KeyringUpdateRootKeyResponse
 	err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	testCases := []struct {
 		key            *structs.UnwrappedRootKey
@@ -211,7 +222,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) {
 			}
 			var updateResp structs.KeyringUpdateRootKeyResponse
 			err := msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp)
-			require.EqualError(t, err, tc.expectedErrMsg)
+			must.EqError(t, err, tc.expectedErrMsg)
 		})
 	}
 

nomad/structs/keyring.go

@@ -500,6 +500,7 @@ type KeyringUpdateRootKeyMetaResponse struct {
 
 type KeyringDeleteRootKeyRequest struct {
 	KeyID string
+	Force bool
 	WriteRequest
 }
 

website/content/api-docs/operator/keyring.mdx

@@ -233,6 +233,11 @@ The table below shows this endpoint's support for [blocking queries] and
 |------------------|--------------|
 | `NO`             | `management` |
 
+### Parameters
+
+- `force` `(bool: false)` - Remove the key even if it was used to sign an existing variable
+or workload identity.
+
 ### Sample Request
 
 ```shell-session

website/content/docs/commands/operator/root/keyring-remove.mdx

@@ -23,6 +23,12 @@ nomad operator root keyring remove [options] <key ID>
 
 @include 'general_options.mdx'
 
+## Remove Options
+
+- `-force`: Remove the key even if it was used to sign an existing variable
+or workload identity.
+
+
 ## Examples
 
 ```shell-session