You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/11/27 18:05:27 UTC
svn commit: r1039707 - in /tomcat/trunk:
java/org/apache/catalina/session/ManagerBase.java
webapps/docs/config/manager.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1039707 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java
webapps/docs/config/manager.xml
Posted by Phil Steitz <ph...@gmail.com>.
On Sun, Nov 28, 2010 at 8:37 AM, Mark Thomas <ma...@apache.org> wrote:
> On 27/11/2010 21:01, Phil Steitz wrote:
> > 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 not a huge amount.
>
> > 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.
>
> That makes sense but my focus right now isn't on performance at
> initialisation but performance of session ID generation. I want to get
> to the bottom of why SecureRandom on OSX was behaving as if it was
> synchronized internally even though I didn't see any blocking in
> YourKit. More importantly, does it behave the same way on Linux? More
> testing is required.
>
>
OK, sorry for the noise. I may be wrong, but I think the default on Linux
at least used to be NativePRNG. Looking at the source for NativePRNG (e.g.,
[1]), it appears that it mixes entropy bytes from /dev/urandom on each call
to engineNextBytes - not just at initialization, which could cause periodic
waits on /dev/urandom. See [2] for more info.
[1] h*ttp://s.apache.org/5KC <http://s.apache.org/5KC>*
[2] http://s.apache.org/SRg
Phil
Re: svn commit: r1039707 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java
webapps/docs/config/manager.xml
Posted by Mark Thomas <ma...@apache.org>.
On 27/11/2010 21:01, Phil Steitz wrote:
> 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 not a huge amount.
> 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.
That makes sense but my focus right now isn't on performance at
initialisation but performance of session ID generation. I want to get
to the bottom of why SecureRandom on OSX was behaving as if it was
synchronized internally even though I didn't see any blocking in
YourKit. More importantly, does it behave the same way on Linux? More
testing is required.
Mark
>
> 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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1039707 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java
webapps/docs/config/manager.xml
Posted by Phil Steitz <ph...@gmail.com>.
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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>