7197546: (proxy) Reflect about creating reflective proxies

Reviewed-by: alanb, jdn, jrose

jdk/src/share/classes/java/lang/Class.java

@@ -60,7 +60,9 @@ import sun.reflect.generics.repository.ConstructorRepository;
 import sun.reflect.generics.scope.ClassScope;
 import sun.security.util.SecurityConstants;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Proxy;
 import sun.reflect.annotation.*;
+import sun.reflect.misc.ReflectUtil;
 
 /**
  * Instances of the class {@code Class} represent classes and
@@ -247,11 +249,11 @@ public final
                                    ClassLoader loader)
         throws ClassNotFoundException
     {
-        if (loader == null) {
+        if (sun.misc.VM.isSystemDomainLoader(loader)) {
             SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
                 ClassLoader ccl = ClassLoader.getCallerClassLoader();
-                if (ccl != null) {
+                if (!sun.misc.VM.isSystemDomainLoader(ccl)) {
                     sm.checkPermission(
                         SecurityConstants.GET_CLASSLOADER_PERMISSION);
                 }
@@ -316,7 +318,7 @@ public final
         throws InstantiationException, IllegalAccessException
     {
         if (System.getSecurityManager() != null) {
-            checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+            checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), false);
         }
         return newInstance0();
     }
@@ -1295,7 +1297,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), false);
 
         // Privileged so this implementation can look at DECLARED classes,
         // something the caller might not have privilege to do.  The code here
@@ -1370,7 +1372,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         return copyFields(privateGetPublicFields(null));
     }
 
@@ -1421,7 +1423,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         return copyMethods(privateGetPublicMethods());
     }
 
@@ -1470,7 +1472,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         return copyConstructors(privateGetDeclaredConstructors(true));
     }
 
@@ -1529,7 +1531,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         Field field = getField0(name);
         if (field == null) {
             throw new NoSuchFieldException(name);
@@ -1614,7 +1616,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         Method method = getMethod0(name, parameterTypes);
         if (method == null) {
             throw new NoSuchMethodException(getName() + "." + name + argumentTypesToString(parameterTypes));
@@ -1668,7 +1670,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.PUBLIC, ClassLoader.getCallerClassLoader(), true);
         return getConstructor0(parameterTypes, Member.PUBLIC);
     }
 
@@ -1710,7 +1712,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), false);
         return getDeclaredClasses0();
     }
 
@@ -1754,7 +1756,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         return copyFields(privateGetDeclaredFields(false));
     }
 
@@ -1802,7 +1804,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         return copyMethods(privateGetDeclaredMethods(false));
     }
 
@@ -1847,7 +1849,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         return copyConstructors(privateGetDeclaredConstructors(false));
     }
 
@@ -1891,7 +1893,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         Field field = searchFields(privateGetDeclaredFields(false), name);
         if (field == null) {
             throw new NoSuchFieldException(name);
@@ -1946,7 +1948,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         Method method = searchMethods(privateGetDeclaredMethods(false), name, parameterTypes);
         if (method == null) {
             throw new NoSuchMethodException(getName() + "." + name + argumentTypesToString(parameterTypes));
@@ -1996,7 +1998,7 @@ public final
         // be very careful not to change the stack depth of this
         // checkMemberAccess call for security reasons
         // see java.lang.SecurityManager.checkMemberAccess
-        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader());
+        checkMemberAccess(Member.DECLARED, ClassLoader.getCallerClassLoader(), true);
         return getConstructor0(parameterTypes, Member.DECLARED);
     }
 
@@ -2166,18 +2168,26 @@ public final
      * <p> Default policy: allow all clients access with normal Java access
      * control.
      */
