This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new d69935a118 Fix BZ 66120 d69935a118 is described below commit d69935a11875769b765d44652bbedcc1f3c38356 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. --- .../apache/catalina/ha/session/DeltaRequest.java | 24 ++++++++ .../apache/catalina/ha/session/DeltaSession.java | 69 +++++++++++++++++++++- .../apache/catalina/session/StandardSession.java | 44 +++++++++++--- webapps/docs/changelog.xml | 9 +++ 4 files changed, 138 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..968f02272c 100644 --- a/java/org/apache/catalina/ha/session/DeltaSession.java +++ b/java/org/apache/catalina/ha/session/DeltaSession.java @@ -808,6 +808,48 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus } + @Override + public void removeNote(String name) { + removeNote(name, true); + } + + public void removeNote(String name, boolean addDeltaRequest) { + lockInternal(); + try { + super.removeNote(name); + if (addDeltaRequest) { + deltaRequest.removeNote(name); + } + } finally { + unlockInternal(); + } + } + + + @Override + public void setNote(String name, Object value) { + setNote(name, value, true); + } + + public void setNote(String name, Object value, boolean addDeltaRequest) { + + if (value == null) { + removeNote(name, addDeltaRequest); + return; + } + + lockInternal(); + try { + super.setNote(name, value); + if (addDeltaRequest) { + deltaRequest.setNote(name, value); + } + } finally { + unlockInternal(); + } + } + + // -------------------------------------------- HttpSession Private Methods /** @@ -852,11 +894,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++) { @@ -955,6 +1016,12 @@ public class DeltaSession extends StandardSession implements Externalizable,Clus log.debug(sm.getString("deltaSession.writeSession", id)); } + // 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/StandardSession.java b/java/org/apache/catalina/session/StandardSession.java index 545529f4b4..8525072e3c 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; @@ -1444,10 +1445,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 BZ 66120 is fixed + * - saved request - present if BZ 66120 is fixed + * + * 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()); @@ -1460,8 +1474,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 @@ -1508,10 +1536,6 @@ public class StandardSession implements HttpSession, Session, Serializable { if (listeners == null) { listeners = new ArrayList<>(); } - - if (notes == null) { - notes = new Hashtable<>(); - } } @@ -1552,6 +1576,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(); @@ -1560,6 +1586,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) @@ -1570,6 +1598,8 @@ public class StandardSession implements HttpSession, Session, Serializable { manager.getContext().getLogger().warn( sm.getString("standardSession.principalNotSerializable", id), e); } + 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 35de617224..d669f9be4b 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"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org