Add transit key config to disable upserting (#18272)

* Rename path_config -> path_keys_config

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* 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 <alex.scheel@hashicorp.com>

* Add documentation on disabling upsert

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Update website/content/api-docs/secret/transit.mdx

Co-authored-by: tjperry07 <tjperry07@users.noreply.github.com>

* Update website/content/api-docs/secret/transit.mdx

Co-authored-by: tjperry07 <tjperry07@users.noreply.github.com>

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Co-authored-by: tjperry07 <tjperry07@users.noreply.github.com>

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{},

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
+				fd.Schema = be.pathKeysConfig().Fields
-				resp, err = be.pathConfigWrite(context.Background(), req, fd)
+				resp, err = be.pathKeysConfigWrite(context.Background(), req, fd)
 				if err != nil {
 					t.Errorf("got an error setting min decryption version: %v", err)
 				}

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).
+`

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)
+}

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,

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,
+		HelpSynopsis:    pathKeysConfigHelpSyn,
-		HelpDescription: pathConfigHelpDesc,
+		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.

changelog/18272.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/transit: Allow configuring whether upsert of keys is allowed.
+```

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` |