diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java index 0e6b97f80..99f6cc04a 100644 --- a/java/org/apache/catalina/session/ManagerBase.java +++ b/java/org/apache/catalina/session/ManagerBase.java @@ -37,6 +37,7 @@ import org.apache.catalina.Container; import org.apache.catalina.Context; import org.apache.catalina.Engine; +import org.apache.catalina.Globals; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleState; @@ -226,8 +227,31 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager private boolean warnOnSessionAttributeFilterFailure; - // ------------------------------------------------------------- Properties + // ------------------------------------------------------------ Constructors + public ManagerBase() { + if (Globals.IS_SECURITY_ENABLED) { + // Minimum set required for default distribution/persistence to work + // plus String + setSessionAttributeValueClassNameFilter( + "java\\.lang\\.(?:Boolean|Integer|Long|Number|String)"); + setWarnOnSessionAttributeFilterFailure(true); + } + } + + + // -------------------------------------------------------------- Properties + + /** + * Obtain the regular expression used to filter session attribute based on + * attribute name. The regular expression is anchored so it must match the + * entire name + * + * @return The regular expression currently used to filter attribute names. + * {@code null} means no filter is applied. If an empty string is + * specified then no names will match the filter and all attributes + * will be blocked. + */ public String getSessionAttributeNameFilter() { if (sessionAttributeNamePattern == null) { return null; diff --git a/java/org/apache/catalina/session/StandardManager.java b/java/org/apache/catalina/session/StandardManager.java index 1df2b581e..936f30f33 100644 --- a/java/org/apache/catalina/session/StandardManager.java +++ b/java/org/apache/catalina/session/StandardManager.java @@ -191,10 +191,12 @@ protected void doLoad() throws ClassNotFoundException, IOException { } Loader loader = null; ClassLoader classLoader = null; + Log logger = null; try (FileInputStream fis = new FileInputStream(file.getAbsolutePath()); - BufferedInputStream bis = new BufferedInputStream(fis);) { + BufferedInputStream bis = new BufferedInputStream(fis)) { Context c = getContext(); loader = c.getLoader(); + logger = c.getLogger(); if (loader != null) { classLoader = loader.getClassLoader(); } @@ -204,7 +206,9 @@ protected void doLoad() throws ClassNotFoundException, IOException { // Load the previously unloaded active sessions synchronized (sessions) { - try (ObjectInputStream ois = new CustomObjectInputStream(bis, classLoader)) { + try (ObjectInputStream ois = new CustomObjectInputStream(bis, classLoader, logger, + getSessionAttributeValueClassNamePattern(), + getWarnOnSessionAttributeFilterFailure())) { Integer count = (Integer) ois.readObject(); int n = count.intValue(); if (log.isDebugEnabled()) diff --git a/java/org/apache/catalina/session/StoreBase.java b/java/org/apache/catalina/session/StoreBase.java index 679ac983b..09c88e01d 100644 --- a/java/org/apache/catalina/session/StoreBase.java +++ b/java/org/apache/catalina/session/StoreBase.java @@ -215,7 +215,20 @@ public void processExpires() { */ protected ObjectInputStream getObjectInputStream(InputStream is) throws IOException { BufferedInputStream bis = new BufferedInputStream(is); - return new CustomObjectInputStream(bis, Thread.currentThread().getContextClassLoader()); + + CustomObjectInputStream ois; + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + + if (manager instanceof ManagerBase) { + ManagerBase managerBase = (ManagerBase) manager; + ois = new CustomObjectInputStream(bis, classLoader, manager.getContext().getLogger(), + managerBase.getSessionAttributeValueClassNamePattern(), + managerBase.getWarnOnSessionAttributeFilterFailure()); + } else { + ois = new CustomObjectInputStream(bis, classLoader); + } + + return ois; } diff --git a/java/org/apache/catalina/util/CustomObjectInputStream.java b/java/org/apache/catalina/util/CustomObjectInputStream.java index b6ec5f88a..5d1fb58fa 100644 --- a/java/org/apache/catalina/util/CustomObjectInputStream.java +++ b/java/org/apache/catalina/util/CustomObjectInputStream.java @@ -18,9 +18,18 @@ import java.io.IOException; import java.io.InputStream; +import java.io.InvalidClassException; import java.io.ObjectInputStream; import java.io.ObjectStreamClass; import java.lang.reflect.Proxy; +import java.util.Collections; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +import org.apache.juli.logging.Log; +import org.apache.tomcat.util.res.StringManager; /** * Custom subclass of ObjectInputStream that loads from the @@ -32,14 +41,26 @@ */ public final class CustomObjectInputStream extends ObjectInputStream { + private static final StringManager sm = StringManager.getManager(CustomObjectInputStream.class); + + private static final WeakHashMap> reportedClassCache = + new WeakHashMap<>(); + /** * The class loader we will use to resolve classes. */ private final ClassLoader classLoader; + private final Set reportedClasses; + private final Log log; + + private final Pattern allowedClassNamePattern; + private final String allowedClassNameFilter; + private final boolean warnOnFailure; /** - * Construct a new instance of CustomObjectInputStream + * Construct a new instance of CustomObjectInputStream without any filtering + * of deserialized classes. * * @param stream The input stream we will read from * @param classLoader The class loader used to instantiate objects @@ -47,8 +68,54 @@ public final class CustomObjectInputStream extends ObjectInputStream { * @exception IOException if an input/output error occurs */ public CustomObjectInputStream(InputStream stream, ClassLoader classLoader) throws IOException { + this(stream, classLoader, null, null, false); + } + + + /** + * Construct a new instance of CustomObjectInputStream with filtering of + * deserialized classes. + * + * @param stream The input stream we will read from + * @param classLoader The class loader used to instantiate objects + * @param log The logger to use to report any issues. It may only be null if + * the filterMode does not require logging + * @param allowedClassNamePattern The regular expression to use to filter + * deserialized classes. The fully qualified + * class name must match this pattern for + * deserialization to be allowed if filtering + * is enabled. + * @param warnOnFailure Should any failures be logged? + * + * @exception IOException if an input/output error occurs + */ + public CustomObjectInputStream(InputStream stream, ClassLoader classLoader, + Log log, Pattern allowedClassNamePattern, boolean warnOnFailure) + throws IOException { super(stream); + if (log == null && allowedClassNamePattern != null && warnOnFailure) { + throw new IllegalArgumentException( + sm.getString("customObjectInputStream.logRequired")); + } this.classLoader = classLoader; + this.log = log; + this.allowedClassNamePattern = allowedClassNamePattern; + if (allowedClassNamePattern == null) { + this.allowedClassNameFilter = null; + } else { + this.allowedClassNameFilter = allowedClassNamePattern.toString(); + } + this.warnOnFailure = warnOnFailure; + + Set reportedClasses; + synchronized (reportedClassCache) { + reportedClasses = reportedClassCache.get(classLoader); + if (reportedClasses == null) { + reportedClasses = Collections.newSetFromMap(new ConcurrentHashMap()); + reportedClassCache.put(classLoader, reportedClasses); + } + } + this.reportedClasses = reportedClasses; } @@ -64,8 +131,24 @@ public CustomObjectInputStream(InputStream stream, ClassLoader classLoader) thro @Override public Class resolveClass(ObjectStreamClass classDesc) throws ClassNotFoundException, IOException { + + String name = classDesc.getName(); + if (allowedClassNamePattern != null) { + boolean allowed = allowedClassNamePattern.matcher(name).matches(); + if (!allowed) { + boolean doLog = warnOnFailure && reportedClasses.add(name); + String msg = sm.getString("customObjectInputStream.nomatch", name, allowedClassNameFilter); + if (doLog) { + log.warn(msg); + } else if (log.isDebugEnabled()) { + log.debug(msg); + } + throw new InvalidClassException(msg); + } + } + try { - return Class.forName(classDesc.getName(), false, classLoader); + return Class.forName(name, false, classLoader); } catch (ClassNotFoundException e) { try { // Try also the superclass because of primitive types diff --git a/java/org/apache/catalina/util/LocalStrings.properties b/java/org/apache/catalina/util/LocalStrings.properties index 55dea989b..6aeb97314 100644 --- a/java/org/apache/catalina/util/LocalStrings.properties +++ b/java/org/apache/catalina/util/LocalStrings.properties @@ -17,6 +17,8 @@ parameterMap.locked=No modifications are allowed to a locked ParameterMap resourceSet.locked=No modifications are allowed to a locked ResourceSet hexUtil.bad=Bad hexadecimal digit hexUtil.odd=Odd number of hexadecimal digits +customObjectInputStream.logRequired=A valid logger is required for class name filtering with logging +customObjectInputStream.nomatch=The class [{0}] did not match the regular expression [{1}] for classes allowed to be deserialized #Default Messages Utilized by the ExtensionValidator extensionValidator.web-application-manifest=Web Application Manifest extensionValidator.extension-not-found-error=ExtensionValidator[{0}][{1}]: Required extension [{2}] not found. diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 7938c2f38..3395d5405 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -166,7 +166,8 @@ based on the implementation class of the value and optional WARN level logging if an attribute is filtered. These options are avaialble for all of the Manager implementations that ship - with Tomcat. (markt) + with Tomcat. When a SecurityManager is used filtering will + be enabled by default. (markt) diff --git a/webapps/docs/config/cluster-manager.xml b/webapps/docs/config/cluster-manager.xml index e0a7a9ddc..6ebe69efc 100644 --- a/webapps/docs/config/cluster-manager.xml +++ b/webapps/docs/config/cluster-manager.xml @@ -198,7 +198,9 @@ length or null, all attributes are eligible for replication. The pattern is anchored so the fully qualified class name must fully match the pattern. If not specified, the default value of - null will be used.

