Skip to content

Commit 6bfd527

Browse files
cipherboysatoqz
cipherboy
and
satoqz
authored
Fix nil panic with unsafe_cross_namespace_identity (#1715)
* Fix nil panic with unsafe_cross_namespace_identity The only place where IdentityStore.db(...) is not called is when ranging over all views to collect metrics. This did not handle the unsafe cross-namespace identity behavior of using a single memdb (only available on the root namespace's view entry), leading to a nil panic. Resolves: #1708 Signed-off-by: Alexander Scheel <ascheel@gitlab.com> * Add changelog entry Signed-off-by: Alexander Scheel <ascheel@gitlab.com> * Update vault/identity_store_test.go Co-authored-by: Jonas Köhnen <mail@satoqz.net> Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com> --------- Signed-off-by: Alexander Scheel <ascheel@gitlab.com> Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com> Co-authored-by: Jonas Köhnen <mail@satoqz.net>
1 parent 6d41413 commit 6bfd527

File tree

6 files changed

+114
-30
lines changed

6 files changed

+114
-30
lines changed

‎changelog/1715.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
identity: fix nil panic when collecting metrics with unsafe_cross_namespace_identity=true.
3+
```

‎vault/core_metrics_test.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package vault
55

66
import (
7+
"context"
78
"errors"
89
"sort"
910
"strings"
@@ -262,9 +263,20 @@ func metricLabelsMatch(t *testing.T, actual []metrics.Label, expected map[string
262263
}
263264

264265
func TestCoreMetrics_EntityGauges(t *testing.T) {
265-
ctx := namespace.RootContext(nil)
266-
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t)
266+
ctx := namespace.RootContext(context.Background())
267+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, false)
268+
269+
testCoreMetricsEntityGauges(t, ctx, is, approleAccessor, upAccessor, core)
270+
}
271+
272+
func TestCoreMetrics_EntityGaugesUnsafeSharedIdentity(t *testing.T) {
273+
ctx := namespace.RootContext(context.Background())
274+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, true)
275+
276+
testCoreMetricsEntityGauges(t, ctx, is, approleAccessor, upAccessor, core)
277+
}
267278

279+
func testCoreMetricsEntityGauges(t *testing.T, ctx context.Context, is *IdentityStore, approleAccessor string, upAccessor string, core *Core) {
268280
// Create an entity
269281
alias1 := &logical.Alias{
270282
MountType: "approle",

‎vault/identity_store_aliases_test.go‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package vault
55

66
import (
7+
"context"
78
"reflect"
89
"strings"
910
"testing"
@@ -654,11 +655,24 @@ func TestIdentityStore_AliasMove_DuplicateAccessor(t *testing.T) {
654655
// Test that the alias cannot be changed to a mount for which
655656
// the entity already has an alias
656657
func TestIdentityStore_AliasUpdate_DuplicateAccessor(t *testing.T) {
658+
ctx := namespace.RootContext(context.Background())
659+
660+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, false)
661+
662+
testIdentityStoreAliasUpdateDuplicateAccessor(t, ctx, is, approleAccessor, upAccessor, core)
663+
}
664+
665+
func TestIdentityStore_AliasUpdate_DuplicateAccessor_UnsafeShared(t *testing.T) {
666+
ctx := namespace.RootContext(context.Background())
667+
668+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, true)
669+
670+
testIdentityStoreAliasUpdateDuplicateAccessor(t, ctx, is, approleAccessor, upAccessor, core)
671+
}
672+
673+
func testIdentityStoreAliasUpdateDuplicateAccessor(t *testing.T, ctx context.Context, is *IdentityStore, approleAccessor string, upAccessor string, core *Core) {
657674
var err error
658675
var resp *logical.Response
659-
ctx := namespace.RootContext(nil)
660-
661-
is, ghAccessor, upAccessor, _ := testIdentityStoreWithAppRoleUserpassAuth(ctx, t)
662676

663677
// Create 1 entity and 2 aliases on it, one for each mount
664678
resp, err = is.HandleRequest(ctx, &logical.Request{
@@ -675,7 +689,7 @@ func TestIdentityStore_AliasUpdate_DuplicateAccessor(t *testing.T) {
675689

676690
alias1Data := map[string]interface{}{
677691
"name": "testaliasname1",
678-
"mount_accessor": ghAccessor,
692+
"mount_accessor": approleAccessor,
679693
"canonical_id": entityID,
680694
}
681695

@@ -708,7 +722,7 @@ func TestIdentityStore_AliasUpdate_DuplicateAccessor(t *testing.T) {
708722

709723
// Attempt to update the userpass mount to point to the github mount
710724
updateData := map[string]interface{}{
711-
"mount_accessor": ghAccessor,
725+
"mount_accessor": approleAccessor,
712726
}
713727

714728
aliasReq.Data = updateData

‎vault/identity_store_entities_test.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,12 +1009,20 @@ func TestIdentityStore_EntityCRUD(t *testing.T) {
10091009
}
10101010

10111011
func TestIdentityStore_MergeEntitiesByID(t *testing.T) {
1012-
var err error
1013-
var resp *logical.Response
1012+
ctx := namespace.RootContext(context.Background())
1013+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, false)
1014+
testIdentityStoreMergeEntitiesById(t, ctx, is, approleAccessor, upAccessor, core)
1015+
}
10141016

1015-
ctx := namespace.RootContext(nil)
1016-
is, approleAccessor, upAccessor, _ := testIdentityStoreWithAppRoleUserpassAuth(ctx, t)
1017+
func TestIdentityStore_MergeEntitiesByID_UnsafeShared(t *testing.T) {
1018+
ctx := namespace.RootContext(context.Background())
1019+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, true)
1020+
testIdentityStoreMergeEntitiesById(t, ctx, is, approleAccessor, upAccessor, core)
1021+
}
10171022

1023+
func testIdentityStoreMergeEntitiesById(t *testing.T, ctx context.Context, is *IdentityStore, approleAccessor string, upAccessor string, core *Core) {
1024+
var err error
1025+
var resp *logical.Response
10181026
registerData := map[string]interface{}{
10191027
"name": "testentityname2",
10201028
"metadata": []string{"someusefulkey=someusefulvalue"},

‎vault/identity_store_test.go‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ func TestIdentityStore_UnsealingWhenConflictingAliasNames(t *testing.T) {
185185
func TestIdentityStore_EntityIDPassthrough(t *testing.T) {
186186
// Enable AppRole auth and initialize
187187
ctx := namespace.RootContext(nil)
188-
is, ghAccessor, core := testIdentityStoreWithAppRoleAuth(ctx, t)
188+
is, approleAccessor, core := testIdentityStoreWithAppRoleAuth(ctx, t)
189189
alias := &logical.Alias{
190190
MountType: "approle",
191-
MountAccessor: ghAccessor,
191+
MountAccessor: approleAccessor,
192192
Name: "approleuser",
193193
}
194194

@@ -257,12 +257,21 @@ func TestIdentityStore_EntityIDPassthrough(t *testing.T) {
257257
}
258258

259259
func TestIdentityStore_CreateOrFetchEntity(t *testing.T) {
260-
ctx := namespace.RootContext(nil)
261-
is, ghAccessor, upAccessor, _ := testIdentityStoreWithAppRoleUserpassAuth(ctx, t)
260+
ctx := namespace.RootContext(t.Context())
261+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, false)
262+
testIdentityStoreCreateOrFetchEntity(t, ctx, is, approleAccessor, upAccessor, core)
263+
}
262264

265+
func TestIdentityStore_CreateOrFetchEntity_UnsafeShared(t *testing.T) {
266+
ctx := namespace.RootContext(t.Context())
267+
is, approleAccessor, upAccessor, core := testIdentityStoreWithAppRoleUserpassAuth(ctx, t, true)
268+
testIdentityStoreCreateOrFetchEntity(t, ctx, is, approleAccessor, upAccessor, core)
269+
}
270+
271+
func testIdentityStoreCreateOrFetchEntity(t *testing.T, ctx context.Context, is *IdentityStore, approleAccessor string, upAccessor string, core *Core) {
263272
alias := &logical.Alias{
264273
MountType: "approle",
265-
MountAccessor: ghAccessor,
274+
MountAccessor: approleAccessor,
266275
Name: "approleuser",
267276
Metadata: map[string]string{
268277
"foo": "a",
@@ -372,7 +381,7 @@ func TestIdentityStore_EntityByAliasFactors(t *testing.T) {
372381
var resp *logical.Response
373382

374383
ctx := namespace.RootContext(nil)
375-
is, ghAccessor, _ := testIdentityStoreWithAppRoleAuth(ctx, t)
384+
is, approleAccessor, _ := testIdentityStoreWithAppRoleAuth(ctx, t)
376385

377386
registerData := map[string]interface{}{
378387
"name": "testentityname",
@@ -403,7 +412,7 @@ func TestIdentityStore_EntityByAliasFactors(t *testing.T) {
403412
aliasData := map[string]interface{}{
404413
"entity_id": entityID,
405414
"name": "alias_name",
406-
"mount_accessor": ghAccessor,
415+
"mount_accessor": approleAccessor,
407416
}
408417
aliasReq := &logical.Request{
409418
Operation: logical.UpdateOperation,
@@ -419,7 +428,7 @@ func TestIdentityStore_EntityByAliasFactors(t *testing.T) {
419428
t.Fatal("expected a non-nil response")
420429
}
421430

422-
entity, err := is.entityByAliasFactors(ctx, ghAccessor, "alias_name", false)
431+
entity, err := is.entityByAliasFactors(ctx, approleAccessor, "alias_name", false)
423432
if err != nil {
424433
t.Fatal(err)
425434
}
@@ -648,13 +657,13 @@ func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
648657
}
649658

650659
func testCoreWithIdentityTokenAppRole(ctx context.Context, t *testing.T) (*Core, *IdentityStore, *TokenStore, string) {
651-
is, ghAccessor, core := testIdentityStoreWithAppRoleAuth(ctx, t)
652-
return core, is, core.tokenStore, ghAccessor
660+
is, approleAccessor, core := testIdentityStoreWithAppRoleAuth(ctx, t)
661+
return core, is, core.tokenStore, approleAccessor
653662
}
654663

655664
func testCoreWithIdentityTokenAppRoleRoot(ctx context.Context, t *testing.T) (*Core, *IdentityStore, *TokenStore, string, string) {
656-
is, ghAccessor, core, root := testIdentityStoreWithAppRoleAuthRoot(ctx, t)
657-
return core, is, core.tokenStore, ghAccessor, root
665+
is, approleAccessor, core, root := testIdentityStoreWithAppRoleAuthRoot(ctx, t)
666+
return core, is, core.tokenStore, approleAccessor, root
658667
}
659668

660669
func testIdentityStoreWithAppRoleAuth(ctx context.Context, t *testing.T) (*IdentityStore, string, *Core) {
@@ -692,7 +701,7 @@ func testIdentityStoreWithAppRoleAuthRoot(ctx context.Context, t *testing.T) (*I
692701
return c.identityStore, meGH.Accessor, c, root
693702
}
694703

695-
func testIdentityStoreWithAppRoleUserpassAuth(ctx context.Context, t *testing.T) (*IdentityStore, string, string, *Core) {
704+
func testIdentityStoreWithAppRoleUserpassAuth(ctx context.Context, t *testing.T, unsafeShared bool) (*IdentityStore, string, string, *Core) {
696705
// Setup 2 auth backends, github and userpass
697706
err := AddTestCredentialBackend("approle", credAppRole.Factory)
698707
if err != nil {
@@ -706,7 +715,14 @@ func testIdentityStoreWithAppRoleUserpassAuth(ctx context.Context, t *testing.T)
706715

707716
defer ClearTestCredentialBackends()
708717

709-
c, _, _ := TestCoreUnsealed(t)
718+
conf := &CoreConfig{
719+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
720+
UnsafeCrossNamespaceIdentity: unsafeShared,
721+
AuditBackends: map[string]audit.Factory{
722+
"file": auditFile.Factory,
723+
},
724+
}
725+
c, _, _ := TestCoreUnsealedWithConfig(t, conf)
710726

711727
githubMe := &MountEntry{
712728
Table: credentialTableType,
@@ -802,11 +818,11 @@ func TestIdentityStore_NewEntityCounter(t *testing.T) {
802818
}
803819

804820
is := c.identityStore
805-
ghAccessor := meGH.Accessor
821+
approleAccessor := meGH.Accessor
806822

807823
alias := &logical.Alias{
808824
MountType: "approle",
809-
MountAccessor: ghAccessor,
825+
MountAccessor: approleAccessor,
810826
Name: "approleuser",
811827
Metadata: map[string]string{
812828
"foo": "a",

‎vault/identity_store_util.go‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2435,11 +2435,22 @@ func (i *IdentityStore) handleAliasListCommon(ctx context.Context, groupAlias bo
24352435
func (i *IdentityStore) countEntities(ctx context.Context) (int, error) {
24362436
var count int
24372437
var outErr error
2438+
2439+
rootViewRaw, ok := i.views.Load(namespace.RootNamespaceUUID)
2440+
if !ok || rootViewRaw == nil {
2441+
return -1, fmt.Errorf("failed to load root namespace")
2442+
}
2443+
rootView := rootViewRaw.(*identityStoreNamespaceView)
2444+
24382445
i.views.Range(func(uuidRaw, viewsRaw any) bool {
24392446
uuid := uuidRaw.(string)
24402447
views := viewsRaw.(*identityStoreNamespaceView)
2448+
db := views.db
2449+
if db == nil {
2450+
db = rootView.db
2451+
}
24412452

2442-
txn := views.db.Txn(false)
2453+
txn := db.Txn(false)
24432454

24442455
iter, err := txn.Get(entitiesTable, "id")
24452456
if err != nil {
@@ -2467,12 +2478,22 @@ func (i *IdentityStore) countEntities(ctx context.Context) (int, error) {
24672478
func (i *IdentityStore) countEntitiesByNamespace(ctx context.Context) (map[string]int, error) {
24682479
byNamespace := make(map[string]int)
24692480

2481+
rootViewRaw, ok := i.views.Load(namespace.RootNamespaceUUID)
2482+
if !ok || rootViewRaw == nil {
2483+
return nil, fmt.Errorf("failed to load root namespace")
2484+
}
2485+
rootView := rootViewRaw.(*identityStoreNamespaceView)
2486+
24702487
var err error
24712488
i.views.Range(func(uuidRaw, viewsRaw any) bool {
24722489
uuid := uuidRaw.(string)
24732490
views := viewsRaw.(*identityStoreNamespaceView)
2491+
db := views.db
2492+
if db == nil {
2493+
db = rootView.db
2494+
}
24742495

2475-
txn := views.db.Txn(false)
2496+
txn := db.Txn(false)
24762497

24772498
var iter memdb.ResultIterator
24782499
iter, err = txn.Get(entitiesTable, "id")
@@ -2510,12 +2531,22 @@ func (i *IdentityStore) countEntitiesByNamespace(ctx context.Context) (map[strin
25102531
func (i *IdentityStore) countEntitiesByMountAccessor(ctx context.Context) (map[string]int, error) {
25112532
byMountAccessor := make(map[string]int)
25122533

2534+
rootViewRaw, ok := i.views.Load(namespace.RootNamespaceUUID)
2535+
if !ok || rootViewRaw == nil {
2536+
return nil, fmt.Errorf("failed to load root namespace")
2537+
}
2538+
rootView := rootViewRaw.(*identityStoreNamespaceView)
2539+
25132540
var err error
25142541
i.views.Range(func(uuidRaw, viewsRaw any) bool {
25152542
uuid := uuidRaw.(string)
25162543
views := viewsRaw.(*identityStoreNamespaceView)
2544+
db := views.db
2545+
if db == nil {
2546+
db = rootView.db
2547+
}
25172548

2518-
txn := views.db.Txn(false)
2549+
txn := db.Txn(false)
25192550

25202551
var iter memdb.ResultIterator
25212552
iter, err = txn.Get(entitiesTable, "id")

0 commit comments

Comments
 (0)