Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8268826: Cleanup Override in Context-Specific Deserialization Filters
Reviewed-by: dfuchs, bchristi
  • Loading branch information
Roger Riggs committed Jul 9, 2021
1 parent f791fdf commit 6889a39a3f124d2442584cb7646b2d6a18745e78
Showing 13 changed files with 211 additions and 103 deletions.
@@ -45,6 +45,7 @@
import static java.io.ObjectInputFilter.Status.*;
import static java.lang.System.Logger.Level.TRACE;
import static java.lang.System.Logger.Level.DEBUG;
import static java.lang.System.Logger.Level.ERROR;

/**
* Filter classes, array lengths, and graph metrics during deserialization.
@@ -308,10 +309,12 @@ public interface ObjectInputFilter {
* <li>Otherwise, return {@code otherStatus}.</li>
* </ul>
* <p>
* Example, to create a filter that will allow any class loaded from the platform classloader.
* Example, to create a filter that will allow any class loaded from the platform
* or bootstrap classloaders.
* <pre><code>
* ObjectInputFilter f = allowFilter(cl -> cl.getClassLoader() == ClassLoader.getPlatformClassLoader()
* || cl.getClassLoader() == null, Status.UNDECIDED);
* ObjectInputFilter f
* = allowFilter(cl -> cl.getClassLoader() == ClassLoader.getPlatformClassLoader() ||
* cl.getClassLoader() == null, Status.UNDECIDED);
* </code></pre>
*
* @param predicate a predicate to test a non-null Class
@@ -527,23 +530,17 @@ enum Status {
* The syntax for the property value is the same as for the
* {@link #createFilter(String) createFilter} method.
*
* <p> If only `jdk.serialFilter` is set and not `jdk.serialFilterFactory` the builtin
* filter factory, compatible with previous versions, is set and can not be replaced,
* see below to override the builtin filter factory.
* <p>
* If the Java virtual machine is started with the system property
* {@systemProperty jdk.serialFilterFactory} or the {@link java.security.Security} property
* of the same name, its value names the class to configure the JVM-wide deserialization
* filter factory or the special value `OVERRIDE`.
* filter factory.
* If the system property is not defined, and the {@link java.security.Security} property
* {@code jdk.serialFilterFactory} is defined then it is used to configure the filter factory.
*
* If the value is `OVERRIDE`, the filter factory can be set by the application before
* the first deserialization using {@link Config#setSerialFilterFactory(BinaryOperator)};
* If it remains unset, the filter factory is a builtin filter factory compatible
* with previous versions.
*
* <p>If not `OVERRIDE`, the class must be public, must have a public zero-argument constructor, implement the
* <p>The class must be public, must have a public zero-argument constructor, implement the
* {@link BinaryOperator {@literal BinaryOperator<ObjectInputFilter>}} interface, provide its implementation and
* be accessible via the {@linkplain ClassLoader#getSystemClassLoader() application class loader}.
* If the filter factory constructor is not invoked successfully, an {@link ExceptionInInitializerError}
@@ -579,11 +576,6 @@ final class Config {
*/
private static final String SERIAL_FILTER_FACTORY_PROPNAME = "jdk.serialFilterFactory";

/**
* The property name to enable tracing of filters.
*/
private static final String SERIAL_FILTER_TRACE_PROPNAME = "jdk.serialFilterTrace";

