[crac] RFR: Reseed secure random on checkpoint restore [v3]

[Anton Kozlov]

On Wed, 12 Jan 2022 14:09:34 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

>> Proposed changes in the SecureRandom implementation allow invalidating and reseeding SHA1PRNG secure random during checkpoint/restore. SHA1PRNG can be invalidated and reseeded in case of being created with a default embedded seed generator. Also, SHA1PRNG is used as an additional seed generator to the SUN NativePRNG implementation, so it is desirable to have reseeded SHA1PRNG after restore.
>> Two jtreg tests added: 
>> - verify if no deadlocks introduced by checkpoint/restore
>> - verify if SHA1PRNG is reseeded if created with default embedded seed generator
>
> Alexey Bakhtin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Set JDKResource priorities for SecureRandom
>  - Merge branch 'crac' of https://github.com/openjdk/crac into SecureRandom
>  - Add separate JDKResorce for seeder
>  - Reseed secure random on checkpoint restore

src/java.base/share/classes/sun/security/provider/SecureRandom.java line 256:

> 254:             SeedGenerator.generateSeed(b);
> 255:             seeder.engineSetSeed(b);
> 256:             jdk.internal.crac.Core.getJDKContext().register(new SeederHolder());

Resources are weakly referenced [1], so this SeederHolder object will likely be collected very soon

[1] https://github.com/openjdk/crac/blob/43aba3d502832a5a3d2e9712558f62e0cf93dbbb/src/java.base/share/classes/jdk/crac/package-info.java#L74

src/java.base/share/classes/sun/security/provider/SecureRandom.java line 261:

> 259:         @Override
> 260:         public void beforeCheckpoint(Context<? extends Resource> context) throws Exception {
> 261:             objLock.lock();

Do we assume a special state of seeder? A comment is really needed. This lock is not acquired anywhere else, i.e. it does not guard access to the static `seeder` field. The field can be referenced by a SecureRandom object that was created after Priority.SECURE_RANDOM was handled (the new SecureRandom was not notified about checkpoint -- there is no problem unless the new SecureRandom catches some state). If seeder has built-in protection and will block the new SecureRandom, the lock is not necessary. If not, seeder needs to be guarded. In the latter case, it should probably be a ReadWriteLock as in #9.

-------------

PR: https://git.openjdk.java.net/crac/pull/7