This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push: new c2493ab782 Fix BZ 66120 c2493ab782 is described below commit c2493ab782f1e4d4190b2ea2a6931373a1f502be 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 8e4e803f79..21d411936d 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 d142701d43..a6070a6c34 100644 --- a/java/org/apache/catalina/session/ManagerBase.java +++ b/java/org/apache/catalina/session/ManagerBase.java @@ -204,6 +204,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; + private boolean sessionActivityCheck = Globals.STRICT_SERVLET_COMPLIANCE; private boolean sessionLastAccessAtStart = Globals.STRICT_SERVLET_COMPLIANCE; @@ -605,6 +611,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 db32b3231a..439f71bb41 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; @@ -1565,10 +1566,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()); @@ -1581,8 +1595,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 @@ -1629,10 +1657,6 @@ public class StandardSession implements HttpSession, Session, Serializable { if (listeners == null) { listeners = new ArrayList<>(); } - - if (notes == null) { - notes = new Hashtable<>(); - } } @@ -1655,6 +1679,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) @@ -1673,6 +1698,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(); @@ -1681,6 +1708,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) @@ -1691,6 +1720,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 183597493d..db270bcba7 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -137,6 +137,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"> @@ -195,6 +200,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..85740da5a0 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 10.0.24 (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 10.0.24 (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 93489f8f9c..5747447e14 100644 --- a/webapps/docs/config/manager.xml +++ b/webapps/docs/config/manager.xml @@ -159,6 +159,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 @@ -301,6 +312,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 10.0.24 (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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org