diff --git a/.changelog/24766.txt b/.changelog/24766.txt new file mode 100644 index 000000000..c173f439c --- /dev/null +++ b/.changelog/24766.txt @@ -0,0 +1,3 @@ +```release-note:improvement +keyring: Warn if deleting a key previously used to encrypt an existing variable +``` diff --git a/api/keyring.go b/api/keyring.go index 69e0c9c10..f528b3261 100644 --- a/api/keyring.go +++ b/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 diff --git a/api/keyring_test.go b/api/keyring_test.go index 9320aaf9d..cabbf3482 100644 --- a/api/keyring_test.go +++ b/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) diff --git a/command/agent/keyring_endpoint.go b/command/agent/keyring_endpoint.go index 870aae3f0..28170a1a1 100644 --- a/command/agent/keyring_endpoint.go +++ b/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 diff --git a/command/agent/keyring_endpoint_test.go b/command/agent/keyring_endpoint_test.go index 428da9d50..2c99be9cf 100644 --- a/command/agent/keyring_endpoint_test.go +++ b/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) diff --git a/command/operator_root_keyring_remove.go b/command/operator_root_keyring_remove.go index 942c351b0..b3afc92c1 100644 --- a/command/operator_root_keyring_remove.go +++ b/command/operator_root_keyring_remove.go @@ -29,7 +29,14 @@ Usage: nomad operator root keyring remove [options] 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)) diff --git a/nomad/keyring_endpoint.go b/nomad/keyring_endpoint.go index 692cb84ed..018f7f638 100644 --- a/nomad/keyring_endpoint.go +++ b/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 diff --git a/nomad/keyring_endpoint_test.go b/nomad/keyring_endpoint_test.go index 071d17686..5b28d65ee 100644 --- a/nomad/keyring_endpoint_test.go +++ b/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) }) } diff --git a/nomad/structs/keyring.go b/nomad/structs/keyring.go index a858a771a..5dc85a05a 100644 --- a/nomad/structs/keyring.go +++ b/nomad/structs/keyring.go @@ -500,6 +500,7 @@ type KeyringUpdateRootKeyMetaResponse struct { type KeyringDeleteRootKeyRequest struct { KeyID string + Force bool WriteRequest } diff --git a/website/content/api-docs/operator/keyring.mdx b/website/content/api-docs/operator/keyring.mdx index 7080d4828..feeb0b237 100644 --- a/website/content/api-docs/operator/keyring.mdx +++ b/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 diff --git a/website/content/docs/commands/operator/root/keyring-remove.mdx b/website/content/docs/commands/operator/root/keyring-remove.mdx index bcf701378..252e6d741 100644 --- a/website/content/docs/commands/operator/root/keyring-remove.mdx +++ b/website/content/docs/commands/operator/root/keyring-remove.mdx @@ -23,6 +23,12 @@ nomad operator root keyring remove [options] @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