8185346: Relax RMI Registry Serial Filter to allow arrays of any type
authorrriggs
Wed, 16 Aug 2017 16:46:51 -0400
changeset 46160 c647e44ea1b9
parent 46159 97256df3734f
child 46161 f6a5a6241f79
8185346: Relax RMI Registry Serial Filter to allow arrays of any type Summary: Registry filter should allow arrays of any type Reviewed-by: dfuchs, smarks, coffeys
jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java
jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputFilterAccess.java
jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java
jdk/src/java.base/share/conf/security/java.security
jdk/src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java
jdk/test/java/rmi/registry/serialFilter/RegistryFilterTest.java
--- a/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java	Wed Aug 16 13:15:45 2017 -0700
+++ b/jdk/src/java.base/share/classes/java/io/ObjectInputFilter.java	Wed Aug 16 16:46:51 2017 -0400
@@ -34,6 +34,7 @@
 import java.util.Optional;
 import java.util.function.Function;
 
+import jdk.internal.misc.SharedSecrets;
 
 /**
  * Filter classes, array lengths, and graph metrics during deserialization.
@@ -265,6 +266,9 @@
                         return null;
                     });
             configLog = (configuredFilter != null) ? System.getLogger("java.io.serialization") : null;
+
+            // Setup shared secrets for RegistryImpl to use.
+            SharedSecrets.setJavaObjectInputFilterAccess(Config::createFilter2);
         }
 
         /**
@@ -370,7 +374,20 @@
          */
         public static ObjectInputFilter createFilter(String pattern) {
             Objects.requireNonNull(pattern, "pattern");
-            return Global.createFilter(pattern);
+            return Global.createFilter(pattern, true);
+        }
+
+        /**
+         * Returns an ObjectInputFilter from a string of patterns that
+         * checks only the length for arrays, not the component type.
+         *
+         * @param pattern the pattern string to parse; not null
+         * @return a filter to check a class being deserialized;
+         *          {@code null} if no patterns
+         */
+        static ObjectInputFilter createFilter2(String pattern) {
+            Objects.requireNonNull(pattern, "pattern");
+            return Global.createFilter(pattern, false);
         }
 
         /**
@@ -404,20 +421,26 @@
              * Maximum length of any array.
              */
             private long maxArrayLength;
+            /**
+             * True to check the component type for arrays.
+             */
+            private final boolean checkComponentType;
 
             /**
              * Returns an ObjectInputFilter from a string of patterns.
              *
              * @param pattern the pattern string to parse
+             * @param checkComponentType true if the filter should check
+             *                           the component type of arrays
              * @return a filter to check a class being deserialized;
              *          {@code null} if no patterns
              * @throws IllegalArgumentException if the parameter is malformed
              *                if the pattern is missing the name, the long value
              *                is not a number or is negative.
              */
