From 5d17f9b142951b045f8505fecb7bc4ac47b43bd1 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 1 Feb 2023 10:09:16 -0500 Subject: [PATCH] Allow cleanup ssh dynamic keys host keys (#18939) * Add ability to clean up host keys for dynamic keys This adds a new endpoint, tidy/dynamic-keys that removes any stale host keys still present on the mount. This does not clean up any pending dynamic key leases and will not remove these keys from systems with authorized hosts entries created by Vault. Signed-off-by: Alexander Scheel * Add documentation Signed-off-by: Alexander Scheel * Add changelog entry Signed-off-by: Alexander Scheel --------- Signed-off-by: Alexander Scheel --- builtin/logical/ssh/backend.go | 3 +- builtin/logical/ssh/backend_test.go | 58 +++++++++++++++++++ .../ssh/path_cleanup_dynamic_host_keys.go | 42 ++++++++++++++ changelog/18939.txt | 3 + website/content/api-docs/secret/ssh.mdx | 45 ++++++++++++++ 5 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 builtin/logical/ssh/path_cleanup_dynamic_host_keys.go create mode 100644 changelog/18939.txt diff --git a/builtin/logical/ssh/backend.go b/builtin/logical/ssh/backend.go index b7b1b5acf..454937f16 100644 --- a/builtin/logical/ssh/backend.go +++ b/builtin/logical/ssh/backend.go @@ -47,7 +47,7 @@ func Backend(conf *logical.BackendConfig) (*backend, error) { SealWrapStorage: []string{ caPrivateKey, caPrivateKeyStoragePath, - "keys/", + keysStoragePrefix, }, }, @@ -62,6 +62,7 @@ func Backend(conf *logical.BackendConfig) (*backend, error) { pathSign(&b), pathIssue(&b), pathFetchPublicKey(&b), + pathCleanupKeys(&b), }, Secrets: []*framework.Secret{ diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 5b28e6ea2..6382d61d2 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/builtin/logical/ssh/backend_test.go @@ -23,6 +23,8 @@ import ( vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/vault" "github.com/mitchellh/mapstructure" + + "github.com/stretchr/testify/require" ) const ( @@ -2404,3 +2406,59 @@ func testCredsWrite(t *testing.T, roleName string, data map[string]interface{}, }, } } + +func TestBackend_CleanupDynamicHostKeys(t *testing.T) { + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + + b, err := Backend(config) + if err != nil { + t.Fatal(err) + } + err = b.Setup(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + // Running on a clean mount shouldn't do anything. + cleanRequest := &logical.Request{ + Operation: logical.DeleteOperation, + Path: "tidy/dynamic-keys", + Storage: config.StorageView, + } + + resp, err := b.HandleRequest(context.Background(), cleanRequest) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotNil(t, resp.Data["message"]) + require.Contains(t, resp.Data["message"], "0 of 0") + + // Write a bunch of bogus entries. + for i := 0; i < 15; i++ { + data := map[string]interface{}{ + "host": "localhost", + "key": "nothing-to-see-here", + } + entry, err := logical.StorageEntryJSON(fmt.Sprintf("%vexample-%v", keysStoragePrefix, i), &data) + require.NoError(t, err) + err = config.StorageView.Put(context.Background(), entry) + require.NoError(t, err) + } + + // Should now have 15 + resp, err = b.HandleRequest(context.Background(), cleanRequest) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotNil(t, resp.Data["message"]) + require.Contains(t, resp.Data["message"], "15 of 15") + + // Should have none left. + resp, err = b.HandleRequest(context.Background(), cleanRequest) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + require.NotNil(t, resp.Data["message"]) + require.Contains(t, resp.Data["message"], "0 of 0") +} diff --git a/builtin/logical/ssh/path_cleanup_dynamic_host_keys.go b/builtin/logical/ssh/path_cleanup_dynamic_host_keys.go new file mode 100644 index 000000000..4318e0b01 --- /dev/null +++ b/builtin/logical/ssh/path_cleanup_dynamic_host_keys.go @@ -0,0 +1,42 @@ +package ssh + +import ( + "context" + "fmt" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +const keysStoragePrefix = "keys/" + +func pathCleanupKeys(b *backend) *framework.Path { + return &framework.Path{ + Pattern: "tidy/dynamic-keys", + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.DeleteOperation: b.handleCleanupKeys, + }, + HelpSynopsis: `This endpoint removes the stored host keys used for the removed Dynamic Key feature, if present.`, + HelpDescription: `For more information, refer to the API documentation.`, + } +} + +func (b *backend) handleCleanupKeys(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + names, err := req.Storage.List(ctx, keysStoragePrefix) + if err != nil { + return nil, fmt.Errorf("unable to list keys for removal: %w", err) + } + + for index, name := range names { + keyPath := keysStoragePrefix + name + if err := req.Storage.Delete(ctx, keyPath); err != nil { + return nil, fmt.Errorf("unable to delete key %v of %v: %w", index+1, len(names), err) + } + } + + return &logical.Response{ + Data: map[string]interface{}{ + "message": fmt.Sprintf("Removed %v of %v host keys.", len(names), len(names)), + }, + }, nil +} diff --git a/changelog/18939.txt b/changelog/18939.txt new file mode 100644 index 000000000..aa7f8e7c6 --- /dev/null +++ b/changelog/18939.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/ssh: Allow removing SSH host keys from the dynamic keys feature. +``` diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index 97e5fa3cd..2c2c1db0a 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/website/content/api-docs/secret/ssh.mdx @@ -879,3 +879,48 @@ $ curl \ "auth": null } ``` + +## Tidy Host Keys + +This endpoint removes all existing host keys from Vault, if any are present. +These keys were used with the Dynamic Keys functionality, which were removed +from this engine. + +~> Note: This does not clean up any pending dynamic key leases and will not + remove these keys from systems with authorized hosts entries created by + Vault. That must be done manually by an operator, potentially before the + removal of these host keys if they are necessary to access these + systems.

+ For a more effective cleanup process, it is suggest to stay on Vault 1.12, + manually revoke all dynamic key leases, wait for this to finish, and then + upgrade to Vault 1.13+. + +| Method | Path | +| :------- | :----------------------- | +| `DELETE` | `/ssh/tidy/dynamic-keys` | + +### Sample Request + +```shell-session +$ curl \ + --header "X-Vault-Token: ..." \ + --request DELETE \ + http://127.0.0.1:8200/v1/ssh/issue/my-role +``` + +### Sample Response + +```json +{ + "request_id": "", + "lease_id": "", + "renewable": false, + "lease_duration": 0, + "data": { + "message": "Removed 15 of 15 host keys." + }, + "wrap_info": null, + "warnings": null, + "auth": null +} +```