Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Oleksandr Otenko]

Can someone summarize what happened?

SecureRandom used to get entropy from /dev/random, which is configurable 
through a policy file to /dev/urandom. Has this changed?


Alex

On 12/07/2014 00:33, Martin Buchholz wrote:
Thanks to Peter for digging into the secure seed generator classes and 
coming up with a patch.  Openjdk security folks, please review.  I 
confess to getting lost whenever I try to orient myself in the twisty 
maze of seed generator implementation files.


Anyways, it seems important to have prngs like ThreadLocalRandom be 
able to get a few bits of seed entropy without loading hundreds of 
classes and without occupying any file descriptors permanently. 
 Perhaps at Google we will go back to writing some simple non-portable 
startup code to read /dev/urandom until openjdk security team comes up 
with a more principled solution (but one that doesn't drag in too much 
machinery).



___
Concurrency-interest mailing list
concurrency-inter...@cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Martin Buchholz]

I support Peter at al's plan to add an API that ThreadLocalRandom et al can
use to get some system entropy without unbounded class dependency loading.

It should not surprise anyone that at Google, we are most interested in
running on Linux, so while we're waiting for a proper fix to happen we are
locally applying a simpler patch that tries explicitly /dev/urandom,
falling back to SecureRandom if necessary:

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ThreadLocalRandom-system-entropy/


On Mon, Jul 14, 2014 at 1:54 PM, Bernd  wrote:

> SecureRandom is unfortunatelly pretty  complex. It is interpreting the
> seed url in some way (the configuration you mentioned behave very special
> since Java 6) , it is mixing seed and continues data and it reorders the
> implementations used.
>
> JEP 123 intended to clear things, but getInstanceStrong() (which nobody
> uses?!) did not improve things IMHO.
>
> Bernd
>
> PS: I think the webrev changed since then, but the mail from Brad
> describes the problem well:
> http://mail.openjdk.java.net/pipermail/security-dev/2013-January/006288.html
> Am 14.07.2014 21:05 schrieb "Oleksandr Otenko" <
> oleksandr.ote...@oracle.com>:
>
>  Can someone summarize what happened?
>>
>> SecureRandom used to get entropy from /dev/random, which is configurable
>> through a policy file to /dev/urandom. Has this changed?
>>
>> Alex
>>
>> On 12/07/2014 00:33, Martin Buchholz wrote:
>>
>>  Thanks to Peter for digging into the secure seed generator classes and
>> coming up with a patch.  Openjdk security folks, please review.  I confess
>> to getting lost whenever I try to orient myself in the twisty maze of seed
>> generator implementation files.
>>
>>  Anyways, it seems important to have prngs like ThreadLocalRandom be
>> able to get a few bits of seed entropy without loading hundreds of classes
>> and without occupying any file descriptors permanently.  Perhaps at Google
>> we will go back to writing some simple non-portable startup code to read
>> /dev/urandom until openjdk security team comes up with a more principled
>> solution (but one that doesn't drag in too much machinery).
>>
>>
>> ___
>> Concurrency-interest mailing 
>> listConcurrency-interest@cs.oswego.eduhttp://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>
>>
>>

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Bernd]

SecureRandom is unfortunatelly pretty  complex. It is interpreting the seed
url in some way (the configuration you mentioned behave very special since
Java 6) , it is mixing seed and continues data and it reorders the
implementations used.

JEP 123 intended to clear things, but getInstanceStrong() (which nobody
uses?!) did not improve things IMHO.

Bernd

PS: I think the webrev changed since then, but the mail from Brad describes
the problem well:
http://mail.openjdk.java.net/pipermail/security-dev/2013-January/006288.html
Am 14.07.2014 21:05 schrieb "Oleksandr Otenko" :