-            static ObjectInputFilter createFilter(String pattern) {
+            static ObjectInputFilter createFilter(String pattern, boolean checkComponentType) {
                 try {
-                    return new Global(pattern);
+                    return new Global(pattern, checkComponentType);
                 } catch (UnsupportedOperationException uoe) {
                     // no non-empty patterns
                     return null;
@@ -428,12 +451,15 @@
              * Construct a new filter from the pattern String.
              *
              * @param pattern a pattern string of filters
+             * @param checkComponentType true if the filter should check
+             *                           the component type of arrays
              * @throws IllegalArgumentException if the pattern is malformed
              * @throws UnsupportedOperationException if there are no non-empty patterns
              */
-            private Global(String pattern) {
+            private Global(String pattern, boolean checkComponentType) {
                 boolean hasLimits = false;
                 this.pattern = pattern;
+                this.checkComponentType = checkComponentType;
 
                 maxArrayLength = Long.MAX_VALUE; // Default values are unlimited
                 maxDepth = Long.MAX_VALUE;
@@ -595,6 +621,10 @@
                             // array length is too big
                             return Status.REJECTED;
                         }
+                        if (!checkComponentType) {
+                            // As revised; do not check the component type for arrays
+                            return Status.UNDECIDED;
+                        }
                         do {
                             // Arrays are decided based on the component type
                             clazz = clazz.getComponentType();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputFilterAccess.java	Wed Aug 16 16:46:51 2017 -0400
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.internal.misc;
+
+import java.io.ObjectInputFilter;
+
+/**
+ * Access to the alternative ObjectInputFilter.Config.createFilter2 for RMI.
+ */
+public interface JavaObjectInputFilterAccess {
+    /**
+     * Creates a filter from the pattern.
+     */
+    ObjectInputFilter createFilter2(String pattern);
+}
--- a/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java	Wed Aug 16 13:15:45 2017 -0700
+++ b/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java	Wed Aug 16 16:46:51 2017 -0400
@@ -25,6 +25,7 @@
 
 package jdk.internal.misc;
 
+import java.io.ObjectInputFilter;
 import java.lang.module.ModuleDescriptor;
 import java.util.ResourceBundle;
 import java.util.jar.JarFile;
@@ -70,6 +71,7 @@
     private static JavaAWTFontAccess javaAWTFontAccess;
     private static JavaBeansAccess javaBeansAccess;
     private static JavaObjectInputStreamAccess javaObjectInputStreamAccess;
+    private static JavaObjectInputFilterAccess javaObjectInputFilterAccess;
     private static JavaIORandomAccessFileAccess javaIORandomAccessFileAccess;
 
     public static JavaUtilJarAccess javaUtilJarAccess() {
@@ -315,6 +317,17 @@
         javaObjectInputStreamAccess = access;
     }
 
+    public static JavaObjectInputFilterAccess getJavaObjectInputFilterAccess() {
+        if (javaObjectInputFilterAccess == null) {
+            unsafe.ensureClassInitialized(ObjectInputFilter.Config.class);
+        }
+        return javaObjectInputFilterAccess;
+    }
+
+    public static void setJavaObjectInputFilterAccess(JavaObjectInputFilterAccess access) {
+        javaObjectInputFilterAccess = access;
+    }
+
     public static void setJavaIORandomAccessFileAccess(JavaIORandomAccessFileAccess jirafa) {
         javaIORandomAccessFileAccess = jirafa;
     }
--- a/jdk/src/java.base/share/conf/security/java.security	Wed Aug 16 13:15:45 2017 -0700
+++ b/jdk/src/java.base/share/conf/security/java.security	Wed Aug 16 16:46:51 2017 -0400
@@ -951,12 +951,36 @@
 #
 # The filter pattern uses the same format as jdk.serialFilter.
 # This filter can override the builtin filter if additional types need to be
-# allowed or rejected from the RMI Registry.
+# allowed or rejected from the RMI Registry or to decrease limits but not
+# to increase limits.
+# If the limits (maxdepth, maxrefs, or maxbytes) are exceeded, the object is rejected.
+#
+# Each non-array type is allowed or rejected if it matches one of the patterns,
+# evaluated from left to right, and is otherwise allowed. Arrays of any
+# component type, including subarrays and arrays of primitives, are allowed.
+#
+# Array construction of any component type, including subarrays and arrays of
+# primitives, are allowed unless the length is greater than the maxarray limit.
+# The filter is applied to each array element.
 #
 # Note: This property is currently used by the JDK Reference implementation.
 # It is not guaranteed to be examined and used by other implementations.
 #
-#sun.rmi.registry.registryFilter=pattern;pattern
+# The built-in filter allows subclasses of allowed classes and
+# can approximately be represented as the pattern:
+#
+#sun.rmi.registry.registryFilter=\
+#    maxarray=1000000;\
+#    maxdepth=20;\
+#    java.lang.String;\
+#    java.lang.Number;\
+#    java.lang.reflect.Proxy;\
+#    java.rmi.Remote;\
+#    sun.rmi.server.UnicastRef;\
+#    sun.rmi.server.RMIClientSocketFactory;\
+#    sun.rmi.server.RMIServerSocketFactory;\
+#    java.rmi.activation.ActivationID;\
+#    java.rmi.server.UID
 #
 # RMI Distributed Garbage Collector (DGC) Serial Filter
 #
--- a/jdk/src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java	Wed Aug 16 13:15:45 2017 -0700
+++ b/jdk/src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java	Wed Aug 16 16:46:51 2017 -0400
@@ -28,7 +28,6 @@
 import java.io.ObjectInputFilter;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.rmi.server.LogStream;
 import java.security.PrivilegedAction;
 import java.security.Security;
 import java.util.ArrayList;
@@ -58,6 +57,7 @@
 import java.security.ProtectionDomain;
 import java.text.MessageFormat;
 
+import jdk.internal.misc.SharedSecrets;
 import sun.rmi.runtime.Log;
 import sun.rmi.server.UnicastRef;
 import sun.rmi.server.UnicastServerRef;
@@ -109,7 +109,7 @@
     private static final int REGISTRY_MAX_DEPTH = 20;
 
     /** Registry maximum array size in remote invocations. **/
-    private static final int REGISTRY_MAX_ARRAY_SIZE = 10000;
+    private static final int REGISTRY_MAX_ARRAY_SIZE = 1_000_000;
 
     /**
      * The registryFilter created from the value of the {@code "sun.rmi.registry.registryFilter"}
@@ -130,7 +130,7 @@
             props = Security.getProperty(REGISTRY_FILTER_PROPNAME);
         }
         if (props != null) {
-            filter = ObjectInputFilter.Config.createFilter(props);
+            filter = SharedSecrets.getJavaObjectInputFilterAccess().createFilter2(props);
             Log regLog = Log.getLog("sun.rmi.registry", "registry", -1);
             if (regLog.isLoggable(Log.BRIEF)) {
                 regLog.log(Log.BRIEF, "registryFilter = " + filter);
@@ -451,17 +451,10 @@
         Class<?> clazz = filterInfo.serialClass();
         if (clazz != null) {
             if (clazz.isArray()) {
-                if (filterInfo.arrayLength() >= 0 && filterInfo.arrayLength() > REGISTRY_MAX_ARRAY_SIZE) {
-                    return ObjectInputFilter.Status.REJECTED;
-                }
-                do {
-                    // Arrays are allowed depending on the component type
-                    clazz = clazz.getComponentType();
-                } while (clazz.isArray());
-            }
-            if (clazz.isPrimitive()) {
-                // Arrays of primitives are allowed
-                return ObjectInputFilter.Status.ALLOWED;
+                // Arrays are REJECTED only if they exceed the limit
+                return (filterInfo.arrayLength() >= 0 && filterInfo.arrayLength() > REGISTRY_MAX_ARRAY_SIZE)
+                    ? ObjectInputFilter.Status.REJECTED
+                    : ObjectInputFilter.Status.UNDECIDED;
             }
             if (String.class == clazz
                     || java.lang.Number.class.isAssignableFrom(clazz)
--- a/jdk/test/java/rmi/registry/serialFilter/RegistryFilterTest.java	Wed Aug 16 13:15:45 2017 -0700
+++ b/jdk/test/java/rmi/registry/serialFilter/RegistryFilterTest.java	Wed Aug 16 16:46:51 2017 -0400
@@ -35,7 +35,6 @@
 import java.util.Objects;
 
 import org.testng.Assert;
-import org.testng.TestNG;
 import org.testng.annotations.BeforeSuite;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -65,21 +64,14 @@
 
     static final int REGISTRY_MAX_DEPTH = 20;
 
-    static final int REGISTRY_MAX_ARRAY = 10000;
+    static final int REGISTRY_MAX_ARRAY = 1_000_000;
 
     static final String registryFilter =
             System.getProperty("sun.rmi.registry.registryFilter",
                     Security.getProperty("sun.rmi.registry.registryFilter"));
 
-    @DataProvider(name = "bindAllowed")
-    static Object[][] bindAllowedObjects() {
-        Object[][] objects = {
-        };
-        return objects;
-    }
-
     /**
-     * Data RMI Regiry bind test.
+     * Data RMI Registry bind test.
      * - name
      * - Object
      * - true/false if object is blacklisted by a filter (implicit or explicit)
@@ -90,9 +82,11 @@
         Object[][] data = {
                 { "byte[max]", new XX(new byte[REGISTRY_MAX_ARRAY]), false },
                 { "String", new XX("now is the time"), false},
-                { "String[]", new XX(new String[3]), false},
-                { "Long[4]", new XX(new Long[4]), registryFilter != null },
+                { "String[3]", new XX(new String[3]), false},
+                { "Long[4]", new XX(new Long[4]), false },
+                { "Object[REGISTRY_MAX_ARRAY]", new XX(new Object[REGISTRY_MAX_ARRAY]), false },
                 { "rej-byte[toobig]", new XX(new byte[REGISTRY_MAX_ARRAY + 1]), true },
+                { "rej-Object[toobig]", new XX(new Object[REGISTRY_MAX_ARRAY + 1]), true },
                 { "rej-MarshalledObject", createMarshalledObject(), true },
                 { "rej-RejectableClass", new RejectableClass(), registryFilter != null},
         };