Data race in bbs.f2192() curve cache (unsynchronized map, potential fatal concurrent map access)

[EvanYan1024]

Summary

bbs.f2192() caches the constant 2^192 per curve in a package-level map with no synchronization:

https://github.com/IBM/idemix/blob/main/bbs/fr.go#L29-L38

f2192Cache = make(map[*ml.Curve]*ml.Zr)

func f2192(curve *ml.Curve) *ml.Zr {
	if cached, ok := f2192Cache[curve]; ok {  // read
		return cached
	}
	val := curve.NewZrFromBytes(f2192Bytes)
	f2192Cache[curve] = val                    // write
	return val
}

FrFromOKM calls it on every hash-to-field, so essentially every BBS operation (sign, verify, NymEid audit) goes through it. Two goroutines performing their first BBS operations concurrently race on the map: the write happens only on the first use per curve, so the race is confined to that startup window — which makes it intermittent, but a process that serves concurrent verifications right after start hits exactly that window. Besides failing -race builds, unsynchronized concurrent map access can terminate the process with an unrecoverable fatal error: concurrent map read and map write.

Observed

Race detector report from a downstream CI run (LFDT-Panurus/panurus unit tests, -race, parallel verifications; Go 1.26.3, idemix v0.1.1) — full log: https://github.com/LFDT-Panurus/panurus/actions/runs/28915702306/job/85783037774

WARNING: DATA RACE
Read at 0x00c0003105a0 by goroutine 18:
  runtime.mapaccess2_fast64()
  github.com/IBM/idemix/bbs.f2192()                       bbs/fr.go:32
  github.com/IBM/idemix/bbs.FrFromOKM()                   bbs/fr.go:58
  github.com/IBM/idemix/bbs.ParseProofNonce()             bbs/bbs12381g2pub.go:382
  github.com/IBM/idemix/bccsp/schemes/aries.(*Signer).Verify()
  ...

Previous write at 0x00c0003105a0 by goroutine 16:
  runtime.mapassign_fast64ptr()
  github.com/IBM/idemix/bbs.f2192()                       bbs/fr.go:36
  github.com/IBM/idemix/bbs.FrFromOKM()                   bbs/fr.go:58
  github.com/IBM/idemix/bccsp/schemes/aries.(*Signer).AuditNymEid()
  ...

History

• The cache was introduced by perf: migrate to internal optimized bbs module #68 as a single f2192Cached *ml.Zr (which additionally served the first curve's value to every curve).
• modernization #77 turned it into the per-curve map, fixing the cross-curve value but keeping the unsynchronized access.
• First released in v0.1.0 / v0.1.1. The code this was migrated from (hyperledger/aries-bbs-go bbs/fr.go) computes the value on every call and has no race.

[adecaro]

Hi @EvanYan1024 , good catch. Thanks much 🙏
Are you gonna submit a PR to fix this as well? Let me know, in case I can submit a fix as well.

[EvanYan1024]

Hi @EvanYan1024 , good catch. Thanks much 🙏 Are you gonna submit a PR to fix this as well? Let me know, in case I can submit a fix as well.

I opened #84 with a fix for this.

The change precomputes the f2192 value for every registered ml.Curve during package initialization and keeps the cache read-only afterwards, so concurrent FrFromOKM calls no longer race on the package-level map. For curves outside ml.Curves, it computes the value without caching.

I also added a concurrent FrFromOKM test covering all registered curves. go test -race ./bbs/... passes locally.