Harden the FORM authentication process

When the session ID is configured to change on authentication, track the expected session ID through the authentication process and ensure that the expected value is seen at each stage.
apache · Dec 6, 2019 · fd381e9 · fd381e9
1 parent 1ecba14
commit fd381e9
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 2 deletions.
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -1126,7 +1126,11 @@ private void register(Request request, HttpServletResponse response, Principal p
             // If the principal is null then this is a logout. No need to change
             // the session ID. See BZ 59043.
             if (getChangeSessionIdOnAuthentication() && principal != null) {
-                changeSessionID(request, session);
+                String newSessionId = changeSessionID(request, session);
+                // If the current session ID is being tracked, update it.
+                if (session.getNote(Constants.SESSION_ID_NOTE) != null) {
+                    session.setNote(Constants.SESSION_ID_NOTE, newSessionId);
+                }
             }
         } else if (alwaysUseSession) {
             session = request.getSessionInternal(true);

diff --git a/java/org/apache/catalina/authenticator/Constants.java b/java/org/apache/catalina/authenticator/Constants.java
@@ -52,6 +52,12 @@ public class Constants {
 
     // ---------------------------------------------------------- Session Notes
 
+    /**
+     * The session id used as a CSRF marker when redirecting a user's request.
+     */
+    public static final String SESSION_ID_NOTE = "org.apache.catalina.authenticator.SESSION_ID";
+
+
     /**
      * If the <code>cache</code> property of our authenticator is set, and
      * the current request is part of a session, authentication information

diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -253,6 +253,14 @@ protected boolean doAuthenticate(Request request, HttpServletResponse response)
         if (session == null) {
             session = request.getSessionInternal(false);
         }
+        if (session != null && getChangeSessionIdOnAuthentication()) {
+            // Does session id match?
+            String expectedSessionId = (String) session.getNote(Constants.SESSION_ID_NOTE);
+            if (expectedSessionId == null || !expectedSessionId.equals(request.getRequestedSessionId())) {
+                session.expire();
+                session = null;
+            }
+        }
         if (session == null) {
             if (containerLog.isDebugEnabled()) {
                 containerLog.debug("User took so long to log on the session expired");
@@ -382,7 +390,8 @@ protected void forwardToLoginPage(Request request,
         if (getChangeSessionIdOnAuthentication()) {
             Session session = request.getSessionInternal(false);
             if (session != null) {
-                changeSessionID(request, session);
+                String newSessionId = changeSessionID(request, session);
+                session.setNote(Constants.SESSION_ID_NOTE, newSessionId);
             }
         }
 
@@ -479,6 +488,14 @@ protected boolean matchRequest(Request request) {
             return false;
         }
 
+        // Does session id match?
+        if (getChangeSessionIdOnAuthentication()) {
+            String expectedSessionId = (String) session.getNote(Constants.SESSION_ID_NOTE);
+            if (expectedSessionId == null || !expectedSessionId.equals(request.getRequestedSessionId())) {
+                return false;
+            }
+        }
+
         // Does the request URI match?
         String decodedRequestURI = request.getDecodedRequestURI();
         if (decodedRequestURI == null) {
@@ -505,6 +522,7 @@ protected boolean restoreRequest(Request request, Session session)
         // Retrieve and remove the SavedRequest object from our session
         SavedRequest saved = (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
         session.removeNote(Constants.FORM_REQUEST_NOTE);
+        session.removeNote(Constants.SESSION_ID_NOTE);
         if (saved == null) {
             return false;
         }