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