>  Can someone summarize what happened?
>
> SecureRandom used to get entropy from /dev/random, which is configurable
> through a policy file to /dev/urandom. Has this changed?
>
> Alex
>
> On 12/07/2014 00:33, Martin Buchholz wrote:
>
>  Thanks to Peter for digging into the secure seed generator classes and
> coming up with a patch.  Openjdk security folks, please review.  I confess
> to getting lost whenever I try to orient myself in the twisty maze of seed
> generator implementation files.
>
>  Anyways, it seems important to have prngs like ThreadLocalRandom be able
> to get a few bits of seed entropy without loading hundreds of classes and
> without occupying any file descriptors permanently.  Perhaps at Google we
> will go back to writing some simple non-portable startup code to read
> /dev/urandom until openjdk security team comes up with a more principled
> solution (but one that doesn't drag in too much machinery).
>
>
> ___
> Concurrency-interest mailing 
> listConcurrency-interest@cs.oswego.eduhttp://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>
>

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

Hi Sean, Alex

Here's a sum-up post:

http://mail.openjdk.java.net/pipermail/security-dev/2014-June/010700.html

Regards, Peter


On 07/14/2014 04:44 PM, Sean Mullan wrote:
I don't see a pointer to the webrev/patch -- did you forget to include 
it?


--Sean

On 07/11/2014 07:33 PM, Martin Buchholz wrote:

Thanks to Peter for digging into the secure seed generator classes and
coming up with a patch.  Openjdk security folks, please review. I 
confess
to getting lost whenever I try to orient myself in the twisty maze of 
seed

generator implementation files.

Anyways, it seems important to have prngs like ThreadLocalRandom be 
able to

get a few bits of seed entropy without loading hundreds of classes and
without occupying any file descriptors permanently.  Perhaps at 
Google we

will go back to writing some simple non-portable startup code to read
/dev/urandom until openjdk security team comes up with a more principled
solution (but one that doesn't drag in too much machinery).

Re: ThreadLocalRandom clinit troubles

[Sean Mullan]

I don't see a pointer to the webrev/patch -- did you forget to include it?

--Sean

On 07/11/2014 07:33 PM, Martin Buchholz wrote:

Thanks to Peter for digging into the secure seed generator classes and
coming up with a patch.  Openjdk security folks, please review.  I confess
to getting lost whenever I try to orient myself in the twisty maze of seed
generator implementation files.

Anyways, it seems important to have prngs like ThreadLocalRandom be able to
get a few bits of seed entropy without loading hundreds of classes and
without occupying any file descriptors permanently.  Perhaps at Google we
will go back to writing some simple non-portable startup code to read
/dev/urandom until openjdk security team comes up with a more principled
solution (but one that doesn't drag in too much machinery).

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Stanimir Simeonoff]

Hi Peter,
Irrc, ServiceLoader.load(NameServiceDescriptor.class) uses
Thread.contextClassLoader to load the services. Depending how late the
NameServices is getting initialized, perhaps it can be used to circumvent
the loader available at clinit of InetAddress.

I am not sure it could be a real security concern (as the caller has to be
authorized to create custom classloaders), yet the behavior is not exactly
the same. Storing Thread.currentThread().getContextClassLoader() during
clinit in a WeakRefernce (and clearing on use) should be closest to the
original code.

Cheers,
Stanimir



On Fri, Jun 20, 2014 at 2:20 PM, Peter Levart 
wrote:

> On 06/20/2014 11:10 AM, Peter Levart wrote:
>
>> Perhaps a more lazy initialization of NetworkInterface class that does
>> not trigger initialization of NS providers could help. We just need to
>> invoke two methods on NetworkInterface:
>>
>> - static NetworkInterface.getNetworkInterfaces()
>> - instance NetworkInterface.getHardwareAddress()
>>
>> both of which could provide the result without the need of NS providers,
>> I think.
>>
>> This would solve the most general case of using TLR. The case that
>> doesn't involve SecureRandom's help which I think is rarely needed and not
>> default.
>>
>>
>> Regards, Peter
>>
>
> Hi,
>
> A patch that solves this is a patch to InetAddress. We can't suppress
> initialization of InetAddress as part of NetworkInterface initialization,
> but we can suppress initialization of NameService providers as part of
> InetAddress initialization by moving them into a nested class called
> NameServices:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.
> NameServices/webrev.01/
>
> This should solve Martin's issue, but the patch to TLR initialization
> could be applied nevertheless, since it might help overcome possible issues
> when using SecureRandom for TLR's seed.
>
> Regards, Peter
>
>
> ___
> Concurrency-interest mailing list
> concurrency-inter...@cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Stanimir Simeonoff]

I wonder why just don't use the /dev/random if available on *nix -
implemented by sun.security.provider.NativePRNG or
sun.security.mscapi.PRNG() on Windows that calls CryptGenRandom.
Both support SecureRandomSpi.engineGenerateSeed(int) that provides an
arbitrary amount of entropy.
Although the approach would cause some more classes to load, no arbitrary
providers should be initialized.

Stanimir



On Thu, Jun 19, 2014 at 7:25 AM, Martin Buchholz 
wrote:

> ThreadLocalRandom's clinit method creates an intermediate broken state of
> ThreadLocalRandom and then proceeds to run some networking code to get some
> more machine-specific entropy in initialSeed().  This will fail if the
> networking code ever recursively uses a (not yet functional)
> ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
> be run,
>
> at
> java.util.ServiceLoader$LazyIterator.hasNextService(ServiceLoader.java:354)
> at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
>  at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
> at java.net.InetAddress$3.run(InetAddress.java:923)
>  at java.net.InetAddress$3.run(InetAddress.java:918)
> at java.security.AccessController.doPrivileged(Native Method)
>  at java.net.InetAddress.createNSProvider(InetAddress.java:917)
> at java.net.InetAddress.(InetAddress.java:962)
>
> if the sun.net.spi.nameservice.provider system property is defined.
>
> The current strategy of ThreadLocalRandom relying on other java code for
> initialization seems risky.  Safer would be to have native code provide
> some entropy at program startup for use by ThreadLocalRandom.  I don't have
> a clean solution for this problem (other than to rip out initialSeed()).
>  Strictly more reliable would be to mix in the entropy from the system at
> the end of ThreadLocalRandom's clinit instead of the beginning, but the
> basic problem remains.
>
> ___
> Concurrency-interest mailing list
> concurrency-inter...@cs.oswego.edu
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>

Re: ThreadLocalRandom clinit troubles

[Bradford Wetmore]

On 6/26/2014 4:10 PM, Doug Lea wrote:

This seems to be the best idea yet, assuming that people are OK
with the changes to sun.security.provider.SeedGenerator and
NativeSeedGenerator.java


I've been meaning to review this thread, but have been chasing several 
urgent escalations.


Brad

Re: ThreadLocalRandom clinit troubles

[Doug Lea]

Peter: Thanks very much for attacking the shocking impact/complexity
of getting a few seed bits.

On 06/25/2014 01:41 PM, Peter Levart wrote:


Peeking around in the sun.security.provider package, I found there already is a
minimal internal infrastructure for obtaining random seed. It's encapsulated in
package-private abstract class sun.security.provider.SeedGenerator with 4
implementations. It turns out that, besides Java-only fall-back implementation
called ThreadedSeedGenerator and generic URLSeedGenerator, there are also two
implementations of NativeSeedGenerator (one for UNIX-es which is just an
extension of URLSeedGenerator and the other for Windows which uses MS
CryptoAPI). I made a few tweaks that allow this sub-infrastructure to be used
directly in ThreadLocalRandom and SplittableRandom:

http://cr.openjdk.java.net/~plevart/jdk9-dev/TLR_SR_SeedGenerator/webrev.01/



This seems to be the best idea yet, assuming that people are OK
with the changes to sun.security.provider.SeedGenerator and
NativeSeedGenerator.java

-Doug

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

To sum-up: We have a problem with TLR initialization since by default it 
uses networking code to compute initial "seeder" value which can execute 
user code in at least two situations:


- when "sun.net.spi.nameservice.provider" system property is defined to 
use custom NameService provider

- when custom SecurityManager is in effect while TLR is being initialized
- also, when "java.util.secureRandomSeed" is defined, at least on 
Windows this means that default SecureRandom algorithm will be using 
networking code too to gather system entropy


We could work-around these problems by delaying initialization of 
NameService provider(s), by-passing SecurityManager when obtaining MAC 
address from NetworkInterface and extending the semantics of 
"java.util.secureRandomSeed" property to specify explicit SecureRandom 
algorithm and provider to use when obtaining SecureRandom instance, like 
in the following patch:


http://cr.openjdk.java.net/~plevart/jdk9-dev/TLR.initialSeed/webrev.01/

But on the other hand this seems too many knobs to worry about. Ideally 
one would like to always use OS provided native seed source, but 
SecureRandom (with all the security providers infrastructure) seems too 
heavy-weight to be used in classes like ThreadLocalRandom or 
SplittableRandom by default since they can be initialized very early in 
the start-up sequence. I made an experiment with class-loading. Recent 
JDK9 build loads 381 classes when running the following empty program on 
Linux:


public class test0 {
public static void main(String[] args) {
}
}

...ThreadLocalRandom is not among them. But in special configurations 
(like when using java agents) or in the future, it could be. The 
following program:


public class test {
public static void main(String[] args) {
java.util.concurrent.ThreadLocalRandom.current();
}
}

...loads 403 classes. That's 22 more than an empty program. The 
following classes are loaded in addition:


+ java.util.Random
+ java.util.concurrent.ThreadLocalRandom
+ java.net.NetworkInterface
+ java.net.NetworkInterface$1
+ java.net.InterfaceAddress
+ java.net.InetAddress
+ sun.security.action.GetBooleanAction
+ java.net.InetAddress$1
+ java.net.InetAddress$InetAddressHolder
+ java.net.InetAddress$Cache
+ java.net.InetAddress$Cache$Type
+ java.net.InetAddressImplFactory
+ java.net.InetAddressImpl
+ java.net.Inet6AddressImpl
+ sun.net.spi.nameservice.NameService
+ java.net.InetAddress$2
+ java.net.Inet4Address
+ java.net.Inet6Address
+ java.net.Inet6Address$Inet6AddressHolder
+ java.net.DefaultInterface
+ java.net.NetworkInterface$2
+ sun.nio.ch.Interruptible

If I run the same program but set the "java.util.secureRandomSeed=true", 
it loads 435 classes. Besides 381 classes loaded by an empty program, 
the following 54 classes are loaded in addition:


+ java.util.Random
+ java.util.concurrent.ThreadLocalRandom
+ java.security.SecureRandom
+ sun.security.jca.Providers
+ java.lang.InheritableThreadLocal
+ sun.security.jca.ProviderList
+ sun.security.jca.ProviderConfig
+ java.security.Provider
+ sun.security.jca.ProviderList$3
+ sun.security.jca.ProviderList$1
+ java.security.Provider$ServiceKey
+ java.security.Provider$EngineDescription
+ sun.misc.FloatingDecimal
+ sun.misc.FloatingDecimal$BinaryToASCIIConverter
+ sun.misc.FloatingDecimal$ExceptionalBinaryToASCIIBuffer
+ sun.misc.FloatingDecimal$BinaryToASCIIBuffer
+ sun.misc.FloatingDecimal$1
+ sun.misc.FloatingDecimal$ASCIIToBinaryConverter
+ sun.misc.FloatingDecimal$PreparedASCIIToBinaryBuffer
+ sun.misc.FDBigInteger
+ sun.security.jca.ProviderList$2
+ java.security.Security
+ java.security.Security$1
+ java.util.Properties$LineReader
+ java.util.AbstractList$Itr
+ sun.security.jca.ProviderConfig$2
+ sun.security.provider.Sun
+ sun.security.provider.SunEntries
+ sun.security.provider.SunEntries$1
+ java.security.SecureRandomSpi
+ sun.security.provider.NativePRNG
+ sun.security.provider.NativePRNG$Variant
+ sun.security.provider.NativePRNG$1
+ sun.security.provider.NativePRNG$2
+ java.net.URI
+ java.net.URI$Parser
+ sun.security.provider.NativePRNG$RandomIO
+ sun.security.provider.NativePRNG$Blocking
+ sun.security.provider.NativePRNG$NonBlocking
+ java.util.LinkedHashMap$LinkedEntrySet
+ java.util.LinkedHashMap$LinkedHashIterator
+ java.util.LinkedHashMap$LinkedEntryIterator
+ java.security.Provider$Service
+ java.security.Provider$UString
+ java.util.LinkedHashSet
+ java.util.LinkedHashMap$LinkedValues
+ java.util.LinkedHashMap$LinkedValueIterator
+ java.util.Collections$UnmodifiableSet
+ java.util.Collections$UnmodifiableCollection$1
+ java.util.LinkedHashMap$LinkedKeySet
+ java.util.LinkedHashMap$LinkedKeyIterator
+ sun.security.jca.GetInstance
+ sun.security.jca.GetInstance$Instance
+ sun.nio.ch.Interruptible


This seems too heavy-weight even if the initialization issue on Windows 
where default SecureRandom algorithm is using networking code to gather 
system entropy is worked-around by reque

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/24/2014 06:01 PM, Martin Buchholz wrote:




On Tue, Jun 24, 2014 at 7:03 AM, Peter Levart > wrote:



I would rather use SecureRandom.generateSeed() instance method
instead of SecureRandom.nextBytes(). Why? Because every
SecureRandom instance has to initialize it's seed 1st before
getBytes() can provide the next random bytes from the PRNG. Since
we only need the 1st 8 bytes from the SecureRandom instance to
initialize TLR's seeder, we might as well directly call the
SecureRandom.generateSeed() method.


If I strace this program on Linux using strace -ff -q java SecureRandoms:

public class SecureRandoms {
public static void main(String[] args) throws Throwable {
byte[] bytes = new byte[8];
new java.security.SecureRandom().nextBytes(bytes);
}
}

I see a read from /dev/urandom, but not from /dev/random, so I 
conclude your intuitive understanding of how the seeding works must be 
wrong.  It makes sense that NativePRNG doesn't need to do any special 
seeding of its own, since it reuses the operating system's.


You're right. I checked again. The  NativePRNG is actually using 
/dev/urandom (by default unless java.security.egd or securerandom.source 
is defined). It's mixing the /dev/urandom stream with the stream 
obtained from SHA1 generator which is seeded by 20 bytes from 
/dev/urandom too. So by default yes, plain NativePRNG (the default on 
UNIX-es) is using /dev/urandom for nextBytes(), but this can be changed 
by defining java.security.egd or securerandom.source system property. I 
still think that for configuration-independent PRNG seed on UNIX-es it's 
better to invoke generateSeed() on NativePRNG$NonBlocking, which 
hard-codes /dev/urandom and doesn't mix it with SHA1 stream.


On Windows, there's a different story, since the default SecureRandom 
algorithm is SHA1, seeded by SeedGenerator.getSystemEntropy() and 
SeedGenerator.generateSeed(). The former call includes invoking 
networking code and resolving local host name. Which we would like to 
avoid. So I think we need a nicer story on windows then just: new 
SecureRandom().nextBytes(). I propose requesting explicit algorithm / 
provider on each particular platform that we know does best what we 
want, rather than using default which can still be used as a fall-back.


Regards, Peter

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

Hi Martin,

On 06/22/2014 07:12 PM, Martin Buchholz wrote:

We know that loading the networking machinery is problematic.  On Linux we
would be content to hard-code a read from /dev/urandom, which is safer and
strictly more random than the existing network hardware determination, but
y'all will reject that as too system-dependent (insufficient machinery!).
H  maybe not  as long as we code up a good fallback ...

I learned that SecureRandom by default on Unix uses /dev/random for "seed
bytes" and /dev/urandom for nextBytes.

Here's my proposal, that in the default case on Unix doesn't load any
machinery, and as a fallback loads the SecureRandom machinery instead of
the network machinery, while maintaining the ultra-secure behavior of the
java.util.secureRandomSeed system property:


 private static long initialSeed() {
 byte[] seedBytes = initialSeedBytes();
 long s = (long)(seedBytes[0]) & 0xffL;
 for (int i = 1; i < seedBytes.length; ++i)
 s = (s << 8) | ((long)(seedBytes[i]) & 0xffL);
 return s ^ mix64(System.currentTimeMillis()) ^
mix64(System.nanoTime());
 }

 private static byte[] initialSeedBytes() {
 String pp = java.security.AccessController.doPrivileged(
 new sun.security.action.GetPropertyAction(
 "java.util.secureRandomSeed"));
 boolean secureRandomSeed = (pp != null &&
pp.equalsIgnoreCase("true"));
 if (secureRandomSeed)
 return java.security.SecureRandom.getSeed(8);
 final byte[] seedBytes = new byte[8];
 File seedSource = new File("/dev/urandom");
 if (seedSource.exists()) {
 try (FileInputStream stream = new FileInputStream(seedSource)) {
 if (stream.read(seedBytes) == 8)
 return seedBytes;
 } catch (IOException ignore) { }
 }
 new java.security.SecureRandom().nextBytes(seedBytes);
 return seedBytes;
 }


So on platforms lacking "/dev/urandom" special file (Windows only?), the 
fall-back would be to use SecureRandom.nextBytes() whatever default 
SecureRandom PRNG is configured to be on such platform. This might be a 
user-supplied SecureRandom PRNG. The user might want it's own default 
SecureRandom but not use it for TLR's seed computation (unless requested 
by "java.util.secureRandomSeed" system property).


