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 2022/08/22 14:59:01 UTC
[tomcat] branch 8.5.x updated: Fix BZ 66120
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
The following commit(s) were added to refs/heads/8.5.x by this push:
new e1e55f954a Fix BZ 66120
e1e55f954a is described below
commit e1e55f954ab95239a9484d03ecc531abc7c6f75f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 22 15:12:46 2022 +0100
Fix BZ 66120
Replicate the session note required by FORM authentication. This enables
session failover / persistence+restoration to work correctly if it
occurs during the FORM authentication process.
The feature is disabled by default for backwards compatibility of the
serialized session data.
---
.../apache/catalina/ha/session/DeltaRequest.java | 24 +++++++
.../apache/catalina/ha/session/DeltaSession.java | 76 +++++++++++++++++++++-
java/org/apache/catalina/session/ManagerBase.java | 39 +++++++++++
.../apache/catalina/session/StandardSession.java | 50 ++++++++++++--
webapps/docs/changelog.xml | 9 +++
webapps/docs/config/cluster-manager.xml | 30 +++++++++
webapps/docs/config/manager.xml | 30 +++++++++
7 files changed, 250 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java b/java/org/apache/catalina/ha/session/DeltaRequest.java
index 4a5c65792f..1d1dd23e44 100644
--- a/java/org/apache/catalina/ha/session/DeltaRequest.java
+++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
@@ -53,6 +53,7 @@ public class DeltaRequest implements Externalizable {
public static final int TYPE_MAXINTERVAL = 3;
public static final int TYPE_AUTHTYPE = 4;
public static final int TYPE_LISTENER = 5;
+ public static final int TYPE_NOTE = 6;
public static final int ACTION_SET = 0;
public static final int ACTION_REMOVE = 1;
@@ -90,6 +91,15 @@ public class DeltaRequest implements Externalizable {
addAction(TYPE_ATTRIBUTE, ACTION_REMOVE, name, null);
}
+ public void setNote(String name, Object value) {
+ int action = (value == null) ? ACTION_REMOVE : ACTION_SET;
+ addAction(TYPE_NOTE, action, name, value);
+ }
+
+ public void removeNote(String name) {
+ addAction(TYPE_NOTE, ACTION_REMOVE, name, null);
+ }
+
public void setMaxInactiveInterval(int interval) {
addAction(TYPE_MAXINTERVAL, ACTION_SET, NAME_MAXINTERVAL, Integer.valueOf(interval));
}
@@ -216,6 +226,20 @@ public class DeltaRequest implements Externalizable {
} else {
session.removeSessionListener(listener, false);
}
+ break;
+ case TYPE_NOTE:
+ if (info.getAction() == ACTION_SET) {
+ if (log.isTraceEnabled()) {
+ log.trace("Session.setNote('" + info.getName() + "', '" + info.getValue() + "')");
+ }
+ session.setNote(info.getName(), info.getValue(), false);
+ } else {
+ if (log.isTraceEnabled()) {
+ log.trace("Session.removeNote('" + info.getName() + "')");
+ }
+ session.removeNote(info.getName(), false);
+ }
+
break;
default:
log.warn(sm.getString("deltaRequest.invalidAttributeInfoType", info));
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java
index 91725fd8b8..2b72a3ab36 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -808,6 +808,52 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
}
+ @Override
+ public void removeNote(String name) {
+ removeNote(name, true);
+ }
+
+ @SuppressWarnings("deprecation")
+ public void removeNote(String name, boolean addDeltaRequest) {
+ lockInternal();
+ try {
+ super.removeNote(name);
+ if (addDeltaRequest && manager instanceof ManagerBase &&
+ ((ManagerBase) manager).getPersistAuthenticationNotes()) {
+ deltaRequest.removeNote(name);
+ }
+ } finally {
+ unlockInternal();
+ }
+ }
+
+
+ @Override
+ public void setNote(String name, Object value) {
+ setNote(name, value, true);
+ }
+
+ @SuppressWarnings("deprecation")
+ public void setNote(String name, Object value, boolean addDeltaRequest) {
+
+ if (value == null) {
+ removeNote(name, addDeltaRequest);
+ return;
+ }
+
+ lockInternal();
+ try {
+ super.setNote(name, value);
+ if (addDeltaRequest && manager instanceof ManagerBase &&
+ ((ManagerBase) manager).getPersistAuthenticationNotes()) {
+ deltaRequest.setNote(name, value);
+ }
+ } finally {
+ unlockInternal();
+ }
+ }
+
+
// -------------------------------------------- HttpSession Private Methods
/**
@@ -852,11 +898,30 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
log.debug(sm.getString("deltaSession.readSession", id));
}
+ Object nextObject = stream.readObject();
+
+ // Compatibility with versions that do not persist the authentication
+ // notes
+ if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be
+ // 'expected session ID' and 'saved request'
+ if (nextObject != null) {
+ notes.put(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE, nextObject);
+ }
+ nextObject = stream.readObject();
+ if (nextObject != null) {
+ notes.put(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE, nextObject);
+ }
+
+ // Next object will be the number of attributes
+ nextObject = stream.readObject();
+ }
+
// Deserialize the attribute count and attribute values
if (attributes == null) {
attributes = new ConcurrentHashMap<>();
}
- int n = ( (Integer) stream.readObject()).intValue();
+ int n = ((Integer) nextObject).intValue();
boolean isValidSave = isValid;
isValid = true;
for (int i = 0; i < n; i++) {
@@ -936,6 +1001,7 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
doWriteObject((ObjectOutput)stream);
}
+ @SuppressWarnings("deprecation")
private void doWriteObject(ObjectOutput stream) throws IOException {
// Write the scalar instance variables (except Manager)
stream.writeObject(Long.valueOf(creationTime));
@@ -955,6 +1021,14 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus
log.debug(sm.getString("deltaSession.writeSession", id));
}
+ if (manager instanceof ManagerBase && ((ManagerBase) manager).getPersistAuthenticationNotes()) {
+ // Write the notes associated with authentication. Without these,
+ // authentication can fail without sticky sessions or if there is a
+ // fail-over during authentication.
+ stream.writeObject(notes.get(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE));
+ stream.writeObject(notes.get(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE));
+ }
+
// Accumulate the names of serializable and non-serializable attributes
String keys[] = keys();
List<String> saveNames = new ArrayList<>();
diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java
index 93c6216706..981f5996eb 100644
--- a/java/org/apache/catalina/session/ManagerBase.java
+++ b/java/org/apache/catalina/session/ManagerBase.java
@@ -199,6 +199,12 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager
*/
private boolean persistAuthentication = false;
+ /**
+ * Determines if the session notes required by the FORM authentication
+ * process are persisted (or replicated for clusters) by this Manager.
+ */
+ private boolean persistAuthenticationNotes = false;
+
// ------------------------------------------------------------ Constructors
@@ -548,6 +554,39 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager
}
+ /**
+ * Return whether sessions managed by this manager shall persist
+ * authentication notes used by FORM authentication or not.
+ *
+ * @return {@code true}, sessions managed by this manager shall persist
+ * authentication notes used by FORM authentication; {@code false}
+ * otherwise
+ *
+ * @deprecated Will be removed in Tomcat 10.1.x where it is effectively
+ * hard-coded to <code>true</code>
+ */
+ @Deprecated
+ public boolean getPersistAuthenticationNotes() {
+ return this.persistAuthenticationNotes;
+ }
+
+ /**
+ * Set whether sessions managed by this manager shall persist authentication
+ * notes used by FORM authentication or not.
+ *
+ * @param persistAuthentication if {@code true}, sessions managed by this
+ * manager shall persist authentication notes
+ * used by FORM authentication
+ *
+ * @deprecated Will be removed in Tomcat 10.1.x where it is effectively
+ * hard-coded to <code>true</code>
+ */
+ @Deprecated
+ public void setPersistAuthenticationNotes(boolean persistAuthenticationNotes) {
+ this.persistAuthenticationNotes = persistAuthenticationNotes;
+ }
+
+
// --------------------------------------------------------- Public Methods
/**
diff --git a/java/org/apache/catalina/session/StandardSession.java b/java/org/apache/catalina/session/StandardSession.java
index 2d1f29621a..3bf4c069bb 100644
--- a/java/org/apache/catalina/session/StandardSession.java
+++ b/java/org/apache/catalina/session/StandardSession.java
@@ -57,6 +57,7 @@ import org.apache.catalina.Session;
import org.apache.catalina.SessionEvent;
import org.apache.catalina.SessionListener;
import org.apache.catalina.TomcatPrincipal;
+import org.apache.catalina.authenticator.SavedRequest;
import org.apache.catalina.security.SecurityUtil;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.res.StringManager;
@@ -1569,10 +1570,23 @@ public class StandardSession implements HttpSession, Session, Serializable {
("readObject() loading session " + id);
}
- // The next object read could either be the number of attributes (Integer) or the session's
- // authType followed by a Principal object (not an Integer)
+ if (notes == null) {
+ notes = new Hashtable<>();
+ }
+ /*
+ * The next object read could either be the number of attributes
+ * (Integer) or if authentication information is persisted then:
+ * - authType (String) - always present
+ * - Principal object - always present
+ * - expected session ID - present if persistAuthenticationNotes == true
+ * - saved request - present if persistAuthenticationNotes == true
+ *
+ * Note: Some, all or none of the above objects may be null
+ */
Object nextObject = stream.readObject();
if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be authType and
+ // Principal
setAuthType((String) nextObject);
try {
setPrincipal((Principal) stream.readObject());
@@ -1585,8 +1599,22 @@ public class StandardSession implements HttpSession, Session, Serializable {
}
throw e;
}
- // After that, the next object read should be the number of attributes (Integer)
+
nextObject = stream.readObject();
+ if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be
+ // 'expected session ID' and 'saved request'
+ if (nextObject != null) {
+ notes.put(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE, nextObject);
+ }
+ nextObject = stream.readObject();
+ if (nextObject != null) {
+ notes.put(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE, nextObject);
+ }
+
+ // Next object will be the number of attributes
+ nextObject = stream.readObject();
+ }
}
// Deserialize the attribute count and attribute values
@@ -1633,10 +1661,6 @@ public class StandardSession implements HttpSession, Session, Serializable {
if (listeners == null) {
listeners = new ArrayList<>();
}
-
- if (notes == null) {
- notes = new Hashtable<>();
- }
}
@@ -1659,6 +1683,7 @@ public class StandardSession implements HttpSession, Session, Serializable {
*
* @exception IOException if an input/output error occurs
*/
+ @SuppressWarnings("deprecation")
protected void doWriteObject(ObjectOutputStream stream) throws IOException {
// Write the scalar instance variables (except Manager)
@@ -1677,6 +1702,8 @@ public class StandardSession implements HttpSession, Session, Serializable {
// Gather authentication information (if configured)
String sessionAuthType = null;
Principal sessionPrincipal = null;
+ String expectedSessionId = null;
+ SavedRequest savedRequest = null;
if (getPersistAuthentication()) {
sessionAuthType = getAuthType();
sessionPrincipal = getPrincipal();
@@ -1685,6 +1712,8 @@ public class StandardSession implements HttpSession, Session, Serializable {
manager.getContext().getLogger().warn(
sm.getString("standardSession.principalNotSerializable", id));
}
+ expectedSessionId = (String) notes.get(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE);
+ savedRequest = (SavedRequest) notes.get(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE);
}
// Write authentication information (may be null values)
@@ -1695,6 +1724,13 @@ public class StandardSession implements HttpSession, Session, Serializable {
manager.getContext().getLogger().warn(
sm.getString("standardSession.principalNotSerializable", id), e);
}
+ if (manager instanceof ManagerBase && ((ManagerBase) manager).getPersistAuthenticationNotes()) {
+ // Write the notes associated with authentication. Without these,
+ // authentication can fail if there is a persist/restore during
+ // FORM authentication
+ stream.writeObject(expectedSessionId);
+ stream.writeObject(savedRequest);
+ }
// Accumulate the names of serializable and non-serializable attributes
String keys[] = keys();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2925eeda67..582f35613c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -116,6 +116,11 @@
Improve handling of stack overflow errors when parsing SSI expressions.
(markt)
</fix>
+ <fix>
+ <bug>66120</bug>: Enable FORM authentication to work correctly if
+ session persistence and restoration occurs during the authentication
+ process. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
@@ -149,6 +154,10 @@
passed an unrecognised action type, a warning message will now be
logged. (markt)
</fix>
+ <fix>
+ <bug>66120</bug>: Enable FORM authentication to work correctly if
+ session failover occurs during the authentication process. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Other">
diff --git a/webapps/docs/config/cluster-manager.xml b/webapps/docs/config/cluster-manager.xml
index 7d742cbe5f..b4a6741cf9 100644
--- a/webapps/docs/config/cluster-manager.xml
+++ b/webapps/docs/config/cluster-manager.xml
@@ -141,6 +141,21 @@
Set to <code>true</code> if you wish to have container listeners notified
across Tomcat nodes in the cluster.
</attribute>
+ <attribute name="persistAuthenticationNotes" required="false">
+ <p>Should authentication notes (used during FORM authentication) be
+ included when sessions are replicated? If <code>true</code>, the
+ session's authentication notes (used during FORM authentication) are
+ replicated so that an in progress FORM authentication can continue if
+ failover occurs during FORM authentication. If not specified, the
+ default value of <code>false</code> will be used.</p>
+
+ <p>Please note that all nodes must be upgraded to at least 8.5.83 (the
+ version where this feature was introduced) before this feature is
+ enabled else session replication may fail.</p>
+
+ <p>This attribute has been removed for Tomcat 10.1.x onwards which
+ always replicates FORM authenticaton notes.</p>
+ </attribute>
<attribute name="stateTransferTimeout" required="false">
The time in seconds to wait for a session state transfer to complete
from another node when a node is starting up.
@@ -222,6 +237,21 @@
sessions where the current node is the primary node for the session are
considered active sessions.
</attribute>
+ <attribute name="persistAuthenticationNotes" required="false">
+ <p>Should authentication notes (used during FORM authentication) be
+ included when sessions are replicated? If <code>true</code>, the
+ session's authentication notes (used during FORM authentication) are
+ replicated so that an in progress FORM authentication can continue if
+ failover occurs during FORM authentication. If not specified, the
+ default value of <code>false</code> will be used.</p>
+
+ <p>Please note that all nodes must be upgraded to at least 8.5.83 (the
+ version where this feature was introduced) before this feature is
+ enabled else session replication may fail.</p>
+
+ <p>This attribute has been removed for Tomcat 10.1.x onwards which
+ always replicates FORM authenticaton notes.</p>
+ </attribute>
<attribute name="rpcTimeout" required="false">
Timeout for RPC message used for broadcast and transfer state from
another map.
diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index 6d314f9dc1..1890656e5b 100644
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -122,6 +122,17 @@
filter pattern in order to be restored.</p>
</attribute>
+ <attribute name="persistAuthenticationNotes" required="false">
+ <p>Should authentication notes (used during FORM authentication) be
+ included when session state is preserved across application restarts?
+ If <code>true</code>, the session's authentication notes are preserved
+ so that an in progress FORM authentication can continue after the
+ application has been restarted. If not specified, the default value of
+ <code>false</code> will be used.<br />See
+ <a href="#Persistence_Across_Restarts">Persistence Across Restarts</a>
+ for more information.</p>
+ </attribute>
+
<attribute name="processExpiresFrequency" required="false">
<p>Frequency of the session expiration, and related manager operations.
Manager operations will be done once for the specified amount of
@@ -264,6 +275,25 @@
filter pattern in order to be restored.</p>
</attribute>
+ <attribute name="persistAuthenticationNotes" required="false">
+ <p>Should authentication notes (used during FORM authentication) be
+ included when sessions are swapped out to persistent storage? If
+ <code>true</code>, the session's authentication notes (used during
+ FORM authentication) are preserved so that an in progress FORM
+ authentication can continue after being reloaded (swapped in) from
+ persistent storage. If not specified, the default value of
+ <code>false</code> will be used.</p>
+
+ <p>Please note that if it is possible that the session will be swapped
+ in to a different node to the node from which it was swapped out, all
+ nodes must be upgraded to at least 8.5.83 (the version where this
+ feature was introduced) before this feature is enabled else the swapping
+ in may fail.</p>
+
+ <p>This attribute has been removed for Tomcat 10.1.x onwards which
+ always persists FORM authenticaton notes.</p>
+ </attribute>
+
<attribute name="processExpiresFrequency" required="false">
<p>It is the same as described above for the
<code>org.apache.catalina.session.StandardManager</code> class.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org