RFR 8066397 Remove network-related seed initialization code in ThreadLocal/SplittableRandom
Peter Levart
peter.levart at gmail.com
Wed Dec 3 18:42:08 UTC 2014
Hi Roger,
Thanks for looking at my baby steps in the JNI / Windows worlds...
Answers inline...
On 12/03/2014 03:14 PM, roger riggs wrote:
> Hi Peter,
>
> A few questions and comments:
>
> - Can/should this function be fit into one of the existing classes?
As a static method perhaps, yes. The webrev.02 uses this approach.
>
> - Is more than one instance needed?
> Seed material seems to be needed only as a one-shot so a simpler
> implementation that opens,
> uses and closes would leave fewer leftovers (and not introduce a
> new finalizer use).
If it is to be used only for seeding cryptographicaly insecure PRNGs
then this would be simpler, yes. I have one concern though. If the
"cryptographically secure" promise of the function is started to be
exploited for seeding cryptographically secure PRNGs with less secure
backup plans, then an attacker could force using the backup plan by
exhausting the resources which would make the function fail (in UNIX
variant for example, the attacker could just open max. # of allowed
files). The open/use/eventually-close variant of the API can be used in
a setting where the resource is pre-allocated at the startup of the JVM
(the file is opened), so the use part can happen anytime later without
the worry to be prevented by an attacker.
>
> - The Windows native code could be simpler if the hCryptProv
> was returned from the init function and passed as an argument where
> needed in getBytes and close .
It occurred to me, yes. No need for staticInit0() then. Native methods
can all be static too...
>
> - The static checking tool we use will complain about JNI functions
> that may
> throw exceptions if those exceptions are not checked for.
> For example, SystemRandomImpl_md.c:80.
> The macros in jni_util.h like CHECK_NULL, CHECK_NULL_RETURN can be
> used.
I'll take a look at how these are used elsewhere. Thanks for comments.
Regards, Peter
>
> Thanks, Roger
>
>
> On 12/2/2014 11:42 AM, Peter Levart wrote:
>> On 12/02/2014 11:02 AM, Paul Sandoz wrote:
>>> Hi,
>>>
>>> Please find below a patch to remove the networking code computing a
>>> seed in ThreadLocal/SplittableRandom.
>>>
>>> We thought it a good idea at the time :-) but subsequently on
>>> certain platforms this results in very high initalization costs that
>>> can propagate to other classes such as ConcurrentSkipList*.
>>>
>>> The short-term solution is to remove this code and fallback just
>>> using current system time. This needs to be back-ported to 8u40.
>>>
>>> A longer term solution is to provide a simple public API to get
>>> access to some seed bytes that is optimal for the underlying
>>> platform, for example, based on Peter's investigations. For linux
>>> /dev/urandom is sufficient as a source of bytes. The main problem
>>> seems to be Windows. It would also be nice to back-port to say 8u60
>>> using a private API and update TLR/SR.
>>
>> Hi,
>>
>> Here's a proof of concept for an API that just delegates to
>> system-provided "cryptographically secure" (as declared by the
>> system(s)) pseudo random number generator:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/SystemRandom/webrev.01/
>>
>> On UNIX-es this uses /dev/urandom (which is non-blocking and uses
>> system entropy at least for it's seed):
>>
>> http://en.wikipedia.org/?title=/dev/random
>>
>> On Windows it uses MS Crypto API's function CryptGenRandom (the JNI
>> code is ripped from the sun.security.provider.NativeSeedGenerator),
>> which also seeds from various system sources of entropy:
>>
>> http://en.wikipedia.org/wiki/CryptGenRandom
>>
>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa379942%28v=vs.85%29.aspx
>>
>>
>> The initialization overhead is low on UNIX (the enclosed test run on
>> 64 bit Fedora 20, i7 PC):
>>
>> SystemRandomTest... (8 bytes / invocation)
>> 1st invocation: 112315 ns, result: [25, 61, -12, -106, 75, -7, -73, -55]
>> Following 1000000 invocations: 0.636644474 s, (636 ns/invocation)
>>
>> The same test run on 32 bit Windows 7 (as VirtualBox guest on the
>> same machine):
>>
>> SystemRandomTest... (8 bytes / invocation)
>> 1st invocation: 4880788 ns, result: [-32, 53, -31, 62, 51, 83, 9, -5]
>> Following 1000000 invocations: 1.761087512 s, (1761 ns/invocation)
>>
>> I think the initialization on Windows has an initial latency of
>> approx 5ms because it has to initialize the whole MS Crypto API with
>> it's providers. But CryptGenRandom, which is part of this API,
>> actually delegates it's work to RtlGenRandom function:
>>
>> http://msdn.microsoft.com/en-us/library/windows/desktop/aa387694%28v=vs.85%29.aspx
>>
>>
>> ...which might have lower initialization costs. Unfortunately, the
>> wording in the Microsoft document states that it might be removed in
>> the future. Perhaps we could try to use it and fallback to
>> CryptGenRandom if it is not available...
>>
>>
>> Regards, Peter
>>
>>
>>>
>>> Paul.
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8060435
>>>
>>> diff -r 1b599b4755bd
>>> src/java.base/share/classes/java/util/SplittableRandom.java
>>> --- a/src/java.base/share/classes/java/util/SplittableRandom.java
>>> Mon Dec 01 17:59:39 2014 -0800
>>> +++ b/src/java.base/share/classes/java/util/SplittableRandom.java
>>> Tue Dec 02 10:53:47 2014 +0100
>>> @@ -237,34 +237,7 @@
>>> s = (s << 8) | ((long)(seedBytes[i]) & 0xffL);
>>> return s;
>>> }
>>> - long h = 0L;
>>> - try {
>>> - Enumeration<NetworkInterface> ifcs =
>>> - NetworkInterface.getNetworkInterfaces();
>>> - boolean retry = false; // retry once if
>>> getHardwareAddress is null
>>> - while (ifcs.hasMoreElements()) {
>>> - NetworkInterface ifc = ifcs.nextElement();
>>> - if (!ifc.isVirtual()) { // skip fake addresses
>>> - byte[] bs = ifc.getHardwareAddress();
>>> - if (bs != null) {
>>> - int n = bs.length;
>>> - int m = Math.min(n >>> 1, 4);
>>> - for (int i = 0; i < m; ++i)
>>> - h = (h << 16) ^ (bs[i] << 8) ^ bs[n-1-i];
>>> - if (m < 4)
>>> - h = (h << 8) ^ bs[n-1-m];
>>> - h = mix64(h);
>>> - break;
>>> - }
>>> - else if (!retry)
>>> - retry = true;
>>> - else
>>> - break;
>>> - }
>>> - }
>>> - } catch (Exception ignore) {
>>> - }
>>> - return (h ^ mix64(System.currentTimeMillis()) ^
>>> + return (mix64(System.currentTimeMillis()) ^
>>> mix64(System.nanoTime()));
>>> }
>>> diff -r 1b599b4755bd
>>> src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java
>>> ---
>>> a/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java
>>> Mon Dec 01 17:59:39 2014 -0800
>>> +++
>>> b/src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java
>>> Tue Dec 02 10:53:47 2014 +0100
>>> @@ -147,34 +147,7 @@
>>> s = (s << 8) | ((long)(seedBytes[i]) & 0xffL);
>>> return s;
>>> }
>>> - long h = 0L;
>>> - try {
>>> - Enumeration<NetworkInterface> ifcs =
>>> - NetworkInterface.getNetworkInterfaces();
>>> - boolean retry = false; // retry once if
>>> getHardwareAddress is null
>>> - while (ifcs.hasMoreElements()) {
>>> - NetworkInterface ifc = ifcs.nextElement();
>>> - if (!ifc.isVirtual()) { // skip fake addresses
>>> - byte[] bs = ifc.getHardwareAddress();
>>> - if (bs != null) {
>>> - int n = bs.length;
>>> - int m = Math.min(n >>> 1, 4);
>>> - for (int i = 0; i < m; ++i)
>>> - h = (h << 16) ^ (bs[i] << 8) ^ bs[n-1-i];
>>> - if (m < 4)
>>> - h = (h << 8) ^ bs[n-1-m];
>>> - h = mix64(h);
>>> - break;
>>> - }
>>> - else if (!retry)
>>> - retry = true;
>>> - else
>>> - break;
>>> - }
>>> - }
>>> - } catch (Exception ignore) {
>>> - }
>>> - return (h ^ mix64(System.currentTimeMillis()) ^
>>> + return (mix64(System.currentTimeMillis()) ^
>>> mix64(System.nanoTime()));
>>> }
>>
>
More information about the core-libs-dev
mailing list