I would rather use SecureRandom.generateSeed() instance method instead 
of SecureRandom.nextBytes(). Why? Because every SecureRandom instance 
has to initialize it's seed 1st before getBytes() can provide the next 
random bytes from the PRNG. Since we only need the 1st 8 bytes from the 
SecureRandom instance to initialize TLR's seeder, we might as well 
directly call the SecureRandom.generateSeed() method. That's one reason. 
The other is the peculiar initialization of default SecureRandom 
algorithm on Windows (see below)...


Even if user does not provide it's own default SecureRandom PRNG, there 
are basically two algorithms that are used by default on OpenJDK:


On Solaris/Linux/Mac/AIX:

the "NativePRNG" algorithm from "SUN" provider (implemented by 
sun.security.provider.NativePRNG) which uses /dev/random for getBytes() 
and /dev/urandom (or whatever is configured with "java.security.egd" or 
"securerandom.source" system properties) for generateSeed(), and


On Windows:

the "SHA1PRNG" algorithm from "SUN" provider (implemented by 
sun.security.provider.SecureRandom) which uses SHA1 message digest for 
generating random numbers with seed computed by gathering system entropy...



The most problematic one is the default on Windows platform (the 
platform that does not have the "/dev/urandom" special file and would be 
used as a fall-back by your proposal) - 
sun.security.provider.SecureRandom. This one seeds itself by 
constructing an instance of itself with the result returned from 
SeedGenerator.getSystemEntropy() method. This method, among other 
things, uses networking code to gather system entropy:


