From 4073acd78f25853337b76e022b343b887fe94e27 Mon Sep 17 00:00:00 2001 From: markt Date: Fri, 11 Dec 2009 17:30:59 +0000 Subject: [PATCH] Address session fixation by changing the session ID on authentication. This is enabled by default. This should be safe since this also happens when sessions migrate between nodes in a cluster. If an app can't handle a changing ID, then the feature can be disabled in the authenticator. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@889716 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Manager.java | 9 ++++++ .../catalina/authenticator/AuthenticatorBase.java | 15 +++++++++ java/org/apache/catalina/connector/Request.java | 36 +++++++++++++++++++++- .../catalina/ha/session/JvmRouteBinderValve.java | 7 +++-- java/org/apache/catalina/session/ManagerBase.java | 11 +++++++ webapps/docs/config/valve.xml | 28 +++++++++++++++++ 6 files changed, 102 insertions(+), 4 deletions(-) diff --git a/java/org/apache/catalina/Manager.java b/java/org/apache/catalina/Manager.java index 36e170dc2..9689528c0 100644 --- a/java/org/apache/catalina/Manager.java +++ b/java/org/apache/catalina/Manager.java @@ -260,6 +260,15 @@ public interface Manager { /** + * Change the session ID of the current session to a new randomly generated + * session ID. + * + * @param session The session to change the session ID for + */ + public void changeSessionId(Session session); + + + /** * Get a session from the recycled ones or create a new empty one. * The PersistentManager manager does not need to create session data * because it reads it from the Store. diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java index 8af48ae12..51cea993a 100644 --- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java +++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java @@ -38,6 +38,7 @@ import org.apache.catalina.Context; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleListener; +import org.apache.catalina.Manager; import org.apache.catalina.Pipeline; import org.apache.catalina.Realm; import org.apache.catalina.Session; @@ -123,6 +124,12 @@ public abstract class AuthenticatorBase /** + * Should the session ID, if any, be changed upon a successful + * authentication to prevent a session fixation attack? + */ + protected boolean changeSessionIdOnAuthentication = true; + + /** * The Context to which this Valve is attached. */ protected Context context = null; @@ -513,6 +520,7 @@ public abstract class AuthenticatorBase */ return; } + } if (log.isDebugEnabled()) { @@ -726,6 +734,13 @@ public abstract class AuthenticatorBase request.setUserPrincipal(principal); Session session = request.getSessionInternal(false); + + if (session != null && changeSessionIdOnAuthentication) { + Manager manager = request.getContext().getManager(); + manager.changeSessionId(session); + request.changeSessionId(session.getId()); + } + // Cache the authentication information in our session, if any if (cache) { if (session != null) { diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index a73f20477..f0bfc3823 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -2252,6 +2252,40 @@ public class Request /** + * Change the ID of the session that this request is associated with. There + * are several things that may trigger an ID change. These include mmoving + * between nodes in a cluster and session fixation prevention during the + * authentication process. + * + * @param session The session to change the session ID for + */ + public void changeSessionId(String newSessionId) { + // This should only ever be called if there was an old session ID but + // double check to be sure + if (requestedSessionId != null && requestedSessionId.length() > 0) { + requestedSessionId = newSessionId; + } + + if (context != null && !context.getServletContext() + .getEffectiveSessionTrackingModes().contains( + SessionTrackingMode.COOKIE)) + return; + + if (response != null) { + Cookie newCookie = + ApplicationSessionCookieConfig.createSessionCookie( + context.getServletContext().getSessionCookieConfig(), + newSessionId, + secure, + context.getUseHttpOnly(), + response.getConnector().getEmptySessionPath(), + context.getEncodedPath()); + response.addCookie(newCookie); + } + } + + + /** * Return the session associated with this Request, creating one * if necessary and requested. * @@ -2370,7 +2404,7 @@ public class Request throw new ServletException( sm.getString("coyoteRequest.authFail", username)); } - // Assume if we have a non-null LogonConfig then we must have an + // Assume if we have a non-null LoginConfig then we must have an // authenticator context.getAuthenticator().register(this, getResponse(), principal, authMethod, username, password); diff --git a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java index 80c2c2197..015f0dc34 100644 --- a/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java +++ b/java/org/apache/catalina/ha/session/JvmRouteBinderValve.java @@ -410,9 +410,8 @@ public class JvmRouteBinderValve extends ValveBase implements ClusterValve, Life * new session id for node migration */ protected void changeRequestSessionID(Request request, Response response, String sessionId, String newSessionID) { - request.setRequestedSessionId(newSessionID); - if(request.isRequestedSessionIdFromCookie()) - setNewSessionCookie(request, response,newSessionID); + request.changeSessionId(newSessionID); + // set original sessionid at request, to allow application detect the // change if (sessionIdAttribute != null && !"".equals(sessionIdAttribute)) { @@ -454,6 +453,8 @@ public class JvmRouteBinderValve extends ValveBase implements ClusterValve, Life * @param request current request * @param response Tomcat Response * @param sessionId The session id + * + * @deprecated Use {@link Request#changeSessionId(String)} */ protected void setNewSessionCookie(Request request, Response response, String sessionId) { diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java index f7f1799b2..865200f70 100644 --- a/java/org/apache/catalina/session/ManagerBase.java +++ b/java/org/apache/catalina/session/ManagerBase.java @@ -922,6 +922,17 @@ public abstract class ManagerBase implements Manager, MBeanRegistration { } + /** + * Change the session ID of the current session to a new randomly generated + * session ID. + * + * @param session The session to change the session ID for + */ + public void changeSessionId(Session session) { + session.setId(generateSessionId()); + } + + // ------------------------------------------------------ Protected Methods diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml index a15e6e971..875bcdd8d 100644 --- a/webapps/docs/config/valve.xml +++ b/webapps/docs/config/valve.xml @@ -395,6 +395,13 @@ org.apache.catalina.authenticator.BasicAuthenticator.

+ +

Controls if the session ID is changed if a session exists at the + point where users are authenticated. This is to prevent session fixation + attacks. If not set, the default value of true will be + used.

+ +

Controls the caching of pages that are protected by security constraints. Setting this to false may help work around @@ -447,6 +454,13 @@ org.apache.catalina.authenticator.DigestAuthenticator.

 + +

Controls if the session ID is changed if a session exists at the + point where users are authenticated. This is to prevent session fixation + attacks. If not set, the default value of true will be + used.

+ +

Controls the caching of pages that are protected by security constraints. Setting this to false may help work around @@ -499,6 +513,13 @@ org.apache.catalina.authenticator.FormAuthenticator.

 + +

Controls if the session ID is changed if a session exists at the + point where users are authenticated. This is to prevent session fixation + attacks. If not set, the default value of true will be + used.

+ +

Character encoding to use to read the username and password parameters from the request. If not set, the encoding of the request body will be @@ -557,6 +578,13 @@ org.apache.catalina.authenticator.SSLAuthenticator.

 + +

Controls if the session ID is changed if a session exists at the + point where users are authenticated. This is to prevent session fixation + attacks. If not set, the default value of true will be + used.

+ +

Controls the caching of pages that are protected by security constraints. Setting this to false may help work around -- 2.11.0