With this change to createRandom it is not clear to me what the value of the reseeding is when the SecureRandom is not user-supplied. Maybe a crypto expert can comment. It would speed up initialization in the default case if the reseeding was only done for user-defined generators. Alternatively, you could remove it altogether and doc the fact that user-supplied classes are expected to be self-seeding.
Phil On Sat, Nov 27, 2010 at 12:05 PM, <ma...@apache.org> wrote: > Author: markt > Date: Sat Nov 27 17:05:27 2010 > New Revision: 1039707 > > URL: http://svn.apache.org/viewvc?rev=1039707&view=rev > Log: > Make SecureRandom the fall-back and use SecureRandom throughout rather than > Random > > Modified: > tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > tomcat/trunk/webapps/docs/config/manager.xml > > Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1039707&r1=1039706&r2=1039707&view=diff > > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sat Nov > 27 17:05:27 2010 > @@ -41,7 +41,6 @@ import java.util.LinkedList; > import java.util.List; > import java.util.Map; > import java.util.Queue; > -import java.util.Random; > import java.util.concurrent.ConcurrentHashMap; > import java.util.concurrent.ConcurrentLinkedQueue; > import java.util.concurrent.atomic.AtomicLong; > @@ -128,7 +127,8 @@ public abstract class ManagerBase extend > * designed this way since random number generator use a sync to make > them > * thread-safe and the sync makes using a a single object slow(er). > */ > - protected Queue<Random> randoms = new ConcurrentLinkedQueue<Random>(); > + protected Queue<SecureRandom> randoms = > + new ConcurrentLinkedQueue<SecureRandom>(); > > /** > * Random number generator used to see @{link {...@link #randoms}. > @@ -136,9 +136,9 @@ public abstract class ManagerBase extend > protected SecureRandom randomSeed = null; > > /** > - * The Java class name of the random number generator class to be used > - * when generating session identifiers. The random number generator(s) > will > - * always be seeded from a SecureRandom instance. > + * The Java class name of the secure random number generator class to > be > + * used when generating session identifiers. The random number > generator(s) > + * will always be seeded from a SecureRandom instance. > */ > protected String randomClass = "java.security.SecureRandom"; > > @@ -505,23 +505,23 @@ public abstract class ManagerBase extend > * Create a new random number generator instance we should use for > * generating session identifiers. > */ > - protected Random createRandom() { > + protected SecureRandom createRandom() { > if (randomSeed == null) { > createRandomSeed(); > } > > - Random result = null; > + SecureRandom result = null; > > long t1 = System.currentTimeMillis(); > try { > // Construct and seed a new random number generator > Class<?> clazz = Class.forName(randomClass); > - result = (Random) clazz.newInstance(); > + result = (SecureRandom) clazz.newInstance(); > } catch (Exception e) { > - // Fall back to the simple case > + // Fall back to the default case > log.error(sm.getString("managerBase.random", > randomClass), e); > - result = new java.util.Random(); > + result = new java.security.SecureRandom(); > } > byte[] seedBytes = randomSeed.generateSeed(64); > ByteArrayInputStream bais = new ByteArrayInputStream(seedBytes); > @@ -966,7 +966,7 @@ public abstract class ManagerBase extend > } > closeRandomInputStreams(); > } > - Random random = randoms.poll(); > + SecureRandom random = randoms.poll(); > if (random == null) { > random = createRandom(); > } > > Modified: tomcat/trunk/webapps/docs/config/manager.xml > URL: > http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1039707&r1=1039706&r2=1039707&view=diff > > ============================================================================== > --- tomcat/trunk/webapps/docs/config/manager.xml (original) > +++ tomcat/trunk/webapps/docs/config/manager.xml Sat Nov 27 17:05:27 2010 > @@ -134,8 +134,9 @@ > </attribute> > > <attribute name="randomClass" required="false"> > - <p>Java class name of the <code>java.util.Random</code> > - implementation class to use. If not specified, the default value > is > + <p>Name of the Java class that extends > + <code>java.security.SecureRandom</code> to use to generate session > IDs. > + If not specified, the default value is > <code>java.security.SecureRandom</code>.</p> > </attribute> > > @@ -222,8 +223,9 @@ > </attribute> > > <attribute name="randomClass" required="false"> > - <p>Java class name of the <code>java.util.Random</code> > - implementation class to use. If not specified, the default value > is > + <p>Name of the Java class that extends > + <code>java.security.SecureRandom</code> to use to generate session > IDs. > + If not specified, the default value is > <code>java.security.SecureRandom</code>.</p> > </attribute> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