...
md.update
(InetAddress.getLocalHost().toString().getBytes());
...

This is problematic since it not only initializes NameService providers 
but also uses them to resolve local host name. This can block for 
several seconds on unusual configurations and was the main motivation to 
replace similar code in TLR with code that uses 
NetworkInterface.getHardwareAddress() instead. Using 
SecureRandom.generateSeed() instead of SecureRandom.getBytes() does not 
invoke SeedGenerator.getSystemEntropy(), but uses just 
SeedGenerator.generateSeed(), which by default on Windows uses 
ThreadedSeedGenerator.getSeedBytes()...


I showed how we could suppress NameService providers initialization 
while still using NetworkInterface.getHardwareAddress() for TLR's seeder 
initialization. If that's not enough and we would like to get-away 
without using networking code at al

Re: ThreadLocalRandom clinit troubles

[Bradford Wetmore]

Martin,

Thanks for filing.  I was positive there was already a bug for this, but 
for the life of me I can't find it now.  There's some other more minor 
cleanup that needs to take place, but seems like I've been in 
escalation/firefighting mode for more than a year now and it hasn't 
bubbled to the top.


Brad


On 6/21/2014 9:05 PM, Martin Buchholz wrote:

While looking at NativePRNG, I filed

https://bugs.openjdk.java.net/browse/JDK-8047769

