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 2019/12/07 17:14:17 UTC

[tomcat] 14/18: Harden the FORM authentication process

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit e8ae801192b3b07afdffeb0c653eb2abe4808e9e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Dec 5 23:01:42 2019 +0000

    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.
---
 .../catalina/authenticator/AuthenticatorBase.java    |  6 +++++-
 .../org/apache/catalina/authenticator/Constants.java |  6 ++++++
 .../catalina/authenticator/FormAuthenticator.java    | 20 +++++++++++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index cec4baa..f35fbd6 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -1128,7 +1128,11 @@ public abstract class AuthenticatorBase extends ValveBase
             // 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
index f257b4f..66f2a49 100644
--- a/java/org/apache/catalina/authenticator/Constants.java
+++ b/java/org/apache/catalina/authenticator/Constants.java
@@ -58,6 +58,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
      * will be cached to avoid the need for repeated calls to
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index f326f77..e9b9839 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -253,6 +253,14 @@ public class FormAuthenticator
         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 @@ public class FormAuthenticator
         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 @@ public class FormAuthenticator
             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 @@ public class FormAuthenticator
         // 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;
         }


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