scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-11-01 19:17:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

VAULT-21474 Run oidcPeriodicFunc for each namespace id store (#29312)

Browse Source 
* run oidcPeriodicFunc for each namespace id store

* remove unused noNamespace var

* properly check for errors getting namespace

not sure why I decided to ignore the NoNamespace error before
or not log the unexpected error, that doesn't make sense.

* add changelog

* improve changelog

* remove too many namespace warning for OIDC rotations

this was already in the ENT PR, I had already checked that the file didn't exist on CE before but somehow I missed it.
This commit is contained in:
Bruno Oliveira de Souza
2025-01-31 13:04:04 -03:00
committed by GitHub
parent 6d5759ecb3
commit d127c4de93
5 changed files with 90 additions and 144 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
changelog/29312.txt Normal file
View File

@@ -0,0 +1,5 @@
```release-note:bug
identity/oidc (enterprise): Fix delays in rotation and invalidation of OIDC keys when there are too many namespaces.
The Cache-Control header returned by the identity/oidc/.well-known/keys endpoint now depends only on the named keys for
the queried namespace.
```

5
vault/identity_store.go
View File

@@ -122,7 +122,7 @@ func NewIdentityStore(ctx context.Context, core *Core, config *logical.BackendCo
},
},
PeriodicFunc: func(ctx context.Context, req *logical.Request) error {
iStore.oidcPeriodicFunc(ctx)
iStore.oidcPeriodicFunc(ctx, req.Storage)
return nil
},
@@ -885,8 +885,7 @@ func (i *IdentityStore) invalidateOIDCToken(ctx context.Context) {
return
}
// Wipe the cache for the requested namespace. This will also clear
// the shared namespace as well.
// Wipe the cache for the requested namespace
if err := i.oidcCache.Flush(ns); err != nil {
i.logger.Error("error flushing oidc cache", "error", err)
return

121
vault/identity_store_oidc.go
View File

@@ -126,9 +126,6 @@ type oidcCache struct {
var (
errNilNamespace = errors.New("nil namespace in oidc cache request")
// pseudo-namespace for cache items that don't belong to any real namespace.
noNamespace = &namespace.Namespace{ID: "__NO_NAMESPACE"}
reservedClaims = []string{
"iat", "aud", "exp", "iss",
"sub", "namespace", "nonce",
@@ -1503,10 +1500,10 @@ func (i *IdentityStore) pathOIDCDiscovery(ctx context.Context, req *logical.Requ
// getKeysCacheControlHeader returns the cache control header for all public
// keys at the .well-known/keys endpoint
func (i *IdentityStore) getKeysCacheControlHeader() (string, error) {
func (i *IdentityStore) getKeysCacheControlHeader(ns *namespace.Namespace) (string, error) {
// if jwksCacheControlMaxAge is set use that, otherwise fall back on the
// more conservative nextRun values
jwksCacheControlMaxAge, ok, err := i.oidcCache.Get(noNamespace, "jwksCacheControlMaxAge")
jwksCacheControlMaxAge, ok, err := i.oidcCache.Get(ns, "jwksCacheControlMaxAge")
if err != nil {
return "", err
}
@@ -1518,7 +1515,7 @@ func (i *IdentityStore) getKeysCacheControlHeader() (string, error) {
return fmt.Sprintf("max-age=%.0f", durationInSeconds), nil
}
nextRun, ok, err := i.oidcCache.Get(noNamespace, "nextRun")
nextRun, ok, err := i.oidcCache.Get(ns, "nextRun")
if err != nil {
return "", err
}
@@ -1590,7 +1587,7 @@ func (i *IdentityStore) pathOIDCReadPublicKeys(ctx context.Context, req *logical
return nil, err
}
if len(keys) > 0 {
header, err := i.getKeysCacheControlHeader()
header, err := i.getKeysCacheControlHeader(ns)
if err != nil {
return nil, err
}
@@ -2118,17 +2115,23 @@ func (i *IdentityStore) oidcKeyRotation(ctx context.Context, s logical.Storage)
// oidcPeriodFunc is invoked by the backend's periodFunc and runs regular key
// rotations and expiration actions.
func (i *IdentityStore) oidcPeriodicFunc(ctx context.Context) {
func (i *IdentityStore) oidcPeriodicFunc(ctx context.Context, s logical.Storage) {
// Key rotations write to storage, so only run this on the primary cluster.
// The periodic func does not run on perf standbys or DR secondaries.
if i.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary) {
return
}
var nextRun time.Time
now := time.Now()
v, ok, err := i.oidcCache.Get(noNamespace, "nextRun")
ns, err := namespace.FromContext(ctx)
if err != nil {
i.Logger().Error("error getting namespace from context", "err", err)
return
}
var nextRun time.Time
v, ok, err := i.oidcCache.Get(ns, "nextRun")
if err != nil {
i.Logger().Error("error reading oidc cache", "err", err)
return
@@ -2142,68 +2145,52 @@ func (i *IdentityStore) oidcPeriodicFunc(ctx context.Context) {
// be run at any time safely, but there is no need to invoke them (which
// might be somewhat expensive if there are many roles/keys) if we're not
// past any rotation/expiration TTLs.
if now.After(nextRun) {
// Initialize to a fairly distant next run time. This will be brought in
// based on key rotation times.
nextRun = now.Add(24 * time.Hour)
minJwksClientCacheDuration := time.Duration(math.MaxInt64)
if now.Before(nextRun) {
return
}
for _, ns := range i.namespacer.ListNamespaces(true) {
nsPath := ns.Path
nextRotation, jwksClientCacheDuration, err := i.oidcKeyRotation(ctx, s)
if err != nil {
i.Logger().Warn("error rotating OIDC keys", "err", err)
}
s := i.router.MatchingStorageByAPIPath(ctx, nsPath+"identity/oidc")
nextExpiration, err := i.expireOIDCPublicKeys(ctx, s)
if err != nil {
i.Logger().Warn("error expiring OIDC public keys", "err", err)
}
if s == nil {
continue
}
if err := i.oidcCache.Flush(ns); err != nil {
i.Logger().Error("error flushing oidc cache", "err", err)
}
nextRotation, jwksClientCacheDuration, err := i.oidcKeyRotation(ctx, s)
if err != nil {
i.Logger().Warn("error rotating OIDC keys", "err", err)
}
// use the soonest time between nextRotation and nextExpiration for the next run.
// Allow at most 24 hours though, keeping the legacy behavior from the original
// introduction of namespaces (unclear if necessary but safer to keep for now).
nextRun = now.Add(24 * time.Hour)
if nextRotation.Before(nextRun) {
nextRun = nextRotation
}
if nextExpiration.Before(nextRun) {
nextRun = nextExpiration
}
nextExpiration, err := i.expireOIDCPublicKeys(ctx, s)
if err != nil {
i.Logger().Warn("error expiring OIDC public keys", "err", err)
}
if err := i.oidcCache.SetDefault(ns, "nextRun", nextRun); err != nil {
i.Logger().Error("error setting oidc cache", "err", err)
}
if err := i.oidcCache.Flush(ns); err != nil {
i.Logger().Error("error flushing oidc cache", "err", err)
}
// re-run at the soonest expiration or rotation time
if nextRotation.Before(nextRun) {
nextRun = nextRotation
}
if nextExpiration.Before(nextRun) {
nextRun = nextExpiration
}
if jwksClientCacheDuration < minJwksClientCacheDuration {
minJwksClientCacheDuration = jwksClientCacheDuration
}
if jwksClientCacheDuration < math.MaxInt64 {
// the OIDC JWKS endpoint returns a Cache-Control HTTP header time between
// 0 and the minimum verificationTTL or minimum rotationPeriod out of all
// keys, whichever value is lower.
//
// This smooths calls from services validating JWTs to Vault, while
// ensuring that operators can assert that servers honoring the
// Cache-Control header will always have a superset of all valid keys, and
// not trust any keys longer than a jwksCacheControlMaxAge duration after a
// key is rotated out of signing use
if err := i.oidcCache.SetDefault(ns, "jwksCacheControlMaxAge", jwksClientCacheDuration); err != nil {
i.Logger().Error("error setting jwksCacheControlMaxAge in oidc cache", "err", err)
}
if err := i.oidcCache.SetDefault(noNamespace, "nextRun", nextRun); err != nil {
i.Logger().Error("error setting oidc cache", "err", err)
}
if minJwksClientCacheDuration < math.MaxInt64 {
// the OIDC JWKS endpoint returns a Cache-Control HTTP header time between
// 0 and the minimum verificationTTL or minimum rotationPeriod out of all
// keys, whichever value is lower.
//
// This smooths calls from services validating JWTs to Vault, while
// ensuring that operators can assert that servers honoring the
// Cache-Control header will always have a superset of all valid keys, and
// not trust any keys longer than a jwksCacheControlMaxAge duration after a
// key is rotated out of signing use
if err := i.oidcCache.SetDefault(noNamespace, "jwksCacheControlMaxAge", minJwksClientCacheDuration); err != nil {
i.Logger().Error("error setting jwksCacheControlMaxAge in oidc cache", "err", err)
}
}
}
}
@@ -2248,9 +2235,9 @@ func (c *oidcCache) Flush(ns *namespace.Namespace) error {
return errNilNamespace
}
// Remove all items from the provided namespace as well as the shared, "no namespace" section.
// Remove all items from the provided namespace
for itemKey := range c.c.Items() {
if isTargetNamespacedKey(itemKey, []string{noNamespace.ID, ns.ID}) {
if isTargetNamespacedKey(itemKey, []string{ns.ID}) {
c.c.Delete(itemKey)
}
}

82
vault/identity_store_oidc_test.go
View File

@@ -1192,14 +1192,14 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
namedKeySamples := make([]*logical.StorageEntry, len(testSet.testCases))
publicKeysSamples := make([][]string, len(testSet.testCases))
for i := range testSet.testCases {
c.identityStore.oidcPeriodicFunc(ctx)
c.identityStore.oidcPeriodicFunc(ctx, storage)
namedKeyEntry, _ := storage.Get(ctx, namedKeyConfigPath+testSet.namedKey.name)
publicKeysEntry, _ := storage.List(ctx, publicKeysConfigPath)
namedKeySamples[i] = namedKeyEntry
publicKeysSamples[i] = publicKeysEntry
// sleep until we are in the next cycle - where a next run will happen
v, _, _ := c.identityStore.oidcCache.Get(noNamespace, "nextRun")
v, _, _ := c.identityStore.oidcCache.Get(namespace.RootNamespace, "nextRun")
nextRun := v.(time.Time)
now := time.Now()
diff := nextRun.Sub(now)
@@ -1602,61 +1602,37 @@ func TestOIDC_isTargetNamespacedKey(t *testing.T) {
func TestOIDC_Flush(t *testing.T) {
c := newOIDCCache(gocache.NoExpiration, gocache.NoExpiration)
ns := []*namespace.Namespace{
noNamespace, // ns[0] is nilNamespace
{ID: "ns0"},
{ID: "ns1"},
{ID: "ns2"},
}
// populateNs populates cache by ns with some data
populateNs := func() {
for i := range ns {
for _, val := range []string{"keyA", "keyB", "keyC"} {
if err := c.SetDefault(ns[i], val, struct{}{}); err != nil {
t.Fatal(err)
}
}
}
}
// validate verifies that cache items exist or do not exist based on their namespaced key
verify := func(items map[string]gocache.Item, expect, doNotExpect []*namespace.Namespace) {
for _, expectNs := range expect {
found := false
for i := range items {
if isTargetNamespacedKey(i, []string{expectNs.ID}) {
found = true
break
}
}
if !found {
t.Fatalf("Expected cache to contain an entry with a namespaced key for namespace: %q but did not find one", expectNs.ID)
}
}
for _, doNotExpectNs := range doNotExpect {
for i := range items {
if isTargetNamespacedKey(i, []string{doNotExpectNs.ID}) {
t.Fatalf("Did not expect cache to contain an entry with a namespaced key for namespace: %q but found the key: %q", doNotExpectNs.ID, i)
}
}
}
}
// flushing ns1 should flush ns1 and nilNamespace but not ns2
populateNs()
if err := c.Flush(ns[1]); err != nil {
// Add a key to the cache under each namespace
if err := c.SetDefault(ns[0], "keyA", struct{}{}); err != nil {
t.Fatal(err)
}
if err := c.SetDefault(ns[1], "keyB", struct{}{}); err != nil {
t.Fatal(err)
}
items := c.c.Items()
verify(items, []*namespace.Namespace{ns[2]}, []*namespace.Namespace{ns[0], ns[1]})
// flushing nilNamespace should flush nilNamespace but not ns1 or ns2
populateNs()
// Flush ns0
if err := c.Flush(ns[0]); err != nil {
t.Fatal(err)
}
items = c.c.Items()
verify(items, []*namespace.Namespace{ns[1], ns[2]}, []*namespace.Namespace{ns[0]})
// Verify that ns0 was flushed but ns1 was not
items := c.c.Items()
ns1KeyFound := false
for i := range items {
if isTargetNamespacedKey(i, []string{ns[0].ID}) {
t.Fatalf("Expected cache to not contain an entry with a namespaced key for namespace: %q but found one", ns[0].ID)
}
if isTargetNamespacedKey(i, []string{ns[1].ID}) {
ns1KeyFound = true
}
}
if !ns1KeyFound {
t.Fatalf("Expected cache to contain an entry with a namespaced key for namespace: %q but did not find one", ns[1].ID)
}
}
func TestOIDC_CacheNamespaceNilCheck(t *testing.T) {
@@ -1679,7 +1655,7 @@ func TestOIDC_GetKeysCacheControlHeader(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
// get default value
header, err := c.identityStore.getKeysCacheControlHeader()
header, err := c.identityStore.getKeysCacheControlHeader(namespace.RootNamespace)
if err != nil {
t.Fatalf("expected success, got error:\n%v", err)
}
@@ -1691,11 +1667,11 @@ func TestOIDC_GetKeysCacheControlHeader(t *testing.T) {
// set nextRun
nextRun := time.Now().Add(24 * time.Hour)
if err = c.identityStore.oidcCache.SetDefault(noNamespace, "nextRun", nextRun); err != nil {
if err = c.identityStore.oidcCache.SetDefault(namespace.RootNamespace, "nextRun", nextRun); err != nil {
t.Fatal(err)
}
header, err = c.identityStore.getKeysCacheControlHeader()
header, err = c.identityStore.getKeysCacheControlHeader(namespace.RootNamespace)
if err != nil {
t.Fatalf("expected success, got error:\n%v", err)
}
@@ -1708,11 +1684,11 @@ func TestOIDC_GetKeysCacheControlHeader(t *testing.T) {
// set jwksCacheControlMaxAge
durationSeconds := 60
jwksCacheControlMaxAge := time.Duration(durationSeconds) * time.Second
if err = c.identityStore.oidcCache.SetDefault(noNamespace, "jwksCacheControlMaxAge", jwksCacheControlMaxAge); err != nil {
if err = c.identityStore.oidcCache.SetDefault(namespace.RootNamespace, "jwksCacheControlMaxAge", jwksCacheControlMaxAge); err != nil {
t.Fatal(err)
}
header, err = c.identityStore.getKeysCacheControlHeader()
header, err = c.identityStore.getKeysCacheControlHeader(namespace.RootNamespace)
if err != nil {
t.Fatalf("expected success, got error:\n%v", err)
}

21
website/content/docs/enterprise/namespaces/namespace-limits.mdx
View File

@@ -116,24 +116,3 @@ the following metrics:
|----------------|--------------------------------------------------------------------------------------------------|
| 0 – 256 | [`vault.rollback.queued`](/vault/docs/internals/telemetry/metrics/core-system#rollback-metrics) |
| 0 – 60000 | [`vault.rollback.waiting`](/vault/docs/internals/telemetry/metrics/core-system#rollback-metrics) |
## Identity secret engine warnings
When using OIDC with many namespaces, you may see warnings in your Vault logs
from the `identity` secret mount under the `root` namespace. For example:
```text
2023-10-24T15:47:56.594Z [WARN] secrets.identity.identity_51eb2411: error expiring OIDC public keys: err="context deadline exceeded"
2023-10-24T15:47:56.594Z [WARN] secrets.identity.identity_51eb2411: error rotating OIDC keys: err="context deadline exceeded"
```
The `secrets.identity` warnings occur because the root namespace is responsible
for rotating the [OIDC keys](/vault/docs/secrets/identity/oidc-provider) of all
other namespaces.
<Warning title="Avoid OIDC with many namespaces">
Using Vault as an [OIDC provider](/vault/docs/concepts/oidc-provider) with
many namespaces can severely delay the rotation and invalidation of OIDC keys.
</Warning>