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 <bschucha...@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
bschucha...@apache.org.

Reply via email to