fix(keysharecore): guard Core.trustedKeys with an RWMutex (#594)

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+### Fixed
+- Guard the keyshare core's `Core.trustedKeys` map with an `RWMutex`. It was the only one of the three maps on `Core` without a lock, yet `DangerousAddTrustedPublicKey` (a `Configuration` `UpdateListener`, so it fires on the request path on every scheme update) writes it while the `Generate*` request handlers read it concurrently, which could trigger a fatal `concurrent map read and map write` panic and crash the keyshare server.
 
 ## [1.2.0] - 2026-07-22
 ### Added

internal/keysharecore/core.go

@@ -41,7 +41,8 @@ type (
 
 		// IRMA issuer keys that are allowed to be used in keyshare
 		//  sessions
-		trustedKeys map[irma.PublicKeyIdentifier]*gabikeys.PublicKey
+		trustedKeys      map[irma.PublicKeyIdentifier]*gabikeys.PublicKey
+		trustedKeysMutex sync.RWMutex
 	}
 
 	Configuration struct {
@@ -111,5 +112,7 @@ func (c *Core) setJWTPrivateKey(id uint32, key *rsa.PrivateKey) {
 // DangerousAddTrustedPublicKey adds a public key as trusted by keysharecore.
 // Calling this on incorrectly generated key material WILL compromise keyshare secrets!
 func (c *Core) DangerousAddTrustedPublicKey(keyID irma.PublicKeyIdentifier, key *gabikeys.PublicKey) {
+	c.trustedKeysMutex.Lock()
+	defer c.trustedKeysMutex.Unlock()
 	c.trustedKeys[keyID] = key
 }

internal/keysharecore/operations.go

@@ -6,6 +6,7 @@ import (
 	"crypto/subtle"
 	"encoding/base64"
 	"encoding/binary"
+	"maps"
 	"strconv"
 	"time"
 
@@ -203,13 +204,16 @@ func (c *Core) verifyAccess(secrets UserSecrets, jwtToken string) (unencryptedUs
 func (c *Core) GeneratePs(secrets UserSecrets, accessToken string, keyIDs []irma.PublicKeyIdentifier) ([]*big.Int, error) {
 	// Validate input request and build key list
 	var keyList []*gabikeys.PublicKey
+	c.trustedKeysMutex.RLock()
 	for _, keyID := range keyIDs {
 		key, ok := c.trustedKeys[keyID]
 		if !ok {
+			c.trustedKeysMutex.RUnlock()
 			return nil, ErrKeyNotFound
 		}
 		keyList = append(keyList, key)
 	}
+	c.trustedKeysMutex.RUnlock()
 
 	// Use verifyAccess to get the decrypted secrets. The access has already been verified in the
 	// middleware. We use the call merely to fetch the unencryptedUserSecrets here.
@@ -232,13 +236,16 @@ func (c *Core) GeneratePs(secrets UserSecrets, accessToken string, keyIDs []irma
 func (c *Core) GenerateCommitments(secrets UserSecrets, accessToken string, keyIDs []irma.PublicKeyIdentifier) ([]*gabi.ProofPCommitment, uint64, error) {
 	// Validate input request and build key list
 	var keyList []*gabikeys.PublicKey
+	c.trustedKeysMutex.RLock()
 	for _, keyID := range keyIDs {
 		key, ok := c.trustedKeys[keyID]
 		if !ok {
+			c.trustedKeysMutex.RUnlock()
 			return nil, 0, ErrKeyNotFound
 		}
 		keyList = append(keyList, key)
 	}
+	c.trustedKeysMutex.RUnlock()
 
 	// Use verifyAccess to get the decrypted secrets. The access has already been verified in the
 	// middleware. We use the call merely to fetch the unencryptedUserSecrets here.
@@ -274,7 +281,9 @@ func (c *Core) GenerateResponse(secrets UserSecrets, accessToken string, commitI
 	if uint(challenge.BitLen()) > gabikeys.DefaultSystemParameters[1024].Lh || challenge.Cmp(big.NewInt(0)) < 0 {
 		return "", ErrInvalidChallenge
 	}
+	c.trustedKeysMutex.RLock()
 	key, ok := c.trustedKeys[keyID]
+	c.trustedKeysMutex.RUnlock()
 	if !ok {
 		return "", ErrKeyNotFound
 	}
@@ -317,8 +326,14 @@ func (c *Core) GenerateResponseV2(
 	req gabi.KeyshareResponseRequest[irma.PublicKeyIdentifier],
 	keyID irma.PublicKeyIdentifier,
 	linkable bool) (string, error) {
-	// Validate request
+	// Validate request. Look up the requested key and take a snapshot of the
+	// trusted keys map under the read lock, so gabi.KeyshareResponse below never
+	// reads the live map while DangerousAddTrustedPublicKey may be writing it.
+	c.trustedKeysMutex.RLock()
 	key, ok := c.trustedKeys[keyID]
+	trustedKeys := make(map[irma.PublicKeyIdentifier]*gabikeys.PublicKey, len(c.trustedKeys))
+	maps.Copy(trustedKeys, c.trustedKeys)
+	c.trustedKeysMutex.RUnlock()
 	if !ok {
 		return "", ErrKeyNotFound
 	}
@@ -339,7 +354,7 @@ func (c *Core) GenerateResponseV2(
 		return "", ErrUnknownCommit
 	}
 
-	proofP, err := gabi.KeyshareResponse(s.KeyshareSecret, commit, hashedComms, req, c.trustedKeys)
+	proofP, err := gabi.KeyshareResponse(s.KeyshareSecret, commit, hashedComms, req, trustedKeys)
 	if err != nil {
 		return "", err
 	}

internal/keysharecore/operations_test.go

@@ -7,6 +7,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"os"
+	"sync"
 	"testing"
 	"time"
 
@@ -440,6 +441,61 @@ func TestNonExistingCommit(t *testing.T) {
 	}
 }
 
+// TestTrustedKeysConcurrentAccess is a regression test for the data race on the
+// unsynchronized Core.trustedKeys map (issue #594). DangerousAddTrustedPublicKey
+// is registered as a Configuration UpdateListener, so it runs on the request path
+// (on every scheme update), while the Generate* readers iterate the same map
+// concurrently. Before the fix this is a "concurrent map read and map write"
+// fatal panic. Run with the race detector to guard against a regression:
+//
+//	go test -race -run TestTrustedKeysConcurrentAccess ./internal/keysharecore/
+func TestTrustedKeysConcurrentAccess(t *testing.T) {
+	var key AESKey
+	_, err := rand.Read(key[:])
+	require.NoError(t, err)
+	c := NewKeyshareCore(&Configuration{DecryptionKeyID: 1, DecryptionKey: key, JWTPrivateKeyID: 1, JWTPrivateKey: jwtTestKey})
+	trustedKeyID := irma.PublicKeyIdentifier{Issuer: irma.NewIssuerIdentifier("test"), Counter: 1}
+	c.DangerousAddTrustedPublicKey(trustedKeyID, testPubK1)
+
+	// Build valid user secrets and an auth JWT so the readers exercise their full
+	// path (they call verifyAccess after reading trustedKeys).
+	pin := generatePin()
+	secrets, err := c.NewUserSecrets(pin, nil)
+	require.NoError(t, err)
+	jwtt, err := validateAuth(t, c, nil, secrets, pin)
+	require.NoError(t, err)
+
+	const iterations = 200
+	var wg sync.WaitGroup
+
+	// Writer: keeps adding new trusted keys, mimicking the config UpdateListener
+	// that fires DangerousAddTrustedPublicKey on every scheme update.
+	wg.Go(func() {
+		for i := range iterations {
+			c.DangerousAddTrustedPublicKey(
+				irma.PublicKeyIdentifier{Issuer: irma.NewIssuerIdentifier("test"), Counter: uint(i + 2)},
+				testPubK1,
+			)
+		}
+	})
+
+	// Readers: GeneratePs and GenerateCommitments both iterate trustedKeys, which
+	// is exactly the read that races with the writer above on the live map.
+	keyIDs := []irma.PublicKeyIdentifier{trustedKeyID}
+	for range 4 {
+		wg.Go(func() {
+			for range iterations {
+				_, err := c.GeneratePs(secrets, jwtt, keyIDs)
+				assert.NoError(t, err)
+				_, _, err = c.GenerateCommitments(secrets, jwtt, keyIDs)
+				assert.NoError(t, err)
+			}
+		})
+	}
+
+	wg.Wait()
+}
+
 // Test data
 const xmlPubKey1 = `<?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <IssuerPublicKey xmlns="http://www.zurich.ibm.com/security/idemix">