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 2015/11/08 21:05:27 UTC
svn commit: r1713285 - in /tomcat/trunk: java/org/apache/catalina/
java/org/apache/catalina/session/ java/org/apache/catalina/util/
test/org/apache/catalina/util/
Author: markt
Date: Sun Nov 8 20:05:27 2015
New Revision: 1713285
URL: http://svn.apache.org/viewvc?rev=1713285&view=rev
Log:
Add the ability to validate client provided session IDs and implement basic validation for the Standard session ID generator.
Added:
tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java (with props)
Modified:
tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java
tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java
tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java
Modified: tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java (original)
+++ tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java Sun Nov 8 20:05:27 2015
@@ -56,4 +56,17 @@ public interface SessionIdGenerator {
*/
public String generateSessionId(String route);
+ /**
+ * Determine, based on implementation specific rules which may be as strict
+ * or as relaxed as the implementor wishes, if the provided session ID is
+ * valid. This may be used when generating sessions with user provided
+ * session IDs to ensure that they are suitable or if a new ID needs to be
+ * generated.
+ *
+ * @param sessionId The proposed session ID to test
+ *
+ * @return {@code true} if the proposed session ID is acceptable, otherwise
+ * {@code false}
+ */
+ public boolean validateSessionId(String sessionId);
}
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=1713285&r1=1713284&r2=1713285&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sun Nov 8 20:05:27 2015
@@ -627,7 +627,7 @@ public abstract class ManagerBase extend
session.setCreationTime(System.currentTimeMillis());
session.setMaxInactiveInterval(this.maxInactiveInterval);
String id = sessionId;
- if (id == null) {
+ if (id == null || !sessionIdGenerator.validateSessionId(id)) {
id = generateSessionId();
}
session.setId(id);
Modified: tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java?rev=1713285&r1=1713284&r2=1713285&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java Sun Nov 8 20:05:27 2015
@@ -273,6 +273,18 @@ public abstract class SessionIdGenerator
}
+ /**
+ * {@inheritDoc}
+ * <p>
+ * The base implementation performs no validation and treats all proposed
+ * session IDs as valid.
+ */
+ @Override
+ public boolean validateSessionId(String sessionId) {
+ return true;
+ }
+
+
@Override
protected void initInternal() throws LifecycleException {
// NO-OP
Modified: tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java (original)
+++ tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java Sun Nov 8 20:05:27 2015
@@ -16,6 +16,8 @@
*/
package org.apache.catalina.util;
+import org.apache.tomcat.util.buf.HexUtils;
+
public class StandardSessionIdGenerator extends SessionIdGeneratorBase {
@Override
@@ -61,4 +63,39 @@ public class StandardSessionIdGenerator
return buffer.toString();
}
+ /**
+ * {@inheritDoc}
+ * <p>
+ * This implementation performs the following checks:
+ * <ul>
+ * <li>The characters up to the first period (if any) are valid hex
+ * digits</li>
+ * <li>There are at least enough hex digits to represent the specified
+ * session ID length</li>
+ * <li>Anything after the first period is not validated since that is
+ * assumed to be a JVM route and we can't easily determine valid
+ * values</li>
+ * </ul>
+ */
+ @Override
+ public boolean validateSessionId(String sessionId) {
+ if (sessionId == null) {
+ return false;
+ }
+ int len = sessionId.indexOf('.');
+ if (len == -1) {
+ len = sessionId.length();
+ }
+ // Session ID length is in bytes and 2 hex digits are required for each
+ // byte
+ if (len < getSessionIdLength() * 2) {
+ return false;
+ }
+ for (int i = 0; i < len; i++) {
+ if (HexUtils.getDec(sessionId.charAt(i)) == -1) {
+ return false;
+ }
+ }
+ return true;
+ }
}
Added: tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java?rev=1713285&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java (added)
+++ tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java Sun Nov 8 20:05:27 2015
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStandardSessionIdGenerator {
+
+ // 100 character long valid session ID. This long to accomodate any future
+ // changes in defaut session ID length
+ private static final String VALID = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+
+ private StandardSessionIdGenerator generator = new StandardSessionIdGenerator();
+
+ @Test
+ public void testValidateNull() {
+ Assert.assertFalse(generator.validateSessionId(null));
+ }
+
+ @Test
+ public void testValidateEmpty() {
+ Assert.assertFalse(generator.validateSessionId(""));
+ }
+
+ @Test
+ public void testValidateOneChar() {
+ Assert.assertFalse(generator.validateSessionId("A"));
+ }
+
+ @Test
+ public void testValidateShort() {
+ Assert.assertFalse(generator.validateSessionId(
+ VALID.substring(0, (generator.getSessionIdLength() * 2) -1)));
+ }
+
+ @Test
+ public void testValidateJustRight() {
+ Assert.assertTrue(generator.validateSessionId(
+ VALID.substring(0, (generator.getSessionIdLength() * 2))));
+ }
+
+ @Test
+ public void testValidateLong() {
+ Assert.assertTrue(generator.validateSessionId(VALID));
+ }
+
+ @Test
+ public void testValidateInvalid() {
+ Assert.assertFalse(generator.validateSessionId(VALID + "g"));
+ }
+
+ @Test
+ public void testValidateWithJvmRoute() {
+ Assert.assertTrue(generator.validateSessionId(VALID + ".g"));
+ }
+
+ @Test
+ public void testValidateWithJvmRouteWithPerid() {
+ Assert.assertTrue(generator.validateSessionId(VALID + ".g.h.i"));
+ }
+
+}
Propchange: tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java
------------------------------------------------------------------------------
svn:eol-style = native
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1713285 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/session/ java/org/apache/catalina/util/ test/org/apache/catalina/util/
Posted by Mark Thomas <ma...@apache.org>.
On 8 November 2015 22:03:39 GMT+00:00, Martin Grigorov <mg...@apache.org> wrote:
>On Sun, Nov 8, 2015 at 9:05 PM, <ma...@apache.org> wrote:
>
>> public void testValidateWithJvmRouteWithPerid() {
>>
>
>I'd fix it myself with my new superpowers but I'm not sure whether
>there is
>a typo in Period above or it is just my non-native English.
It's a typo. Go for it.
Mark
Re: svn commit: r1713285 - in /tomcat/trunk: java/org/apache/catalina/
java/org/apache/catalina/session/ java/org/apache/catalina/util/ test/org/apache/catalina/util/
Posted by Martin Grigorov <mg...@apache.org>.
On Sun, Nov 8, 2015 at 9:05 PM, <ma...@apache.org> wrote:
> public void testValidateWithJvmRouteWithPerid() {
>
I'd fix it myself with my new superpowers but I'm not sure whether there is
a typo in Period above or it is just my non-native English.
Re: svn commit: r1713285 - in /tomcat/trunk:
java/org/apache/catalina/ java/org/apache/catalina/session/
java/org/apache/catalina/util/ test/org/apache/catalina/util/
Posted by Mark Thomas <ma...@apache.org>.
On 08/11/2015 20:05, markt@apache.org wrote:
> Author: markt
> Date: Sun Nov 8 20:05:27 2015
> New Revision: 1713285
>
> URL: http://svn.apache.org/viewvc?rev=1713285&view=rev
> Log:
> Add the ability to validate client provided session IDs and implement basic validation for the Standard session ID generator.
I've been thinking about this and I'm not convinced this is a good idea.
There are multiple issues:
- jvmRoute isn't validated
- session IDs are only validated using the current context's settings
Given that the whole point of re-using a client provided session is to
have a common ID across multiple webapps, validating a session ID with
the current context's rules doesn't make a whole lot of sense.
Remy pointed me to a different solution he deployed in JBoss off-list.
It looks much more promising.
I intend to revert this and use Remy's approach.
Mark
>
> Added:
> tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java (with props)
> Modified:
> tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java
> tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java
> tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java
> tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java Sun Nov 8 20:05:27 2015
> @@ -56,4 +56,17 @@ public interface SessionIdGenerator {
> */
> public String generateSessionId(String route);
>
> + /**
> + * Determine, based on implementation specific rules which may be as strict
> + * or as relaxed as the implementor wishes, if the provided session ID is
> + * valid. This may be used when generating sessions with user provided
> + * session IDs to ensure that they are suitable or if a new ID needs to be
> + * generated.
> + *
> + * @param sessionId The proposed session ID to test
> + *
> + * @return {@code true} if the proposed session ID is acceptable, otherwise
> + * {@code false}
> + */
> + public boolean validateSessionId(String sessionId);
> }
>
> 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=1713285&r1=1713284&r2=1713285&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sun Nov 8 20:05:27 2015
> @@ -627,7 +627,7 @@ public abstract class ManagerBase extend
> session.setCreationTime(System.currentTimeMillis());
> session.setMaxInactiveInterval(this.maxInactiveInterval);
> String id = sessionId;
> - if (id == null) {
> + if (id == null || !sessionIdGenerator.validateSessionId(id)) {
> id = generateSessionId();
> }
> session.setId(id);
>
> Modified: tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java?rev=1713285&r1=1713284&r2=1713285&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java Sun Nov 8 20:05:27 2015
> @@ -273,6 +273,18 @@ public abstract class SessionIdGenerator
> }
>
>
> + /**
> + * {@inheritDoc}
> + * <p>
> + * The base implementation performs no validation and treats all proposed
> + * session IDs as valid.
> + */
> + @Override
> + public boolean validateSessionId(String sessionId) {
> + return true;
> + }
> +
> +
> @Override
> protected void initInternal() throws LifecycleException {
> // NO-OP
>
> Modified: tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java Sun Nov 8 20:05:27 2015
> @@ -16,6 +16,8 @@
> */
> package org.apache.catalina.util;
>
> +import org.apache.tomcat.util.buf.HexUtils;
> +
> public class StandardSessionIdGenerator extends SessionIdGeneratorBase {
>
> @Override
> @@ -61,4 +63,39 @@ public class StandardSessionIdGenerator
> return buffer.toString();
> }
>
> + /**
> + * {@inheritDoc}
> + * <p>
> + * This implementation performs the following checks:
> + * <ul>
> + * <li>The characters up to the first period (if any) are valid hex
> + * digits</li>
> + * <li>There are at least enough hex digits to represent the specified
> + * session ID length</li>
> + * <li>Anything after the first period is not validated since that is
> + * assumed to be a JVM route and we can't easily determine valid
> + * values</li>
> + * </ul>
> + */
> + @Override
> + public boolean validateSessionId(String sessionId) {
> + if (sessionId == null) {
> + return false;
> + }
> + int len = sessionId.indexOf('.');
> + if (len == -1) {
> + len = sessionId.length();
> + }
> + // Session ID length is in bytes and 2 hex digits are required for each
> + // byte
> + if (len < getSessionIdLength() * 2) {
> + return false;
> + }
> + for (int i = 0; i < len; i++) {
> + if (HexUtils.getDec(sessionId.charAt(i)) == -1) {
> + return false;
> + }
> + }
> + return true;
> + }
> }
>
> Added: tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java?rev=1713285&view=auto
> ==============================================================================
> --- tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java (added)
> +++ tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java Sun Nov 8 20:05:27 2015
> @@ -0,0 +1,78 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.catalina.util;
> +
> +import org.junit.Assert;
> +import org.junit.Test;
> +
> +public class TestStandardSessionIdGenerator {
> +
> + // 100 character long valid session ID. This long to accomodate any future
> + // changes in defaut session ID length
> + private static final String VALID = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
> + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
> +
> + private StandardSessionIdGenerator generator = new StandardSessionIdGenerator();
> +
> + @Test
> + public void testValidateNull() {
> + Assert.assertFalse(generator.validateSessionId(null));
> + }
> +
> + @Test
> + public void testValidateEmpty() {
> + Assert.assertFalse(generator.validateSessionId(""));
> + }
> +
> + @Test
> + public void testValidateOneChar() {
> + Assert.assertFalse(generator.validateSessionId("A"));
> + }
> +
> + @Test
> + public void testValidateShort() {
> + Assert.assertFalse(generator.validateSessionId(
> + VALID.substring(0, (generator.getSessionIdLength() * 2) -1)));
> + }
> +
> + @Test
> + public void testValidateJustRight() {
> + Assert.assertTrue(generator.validateSessionId(
> + VALID.substring(0, (generator.getSessionIdLength() * 2))));
> + }
> +
> + @Test
> + public void testValidateLong() {
> + Assert.assertTrue(generator.validateSessionId(VALID));
> + }
> +
> + @Test
> + public void testValidateInvalid() {
> + Assert.assertFalse(generator.validateSessionId(VALID + "g"));
> + }
> +
> + @Test
> + public void testValidateWithJvmRoute() {
> + Assert.assertTrue(generator.validateSessionId(VALID + ".g"));
> + }
> +
> + @Test
> + public void testValidateWithJvmRouteWithPerid() {
> + Assert.assertTrue(generator.validateSessionId(VALID + ".g.h.i"));
> + }
> +
> +}
>
> Propchange: tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java
> ------------------------------------------------------------------------------
> svn:eol-style = native
>
>
>
> ---------------------------------------------------------------------
> 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