From a201657deac86e42f1ee646c9f2cd12331aafe55 Mon Sep 17 00:00:00 2001 From: Alexander Scheel <alex.scheel@hashicorp.com> Date: Wed, 17 May 2023 17:10:12 -0400 Subject: [PATCH 1/5] Respond with cache size on config write Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --- builtin/logical/transit/path_cache_config.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 4a12bee0bb19..f8f0cea551ee 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -84,7 +84,11 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - return nil, nil + return &logical.Response{ + Data: map[string]interface{}{ + "size": cacheSize, + }, + }, nil } type configCache struct { From 696d9117aca8f4f87cf774b92dea1b712df1992c Mon Sep 17 00:00:00 2001 From: Alexander Scheel <alex.scheel@hashicorp.com> Date: Thu, 18 May 2023 11:12:38 -0400 Subject: [PATCH 2/5] Respond with key policy on write This includes creating a key, but also trimming or rotating an existing key. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --- builtin/logical/transit/path_keys.go | 30 ++++++++++++--------- builtin/logical/transit/path_keys_config.go | 28 ++++++++++++++----- builtin/logical/transit/path_rotate.go | 7 ++++- builtin/logical/transit/path_trim.go | 2 +- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/builtin/logical/transit/path_keys.go b/builtin/logical/transit/path_keys.go index 65eec260acc0..285f32d33cdc 100644 --- a/builtin/logical/transit/path_keys.go +++ b/builtin/logical/transit/path_keys.go @@ -257,12 +257,14 @@ func (b *backend) pathPolicyWrite(ctx context.Context, req *logical.Request, d * p.Unlock() } - resp := &logical.Response{} + resp, err := b.formatKeyPolicy(p, nil) + if err != nil { + return nil, err + } if !upserted { resp.AddWarning(fmt.Sprintf("key %s already existed", name)) } - - return nil, nil + return resp, nil } // Built-in helper type for returning asymmetric keys @@ -290,6 +292,19 @@ func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *f } defer p.Unlock() + contextRaw := d.Get("context").(string) + var context []byte + if len(contextRaw) != 0 { + context, err = base64.StdEncoding.DecodeString(contextRaw) + if err != nil { + return logical.ErrorResponse("failed to base64-decode context"), logical.ErrInvalidRequest + } + } + + return b.formatKeyPolicy(p, context) +} + +func (b *backend) formatKeyPolicy(p *keysutil.Policy, context []byte) (*logical.Response, error) { // Return the response resp := &logical.Response{ Data: map[string]interface{}{ @@ -346,15 +361,6 @@ func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *f } } - contextRaw := d.Get("context").(string) - var context []byte - if len(contextRaw) != 0 { - context, err = base64.StdEncoding.DecodeString(contextRaw) - if err != nil { - return logical.ErrorResponse("failed to base64-decode context"), logical.ErrInvalidRequest - } - } - switch p.Type { case keysutil.KeyType_AES128_GCM96, keysutil.KeyType_AES256_GCM96, keysutil.KeyType_ChaCha20_Poly1305: retKeys := map[string]int64{} diff --git a/builtin/logical/transit/path_keys_config.go b/builtin/logical/transit/path_keys_config.go index 1fc44e770ef4..722d39c1e3b5 100644 --- a/builtin/logical/transit/path_keys_config.go +++ b/builtin/logical/transit/path_keys_config.go @@ -97,6 +97,8 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, } defer p.Unlock() + var warning string + originalMinDecryptionVersion := p.MinDecryptionVersion originalMinEncryptionVersion := p.MinEncryptionVersion originalDeletionAllowed := p.DeletionAllowed @@ -113,8 +115,6 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, } }() - resp = &logical.Response{} - persistNeeded := false minDecryptionVersionRaw, ok := d.GetOk("min_decryption_version") @@ -127,7 +127,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, if minDecryptionVersion == 0 { minDecryptionVersion = 1 - resp.AddWarning("since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1") + warning = "since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1" } if minDecryptionVersion != p.MinDecryptionVersion { @@ -221,7 +221,14 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, } if !persistNeeded { - return nil, nil + resp, err := b.formatKeyPolicy(p, nil) + if err != nil { + return nil, err + } + if warning != "" { + resp.AddWarning(warning) + } + return resp, nil } switch { @@ -231,11 +238,18 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request, return logical.ErrorResponse("min decryption version should not be less then min available version"), nil } - if len(resp.Warnings) == 0 { - return nil, p.Persist(ctx, req.Storage) + if err := p.Persist(ctx, req.Storage); err != nil { + return nil, err } - return resp, p.Persist(ctx, req.Storage) + resp, err = b.formatKeyPolicy(p, nil) + if err != nil { + return nil, err + } + if warning != "" { + resp.AddWarning(warning) + } + return resp, nil } const pathKeysConfigHelpSyn = `Configure a named encryption key` diff --git a/builtin/logical/transit/path_rotate.go b/builtin/logical/transit/path_rotate.go index a4ee8d87062b..41dbc64db779 100644 --- a/builtin/logical/transit/path_rotate.go +++ b/builtin/logical/transit/path_rotate.go @@ -79,7 +79,12 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d * } p.Unlock() - return nil, err + + if err != nil { + return nil, err + } + + return b.formatKeyPolicy(p, nil) } const pathRotateHelpSyn = `Rotate named encryption key` diff --git a/builtin/logical/transit/path_trim.go b/builtin/logical/transit/path_trim.go index 1afe019f55ed..71f4181db859 100644 --- a/builtin/logical/transit/path_trim.go +++ b/builtin/logical/transit/path_trim.go @@ -100,7 +100,7 @@ func (b *backend) pathTrimUpdate() framework.OperationFunc { return nil, err } - return nil, nil + return b.formatKeyPolicy(p, nil) } } From 8fac637f60e5330b4f0ad2432751b50b8a20fee3 Mon Sep 17 00:00:00 2001 From: Alexander Scheel <alex.scheel@hashicorp.com> Date: Thu, 18 May 2023 11:16:35 -0400 Subject: [PATCH 3/5] Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --- changelog/20652.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/20652.txt diff --git a/changelog/20652.txt b/changelog/20652.txt new file mode 100644 index 000000000000..c41e750c0472 --- /dev/null +++ b/changelog/20652.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/transit: Respond to writes with updated key policy, cache configuration. +``` From f52173ec12ca52afa3e7610656ad7972aa9f6846 Mon Sep 17 00:00:00 2001 From: Alexander Scheel <alex.scheel@hashicorp.com> Date: Thu, 18 May 2023 12:23:44 -0400 Subject: [PATCH 4/5] Correctly handle locking around policy formatting Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --- builtin/logical/transit/path_rotate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/logical/transit/path_rotate.go b/builtin/logical/transit/path_rotate.go index 41dbc64db779..0035dcfbb976 100644 --- a/builtin/logical/transit/path_rotate.go +++ b/builtin/logical/transit/path_rotate.go @@ -64,6 +64,7 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d * if !b.System().CachingDisabled() { p.Lock(true) } + defer p.Unlock() if p.Type == keysutil.KeyType_MANAGED_KEY { var keyId string @@ -78,8 +79,6 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d * err = p.Rotate(ctx, req.Storage, b.GetRandomReader()) } - p.Unlock() - if err != nil { return nil, err } From dbe8ee726d72da5cf331e66261b0d1ea2cfba60f Mon Sep 17 00:00:00 2001 From: Alexander Scheel <alex.scheel@hashicorp.com> Date: Thu, 18 May 2023 12:23:55 -0400 Subject: [PATCH 5/5] Validate that responses are non-empty Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --- builtin/logical/transit/backend_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/builtin/logical/transit/backend_test.go b/builtin/logical/transit/backend_test.go index 3ead8960de20..f7109d8debc5 100644 --- a/builtin/logical/transit/backend_test.go +++ b/builtin/logical/transit/backend_test.go @@ -1071,9 +1071,7 @@ func testConvergentEncryptionCommon(t *testing.T, ver int, keyType keysutil.KeyT if err != nil { t.Fatal(err) } - if resp != nil { - t.Fatal("expected nil response") - } + require.NotNil(t, resp, "expected populated request") p, err := keysutil.LoadPolicy(context.Background(), storage, path.Join("policy", "testkey")) if err != nil { @@ -1559,9 +1557,7 @@ func TestBadInput(t *testing.T) { if err != nil { t.Fatal(err) } - if resp != nil { - t.Fatal("expected nil response") - } + require.NotNil(t, resp, "expected populated request") req.Path = "decrypt/test" req.Data = map[string]interface{}{ @@ -1650,9 +1646,7 @@ func TestTransit_AutoRotateKeys(t *testing.T) { if err != nil { t.Fatal(err) } - if resp != nil { - t.Fatal("expected nil response") - } + require.NotNil(t, resp, "expected populated request") // Write a key with an auto rotate value one day in the future req = &logical.Request{ @@ -1667,9 +1661,7 @@ func TestTransit_AutoRotateKeys(t *testing.T) { if err != nil { t.Fatal(err) } - if resp != nil { - t.Fatal("expected nil response") - } + require.NotNil(t, resp, "expected populated request") // Run the rotation check and ensure none of the keys have rotated b.checkAutoRotateAfter = time.Now()