SecureRandom should be more frugal with file descriptors

If I run this java program on Linux

public class SecureRandoms {
 public static void main(String[] args) throws Throwable {
 new java.security.SecureRandom();
 }
}

it creates 6 file descriptors for /dev/random and /dev/urandom, as shown
by:

strace -q -ff -e open java SecureRandoms |& grep /dev/
[pid 20769] open("/dev/random", O_RDONLY) = 5
[pid 20769] open("/dev/urandom", O_RDONLY) = 6
[pid 20769] open("/dev/random", O_RDONLY) = 7
[pid 20769] open("/dev/random", O_RDONLY) = 8
[pid 20769] open("/dev/urandom", O_RDONLY) = 9
[pid 20769] open("/dev/urandom", O_RDONLY) = 10

Looking at jdk/src/solaris/classes/sun/security/provider/NativePRNG.java
it looks like 2 file descriptors are created for every variant of
NativePRNG, whether or not they are ever used. Which is wasteful. In
fact, you only ever need at most two file descriptors, one for
/dev/random and one for /dev/urandom.

Further, it would be nice if the file descriptors were closed when idle
and lazily re-created. Especially /dev/random should typically be used
at startup and never thereafter.


On Fri, Jun 20, 2014 at 7:59 AM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote:

On 20/06/2014 15:02, Peter Levart wrote:


And, as Martin pointed out, it seems to be used for tests that
exercise particular responses from NameService API to test the
behaviour of JDK classes. It would be a shame for those tests to
go away.

We've been talking about removing it for many years because it has
been so troublesome. If we really need to having something for
testing then I don't think it needs to be general purpose, we can
get right of the lookup at least.

-Alan.

Re: ThreadLocalRandom clinit troubles

[Alan Bateman]

On 20/06/2014 15:02, Peter Levart wrote:


And, as Martin pointed out, it seems to be used for tests that 
exercise particular responses from NameService API to test the 
behaviour of JDK classes. It would be a shame for those tests to go away.
We've been talking about removing it for many years because it has been 
so troublesome. If we really need to having something for testing then I 
don't think it needs to be general purpose, we can get right of the 
lookup at least.


-Alan.

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/20/2014 03:00 PM, Alan Bateman wrote:

On 20/06/2014 12:20, Peter Levart wrote:


Hi,

A patch that solves this is a patch to InetAddress. We can't suppress 
initialization of InetAddress as part of NetworkInterface 
initialization, but we can suppress initialization of NameService 
providers as part of InetAddress initialization by moving them into a 
nested class called NameServices:


http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.NameServices/webrev.01/ 



This should solve Martin's issue, but the patch to TLR initialization 
could be applied nevertheless, since it might help overcome possible 
issues when using SecureRandom for TLR's seed.
That should work. An alternative is to just get rid of this mechanism. 
Way back then it was useful for running on systems that has NIS/YP 
configured as it wasn't always possible get to the fully qualified 
host name. It was also used as a workaround to long timeouts on 
Windows doing NetBIOS lookups. I don't think if either of these cases 
it interesting these days so it might be the simplest thing to just 
get rid of it. The other thing is that it was never meant to be a 
general service provider mechanism, it's not usable outside of the JDK 
without extending JDK-internal classes for example.


-Alan


And, as Martin pointed out, it seems to be used for tests that exercise 
particular responses from NameService API to test the behaviour of JDK 
classes. It would be a shame for those tests to go away.


Regards, Peter

