[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