From a8d316de85a14097e864f1345c1de1b2fd4a3bb3 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 391b392c69..6682547479 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 97ceedf953..71cbfb641d 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 0000000000..2294636e39 --- /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 0000000000..8d8f9f940f --- /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 6c0a3fc98f..48a6ab7fac 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 a3d72731b7..f2628e4f03 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 0000000000..168913be81 --- /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 eab3282a94..42d62b2ae9 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` |