[crac] RFR: Reseed NativePRNG on checkpoint restore
Anton Kozlov
akozlov at openjdk.java.net
Fri Dec 24 12:59:47 UTC 2021
On Thu, 23 Dec 2021 11:30:13 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:
> NativePRNG should be re-seeded during checkpoint/restore because it uses SHA1PRNG secure random for additional seed. It is seeded at initialization, so it is not re-seeded automatically during checkpoint/restore
> Also, the internal buffer should be cleared at the checkpoint.
src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 391:
> 389:
> 390: // lock for checkpoint/restore
> 391: private final ReentrantLock crLock = new ReentrantLock();
The comment should specify it protects both mixRandom and the buffer of the truly random values read from OS.
src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 589:
> 587: @Override
> 588: public void beforeCheckpoint(Context<? extends Resource> context) throws Exception {
> 589: crLock.lock();
The new lock is very related to `LOCK_GET_BYTES`, however, this code does not acquire `LOCK_GET_BYTES`. Now all `synchronized (LOCK_GET_BYTES)` blocks are executed under `crLock`. But I would prefer at least `assert crLock.isHeldByCurrentThread()` near those synchronized blocks, or any other way to ensure / document locks relation. Or use the LOCK_GET_BYTES, considering the other problem with premature buffer and mixRandom initialzation.
src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 595:
> 593: for(int i=0; i<nextBuffer.length; i++) {
> 594: nextBuffer[i] = 0;
> 595: }
I assume this clean-up serves two purposes: 1) to clear the state that was already used to generate other values, so to prevent guessing them; 2) to force next request for random values will force filling of the buffer with real random data and fresh initialization of mixRandom.
For 2, what if another `beforeCheckpoint` inadvertently calls `NativePRNG.engineNextBytes`? `crLock` (reentrant) will not prevent reading random values from OS and setting an instance for `mixRandom` before the checkpoint, but they will live later forever on restore. So I assume a similar clean-up is required in `afterRestore` (we may happen to store some state for NativePRNG, but this state won't be related to the previous state before the checkpoint and the one created later after restore).
-------------
PR: https://git.openjdk.java.net/crac/pull/9
More information about the crac-dev
mailing list