geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject [geode] 01/01: GEODE-4672 Geode fails to start with JDK 9 if validate-serializable-objects is set
Date Wed, 28 Feb 2018 18:36:22 GMT
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-4672
in repository https://gitbox.apache.org/repos/asf/geode.git

commit eccd68974317c6f1d7d7fb27c115282cdab13f2f
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
AuthorDate: Wed Feb 28 10:30:10 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.
---
 .../apache/geode/internal/InputStreamFilter.java   |   3 +
 .../geode/internal/InternalDataSerializer.java     |  34 +++-
 .../internal/ObjectInputStreamFilterWrapper.java   | 187 ++++++++++++++++++---
 ...alDataSerializerSerializationWhitelistTest.java |   9 +-
 4 files changed, 199 insertions(+), 34 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..dbb94b5 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,6 +138,28 @@ 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 Gadge 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
@@ -155,10 +177,11 @@ public abstract class InternalDataSerializer extends DataSerializer
implements D
           + ";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.logging.log4j.Level"
+          + ";com.healthmarketscience.rmiio.RemoteInputStream"
+          + ";org.apache.geode.connectors.jdbc.internal.**"  // TODO - remove this!  See
GEODE-4752
           + ";org.apache.geode.modules.util.SessionCustomExpiry" // geode-modules
-          + ";com.healthmarketscience.rmiio.*;com.sun.proxy.*" // Jar deployment
+          + ";com.sun.proxy.*"
           + ";";
 
 
@@ -233,9 +256,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..59a95f5 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,143 @@ 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");
 
-      String className = filterInfo.serialClass().getName();
-      if (filterInfo.serialClass().isArray()) {
-        className = filterInfo.serialClass().getComponentType().getName();
+      filterClass = classPathLoader.forName("sun.misc.ObjectInputFilter");
+      checkInputMethod = filterClass.getDeclaredMethod("checkInput", filterInfoClass);
+
+      Class statusClass = classPathLoader.forName("sun.misc.ObjectInputFilter$Status");
+      ALLOWED = statusClass.getEnumConstants()[1];
+      REJECTED = statusClass.getEnumConstants()[2];
+
+      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);
       }
-      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;
+      return false;
+    }
+
+    return true;
+  }
+
+  /** 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];
+
+      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()) {
+            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);
 
-  @Override
-  public void setFilterOn(ObjectInputStream objectInputStream) {
-    ObjectInputFilter.Config.setObjectInputFilter(objectInputStream, serializationFilter);
   }
+
+
 }
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.

Mime
View raw message