Filter identity token keys

changelog/12780.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+identity: Prevent clients to the identity token system from reading/caching keys that aren’t eligible to sign tokens (aren’t associated with any role).
+```

vault/identity_store_oidc.go

@@ -613,6 +613,27 @@ func (i *IdentityStore) pathOIDCReadKey(ctx context.Context, req *logical.Reques
 	}, nil
 }
 
+// keyIDsByName will return a slice of key IDs for the given key name
+func (i *IdentityStore) keyIDsByName(ctx context.Context, s logical.Storage, name string) ([]string, error) {
+	var keyIDs []string
+	entry, err := s.Get(ctx, namedKeyConfigPath+name)
+	if err != nil {
+		return keyIDs, err
+	}
+	if entry == nil {
+		return keyIDs, nil
+	}
+
+	var key namedKey
+	if err := entry.DecodeJSON(&key); err != nil {
+		return keyIDs, err
+	}
+	for _, k := range key.KeyRing {
+		keyIDs = append(keyIDs, k.KeyID)
+	}
+	return keyIDs, nil
+}
+
 // rolesReferencingTargetKeyName returns a map of role names to roles
 // referencing targetKeyName.
 //
@@ -1538,21 +1559,37 @@ func (i *IdentityStore) generatePublicJWKS(ctx context.Context, s logical.Storag
 		return nil, err
 	}
 
-	keyIDs, err := listOIDCPublicKeys(ctx, s)
+	jwks := &jose.JSONWebKeySet{
+		Keys: make([]jose.JSONWebKey, 0),
+	}
+
+	// only return keys that are associated with a role
+	roleNames, err := s.List(ctx, roleConfigPath)
 	if err != nil {
 		return nil, err
 	}
 
-	jwks := &jose.JSONWebKeySet{
-		Keys: make([]jose.JSONWebKey, 0, len(keyIDs)),
-	}
+	for _, roleName := range roleNames {
+		role, err := i.getOIDCRole(ctx, s, roleName)
+		if err != nil {
+			return nil, err
+		}
+		if role == nil {
+			continue
+		}
 
-	for _, keyID := range keyIDs {
-		key, err := loadOIDCPublicKey(ctx, s, keyID)
+		keyIDs, err := i.keyIDsByName(ctx, s, role.Key)
 		if err != nil {
 			return nil, err
 		}
-		jwks.Keys = append(jwks.Keys, *key)
+
+		for _, keyID := range keyIDs {
+			key, err := loadOIDCPublicKey(ctx, s, keyID)
+			if err != nil {
+				return nil, err
+			}
+			jwks.Keys = append(jwks.Keys, *key)
+		}
 	}
 
 	if err := i.oidcCache.SetDefault(ns, "jwks", jwks); err != nil {

vault/identity_store_oidc_test.go

@@ -1,6 +1,7 @@
 package vault
 
 import (
+	"context"
 	"crypto/rand"
 	"crypto/rsa"
 	"encoding/json"
@@ -637,37 +638,73 @@ func TestOIDC_Path_OIDCKey_DeleteWithExistingClient(t *testing.T) {
 	expectError(t, resp, err)
 }
 
-// TestOIDC_PublicKeys tests that public keys are updated by
-// key creation, rotation, and deletion
-func TestOIDC_PublicKeys(t *testing.T) {
+// TestOIDC_PublicKeys_NoRole tests that public keys are not returned by the
+// oidc/.well-known/keys endpoint when they are not associated with a role
+func TestOIDC_PublicKeys_NoRole(t *testing.T) {
 	c, _, _ := TestCoreUnsealed(t)
 	ctx := namespace.RootContext(nil)
-	storage := &logical.InmemStorage{}
+	s := &logical.InmemStorage{}
 
 	// Create a test key "test-key"
-	c.identityStore.HandleRequest(ctx, &logical.Request{
+	resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
 		Path:      "oidc/key/test-key",
 		Operation: logical.CreateOperation,
-		Storage:   storage,
+		Storage:   s,
 	})
+	expectSuccess(t, resp, err)
 
-	// .well-known/keys should contain 2 public keys
+	// .well-known/keys should contain 0 public keys
+	assertPublicKeyCount(t, ctx, s, c, 0)
+}
+
+func assertPublicKeyCount(t *testing.T, ctx context.Context, s logical.Storage, c *Core, keyCount int) {
+	t.Helper()
+
+	// .well-known/keys should contain keyCount public keys
 	resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
 		Path:      "oidc/.well-known/keys",
 		Operation: logical.ReadOperation,
-		Storage:   storage,
+		Storage:   s,
 	})
 	expectSuccess(t, resp, err)
 	// parse response
 	responseJWKS := &jose.JSONWebKeySet{}
 	json.Unmarshal(resp.Data["http_raw_body"].([]byte), responseJWKS)
-	if len(responseJWKS.Keys) != 2 {
-		t.Fatalf("expected 2 public keys but instead got %d", len(responseJWKS.Keys))
+	if len(responseJWKS.Keys) != keyCount {
+		t.Fatalf("expected %d public keys but instead got %d", keyCount, len(responseJWKS.Keys))
 	}
+}
+
+// TestOIDC_PublicKeys tests that public keys are updated by
+// key creation, rotation, and deletion
+func TestOIDC_PublicKeys(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	ctx := namespace.RootContext(nil)
+	storage := &logical.InmemStorage{}
+
+	// Create a test key "test-key"
+	c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/key/test-key",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+	})
+
+	// Create a test role "test-role"
+	c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/role/test-role",
+		Operation: logical.CreateOperation,
+		Data: map[string]interface{}{
+			"key": "test-key",
+		},
+		Storage: storage,
+	})
+
+	// .well-known/keys should contain 2 public keys
+	assertPublicKeyCount(t, ctx, storage, c, 2)
 
 	// rotate test-key a few times, each rotate should increase the length of public keys returned
 	// by the .well-known endpoint
-	resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
+	resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
 		Path:      "oidc/key/test-key/rotate",
 		Operation: logical.UpdateOperation,
 		Storage:   storage,
@@ -681,45 +718,47 @@ func TestOIDC_PublicKeys(t *testing.T) {
 	expectSuccess(t, resp, err)
 
 	// .well-known/keys should contain 4 public keys
+	assertPublicKeyCount(t, ctx, storage, c, 4)
+
+	// create another named key "test-key2"
 	resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
-		Path:      "oidc/.well-known/keys",
-		Operation: logical.ReadOperation,
+		Path:      "oidc/key/test-key2",
+		Operation: logical.CreateOperation,
 		Storage:   storage,
 	})
 	expectSuccess(t, resp, err)
-	// parse response
-	json.Unmarshal(resp.Data["http_raw_body"].([]byte), responseJWKS)
-	if len(responseJWKS.Keys) != 4 {
-		t.Fatalf("expected 4 public keys but instead got %d", len(responseJWKS.Keys))
-	}
 
-	// create another named key
-	c.identityStore.HandleRequest(ctx, &logical.Request{
-		Path:      "oidc/key/test-key2",
+	// Create a test role "test-role2"
+	resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/role/test-role2",
 		Operation: logical.CreateOperation,
-		Storage:   storage,
+		Data: map[string]interface{}{
+			"key": "test-key2",
+		},
+		Storage: storage,
 	})
+	expectSuccess(t, resp, err)
+	// .well-known/keys should contain 6 public keys
+	assertPublicKeyCount(t, ctx, storage, c, 6)
 
-	// delete test key
-	c.identityStore.HandleRequest(ctx, &logical.Request{
-		Path:      "oidc/key/test-key",
+	// delete test role that references "test-key"
+	resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/role/test-role",
 		Operation: logical.DeleteOperation,
 		Storage:   storage,
 	})
-
-	// .well-known/keys should contain 2 public key, all of the public keys
-	// from named key "test-key" should have been deleted
+	expectSuccess(t, resp, err)
+	// delete test key
 	resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{
-		Path:      "oidc/.well-known/keys",
-		Operation: logical.ReadOperation,
+		Path:      "oidc/key/test-key",
+		Operation: logical.DeleteOperation,
 		Storage:   storage,
 	})
 	expectSuccess(t, resp, err)
-	// parse response
-	json.Unmarshal(resp.Data["http_raw_body"].([]byte), responseJWKS)
-	if len(responseJWKS.Keys) != 2 {
-		t.Fatalf("expected 2 public keys but instead got %d", len(responseJWKS.Keys))
-	}
+
+	// .well-known/keys should contain 2 public keys, all of the public keys
+	// from named key "test-key" should have been deleted
+	assertPublicKeyCount(t, ctx, storage, c, 2)
 }
 
 // TestOIDC_SignIDToken tests acquiring a signed token and verifying the public portion