diff --git a/plan.md b/plan.md new file mode 100644 index 0000000000..0a10048648 --- /dev/null +++ b/plan.md @@ -0,0 +1,24 @@ +# Plan: Clean Unused Pseudonyms When Reducing Cache Size (#1820) + +## Goal Description +When the pseudonym cache size is reduced and the service restarts, previously pre-generated cached pseudonyms remain on disk. To prevent this, pre-generated cached pseudonyms should not be persisted on disk (they should be ephemeral). We implement an `EphemeralIdentity()` method that uses `Temporary: true` in the nym key derivation options, so that cached pseudonyms do not persist in the KeyStore. + +## Implementation Progress +- [x] Add `identityWithOpts()` internal method and `EphemeralIdentity()` to `idemix.KeyManager` (km.go) +- [x] Add `EphemeralIdentity()` to `idemixnym.KeyManager` (km.go) +- [x] Update `idemix.kmp.go` to wire cache backend to use `EphemeralIdentity` +- [x] Add unit tests for `EphemeralIdentity` in `km_test.go` +- [x] Add unit test for cache using ephemeral identities in `kmp_test.go` +- [x] Run unit tests and verify +- [x] Fix fp256bn_amcl references in TestEphemeralIdentity in km_test.go +- [x] Fix fp256bn_amcl references in TestCacheUsesEphemeralIdentity in kmp_test.go +- [x] Remove/fix FP256BN_AMCL benchmark in km_bench_test.go +- [x] Verify that all unit tests pass successfully +- [x] Create walkthrough summary + +## Status: ✅ COMPLETE + +## Notes & Decisions +- Checked `membership.KeyManager` interface. It doesn't have `EphemeralIdentity`. +- To avoid modifying `membership.KeyManager` and causing ripple effects, we use type assertion on `keyManager` in `kmp.go` to check for `EphemeralIdentity` method. +- The `fp256bn_amcl/idemix` testdata was deleted in main branch commit `a10d0d27` and replaced by `bls12_381_bbs_gurvy/idemix`. We must update the new tests/benchmarks in this branch to use the active curves. diff --git a/token/services/identity/idemix/crypto/id.go b/token/services/identity/idemix/crypto/id.go index 09e5f369c4..c269d5f961 100644 --- a/token/services/identity/idemix/crypto/id.go +++ b/token/services/identity/idemix/crypto/id.go @@ -141,13 +141,21 @@ type SigningIdentity struct { NymKeySKI []byte // User secret key identifier UserKeySKI []byte + // Ephemeral/derived nym key (only used in memory for ephemeral signers) + NymKey bccsp.Key `json:"-"` } // Sign creates a signature using secret keys. func (id *SigningIdentity) Sign(msg []byte) ([]byte, error) { - nymKey, err := id.CSP.GetKey(id.NymKeySKI) - if err != nil { - return nil, errors.Wrap(err, "cannot find nym secret key") + var nymKey bccsp.Key + var err error + if id.NymKey != nil { + nymKey = id.NymKey + } else { + nymKey, err = id.CSP.GetKey(id.NymKeySKI) + if err != nil { + return nil, errors.Wrap(err, "cannot find nym secret key") + } } // Load the user key diff --git a/token/services/identity/idemix/km.go b/token/services/identity/idemix/km.go index 2a996b511a..f25954a094 100644 --- a/token/services/identity/idemix/km.go +++ b/token/services/identity/idemix/km.go @@ -211,8 +211,24 @@ func NewKeyManagerWithSchema( }, nil } -// Identity returns the identity descriptor for the given audit information +// Identity returns the identity descriptor for the given audit information. +// The derived nym key is persisted in the key store for later use. func (p *KeyManager) Identity(ctx context.Context, auditInfo []byte) (*idriver.IdentityDescriptor, error) { + return p.identityWithOpts(ctx, auditInfo, false) +} + +// EphemeralIdentity returns the identity descriptor for the given audit information +// without persisting the derived nym key in the key store. +// This is intended for pre-generated cache identities that are used once and +// do not need to survive a restart. Using ephemeral nym keys avoids accumulating +// orphaned pseudonyms on disk when the cache size is later reduced. +func (p *KeyManager) EphemeralIdentity(ctx context.Context, auditInfo []byte) (*idriver.IdentityDescriptor, error) { + return p.identityWithOpts(ctx, auditInfo, true) +} + +// identityWithOpts is the core implementation for identity generation. +// When temporaryNym is true, the derived nym key is not persisted in the key store. +func (p *KeyManager) identityWithOpts(ctx context.Context, auditInfo []byte, temporaryNym bool) (*idriver.IdentityDescriptor, error) { logger.DebugfContext(ctx, "get user secret key") // Load the user key userKey, err := p.Csp.GetKey(p.userKeySKI) @@ -225,7 +241,7 @@ func (p *KeyManager) Identity(ctx context.Context, auditInfo []byte) (*idriver.I nymKey, err := p.Csp.KeyDeriv( userKey, &bccsp.IdemixNymKeyDerivationOpts{ - Temporary: false, + Temporary: temporaryNym, IssuerPK: p.IssuerPublicKey, }, ) @@ -283,6 +299,7 @@ func (p *KeyManager) Identity(ctx context.Context, auditInfo []byte) (*idriver.I NymKeySKI: nymPublicKey.SKI(), UserKeySKI: p.userKeySKI, EnrollmentId: enrollmentID, + NymKey: nymKey, } serializedIdentity, err := sID.Serialize() if err != nil { diff --git a/token/services/identity/idemix/km_bench_test.go b/token/services/identity/idemix/km_bench_test.go index f96450ccfa..05fdfca7cf 100644 --- a/token/services/identity/idemix/km_bench_test.go +++ b/token/services/identity/idemix/km_bench_test.go @@ -20,16 +20,6 @@ import ( // BenchmarkKmIdentity benchmarks the identity creation func BenchmarkKmIdentity(b *testing.B) { - b.Run("FP256BN_AMCL", func(b *testing.B) { - b.ReportAllocs() - - keyManager, cleanup := setupKeyManager(b, "./testdata/fp256bn_amcl/idemix", math.FP256BN_AMCL) - defer cleanup() - for b.Loop() { - _, _ = keyManager.Identity(b.Context(), nil) - } - }) - b.Run("BLS12_381_BBS", func(b *testing.B) { b.ReportAllocs() diff --git a/token/services/identity/idemix/km_test.go b/token/services/identity/idemix/km_test.go index dac15c05d4..ba186ebf2d 100644 --- a/token/services/identity/idemix/km_test.go +++ b/token/services/identity/idemix/km_test.go @@ -872,3 +872,75 @@ func TestIdentityWithDifferentAuditInfo(t *testing.T) { require.NoError(t, verifier1.Verify(msg, sig1)) require.NoError(t, verifier2.Verify(msg, sig2)) } + +// TestEphemeralIdentity verifies that EphemeralIdentity produces valid identities +// without persisting the derived nym key in the key store. +// This ensures that pre-generated cache pseudonyms do not accumulate on disk. +func TestEphemeralIdentity(t *testing.T) { + testEphemeralIdentity(t, "./testdata/bls12_381_bbs_gurvy/idemix", math.BLS12_381_BBS_GURVY) + testEphemeralIdentity(t, "./testdata/bls12_381_bbs/idemix", math.BLS12_381_BBS_GURVY) +} + +func testEphemeralIdentity(t *testing.T, configPath string, curveID math.CurveID) { + t.Helper() + + // Setup: create a tracked KVS to observe key store operations + kvs, err := kvs2.NewInMemory() + require.NoError(t, err) + config, err := crypto.NewConfig(configPath) + require.NoError(t, err) + tracker := kvs2.NewTrackedMemoryFrom(kvs) + keyStore, err := crypto.NewKeyStore(curveID, tracker) + require.NoError(t, err) + cryptoProvider, err := crypto.NewBCCSP(keyStore, curveID) + require.NoError(t, err) + + keyManager, err := NewKeyManager(config, types.EidNymRhNym, cryptoProvider) + require.NoError(t, err) + + // Creating the key manager should store the user secret key (1 Put) + putCountAfterSetup := tracker.PutCounter + + // Generate an ephemeral identity + desc, err := keyManager.EphemeralIdentity(t.Context(), nil) + require.NoError(t, err) + assert.NotNil(t, desc) + assert.NotNil(t, desc.Identity) + assert.NotNil(t, desc.AuditInfo) + assert.NotNil(t, desc.Signer) + + // Verify that NO additional keys were persisted in the key store. + // The nym key should have been temporary. + assert.Equal(t, putCountAfterSetup, tracker.PutCounter, + "EphemeralIdentity should not persist nym keys in the key store") + + // Generate a regular (persistent) identity for comparison + descPersistent, err := keyManager.Identity(t.Context(), nil) + require.NoError(t, err) + assert.NotNil(t, descPersistent) + + // The persistent Identity() should have stored the nym key (additional Puts) + assert.Greater(t, tracker.PutCounter, putCountAfterSetup, + "Identity() should persist nym keys in the key store") + + // Verify that the ephemeral identity can still be used for signing + signer := desc.Signer + msg := []byte("test ephemeral signing") + sigma, err := signer.Sign(msg) + require.NoError(t, err) + + // Verify the ephemeral identity's signature + verifier, err := keyManager.DeserializeVerifier(t.Context(), desc.Identity) + require.NoError(t, err) + require.NoError(t, verifier.Verify(msg, sigma)) + + // Generate multiple ephemeral identities and verify none persist nym keys + putCountBefore := tracker.PutCounter + for i := 0; i < 5; i++ { + d, err := keyManager.EphemeralIdentity(t.Context(), nil) + require.NoError(t, err) + assert.NotNil(t, d) + } + assert.Equal(t, putCountBefore, tracker.PutCounter, + "Multiple EphemeralIdentity calls should not persist any nym keys") +} diff --git a/token/services/identity/idemix/kmp.go b/token/services/identity/idemix/kmp.go index 9a5b8e51ec..a1a612edfc 100644 --- a/token/services/identity/idemix/kmp.go +++ b/token/services/identity/idemix/kmp.go @@ -128,12 +128,34 @@ func (l *KeyManagerProvider) Get(ctx context.Context, identityConfig *driver.Ide return nil, errors.Errorf("cannot invoke this function, remote must register pseudonyms on wallet [%v]", id) } } else { - getIdentityFunc = cache.NewIdentityCache( - keyManager.Identity, + identityGetter := keyManager.Identity + if ephemeralKM, ok := keyManager.(interface { + EphemeralIdentity(context.Context, []byte) (*idriver.IdentityDescriptor, error) + }); ok { + identityGetter = ephemeralKM.EphemeralIdentity + } + c := cache.NewIdentityCache( + identityGetter, cacheSize, nil, cache.NewMetrics(l.metricsProvider), - ).Identity + ) + getIdentityFunc = func(ctx context.Context, auditInfo []byte) (*idriver.IdentityDescriptor, error) { + desc, err := c.Identity(ctx, auditInfo) + if err != nil { + return nil, err + } + if desc != nil && desc.Signer != nil { + if signingID, ok := desc.Signer.(*crypto2.SigningIdentity); ok { + if signingID.NymKey != nil { + if err := l.keyStore.StoreKey(signingID.NymKey); err != nil { + return nil, errors.Wrap(err, "failed to persist nym key from cache") + } + } + } + } + return desc, nil + } } // finalize identity configuration diff --git a/token/services/identity/idemix/kmp_test.go b/token/services/identity/idemix/kmp_test.go index 075ca0d282..d39ad436c1 100644 --- a/token/services/identity/idemix/kmp_test.go +++ b/token/services/identity/idemix/kmp_test.go @@ -9,6 +9,7 @@ package idemix import ( "context" "testing" + "time" math "github.com/IBM/mathlib" "github.com/hyperledger-labs/fabric-smart-client/pkg/utils/proto" @@ -341,3 +342,62 @@ func TestKeyManagerProviderGetWithRawConfig(t *testing.T) { require.NoError(t, err) assert.NotNil(t, km2) } + +// TestCacheUsesEphemeralIdentity verifies that cache-generated identities +// from the WrappedKeyManager do not persist nym keys in the key store. +// This validates the fix for issue #1820: when the cache size is reduced, +// previously cached pseudonyms should not leave orphaned keys on disk. +func TestCacheUsesEphemeralIdentity(t *testing.T) { + testCacheUsesEphemeralIdentity(t, "./testdata/bls12_381_bbs_gurvy/idemix", math.BLS12_381_BBS_GURVY) + testCacheUsesEphemeralIdentity(t, "./testdata/bls12_381_bbs/idemix", math.BLS12_381_BBS_GURVY) +} + +func testCacheUsesEphemeralIdentity(t *testing.T, configPath string, curveID math.CurveID) { + t.Helper() + + backend, err := kvs2.NewInMemory() + require.NoError(t, err) + config, err := crypto.NewConfig(configPath) + require.NoError(t, err) + tracker := kvs2.NewTrackedMemoryFrom(backend) + keyStore, err := crypto.NewKeyStore(curveID, tracker) + require.NoError(t, err) + + kmp := NewKeyManagerProvider( + config.Ipk, + curveID, + keyStore, + &mockConfig{}, + 3, // Set cache size to 3 + false, + &disabled.Provider{}, + ) + + idConfig := &token.IdentityConfiguration{ + ID: "alice", + URL: configPath, + } + // Get the WrappedKeyManager (km) which uses the cache internally + km, err := kmp.Get(context.Background(), idConfig) + require.NoError(t, err) + + // Record the put counter after setup (key manager creation stores the user secret key) + putCountAfterSetup := tracker.PutCounter + + // Allow some time for background cache provisioning to fill the cache (cacheSize = 3) + time.Sleep(500 * time.Millisecond) + + // Verify that background pre-generation did NOT persist any nym keys. + // If the cache was not using ephemeral keys, this would have stored 3 keys. + assert.Equal(t, putCountAfterSetup, tracker.PutCounter, + "Background cache pre-generation should not persist nym keys in the key store") + + // Now fetch an identity from the cache. This simulates the identity being used/given out. + desc, err := km.Identity(context.Background(), nil) + require.NoError(t, err) + assert.NotNil(t, desc) + + // It should now be persisted in the key store so it can be reloaded/re-signed after restart. + assert.Equal(t, putCountAfterSetup+1, tracker.PutCounter, + "Fetched/used identity should be persisted in the key store") +} diff --git a/token/services/identity/idemixnym/km.go b/token/services/identity/idemixnym/km.go index 141439d3c1..6a219fb349 100644 --- a/token/services/identity/idemixnym/km.go +++ b/token/services/identity/idemixnym/km.go @@ -84,6 +84,18 @@ func (k *KeyManager) IdentityType() idriver.IdentityType { } func (k *KeyManager) Identity(ctx context.Context, referenceAuditInfo []byte) (*idriver.IdentityDescriptor, error) { + return k.identityWithBackendFunc(ctx, referenceAuditInfo, k.backend.Identity) +} + +// EphemeralIdentity returns the identity descriptor without persisting the derived nym key. +// This is intended for pre-generated cache identities that do not need to survive a restart. +func (k *KeyManager) EphemeralIdentity(ctx context.Context, referenceAuditInfo []byte) (*idriver.IdentityDescriptor, error) { + return k.identityWithBackendFunc(ctx, referenceAuditInfo, k.backend.EphemeralIdentity) +} + +// identityWithBackendFunc is the core implementation for identity generation. +// The backendIdentityFunc parameter controls whether the nym key is persisted or ephemeral. +func (k *KeyManager) identityWithBackendFunc(ctx context.Context, referenceAuditInfo []byte, backendIdentityFunc func(context.Context, []byte) (*idriver.IdentityDescriptor, error)) (*idriver.IdentityDescriptor, error) { var backendAuditInfoRaw []byte if len(referenceAuditInfo) != 0 { // extract the audit infor for the backed @@ -97,7 +109,7 @@ func (k *KeyManager) Identity(ctx context.Context, referenceAuditInfo []byte) (* } } - descriptor, err := k.backend.Identity(ctx, backendAuditInfoRaw) + descriptor, err := backendIdentityFunc(ctx, backendAuditInfoRaw) if err != nil { return nil, err }