Re: RFR: 8260274: Cipher.init(int, key) does not use highest priority provider for random bytes [v2]

[SalusaSecondus]

On Wed, 17 Mar 2021 19:25:13 GMT, Valerie Peng <valer...@openjdk.org> wrote:

>> Can someone help review this somewhat trivial change?
>> 
>> Updated JCAUtil class to return the cached SecureRandom object only when the 
>> provider configuration has not changed. 
>> Added a regression test to check the affected classes, i.e. 
>> AlgorithmParameterGenerator, KeyPairGenerator, Cipher, KeyAgreement, 
>> KeyGenerator. 
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changed to use volatile for the default SecureRandom object reference

src/java.base/share/classes/sun/security/jca/JCAUtil.java line 29:

> 27: 
> 28: import java.lang.ref.*;
> 29: import java.util.Arrays;

Import no longer needed.

src/java.base/share/classes/sun/security/jca/JCAUtil.java line 84:

> 82:      * SecureRandom impl if the provider table is the same.
> 83:      */
> 84:     public static SecureRandom getDefSecureRandom() {

When should people use `getSecureRandom` rather than `getDefSecureRandom()`? 
Are we leaving the old to avoid changing behavior out from under people who may 
not expect it?

src/java.base/share/classes/sun/security/jca/JCAUtil.java line 92:

> 90:             }
> 91:         }
> 92:         return def;

Do we need to worry about `clearDefSecureRandom()` being called between lines 
88 and 92?

Thread 1: `getDefSecureRandom()#85` (`def` is `null`)
Thread 1: `getDefSecureRandom()#86`
Thread 1: `getDefSecureRandom()#87` (`def` is still `null`)
Thread 1: `getDefSecureRandom()#88` (`def` is **not** `null`)
Thread 2: `clearDefSecureRandom()` (`def` is `null` again)
Thread 1: `getDefSecureRandom()#92` (return `def` which is incorrectly `null`)


Suggestion:

        SecureRandom result = def;
        if (result == null) {
            synchronized (JCAUtil.class) {
                result = def;
                if (result == null) {
                    def = result = new SecureRandom()
                }
            }
        }
        return result;

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

PR: https://git.openjdk.java.net/jdk/pull/3018