Re: ThreadLocalRandom clinit troubles

[Alan Bateman]

On 20/06/2014 12:20, Peter Levart wrote:


Hi,

A patch that solves this is a patch to InetAddress. We can't suppress 
initialization of InetAddress as part of NetworkInterface 
initialization, but we can suppress initialization of NameService 
providers as part of InetAddress initialization by moving them into a 
nested class called NameServices:


http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.NameServices/webrev.01/ 



This should solve Martin's issue, but the patch to TLR initialization 
could be applied nevertheless, since it might help overcome possible 
issues when using SecureRandom for TLR's seed.
That should work. An alternative is to just get rid of this mechanism. 
Way back then it was useful for running on systems that has NIS/YP 
configured as it wasn't always possible get to the fully qualified host 
name. It was also used as a workaround to long timeouts on Windows doing 
NetBIOS lookups. I don't think if either of these cases it interesting 
these days so it might be the simplest thing to just get rid of it. The 
other thing is that it was never meant to be a general service provider 
mechanism, it's not usable outside of the JDK without extending 
JDK-internal classes for example.


-Alan

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Chris Hegarty]

I'm in favor of Peters approach ( I would need to do a more detailed 
review though ).


Looking up name service providers during initialization of InetAddress 
has been the cause of several issues in the past.


I agree with Stanimir's point about TCCL, but I don't think we should 
try to do anything about it. If some application depends on having a 
particular thread, with a "special" TCCL set, initialize InetAddress, 
then it is just broken.


I would prefer to keep this code as simple as possible, and stashing the 
TCCL for later use just adds complication.


-Chris.

On 20/06/14 13:16, Stanimir Simeonoff wrote:

Hi Peter,
Irrc, ServiceLoader.load(NameServiceDescriptor.class) uses
Thread.contextClassLoader to load the services. Depending how late the
NameServices is getting initialized, perhaps it can be used to
circumvent the loader available at clinit of InetAddress.

I am not sure it could be a real security concern (as the caller has to
be authorized to create custom classloaders), yet the behavior is not
exactly the same. Storing Thread.currentThread().getContextClassLoader()
during clinit in a WeakRefernce (and clearing on use) should be closest
to the original code.

Cheers,
Stanimir



On Fri, Jun 20, 2014 at 2:20 PM, Peter Levart mailto:peter.lev...@gmail.com>> wrote:

On 06/20/2014 11:10 AM, Peter Levart wrote:

Perhaps a more lazy initialization of NetworkInterface class
that does not trigger initialization of NS providers could help.
We just need to invoke two methods on NetworkInterface:

- static NetworkInterface.__getNetworkInterfaces()
- instance NetworkInterface.__getHardwareAddress()

both of which could provide the result without the need of NS
providers, I think.

This would solve the most general case of using TLR. The case
that doesn't involve SecureRandom's help which I think is rarely
needed and not default.


Regards, Peter


Hi,

A patch that solves this is a patch to InetAddress. We can't
suppress initialization of InetAddress as part of NetworkInterface
initialization, but we can suppress initialization of NameService
providers as part of InetAddress initialization by moving them into
a nested class called NameServices:


http://cr.openjdk.java.net/~__plevart/jdk9-dev/InetAddress.__NameServices/webrev.01/



This should solve Martin's issue, but the patch to TLR
initialization could be applied nevertheless, since it might help
overcome possible issues when using SecureRandom for TLR's seed.

Regards, Peter


_
Concurrency-interest mailing list
Concurrency-interest@cs.__oswego.edu

http://cs.oswego.edu/mailman/__listinfo/concurrency-interest





___
Concurrency-interest mailing list
concurrency-inter...@cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/20/2014 11:10 AM, Peter Levart wrote:
Perhaps a more lazy initialization of NetworkInterface class that does 
not trigger initialization of NS providers could help. We just need to 
invoke two methods on NetworkInterface:


- static NetworkInterface.getNetworkInterfaces()
- instance NetworkInterface.getHardwareAddress()

both of which could provide the result without the need of NS 
providers, I think.


This would solve the most general case of using TLR. The case that 
doesn't involve SecureRandom's help which I think is rarely needed and 
not default.



Regards, Peter 


Hi,

A patch that solves this is a patch to InetAddress. We can't suppress 
initialization of InetAddress as part of NetworkInterface 
initialization, but we can suppress initialization of NameService 
providers as part of InetAddress initialization by moving them into a 
nested class called NameServices:


http://cr.openjdk.java.net/~plevart/jdk9-dev/InetAddress.NameServices/webrev.01/

This should solve Martin's issue, but the patch to TLR initialization 
could be applied nevertheless, since it might help overcome possible 
issues when using SecureRandom for TLR's seed.


Regards, Peter

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/19/2014 08:04 PM, Martin Buchholz wrote:

On Thu, Jun 19, 2014 at 1:22 AM, Alan Bateman 
wrote:


On 19/06/2014 05:25, Martin Buchholz wrote:


ThreadLocalRandom's clinit method creates an intermediate broken state of
ThreadLocalRandom and then proceeds to run some networking code to get
some
more machine-specific entropy in initialSeed().  This will fail if the
networking code ever recursively uses a (not yet functional)
ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
be run,

at
java.util.ServiceLoader$LazyIterator.hasNextService(
ServiceLoader.java:354)
at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
at java.net.InetAddress$3.run(InetAddress.java:923)
at java.net.InetAddress$3.run(InetAddress.java:918)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.InetAddress.createNSProvider(InetAddress.java:917)
at java.net.InetAddress.(InetAddress.java:962)

