From f719cd12722009db7c945228807d8e50de0e424f Mon Sep 17 00:00:00 2001 From: Divya Chandrasekaran Date: Mon, 6 Nov 2023 11:36:30 -0800 Subject: [PATCH 1/7] Applied OSS changes patch --- vault/core.go | 2 +- vault/logical_system_quotas.go | 95 +++++++++++++++++++++++++++---- vault/quotas/quotas.go | 62 +++++++++++++++----- vault/quotas/quotas_rate_limit.go | 4 ++ 4 files changed, 138 insertions(+), 25 deletions(-) diff --git a/vault/core.go b/vault/core.go index 8a641c3f8a24..021051944a6b 100644 --- a/vault/core.go +++ b/vault/core.go @@ -3452,7 +3452,7 @@ func (c *Core) ApplyRateLimitQuota(ctx context.Context, req *quotas.Request) (qu if c.quotaManager != nil { // skip rate limit checks for paths that are exempt from rate limiting - if c.quotaManager.RateLimitPathExempt(req.Path) { + if c.quotaManager.RateLimitPathExempt(req.Path, req.NamespacePath) { return resp, nil } diff --git a/vault/logical_system_quotas.go b/vault/logical_system_quotas.go index 1cc5be3a5e56..2a12775d0f0b 100644 --- a/vault/logical_system_quotas.go +++ b/vault/logical_system_quotas.go @@ -5,6 +5,7 @@ package vault import ( "context" + "errors" "net/http" "strings" "time" @@ -16,6 +17,13 @@ import ( "github.com/hashicorp/vault/vault/quotas" ) +var ( + ErrExemptRateLimitsOnChildNs = errors.New("exempt paths can only be be configured in the root namespace") + ErrInvalidQuotaDeletion = "cannot delete quota configured for a parent namespace" + ErrInvalidQuotaUpdate = "quotas in parent namespaces cannot be updated" + ErrInvalidQuotaOnParentNs = "quotas cannot be configured for parent namespaces" +) + // quotasPaths returns paths that enable quota management func (b *SystemBackend) quotasPaths() []*framework.Path { return []*framework.Path{ @@ -39,6 +47,10 @@ func (b *SystemBackend) quotasPaths() []*framework.Path { Type: framework.TypeBool, Description: "If set, additional rate limit quota HTTP headers will be added to responses.", }, + "absolute_rate_limit_exempt_paths": { + Type: framework.TypeStringSlice, + Description: "Specifies the list of exempt global paths from all rate limit quotas. If empty no global paths will be exempt.", + }, }, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ @@ -73,6 +85,10 @@ func (b *SystemBackend) quotasPaths() []*framework.Path { Type: framework.TypeStringSlice, Required: true, }, + "absolute_rate_limit_exempt_paths": { + Type: framework.TypeStringSlice, + Required: true, + }, }, }}, }, @@ -220,6 +236,11 @@ from any further requests until after the 'block_interval' has elapsed.`, func (b *SystemBackend) handleQuotasConfigUpdate() framework.OperationFunc { return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + ns, err := namespace.FromContext(ctx) + if err != nil { + return nil, err + } + config, err := quotas.LoadConfig(ctx, b.Core.systemBarrierView) if err != nil { return nil, err @@ -227,13 +248,30 @@ func (b *SystemBackend) handleQuotasConfigUpdate() framework.OperationFunc { config.EnableRateLimitAuditLogging = d.Get("enable_rate_limit_audit_logging").(bool) config.EnableRateLimitResponseHeaders = d.Get("enable_rate_limit_response_headers").(bool) - config.RateLimitExemptPaths = d.Get("rate_limit_exempt_paths").([]string) + + _, ok := d.GetOk("absolute_rate_limit_exempt_paths") + // Global rate limit exempt paths can only be defined in the root namespace + if ns.ID != namespace.RootNamespaceID && ok { + return nil, ErrExemptRateLimitsOnChildNs + } + + _, ok = d.GetOk("rate_limit_exempt_paths") + // Relative rate limit exempt paths can only be defined in the root namespace + if ns.ID != namespace.RootNamespaceID && ok { + return nil, ErrExemptRateLimitsOnChildNs + } + + // Set rate limit exempt paths to correct configuration fields only if in root namespace + if ns.ID == namespace.RootNamespaceID { + config.RateLimitExemptPaths = d.Get("rate_limit_exempt_paths").([]string) + config.AbsoluteRateLimitExemptPaths = d.Get("absolute_rate_limit_exempt_paths").([]string) + } entry, err := logical.StorageEntryJSON(quotas.ConfigPath, config) if err != nil { return nil, err } - if err := req.Storage.Put(ctx, entry); err != nil { + if err := b.Core.systemBarrierView.Put(ctx, entry); err != nil { return nil, err } @@ -241,12 +279,13 @@ func (b *SystemBackend) handleQuotasConfigUpdate() framework.OperationFunc { if err != nil { return nil, err } - if err := req.Storage.Put(ctx, entry); err != nil { + if err := b.Core.systemBarrierView.Put(ctx, entry); err != nil { return nil, err } b.Core.quotaManager.SetEnableRateLimitAuditLogging(config.EnableRateLimitAuditLogging) b.Core.quotaManager.SetEnableRateLimitResponseHeaders(config.EnableRateLimitResponseHeaders) + b.Core.quotaManager.SetGlobalRateLimitExemptPaths(config.AbsoluteRateLimitExemptPaths) b.Core.quotaManager.SetRateLimitExemptPaths(config.RateLimitExemptPaths) return nil, nil @@ -261,6 +300,7 @@ func (b *SystemBackend) handleQuotasConfigRead() framework.OperationFunc { "enable_rate_limit_audit_logging": config.EnableRateLimitAuditLogging, "enable_rate_limit_response_headers": config.EnableRateLimitResponseHeaders, "rate_limit_exempt_paths": config.RateLimitExemptPaths, + "absolute_rate_limit_exempt_paths": config.AbsoluteRateLimitExemptPaths, }, }, nil } @@ -297,7 +337,29 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc { return logical.ErrorResponse("'block' is invalid"), nil } - mountPath := sanitizePath(d.Get("path").(string)) + rawPath := sanitizePath(d.Get("path").(string)) + mountPath := rawPath + + // If the quota creation endpoint is being called from the privileged namespace, we want to prepend the namespace to the path + currentNamespace, err := namespace.FromContext(ctx) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } + if currentNamespace.ID != namespace.RootNamespaceID && !strings.HasPrefix(mountPath, currentNamespace.Path) { + return logical.ErrorResponse(ErrInvalidQuotaOnParentNs), nil + } + + // If there is a quota by the same name that was configured on a parent namespace, prohibit updating this quota + if currentNamespace.ID != namespace.RootNamespaceID { + quota, err := b.Core.quotaManager.QuotaByName(qType, name) + if err != nil { + return nil, err + } + if quota != nil && !strings.HasPrefix(quota.GetNamespacePath(), currentNamespace.Path) { + return logical.ErrorResponse(ErrInvalidQuotaUpdate), nil + } + } + ns := b.Core.namespaceByPath(mountPath) if ns.ID != namespace.RootNamespaceID { mountPath = strings.TrimPrefix(mountPath, ns.Path) @@ -337,7 +399,7 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc { var inheritable bool // All global quotas should be inherited by default - if ns.Path == "" { + if rawPath == "" { inheritable = true } @@ -347,14 +409,14 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc { if pathSuffix != "" || role != "" || mountPath != "" { return logical.ErrorResponse("only namespace quotas can be configured as inheritable"), nil } - } else if ns.Path == "" { + } else if rawPath == "" { // User should not try to configure a global quota that cannot be inherited return logical.ErrorResponse("all global quotas must be inheritable"), nil } } // User should not try to configure a global quota to be uninheritable - if ns.Path == "" && !inheritable { + if rawPath == "" && !inheritable { return logical.ErrorResponse("all global quotas must be inheritable"), nil } @@ -391,13 +453,12 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc { rlq.BlockInterval = blockInterval quota = rlq } - entry, err := logical.StorageEntryJSON(quotas.QuotaStoragePath(qType, name), quota) if err != nil { return nil, err } - if err := req.Storage.Put(ctx, entry); err != nil { + if err := b.Core.systemBarrierView.storage.Put(ctx, entry); err != nil { return nil, err } @@ -451,7 +512,21 @@ func (b *SystemBackend) handleRateLimitQuotasDelete() framework.OperationFunc { name := d.Get("name").(string) qType := quotas.TypeRateLimit.String() - if err := req.Storage.Delete(ctx, quotas.QuotaStoragePath(qType, name)); err != nil { + ns, err := namespace.FromContext(ctx) + if err != nil { + return nil, err + } + if ns.ID != namespace.RootNamespaceID { + quota, err := b.Core.quotaManager.QuotaByName(qType, name) + if err != nil { + return nil, err + } + if quota != nil && !strings.HasPrefix(quota.GetNamespacePath(), ns.Path) { + return logical.ErrorResponse(ErrInvalidQuotaDeletion), nil + } + } + + if err := b.Core.systemBarrierView.Delete(ctx, quotas.QuotaStoragePath(qType, name)); err != nil { return nil, err } diff --git a/vault/quotas/quotas.go b/vault/quotas/quotas.go index cefa3d13cf10..22a52b2a21e5 100644 --- a/vault/quotas/quotas.go +++ b/vault/quotas/quotas.go @@ -161,7 +161,8 @@ type Manager struct { // config containing operator preferences and quota behaviors config *Config - rateLimitPathManager *pathmanager.PathManager + rateLimitPathManager *pathmanager.PathManager + globalRateLimitPathManager *pathmanager.PathManager storage logical.Storage ctx context.Context @@ -214,6 +215,9 @@ type Quota interface { // Inheritable indicates if this quota can be applied to child namespaces IsInheritable() bool + // GetNamespacePath gets the namespace path of the quota + GetNamespacePath() string + // handleRemount updates the mount and namesapce paths of the quota handleRemount(string, string) } @@ -247,7 +251,14 @@ type Config struct { // RateLimitExemptPaths defines the set of exempt paths used for all rate limit // quotas. Any request path that exists in this set is exempt from rate limiting. // If the set is empty, no paths are exempt. + // The paths specified here are relative and are appended to every namespace during search. RateLimitExemptPaths []string `json:"rate_limit_exempt_paths"` + + // AbsoluteRateLimitExemptPaths defines the set of exempt paths used for all rate limit + // quotas. Any request path that exists in this set is exempt from rate limiting. + // If it is empty, no paths are exempt. + // The paths specified here are absolute paths, and can only be set from the root namespace + AbsoluteRateLimitExemptPaths []string `json:"absolute_rate_limit_exempt_paths"` } // Request contains information required by the quota manager to query and @@ -282,14 +293,15 @@ func NewManager(logger log.Logger, walkFunc leaseWalkFunc, ms *metricsutil.Clust } manager := &Manager{ - db: db, - logger: logger, - metricSink: ms, - rateLimitPathManager: pathmanager.New(), - config: new(Config), - quotaLock: &locking.SyncRWMutex{}, - quotaConfigLock: &locking.SyncRWMutex{}, - dbAndCacheLock: &locking.SyncRWMutex{}, + db: db, + logger: logger, + metricSink: ms, + rateLimitPathManager: pathmanager.New(), + globalRateLimitPathManager: pathmanager.New(), + config: new(Config), + quotaLock: new(locking.DeadlockRWMutex), + quotaConfigLock: new(locking.DeadlockRWMutex), + dbAndCacheLock: new(locking.DeadlockRWMutex), } if detectDeadlocks { @@ -764,6 +776,25 @@ func (m *Manager) setRateLimitExemptPathsLocked(vals []string) { m.rateLimitPathManager.AddPaths(vals) } +// SetGlobalRateLimitExemptPaths updates the global rate limit exempt paths in the Manager's +// configuration in addition to updating the path manager. Every call to +// SetGlobalRateLimitExemptPaths will wipe out the existing path manager and set the +// paths based on the provided argument. +func (m *Manager) SetGlobalRateLimitExemptPaths(vals []string) { + m.quotaConfigLock.Lock() + defer m.quotaConfigLock.Unlock() + m.setGlobalRateLimitExemptPathsLocked(vals) +} + +func (m *Manager) setGlobalRateLimitExemptPathsLocked(vals []string) { + if vals == nil { + vals = []string{} + } + m.config.AbsoluteRateLimitExemptPaths = vals + m.globalRateLimitPathManager = pathmanager.New() + m.globalRateLimitPathManager.AddPaths(vals) +} + // RateLimitAuditLoggingEnabled returns if the quota configuration allows audit // logging of request rejections due to rate limiting quota rule violations. func (m *Manager) RateLimitAuditLoggingEnabled() bool { @@ -785,15 +816,18 @@ func (m *Manager) RateLimitResponseHeadersEnabled() bool { // RateLimitPathExempt returns a boolean dictating if a given path is exempt from // any rate limit quota. If not rate limit path manager is defined, false is // returned. -func (m *Manager) RateLimitPathExempt(path string) bool { +func (m *Manager) RateLimitPathExempt(path string, namespacePath string) bool { m.quotaConfigLock.RLock() defer m.quotaConfigLock.RUnlock() if m.rateLimitPathManager == nil { return false } - - return m.rateLimitPathManager.HasPath(path) + globalRateLimitPath := path + if namespacePath != "root" { + globalRateLimitPath = strings.Join([]string{namespace.Canonicalize(namespacePath), path}, "") + } + return m.globalRateLimitPathManager.HasPath(globalRateLimitPath) || m.rateLimitPathManager.HasPath(path) } // Config returns the operator preferences in the quota manager @@ -990,6 +1024,7 @@ func (m *Manager) Invalidate(key string) { m.SetEnableRateLimitAuditLogging(config.EnableRateLimitAuditLogging) m.SetEnableRateLimitResponseHeaders(config.EnableRateLimitResponseHeaders) + m.SetGlobalRateLimitExemptPaths(config.AbsoluteRateLimitExemptPaths) m.SetRateLimitExemptPaths(config.RateLimitExemptPaths) default: @@ -1163,8 +1198,7 @@ func (m *Manager) setupQuotaType(ctx context.Context, storage logical.Storage, q return nil } -// QuotaStoragePath returns the storage path suffix for persisting the quota -// rule. +// QuotaStoragePath returns the storage path suffix for persisting the quota rule. func QuotaStoragePath(quotaType, name string) string { return path.Join(StoragePrefix+quotaType, name) } diff --git a/vault/quotas/quotas_rate_limit.go b/vault/quotas/quotas_rate_limit.go index 7a66926ff5e1..a648f92d1a4f 100644 --- a/vault/quotas/quotas_rate_limit.go +++ b/vault/quotas/quotas_rate_limit.go @@ -89,6 +89,10 @@ type RateLimitQuota struct { closePurgeBlockedCh chan struct{} } +func (q *RateLimitQuota) GetNamespacePath() string { + return q.NamespacePath +} + // NewRateLimitQuota creates a quota checker for imposing limits on the number // of requests in a given interval. An interval time duration of zero may be // provided, which will default to 1s when initialized. An optional block From b65f32d9ea5ed4c472b4887bed13e6a1a2e8d19a Mon Sep 17 00:00:00 2001 From: Divya Chandrasekaran Date: Mon, 6 Nov 2023 11:41:53 -0800 Subject: [PATCH 2/7] Added changelog --- changelog/24040.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/24040.txt diff --git a/changelog/24040.txt b/changelog/24040.txt new file mode 100644 index 000000000000..45bd8a1cbf62 --- /dev/null +++ b/changelog/24040.txt @@ -0,0 +1,3 @@ +```release-note:feature +**Quotas in Privileged Namespaces**: Enable creation/update/deletion of quotas from the privileged namespace +``` \ No newline at end of file From 7a3594c2328a37ac173ccc3832d0075a4ee0253d Mon Sep 17 00:00:00 2001 From: Divya Chandrasekaran Date: Mon, 6 Nov 2023 11:59:23 -0800 Subject: [PATCH 3/7] Fix lease count quota issues with stub --- vault/quotas/quotas_util.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vault/quotas/quotas_util.go b/vault/quotas/quotas_util.go index e758a148bb90..eb4999ca5eff 100644 --- a/vault/quotas/quotas_util.go +++ b/vault/quotas/quotas_util.go @@ -10,8 +10,6 @@ import ( log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/metricsutil" - - "github.com/hashicorp/go-memdb" ) func quotaTypes() []string { @@ -43,6 +41,10 @@ func (*entManager) Reset() error { type LeaseCountQuota struct{} +func (l LeaseCountQuota) GetNamespacePath() string { + panic("implement me") +} + func (l LeaseCountQuota) IsInheritable() bool { panic("implement me") } From 621ebf5766973983748bba0e15732cc997461fe2 Mon Sep 17 00:00:00 2001 From: divyaac Date: Mon, 6 Nov 2023 12:15:51 -0800 Subject: [PATCH 4/7] Update quotas_util.go --- vault/quotas/quotas_util.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vault/quotas/quotas_util.go b/vault/quotas/quotas_util.go index eb4999ca5eff..7b53d3e73068 100644 --- a/vault/quotas/quotas_util.go +++ b/vault/quotas/quotas_util.go @@ -10,6 +10,9 @@ import ( log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/metricsutil" + + "github.com/hashicorp/go-memdb" + ) func quotaTypes() []string { From 148ddd9d5835e0ad064504b9ecc1074b477c4f3f Mon Sep 17 00:00:00 2001 From: divyaac Date: Mon, 6 Nov 2023 12:23:57 -0800 Subject: [PATCH 5/7] Update quotas_util.go --- vault/quotas/quotas_util.go | 1 - 1 file changed, 1 deletion(-) diff --git a/vault/quotas/quotas_util.go b/vault/quotas/quotas_util.go index 7b53d3e73068..f2d1e1d80c85 100644 --- a/vault/quotas/quotas_util.go +++ b/vault/quotas/quotas_util.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/go-memdb" - ) func quotaTypes() []string { From 6eb445e0d8c06c7f2814abbd52ae0f77f9581c90 Mon Sep 17 00:00:00 2001 From: Divya Chandrasekaran Date: Mon, 6 Nov 2023 13:23:11 -0800 Subject: [PATCH 6/7] Edit quota manager to initialize with sync --- vault/quotas/quotas.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vault/quotas/quotas.go b/vault/quotas/quotas.go index 22a52b2a21e5..e63836589e48 100644 --- a/vault/quotas/quotas.go +++ b/vault/quotas/quotas.go @@ -11,7 +11,6 @@ import ( "strings" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-memdb" "github.com/hashicorp/vault/helper/locking" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" @@ -299,9 +298,9 @@ func NewManager(logger log.Logger, walkFunc leaseWalkFunc, ms *metricsutil.Clust rateLimitPathManager: pathmanager.New(), globalRateLimitPathManager: pathmanager.New(), config: new(Config), - quotaLock: new(locking.DeadlockRWMutex), - quotaConfigLock: new(locking.DeadlockRWMutex), - dbAndCacheLock: new(locking.DeadlockRWMutex), + quotaLock: &locking.SyncRWMutex{}, + quotaConfigLock: &locking.SyncRWMutex{}, + dbAndCacheLock: &locking.SyncRWMutex{}, } if detectDeadlocks { From afa2772f4216ee1d0d197a9adba13a7cb4005b31 Mon Sep 17 00:00:00 2001 From: divyaac Date: Mon, 6 Nov 2023 13:43:07 -0800 Subject: [PATCH 7/7] Update quotas.go --- vault/quotas/quotas.go | 1 + 1 file changed, 1 insertion(+) diff --git a/vault/quotas/quotas.go b/vault/quotas/quotas.go index e63836589e48..05fc7a7130bb 100644 --- a/vault/quotas/quotas.go +++ b/vault/quotas/quotas.go @@ -11,6 +11,7 @@ import ( "strings" log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-memdb" "github.com/hashicorp/vault/helper/locking" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace"