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 12:41:11 UTC

svn commit: r1039657 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java java/org/apache/catalina/session/mbeans-descriptors.xml webapps/docs/changelog.xml webapps/docs/config/manager.xml

Author: markt
Date: Sat Nov 27 11:41:10 2010
New Revision: 1039657

URL: http://svn.apache.org/viewvc?rev=1039657&view=rev
Log:
Drop the entropy attribute. SecureRandom has a sufficiently secure self-seeding mechanism.

Modified:
    tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
    tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
    tomcat/trunk/webapps/docs/changelog.xml
    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=1039657&r1=1039656&r2=1039657&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sat Nov 27 11:41:10 2010
@@ -28,7 +28,6 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.security.SecureRandom;
@@ -55,11 +54,9 @@ import org.apache.catalina.LifecycleExce
 import org.apache.catalina.Manager;
 import org.apache.catalina.Session;
 import org.apache.catalina.mbeans.MBeanUtils;
-import org.apache.catalina.util.Base64;
 import org.apache.catalina.util.LifecycleMBeanBase;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
-import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
 
@@ -100,13 +97,6 @@ public abstract class ManagerBase extend
 
 
     /**
-     * A String initialization parameter used to increase the entropy of
-     * the initialization of our random number generator.
-     */
-    protected String entropy = null;
-
-
-    /**
      * The descriptive information string for this implementation.
      */
     private static final String info = "ManagerBase/1.0";
