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

[Xue-Lei Andrew Fan]

On Tue, 16 Mar 2021 00:10:00 GMT, SalusaSecondus <github.com+829871+SalusaSecondus at openjdk.org> wrote:

>> Or, I can also separate out the instance returned by getSecureRandom() so that code path should be unaffected, e.g.:
>>     // cached SecureRandom instance
>>     private static class CachedSecureRandomHolder {
>>         public static SecureRandom instance = new SecureRandom();
>>         private static Provider[] cachedConfig = Security.getProviders();
>>         private static SecureRandom def = instance;
>> 
>>         public static SecureRandom getDefault() {
>>             synchronized (CachedSecureRandomHolder.class) {
>>                 Provider[] currConfig = Security.getProviders();
>>                 if (!Arrays.equals(cachedConfig, currConfig)) {
>>                     def = new SecureRandom();
>>                     cachedConfig = currConfig;
>>                 }
>>             }
>>             return def;
>>         }
>>     }
>> 
>>     /**
>>      * Get a SecureRandom instance. This method should be used by JDK
>>      * internal code in favor of calling "new SecureRandom()". That needs to
>>      * iterate through the provider table to find the default SecureRandom
>>      * implementation, which is fairly inefficient.
>>      */
>>     public static SecureRandom getSecureRandom() {
>>         return CachedSecureRandomHolder.instance;
>>     }
>> 
>>     /**
>>      * Get the default SecureRandom instance. This method is the
>>      * optimized version of "new SecureRandom()" which re-uses the default
>>      * SecureRandom impl if the provider table is the same.
>>      */
>>     public static SecureRandom getDefSecureRandom() {
>>         return CachedSecureRandomHolder.getDefault();
>>     }
>
> Doesn't that still incur a JVM-wide lock whenever `CachedSecureRandomHolder.getDefSecureRandom()` is called?  I recall a different JVM-wide lock in the `getInstance` path ([JDK-7107615](https://bugs.openjdk.java.net/browse/JDK-7107615)) creating significant performance issues.

I did not get the point that the code path should be unaffected". As there are a few update like:
         init(keysize, JCAUtil.getSecureRandom());
         init(keysize, JCAUtil.getDefSecureRandom());

I think there are some impact on the existing application which use the default sec

> Or, I can also separate out the instance returned by getSecureRandom() so that code path should be unaffected, e.g.:

I did not get the point that the "code path should be unaffected"? JCAUtil.getDefSecureRandom() is used in updates like:
         init(keysize, JCAUtil.getSecureRandom());
         init(keysize, JCAUtil.getDefSecureRandom());

> 
> ```
>     // cached SecureRandom instance
>     private static class CachedSecureRandomHolder {
>         public static SecureRandom instance = new SecureRandom();
>         private static Provider[] cachedConfig = Security.getProviders();
>         private static SecureRandom def = instance;
> 
>         public static SecureRandom getDefault() {
>             synchronized (CachedSecureRandomHolder.class) {
>                 Provider[] currConfig = Security.getProviders();
>                 if (!Arrays.equals(cachedConfig, currConfig)) {
>                     def = new SecureRandom();
>                     cachedConfig = currConfig;
>                 }
>             }
>             return def;
>         }
>     }
> 
>     /**
>      * Get a SecureRandom instance. This method should be used by JDK
>      * internal code in favor of calling "new SecureRandom()". That needs to
>      * iterate through the provider table to find the default SecureRandom
>      * implementation, which is fairly inefficient.
>      */
>     public static SecureRandom getSecureRandom() {
>         return CachedSecureRandomHolder.instance;
>     }
> 
>     /**
>      * Get the default SecureRandom instance. This method is the
>      * optimized version of "new SecureRandom()" which re-uses the default
>      * SecureRandom impl if the provider table is the same.
>      */
>     public static SecureRandom getDefSecureRandom() {
>         return CachedSecureRandomHolder.getDefault();
>     }
> ```

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

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