[16] RFR JDK-8246383: NullPointerException in JceSecurity.getVerificationResult when using Entrust provider

[Xuelei Fan]

Looks good to me.  Thanks!

Xuelei

On 8/20/2020 1:28 PM, Valerie Peng wrote:
> Updated to use the "JCAUtil.getSecureRandom()" call:
> 
> http://cr.openjdk.java.net/~valeriep/8246383/webrev.01/
> 
> Thanks,
> Valerie
> 
> On 8/19/2020 11:20 AM, Valerie Peng wrote:
> 
>> Hi Xuelei,
>>
>> Please find comments in line.
>>
>> On 8/18/2020 10:13 PM, Xuelei Fan wrote:
>>> On 8/18/2020 2:43 PM, Valerie Peng wrote:
>>>>
>>>> Using a shared instance is surely faster. However, the API specified 
>>>> that the most preferred SecureRandom impl will be used. To ensure 
>>>> this for all scenarios, creating default SecureRandom obj will 
>>>> provide correct result but shared instance may not.
>>> I understand your point.  It might not break the spec if a shared 
>>> instance is used.  It depends on the understanding of "most preferred 
>>> SecureRandom impl" in the context.
>>
>> My reading of the spec is more strict I guess. I went back and force 
>> from 2 approaches, one is to use the slow "new SecureRandom()", the 
>> other is to still use a shared instance (located elsewhele instead of 
>> the sensitive JceSecurity class which is used when verifying JCE 
>> providers). The former is slow but correct at all times, the later can 
>> be correct as long as there are no provider changes after the first call.
>>
>>>> Apps can call other init functions which takes SecureRandom objects 
>>>> to avoid this default SecureRandom obj creation if needed.
>>>>
>>> Yes, it's an alternative solution.  If an application used the 
>>> default SecureRandom, it would be nice if there is no performance 
>>> regression.
>>>
>>> The SecureRandom initialization may be not cheap in some 
>>> circumstances. As this bug did not complain about the use of shared 
>>> instance, it may be fine if we want to avoid the performance impact 
>>> if the impact exists.
>>
>> Yes, I ran a test to profile the numbers. It's a painful decision to 
>> go with "new SecureRandom()"...
>>
>> I can change to my other approach of using JCAUtil.getSecureRandom() 
>> depending on Sean's feedback.
>>
>> Thanks,
>> Valerie
>>
>>
>>>
>>> Just for your consideration.
>>>
>>> Xuelei
>>>
>>>> Valerie
>>>>
>>>> On 8/18/2020 2:10 PM, Xuelei Fan wrote:
>>>>> Is there any performance impact?
>>>>>
>>>>> Xuelei
>>>>>
>>>>> On 8/18/2020 12:51 PM, Valerie Peng wrote:
>>>>>>
>>>>>> Anyone has cycles to review this somewhat trivial changes? 
>>>>>> JceSecurity has this shared SecureRandom instance which may lead 
>>>>>> to NPE when certain 3rd party JCE provider is set as most 
>>>>>> preferred. Removing this shared instance and change to create 
>>>>>> default SecureRandom obj when needed.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8246383
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8246383/webrev.00/
>>>>>>
>>>>>> Thanks,
>>>>>> Valerie