/**
* Current static filter.
*/
@@ -599,34 +591,30 @@ final class Config {
* Boolean to indicate that the filter factory can not be set or replaced.
* - an ObjectInputStream has already been created using the current filter factory
* - has been set on the command line
* - jdk.serialFilter is set and jdk.serialFilterFactory is unset, the builtin can not be replaced
* @see Config#setSerialFilterFactory(BinaryOperator)
*/
private static final AtomicBoolean filterFactoryNoReplace = new AtomicBoolean();

/**
* Debug: Logger
* Debug and Trace Logger
*/
private static final System.Logger configLog;

/**
* True when tracing of filters is enabled.
*/
private static final boolean traceFilters;

static {
/*
* Initialize the configuration containing the filter factory, static filter, and logger.
* <ul>
* <li>The logger is created.
* <li>The property 'jdk.serialFilter" is read, either as a system property or a security property,
* and if set, defines the configured static JVM-wide filter and is logged.
* <li>The property jdk.serialFilterFactory is read, either as a system property or a security property,
* and if set, defines the initial filter factory and is logged.
* <li>The property jdk.serialFilterTrace, is read, and if set enables tracing of filters.
* <li>If either property is defined or tracing is enabled, the logger is created.
* </ul>
*/

// Initialize the logger.
configLog = System.getLogger("java.io.serialization");

// Get the values of the system properties, if they are defined
String factoryClassName = StaticProperty.jdkSerialFilterFactory();
if (factoryClassName == null) {
@@ -642,12 +630,6 @@ final class Config {
Security.getProperty(SERIAL_FILTER_PROPNAME));
}

traceFilters = GetBooleanAction.privilegedGetProperty(SERIAL_FILTER_TRACE_PROPNAME);

// Initialize the logger if either filter factory or filter property is set
configLog = (filterString != null || factoryClassName != null || traceFilters)
? System.getLogger("java.io.serialization") : null;

// Initialize the static filter if the jdk.serialFilter is present
ObjectInputFilter filter = null;
if (filterString != null) {
@@ -656,53 +638,40 @@ final class Config {
try {
filter = createFilter(filterString);
} catch (RuntimeException re) {
configLog.log(System.Logger.Level.ERROR,
configLog.log(ERROR,
"Error configuring filter: {0}", re);
}
}
serialFilter = filter;

// Initialize the filter factory if the jdk.serialFilterFactory is defined
// otherwise use the builtin filter factory.
if (factoryClassName == null || "OVERRIDE".equals(factoryClassName)) {
if (factoryClassName == null) {
serialFilterFactory = new BuiltinFilterFactory();
if (serialFilter != null && factoryClassName == null) {
// Ensure backward compatibility, unless factory is explicitly allowed to override
// Do not allow factory to be overridden by Config.setSerialFilterFactory
filterFactoryNoReplace.set(true);
}

} else {
configLog.log(DEBUG,
"Creating deserialization filter factory for {0}", factoryClassName);
try {
// Load using the system class loader, the named class may be an application class.
// The static initialization of the class or constructor may create a race
// if either calls Config.setSerialFilterFactory; the command line configured
// Class should not be overridden.
// Cause Config.setSerialFilterFactory to throw {@link IllegalStateException}
// if Config.setSerialFilterFactory is called as a side effect of the
// static initialization of the class or constructor.
filterFactoryNoReplace.set(true);

Class<?> factoryClass = Class.forName(factoryClassName, true,
ClassLoader.getSystemClassLoader());
@SuppressWarnings("unchecked")
BinaryOperator<ObjectInputFilter> f =
BinaryOperator<ObjectInputFilter> factory =
(BinaryOperator<ObjectInputFilter>)
factoryClass.getConstructor().newInstance(new Object[0]);
if (serialFilterFactory != null) {
// Init cycle if Config.setSerialFilterFactory called from class initialization
configLog.log(System.Logger.Level.ERROR,
"FilterFactory provided on the command line can not be overridden");
// Do not continue if configuration not initialized
throw new ExceptionInInitializerError(
"FilterFactory provided on the command line can not be overridden");
}
serialFilterFactory = f;
filterFactoryNoReplace.set(true);
configLog.log(DEBUG,
"Creating deserialization filter factory for {0}", factoryClassName);
serialFilterFactory = factory;
} catch (RuntimeException | ClassNotFoundException | NoSuchMethodException |
IllegalAccessException | InstantiationException | InvocationTargetException ex) {
configLog.log(System.Logger.Level.ERROR,
"Error configuring filter factory", ex);
Throwable th = (ex instanceof InvocationTargetException ite) ? ite.getCause() : ex;
configLog.log(ERROR,
"Error configuring filter factory: {0}", (Object)th);
// Do not continue if configuration not initialized
throw new ExceptionInInitializerError(
"FilterFactory configuration: jdk.serialFilterFactory: " + ex.getMessage());
throw new ExceptionInInitializerError(th);
}
}
// Setup shared secrets for RegistryImpl to use.
@@ -719,9 +688,7 @@ private Config() {
* Logger for filter actions.
*/
private static void traceFilter(String msg, Object... args) {
if (traceFilters && configLog != null) {
configLog.log(TRACE, msg, args);
}
configLog.log(TRACE, msg, args);
}

/**
@@ -840,12 +807,14 @@ public static void setSerialFilterFactory(BinaryOperator<ObjectInputFilter> filt
if (sm != null) {
sm.checkPermission(ObjectStreamConstants.SERIAL_FILTER_PERMISSION);
}
if (serialFilterFactory == null)
throw new IllegalStateException("Serial filter factory initialization incomplete");
if (filterFactoryNoReplace.getAndSet(true)) {
throw new IllegalStateException("Cannot replace filter factory: " +
serialFilterFactory.getClass().getName());
final String msg = serialFilterFactory != null
? serialFilterFactory.getClass().getName()
: "initialization incomplete";
throw new IllegalStateException("Cannot replace filter factory: " + msg);
}
configLog.log(DEBUG,
"Setting deserialization filter factory to {0}", filterFactory.getClass().getName());
serialFilterFactory = filterFactory;
}

@@ -1163,7 +1132,7 @@ public Status checkInput(FilterInfo filterInfo) {
}
if (!checkComponentType) {
// As revised; do not check the component type for arrays
traceFilter("Pattern array class: {0}, filter: {1}", clazz, this);
traceFilter("Pattern filter array class: {0}, filter: {1}", clazz, this);
return Status.UNDECIDED;
}
do {
@@ -1174,7 +1143,7 @@ public Status checkInput(FilterInfo filterInfo) {

if (clazz.isPrimitive()) {
// Primitive types are undecided; let someone else decide
traceFilter("Pattern UNDECIDED, primitive class: {0}, filter: {1}", clazz, this);
traceFilter("Pattern filter UNDECIDED, primitive class: {0}, filter: {1}", clazz, this);
return UNDECIDED;
} else {
// Find any filter that allowed or rejected the class
@@ -1184,7 +1153,7 @@ public Status checkInput(FilterInfo filterInfo) {
.filter(p -> p != Status.UNDECIDED)
.findFirst();
Status s = status.orElse(Status.UNDECIDED);
traceFilter("Pattern {0}, class: {1}, filter: {2}", s, cl, this);
traceFilter("Pattern filter {0}, class: {1}, filter: {2}", s, cl, this);
return s;
}
}
@@ -1283,18 +1252,18 @@ private static class MergeFilter implements ObjectInputFilter {
public ObjectInputFilter.Status checkInput(FilterInfo info) {
Status firstStatus = Objects.requireNonNull(first.checkInput(info), "status");
if (REJECTED.equals(firstStatus)) {
traceFilter("MergeFilter REJECT first: {0}, filter: {1}",
traceFilter("MergeFilter REJECTED first: {0}, filter: {1}",
firstStatus, this);
return REJECTED;
}
Status secondStatus = Objects.requireNonNull(second.checkInput(info), "other status");
if (REJECTED.equals(secondStatus)) {
traceFilter("MergeFilter REJECT {0}, {1}, filter: {2}",
traceFilter("MergeFilter REJECTED {0}, {1}, filter: {2}",
firstStatus, secondStatus, this);
return REJECTED;
}
if (ALLOWED.equals(firstStatus) || ALLOWED.equals(secondStatus)) {
traceFilter("MergeFilter ALLOW either: {0}, {1}, filter: {2}",
traceFilter("MergeFilter ALLOWED either: {0}, {1}, filter: {2}",
firstStatus, secondStatus, this);
return ALLOWED;
}
@@ -1332,7 +1301,6 @@ public ObjectInputFilter.Status checkInput(FilterInfo info) {
Class<?> clazz = info.serialClass();
if (clazz == null || !UNDECIDED.equals(status))
return status;
status = REJECTED;
// Find the base component type
while (clazz.isArray()) {
clazz = clazz.getComponentType();
@@ -194,7 +194,7 @@ public static String jdkSerialFilter() {
* in this method. The caller of this method should take care to ensure
* that the returned property is not made accessible to untrusted code.</strong>
*
* @return the {@code user.name} system property
* @return the {@code jdk.serialFilterFactory} system property
*/
public static String jdkSerialFilterFactory() {
return JDK_SERIAL_FILTER_FACTORY;
@@ -980,14 +980,12 @@ jdk.xml.dsig.secureValidationPolicy=\


#
# Deserialization system-wide filter factory
# Deserialization JVM-wide filter factory
#
# A filter factory class name is used to configure the system-wide filter factory.
# The filter factory value "OVERRIDE" in combination with setting "jdk.serialFilter"
# indicates that the builtin filter factory can be overridden by the application.
# A filter factory class name is used to configure the JVM-wide filter factory.
# The class must be public, must have a public zero-argument constructor, implement the
# java.util.stream.BinaryOperator<ObjectInputFilter> interface, provide its implementation and
# be accessible via the application class loader.
# java.util.function.BinaryOperator<java.io.ObjectInputFilter> interface, provide its
# implementation and be accessible via the application class loader.
# A builtin filter factory is used if no filter factory is defined.
# See java.io.ObjectInputFilter.Config for more information.
#
@@ -997,7 +995,7 @@ jdk.xml.dsig.secureValidationPolicy=\
#jdk.serialFilterFactory=<classname>

#
# Deserialization system-wide filter
# Deserialization JVM-wide filter
#
# A filter, if configured, is used by the filter factory to provide the filter used by
# java.io.ObjectInputStream during deserialization to check the contents of the stream.
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -54,6 +54,7 @@ public class FilterWithSecurityManagerTest {
ObjectInputFilter filter;

@BeforeClass
@SuppressWarnings("removal")
public void setup() throws Exception {
setSecurityManager = System.getSecurityManager() != null;
Object toDeserialized = Long.MAX_VALUE;
@@ -65,7 +66,8 @@ public void setup() throws Exception {
* Test that setting process-wide filter is checked by security manager.
*/
@Test
public void testGlobalFilter() throws Exception {
@SuppressWarnings("removal")
public void testGlobalFilter() {
ObjectInputFilter global = ObjectInputFilter.Config.getSerialFilter();

try {
@@ -88,6 +90,7 @@ public void testGlobalFilter() throws Exception {
* Test that setting specific filter is checked by security manager.
*/
@Test(dependsOnMethods = { "testGlobalFilter" })
@SuppressWarnings("removal")
public void testSpecificFilter() throws Exception {
try (ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
ObjectInputStream ois = new ObjectInputStream(bais)) {
@@ -142,6 +142,7 @@ static void globalFilter() {
* If there is no security manager then setting it should work.
*/
@Test()
@SuppressWarnings("removal")
static void setGlobalFilter() {
SecurityManager sm = System.getSecurityManager();
ObjectInputFilter filter = new SerialFilterTest.Validator();

1 comment on commit 6889a39

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 6889a39 Jul 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.