From f3911cce6673c6651f723ebf77d06030286ddb80 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Thu, 8 Dec 2022 15:45:18 -0500 Subject: [PATCH] Add transit key config to disable upserting (#18272) * Rename path_config -> path_keys_config Signed-off-by: Alexander Scheel * Add config/keys to disable upserting Transit would allow anyone with Create permissions on the encryption endpoint to automatically create new encryption keys. This becomes hard to reason about for operators, especially if typos are subtly introduced (e.g., my-key vs my_key) -- there is no way to merge these two keys afterwards. Add the ability to globally disable upserting, so that if the applications using Transit do not need the capability, it can be globally disallowed even under permissive policies. Signed-off-by: Alexander Scheel * Add documentation on disabling upsert Signed-off-by: Alexander Scheel * Add changelog entry Signed-off-by: Alexander Scheel * Update website/content/api-docs/secret/transit.mdx Co-authored-by: tjperry07 * Update website/content/api-docs/secret/transit.mdx Co-authored-by: tjperry07 Signed-off-by: Alexander Scheel Co-authored-by: tjperry07 --- builtin/logical/transit/backend.go | 3 +- builtin/logical/transit/backend_test.go | 4 +- builtin/logical/transit/path_config_keys.go | 117 ++++++++++++++++++ .../logical/transit/path_config_keys_test.go | 67 ++++++++++ builtin/logical/transit/path_encrypt.go | 7 +- .../{path_config.go => path_keys_config.go} | 14 +-- ...onfig_test.go => path_keys_config_test.go} | 0 changelog/18272.txt | 3 + website/content/api-docs/secret/transit.mdx | 74 +++++++++++ 9 files changed, 278 insertions(+), 11 deletions(-) create mode 100644 builtin/logical/transit/path_config_keys.go create mode 100644 builtin/logical/transit/path_config_keys_test.go rename builtin/logical/transit/{path_config.go => path_keys_config.go} (94%) rename builtin/logical/transit/{path_config_test.go => path_keys_config_test.go} (100%) create mode 100644 changelog/18272.txt diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 391b392c6..668254747 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -43,7 +43,6 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) Paths: []*framework.Path{ // Rotate/Config needs to come before Keys // as the handler is greedy - b.pathConfig(), b.pathRotate(), b.pathRewrap(), b.pathWrappingKey(), @@ -52,6 +51,7 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) b.pathKeys(), b.pathListKeys(), b.pathExportKeys(), + b.pathKeysConfig(), b.pathEncrypt(), b.pathDecrypt(), b.pathDatakey(), @@ -64,6 +64,7 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) b.pathRestore(), b.pathTrim(), b.pathCacheConfig(), + b.pathConfigKeys(), }, Secrets: []*framework.Secret{}, diff --git a/builtin/logical/transit/backend_test.go b/builtin/logical/transit/backend_test.go index 97ceedf95..71cbfb641 100644 --- a/builtin/logical/transit/backend_test.go +++ b/builtin/logical/transit/backend_test.go @@ -1524,8 +1524,8 @@ func testPolicyFuzzingCommon(t *testing.T, be *backend) { // keys start at version 1 so we want [1, latestVersion] not [0, latestVersion) setVersion := (rand.Int() % latestVersion) + 1 fd.Raw["min_decryption_version"] = setVersion - fd.Schema = be.pathConfig().Fields - resp, err = be.pathConfigWrite(context.Background(), req, fd) + fd.Schema = be.pathKeysConfig().Fields + resp, err = be.pathKeysConfigWrite(context.Background(), req, fd) if err != nil { t.Errorf("got an error setting min decryption version: %v", err) } diff --git a/builtin/logical/transit/path_config_keys.go b/builtin/logical/transit/path_config_keys.go new file mode 100644 index 000000000..2294636e3 --- /dev/null +++ b/builtin/logical/transit/path_config_keys.go @@ -0,0 +1,117 @@ +package transit + +import ( + "context" + "fmt" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +const keysConfigPath = "config/keys" + +type keysConfig struct { + DisableUpsert bool `json:"disable_upsert"` +} + +var defaultKeysConfig = keysConfig{ + DisableUpsert: false, +} + +func (b *backend) pathConfigKeys() *framework.Path { + return &framework.Path{ + Pattern: "config/keys", + Fields: map[string]*framework.FieldSchema{ + "disable_upsert": { + Type: framework.TypeBool, + Description: `Whether to allow automatic upserting (creation) of +keys on the encrypt endpoint.`, + }, + }, + + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathConfigKeysWrite, + logical.ReadOperation: b.pathConfigKeysRead, + }, + + HelpSynopsis: pathConfigKeysHelpSyn, + HelpDescription: pathConfigKeysHelpDesc, + } +} + +func (b *backend) readConfigKeys(ctx context.Context, req *logical.Request) (*keysConfig, error) { + entry, err := req.Storage.Get(ctx, keysConfigPath) + if err != nil { + return nil, fmt.Errorf("failed to fetch keys configuration: %w", err) + } + + var cfg keysConfig + if entry == nil { + cfg = defaultKeysConfig + return &cfg, nil + } + + if err := entry.DecodeJSON(&cfg); err != nil { + return nil, fmt.Errorf("failed to decode keys configuration: %w", err) + } + + return &cfg, nil +} + +func (b *backend) writeConfigKeys(ctx context.Context, req *logical.Request, cfg *keysConfig) error { + entry, err := logical.StorageEntryJSON(keysConfigPath, cfg) + if err != nil { + return fmt.Errorf("failed to marshal keys configuration: %w", err) + } + + return req.Storage.Put(ctx, entry) +} + +func respondConfigKeys(cfg *keysConfig) *logical.Response { + return &logical.Response{ + Data: map[string]interface{}{ + "disable_upsert": cfg.DisableUpsert, + }, + } +} + +func (b *backend) pathConfigKeysWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + upsert := d.Get("disable_upsert").(bool) + + cfg, err := b.readConfigKeys(ctx, req) + if err != nil { + return nil, err + } + + modified := false + + if cfg.DisableUpsert != upsert { + cfg.DisableUpsert = upsert + modified = true + } + + if modified { + if err := b.writeConfigKeys(ctx, req, cfg); err != nil { + return nil, err + } + } + + return respondConfigKeys(cfg), nil +} + +func (b *backend) pathConfigKeysRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) { + cfg, err := b.readConfigKeys(ctx, req) + if err != nil { + return nil, err + } + + return respondConfigKeys(cfg), nil +} + +const pathConfigKeysHelpSyn = `Configuration common across all keys` + +const pathConfigKeysHelpDesc = ` +This path is used to configure common functionality across all keys. Currently, +this supports limiting the ability to automatically create new keys when an +unknown key is used for encryption (upsert). +` diff --git a/builtin/logical/transit/path_config_keys_test.go b/builtin/logical/transit/path_config_keys_test.go new file mode 100644 index 000000000..8d8f9f940 --- /dev/null +++ b/builtin/logical/transit/path_config_keys_test.go @@ -0,0 +1,67 @@ +package transit + +import ( + "context" + "testing" + + "github.com/hashicorp/vault/sdk/logical" +) + +func TestTransit_ConfigKeys(t *testing.T) { + b, s := createBackendWithSysView(t) + + doReq := func(req *logical.Request) *logical.Response { + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("got err:\n%#v\nreq:\n%#v\n", err, *req) + } + return resp + } + doErrReq := func(req *logical.Request) { + resp, err := b.HandleRequest(context.Background(), req) + if err == nil { + if resp == nil || !resp.IsError() { + t.Fatalf("expected error; req:\n%#v\n", *req) + } + } + } + + // First read the global config + req := &logical.Request{ + Storage: s, + Operation: logical.ReadOperation, + Path: "config/keys", + } + resp := doReq(req) + if resp.Data["disable_upsert"].(bool) != false { + t.Fatalf("expected disable_upsert to be false; got: %v", resp) + } + + // Ensure we can upsert. + req.Operation = logical.CreateOperation + req.Path = "encrypt/upsert-1" + req.Data = map[string]interface{}{ + "plaintext": "aGVsbG8K", + } + doReq(req) + + // Disable upserting. + req.Operation = logical.UpdateOperation + req.Path = "config/keys" + req.Data = map[string]interface{}{ + "disable_upsert": true, + } + doReq(req) + + // Attempt upserting again, it should fail. + req.Operation = logical.CreateOperation + req.Path = "encrypt/upsert-2" + req.Data = map[string]interface{}{ + "plaintext": "aGVsbG8K", + } + doErrReq(req) + + // Redoing this with the first key should succeed. + req.Path = "encrypt/upsert-1" + doReq(req) +} diff --git a/builtin/logical/transit/path_encrypt.go b/builtin/logical/transit/path_encrypt.go index 6c0a3fc98..48a6ab7fa 100644 --- a/builtin/logical/transit/path_encrypt.go +++ b/builtin/logical/transit/path_encrypt.go @@ -373,8 +373,13 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d return logical.ErrorResponse("convergent encryption requires derivation to be enabled, so context is required"), nil } + cfg, err := b.readConfigKeys(ctx, req) + if err != nil { + return nil, err + } + polReq = keysutil.PolicyRequest{ - Upsert: true, + Upsert: !cfg.DisableUpsert, Storage: req.Storage, Name: name, Derived: contextSet, diff --git a/builtin/logical/transit/path_config.go b/builtin/logical/transit/path_keys_config.go similarity index 94% rename from builtin/logical/transit/path_config.go rename to builtin/logical/transit/path_keys_config.go index a3d72731b..f2628e4f0 100644 --- a/builtin/logical/transit/path_config.go +++ b/builtin/logical/transit/path_keys_config.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -func (b *backend) pathConfig() *framework.Path { +func (b *backend) pathKeysConfig() *framework.Path { return &framework.Path{ Pattern: "keys/" + framework.GenericNameRegex("name") + "/config", Fields: map[string]*framework.FieldSchema{ @@ -58,15 +58,15 @@ disables automatic rotation for the key.`, }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathConfigWrite, + logical.UpdateOperation: b.pathKeysConfigWrite, }, - HelpSynopsis: pathConfigHelpSyn, - HelpDescription: pathConfigHelpDesc, + HelpSynopsis: pathKeysConfigHelpSyn, + HelpDescription: pathKeysConfigHelpDesc, } } -func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (resp *logical.Response, retErr error) { +func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (resp *logical.Response, retErr error) { name := d.Get("name").(string) // Check if the policy already exists before we lock everything @@ -228,9 +228,9 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d * return resp, p.Persist(ctx, req.Storage) } -const pathConfigHelpSyn = `Configure a named encryption key` +const pathKeysConfigHelpSyn = `Configure a named encryption key` -const pathConfigHelpDesc = ` +const pathKeysConfigHelpDesc = ` This path is used to configure the named key. Currently, this supports adjusting the minimum version of the key allowed to be used for decryption via the min_decryption_version parameter. diff --git a/builtin/logical/transit/path_config_test.go b/builtin/logical/transit/path_keys_config_test.go similarity index 100% rename from builtin/logical/transit/path_config_test.go rename to builtin/logical/transit/path_keys_config_test.go diff --git a/changelog/18272.txt b/changelog/18272.txt new file mode 100644 index 000000000..168913be8 --- /dev/null +++ b/changelog/18272.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/transit: Allow configuring whether upsert of keys is allowed. +``` diff --git a/website/content/api-docs/secret/transit.mdx b/website/content/api-docs/secret/transit.mdx index eab3282a9..42d62b2ae 100644 --- a/website/content/api-docs/secret/transit.mdx +++ b/website/content/api-docs/secret/transit.mdx @@ -513,6 +513,77 @@ $ curl \ } ``` +## Write Keys Configuration + +This endpoint maintains global configuration across all keys. This +allows removing the upsert capability of the `/encrypt/:key` endpoint, +preventing new keys from being created if none exists. + +| Method | Path | +| :----- | :--------------------- | +| `POST` | `/transit/config/keys` | + +### Parameters + +- `disable_upsert` `(bool: false)` - Specifies whether to disable upserting on + encryption (automatic creation of unknown keys). + +### Sample Payload + +```json +{ + "disable_upsert": true +} +``` + +### Sample Request + +```shell-session +$ curl \ + --header "X-Vault-Token: ..." \ + --request POST \ + --data @payload.json \ + http://127.0.0.1:8200/v1/transit/config/keys +``` + +### Sample Response + +```json +{ + "data": { + "disable_upsert": true, + } +} +``` + +## Read Keys Configuration + +This endpoint maintains global configuration across all keys. This +allows removing the upsert capability of the `/encrypt/:key` endpoint, +preventing new keys from being created if none exists. + +| Method | Path | +| :----- | :--------------------- | +| `GET` | `/transit/config/keys` | + +### Sample Request + +```shell-session +$ curl \ + --header "X-Vault-Token: ..." \ + http://127.0.0.1:8200/v1/transit/config/keys +``` + +### Sample Response + +```json +{ + "data": { + "disable_upsert": false, + } +} +``` + ## Encrypt Data This endpoint encrypts the provided plaintext using the named key. This path @@ -523,6 +594,9 @@ requires derivation depends on whether the context parameter is empty or not). If the user only has `update` capability and the key does not exist, an error will be returned. +~> Note: If upsert is disallowed by global keys configuration, `create` + requests will behave like `update` requests. + | Method | Path | | :----- | :----------------------- | | `POST` | `/transit/encrypt/:name` |