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/06 21:14:43 UTC
[tomcat] branch master updated (0499c74 -> 50ac62d)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 0499c74 Fix codec
new 14dea88 Clean-up. No functional change.
new e55e662 Clean-up prior to some refactoring.
new d237110 Refactor change of session ID to reduce duplicate code
new 1ecba14 Refactor so Principal is never cached in session with cache==false
new fd381e9 Harden the FORM authentication process
new 1949da1 Add an atomic method to rotate session ID and return new value. Use it.
new 50ac62d Update changelog
The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/catalina/Manager.java | 33 +++++++
.../catalina/authenticator/AuthenticatorBase.java | 36 ++++---
.../apache/catalina/authenticator/Constants.java | 48 ++++-----
.../catalina/authenticator/FormAuthenticator.java | 107 +++++++++------------
java/org/apache/catalina/connector/Request.java | 3 +-
java/org/apache/catalina/session/ManagerBase.java | 7 ++
webapps/docs/changelog.xml | 5 +
7 files changed, 130 insertions(+), 109 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/07: Clean-up. No functional change.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 14dea885579e275dc3633b2a0df328dff5beb653
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Dec 5 19:59:47 2019 +0000
Clean-up. No functional change.
---
.../apache/catalina/authenticator/Constants.java | 41 ++++++----------------
1 file changed, 10 insertions(+), 31 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/Constants.java b/java/org/apache/catalina/authenticator/Constants.java
index 452a4f0..69b6066 100644
--- a/java/org/apache/catalina/authenticator/Constants.java
+++ b/java/org/apache/catalina/authenticator/Constants.java
@@ -14,11 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
package org.apache.catalina.authenticator;
-
public class Constants {
// Authentication methods for login configuration
// Servlet spec schemes are defined in HttpServletRequest
@@ -33,17 +30,13 @@ public class Constants {
// SPNEGO authentication constants
public static final String KRB5_CONF_PROPERTY = "java.security.krb5.conf";
public static final String DEFAULT_KRB5_CONF = "conf/krb5.ini";
- public static final String JAAS_CONF_PROPERTY =
- "java.security.auth.login.config";
+ public static final String JAAS_CONF_PROPERTY = "java.security.auth.login.config";
public static final String DEFAULT_JAAS_CONF = "conf/jaas.conf";
- public static final String DEFAULT_LOGIN_MODULE_NAME =
- "com.sun.security.jgss.krb5.accept";
+ public static final String DEFAULT_LOGIN_MODULE_NAME = "com.sun.security.jgss.krb5.accept";
// Cookie name for single sign on support
- public static final String SINGLE_SIGN_ON_COOKIE =
- System.getProperty(
- "org.apache.catalina.authenticator.Constants.SSO_SESSION_COOKIE_NAME",
- "JSESSIONIDSSO");
+ public static final String SINGLE_SIGN_ON_COOKIE = System.getProperty(
+ "org.apache.catalina.authenticator.Constants.SSO_SESSION_COOKIE_NAME", "JSESSIONIDSSO");
// --------------------------------------------------------- Request Notes
@@ -52,17 +45,13 @@ public class Constants {
* The notes key to track the single-sign-on identity with which this
* request is associated.
*/
- public static final String REQ_SSOID_NOTE =
- "org.apache.catalina.request.SSOID";
+ public static final String REQ_SSOID_NOTE = "org.apache.catalina.request.SSOID";
-
- public static final String REQ_JASPIC_SUBJECT_NOTE =
- "org.apache.catalina.authenticator.jaspic.SUBJECT";
+ public static final String REQ_JASPIC_SUBJECT_NOTE = "org.apache.catalina.authenticator.jaspic.SUBJECT";
// ---------------------------------------------------------- Session Notes
-
/**
* If the <code>cache</code> property of our authenticator is set, and
* the current request is part of a session, authentication information
@@ -70,19 +59,15 @@ public class Constants {
* <code>Realm.authenticate()</code>, under the following keys:
*/
-
/**
* The notes key for the password used to authenticate this user.
*/
- public static final String SESS_PASSWORD_NOTE =
- "org.apache.catalina.session.PASSWORD";
-
+ public static final String SESS_PASSWORD_NOTE = "org.apache.catalina.session.PASSWORD";
/**
* The notes key for the username used to authenticate this user.
*/
- public static final String SESS_USERNAME_NOTE =
- "org.apache.catalina.session.USERNAME";
+ public static final String SESS_USERNAME_NOTE = "org.apache.catalina.session.USERNAME";
/**
@@ -90,20 +75,14 @@ public class Constants {
* cache required information prior to the completion of authentication.
*/
-
/**
* The previously authenticated principal (if caching is disabled).
*/
- public static final String FORM_PRINCIPAL_NOTE =
- "org.apache.catalina.authenticator.PRINCIPAL";
-
+ public static final String FORM_PRINCIPAL_NOTE = "org.apache.catalina.authenticator.PRINCIPAL";
/**
* The original request information, to which the user will be
* redirected if authentication succeeds.
*/
- public static final String FORM_REQUEST_NOTE =
- "org.apache.catalina.authenticator.REQUEST";
-
-
+ public static final String FORM_REQUEST_NOTE = "org.apache.catalina.authenticator.REQUEST";
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 07/07: Update changelog
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 50ac62d5c94414a31264479cf951eda4b4e86617
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Dec 6 21:14:06 2019 +0000
Update changelog
---
webapps/docs/changelog.xml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d5a71ba..6d904d4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -102,6 +102,11 @@
<update>
Moved server-side include (SSI) module into a separate JAR library. (schultz)
</update>
+ <fix>
+ Refactor FORM authentication to reduce duplicate code and to ensure that
+ the authenticated Principal is not cached in the session when caching is
+ disabled. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/07: Clean-up prior to some refactoring.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e55e662708996848e0709a1859289e4f562528d8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Dec 5 20:00:02 2019 +0000
Clean-up prior to some refactoring.
---
.../catalina/authenticator/FormAuthenticator.java | 57 ++++++++--------------
1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 1b54ddd..8f4268b 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -147,22 +147,17 @@ public class FormAuthenticator
if (log.isDebugEnabled()) {
log.debug("Checking for reauthenticate in session " + session);
}
- String username =
- (String) session.getNote(Constants.SESS_USERNAME_NOTE);
- String password =
- (String) session.getNote(Constants.SESS_PASSWORD_NOTE);
- if ((username != null) && (password != null)) {
+ String username = (String) session.getNote(Constants.SESS_USERNAME_NOTE);
+ String password = (String) session.getNote(Constants.SESS_PASSWORD_NOTE);
+ if (username != null && password != null) {
if (log.isDebugEnabled()) {
log.debug("Reauthenticating username '" + username + "'");
}
- principal =
- context.getRealm().authenticate(username, password);
+ principal = context.getRealm().authenticate(username, password);
if (principal != null) {
session.setNote(Constants.FORM_PRINCIPAL_NOTE, principal);
if (!matchRequest(request)) {
- register(request, response, principal,
- HttpServletRequest.FORM_AUTH,
- username, password);
+ register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password);
return true;
}
}
@@ -177,16 +172,13 @@ public class FormAuthenticator
if (matchRequest(request)) {
session = request.getSessionInternal(true);
if (log.isDebugEnabled()) {
- log.debug("Restore request from session '"
- + session.getIdInternal()
- + "'");
+ log.debug("Restore request from session '" + session.getIdInternal() + "'");
}
- principal = (Principal)
- session.getNote(Constants.FORM_PRINCIPAL_NOTE);
+ principal = (Principal) session.getNote(Constants.FORM_PRINCIPAL_NOTE);
register(request, response, principal, HttpServletRequest.FORM_AUTH,
(String) session.getNote(Constants.SESS_USERNAME_NOTE),
(String) session.getNote(Constants.SESS_PASSWORD_NOTE));
- // If we're caching principals we no longer need the username
+ // If we're caching principals we no longer need the user name
// and password in the session, so remove them
if (cache) {
session.removeNote(Constants.SESS_USERNAME_NOTE);
@@ -211,9 +203,7 @@ public class FormAuthenticator
String requestURI = request.getDecodedRequestURI();
// Is this the action request from the login page?
- boolean loginAction =
- requestURI.startsWith(contextPath) &&
- requestURI.endsWith(Constants.FORM_ACTION);
+ boolean loginAction = requestURI.startsWith(contextPath) && requestURI.endsWith(Constants.FORM_ACTION);
LoginConfig config = context.getLoginConfig();
@@ -241,8 +231,7 @@ public class FormAuthenticator
saveRequest(request, session);
} catch (IOException ioe) {
log.debug("Request body too big to save during authentication");
- response.sendError(HttpServletResponse.SC_FORBIDDEN,
- sm.getString("authenticator.requestBodyTooBig"));
+ response.sendError(HttpServletResponse.SC_FORBIDDEN, sm.getString("authenticator.requestBodyTooBig"));
return false;
}
forwardToLoginPage(request, response, config);
@@ -276,12 +265,11 @@ public class FormAuthenticator
}
if (session == null) {
if (containerLog.isDebugEnabled()) {
- containerLog.debug
- ("User took so long to log on the session expired");
+ containerLog.debug("User took so long to log on the session expired");
}
if (landingPage == null) {
- response.sendError(HttpServletResponse.SC_REQUEST_TIMEOUT,
- sm.getString("authenticator.sessionExpired"));
+ response.sendError(
+ HttpServletResponse.SC_REQUEST_TIMEOUT, sm.getString("authenticator.sessionExpired"));
} else {
// Make the authenticator think the user originally requested
// the landing page
@@ -290,8 +278,7 @@ public class FormAuthenticator
saved.setMethod("GET");
saved.setRequestURI(uri);
saved.setDecodedRequestURI(uri);
- request.getSessionInternal(true).setNote(
- Constants.FORM_REQUEST_NOTE, saved);
+ request.getSessionInternal(true).setNote(Constants.FORM_REQUEST_NOTE, saved);
response.sendRedirect(response.encodeRedirectURL(uri));
}
return false;
@@ -312,8 +299,7 @@ public class FormAuthenticator
}
if (requestURI == null) {
if (landingPage == null) {
- response.sendError(HttpServletResponse.SC_BAD_REQUEST,
- sm.getString("authenticator.formlogin"));
+ response.sendError(HttpServletResponse.SC_BAD_REQUEST, sm.getString("authenticator.formlogin"));
} else {
// Make the authenticator think the user originally requested
// the landing page
@@ -331,15 +317,12 @@ public class FormAuthenticator
Response internalResponse = request.getResponse();
String location = response.encodeRedirectURL(requestURI);
if ("HTTP/1.1".equals(request.getProtocol())) {
- internalResponse.sendRedirect(location,
- HttpServletResponse.SC_SEE_OTHER);
+ internalResponse.sendRedirect(location, HttpServletResponse.SC_SEE_OTHER);
} else {
- internalResponse.sendRedirect(location,
- HttpServletResponse.SC_FOUND);
+ internalResponse.sendRedirect(location, HttpServletResponse.SC_FOUND);
}
}
return false;
-
}
@@ -503,8 +486,7 @@ public class FormAuthenticator
}
// Is there a saved request?
- SavedRequest sreq =
- (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
+ SavedRequest sreq = (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
if (sreq == null) {
return false;
}
@@ -538,8 +520,7 @@ public class FormAuthenticator
throws IOException {
// Retrieve and remove the SavedRequest object from our session
- SavedRequest saved = (SavedRequest)
- session.getNote(Constants.FORM_REQUEST_NOTE);
+ SavedRequest saved = (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
session.removeNote(Constants.FORM_REQUEST_NOTE);
session.removeNote(Constants.FORM_PRINCIPAL_NOTE);
if (saved == null) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/07: Refactor change of session ID to reduce duplicate
code
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d237110e056984989a3b64ade86ef9d88fef0db5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Dec 5 23:11:03 2019 +0000
Refactor change of session ID to reduce duplicate code
---
.../catalina/authenticator/AuthenticatorBase.java | 29 ++++++++++++----------
.../catalina/authenticator/FormAuthenticator.java | 5 +---
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index b6f2c9e..b644934 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -46,7 +46,6 @@ import org.apache.catalina.Container;
import org.apache.catalina.Context;
import org.apache.catalina.Globals;
import org.apache.catalina.LifecycleException;
-import org.apache.catalina.Manager;
import org.apache.catalina.Realm;
import org.apache.catalina.Session;
import org.apache.catalina.TomcatPrincipal;
@@ -1126,18 +1125,8 @@ public abstract class AuthenticatorBase extends ValveBase
if (session != null) {
// If the principal is null then this is a logout. No need to change
// the session ID. See BZ 59043.
- if (changeSessionIdOnAuthentication && principal != null) {
- String oldId = null;
- if (log.isDebugEnabled()) {
- oldId = session.getId();
- }
- Manager manager = request.getContext().getManager();
- manager.changeSessionId(session);
- request.changeSessionId(session.getId());
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("authenticator.changeSessionId",
- oldId, session.getId()));
- }
+ if (getChangeSessionIdOnAuthentication() && principal != null) {
+ changeSessionID(request, session);
}
} else if (alwaysUseSession) {
session = request.getSessionInternal(true);
@@ -1224,6 +1213,20 @@ public abstract class AuthenticatorBase extends ValveBase
}
+
+ protected String changeSessionID(Request request, Session session) {
+ String oldId = null;
+ if (log.isDebugEnabled()) {
+ oldId = session.getId();
+ }
+ String newId = request.changeSessionId();
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("authenticator.changeSessionId", oldId, newId));
+ }
+ return newId;
+ }
+
+
@Override
public void login(String username, String password, Request request) throws ServletException {
Principal principal = doLogin(request, username, password);
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 8f4268b..9d5e3f8 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -28,7 +28,6 @@ import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.apache.catalina.Manager;
import org.apache.catalina.Realm;
import org.apache.catalina.Session;
import org.apache.catalina.connector.Request;
@@ -397,9 +396,7 @@ public class FormAuthenticator
if (getChangeSessionIdOnAuthentication()) {
Session session = request.getSessionInternal(false);
if (session != null) {
- Manager manager = request.getContext().getManager();
- manager.changeSessionId(session);
- request.changeSessionId(session.getId());
+ changeSessionID(request, session);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 06/07: Add an atomic method to rotate session ID and
return new value. Use it.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1949da1cf5e6be10c8e39572a701fef217fa99f1
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Dec 6 12:13:15 2019 +0000
Add an atomic method to rotate session ID and return new value. Use it.
---
java/org/apache/catalina/Manager.java | 33 +++++++++++++++++++++++
java/org/apache/catalina/connector/Request.java | 3 +--
java/org/apache/catalina/session/ManagerBase.java | 7 +++++
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/Manager.java b/java/org/apache/catalina/Manager.java
index ac9b8fb..86b47e5 100644
--- a/java/org/apache/catalina/Manager.java
+++ b/java/org/apache/catalina/Manager.java
@@ -215,11 +215,44 @@ public interface Manager {
* session ID.
*
* @param session The session to change the session ID for
+ *
+ * @deprecated Use {@link #rotateSessionId(Session)}.
+ * Will be removed in Tomcat 10
*/
+ @Deprecated
public void changeSessionId(Session session);
/**
+ * 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
+ *
+ * @return The new session ID
+ */
+ public default String rotateSessionId(Session session) {
+ String newSessionId = null;
+ // Assume there new Id is a duplicate until we prove it isn't. The
+ // chances of a duplicate are extremely low but the current ManagerBase
+ // code protects against duplicates so this default method does too.
+ boolean duplicate = true;
+ do {
+ newSessionId = getSessionIdGenerator().generateSessionId();
+ try {
+ if (findSession(newSessionId) == null) {
+ duplicate = false;
+ }
+ } catch (IOException ioe) {
+ // Swallow. An IOE means the ID was known so continue looping
+ }
+ } while (duplicate);
+ changeSessionId(session, newSessionId);
+ return newSessionId;
+ }
+
+
+ /**
* Change the session ID of the current session to a specified session ID.
*
* @param session The session to change the session ID for
diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index 7cd30f7..8608276 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -2675,9 +2675,8 @@ public class Request implements HttpServletRequest {
}
Manager manager = this.getContext().getManager();
- manager.changeSessionId(session);
- String newSessionId = session.getId();
+ String newSessionId = manager.rotateSessionId(session);
this.changeSessionId(newSessionId);
return newSessionId;
diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java
index de6ae81..5e769c8 100644
--- a/java/org/apache/catalina/session/ManagerBase.java
+++ b/java/org/apache/catalina/session/ManagerBase.java
@@ -753,8 +753,15 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager
@Override
public void changeSessionId(Session session) {
+ rotateSessionId(session);
+ }
+
+
+ @Override
+ public String rotateSessionId(Session session) {
String newId = generateSessionId();
changeSessionId(session, newId, true, true);
+ return newId;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/07: Refactor so Principal is never cached in session
with cache==false
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1ecba14e690cf5f3f143eef6ae7037a6d3c16652
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Dec 5 23:25:37 2019 +0000
Refactor so Principal is never cached in session with cache==false
---
.../catalina/authenticator/AuthenticatorBase.java | 5 ++--
.../apache/catalina/authenticator/Constants.java | 3 ++
.../catalina/authenticator/FormAuthenticator.java | 33 ++++++----------------
3 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index b644934..0b63fd9 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -1133,10 +1133,11 @@ public abstract class AuthenticatorBase extends ValveBase
}
// Cache the authentication information in our session, if any
- if (cache) {
- if (session != null) {
+ if (session != null) {
+ if (cache) {
session.setAuthType(authType);
session.setPrincipal(principal);
+ } else {
if (username != null) {
session.setNote(Constants.SESS_USERNAME_NOTE, username);
} else {
diff --git a/java/org/apache/catalina/authenticator/Constants.java b/java/org/apache/catalina/authenticator/Constants.java
index 69b6066..9857c09 100644
--- a/java/org/apache/catalina/authenticator/Constants.java
+++ b/java/org/apache/catalina/authenticator/Constants.java
@@ -77,7 +77,10 @@ public class Constants {
/**
* The previously authenticated principal (if caching is disabled).
+ *
+ * @deprecated Unused. Will be removed in Tomcat 10.
*/
+ @Deprecated
public static final String FORM_PRINCIPAL_NOTE = "org.apache.catalina.authenticator.PRINCIPAL";
/**
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 9d5e3f8..f326f77 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -132,10 +132,6 @@ public class FormAuthenticator
protected boolean doAuthenticate(Request request, HttpServletResponse response)
throws IOException {
- if (checkForCachedAuthentication(request, response, true)) {
- return true;
- }
-
// References to objects we will need later
Session session = null;
Principal principal = null;
@@ -154,9 +150,8 @@ public class FormAuthenticator
}
principal = context.getRealm().authenticate(username, password);
if (principal != null) {
- session.setNote(Constants.FORM_PRINCIPAL_NOTE, principal);
+ register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password);
if (!matchRequest(request)) {
- register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password);
return true;
}
}
@@ -173,16 +168,6 @@ public class FormAuthenticator
if (log.isDebugEnabled()) {
log.debug("Restore request from session '" + session.getIdInternal() + "'");
}
- principal = (Principal) session.getNote(Constants.FORM_PRINCIPAL_NOTE);
- register(request, response, principal, HttpServletRequest.FORM_AUTH,
- (String) session.getNote(Constants.SESS_USERNAME_NOTE),
- (String) session.getNote(Constants.SESS_PASSWORD_NOTE));
- // If we're caching principals we no longer need the user name
- // and password in the session, so remove them
- if (cache) {
- session.removeNote(Constants.SESS_USERNAME_NOTE);
- session.removeNote(Constants.SESS_PASSWORD_NOTE);
- }
if (restoreRequest(request, session)) {
if (log.isDebugEnabled()) {
log.debug("Proceed to restored request");
@@ -197,6 +182,12 @@ public class FormAuthenticator
}
}
+ // This check has to be after the previous check for a matching request
+ // because that matching request may also include a cached Principal.
+ if (checkForCachedAuthentication(request, response, true)) {
+ return true;
+ }
+
// Acquire references to objects we will need to evaluate
String contextPath = request.getContextPath();
String requestURI = request.getDecodedRequestURI();
@@ -283,12 +274,7 @@ public class FormAuthenticator
return false;
}
- // Save the authenticated Principal in our session
- session.setNote(Constants.FORM_PRINCIPAL_NOTE, principal);
-
- // Save the username and password as well
- session.setNote(Constants.SESS_USERNAME_NOTE, username);
- session.setNote(Constants.SESS_PASSWORD_NOTE, password);
+ register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password);
// Redirect the user to the original request URI (which will cause
// the original request to be restored)
@@ -489,7 +475,7 @@ public class FormAuthenticator
}
// Is there a saved principal?
- if (session.getNote(Constants.FORM_PRINCIPAL_NOTE) == null) {
+ if (cache && session.getPrincipal() == null || !cache && request.getPrincipal() == null) {
return false;
}
@@ -519,7 +505,6 @@ 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.FORM_PRINCIPAL_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
[tomcat] 05/07: Harden the FORM authentication process
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fd381e94f222831fd2bee697deb6246d417b8f33
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 0b63fd9..d63d652 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -1126,7 +1126,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 9857c09..a8e9a47 100644
--- a/java/org/apache/catalina/authenticator/Constants.java
+++ b/java/org/apache/catalina/authenticator/Constants.java
@@ -53,6 +53,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