-    private void checkMemberAccess(int which, ClassLoader ccl) {
+    private void checkMemberAccess(int which, ClassLoader ccl, boolean checkProxyInterfaces) {
         SecurityManager s = System.getSecurityManager();
         if (s != null) {
             s.checkMemberAccess(this, which);
             ClassLoader cl = getClassLoader0();
-            if (sun.reflect.misc.ReflectUtil.needsPackageAccessCheck(ccl, cl)) {
+            if (ReflectUtil.needsPackageAccessCheck(ccl, cl)) {
                 String name = this.getName();
                 int i = name.lastIndexOf('.');
                 if (i != -1) {
-                    s.checkPackageAccess(name.substring(0, i));
+                    // skip the package access check on a proxy class in default proxy package
+                    String pkg = name.substring(0, i);
+                    if (!Proxy.isProxyClass(this) || !pkg.equals(ReflectUtil.PROXY_PACKAGE)) {
+                        s.checkPackageAccess(pkg);
+                    }
                 }
             }
+            // check package access on the proxy interfaces
+            if (checkProxyInterfaces && Proxy.isProxyClass(this)) {
+                ReflectUtil.checkProxyPackageAccess(ccl, this.getInterfaces());
+            }
         }
     }
 

jdk/src/share/classes/java/lang/invoke/MethodHandleProxies.java

@@ -26,8 +26,12 @@
 package java.lang.invoke;
 
 import java.lang.reflect.*;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import sun.invoke.WrapperInstance;
 import java.util.ArrayList;
+import sun.reflect.Reflection;
+import sun.reflect.misc.ReflectUtil;
 
 /**
  * This class consists exclusively of static methods that help adapt
@@ -137,6 +141,18 @@ public class MethodHandleProxies {
     <T> T asInterfaceInstance(final Class<T> intfc, final MethodHandle target) {
         if (!intfc.isInterface() || !Modifier.isPublic(intfc.getModifiers()))
             throw new IllegalArgumentException("not a public interface: "+intfc.getName());
+        SecurityManager smgr = System.getSecurityManager();
+        if (smgr != null) {
+            final int CALLER_FRAME = 2; // 0: Reflection, 1: asInterfaceInstance, 2: caller
+            final Class<?> caller = Reflection.getCallerClass(CALLER_FRAME);
+            final ClassLoader ccl = caller.getClassLoader();
+            ReflectUtil.checkProxyPackageAccess(ccl, intfc);
+        }
+        ClassLoader proxyLoader = intfc.getClassLoader();
+        if (proxyLoader == null) {
+            ClassLoader cl = Thread.currentThread().getContextClassLoader(); // avoid use of BCP
+            proxyLoader = cl != null ? cl : ClassLoader.getSystemClassLoader();
+        }
         final Method[] methods = getSingleNameMethods(intfc);
         if (methods == null)
             throw new IllegalArgumentException("not a single-method interface: "+intfc.getName());
@@ -148,27 +164,44 @@ public class MethodHandleProxies {
             checkTarget = checkTarget.asType(checkTarget.type().changeReturnType(Object.class));
             vaTargets[i] = checkTarget.asSpreader(Object[].class, smMT.parameterCount());
         }
-        return intfc.cast(Proxy.newProxyInstance(
-                intfc.getClassLoader(),
-                new Class<?>[]{ intfc, WrapperInstance.class },
-                new InvocationHandler() {
-                    private Object getArg(String name) {
-                        if ((Object)name == "getWrapperInstanceTarget")  return target;
-                        if ((Object)name == "getWrapperInstanceType")    return intfc;
-                        throw new AssertionError();
+        final InvocationHandler ih = new InvocationHandler() {
+                private Object getArg(String name) {
+                    if ((Object)name == "getWrapperInstanceTarget")  return target;
+                    if ((Object)name == "getWrapperInstanceType")    return intfc;
+                    throw new AssertionError();
+                }
+                public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+                    for (int i = 0; i < methods.length; i++) {
+                        if (method.equals(methods[i]))
+                            return vaTargets[i].invokeExact(args);
                     }
-                    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-                        for (int i = 0; i < methods.length; i++) {
-                            if (method.equals(methods[i]))
-                                return vaTargets[i].invokeExact(args);
-                        }
-                        if (method.getDeclaringClass() == WrapperInstance.class)
-                            return getArg(method.getName());
-                        if (isObjectMethod(method))
-                            return callObjectMethod(proxy, method, args);
-                        throw new InternalError("bad proxy method: "+method);
-                    }
-                }));
+                    if (method.getDeclaringClass() == WrapperInstance.class)
+                        return getArg(method.getName());
+                    if (isObjectMethod(method))
+                        return callObjectMethod(proxy, method, args);
+                    throw new InternalError("bad proxy method: "+method);
+                }
+            };
+
+        Object proxy;
+        if (smgr != null) {
+            // sun.invoke.WrapperInstance is a restricted interface not accessible
+            // by any non-null class loader.
+            final ClassLoader loader = proxyLoader;
+            proxy = AccessController.doPrivileged(new PrivilegedAction<Object>() {
+                public Object run() {
+                    return Proxy.newProxyInstance(
+                            loader,
+                            new Class<?>[]{ intfc, WrapperInstance.class },
+                            ih);
+                }
+            });
+        } else {
+            proxy = Proxy.newProxyInstance(proxyLoader,
+                                           new Class<?>[]{ intfc, WrapperInstance.class },
+                                           ih);
+        }
+        return intfc.cast(proxy);
     }
 
     /**

jdk/src/share/classes/java/lang/reflect/Proxy.java

@@ -27,6 +27,9 @@ package java.lang.reflect;
 
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
+import java.security.AccessController;
+import java.security.Permission;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -36,6 +39,9 @@ import java.util.Set;
 import java.util.List;
 import java.util.WeakHashMap;
 import sun.misc.ProxyGenerator;
+import sun.reflect.Reflection;
+import sun.reflect.misc.ReflectUtil;
+import sun.security.util.SecurityConstants;
 
 /**
  * {@code Proxy} provides static methods for creating dynamic proxy
@@ -265,9 +271,69 @@ public class Proxy implements java.io.Serializable {
      * @param   h the invocation handler for this proxy instance
      */
     protected Proxy(InvocationHandler h) {
+        doNewInstanceCheck();
         this.h = h;
     }
 
+    private static class ProxyAccessHelper {
+        // The permission is implementation specific.
+        static final Permission PROXY_PERMISSION =
+            new ReflectPermission("proxyConstructorNewInstance");
+        // These system properties are defined to provide a short-term
+        // workaround if customers need to disable the new security checks.
+        static final boolean allowNewInstance;
+        static final boolean allowNullLoader;
+        static {
+            allowNewInstance = getBooleanProperty("sun.reflect.proxy.allowsNewInstance");
+            allowNullLoader = getBooleanProperty("sun.reflect.proxy.allowsNullLoader");
+        }
+
+        private static boolean getBooleanProperty(final String key) {
+            String s = AccessController.doPrivileged(new PrivilegedAction<String>() {
+                public String run() {
+                    return System.getProperty(key);
+                }
+            });
+            return Boolean.valueOf(s);
+        }
+
+        static boolean needsNewInstanceCheck(Class<?> proxyClass) {
+            if (!Proxy.isProxyClass(proxyClass) || allowNewInstance) {
+                return false;
+            }
+
+            if (proxyClass.getName().startsWith(ReflectUtil.PROXY_PACKAGE + ".")) {
+                // all proxy interfaces are public
+                return false;
+            }
+            for (Class<?> intf : proxyClass.getInterfaces()) {
+                if (!Modifier.isPublic(intf.getModifiers())) {
+                    return true;
+                }
+            }
+            return false;
+        }
+    }
+
+    /*
+     * Access check on a proxy class that implements any non-public interface.
+     *
+     * @throws  SecurityException if a security manager exists, and
+     *          the caller does not have the permission.
+     */
+    private void doNewInstanceCheck() {
+        SecurityManager sm = System.getSecurityManager();
+        Class<?> proxyClass = this.getClass();
+        if (sm != null && ProxyAccessHelper.needsNewInstanceCheck(proxyClass)) {
+            try {
+                sm.checkPermission(ProxyAccessHelper.PROXY_PERMISSION);
+            } catch (SecurityException e) {
+                throw new SecurityException("Not allowed to construct a Proxy "
+                        + "instance that implements a non-public interface", e);
+            }
+        }
+    }
+
     /**
      * Returns the {@code java.lang.Class} object for a proxy class
      * given a class loader and an array of interfaces.  The proxy class
@@ -346,6 +412,51 @@ public class Proxy implements java.io.Serializable {
                                          Class<?>... interfaces)
         throws IllegalArgumentException
     {
+        return getProxyClass0(loader, interfaces); // stack walk magic: do not refactor
+    }
+
+    private static void checkProxyLoader(ClassLoader ccl,
+                                         ClassLoader loader)
+    {
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            if (loader == null && ccl != null) {
+                if (!ProxyAccessHelper.allowNullLoader) {
+                    sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION);
+                }
+            }
+        }
+    }
+
+    /*
+     * Generate a proxy class (caller-sensitive).
+     *
+     * To define a proxy class, it performs the access checks as in
+     * Class.forName (VM will invoke ClassLoader.checkPackageAccess):
+     * 1. "getClassLoader" permission check if loader == null
+     * 2. checkPackageAccess on the interfaces it implements
+     *
+     * To get a constructor and new instance of a proxy class, it performs
+     * the package access check on the interfaces it implements
+     * as in Class.getConstructor.
+     *
+     * If an interface is non-public, the proxy class must be defined by
+     * the defining loader of the interface.  If the caller's class loader
+     * is not the same as the defining loader of the interface, the VM
+     * will throw IllegalAccessError when the generated proxy class is
+     * being defined via the defineClass0 method.
+     */
+    private static Class<?> getProxyClass0(ClassLoader loader,
+                                           Class<?>... interfaces) {
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            final int CALLER_FRAME = 3; // 0: Reflection, 1: getProxyClass0 2: Proxy 3: caller
+            final Class<?> caller = Reflection.getCallerClass(CALLER_FRAME);
+            final ClassLoader ccl = caller.getClassLoader();
+            checkProxyLoader(ccl, loader);
+            ReflectUtil.checkProxyPackageAccess(ccl, interfaces);
+        }
+
         if (interfaces.length > 65535) {
             throw new IllegalArgumentException("interface limit exceeded");
         }
@@ -497,8 +608,9 @@ public class Proxy implements java.io.Serializable {
                 }
             }
 
-            if (proxyPkg == null) {     // if no non-public proxy interfaces,
-                proxyPkg = "";          // use the unnamed package
+            if (proxyPkg == null) {
+                // if no non-public proxy interfaces, use sun.proxy package
+                proxyPkg = ReflectUtil.PROXY_PACKAGE + ".";
             }
 
             {
@@ -598,22 +710,46 @@ public class Proxy implements java.io.Serializable {
         /*
          * Look up or generate the designated proxy class.
          */
-        Class<?> cl = getProxyClass(loader, interfaces);
+        Class<?> cl = getProxyClass0(loader, interfaces); // stack walk magic: do not refactor
 
         /*
          * Invoke its constructor with the designated invocation handler.
          */
         try {
-            Constructor<?> cons = cl.getConstructor(constructorParams);
-            return cons.newInstance(new Object[] { h });
-        } catch (NoSuchMethodException |
-                 IllegalAccessException |
-                 InstantiationException |
-                 InvocationTargetException e) {
+            final Constructor<?> cons = cl.getConstructor(constructorParams);
+            final InvocationHandler ih = h;
+            SecurityManager sm = System.getSecurityManager();
+            if (sm != null && ProxyAccessHelper.needsNewInstanceCheck(cl)) {
+                // create proxy instance with doPrivilege as the proxy class may
+                // implement non-public interfaces that requires a special permission
+                return AccessController.doPrivileged(new PrivilegedAction<Object>() {
+                    public Object run() {
+                        return newInstance(cons, ih);
+                    }
+                });
+            } else {
+                return newInstance(cons, ih);
+            }
+        } catch (NoSuchMethodException e) {
             throw new InternalError(e.toString(), e);
         }
     }
 
+    private static Object newInstance(Constructor<?> cons, InvocationHandler h) {
+        try {
+            return cons.newInstance(new Object[] {h} );
+        } catch (IllegalAccessException | InstantiationException e) {
+            throw new InternalError(e.toString(), e);
+        } catch (InvocationTargetException e) {
+            Throwable t = e.getCause();
+            if (t instanceof RuntimeException) {
+                throw (RuntimeException) t;
+            } else {
+                throw new InternalError(t.toString(), t);
+            }
+        }
+    }
+
     /**
      * Returns true if and only if the specified class was dynamically
      * generated to be a proxy class using the {@code getProxyClass}

jdk/src/share/classes/sun/reflect/misc/ReflectUtil.java

@@ -178,4 +178,29 @@ public final class ReflectUtil {
 
         return !isAncestor(from, to);
     }
+
+    /**
+     * Access check on the interfaces that a proxy class implements and throw
+     * {@code SecurityException} if it accesses a restricted package.
+     *
+     * @param ccl the caller's class loader
+     * @param interfaces the list of interfaces that a proxy class implements
+     *
+     * @see Proxy#checkProxyAccess
+     */
+    public static void checkProxyPackageAccess(ClassLoader ccl,
+                                               Class<?>... interfaces)
+    {
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            for (Class<?> intf : interfaces) {
+                ClassLoader cl = intf.getClassLoader();
+                if (needsPackageAccessCheck(ccl, cl)) {
+                    checkPackageAccess(intf);
+                }
+            }
+        }
+    }
+
+    public static final String PROXY_PACKAGE = "sun.proxy";
 }

jdk/test/java/rmi/server/RMIClassLoader/loadProxyClasses/security.policy

@@ -13,6 +13,7 @@ grant {
     permission java.io.FilePermission ".${/}-", "read,write,delete";
 
     permission java.lang.RuntimePermission "createClassLoader";
+    permission java.lang.RuntimePermission "getClassLoader";
     permission java.lang.RuntimePermission "setContextClassLoader";
 
     // used by TestLibrary to determine test environment