Re: [cp-patches] RFC: fix for PR 24642
Committed, including the suggestions people gave.
2006-04-14 Casey Marshall [EMAIL PROTECTED]
Fixes PR classpath/24642
* NEWS: add note about SecureRandom changes, and addition of
VMSecureRandom.
* java/security/SecureRandom.java (isSeeded): new field.
(setSeed, setSeed): set `isSeeded' to `true.'
(nextBytes): seed this instance if `isSeeded' is false.
(getSeed): call `generateSeed.'
(SECURERANDOM_SOURCE, JAVA_SECURITY_EGD, logger): new constants.
(generateSeed, generateSeed): new methods.
* vm/reference/java/security/VMSecureRandom.java: new file.
Thanks.
Index: java/security/SecureRandom.java
===
RCS file: /cvsroot/classpath/classpath/java/security/SecureRandom.java,v
retrieving revision 1.21
diff -u -B -b -r1.21 SecureRandom.java
--- java/security/SecureRandom.java 12 Apr 2006 12:50:09 - 1.21
+++ java/security/SecureRandom.java 14 Apr 2006 18:18:12 -
@@ -1,6 +1,10 @@
/* SecureRandom.java --- Secure Random class implementation
+ SecureRandom.java
+ Copyright (C) 1999, 2001, 2002, 2003, 2005, 2006 Free Software Foundation,
Inc.
+===
Copyright (C) 1999, 2001, 2002, 2003, 2005, 2006
Free Software Foundation, Inc.
+ 1.21
This file is part of GNU Classpath.
@@ -38,11 +42,19 @@
package java.security;
+import gnu.classpath.SystemProperties;
import gnu.java.security.Engine;
+import gnu.java.security.action.GetSecurityPropertyAction;
import gnu.java.security.jce.prng.Sha160RandomSpi;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
import java.util.Enumeration;
import java.util.Random;
+import java.util.logging.Level;
+import java.util.logging.Logger;
/**
* An interface to a cryptographically secure pseudo-random number
@@ -74,6 +86,8 @@
byte[] state = null;
private String algorithm;
+ private boolean isSeeded = false;
+
// Constructors.
//
@@ -303,6 +317,7 @@
public void setSeed(byte[] seed)
{
secureRandomSpi.engineSetSeed(seed);
+isSeeded = true;
}
/**
@@ -330,6 +345,7 @@
(byte) (0xff seed)
};
secureRandomSpi.engineSetSeed(tmp);
+isSeeded = true;
}
}
@@ -341,6 +357,8 @@
*/
public void nextBytes(byte[] bytes)
{
+if (!isSeeded)
+ setSeed(getSeed(32));
randomBytesUsed += bytes.length;
counter++;
secureRandomSpi.engineNextBytes(bytes);
@@ -386,10 +404,8 @@
public static byte[] getSeed(int numBytes)
{
byte[] tmp = new byte[numBytes];
-
-new Random().nextBytes(tmp);
+generateSeed(tmp);
return tmp;
-//return secureRandomSpi.engineGenerateSeed( numBytes );
}
/**
@@ -404,4 +420,64 @@
return secureRandomSpi.engineGenerateSeed(numBytes);
}
+ // Seed methods.
+
+ private static final String SECURERANDOM_SOURCE = securerandom.source;
+ private static final String JAVA_SECURITY_EGD = java.security.egd;
+ private static final Logger logger =
Logger.getLogger(SecureRandom.class.getName());
+
+ private static int generateSeed(byte[] buffer)
+ {
+return generateSeed(buffer, 0, buffer.length);
+ }
+
+ private static int generateSeed(byte[] buffer, int offset, int length)
+ {
+URL sourceUrl = null;
+String urlStr = null;
+
+GetSecurityPropertyAction action = new
GetSecurityPropertyAction(SECURERANDOM_SOURCE);
+try
+ {
+urlStr = (String) AccessController.doPrivileged(action);
+if (urlStr != null)
+ sourceUrl = new URL(urlStr);
+ }
+catch (MalformedURLException ignored)
+ {
+logger.log(Level.WARNING, SECURERANDOM_SOURCE + property is
malformed: {0},
+ urlStr);
+ }
+
+if (sourceUrl == null)
+ {
+try
+ {
+urlStr = SystemProperties.getProperty(JAVA_SECURITY_EGD);
+if (urlStr != null)
+ sourceUrl = new URL(urlStr);
+ }
+catch (MalformedURLException mue)
+ {
+logger.log(Level.WARNING, JAVA_SECURITY_EGD + property is
malformed: {0},
+ urlStr);
+ }
+ }
+
+if (sourceUrl != null)
+ {
+try
+ {
+InputStream in = sourceUrl.openStream();
+return in.read(buffer, offset, length);
+ }
+catch (IOException ioe)
+ {
+logger.log(Level.FINE, error reading random bytes, ioe);
+ }
+ }
+
+// If we get here, we did not get any seed from a property URL.
+return VMSecureRandom.generateSeed(buffer, offset, length);
+ }
}
--- /dev/null 2006-04-14 10:49:23.0 -0700
+++ vm/reference/java/security/VMSecureRandom.java 2006-04-13
09:52:13.0 -0700
@@ -0,0 +1,134 @@
Re: [cp-patches] RFC: fix for PR 24642
Casey Marshall wrote: Getting a seed value does the following: 1. Try to read from the URL given in the security property `securerandom.seed.' 2. If that property is not set, or is set to a malformed URL, try to read from the URL given by the system property `java.security.egd.' 3. If neither property is set to a valid URL, or if reading that URL fails, call `VMSecureRandom.generateSeed.' The thing to do on most Unix systems is to point the `securerandom.source' property to `file:/dev/random' or `file:/dev/ urandom.' So there needs to be a NEWS entry for VM implementors that notifies them they should set the securerandom.seed system property at VM startup, right? If so please add one to your patch. Thanks, -Archie __ Archie Cobbs *CTO, Awarix* http://www.awarix.com
Re: [cp-patches] RFC: fix for PR 24642
Hi Casey, On Wed, 2006-04-12 at 22:15 -0700, Casey Marshall wrote: This introduces a new VM class, `java.security.VMSecureRandom,' that contains a single static method for generating real (or close to real) random seed values. The default implementation uses a set of eight dueling threads, each of which increments a counter in a tight loop. These eight counters are XORed together to produce a single random byte. The idea is that thread scheduling decisions are hard to predict, and thus that the values of these eight counters is hard to guess. This isn't generally true, but seems to work OK on some systems. It's also inefficient, but doesn't seem to bad, even on JamVM, since it is used sparingly. Nice idea. But wouldn't it be better to make sure all threads are running before doing the XOR? By setting the running flag in the run() method of the spinners so you can wait or yield the main thread till you are sure they have all ran at least a little? But maybe that is just a little too paranoia. We should at least put a strong advice to replace the default implementation (or provide a real random source like /dev/random) somewhere. Getting a seed value does the following: 1. Try to read from the URL given in the security property `securerandom.seed.' 2. If that property is not set, or is set to a malformed URL, try to read from the URL given by the system property `java.security.egd.' 3. If neither property is set to a valid URL, or if reading that URL fails, call `VMSecureRandom.generateSeed.' The thing to do on most Unix systems is to point the `securerandom.source' property to `file:/dev/random' or `file:/dev/ urandom.' Should that be the default value put in our classpath.security properties file? Does this look OK? Yes, nice. Please make the class final (which is convention for the vm reference class implementations). And it would be best (to avoid needing extra synthetic accessor methods) to make everything in the spinner class package private. One style point, different from our C coding conventions, we don't put a space after a method invocation. So it is: setSeed(getSeed(32)); Could you add a NEWS entry under the Runtime interface changes: for this? Thanks, Mark signature.asc Description: This is a digitally signed message part
Re: [cp-patches] RFC: fix for PR 24642
On Apr 13, 2006, at 6:39 AM, Archie Cobbs wrote: Casey Marshall wrote: Getting a seed value does the following: 1. Try to read from the URL given in the security property `securerandom.seed.' 2. If that property is not set, or is set to a malformed URL, try to read from the URL given by the system property `java.security.egd.' 3. If neither property is set to a valid URL, or if reading that URL fails, call `VMSecureRandom.generateSeed.' The thing to do on most Unix systems is to point the `securerandom.source' property to `file:/dev/random' or `file:/ dev/ urandom.' So there needs to be a NEWS entry for VM implementors that notifies them they should set the securerandom.seed system property at VM startup, right? If so please add one to your patch. Actually, we should detect if the host system has /dev/urandom or / dev/random, and set the default `securerandom.source' to that file in classpath.security.
Re: [cp-patches] RFC: fix for PR 24642
On Apr 12, 2006, at 11:58 PM, Jeroen Frijters wrote: I like this approach (as a default). +private byte value; +private boolean running; For it to work consistently on all VMs both the value and running field in Spinner must be marked volatile. Good catch. I'll do that. On Apr 13, 2006, at 7:46 AM, Mark Wielaard wrote: Hi Casey, On Wed, 2006-04-12 at 22:15 -0700, Casey Marshall wrote: This introduces a new VM class, `java.security.VMSecureRandom,' that contains a single static method for generating real (or close to real) random seed values. The default implementation uses a set of eight dueling threads, each of which increments a counter in a tight loop. These eight counters are XORed together to produce a single random byte. The idea is that thread scheduling decisions are hard to predict, and thus that the values of these eight counters is hard to guess. This isn't generally true, but seems to work OK on some systems. It's also inefficient, but doesn't seem to bad, even on JamVM, since it is used sparingly. Nice idea. But wouldn't it be better to make sure all threads are running before doing the XOR? By setting the running flag in the run() method of the spinners so you can wait or yield the main thread till you are sure they have all ran at least a little? But maybe that is just a little too paranoia. We should at least put a strong advice to replace the default implementation (or provide a real random source like /dev/random) somewhere. Yes, good points. Getting a seed value does the following: 1. Try to read from the URL given in the security property `securerandom.seed.' 2. If that property is not set, or is set to a malformed URL, try to read from the URL given by the system property `java.security.egd.' 3. If neither property is set to a valid URL, or if reading that URL fails, call `VMSecureRandom.generateSeed.' The thing to do on most Unix systems is to point the `securerandom.source' property to `file:/dev/random' or `file:/dev/ urandom.' Should that be the default value put in our classpath.security properties file? Yeah. We should check for /dev/u?random in configure, and add that to the properties file. Does this look OK? Yes, nice. Please make the class final (which is convention for the vm reference class implementations). And it would be best (to avoid needing extra synthetic accessor methods) to make everything in the spinner class package private. One style point, different from our C coding conventions, we don't put a space after a method invocation. So it is: setSeed(getSeed(32)); OK. I admit that I don't agree with that style convention, and that I have a strong habit for typing a space before parentheses. Could you add a NEWS entry under the Runtime interface changes: for this? OK. Thanks.
[cp-patches] RFC: fix for PR 24642
This patch implements seeding of all SecureRandom instances if you
call `nextBytes' without providing a seed yourself, and provides a
better implementation of the static `getSeed' method.
This introduces a new VM class, `java.security.VMSecureRandom,' that
contains a single static method for generating real (or close to
real) random seed values. The default implementation uses a set of
eight dueling threads, each of which increments a counter in a
tight loop. These eight counters are XORed together to produce a
single random byte. The idea is that thread scheduling decisions are
hard to predict, and thus that the values of these eight counters is
hard to guess. This isn't generally true, but seems to work OK on
some systems. It's also inefficient, but doesn't seem to bad, even on
JamVM, since it is used sparingly.
Getting a seed value does the following:
1. Try to read from the URL given in the security property
`securerandom.seed.'
2. If that property is not set, or is set to a malformed URL, try
to read from the URL given by the system property `java.security.egd.'
3. If neither property is set to a valid URL, or if reading that
URL fails, call `VMSecureRandom.generateSeed.'
The thing to do on most Unix systems is to point the
`securerandom.source' property to `file:/dev/random' or `file:/dev/
urandom.'
Does this look OK?
Index: java/security/SecureRandom.java
===
RCS file: /cvsroot/classpath/classpath/java/security/SecureRandom.java,v
retrieving revision 1.20
diff -u -b -B -r1.20 SecureRandom.java
--- java/security/SecureRandom.java 26 Feb 2006 04:49:18 - 1.20
+++ java/security/SecureRandom.java 13 Apr 2006 04:59:03 -
@@ -1,5 +1,5 @@
/* SecureRandom.java --- Secure Random class implementation
- Copyright (C) 1999, 2001, 2002, 2003, 2005 Free Software Foundation, Inc.
+ Copyright (C) 1999, 2001, 2002, 2003, 2005, 2006 Free Software Foundation,
Inc.
This file is part of GNU Classpath.
@@ -37,11 +37,19 @@
package java.security;
+import gnu.classpath.SystemProperties;
import gnu.java.security.Engine;
+import gnu.java.security.action.GetSecurityPropertyAction;
import gnu.java.security.jce.prng.Sha160RandomSpi;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.MalformedURLException;
+import java.net.URL;
import java.util.Enumeration;
import java.util.Random;
+import java.util.logging.Level;
+import java.util.logging.Logger;
/**
* An interface to a cryptographically secure pseudo-random number
@@ -72,6 +80,8 @@
SecureRandomSpi secureRandomSpi = null;
byte[] state = null;
+ private boolean isSeeded = false;
+
// Constructors.
//
@@ -277,6 +287,7 @@
public void setSeed(byte[] seed)
{
secureRandomSpi.engineSetSeed(seed);
+isSeeded = true;
}
/**
@@ -304,6 +315,7 @@
(byte) (0xff seed)
};
secureRandomSpi.engineSetSeed(tmp);
+isSeeded = true;
}
}
@@ -315,6 +327,10 @@
*/
public void nextBytes(byte[] bytes)
{
+if (!isSeeded)
+ {
+setSeed (getSeed (32));
+ }
randomBytesUsed += bytes.length;
counter++;
secureRandomSpi.engineNextBytes(bytes);
@@ -360,10 +376,8 @@
public static byte[] getSeed(int numBytes)
{
byte[] tmp = new byte[numBytes];
-
-new Random().nextBytes(tmp);
+generateSeed (tmp);
return tmp;
-//return secureRandomSpi.engineGenerateSeed( numBytes );
}
/**
@@ -378,4 +392,64 @@
return secureRandomSpi.engineGenerateSeed(numBytes);
}
+ // Seed methods.
+
+ private static final String SECURERANDOM_SOURCE = securerandom.source;
+ private static final String JAVA_SECURITY_EGD = java.security.egd;
+ private static final Logger logger = Logger.getLogger
(SecureRandom.class.getName ());
+
+ private static int generateSeed (byte[] buffer)
+ {
+return generateSeed (buffer, 0, buffer.length);
+ }
+
+ private static int generateSeed (byte[] buffer, int offset, int length)
+ {
+URL sourceUrl = null;
+String urlStr = null;
+
+GetSecurityPropertyAction action = new GetSecurityPropertyAction
(SECURERANDOM_SOURCE);
+try
+ {
+urlStr = (String) AccessController.doPrivileged (action);
+if (urlStr != null)
+ sourceUrl = new URL (urlStr);
+ }
+catch (MalformedURLException ignored)
+ {
+logger.log (Level.WARNING, SECURERANDOM_SOURCE + property is
malformed: {0},
+urlStr);
+ }
+
+if (sourceUrl == null)
+ {
+try
+ {
+urlStr = SystemProperties.getProperty (JAVA_SECURITY_EGD);
+if (urlStr != null)
+ sourceUrl = new URL (urlStr);
+ }
+catch (MalformedURLException mue)
+ {
+logger.log
