From 3a7fc7bac9709d3a55b85f062a5e74ebfc1c039f Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Tue, 20 Feb 2024 12:02:42 -0600 Subject: [PATCH] Cache trusted cert values, invalidating when anything changes (#25421) (#25465) * Cache trusted cert values, invalidating when anything changes * rename to something more indicative * defer * changelog * Use an LRU cache rather than a static map so we can't use too much memory. Add docs, unit tests * Don't add to cache if disabled. But this races if just a bool, so make the disabled an atomic --- builtin/credential/cert/backend.go | 46 ++++++++++++++++++++-- builtin/credential/cert/path_certs.go | 10 +++-- builtin/credential/cert/path_config.go | 20 ++++++++-- builtin/credential/cert/path_crls.go | 3 ++ builtin/credential/cert/path_login.go | 32 ++++++++++++--- builtin/credential/cert/path_login_test.go | 17 ++++++++ changelog/25421.txt | 3 ++ go.mod | 1 + go.sum | 2 + website/content/api-docs/auth/cert.mdx | 2 + 10 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 changelog/25421.txt diff --git a/builtin/credential/cert/backend.go b/builtin/credential/cert/backend.go index 7bb0601f03d1..f7f1255595f3 100644 --- a/builtin/credential/cert/backend.go +++ b/builtin/credential/cert/backend.go @@ -16,12 +16,19 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" + lru "github.com/hashicorp/golang-lru/v2" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/ocsp" "github.com/hashicorp/vault/sdk/logical" ) -const operationPrefixCert = "cert" +const ( + operationPrefixCert = "cert" + trustedCertPath = "cert/" + + defaultRoleCacheSize = 200 + maxRoleCacheSize = 10000 +) func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { b := Backend() @@ -32,7 +39,11 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, } func Backend() *backend { - var b backend + // ignoring the error as it only can occur with <= 0 size + cache, _ := lru.New[string, *trusted](defaultRoleCacheSize) + b := backend{ + trustedCache: cache, + } b.Backend = &framework.Backend{ Help: backendHelp, PathsSpecial: &logical.Paths{ @@ -59,6 +70,13 @@ func Backend() *backend { return &b } +type trusted struct { + pool *x509.CertPool + trusted []*ParsedCert + trustedNonCAs []*ParsedCert + ocspConf *ocsp.VerifyConfig +} + type backend struct { *framework.Backend MapCertId *framework.PathMap @@ -68,6 +86,9 @@ type backend struct { ocspClientMutex sync.RWMutex ocspClient *ocsp.Client configUpdated atomic.Bool + + trustedCache *lru.Cache[string, *trusted] + trustedCacheDisabled atomic.Bool } func (b *backend) initialize(ctx context.Context, req *logical.InitializationRequest) error { @@ -98,6 +119,7 @@ func (b *backend) invalidate(_ context.Context, key string) { case key == "config": b.configUpdated.Store(true) } + b.flushTrustedCache() } func (b *backend) initOCSPClient(cacheSize int) { @@ -109,9 +131,21 @@ func (b *backend) initOCSPClient(cacheSize int) { func (b *backend) updatedConfig(config *config) { b.ocspClientMutex.Lock() defer b.ocspClientMutex.Unlock() + + switch { + case config.RoleCacheSize < 0: + // Just to clean up memory + b.trustedCacheDisabled.Store(true) + b.trustedCache.Purge() + case config.RoleCacheSize == 0: + config.RoleCacheSize = defaultRoleCacheSize + fallthrough + default: + b.trustedCache.Resize(config.RoleCacheSize) + b.trustedCacheDisabled.Store(false) + } b.initOCSPClient(config.OcspCacheSize) b.configUpdated.Store(false) - return } func (b *backend) fetchCRL(ctx context.Context, storage logical.Storage, name string, crl *CRLInfo) error { @@ -161,6 +195,12 @@ func (b *backend) storeConfig(ctx context.Context, storage logical.Storage, conf return nil } +func (b *backend) flushTrustedCache() { + if b.trustedCache != nil { // defensive + b.trustedCache.Purge() + } +} + const backendHelp = ` The "cert" credential provider allows authentication using TLS client certificates. A client connects to Vault and uses diff --git a/builtin/credential/cert/path_certs.go b/builtin/credential/cert/path_certs.go index 4e8cdc274358..6ddc5ec78292 100644 --- a/builtin/credential/cert/path_certs.go +++ b/builtin/credential/cert/path_certs.go @@ -235,7 +235,7 @@ certificate.`, } func (b *backend) Cert(ctx context.Context, s logical.Storage, n string) (*CertEntry, error) { - entry, err := s.Get(ctx, "cert/"+strings.ToLower(n)) + entry, err := s.Get(ctx, trustedCertPath+strings.ToLower(n)) if err != nil { return nil, err } @@ -268,7 +268,8 @@ func (b *backend) Cert(ctx context.Context, s logical.Storage, n string) (*CertE } func (b *backend) pathCertDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - err := req.Storage.Delete(ctx, "cert/"+strings.ToLower(d.Get("name").(string))) + defer b.flushTrustedCache() + err := req.Storage.Delete(ctx, trustedCertPath+strings.ToLower(d.Get("name").(string))) if err != nil { return nil, err } @@ -276,7 +277,7 @@ func (b *backend) pathCertDelete(ctx context.Context, req *logical.Request, d *f } func (b *backend) pathCertList(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - certs, err := req.Storage.List(ctx, "cert/") + certs, err := req.Storage.List(ctx, trustedCertPath) if err != nil { return nil, err } @@ -333,6 +334,7 @@ func (b *backend) pathCertRead(ctx context.Context, req *logical.Request, d *fra } func (b *backend) pathCertWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + defer b.flushTrustedCache() name := strings.ToLower(d.Get("name").(string)) cert, err := b.Cert(ctx, req.Storage, name) @@ -475,7 +477,7 @@ func (b *backend) pathCertWrite(ctx context.Context, req *logical.Request, d *fr } // Store it - entry, err := logical.StorageEntryJSON("cert/"+name, cert) + entry, err := logical.StorageEntryJSON(trustedCertPath+name, cert) if err != nil { return nil, err } diff --git a/builtin/credential/cert/path_config.go b/builtin/credential/cert/path_config.go index 3eb052118fb0..1183775f6bc4 100644 --- a/builtin/credential/cert/path_config.go +++ b/builtin/credential/cert/path_config.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -const maxCacheSize = 100000 +const maxOcspCacheSize = 100000 func pathConfig(b *backend) *framework.Path { return &framework.Path{ @@ -37,6 +37,11 @@ func pathConfig(b *backend) *framework.Path { Default: 100, Description: `The size of the in memory OCSP response cache, shared by all configured certs`, }, + "role_cache_size": { + Type: framework.TypeInt, + Default: defaultRoleCacheSize, + Description: `The size of the in memory role cache`, + }, }, Operations: map[logical.Operation]framework.OperationHandler{ @@ -70,11 +75,18 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat } if cacheSizeRaw, ok := data.GetOk("ocsp_cache_size"); ok { cacheSize := cacheSizeRaw.(int) - if cacheSize < 2 || cacheSize > maxCacheSize { - return logical.ErrorResponse("invalid cache size, must be >= 2 and <= %d", maxCacheSize), nil + if cacheSize < 2 || cacheSize > maxOcspCacheSize { + return logical.ErrorResponse("invalid ocsp cache size, must be >= 2 and <= %d", maxOcspCacheSize), nil } config.OcspCacheSize = cacheSize } + if cacheSizeRaw, ok := data.GetOk("role_cache_size"); ok { + cacheSize := cacheSizeRaw.(int) + if (cacheSize < 0 && cacheSize != -1) || cacheSize > maxRoleCacheSize { + return logical.ErrorResponse("invalid role cache size, must be <= %d or -1 to disable role caching", maxRoleCacheSize), nil + } + config.RoleCacheSize = cacheSize + } if err := b.storeConfig(ctx, req.Storage, config); err != nil { return nil, err } @@ -91,6 +103,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, d *f "disable_binding": cfg.DisableBinding, "enable_identity_alias_metadata": cfg.EnableIdentityAliasMetadata, "ocsp_cache_size": cfg.OcspCacheSize, + "role_cache_size": cfg.RoleCacheSize, } return &logical.Response{ @@ -119,4 +132,5 @@ type config struct { DisableBinding bool `json:"disable_binding"` EnableIdentityAliasMetadata bool `json:"enable_identity_alias_metadata"` OcspCacheSize int `json:"ocsp_cache_size"` + RoleCacheSize int `json:"role_cache_size"` } diff --git a/builtin/credential/cert/path_crls.go b/builtin/credential/cert/path_crls.go index 823aced99149..f38654869d73 100644 --- a/builtin/credential/cert/path_crls.go +++ b/builtin/credential/cert/path_crls.go @@ -190,6 +190,7 @@ func (b *backend) pathCRLDelete(ctx context.Context, req *logical.Request, d *fr b.crlUpdateMutex.Lock() defer b.crlUpdateMutex.Unlock() + defer b.flushTrustedCache() _, ok := b.crls[name] if !ok { @@ -313,6 +314,8 @@ func (b *backend) setCRL(ctx context.Context, storage logical.Storage, certList } b.crls[name] = crlInfo + b.flushTrustedCache() + return err } diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index ea8ba9748bcc..6b1f279dfd7b 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -257,7 +257,7 @@ func (b *backend) verifyCredentials(ctx context.Context, req *logical.Request, d } // Load the trusted certificates and other details - roots, trusted, trustedNonCAs, verifyConf := b.loadTrustedCerts(ctx, req.Storage, certName) + roots, trusted, trustedNonCAs, verifyConf := b.getTrustedCerts(ctx, req.Storage, certName) // Get the list of full chains matching the connection and validates the // certificate itself @@ -581,10 +581,21 @@ func (b *backend) certificateExtensionsMetadata(clientCert *x509.Certificate, co return metadata } +// getTrustedCerts is used to load all the trusted certificates from the backend, cached + +func (b *backend) getTrustedCerts(ctx context.Context, storage logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert, conf *ocsp.VerifyConfig) { + if !b.trustedCacheDisabled.Load() { + if trusted, found := b.trustedCache.Get(certName); found { + return trusted.pool, trusted.trusted, trusted.trustedNonCAs, trusted.ocspConf + } + } + return b.loadTrustedCerts(ctx, storage, certName) +} + // loadTrustedCerts is used to load all the trusted certificates from the backend -func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, certName string) (pool *x509.CertPool, trusted []*ParsedCert, trustedNonCAs []*ParsedCert, conf *ocsp.VerifyConfig) { +func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, certName string) (pool *x509.CertPool, trustedCerts []*ParsedCert, trustedNonCAs []*ParsedCert, conf *ocsp.VerifyConfig) { pool = x509.NewCertPool() - trusted = make([]*ParsedCert, 0) + trustedCerts = make([]*ParsedCert, 0) trustedNonCAs = make([]*ParsedCert, 0) var names []string @@ -592,7 +603,7 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, names = append(names, certName) } else { var err error - names, err = storage.List(ctx, "cert/") + names, err = storage.List(ctx, trustedCertPath) if err != nil { b.Logger().Error("failed to list trusted certs", "error", err) return @@ -601,7 +612,7 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, conf = &ocsp.VerifyConfig{} for _, name := range names { - entry, err := b.Cert(ctx, storage, strings.TrimPrefix(name, "cert/")) + entry, err := b.Cert(ctx, storage, strings.TrimPrefix(name, trustedCertPath)) if err != nil { b.Logger().Error("failed to load trusted cert", "name", name, "error", err) continue @@ -630,7 +641,7 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, } // Create a ParsedCert entry - trusted = append(trusted, &ParsedCert{ + trustedCerts = append(trustedCerts, &ParsedCert{ Entry: entry, Certificates: parsed, }) @@ -646,6 +657,15 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage, conf.QueryAllServers = conf.QueryAllServers || entry.OcspQueryAllServers } } + + if !b.trustedCacheDisabled.Load() { + b.trustedCache.Add(certName, &trusted{ + pool: pool, + trusted: trustedCerts, + trustedNonCAs: trustedNonCAs, + ocspConf: conf, + }) + } return } diff --git a/builtin/credential/cert/path_login_test.go b/builtin/credential/cert/path_login_test.go index 53be24c3117a..fcc7b37c51a6 100644 --- a/builtin/credential/cert/path_login_test.go +++ b/builtin/credential/cert/path_login_test.go @@ -94,6 +94,10 @@ func TestCert_RoleResolve(t *testing.T) { testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false), testAccStepLoginWithName(t, connState, "web"), testAccStepResolveRoleWithName(t, connState, "web"), + // Test with caching disabled + testAccStepSetRoleCacheSize(t, -1), + testAccStepLoginWithName(t, connState, "web"), + testAccStepResolveRoleWithName(t, connState, "web"), }, }) } @@ -151,10 +155,23 @@ func TestCert_RoleResolveWithoutProvidingCertName(t *testing.T) { testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false), testAccStepLoginWithName(t, connState, "web"), testAccStepResolveRoleWithEmptyDataMap(t, connState, "web"), + testAccStepSetRoleCacheSize(t, -1), + testAccStepLoginWithName(t, connState, "web"), + testAccStepResolveRoleWithEmptyDataMap(t, connState, "web"), }, }) } +func testAccStepSetRoleCacheSize(t *testing.T, size int) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.UpdateOperation, + Path: "config", + Data: map[string]interface{}{ + "role_cache_size": size, + }, + } +} + func testAccStepResolveRoleWithEmptyDataMap(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.ResolveRoleOperation, diff --git a/changelog/25421.txt b/changelog/25421.txt new file mode 100644 index 000000000000..06adb1403e30 --- /dev/null +++ b/changelog/25421.txt @@ -0,0 +1,3 @@ +```release-note:improvement +auth/cert: Cache trusted certs to reduce memory usage and improve performance of logins. +``` \ No newline at end of file diff --git a/go.mod b/go.mod index b9f4a6604e84..76f07dca6286 100644 --- a/go.mod +++ b/go.mod @@ -115,6 +115,7 @@ require ( github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/golang-lru v0.5.4 + github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/hashicorp/hcl v1.0.1-vault-5 github.com/hashicorp/hcl/v2 v2.16.2 github.com/hashicorp/hcp-link v0.2.1 diff --git a/go.sum b/go.sum index 5ccab2ec4bd1..3954ee31cf65 100644 --- a/go.sum +++ b/go.sum @@ -2101,6 +2101,8 @@ github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc= github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/hcl v1.0.1-vault-5 h1:kI3hhbbyzr4dldA8UdTb7ZlVVlI2DACdCfz31RPDgJM= github.com/hashicorp/hcl v1.0.1-vault-5/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM= diff --git a/website/content/api-docs/auth/cert.mdx b/website/content/api-docs/auth/cert.mdx index 2420357b0545..c9fdeef5cd76 100644 --- a/website/content/api-docs/auth/cert.mdx +++ b/website/content/api-docs/auth/cert.mdx @@ -360,6 +360,8 @@ Configuration options for the method. `allowed_metadata_extensions` will be stored in the alias - `ocsp_cache_size` `(int: 100)` - The size of the OCSP response LRU cache. Note that this cache is used for all configured certificates. +- `role_cache_size` `(int: 200)` - The size of the role cache. Use `-1` to disable + role caching. ### Sample payload