if the sun.net.spi.nameservice.provider system property is defined.

The current strategy of ThreadLocalRandom relying on other java code for
initialization seems risky.


Using a name service provider other than the default is going to interact
badly here. So is this configured to use the JNDI-DNS provider ("dns,sun")
or something else? This provider mechanism has been a source of many
problems, recursive initialization and stack overflow mostly because any
custom provider is likely going to use the network and resolve host names.
It can interact very badly with security when the provider doesn't have
AllPermision because attempts to establish connections involve security
checks that often need to do lookups too.

So I'm curious if there is more to this stack trace to put more context on
the issue.


The way we are actually seeing this is:
- there are jdk8 jtreg tests that set the sun.net.spi.nameservice.provider
property.  Grepping:

./java/net/Inet4Address/textToNumericFormat.java
./java/net/URLPermission/nstest/lookup.sh
./sun/net/InetAddress/nameservice/dns/cname.sh
./sun/net/InetAddress/nameservice/deadlock/Hang.java
./sun/net/InetAddress/nameservice/chaining/Providers.java
./sun/net/InetAddress/nameservice/simple/DefaultCaching.java
./sun/net/InetAddress/nameservice/simple/CacheTest.java
./sun/security/krb5/auto/KDC.java
./sun/security/krb5/canonicalize/Test.java

- we have local modifications to classloading that happen to use TLR
via ConcurrentSkipListMap - a "reasonable" thing to do.  Probably the
simplest way to provoke a failure is to try to use a TLR from e.g.
URLClassPath.

In general, any core library boot class should try hard to avoid
loading/invoking code from the user's classpath.


Hi Martin,

Does my proposed patch solve these issues or does the 
"sun.net.spi.nameservice.provider" triggered changed initialization 
order provoke other failures. I can imagine there might be other issues. 
Perhaps a more lazy initialization of NetworkInterface class that does 
not trigger initialization of NS providers could help. We just need to 
invoke two methods on NetworkInterface:


- static NetworkInterface.getNetworkInterfaces()
- instance NetworkInterface.getHardwareAddress()

both of which could provide the result without the need of NS providers, 
I think.


This would solve the most general case of using TLR. The case that 
doesn't involve SecureRandom's help which I think is rarely needed and 
not default.



Regards, Peter





It may be another example to back a suggestion to just drop the
JDK-internal name service provider mechanism. But in general, I think you
are right, it's not good for TLR initialization to trigger arbitrary code
to execute.

-Alan.

Re: ThreadLocalRandom clinit troubles

[Martin Buchholz]

On Thu, Jun 19, 2014 at 1:22 AM, Alan Bateman 
wrote:

> On 19/06/2014 05:25, Martin Buchholz wrote:
>
>> ThreadLocalRandom's clinit method creates an intermediate broken state of
>> ThreadLocalRandom and then proceeds to run some networking code to get
>> some
>> more machine-specific entropy in initialSeed().  This will fail if the
>> networking code ever recursively uses a (not yet functional)
>> ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
>> be run,
>>
>> at
>> java.util.ServiceLoader$LazyIterator.hasNextService(
>> ServiceLoader.java:354)
>> at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
>> at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
>> at java.net.InetAddress$3.run(InetAddress.java:923)
>> at java.net.InetAddress$3.run(InetAddress.java:918)
>> at java.security.AccessController.doPrivileged(Native Method)
>> at java.net.InetAddress.createNSProvider(InetAddress.java:917)
>> at java.net.InetAddress.(InetAddress.java:962)
>>
>> if the sun.net.spi.nameservice.provider system property is defined.
>>
>> The current strategy of ThreadLocalRandom relying on other java code for
>> initialization seems risky.
>>
> Using a name service provider other than the default is going to interact
> badly here. So is this configured to use the JNDI-DNS provider ("dns,sun")
> or something else? This provider mechanism has been a source of many
> problems, recursive initialization and stack overflow mostly because any
> custom provider is likely going to use the network and resolve host names.
> It can interact very badly with security when the provider doesn't have
> AllPermision because attempts to establish connections involve security
> checks that often need to do lookups too.
>
> So I'm curious if there is more to this stack trace to put more context on
> the issue.


The way we are actually seeing this is:
- there are jdk8 jtreg tests that set the sun.net.spi.nameservice.provider
property.  Grepping:

./java/net/Inet4Address/textToNumericFormat.java
./java/net/URLPermission/nstest/lookup.sh
./sun/net/InetAddress/nameservice/dns/cname.sh
./sun/net/InetAddress/nameservice/deadlock/Hang.java
./sun/net/InetAddress/nameservice/chaining/Providers.java
./sun/net/InetAddress/nameservice/simple/DefaultCaching.java
./sun/net/InetAddress/nameservice/simple/CacheTest.java
./sun/security/krb5/auto/KDC.java
./sun/security/krb5/canonicalize/Test.java

- we have local modifications to classloading that happen to use TLR
via ConcurrentSkipListMap - a "reasonable" thing to do.  Probably the
simplest way to provoke a failure is to try to use a TLR from e.g.
URLClassPath.

In general, any core library boot class should try hard to avoid
loading/invoking code from the user's classpath.


> It may be another example to back a suggestion to just drop the
> JDK-internal name service provider mechanism. But in general, I think you
> are right, it's not good for TLR initialization to trigger arbitrary code
> to execute.
>
> -Alan.
>

Re: ThreadLocalRandom clinit troubles

[Alan Bateman]

On 19/06/2014 05:25, Martin Buchholz wrote:

