From 2e131c1459f105965955b12b627e884d60020531 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Tue, 22 Aug 2023 14:48:14 -0400 Subject: [PATCH] backport of commit 35a5fbfc6002e0440c708e722dc8aabbcb7a81b2 (#22507) Co-authored-by: Max Bowsher --- changelog/18809.txt | 3 +++ vault/activity_log.go | 28 +++++++++++++++++----------- vault/activity_log_test.go | 24 ++++++++++++++++++------ vault/request_handling.go | 9 ++++++--- 4 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 changelog/18809.txt diff --git a/changelog/18809.txt b/changelog/18809.txt new file mode 100644 index 000000000..a1ec06f57 --- /dev/null +++ b/changelog/18809.txt @@ -0,0 +1,3 @@ +```release-note:bug +activity (enterprise): Fix misattribution of entities to no or child namespace auth methods +``` diff --git a/vault/activity_log.go b/vault/activity_log.go index 221ad5c8b..43367b8b8 100644 --- a/vault/activity_log.go +++ b/vault/activity_log.go @@ -96,11 +96,6 @@ type segmentInfo struct { tokenCount *activity.TokenCount } -type clients struct { - distinctEntities uint64 - distinctNonEntities uint64 -} - // ActivityLog tracks unique entity counts and non-entity token counts. // It handles assembling log fragments (and sending them to the active // node), writing log segments, and precomputing queries. @@ -1912,39 +1907,50 @@ func (a *ActivityLog) loadConfigOrDefault(ctx context.Context) (activityConfig, // HandleTokenUsage adds the TokenEntry to the current fragment of the activity log // This currently occurs on token usage only. -func (a *ActivityLog) HandleTokenUsage(ctx context.Context, entry *logical.TokenEntry, clientID string, isTWE bool) { +func (a *ActivityLog) HandleTokenUsage(ctx context.Context, entry *logical.TokenEntry, clientID string, isTWE bool) error { // First, check if a is enabled, so as to avoid the cost of creating an ID for // tokens without entities in the case where it not. a.fragmentLock.RLock() if !a.enabled { a.fragmentLock.RUnlock() - return + return nil } a.fragmentLock.RUnlock() // Do not count wrapping tokens in client count if IsWrappingToken(entry) { - return + return nil } // Do not count root tokens in client count. if entry.IsRoot() { - return + return nil } // Tokens created for the purpose of Link should bypass counting for billing purposes if entry.InternalMeta != nil && entry.InternalMeta[IgnoreForBilling] == "true" { - return + return nil } + // Look up the mount accessor of the auth method that issued the token, taking care to resolve the token path + // against the token namespace, which may not be the same as the request namespace! + tokenNS, err := NamespaceByID(ctx, entry.NamespaceID, a.core) + if err != nil { + return err + } + if tokenNS == nil { + return namespace.ErrNoNamespace + } + tokenCtx := namespace.ContextWithNamespace(ctx, tokenNS) mountAccessor := "" - mountEntry := a.core.router.MatchingMountEntry(ctx, entry.Path) + mountEntry := a.core.router.MatchingMountEntry(tokenCtx, entry.Path) if mountEntry != nil { mountAccessor = mountEntry.Accessor } // Parse an entry's client ID and add it to the activity log a.AddClientToFragment(clientID, entry.NamespaceID, entry.CreationTime, isTWE, mountAccessor) + return nil } func (a *ActivityLog) namespaceToLabel(ctx context.Context, nsID string) string { diff --git a/vault/activity_log_test.go b/vault/activity_log_test.go index db85e0463..7baf4a709 100644 --- a/vault/activity_log_test.go +++ b/vault/activity_log_test.go @@ -136,7 +136,10 @@ func TestActivityLog_Creation_WrappingTokens(t *testing.T) { } id, isTWE := te.CreateClientID() - a.HandleTokenUsage(context.Background(), te, id, isTWE) + err := a.HandleTokenUsage(context.Background(), te, id, isTWE) + if err != nil { + t.Fatal(err) + } a.fragmentLock.Lock() if a.fragment != nil { @@ -153,7 +156,10 @@ func TestActivityLog_Creation_WrappingTokens(t *testing.T) { } id, isTWE = teNew.CreateClientID() - a.HandleTokenUsage(context.Background(), teNew, id, isTWE) + err = a.HandleTokenUsage(context.Background(), teNew, id, isTWE) + if err != nil { + t.Fatal(err) + } a.fragmentLock.Lock() if a.fragment != nil { @@ -380,18 +386,24 @@ func TestActivityLog_SaveTokensToStorageDoesNotUpdateTokenCount(t *testing.T) { tokenPath := fmt.Sprintf("%sdirecttokens/%d/0", ActivityLogPrefix, a.GetStartTimestamp()) clientPath := fmt.Sprintf("sys/counters/activity/log/entity/%d/0", a.GetStartTimestamp()) // Create some entries without entityIDs - tokenEntryOne := logical.TokenEntry{NamespaceID: "ns1_id", Policies: []string{"hi"}} - entityEntry := logical.TokenEntry{EntityID: "foo", NamespaceID: "ns1_id", Policies: []string{"hi"}} + tokenEntryOne := logical.TokenEntry{NamespaceID: namespace.RootNamespaceID, Policies: []string{"hi"}} + entityEntry := logical.TokenEntry{EntityID: "foo", NamespaceID: namespace.RootNamespaceID, Policies: []string{"hi"}} idNonEntity, isTWE := tokenEntryOne.CreateClientID() for i := 0; i < 3; i++ { - a.HandleTokenUsage(ctx, &tokenEntryOne, idNonEntity, isTWE) + err := a.HandleTokenUsage(ctx, &tokenEntryOne, idNonEntity, isTWE) + if err != nil { + t.Fatal(err) + } } idEntity, isTWE := entityEntry.CreateClientID() for i := 0; i < 2; i++ { - a.HandleTokenUsage(ctx, &entityEntry, idEntity, isTWE) + err := a.HandleTokenUsage(ctx, &entityEntry, idEntity, isTWE) + if err != nil { + t.Fatal(err) + } } err := a.saveCurrentSegmentToStorage(ctx, false) if err != nil { diff --git a/vault/request_handling.go b/vault/request_handling.go index 9900deb2e..ff5dfd95f 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -271,12 +271,12 @@ func (c *Core) CheckToken(ctx context.Context, req *logical.Request, unauth bool var te *logical.TokenEntry var entity *identity.Entity var identityPolicies map[string][]string - var err error // Even if unauth, if a token is provided, there's little reason not to // gather as much info as possible for the audit log and to e.g. control // trace mode for EGPs. if !unauth || (unauth && req.ClientToken != "") { + var err error acl, te, entity, identityPolicies, err = c.fetchACLTokenEntryAndEntity(ctx, req) // In the unauth case we don't want to fail the command, since it's // unauth, we just have no information to attach to the request, so @@ -438,9 +438,12 @@ func (c *Core) CheckToken(ctx context.Context, req *logical.Request, unauth bool c.activityLogLock.RLock() activityLog := c.activityLog c.activityLogLock.RUnlock() - // If it is an authenticated ( i.e with vault token ) request, increment client count + // If it is an authenticated ( i.e. with vault token ) request, increment client count if !unauth && activityLog != nil { - activityLog.HandleTokenUsage(ctx, te, clientID, isTWE) + err := c.activityLog.HandleTokenUsage(ctx, te, clientID, isTWE) + if err != nil { + return auth, te, err + } } return auth, te, nil }