@@ -339,58 +329,6 @@ public abstract class ManagerBase extend
 
 
     /**
-     * Return the entropy increaser value, or compute a semi-useful value
-     * if this String has not yet been set.
-     */
-    public String getEntropy() {
-
-        // Calculate a semi-useful value if this has not been set
-        if (this.entropy == null) {
-            // Use APR to get a crypto secure entropy value
-            byte[] result = new byte[32];
-            boolean apr = false;
-            try {
-                String methodName = "random";
-                Class<?> paramTypes[] = new Class[2];
-                paramTypes[0] = result.getClass();
-                paramTypes[1] = int.class;
-                Object paramValues[] = new Object[2];
-                paramValues[0] = result;
-                paramValues[1] = Integer.valueOf(32);
-                Method method = Class.forName("org.apache.tomcat.jni.OS")
-                    .getMethod(methodName, paramTypes);
-                method.invoke(null, paramValues);
-                apr = true;
-            } catch (Throwable t) {
-                ExceptionUtils.handleThrowable(t);
-            }
-            if (apr) {
-                setEntropy(Base64.encode(result));
-            } else {
-                setEntropy(this.toString());
-            }
-        }
-
-        return (this.entropy);
-
-    }
-
-
-    /**
-     * Set the entropy increaser value.
-     *
-     * @param entropy The new entropy increaser value
-     */
-    public void setEntropy(String entropy) {
-
-        String oldEntropy = entropy;
-        this.entropy = entropy;
-        support.firePropertyChange("entropy", oldEntropy, this.entropy);
-
-    }
-
-
-    /**
      * Return descriptive information about this Manager implementation and
      * the corresponding version number, in the format
      * <code>&lt;description&gt;/&lt;version&gt;</code>.
@@ -619,11 +557,6 @@ public abstract class ManagerBase extend
 
         long seed = System.currentTimeMillis();
         long t1 = seed;
-        char entropy[] = getEntropy().toCharArray();
-        for (int i = 0; i < entropy.length; i++) {
-            long update = ((byte) entropy[i]) << ((i % 8) * 8);
-            seed ^= update;
-        }
 
         // Construct and seed a new random number generator
         SecureRandom result = new SecureRandom();

Modified: tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml (original)
+++ tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml Sat Nov 27 11:41:10 2010
@@ -42,12 +42,6 @@
           description="Number of duplicated session ids generated"
                  type="int" />
 
-    <attribute   name="entropy"
-          description="A String initialization parameter used to increase the
-                       entropy of the initialization of our random number
-                       generator"
-                 type="java.lang.String"/>
-                 
     <attribute   name="expiredSessions"
           description="Number of sessions that expired ( doesn't include explicit invalidations )"
                  type="long" />
@@ -235,12 +229,6 @@
           description="Number of duplicated session ids generated"
                  type="int" />
 
-    <attribute   name="entropy"
-          description="A String initialization parameter used to increase the
-                       entropy of the initialization of our random number
-                       generator"
-                 type="java.lang.String"/>
-                 
     <attribute   name="expiredSessions"
           description="Number of sessions that expired ( doesn't include explicit invalidations )"
                  type="long" />

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Nov 27 11:41:10 2010
@@ -54,6 +54,10 @@
         <bug>50106</bug>: Correct several MBean descriptors. Patch provided by
         Eiji Takahashi. (markt)
       </fix>
+      <update>
+        Further performance improvements to session ID generation. Remove legacy
+        configuration options that are no longer required.
+      </update>
     </changelog>
   </subsection>
   <subsection name="Coyote">

Modified: tomcat/trunk/webapps/docs/config/manager.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/manager.xml (original)
+++ tomcat/trunk/webapps/docs/config/manager.xml Sat Nov 27 11:41:10 2010
@@ -99,14 +99,6 @@
 
     <attributes>
 
-      <attribute name="entropy" required="false">
-        <p>A String value that is utilized when seeding the random number
-        generator used to create session identifiers for this Manager.
-        If not specified, a semi-useful value is calculated, but a long
-        String value should be specified in security-conscious
-        environments.</p>
-      </attribute>
-
       <attribute name="maxActiveSessions" required="false">
         <p>The maximum number of active sessions that will be created by
         this Manager, or -1 (the default) for no limit.</p>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1039657 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java java/org/apache/catalina/session/mbeans-descriptors.xml webapps/docs/changelog.xml webapps/docs/config/manager.xml

Posted by Mark Thomas <ma...@apache.org>.
On 27/11/2010 20:37, Phil Steitz wrote:
>> Author: markt
>> Date: Sat Nov 27 11:41:10 2010
>> New Revision: 1039657

> I think you need to drop the line
> 
> result.setSeed(seed);
> 
> following this; otherwise you are seeding result with (unadulterated)
> currentTimeMillis.

For a SecureRandom that should not cause a problem but it is of little
to no value. There is no need to removed it as it is not reducing
entropy but it may as well be removed as it is unlikely to be adding
that much entropy either.

> If you want to take the initialization hit in this
> method, you can call nextBytes; otherwise you may as well just return new
> SecureRandom() (in which case, the self-seeding and initialization hit will
> take place the first time the generator is used).

The initialisation hit is taken when the Manager starts triggered by the
call to generateSessionId() in startInternal().

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1039657 - in /tomcat/trunk: java/org/apache/catalina/session/ManagerBase.java java/org/apache/catalina/session/mbeans-descriptors.xml webapps/docs/changelog.xml webapps/docs/config/manager.xml

Posted by Phil Steitz <ph...@gmail.com>.
On Sat, Nov 27, 2010 at 6:41 AM, <ma...@apache.org> wrote:

> Author: markt
> Date: Sat Nov 27 11:41:10 2010
> New Revision: 1039657
>
> URL: http://svn.apache.org/viewvc?rev=1039657&view=rev
> Log:
> Drop the entropy attribute. SecureRandom has a sufficiently secure
> self-seeding mechanism.
>
> Modified:
>    tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
>    tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
>    tomcat/trunk/webapps/docs/changelog.xml
>    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=1039657&r1=1039656&r2=1039657&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sat Nov
> 27 11:41:10 2010
> @@ -28,7 +28,6 @@ import java.io.File;
>  import java.io.FileInputStream;
>  import java.io.IOException;
>  import java.io.InputStream;
> -import java.lang.reflect.Method;
>  import java.security.AccessController;
>  import java.security.PrivilegedAction;
>  import java.security.SecureRandom;
> @@ -55,11 +54,9 @@ import org.apache.catalina.LifecycleExce
>  import org.apache.catalina.Manager;
>  import org.apache.catalina.Session;
>  import org.apache.catalina.mbeans.MBeanUtils;
> -import org.apache.catalina.util.Base64;
>  import org.apache.catalina.util.LifecycleMBeanBase;
>  import org.apache.juli.logging.Log;
>  import org.apache.juli.logging.LogFactory;
> -import org.apache.tomcat.util.ExceptionUtils;
>  import org.apache.tomcat.util.res.StringManager;
>
>
> @@ -100,13 +97,6 @@ public abstract class ManagerBase extend
>
>
>     /**
> -     * A String initialization parameter used to increase the entropy of
> -     * the initialization of our random number generator.
> -     */
> -    protected String entropy = null;
> -
> -
> -    /**
>      * The descriptive information string for this implementation.
>      */
>     private static final String info = "ManagerBase/1.0";
> @@ -339,58 +329,6 @@ public abstract class ManagerBase extend
>
>
>     /**
> -     * Return the entropy increaser value, or compute a semi-useful value
> -     * if this String has not yet been set.
> -     */
> -    public String getEntropy() {
> -
> -        // Calculate a semi-useful value if this has not been set
> -        if (this.entropy == null) {
> -            // Use APR to get a crypto secure entropy value
> -            byte[] result = new byte[32];
> -            boolean apr = false;
> -            try {
> -                String methodName = "random";
> -                Class<?> paramTypes[] = new Class[2];
> -                paramTypes[0] = result.getClass();
> -                paramTypes[1] = int.class;
> -                Object paramValues[] = new Object[2];
> -                paramValues[0] = result;
> -                paramValues[1] = Integer.valueOf(32);
> -                Method method = Class.forName("org.apache.tomcat.jni.OS")
> -                    .getMethod(methodName, paramTypes);
> -                method.invoke(null, paramValues);
> -                apr = true;
> -            } catch (Throwable t) {
> -                ExceptionUtils.handleThrowable(t);
> -            }
> -            if (apr) {
> -                setEntropy(Base64.encode(result));
> -            } else {
> -                setEntropy(this.toString());
> -            }
> -        }
> -
> -        return (this.entropy);
> -
> -    }
> -
> -
> -    /**
> -     * Set the entropy increaser value.
> -     *
> -     * @param entropy The new entropy increaser value
> -     */
> -    public void setEntropy(String entropy) {
> -
> -        String oldEntropy = entropy;
> -        this.entropy = entropy;
> -        support.firePropertyChange("entropy", oldEntropy, this.entropy);
> -
> -    }
> -
> -
> -    /**
>      * Return descriptive information about this Manager implementation and
>      * the corresponding version number, in the format
>      * <code>&lt;description&gt;/&lt;version&gt;</code>.
> @@ -619,11 +557,6 @@ public abstract class ManagerBase extend
>
>         long seed = System.currentTimeMillis();
>         long t1 = seed;
> -        char entropy[] = getEntropy().toCharArray();
> -        for (int i = 0; i < entropy.length; i++) {
> -            long update = ((byte) entropy[i]) << ((i % 8) * 8);
> -            seed ^= update;
> -        }
>
>         // Construct and seed a new random number generator
>         SecureRandom result = new SecureRandom();
>

I think you need to drop the line

result.setSeed(seed);

following this; otherwise you are seeding result with (unadulterated)
currentTimeMillis.    If you want to take the initialization hit in this
method, you can call nextBytes; otherwise you may as well just return new
SecureRandom() (in which case, the self-seeding and initialization hit will
take place the first time the generator is used).

Phil



> Modified:
> tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
>
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/mbeans-descriptors.xml
> Sat Nov 27 11:41:10 2010
> @@ -42,12 +42,6 @@
>           description="Number of duplicated session ids generated"
>                  type="int" />
>
> -    <attribute   name="entropy"
> -          description="A String initialization parameter used to increase
> the
> -                       entropy of the initialization of our random number
> -                       generator"
> -                 type="java.lang.String"/>
> -
>     <attribute   name="expiredSessions"
>           description="Number of sessions that expired ( doesn't include
> explicit invalidations )"
>                  type="long" />
> @@ -235,12 +229,6 @@
>           description="Number of duplicated session ids generated"
>                  type="int" />
>
> -    <attribute   name="entropy"
> -          description="A String initialization parameter used to increase
> the
> -                       entropy of the initialization of our random number
> -                       generator"
> -                 type="java.lang.String"/>
> -
>     <attribute   name="expiredSessions"
>           description="Number of sessions that expired ( doesn't include
> explicit invalidations )"
>                  type="long" />
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
>
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Sat Nov 27 11:41:10 2010
> @@ -54,6 +54,10 @@
>         <bug>50106</bug>: Correct several MBean descriptors. Patch provided
> by
>         Eiji Takahashi. (markt)
>       </fix>
> +      <update>
> +        Further performance improvements to session ID generation. Remove
> legacy
> +        configuration options that are no longer required.
> +      </update>
>     </changelog>
>   </subsection>
>   <subsection name="Coyote">
>
> Modified: tomcat/trunk/webapps/docs/config/manager.xml
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1039657&r1=1039656&r2=1039657&view=diff
>
> ==============================================================================
> --- tomcat/trunk/webapps/docs/config/manager.xml (original)
> +++ tomcat/trunk/webapps/docs/config/manager.xml Sat Nov 27 11:41:10 2010
> @@ -99,14 +99,6 @@
>
>     <attributes>
>
> -      <attribute name="entropy" required="false">
> -        <p>A String value that is utilized when seeding the random number
> -        generator used to create session identifiers for this Manager.
> -        If not specified, a semi-useful value is calculated, but a long
> -        String value should be specified in security-conscious
> -        environments.</p>
> -      </attribute>
> -
>       <attribute name="maxActiveSessions" required="false">
>         <p>The maximum number of active sessions that will be created by
>         this Manager, or -1 (the default) for no limit.</p>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>