ThreadLocalRandom's clinit method creates an intermediate broken state of
ThreadLocalRandom and then proceeds to run some networking code to get some
more machine-specific entropy in initialSeed().  This will fail if the
networking code ever recursively uses a (not yet functional)
ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
be run,

at
java.util.ServiceLoader$LazyIterator.hasNextService(ServiceLoader.java:354)
at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
at java.net.InetAddress$3.run(InetAddress.java:923)
at java.net.InetAddress$3.run(InetAddress.java:918)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.InetAddress.createNSProvider(InetAddress.java:917)
at java.net.InetAddress.(InetAddress.java:962)

if the sun.net.spi.nameservice.provider system property is defined.

The current strategy of ThreadLocalRandom relying on other java code for
initialization seems risky.
Using a name service provider other than the default is going to 
interact badly here. So is this configured to use the JNDI-DNS provider 
("dns,sun") or something else? This provider mechanism has been a source 
of many problems, recursive initialization and stack overflow mostly 
because any custom provider is likely going to use the network and 
resolve host names. It can interact very badly with security when the 
provider doesn't have AllPermision because attempts to establish 
connections involve security checks that often need to do lookups too.


So I'm curious if there is more to this stack trace to put more context 
on the issue. It may be another example to back a suggestion to just 
drop the JDK-internal name service provider mechanism. But in general, I 
think you are right, it's not good for TLR initialization to trigger 
arbitrary code to execute.


-Alan.

Re: [concurrency-interest] ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/19/2014 08:32 AM, Stanimir Simeonoff wrote:

I wonder why just don't use the /dev/random if available on *nix -
implemented by sun.security.provider.NativePRNG or
sun.security.mscapi.PRNG() on Windows that calls CryptGenRandom.
Both support SecureRandomSpi.engineGenerateSeed(int) that provides an
arbitrary amount of entropy.
Although the approach would cause some more classes to load, no arbitrary
providers should be initialized.


I think this is waht you get when you set "java.util.secureRandomSeed" 
system property to "true". TLR uses 
java.security.SecureRandom.getSeed(8) in this case.


Regards, Peter



Stanimir



On Thu, Jun 19, 2014 at 7:25 AM, Martin Buchholz 
wrote:


ThreadLocalRandom's clinit method creates an intermediate broken state of
ThreadLocalRandom and then proceeds to run some networking code to get some
more machine-specific entropy in initialSeed().  This will fail if the
networking code ever recursively uses a (not yet functional)
ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
be run,

at
java.util.ServiceLoader$LazyIterator.hasNextService(ServiceLoader.java:354)
at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
  at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
at java.net.InetAddress$3.run(InetAddress.java:923)
  at java.net.InetAddress$3.run(InetAddress.java:918)
at java.security.AccessController.doPrivileged(Native Method)
  at java.net.InetAddress.createNSProvider(InetAddress.java:917)
at java.net.InetAddress.(InetAddress.java:962)

if the sun.net.spi.nameservice.provider system property is defined.

The current strategy of ThreadLocalRandom relying on other java code for
initialization seems risky.  Safer would be to have native code provide
some entropy at program startup for use by ThreadLocalRandom.  I don't have
a clean solution for this problem (other than to rip out initialSeed()).
  Strictly more reliable would be to mix in the entropy from the system at
the end of ThreadLocalRandom's clinit instead of the beginning, but the
basic problem remains.

___
Concurrency-interest mailing list
concurrency-inter...@cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest





___
Concurrency-interest mailing list
concurrency-inter...@cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

Re: ThreadLocalRandom clinit troubles

[Peter Levart]

Hi Martin,

What version of TLR are you looking at? As far as I remmember, the 
InetAddress-related code to obtain initial seed has been replaced by 
NetworkInterface.getHardwareAddress(). Is this still triggering the 
initialization of InetAddress or is this the case of using 
"java.util.secureRandomSeed" set to "true" ? Can you show the whole 
stack trace?


On 06/19/2014 06:25 AM, Martin Buchholz wrote:
> ThreadLocalRandom's clinit method creates an intermediate broken state of
> ThreadLocalRandom and then proceeds to run some networking code to 
get some

> more machine-specific entropy in initialSeed().
>   This will fail if the
> networking code ever recursively uses a (not yet functional)
> ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary 
code to

> be run,
>
> at
> 
java.util.ServiceLoader$LazyIterator.hasNextService(ServiceLoader.java:354)

> at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
> at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
> at java.net.InetAddress$3.run(InetAddress.java:923)
> at java.net.InetAddress$3.run(InetAddress.java:918)
> at java.security.AccessController.doPrivileged(Native Method)
> at java.net.InetAddress.createNSProvider(InetAddress.java:917)
> at java.net.InetAddress.(InetAddress.java:962)
>
> if the sun.net.spi.nameservice.provider system property is defined.
>
> The current strategy of ThreadLocalRandom relying on other java code for
> initialization seems risky.  Safer would be to have native code provide
> some entropy at program startup for use by ThreadLocalRandom. I don't 
have

> a clean solution for this problem (other than to rip out initialSeed()).
>   Strictly more reliable would be to mix in the entropy from the 
system at

> the end of ThreadLocalRandom's clinit instead of the beginning, but the
> basic problem remains.

Would it be acceptable for TLR to be functional even when invoked during 
it's clinit, but using a less randomized "seeder" (based only on current 
time) and after the "networking" or SecureRandom code is finished, 
complete the initialization of "seeder" state and clear the thread-local 
probe so that TLR's thread state is re-initialized afterwards. for example:


http://cr.openjdk.java.net/~plevart/jdk9-dev/TLR.seeder/webrev.01/

Regards, Peter