8180024: Improve construction of objects during deserialization
Reviewed-by: rriggs, skoivu, ahgross, rhalade
--- a/src/java.base/share/classes/java/io/ObjectStreamClass.java Thu Oct 19 11:45:23 2017 -0700
+++ b/src/java.base/share/classes/java/io/ObjectStreamClass.java Fri May 19 11:18:49 2017 +0100
@@ -32,14 +32,19 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.UndeclaredThrowableException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
+import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.security.PermissionCollection;
+import java.security.Permissions;
import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -53,7 +58,8 @@
import jdk.internal.reflect.Reflection;
import jdk.internal.reflect.ReflectionFactory;
import sun.reflect.misc.ReflectUtil;
-
+import jdk.internal.misc.SharedSecrets;
+import jdk.internal.misc.JavaSecurityAccess;
import static java.io.ObjectStreamField.*;
/**
@@ -176,6 +182,9 @@
/** serialization-appropriate constructor, or null if none */
private Constructor<?> cons;
+ /** protection domains that need to be checked when calling the constructor */
+ private ProtectionDomain[] domains;
+
/** class-defined writeObject method, or null if none */
private Method writeObjectMethod;
/** class-defined readObject method, or null if none */
@@ -508,6 +517,7 @@
cl, "readObjectNoData", null, Void.TYPE);
hasWriteObjectData = (writeObjectMethod != null);
}
+ domains = getProtectionDomains(cons, cl);
writeReplaceMethod = getInheritableMethod(
cl, "writeReplace", null, Object.class);
readResolveMethod = getInheritableMethod(
@@ -551,6 +561,65 @@
}
/**
+ * Creates a PermissionDomain that grants no permission.
+ */
+ private ProtectionDomain noPermissionsDomain() {
+ PermissionCollection perms = new Permissions();
+ perms.setReadOnly();
+ return new ProtectionDomain(null, perms);
+ }
+
+ /**
+ * Aggregate the ProtectionDomains of all the classes that separate
+ * a concrete class {@code cl} from its ancestor's class declaring
+ * a constructor {@code cons}.
+ *
+ * If {@code cl} is defined by the boot loader, or the constructor
+ * {@code cons} is declared by {@code cl}, or if there is no security
+ * manager, then this method does nothing and {@code null} is returned.
+ *
+ * @param cons A constructor declared by {@code cl} or one of its
+ * ancestors.
+ * @param cl A concrete class, which is either the class declaring
+ * the constructor {@code cons}, or a serializable subclass
+ * of that class.
+ * @return An array of ProtectionDomain representing the set of
+ * ProtectionDomain that separate the concrete class {@code cl}
+ * from its ancestor's declaring {@code cons}, or {@code null}.
+ */
+ private ProtectionDomain[] getProtectionDomains(Constructor<?> cons,
+ Class<?> cl) {
+ ProtectionDomain[] domains = null;
+ if (cons != null && cl.getClassLoader() != null
+ && System.getSecurityManager() != null) {
+ Class<?> cls = cl;
+ Class<?> fnscl = cons.getDeclaringClass();
+ Set<ProtectionDomain> pds = null;
+ while (cls != fnscl) {
+ ProtectionDomain pd = cls.getProtectionDomain();
+ if (pd != null) {
+ if (pds == null) pds = new HashSet<>();
+ pds.add(pd);
+ }
+ cls = cls.getSuperclass();
+ if (cls == null) {
+ // that's not supposed to happen
+ // make a ProtectionDomain with no permission.
+ // should we throw instead?
+ if (pds == null) pds = new HashSet<>();
+ else pds.clear();
+ pds.add(noPermissionsDomain());
+ break;
+ }
+ }
+ if (pds != null) {
+ domains = pds.toArray(new ProtectionDomain[0]);
+ }
+ }
+ return domains;
+ }
+
+ /**
* Initializes class descriptor representing a proxy class.
*/
void initProxy(Class<?> cl,
@@ -580,6 +649,7 @@
writeReplaceMethod = localDesc.writeReplaceMethod;
readResolveMethod = localDesc.readResolveMethod;
deserializeEx = localDesc.deserializeEx;
+ domains = localDesc.domains;
cons = localDesc.cons;
}
fieldRefl = getReflector(fields, localDesc);
@@ -666,6 +736,7 @@
if (deserializeEx == null) {
deserializeEx = localDesc.deserializeEx;
}
+ domains = localDesc.domains;
cons = localDesc.cons;
}
@@ -1006,7 +1077,35 @@
requireInitialized();
if (cons != null) {
try {
- return cons.newInstance();
+ if (domains == null || domains.length == 0) {
+ return cons.newInstance();
+ } else {
+ JavaSecurityAccess jsa = SharedSecrets.getJavaSecurityAccess();
+ PrivilegedAction<?> pea = () -> {
+ try {
+ return cons.newInstance();
+ } catch (InstantiationException
+ | InvocationTargetException
+ | IllegalAccessException x) {
+ throw new UndeclaredThrowableException(x);
+ }
+ }; // Can't use PrivilegedExceptionAction with jsa
+ try {
+ return jsa.doIntersectionPrivilege(pea,
+ AccessController.getContext(),
+ new AccessControlContext(domains));
+ } catch (UndeclaredThrowableException x) {
+ Throwable cause = x.getCause();
+ if (cause instanceof InstantiationException)
+ throw (InstantiationException) cause;
+ if (cause instanceof InvocationTargetException)
+ throw (InvocationTargetException) cause;
+ if (cause instanceof IllegalAccessException)
+ throw (IllegalAccessException) cause;
+ // not supposed to happen
+ throw x;
+ }
+ }
} catch (IllegalAccessException ex) {
// should not occur, as access checks have been suppressed
throw new InternalError(ex);
--- a/src/java.corba/share/classes/com/sun/corba/se/impl/io/ObjectStreamClass.java Thu Oct 19 11:45:23 2017 -0700
+++ b/src/java.corba/share/classes/com/sun/corba/se/impl/io/ObjectStreamClass.java Fri May 19 11:18:49 2017 +0100
@@ -38,7 +38,10 @@
import java.security.NoSuchAlgorithmException;
import java.security.DigestOutputStream;
import java.security.AccessController;
+import java.security.PermissionCollection;
+import java.security.Permissions;
import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
import java.lang.reflect.Modifier;
import java.lang.reflect.Field;
@@ -57,6 +60,8 @@
import java.util.Arrays;
import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Set;
import com.sun.corba.se.impl.util.RepositoryId;
@@ -443,6 +448,65 @@
private static final PersistentFieldsValue persistentFieldsValue =
new PersistentFieldsValue();
+ /**
+ * Creates a PermissionDomain that grants no permission.
+ */
+ private ProtectionDomain noPermissionsDomain() {
+ PermissionCollection perms = new Permissions();
+ perms.setReadOnly();
+ return new ProtectionDomain(null, perms);
+ }
+
+ /**
+ * Aggregate the ProtectionDomains of all the classes that separate
+ * a concrete class {@code cl} from its ancestor's class declaring
+ * a constructor {@code cons}.
+ *
+ * If {@code cl} is defined by the boot loader, or the constructor
+ * {@code cons} is declared by {@code cl}, or if there is no security
+ * manager, then this method does nothing and {@code null} is returned.
+ *
+ * @param cons A constructor declared by {@code cl} or one of its
+ * ancestors.
+ * @param cl A concrete class, which is either the class declaring
+ * the constructor {@code cons}, or a serializable subclass
+ * of that class.
+ * @return An array of ProtectionDomain representing the set of
+ * ProtectionDomain that separate the concrete class {@code cl}
+ * from its ancestor's declaring {@code cons}, or {@code null}.
+ */
+ private ProtectionDomain[] getProtectionDomains(Constructor<?> cons,
+ Class<?> cl) {
+ ProtectionDomain[] domains = null;
+ if (cons != null && cl.getClassLoader() != null
+ && System.getSecurityManager() != null) {
+ Class<?> cls = cl;
+ Class<?> fnscl = cons.getDeclaringClass();
+ Set<ProtectionDomain> pds = null;
+ while (cls != fnscl) {
+ ProtectionDomain pd = cls.getProtectionDomain();
+ if (pd != null) {
+ if (pds == null) pds = new HashSet<>();
+ pds.add(pd);
+ }
+ cls = cls.getSuperclass();
+ if (cls == null) {
+ // that's not supposed to happen
+ // make a ProtectionDomain with no permission.
+ // should we throw instead?
+ if (pds == null) pds = new HashSet<>();
+ else pds.clear();
+ pds.add(noPermissionsDomain());
+ break;
+ }
+ }
+ if (pds != null) {
+ domains = pds.toArray(new ProtectionDomain[0]);
+ }
+ }
+ return domains;
+ }
+
/*
* Initialize class descriptor. This method is only invoked on class
* descriptors created via calls to lookupInternal(). This method is kept
@@ -568,11 +632,15 @@
readResolveObjectMethod = bridge.readResolveForSerialization(cl);
+ domains = new ProtectionDomain[] {noPermissionsDomain()};
+
if (externalizable)
cons = getExternalizableConstructor(cl) ;
else
cons = getSerializableConstructor(cl) ;
+ domains = getProtectionDomains(cons, cl);
+
if (serializable && !forProxyClass) {
writeObjectMethod = bridge.writeObjectForSerialization(cl) ;
readObjectMethod = bridge.readObjectForSerialization(cl);
@@ -910,7 +978,7 @@
{
if (cons != null) {
try {
- return cons.newInstance();
+ return bridge.newInstanceForSerialization(cons, domains);
} catch (IllegalAccessException ex) {
// should not occur, as access checks have been suppressed
InternalError ie = new InternalError();
@@ -1506,6 +1574,7 @@
private transient MethodHandle writeReplaceObjectMethod;
private transient MethodHandle readResolveObjectMethod;
private transient Constructor<?> cons;
+ private transient ProtectionDomain[] domains;
/**
* Beginning in Java to IDL ptc/02-01-12, RMI-IIOP has a
--- a/src/java.corba/share/classes/sun/corba/Bridge.java Thu Oct 19 11:45:23 2017 -0700
+++ b/src/java.corba/share/classes/sun/corba/Bridge.java Fri May 19 11:18:49 2017 +0100
@@ -27,8 +27,9 @@
import java.io.OptionalDataException;
import java.lang.invoke.MethodHandle;
-import java.lang.reflect.Field ;
-import java.lang.reflect.Constructor ;
+import java.lang.reflect.Field;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
import java.lang.StackWalker;
import java.lang.StackWalker.StackFrame;
import java.util.Optional;
@@ -37,6 +38,7 @@
import java.security.AccessController;
import java.security.Permission;
import java.security.PrivilegedAction;
+import java.security.ProtectionDomain;
import sun.misc.Unsafe;
import sun.reflect.ReflectionFactory;
@@ -341,6 +343,36 @@
}
/**
+ * Invokes the supplied constructor, adding the provided protection domains
+ * to the invocation stack before invoking {@code Constructor::newInstance}.
+ *
+ * This is equivalent to calling
+ * {@code ReflectionFactory.newInstanceForSerialization(cons,domains)}.
+ *
+ * @param cons A constructor obtained from {@code
+ * newConstructorForSerialization} or {@code
+ * newConstructorForExternalization}.
+ *
+ * @param domains An array of protection domains that limit the privileges
+ * with which the constructor is invoked. Can be {@code null}
+ * or empty, in which case privileges are only limited by the
+ * {@linkplain AccessController#getContext() current context}.
+ *
+ * @return A new object built from the provided constructor.
+ *
+ * @throws NullPointerException if {@code cons} is {@code null}.
+ * @throws InstantiationException if thrown by {@code cons.newInstance()}.
+ * @throws InvocationTargetException if thrown by {@code cons.newInstance()}.
+ * @throws IllegalAccessException if thrown by {@code cons.newInstance()}.
+ */
+ public final Object newInstanceForSerialization(Constructor<?> cons,
+ ProtectionDomain[] domains)
+ throws InstantiationException, InvocationTargetException, IllegalAccessException
+ {
+ return reflectionFactory.newInstanceForSerialization(cons, domains);
+ }
+
+ /**
* Returns true if the given class defines a static initializer method,
* false otherwise.
*/
--- a/src/jdk.unsupported/share/classes/sun/reflect/ReflectionFactory.java Thu Oct 19 11:45:23 2017 -0700
+++ b/src/jdk.unsupported/share/classes/sun/reflect/ReflectionFactory.java Fri May 19 11:18:49 2017 +0100
@@ -29,9 +29,14 @@
import java.lang.invoke.MethodHandle;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.UndeclaredThrowableException;
+import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.Permission;
+import java.security.ProtectionDomain;
import java.security.PrivilegedAction;
+import jdk.internal.misc.SharedSecrets;
+import jdk.internal.misc.JavaSecurityAccess;
/**
* ReflectionFactory supports custom serialization.
@@ -140,6 +145,66 @@
}
/**
+ * Invokes the supplied constructor, adding the provided protection domains
+ * to the invocation stack before invoking {@code Constructor::newInstance}.
+ * If no {@linkplain System#getSecurityManager() security manager} is present,
+ * or no domains are provided, then this method simply calls
+ * {@code cons.newInstance()}. Otherwise, it invokes the provided constructor
+ * with privileges at the intersection of the current context and the provided
+ * protection domains.
+ *
+ * @param cons A constructor obtained from {@code
+ * newConstructorForSerialization} or {@code
+ * newConstructorForExternalization}.
+ * @param domains An array of protection domains that limit the privileges
+ * with which the constructor is invoked. Can be {@code null}
+ * or empty, in which case privileges are only limited by the
+ * {@linkplain AccessController#getContext() current context}.
+ *
+ * @return A new object built from the provided constructor.
+ *
+ * @throws NullPointerException if {@code cons} is {@code null}.
+ * @throws InstantiationException if thrown by {@code cons.newInstance()}.
+ * @throws InvocationTargetException if thrown by {@code cons.newInstance()}.
+ * @throws IllegalAccessException if thrown by {@code cons.newInstance()}.
+ */
+ public final Object newInstanceForSerialization(Constructor<?> cons,
+ ProtectionDomain[] domains)
+ throws InstantiationException, InvocationTargetException, IllegalAccessException
+ {
+ SecurityManager sm = System.getSecurityManager();
+ if (sm == null || domains == null || domains.length == 0) {
+ return cons.newInstance();
+ } else {
+ JavaSecurityAccess jsa = SharedSecrets.getJavaSecurityAccess();
+ PrivilegedAction<?> pea = () -> {
+ try {
+ return cons.newInstance();
+ } catch (InstantiationException
+ | InvocationTargetException
+ | IllegalAccessException x) {
+ throw new UndeclaredThrowableException(x);
+ }
+ }; // Can't use PrivilegedExceptionAction with jsa
+ try {
+ return jsa.doIntersectionPrivilege(pea,
+ AccessController.getContext(),
+ new AccessControlContext(domains));
+ } catch (UndeclaredThrowableException x) {
+ Throwable cause = x.getCause();
+ if (cause instanceof InstantiationException)
+ throw (InstantiationException) cause;
+ if (cause instanceof InvocationTargetException)
+ throw (InvocationTargetException) cause;
+ if (cause instanceof IllegalAccessException)
+ throw (IllegalAccessException) cause;
+ // not supposed to happen
+ throw x;
+ }
+ }
+ }
+
+ /**
* Returns a direct MethodHandle for the {@code readObjectNoData} method on
* a Serializable class.
* The first argument of {@link MethodHandle#invoke} is the serializable
@@ -224,4 +289,3 @@
}
}
}
-