You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/03/02 19:23:40 UTC
[geode] branch develop updated: GEODE-4672 Geode fails to start
with JDK 9 if validate-serializable-objects is set
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 94ded7f GEODE-4672 Geode fails to start with JDK 9 if validate-serializable-objects is set
94ded7f is described below
commit 94ded7f3561a9ec4848843e3841d593fcd4d77d9
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Fri Mar 2 11:21:39 2018 -0800
GEODE-4672 Geode fails to start with JDK 9 if validate-serializable-objects is set
The ObjectInputFilter wrapper now uses reflection and a dynamic
proxy to interact with the serialization filter classes. A dynamic
proxy is needed since we have to implement a method specified in
a class whose package has changed in the JDK. The logic of the filter
has not been changed.
I also replaced a couple of recently added white-list wildcards with specific
classes and added a javadoc for the whitelist in InternalDataSerializer.
I opened a new JIRA ticket concerning another wildcard that was recently
added for the JDBC connector and left a TODO in place for this.
This closes #1526
---
.../apache/geode/internal/InputStreamFilter.java | 3 +
.../geode/internal/InternalDataSerializer.java | 68 +++++--
.../internal/ObjectInputStreamFilterWrapper.java | 196 ++++++++++++++++++---
...alDataSerializerSerializationWhitelistTest.java | 9 +-
4 files changed, 229 insertions(+), 47 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/InputStreamFilter.java b/geode-core/src/main/java/org/apache/geode/internal/InputStreamFilter.java
index 19d4102..43b43e6 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/InputStreamFilter.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/InputStreamFilter.java
@@ -17,5 +17,8 @@ package org.apache.geode.internal;
import java.io.ObjectInputStream;
public interface InputStreamFilter {
+
+ /** establish a serialization filter on the given stream */
void setFilterOn(ObjectInputStream ois);
+
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
index 3d842be..8c2b27d 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
@@ -138,28 +138,59 @@ public abstract class InternalDataSerializer extends DataSerializer implements D
* serialization.
*/
private static final Map<String, DataSerializer> classesToSerializers = new ConcurrentHashMap<>();
+
+
+ /**
+ * This list contains classes that Geode's classes subclass, such as antlr AST classes which
+ * are used by our Object Query Language. It also contains certain
+ * classes that are DataSerializable but end up being serialized as part of other serializable
+ * objects. VersionedObjectList, for instance, is serialized as part of a
+ * partial putAll exception object.
+ * <p>
+ * Do not java-serialize objects that Geode does not have complete control over. This
+ * leaves us open to security attacks such as Gadget Chains and compromises the ability
+ * to do a rolling upgrade from one version of Geode to the next.
+ * <p>
+ * In general you shouldn't use java serialization and you should implement
+ * DataSerializableFixedID
+ * for internal Geode objects. This gives you better control over backward-compatibility.
+ * <p>
+ * Do not add to this list unless absolutely necessary. Instead put your classes either
+ * in the sanctionedSerializables file for your module or in its excludedClasses file.
+ * Run AnalyzeSerializables to generate the content for the file.
+ * <p>
+ */
private static final String SANCTIONED_SERIALIZABLES_DEPENDENCIES_PATTERN =
"java.**;javax.management.**" + ";javax.print.attribute.EnumSyntax" // used for some old enums
+ ";antlr.**" // query AST objects
- + ";org.apache.commons.modeler.AttributeInfo" // old Admin API
- + ";org.apache.commons.modeler.FeatureInfo" // old Admin API
- + ";org.apache.commons.modeler.ManagedBean" // old Admin API
- + ";org.apache.geode.distributed.internal.DistributionConfigSnapshot" // old Admin API
- + ";org.apache.geode.distributed.internal.RuntimeDistributionConfigImpl" // old Admin API
- + ";org.apache.geode.distributed.internal.DistributionConfigImpl" // old Admin API
- + ";org.apache.geode.distributed.internal.membership.InternalDistributedMember" // RegionSnapshotService
- // function
- // WindowedExportFunction
+
+ // old Admin API
+ + ";org.apache.commons.modeler.AttributeInfo" + ";org.apache.commons.modeler.FeatureInfo"
+ + ";org.apache.commons.modeler.ManagedBean"
+ + ";org.apache.geode.distributed.internal.DistributionConfigSnapshot"
+ + ";org.apache.geode.distributed.internal.RuntimeDistributionConfigImpl"
+ + ";org.apache.geode.distributed.internal.DistributionConfigImpl"
+
+ // WindowedExportFunction, RegionSnapshotService
+ + ";org.apache.geode.distributed.internal.membership.InternalDistributedMember"
+ // putAll
+ ";org.apache.geode.internal.cache.persistence.PersistentMemberID" // putAll
+ ";org.apache.geode.internal.cache.persistence.DiskStoreID" // putAll
+ ";org.apache.geode.internal.cache.tier.sockets.VersionedObjectList" // putAll
- + ";org.apache.shiro.*;org.apache.shiro.authz.*;org.apache.shiro.authc.*" // security
- // services
- + ";org.apache.logging.log4j.**" // export logs
- + ";org.apache.geode.connectors.jdbc.internal.**"
- + ";org.apache.geode.modules.util.SessionCustomExpiry" // geode-modules
- + ";com.healthmarketscience.rmiio.*;com.sun.proxy.*" // Jar deployment
- + ";";
+
+ // security services
+ + ";org.apache.shiro.*;org.apache.shiro.authz.*;org.apache.shiro.authc.*"
+
+ // export logs
+ + ";org.apache.logging.log4j.Level" + ";org.apache.logging.log4j.spi.StandardLevel"
+
+ // jar deployment
+ + ";com.sun.proxy.$Proxy*" + ";com.healthmarketscience.rmiio.RemoteInputStream"
+
+ + ";org.apache.geode.connectors.jdbc.internal.**" // TODO - remove this! See GEODE-4752
+
+ // geode-modules
+ + ";org.apache.geode.modules.util.SessionCustomExpiry" + ";";
private static InputStreamFilter defaultSerializationFilter = new EmptyInputStreamFilter();
@@ -233,9 +264,10 @@ public abstract class InternalDataSerializer extends DataSerializer implements D
Collection<DistributedSystemService> services) {
logger.info("initializing InternalDataSerializer with {} services", services.size());
if (distributionConfig.getValidateSerializableObjects()) {
- if (!ClassUtils.isClassAvailable("sun.misc.ObjectInputFilter")) {
+ if (!ClassUtils.isClassAvailable("sun.misc.ObjectInputFilter")
+ && !ClassUtils.isClassAvailable("java.io.ObjectInputFilter")) {
throw new GemFireConfigException(
- "A serialization filter has been specified but this version of Java does not support serialization filters - sun.misc.ObjectInputFilter is not available");
+ "A serialization filter has been specified but this version of Java does not support serialization filters - ObjectInputFilter is not available");
}
serializationFilter =
new ObjectInputStreamFilterWrapper(SANCTIONED_SERIALIZABLES_DEPENDENCIES_PATTERN
diff --git a/geode-core/src/main/java/org/apache/geode/internal/ObjectInputStreamFilterWrapper.java b/geode-core/src/main/java/org/apache/geode/internal/ObjectInputStreamFilterWrapper.java
index 0e5bf83..68b550c 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/ObjectInputStreamFilterWrapper.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/ObjectInputStreamFilterWrapper.java
@@ -16,22 +16,53 @@ package org.apache.geode.internal;
import java.io.IOException;
import java.io.ObjectInputStream;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
import java.net.URL;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import org.apache.logging.log4j.Logger;
-import sun.misc.ObjectInputFilter;
+import org.apache.geode.GemFireConfigException;
+import org.apache.geode.InternalGemFireError;
import org.apache.geode.InternalGemFireException;
import org.apache.geode.distributed.internal.DistributedSystemService;
import org.apache.geode.internal.logging.LogService;
+/**
+ * ObjectInputStreamFilterWrapper isolates InternalDataSerializer from the JDK's
+ * ObjectInputFilter class, which is absent in builds of java8 prior to 121 and
+ * is sun.misc.ObjectInputFilter in later builds but is java.io.ObjectInputFilter
+ * in java9.
+ * <p>
+ * This class uses reflection and dynamic proxies to find the class and create a
+ * serialization filter. Once java8 is retired and no longer supported by Geode the
+ * use of reflection should be removed ObjectInputFilter can be directly used by
+ * InternalDataSerializer.
+ */
public class ObjectInputStreamFilterWrapper implements InputStreamFilter {
private static final Logger logger = LogService.getLogger();
- private final ObjectInputFilter serializationFilter;
+
+ private boolean usingJava8;
+ private Object serializationFilter;
+
+ private Class configClass; // ObjectInputFilter$Config
+ private Method setObjectInputFilterMethod; // method on ObjectInputFilter$Config or
+ // ObjectInputStream
+ private Method createFilterMethod; // method on ObjectInputFilter$Config
+
+ private Class filterClass; // ObjectInputFilter
+ private Object ALLOWED; // field on ObjectInputFilter
+ private Object REJECTED; // field on ObjectInputFilter
+ private Method checkInputMethod; // method on ObjectInputFilter
+
+ private Class filterInfoClass; // ObjectInputFilter$FilterInfo
+ private Method serialClassMethod; // method on ObjectInputFilter$FilterInfo
public ObjectInputStreamFilterWrapper(String serializationFilterSpec,
Collection<DistributedSystemService> services) {
@@ -62,33 +93,152 @@ public class ObjectInputStreamFilterWrapper implements InputStreamFilter {
logger.info("setting a serialization filter containing {}", serializationFilterSpec);
- final ObjectInputFilter userFilter =
- ObjectInputFilter.Config.createFilter(serializationFilterSpec);
- serializationFilter = filterInfo -> {
- if (filterInfo.serialClass() == null) {
- return userFilter.checkInput(filterInfo);
+ // try java8 - this will not throw an exception if ObjectInputFilter can't be found
+ if (createJava8Filter(serializationFilterSpec, sanctionedClasses)) {
+ return;
+ }
+
+ // try java9 - this throws an exception if it fails to create the filter
+ createJava9Filter(serializationFilterSpec, sanctionedClasses);
+ }
+
+ @Override
+ public void setFilterOn(ObjectInputStream objectInputStream) {
+ try {
+ if (usingJava8) {
+ setObjectInputFilterMethod.invoke(configClass, objectInputStream, serializationFilter);
+ } else {
+ setObjectInputFilterMethod.invoke(objectInputStream, serializationFilter);
}
+ } catch (IllegalAccessException | InvocationTargetException e) {
+ throw new InternalGemFireError("Unable to filter serialization", e);
+ }
+ }
+
+ /**
+ * java8 has sun.misc.ObjectInputFilter and uses ObjectInputFilter$Config.setObjectInputFilter()
+ */
+ private boolean createJava8Filter(String serializationFilterSpec,
+ Collection<String> sanctionedClasses) {
+ ClassPathLoader classPathLoader = ClassPathLoader.getLatest();
+ try {
+
+ filterInfoClass = classPathLoader.forName("sun.misc.ObjectInputFilter$FilterInfo");
+ serialClassMethod = filterInfoClass.getDeclaredMethod("serialClass");
+
+ filterClass = classPathLoader.forName("sun.misc.ObjectInputFilter");
+ checkInputMethod = filterClass.getDeclaredMethod("checkInput", filterInfoClass);
- String className = filterInfo.serialClass().getName();
- if (filterInfo.serialClass().isArray()) {
- className = filterInfo.serialClass().getComponentType().getName();
+ Class statusClass = classPathLoader.forName("sun.misc.ObjectInputFilter$Status");
+ ALLOWED = statusClass.getEnumConstants()[1];
+ REJECTED = statusClass.getEnumConstants()[2];
+ if (!ALLOWED.toString().equals("ALLOWED") || !REJECTED.toString().equals("REJECTED")) {
+ throw new GemFireConfigException(
+ "ObjectInputFilter$Status enumeration in this JDK is not as expected");
}
- if (sanctionedClasses.contains(className)) {
- return ObjectInputFilter.Status.ALLOWED;
- // return ObjectInputFilter.Status.UNDECIDED;
- } else {
- ObjectInputFilter.Status status = userFilter.checkInput(filterInfo);
- if (status == ObjectInputFilter.Status.REJECTED) {
- logger.fatal("Serialization filter is rejecting class {}", className, new Exception(""));
- }
- return status;
+
+ configClass = classPathLoader.forName("sun.misc.ObjectInputFilter$Config");
+ setObjectInputFilterMethod = configClass.getDeclaredMethod("setObjectInputFilter",
+ ObjectInputStream.class, filterClass);
+ createFilterMethod = configClass.getDeclaredMethod("createFilter", String.class);
+
+ serializationFilter = createSerializationFilter(serializationFilterSpec, sanctionedClasses);
+ usingJava8 = true;
+
+ } catch (ClassNotFoundException | InvocationTargetException | IllegalAccessException
+ | NoSuchMethodException e) {
+ if (filterInfoClass != null) {
+ throw new GemFireConfigException(
+ "A serialization filter has been specified but Geode was unable to configure a filter",
+ e);
}
- };
+ return false;
+ }
+ return true;
}
- @Override
- public void setFilterOn(ObjectInputStream objectInputStream) {
- ObjectInputFilter.Config.setObjectInputFilter(objectInputStream, serializationFilter);
+ /** java9 has java.io.ObjectInputFilter and uses ObjectInputStream.setObjectInputFilter() */
+ private void createJava9Filter(String serializationFilterSpec,
+ Collection<String> sanctionedClasses) {
+ try {
+ ClassPathLoader classPathLoader = ClassPathLoader.getLatest();
+
+ filterInfoClass = classPathLoader.forName("java.io.ObjectInputFilter$FilterInfo");
+ serialClassMethod = filterInfoClass.getDeclaredMethod("serialClass");
+
+
+ filterClass = classPathLoader.forName("java.io.ObjectInputFilter");
+ checkInputMethod = filterClass.getDeclaredMethod("checkInput", filterInfoClass);
+
+ Class statusClass = classPathLoader.forName("java.io.ObjectInputFilter$Status");
+ ALLOWED = statusClass.getEnumConstants()[1];
+ REJECTED = statusClass.getEnumConstants()[2];
+ if (!ALLOWED.toString().equals("ALLOWED") || !REJECTED.toString().equals("REJECTED")) {
+ throw new GemFireConfigException(
+ "ObjectInputFilter$Status enumeration in this JDK is not as expected");
+ }
+
+ configClass = classPathLoader.forName("java.io.ObjectInputFilter$Config");
+ setObjectInputFilterMethod =
+ ObjectInputStream.class.getDeclaredMethod("setObjectInputFilter", filterClass);
+ createFilterMethod = configClass.getDeclaredMethod("createFilter", String.class);
+
+ serializationFilter = createSerializationFilter(serializationFilterSpec, sanctionedClasses);
+
+ } catch (ClassNotFoundException | InvocationTargetException | IllegalAccessException
+ | NoSuchMethodException e) {
+ throw new GemFireConfigException(
+ "A serialization filter has been specified but Geode was unable to configure a filter",
+ e);
+ }
+ }
+
+
+ private Object createSerializationFilter(String serializationFilterSpec,
+ Collection<String> sanctionedClasses)
+ throws InvocationTargetException, IllegalAccessException {
+
+ /*
+ * create a user filter with the serialization whitelist/blacklist. This will be wrapped
+ * by a filter that white-lists sanctioned classes
+ */
+ Object userFilter = createFilterMethod.invoke(null, serializationFilterSpec);
+
+ InvocationHandler handler = (proxy, method, args) -> {
+ switch (method.getName()) {
+ case "checkInput":
+ Object filterInfo = args[0];
+ Class serialClass = (Class) serialClassMethod.invoke(filterInfo);
+ if (serialClass == null) { // no class to check, so nothing to white-list
+ return checkInputMethod.invoke(userFilter, filterInfo);
+ }
+ String className = serialClass.getName();
+ if (serialClass.isArray()) {
+ serialClass = serialClass.getComponentType();
+ className = serialClass.getComponentType().getName();
+ }
+ if (sanctionedClasses.contains(className)) {
+ return ALLOWED;
+ } else {
+ Object status = checkInputMethod.invoke(userFilter, filterInfo);
+ if (status == REJECTED) {
+ logger.fatal("Serialization filter is rejecting class {}", className,
+ new Exception(""));
+ }
+ return status;
+ }
+ default:
+ throw new UnsupportedOperationException(
+ "ObjectInputFilter." + method.getName() + " is not implemented");
+ }
+ };
+
+ ClassPathLoader classPathLoader = ClassPathLoader.getLatest();
+ return Proxy.newProxyInstance(classPathLoader.asClassLoader(), new Class[] {filterClass},
+ handler);
+
}
+
+
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/InternalDataSerializerSerializationWhitelistTest.java b/geode-core/src/test/java/org/apache/geode/internal/InternalDataSerializerSerializationWhitelistTest.java
index 74740a6..c521eab 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/InternalDataSerializerSerializationWhitelistTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/InternalDataSerializerSerializationWhitelistTest.java
@@ -20,6 +20,7 @@ import org.junit.experimental.categories.Category;
import org.apache.geode.DataSerializer;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.distributed.internal.DistributionConfigImpl;
+import org.apache.geode.internal.lang.ClassUtils;
import org.apache.geode.test.junit.categories.UnitTest;
/*
@@ -52,12 +53,8 @@ public class InternalDataSerializerSerializationWhitelistTest {
}
private boolean hasObjectInputFilter() {
- try {
- Class.forName("sun.misc.ObjectInputFilter");
- return true;
- } catch (ClassNotFoundException e) {
- return false;
- }
+ return (ClassUtils.isClassAvailable("sun.misc.ObjectInputFilter")
+ || ClassUtils.isClassAvailable("java.io.ObjectInputFilter"));
}
@AfterClass
--
To stop receiving notification emails like this one, please contact
bschuchardt@apache.org.