+ null will be used unless a SecurityManager is + enabled in which case the default will be + java\\.lang\\.(?:Boolean|Integer|Long|Number|String).

When this node sends a GET_ALL_SESSIONS message to other @@ -217,7 +219,8 @@ attribute, should this be logged at WARN level? If WARN level logging is disabled then it will be logged at DEBUG. The default value of this attribute is - false.

+ false unless a SecurityManager is enabled in + which case the default will be true.

@@ -260,7 +263,9 @@ length or null, all attributes are eligible for replication. The pattern is anchored so the fully qualified class name must fully match the pattern. If not specified, the default value of - null will be used.

+ null will be used unless a SecurityManager is + enabled in which case the default will be + java\\.lang\\.(?:Boolean|Integer|Long|Number|String).

Set to true if you wish to terminate replication map when replication @@ -275,7 +280,8 @@ attribute, should this be logged at WARN level? If WARN level logging is disabled then it will be logged at DEBUG. The default value of this attribute is - false.

+ false unless a SecurityManager is enabled in + which case the default will be true.

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml index 006d94fff..16facaec4 100644 --- a/webapps/docs/config/manager.xml +++ b/webapps/docs/config/manager.xml @@ -198,7 +198,9 @@ length or null, all attributes are eligible for distribution. The pattern is anchored so the fully qualified class name must fully match the pattern. If not specified, the default value of - null will be used.

+ null will be used unless a SecurityManager is + enabled in which case the default will be + java\\.lang\\.(?:Boolean|Integer|Long|Number|String).

@@ -207,7 +209,8 @@ attribute, should this be logged at WARN level? If WARN level logging is disabled then it will be logged at DEBUG. The default value of this attribute is - false.

+ false unless a SecurityManager is enabled in + which case the default will be true.

@@ -318,7 +321,9 @@ length or null, all attributes are eligible for distribution. The pattern is anchored so the fully qualified class name must fully match the pattern. If not specified, the default value of - null will be used.

+ null will be used unless a SecurityManager is + enabled in which case the default will be + java\\.lang\\.(?:Boolean|Integer|Long|Number|String).

@@ -327,7 +332,8 @@ attribute, should this be logged at WARN level? If WARN level logging is disabled then it will be logged at DEBUG. The default value of this attribute is - false.

+ false unless a SecurityManager is enabled in + which case the default will be true.