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

[Valerie Peng]

Thanks for the review~

Valerie

On 8/20/2020 1:49 PM, Xuelei Fan wrote:
> 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