1
0

Backport of identity/oidc: fix flakey key rotation test into release/1.14.x (#25541)

Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com>
This commit is contained in:
hc-github-team-secure-vault-core 2024-02-21 12:13:33 -07:00 committed by GitHub
parent d3028e6aa7
commit 6a45d27a90
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -20,6 +20,8 @@ import (
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
gocache "github.com/patrickmn/go-cache"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestOIDC_Path_OIDC_RoleNoKeyParameter tests that a role cannot be created
@ -386,8 +388,8 @@ func TestOIDC_Path_OIDCRole(t *testing.T) {
expectStrings(t, respListRoleAfterDelete.Data["keys"].([]string), expectedStrings)
}
// TestOIDC_Path_OIDCKeyKey tests CRUD operations for keys
func TestOIDC_Path_OIDCKeyKey(t *testing.T) {
// TestOIDC_Path_CRUDKey tests CRUD operations for keys
func TestOIDC_Path_CRUDKey(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
ctx := namespace.RootContext(nil)
storage := &logical.InmemStorage{}
@ -457,7 +459,6 @@ func TestOIDC_Path_OIDCKeyKey(t *testing.T) {
Storage: storage,
})
expectSuccess(t, resp, err)
// fmt.Printf("resp is:\n%#v", resp)
// Delete test-key -- should fail because test-role depends on test-key
resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
@ -554,8 +555,8 @@ func TestOIDC_Path_OIDCKey_InvalidTokenTTL(t *testing.T) {
expectError(t, resp, err)
}
// TestOIDC_Path_OIDCKey tests the List operation for keys
func TestOIDC_Path_OIDCKey(t *testing.T) {
// TestOIDC_Path_ListKey tests the List operation for keys
func TestOIDC_Path_ListKey(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
ctx := namespace.RootContext(nil)
storage := &logical.InmemStorage{}
@ -1039,13 +1040,11 @@ func testNamedKey(name string) *namedKey {
// rotations and expiration actions.
func TestOIDC_PeriodicFunc(t *testing.T) {
type testCase struct {
cycle int
numKeys int
numPublicKeys int
minKeyRingLen int
maxKeyRingLen int
}
testSets := []struct {
namedKey *namedKey
expectedKeyCount int
setSigningKey bool
setNextSigningKey bool
testCases []testCase
@ -1055,11 +1054,12 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
setSigningKey: true,
setNextSigningKey: true,
testCases: []testCase{
// Each cycle results in a key going in/out of its verification_ttl period
{1, 2, 2},
{2, 3, 3},
{3, 2, 2},
{4, 3, 3},
// we must always have at least 2 keys and at most 3 through rotation cycles, since
// each rotation cycle results in a key going in/out of its verification_ttl period.
{2, 3},
{2, 3},
{2, 3},
{2, 3},
},
},
{
@ -1068,11 +1068,11 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
setSigningKey: false,
setNextSigningKey: true,
testCases: []testCase{
{1, 1, 1},
{1, 1},
// key counts jump from 1 to 2 because the next signing key becomes
// the signing key, and no key is in its verification_ttl period
{2, 2, 2},
{2, 2},
},
},
{
@ -1081,11 +1081,11 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
setSigningKey: true,
setNextSigningKey: false,
testCases: []testCase{
{1, 1, 1},
{1, 1},
// key counts jump from 1 to 3 because the original signing key is
// still published and within its verification_ttl period
{2, 3, 3},
// max key counts jump from 1 to 3 because the original signing
// key could still be within its verification_ttl period
{2, 3},
},
},
{
@ -1094,8 +1094,10 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
setSigningKey: false,
setNextSigningKey: false,
testCases: []testCase{
{1, 0, 0},
{2, 2, 2},
{0, 0},
// First rotation populates both current/next signing keys
{2, 2},
},
},
}
@ -1105,46 +1107,41 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
t.Run(testSet.namedKey.name, func(t *testing.T) {
t.Parallel()
// Prepare a storage to run through periodicFunc
c, _, _ := TestCoreUnsealed(t)
ctx := namespace.RootContext(nil)
storage := c.router.MatchingStorageByAPIPath(ctx, "identity/oidc")
// Stop the core's rollback manager so that periodic function testing
// doesn't race with the rollback manager.
c.rollback.StopTicker()
// Generate current and next keys as needed by the test. This ensures
// we can rotate a key when either are unset.
if testSet.setSigningKey {
if err := testSet.namedKey.generateAndSetKey(ctx, hclog.NewNullLogger(), storage); err != nil {
t.Fatalf("failed to set signing key")
}
require.NoError(t, testSet.namedKey.generateAndSetKey(ctx, hclog.NewNullLogger(), storage))
}
if testSet.setNextSigningKey {
if err := testSet.namedKey.generateAndSetNextKey(ctx, hclog.NewNullLogger(), storage); err != nil {
t.Fatalf("failed to set next signing key")
}
require.NoError(t, testSet.namedKey.generateAndSetNextKey(ctx, hclog.NewNullLogger(), storage))
}
testSet.namedKey.NextRotation = time.Now().Add(testSet.namedKey.RotationPeriod)
// Store namedKey
// Store the named key so it can be rotated by the periodic func
entry, _ := logical.StorageEntryJSON(namedKeyConfigPath+testSet.namedKey.name, testSet.namedKey)
if err := storage.Put(ctx, entry); err != nil {
t.Fatalf("writing to in mem storage failed")
}
require.NoError(t, storage.Put(ctx, entry))
t.Cleanup(func() {
require.NoError(t, storage.Delete(ctx, namedKeyConfigPath+testSet.namedKey.name))
})
currentCycle := 1
numCases := len(testSet.testCases)
lastCycle := testSet.testCases[numCases-1].cycle
namedKeySamples := make([]*logical.StorageEntry, numCases)
publicKeysSamples := make([][]string, numCases)
i := 0
for currentCycle <= lastCycle {
// Manually execute the periodic func to rotate keys and collect
// both the key ring and public keys.
namedKeySamples := make([]*logical.StorageEntry, len(testSet.testCases))
publicKeysSamples := make([][]string, len(testSet.testCases))
for i := range testSet.testCases {
c.identityStore.oidcPeriodicFunc(ctx)
if currentCycle == testSet.testCases[i].cycle {
namedKeyEntry, _ := storage.Get(ctx, namedKeyConfigPath+testSet.namedKey.name)
publicKeysEntry, _ := storage.List(ctx, publicKeysConfigPath)
namedKeySamples[i] = namedKeyEntry
publicKeysSamples[i] = publicKeysEntry
i = i + 1
}
currentCycle = currentCycle + 1
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")
@ -1152,39 +1149,25 @@ func TestOIDC_PeriodicFunc(t *testing.T) {
now := time.Now()
diff := nextRun.Sub(now)
if now.Before(nextRun) {
time.Sleep(diff)
time.Sleep(diff + 100*time.Millisecond)
}
}
// measure collected samples
for i := range testSet.testCases {
expectedKeyCount := testSet.testCases[i].numKeys
namedKeySamples[i].DecodeJSON(&testSet.namedKey)
// Assert that the key lengths through each rotation match expectations
for i, tc := range testSet.testCases {
require.NoError(t, namedKeySamples[i].DecodeJSON(&testSet.namedKey))
actualKeyRingLen := len(testSet.namedKey.KeyRing)
if actualKeyRingLen != expectedKeyCount {
t.Errorf(
"For key: %s at cycle: %d expected namedKey's KeyRing to be at least of length %d but was: %d",
testSet.namedKey.name,
testSet.testCases[i].cycle,
expectedKeyCount,
actualKeyRingLen,
)
}
expectedPublicKeyCount := testSet.testCases[i].numPublicKeys
actualPubKeysLen := len(publicKeysSamples[i])
if actualPubKeysLen != expectedPublicKeyCount {
t.Errorf(
"For key: %s at cycle: %d expected public keys to be at least of length %d but was: %d",
testSet.namedKey.name,
testSet.testCases[i].cycle,
expectedPublicKeyCount,
actualPubKeysLen,
)
}
}
assert.LessOrEqual(t, actualKeyRingLen, tc.maxKeyRingLen,
"test case index %d: key ring length must be at most %d", i, tc.maxKeyRingLen)
assert.GreaterOrEqual(t, actualKeyRingLen, tc.minKeyRingLen,
"test case index %d: key ring length must be at least %d", i, tc.minKeyRingLen)
if err := storage.Delete(ctx, namedKeyConfigPath+testSet.namedKey.name); err != nil {
t.Fatalf("deleting from in mem storage failed")
actualPubKeysLen := len(publicKeysSamples[i])
assert.LessOrEqual(t, actualPubKeysLen, tc.maxKeyRingLen,
"test case index %d: public key ring length must be at most %d", i, tc.maxKeyRingLen)
assert.GreaterOrEqual(t, actualPubKeysLen, tc.minKeyRingLen,
"test case index %d: public key ring length must be at least %d", i, tc.minKeyRingLen)
}
})
}
@ -1283,7 +1266,7 @@ func TestOIDC_pathOIDCKeyExistenceCheck(t *testing.T) {
t.Fatalf("Expected existence check to return false but instead returned: %t", exists)
}
// Populte storage with a namedKey
// Populate storage with a namedKey
namedKey := &namedKey{}
entry, _ := logical.StorageEntryJSON(namedKeyConfigPath+keyName, namedKey)
if err := storage.Put(ctx, entry); err != nil {