The BlockPool API added in bcgit/bc-java#1646 allows callers to pool and reuse Argon2 memory blocks, avoiding the GC pressure of allocating and discarding several MB of long[] arrays per hash. The API defines BlockPool as an interface, so consumers can provide their own pooling strategy.
Current behavior
Block.clear() is private, so only FixedBlockPool — as an inner class of Argon2BytesGenerator — can zero out block contents. Custom BlockPool implementations have no way to clear sensitive password-derived data from pooled blocks.
This forces consumers to use FixedBlockPool, even when its semantics (synchronized access, pre-sized ArrayList) don't fit their use case. For example, in keycloak/keycloak#52797 we want to wrap individual pools in SoftReferences so the JVM can reclaim them under memory pressure. To correctly size a FixedBlockPool, we have to replicate the internal block count calculation (accounting for max(memory, 8 * parallelism), lane alignment rounding, and the 4 FillBlock scratch blocks) — logic that's an implementation detail of Argon2BytesGenerator and could change between releases.
Expected behavior
Custom BlockPool implementations should be able to clear sensitive data from blocks. Two possible approaches:
- Preferred: Have
Argon2BytesGenerator call Block.clear() on each block before returning it to the pool. This would make any BlockPool implementation safe by default.
- Alternative: Make
Block.clear() public so custom implementations can call it themselves.
Workaround
We currently use FixedBlockPool with a replicated version of the internal block count calculation (max(memory, 8 * parallelism), lane alignment rounding, plus 4 scratch blocks) to size the pool correctly. This works but is fragile — it couples to implementation details that could change between BouncyCastle releases.
Additional question
FixedBlockPool.allocate() clears the block again on retrieval from the pool, with the comment "a deallocate() in another thread may not have published its clear()". Since both allocate() and deallocate() are synchronized on the same monitor, the happens-before relationship should already guarantee visibility of the deallocate()-side clear. Is the double clear intentional, or could it be removed to avoid zeroing each block twice per use? It seems that the synchronize was added after the original suggested PR.
Once there is an agreed direction, I'm happy to provide a pull request.
The
BlockPoolAPI added in bcgit/bc-java#1646 allows callers to pool and reuse Argon2 memory blocks, avoiding the GC pressure of allocating and discarding several MB oflong[]arrays per hash. The API definesBlockPoolas an interface, so consumers can provide their own pooling strategy.Current behavior
Block.clear()isprivate, so onlyFixedBlockPool— as an inner class ofArgon2BytesGenerator— can zero out block contents. CustomBlockPoolimplementations have no way to clear sensitive password-derived data from pooled blocks.This forces consumers to use
FixedBlockPool, even when its semantics (synchronized access, pre-sizedArrayList) don't fit their use case. For example, in keycloak/keycloak#52797 we want to wrap individual pools inSoftReferences so the JVM can reclaim them under memory pressure. To correctly size aFixedBlockPool, we have to replicate the internal block count calculation (accounting formax(memory, 8 * parallelism), lane alignment rounding, and the 4FillBlockscratch blocks) — logic that's an implementation detail ofArgon2BytesGeneratorand could change between releases.Expected behavior
Custom
BlockPoolimplementations should be able to clear sensitive data from blocks. Two possible approaches:Argon2BytesGeneratorcallBlock.clear()on each block before returning it to the pool. This would make anyBlockPoolimplementation safe by default.Block.clear()public so custom implementations can call it themselves.Workaround
We currently use
FixedBlockPoolwith a replicated version of the internal block count calculation (max(memory, 8 * parallelism), lane alignment rounding, plus 4 scratch blocks) to size the pool correctly. This works but is fragile — it couples to implementation details that could change between BouncyCastle releases.Additional question
FixedBlockPool.allocate()clears the block again on retrieval from the pool, with the comment "a deallocate() in another thread may not have published its clear()". Since bothallocate()anddeallocate()are synchronized on the same monitor, the happens-before relationship should already guarantee visibility of thedeallocate()-side clear. Is the double clear intentional, or could it be removed to avoid zeroing each block twice per use? It seems that the synchronize was added after the original suggested PR.Once there is an agreed direction, I'm happy to provide a pull request.