From 9c2c35472d7b6dcad75acefa33acd25fa5f0a3db Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Mon, 23 Aug 2021 14:42:05 -0500 Subject: [PATCH 1/8] Enforce Minimum cache size for transit backend --- builtin/logical/transit/backend.go | 21 ++++++++++++++++++++ builtin/logical/transit/path_cache_config.go | 13 ++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index f438ac1b0290b..8e67832e09c1c 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -68,6 +68,27 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) if err != nil { return nil, fmt.Errorf("Error retrieving cache size from storage: %w", err) } + + minCacheSize := 10 + if cacheSize < minCacheSize { + b.Logger().Warn("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize) + cacheSize = minCacheSize + } + // store cache size + entry, err := logical.StorageEntryJSON("config/cache", &configCache{ + Size: cacheSize, + }) + if err != nil { + return nil, err: + + } + + //var req *logical.Request + //var storage logical.Storage + if err := conf.StorageView.Put(ctx, entry); err != nil { + return nil, err + } + } var err error diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 6239555b37668..4fb0c141a9bb2 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -3,7 +3,7 @@ package transit import ( "context" "errors" - + "fmt" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -48,6 +48,13 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request if cacheSize < 0 { return logical.ErrorResponse("size must be greater or equal to 0"), logical.ErrInvalidRequest } + // minCacheSize := 10 + var resp = &logical.Response{} + + if cacheSize < minCacheSize { + resp.AddWarning(fmt.Sprintf("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize)) + cacheSize = minCacheSize + } // store cache size entry, err := logical.StorageEntryJSON("config/cache", &configCache{ @@ -60,9 +67,7 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - resp := &logical.Response{ - Warnings: []string{"cache configurations will be applied when this backend is restarted"}, - } + resp.AddWarning("cache configurations will be applied when this backend is restarted") return resp, nil } From ba78939a2554476733e1efe747ef64ac130415e7 Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Mon, 23 Aug 2021 17:09:34 -0500 Subject: [PATCH 2/8] enfore minimum cache size and log a warning during backend construction --- builtin/logical/transit/backend.go | 15 --------------- builtin/logical/transit/path_cache_config.go | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 8e67832e09c1c..902c85992a32d 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -74,21 +74,6 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) b.Logger().Warn("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize) cacheSize = minCacheSize } - // store cache size - entry, err := logical.StorageEntryJSON("config/cache", &configCache{ - Size: cacheSize, - }) - if err != nil { - return nil, err: - - } - - //var req *logical.Request - //var storage logical.Storage - if err := conf.StorageView.Put(ctx, entry); err != nil { - return nil, err - } - } var err error diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 4fb0c141a9bb2..c902136a13af5 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -48,7 +48,7 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request if cacheSize < 0 { return logical.ErrorResponse("size must be greater or equal to 0"), logical.ErrInvalidRequest } - // minCacheSize := 10 + minCacheSize := 10 var resp = &logical.Response{} if cacheSize < minCacheSize { From 7d87642430d4b488dc97b3f36a517423a8ed5374 Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Tue, 24 Aug 2021 10:19:41 -0500 Subject: [PATCH 3/8] Update documentation for transit backend cache configuration --- builtin/logical/transit/backend.go | 2 +- builtin/logical/transit/path_cache_config.go | 3 +-- website/content/api-docs/secret/transit.mdx | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 902c85992a32d..20f31c3adb379 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -70,7 +70,7 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) } minCacheSize := 10 - if cacheSize < minCacheSize { + if cacheSize != 0 && cacheSize < minCacheSize { b.Logger().Warn("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize) cacheSize = minCacheSize } diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index c902136a13af5..949f47946d3da 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -50,8 +50,7 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request } minCacheSize := 10 var resp = &logical.Response{} - - if cacheSize < minCacheSize { + if cacheSize != 0 && cacheSize < minCacheSize { resp.AddWarning(fmt.Sprintf("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize)) cacheSize = minCacheSize } diff --git a/website/content/api-docs/secret/transit.mdx b/website/content/api-docs/secret/transit.mdx index e2a27cdd266af..3c63e7eb8c762 100644 --- a/website/content/api-docs/secret/transit.mdx +++ b/website/content/api-docs/secret/transit.mdx @@ -1307,7 +1307,7 @@ using the [`/sys/plugins/reload/backend`][sys-plugin-reload-backend] endpoint. - `size` `(int: 0)` - Specifies the size in terms of number of entries. A size of `0` means unlimited. A _Least Recently Used_ (LRU) caching strategy is used for a - non-zero cache size. + non-zero cache size. Must be 0 (default) or a value greater or equal to 10 (minimum cache size). ### Sample Payload From 1cc707e41b95bdc8cf559fae5e8cb47eed5056b3 Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Tue, 24 Aug 2021 11:06:28 -0500 Subject: [PATCH 4/8] Added changelog --- builtin/logical/transit/path_cache_config.go | 2 +- changelog/12418.txt | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog/12418.txt diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 949f47946d3da..73331252ad231 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -49,7 +49,7 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return logical.ErrorResponse("size must be greater or equal to 0"), logical.ErrInvalidRequest } minCacheSize := 10 - var resp = &logical.Response{} + var resp = &logical.Response{} if cacheSize != 0 && cacheSize < minCacheSize { resp.AddWarning(fmt.Sprintf("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize)) cacheSize = minCacheSize diff --git a/changelog/12418.txt b/changelog/12418.txt new file mode 100644 index 0000000000000..c9be8686b3167 --- /dev/null +++ b/changelog/12418.txt @@ -0,0 +1,3 @@ +```release-note:bug +Enforce minimum cache size for transit backend +``` From 95c7db06d6732307d3416a5281da0d74d54c1cbf Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Tue, 24 Aug 2021 17:19:19 -0500 Subject: [PATCH 5/8] Addressed review feedback and added unit test --- builtin/logical/transit/backend.go | 6 ++++-- builtin/logical/transit/path_cache_config.go | 15 +++++---------- .../logical/transit/path_cache_config_test.go | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 20f31c3adb379..02e611e481064 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -10,6 +10,9 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) +// Minimum cache size for transit backend +const minCacheSize = 10 + func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { b, err := Backend(ctx, conf) if err != nil { @@ -69,9 +72,8 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) return nil, fmt.Errorf("Error retrieving cache size from storage: %w", err) } - minCacheSize := 10 if cacheSize != 0 && cacheSize < minCacheSize { - b.Logger().Warn("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize) + b.Logger().Warn("size %d is less than minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize) cacheSize = minCacheSize } } diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 73331252ad231..60d2371dba2b0 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -3,7 +3,6 @@ package transit import ( "context" "errors" - "fmt" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -45,14 +44,8 @@ func (b *backend) pathCacheConfig() *framework.Path { func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { // get target size cacheSize := d.Get("size").(int) - if cacheSize < 0 { - return logical.ErrorResponse("size must be greater or equal to 0"), logical.ErrInvalidRequest - } - minCacheSize := 10 - var resp = &logical.Response{} - if cacheSize != 0 && cacheSize < minCacheSize { - resp.AddWarning(fmt.Sprintf("size %d is less than default minimum %d. Cache size is set to %d", cacheSize, minCacheSize, minCacheSize)) - cacheSize = minCacheSize + if cacheSize != 0 && cacheSize < minCacheSize { + return logical.ErrorResponse("size must be 0 or a value greater or equal to %d", minCacheSize), logical.ErrInvalidRequest } // store cache size @@ -66,7 +59,9 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - resp.AddWarning("cache configurations will be applied when this backend is restarted") + resp := &logical.Response{ + Warnings: []string{"cache configurations will be applied when this backend is restarted"}, + } return resp, nil } diff --git a/builtin/logical/transit/path_cache_config_test.go b/builtin/logical/transit/path_cache_config_test.go index 6cca1b265676a..10b4a1edd1638 100644 --- a/builtin/logical/transit/path_cache_config_test.go +++ b/builtin/logical/transit/path_cache_config_test.go @@ -8,6 +8,7 @@ import ( ) const targetCacheSize = 12345 +const smallCacheSize = 3 func TestTransit_CacheConfig(t *testing.T) { b1, storage := createBackendWithSysView(t) @@ -58,6 +59,15 @@ func TestTransit_CacheConfig(t *testing.T) { }, } + writeSmallCacheSizeReq := &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "cache-config", + Data: map[string]interface{}{ + "size": smallCacheSize, + }, + } + readReq := &logical.Request{ Storage: storage, Operation: logical.ReadOperation, @@ -77,4 +87,10 @@ func TestTransit_CacheConfig(t *testing.T) { // b3 enables transit without a cache, trying to read it should error b3 := createBackendWithForceNoCacheWithSysViewWithStorage(t, storage) doErrReq(b3, readReq) + + // b4 should spin up with a size less than minimum cache size (10) + b4, storage := createBackendWithSysView(t) + doErrReq(b4, writeSmallCacheSizeReq) + + } From 825d743772fcf943735fbb43bf59aa530b2ec940 Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Wed, 25 Aug 2021 16:49:45 -0500 Subject: [PATCH 6/8] Modify code in pathCacheConfigWrite to make use of the updated cache size --- builtin/logical/transit/path_cache_config.go | 8 ++++++-- builtin/logical/transit/path_cache_config_test.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 60d2371dba2b0..16b59ee8647f2 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -4,6 +4,7 @@ import ( "context" "errors" "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/keysutil" "github.com/hashicorp/vault/sdk/logical" ) @@ -59,8 +60,11 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - resp := &logical.Response{ - Warnings: []string{"cache configurations will be applied when this backend is restarted"}, + resp := &logical.Response{} + + b.lm, err = keysutil.NewLockManager(true, cacheSize) + if err != nil { + return nil, err } return resp, nil diff --git a/builtin/logical/transit/path_cache_config_test.go b/builtin/logical/transit/path_cache_config_test.go index 10b4a1edd1638..41f697e991a76 100644 --- a/builtin/logical/transit/path_cache_config_test.go +++ b/builtin/logical/transit/path_cache_config_test.go @@ -78,7 +78,7 @@ func TestTransit_CacheConfig(t *testing.T) { // b1 should spin up with an unlimited cache validateResponse(doReq(b1, readReq), 0, false) doReq(b1, writeReq) - validateResponse(doReq(b1, readReq), targetCacheSize, true) + validateResponse(doReq(b1, readReq), targetCacheSize, false) // b2 should spin up with a configured cache b2 := createBackendWithSysViewWithStorage(t, storage) From 3a4c78b15dd10f04311c31d53ed7101e6e8dac5e Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Fri, 10 Sep 2021 13:49:17 -0500 Subject: [PATCH 7/8] Updated code to refresh cache size on transit backend without restart --- builtin/logical/transit/backend.go | 39 ++++++++++++++++++- builtin/logical/transit/path_cache_config.go | 7 +--- .../logical/transit/path_cache_config_test.go | 25 ++++++++++++ builtin/logical/transit/path_config.go | 2 +- builtin/logical/transit/path_datakey.go | 2 +- builtin/logical/transit/path_decrypt.go | 2 +- builtin/logical/transit/path_encrypt.go | 4 +- builtin/logical/transit/path_export.go | 2 +- builtin/logical/transit/path_hmac.go | 4 +- builtin/logical/transit/path_hmac_test.go | 4 +- builtin/logical/transit/path_keys.go | 4 +- builtin/logical/transit/path_rewrap.go | 2 +- builtin/logical/transit/path_rotate.go | 2 +- builtin/logical/transit/path_sign_verify.go | 4 +- .../logical/transit/path_sign_verify_test.go | 6 +-- builtin/logical/transit/path_trim.go | 2 +- builtin/logical/transit/path_trim_test.go | 2 +- sdk/helper/keysutil/lock_manager.go | 18 +++++++++ 18 files changed, 104 insertions(+), 27 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 02e611e481064..96393c92347fe 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -3,7 +3,9 @@ package transit import ( "context" "fmt" + "io" "strings" + "sync" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/keysutil" @@ -90,6 +92,9 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) type backend struct { *framework.Backend lm *keysutil.LockManager + // Lock to make changes to any of the backend's cache configuration. + configMutex sync.RWMutex + cacheSizeChanged bool } func GetCacheSizeFromStorage(ctx context.Context, s logical.Storage) (int, error) { @@ -108,7 +113,34 @@ func GetCacheSizeFromStorage(ctx context.Context, s logical.Storage) (int, error return size, nil } -func (b *backend) invalidate(_ context.Context, key string) { +// Update cache size and get policy +func (b *backend) GetPolicy(ctx context.Context, polReq keysutil.PolicyRequest, rand io.Reader) (retP *keysutil.Policy, retUpserted bool, retErr error) { + if b.lm.GetUseCache() && b.cacheSizeChanged { + var err error + currentCacheSize := b.lm.GetCacheSize() + storedCacheSize, err := GetCacheSizeFromStorage(ctx, polReq.Storage) + if err != nil { + return nil, false, err + } + if currentCacheSize != storedCacheSize { + err = b.lm.RefreshCache(storedCacheSize) + if err != nil { + return nil, false, err + } + } + // Acquire the lock to modify cacheSizeChanged + b.configMutex.Lock() + defer b.configMutex.Unlock() + b.cacheSizeChanged = false + } + p, _, err := b.lm.GetPolicy(ctx, polReq, rand) + if err != nil { + return p, false, err + } + return p, true, nil +} + +func (b *backend) invalidate(ctx context.Context, key string) { if b.Logger().IsDebug() { b.Logger().Debug("invalidating key", "key", key) } @@ -116,5 +148,10 @@ func (b *backend) invalidate(_ context.Context, key string) { case strings.HasPrefix(key, "policy/"): name := strings.TrimPrefix(key, "policy/") b.lm.InvalidatePolicy(name) + case strings.HasPrefix(key, "cache-config/"): + // Acquire the lock to set the flag to indicate that cache size needs to be refreshed from storage + b.configMutex.Lock() + defer b.configMutex.Unlock() + b.cacheSizeChanged = true } } diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index 16b59ee8647f2..f3ca1649f7936 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -4,7 +4,6 @@ import ( "context" "errors" "github.com/hashicorp/vault/sdk/framework" - "github.com/hashicorp/vault/sdk/helper/keysutil" "github.com/hashicorp/vault/sdk/logical" ) @@ -60,14 +59,12 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - resp := &logical.Response{} - - b.lm, err = keysutil.NewLockManager(true, cacheSize) + err = b.lm.RefreshCache(cacheSize) if err != nil { return nil, err } - return resp, nil + return nil, nil } type configCache struct { diff --git a/builtin/logical/transit/path_cache_config_test.go b/builtin/logical/transit/path_cache_config_test.go index 41f697e991a76..8d5f0195c7eb3 100644 --- a/builtin/logical/transit/path_cache_config_test.go +++ b/builtin/logical/transit/path_cache_config_test.go @@ -74,11 +74,36 @@ func TestTransit_CacheConfig(t *testing.T) { Path: "cache-config", } + polReq := &logical.Request{ + Storage: storage, + Operation: logical.UpdateOperation, + Path: "keys/aes256", + Data: map[string]interface{}{ + "derived": true, + }, + } + + // test steps // b1 should spin up with an unlimited cache validateResponse(doReq(b1, readReq), 0, false) + + // Change cache size to targetCacheSize 12345 and validate that cache size is updated doReq(b1, writeReq) validateResponse(doReq(b1, readReq), targetCacheSize, false) + b1.invalidate(context.Background(), "cache-config/") + + // Change the cache size to 1000 to mock the scenario where + // current cache size and stored cache size are different and + // a cache update is needed + b1.lm.RefreshCache(1000) + + // Write a new policy which in its code path detects that cache size has changed + // and refreshes the cache to 12345 + doReq(b1, polReq) + + // Validate that cache size is updated to 12345 + validateResponse(doReq(b1, readReq), targetCacheSize, false) // b2 should spin up with a configured cache b2 := createBackendWithSysViewWithStorage(t, storage) diff --git a/builtin/logical/transit/path_config.go b/builtin/logical/transit/path_config.go index 4641e2b6a7cd1..1c41cd0d49dd1 100644 --- a/builtin/logical/transit/path_config.go +++ b/builtin/logical/transit/path_config.go @@ -62,7 +62,7 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d * name := d.Get("name").(string) // Check if the policy already exists before we lock everything - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_datakey.go b/builtin/logical/transit/path_datakey.go index a287bea34415f..9e9ef2c173404 100644 --- a/builtin/logical/transit/path_datakey.go +++ b/builtin/logical/transit/path_datakey.go @@ -99,7 +99,7 @@ func (b *backend) pathDatakeyWrite(ctx context.Context, req *logical.Request, d } // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_decrypt.go b/builtin/logical/transit/path_decrypt.go index 5d8510da89f46..cf6d450604062 100644 --- a/builtin/logical/transit/path_decrypt.go +++ b/builtin/logical/transit/path_decrypt.go @@ -121,7 +121,7 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d } // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: d.Get("name").(string), }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_encrypt.go b/builtin/logical/transit/path_encrypt.go index 321e920998943..c328951645a18 100644 --- a/builtin/logical/transit/path_encrypt.go +++ b/builtin/logical/transit/path_encrypt.go @@ -217,7 +217,7 @@ func decodeBatchRequestItems(src interface{}, dst *[]BatchRequestItem) error { func (b *backend) pathEncryptExistenceCheck(ctx context.Context, req *logical.Request, d *framework.FieldData) (bool, error) { name := d.Get("name").(string) - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) @@ -336,7 +336,7 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d } } - p, upserted, err = b.lm.GetPolicy(ctx, polReq, b.GetRandomReader()) + p, upserted, err = b.GetPolicy(ctx, polReq, b.GetRandomReader()) if err != nil { return nil, err } diff --git a/builtin/logical/transit/path_export.go b/builtin/logical/transit/path_export.go index 33a76cf33b738..3b0d97e15e735 100644 --- a/builtin/logical/transit/path_export.go +++ b/builtin/logical/transit/path_export.go @@ -64,7 +64,7 @@ func (b *backend) pathPolicyExportRead(ctx context.Context, req *logical.Request return logical.ErrorResponse(fmt.Sprintf("invalid export type: %s", exportType)), logical.ErrInvalidRequest } - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_hmac.go b/builtin/logical/transit/path_hmac.go index 025a39efdf01f..30a79a40789ab 100644 --- a/builtin/logical/transit/path_hmac.go +++ b/builtin/logical/transit/path_hmac.go @@ -96,7 +96,7 @@ func (b *backend) pathHMACWrite(ctx context.Context, req *logical.Request, d *fr } // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) @@ -224,7 +224,7 @@ func (b *backend) pathHMACVerify(ctx context.Context, req *logical.Request, d *f } // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_hmac_test.go b/builtin/logical/transit/path_hmac_test.go index b9d6bbc813325..756dc77e559f1 100644 --- a/builtin/logical/transit/path_hmac_test.go +++ b/builtin/logical/transit/path_hmac_test.go @@ -26,7 +26,7 @@ func TestTransit_HMAC(t *testing.T) { } // Now, change the key value to something we control - p, _, err := b.lm.GetPolicy(context.Background(), keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(context.Background(), keysutil.PolicyRequest{ Storage: storage, Name: "foo", }, b.GetRandomReader()) @@ -196,7 +196,7 @@ func TestTransit_batchHMAC(t *testing.T) { } // Now, change the key value to something we control - p, _, err := b.lm.GetPolicy(context.Background(), keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(context.Background(), keysutil.PolicyRequest{ Storage: storage, Name: "foo", }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_keys.go b/builtin/logical/transit/path_keys.go index 4cc25f66c4009..8c43ab593b5b4 100644 --- a/builtin/logical/transit/path_keys.go +++ b/builtin/logical/transit/path_keys.go @@ -162,7 +162,7 @@ func (b *backend) pathPolicyWrite(ctx context.Context, req *logical.Request, d * return logical.ErrorResponse(fmt.Sprintf("unknown key type %v", keyType)), logical.ErrInvalidRequest } - p, upserted, err := b.lm.GetPolicy(ctx, polReq, b.GetRandomReader()) + p, upserted, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) if err != nil { return nil, err } @@ -191,7 +191,7 @@ type asymKey struct { func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_rewrap.go b/builtin/logical/transit/path_rewrap.go index 9d473d256948d..c32fddc99976a 100644 --- a/builtin/logical/transit/path_rewrap.go +++ b/builtin/logical/transit/path_rewrap.go @@ -114,7 +114,7 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d * } // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: d.Get("name").(string), }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_rotate.go b/builtin/logical/transit/path_rotate.go index 3d2c2cdf40453..a74e69980512e 100644 --- a/builtin/logical/transit/path_rotate.go +++ b/builtin/logical/transit/path_rotate.go @@ -31,7 +31,7 @@ func (b *backend) pathRotateWrite(ctx context.Context, req *logical.Request, d * name := d.Get("name").(string) // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_sign_verify.go b/builtin/logical/transit/path_sign_verify.go index 659e6a2091c66..265d63cec1984 100644 --- a/builtin/logical/transit/path_sign_verify.go +++ b/builtin/logical/transit/path_sign_verify.go @@ -246,7 +246,7 @@ func (b *backend) pathSignWrite(ctx context.Context, req *logical.Request, d *fr sigAlgorithm := d.Get("signature_algorithm").(string) // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) @@ -464,7 +464,7 @@ func (b *backend) pathVerifyWrite(ctx context.Context, req *logical.Request, d * sigAlgorithm := d.Get("signature_algorithm").(string) // Get the policy - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_sign_verify_test.go b/builtin/logical/transit/path_sign_verify_test.go index 652dc186c9162..40df90838b19e 100644 --- a/builtin/logical/transit/path_sign_verify_test.go +++ b/builtin/logical/transit/path_sign_verify_test.go @@ -55,7 +55,7 @@ func testTransit_SignVerify_ECDSA(t *testing.T, bits int) { } // Now, change the key value to something we control - p, _, err := b.lm.GetPolicy(context.Background(), keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(context.Background(), keysutil.PolicyRequest{ Storage: storage, Name: "foo", }, b.GetRandomReader()) @@ -377,7 +377,7 @@ func TestTransit_SignVerify_ED25519(t *testing.T) { } // Get the keys for later - fooP, _, err := b.lm.GetPolicy(context.Background(), keysutil.PolicyRequest{ + fooP, _, err := b.GetPolicy(context.Background(), keysutil.PolicyRequest{ Storage: storage, Name: "foo", }, b.GetRandomReader()) @@ -385,7 +385,7 @@ func TestTransit_SignVerify_ED25519(t *testing.T) { t.Fatal(err) } - barP, _, err := b.lm.GetPolicy(context.Background(), keysutil.PolicyRequest{ + barP, _, err := b.GetPolicy(context.Background(), keysutil.PolicyRequest{ Storage: storage, Name: "bar", }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_trim.go b/builtin/logical/transit/path_trim.go index cec7a5648ef7f..d8587f1c18d49 100644 --- a/builtin/logical/transit/path_trim.go +++ b/builtin/logical/transit/path_trim.go @@ -40,7 +40,7 @@ func (b *backend) pathTrimUpdate() framework.OperationFunc { return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (resp *logical.Response, retErr error) { name := d.Get("name").(string) - p, _, err := b.lm.GetPolicy(ctx, keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(ctx, keysutil.PolicyRequest{ Storage: req.Storage, Name: name, }, b.GetRandomReader()) diff --git a/builtin/logical/transit/path_trim_test.go b/builtin/logical/transit/path_trim_test.go index 6b3dfaa9ec2fc..be989b1642459 100644 --- a/builtin/logical/transit/path_trim_test.go +++ b/builtin/logical/transit/path_trim_test.go @@ -36,7 +36,7 @@ func TestTransit_Trim(t *testing.T) { doReq(t, req) // Get the policy and check that the archive has correct number of keys - p, _, err := b.lm.GetPolicy(namespace.RootContext(nil), keysutil.PolicyRequest{ + p, _, err := b.GetPolicy(namespace.RootContext(nil), keysutil.PolicyRequest{ Storage: storage, Name: "aes", }, b.GetRandomReader()) diff --git a/sdk/helper/keysutil/lock_manager.go b/sdk/helper/keysutil/lock_manager.go index 039b05ad05356..6a2aab3c7cfd7 100644 --- a/sdk/helper/keysutil/lock_manager.go +++ b/sdk/helper/keysutil/lock_manager.go @@ -101,6 +101,24 @@ func (lm *LockManager) InvalidatePolicy(name string) { } } +func (lm *LockManager) RefreshCache(cacheSize int) error { + if lm.useCache { + switch { + case cacheSize < 0: + return errors.New("cache size must be greater or equal to zero") + case cacheSize == 0: + lm.cache = NewTransitSyncMap() + case cacheSize > 0: + newLRUCache, err := NewTransitLRU(cacheSize) + if err != nil { + return errwrap.Wrapf("failed to create cache: {{err}}", err) + } + lm.cache = newLRUCache + } + } + return nil +} + // RestorePolicy acquires an exclusive lock on the policy name and restores the // given policy along with the archive. func (lm *LockManager) RestorePolicy(ctx context.Context, storage logical.Storage, name, backup string, force bool) error { From 5f831bfebe811886a93647d79248a72b6ce3323f Mon Sep 17 00:00:00 2001 From: divyapola5 Date: Mon, 13 Sep 2021 14:21:27 -0500 Subject: [PATCH 8/8] Update code to acquire read and write locks appropriately --- builtin/logical/transit/backend.go | 7 +++++-- builtin/logical/transit/path_cache_config.go | 2 +- builtin/logical/transit/path_cache_config_test.go | 2 +- changelog/12418.txt | 3 ++- sdk/helper/keysutil/lock_manager.go | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 96393c92347fe..0e2f8264153b8 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -115,6 +115,8 @@ func GetCacheSizeFromStorage(ctx context.Context, s logical.Storage) (int, error // Update cache size and get policy func (b *backend) GetPolicy(ctx context.Context, polReq keysutil.PolicyRequest, rand io.Reader) (retP *keysutil.Policy, retUpserted bool, retErr error) { + // Acquire read lock to read cacheSizeChanged + b.configMutex.RLock() if b.lm.GetUseCache() && b.cacheSizeChanged { var err error currentCacheSize := b.lm.GetCacheSize() @@ -123,12 +125,13 @@ func (b *backend) GetPolicy(ctx context.Context, polReq keysutil.PolicyRequest, return nil, false, err } if currentCacheSize != storedCacheSize { - err = b.lm.RefreshCache(storedCacheSize) + err = b.lm.InitCache(storedCacheSize) if err != nil { return nil, false, err } } - // Acquire the lock to modify cacheSizeChanged + // Release the read lock and acquire the write lock + b.configMutex.RUnlock() b.configMutex.Lock() defer b.configMutex.Unlock() b.cacheSizeChanged = false diff --git a/builtin/logical/transit/path_cache_config.go b/builtin/logical/transit/path_cache_config.go index f3ca1649f7936..6610548ce1351 100644 --- a/builtin/logical/transit/path_cache_config.go +++ b/builtin/logical/transit/path_cache_config.go @@ -59,7 +59,7 @@ func (b *backend) pathCacheConfigWrite(ctx context.Context, req *logical.Request return nil, err } - err = b.lm.RefreshCache(cacheSize) + err = b.lm.InitCache(cacheSize) if err != nil { return nil, err } diff --git a/builtin/logical/transit/path_cache_config_test.go b/builtin/logical/transit/path_cache_config_test.go index 8d5f0195c7eb3..2d74129f95bfd 100644 --- a/builtin/logical/transit/path_cache_config_test.go +++ b/builtin/logical/transit/path_cache_config_test.go @@ -96,7 +96,7 @@ func TestTransit_CacheConfig(t *testing.T) { // Change the cache size to 1000 to mock the scenario where // current cache size and stored cache size are different and // a cache update is needed - b1.lm.RefreshCache(1000) + b1.lm.InitCache(1000) // Write a new policy which in its code path detects that cache size has changed // and refreshes the cache to 12345 diff --git a/changelog/12418.txt b/changelog/12418.txt index c9be8686b3167..5ec2f6055393b 100644 --- a/changelog/12418.txt +++ b/changelog/12418.txt @@ -1,3 +1,4 @@ ```release-note:bug -Enforce minimum cache size for transit backend +Enforce minimum cache size for transit backend. +Init cache size on transit backend without restart. ``` diff --git a/sdk/helper/keysutil/lock_manager.go b/sdk/helper/keysutil/lock_manager.go index 6a2aab3c7cfd7..c6a0a23d61457 100644 --- a/sdk/helper/keysutil/lock_manager.go +++ b/sdk/helper/keysutil/lock_manager.go @@ -101,7 +101,7 @@ func (lm *LockManager) InvalidatePolicy(name string) { } } -func (lm *LockManager) RefreshCache(cacheSize int) error { +func (lm *LockManager) InitCache(cacheSize int) error { if lm.useCache { switch { case cacheSize < 0: