8210496: Improve filtering for classes with security sensitive fields
Reviewed-by: plevart, mchung
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java Wed Sep 19 12:14:53 2018 +0530
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java Wed Sep 19 08:49:07 2018 +0100
@@ -54,6 +54,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -643,6 +644,10 @@
/** The allowed sorts of members which may be looked up (PUBLIC, etc.). */
private final int allowedModes;
+ static {
+ Reflection.registerFieldsToFilter(Lookup.class, Set.of("lookupClass", "allowedModes"));
+ }
+
/** A single-bit mask representing {@code public} access,
* which may contribute to the result of {@link #lookupModes lookupModes}.
* The value, {@code 0x01}, happens to be the same as the value of the
--- a/src/java.base/share/classes/jdk/internal/reflect/ConstantPool.java Wed Sep 19 12:14:53 2018 +0530
+++ b/src/java.base/share/classes/jdk/internal/reflect/ConstantPool.java Wed Sep 19 08:49:07 2018 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, 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
@@ -26,6 +26,7 @@
package jdk.internal.reflect;
import java.lang.reflect.*;
+import java.util.Set;
/** Provides reflective access to the constant pools of classes.
Currently this is needed to provide reflective access to annotations
@@ -104,7 +105,7 @@
//
static {
- Reflection.registerFieldsToFilter(ConstantPool.class, new String[] { "constantPoolOop" });
+ Reflection.registerFieldsToFilter(ConstantPool.class, Set.of("constantPoolOop"));
}
// HotSpot-internal constant pool object (set by the VM, name known to the VM)
--- a/src/java.base/share/classes/jdk/internal/reflect/Reflection.java Wed Sep 19 12:14:53 2018 +0530
+++ b/src/java.base/share/classes/jdk/internal/reflect/Reflection.java Wed Sep 19 08:49:07 2018 +0100
@@ -25,13 +25,12 @@
package jdk.internal.reflect;
-
import java.lang.reflect.*;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import jdk.internal.HotSpotIntrinsicCandidate;
-import jdk.internal.loader.ClassLoaders;
import jdk.internal.misc.VM;
/** Common utility routines used by both java.lang and
@@ -43,18 +42,23 @@
view, where they are sensitive or they may contain VM-internal objects.
These Maps are updated very rarely. Rather than synchronize on
each access, we use copy-on-write */
- private static volatile Map<Class<?>,String[]> fieldFilterMap;
- private static volatile Map<Class<?>,String[]> methodFilterMap;
+ private static volatile Map<Class<?>, Set<String>> fieldFilterMap;
+ private static volatile Map<Class<?>, Set<String>> methodFilterMap;
+ private static final String WILDCARD = "*";
+ public static final Set<String> ALL_MEMBERS = Set.of(WILDCARD);
static {
- Map<Class<?>,String[]> map = new HashMap<Class<?>,String[]>();
- map.put(Reflection.class,
- new String[] {"fieldFilterMap", "methodFilterMap"});
- map.put(System.class, new String[] {"security"});
- map.put(Class.class, new String[] {"classLoader"});
- fieldFilterMap = map;
-
- methodFilterMap = new HashMap<>();
+ fieldFilterMap = Map.of(
+ Reflection.class, ALL_MEMBERS,
+ AccessibleObject.class, ALL_MEMBERS,
+ Class.class, Set.of("classLoader"),
+ ClassLoader.class, ALL_MEMBERS,
+ Constructor.class, ALL_MEMBERS,
+ Field.class, ALL_MEMBERS,
+ Method.class, ALL_MEMBERS,
+ System.class, Set.of("security")
+ );
+ methodFilterMap = Map.of();
}
/** Returns the class of the caller of the method calling this method,
@@ -236,31 +240,31 @@
// fieldNames must contain only interned Strings
public static synchronized void registerFieldsToFilter(Class<?> containingClass,
- String ... fieldNames) {
+ Set<String> fieldNames) {
fieldFilterMap =
registerFilter(fieldFilterMap, containingClass, fieldNames);
}
// methodNames must contain only interned Strings
public static synchronized void registerMethodsToFilter(Class<?> containingClass,
- String ... methodNames) {
+ Set<String> methodNames) {
methodFilterMap =
registerFilter(methodFilterMap, containingClass, methodNames);
}
- private static Map<Class<?>,String[]> registerFilter(Map<Class<?>,String[]> map,
- Class<?> containingClass, String ... names) {
+ private static Map<Class<?>, Set<String>> registerFilter(Map<Class<?>, Set<String>> map,
+ Class<?> containingClass,
+ Set<String> names) {
if (map.get(containingClass) != null) {
throw new IllegalArgumentException
("Filter already registered: " + containingClass);
}
- map = new HashMap<Class<?>,String[]>(map);
- map.put(containingClass, names);
+ map = new HashMap<>(map);
+ map.put(containingClass, Set.copyOf(names));
return map;
}
- public static Field[] filterFields(Class<?> containingClass,
- Field[] fields) {
+ public static Field[] filterFields(Class<?> containingClass, Field[] fields) {
if (fieldFilterMap == null) {
// Bootstrapping
return fields;
@@ -276,35 +280,24 @@
return (Method[])filter(methods, methodFilterMap.get(containingClass));
}
- private static Member[] filter(Member[] members, String[] filteredNames) {
+ private static Member[] filter(Member[] members, Set<String> filteredNames) {
if ((filteredNames == null) || (members.length == 0)) {
return members;
}
+ Class<?> memberType = members[0].getClass();
+ if (filteredNames.contains(WILDCARD)) {
+ return (Member[]) Array.newInstance(memberType, 0);
+ }
int numNewMembers = 0;
for (Member member : members) {
- boolean shouldSkip = false;
- for (String filteredName : filteredNames) {
- if (member.getName() == filteredName) {
- shouldSkip = true;
- break;
- }
- }
- if (!shouldSkip) {
+ if (!filteredNames.contains(member.getName())) {
++numNewMembers;
}
}
- Member[] newMembers =
- (Member[])Array.newInstance(members[0].getClass(), numNewMembers);
+ Member[] newMembers = (Member[])Array.newInstance(memberType, numNewMembers);
int destIdx = 0;
for (Member member : members) {
- boolean shouldSkip = false;
- for (String filteredName : filteredNames) {
- if (member.getName() == filteredName) {
- shouldSkip = true;
- break;
- }
- }
- if (!shouldSkip) {
+ if (!filteredNames.contains(member.getName())) {
newMembers[destIdx++] = member;
}
}
--- a/src/java.base/share/classes/jdk/internal/reflect/UnsafeStaticFieldAccessorImpl.java Wed Sep 19 12:14:53 2018 +0530
+++ b/src/java.base/share/classes/jdk/internal/reflect/UnsafeStaticFieldAccessorImpl.java Wed Sep 19 08:49:07 2018 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, 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
@@ -28,6 +28,8 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.security.AccessController;
+import java.util.Set;
+
import jdk.internal.misc.Unsafe;
/** Base class for jdk.internal.misc.Unsafe-based FieldAccessors for static
@@ -40,7 +42,7 @@
abstract class UnsafeStaticFieldAccessorImpl extends UnsafeFieldAccessorImpl {
static {
Reflection.registerFieldsToFilter(UnsafeStaticFieldAccessorImpl.class,
- new String[] { "base" });
+ Set.of("base"));
}
protected final Object base; // base
--- a/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java Wed Sep 19 12:14:53 2018 +0530
+++ b/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java Wed Sep 19 08:49:07 2018 +0100
@@ -33,6 +33,7 @@
import sun.nio.ch.DirectBuffer;
import java.lang.reflect.Field;
+import java.util.Set;
/**
@@ -56,7 +57,7 @@
public final class Unsafe {
static {
- Reflection.registerMethodsToFilter(Unsafe.class, "getUnsafe");
+ Reflection.registerMethodsToFilter(Unsafe.class, Set.of("getUnsafe"));
}
private Unsafe() {}
--- a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Wed Sep 19 12:14:53 2018 +0530
+++ b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Wed Sep 19 08:49:07 2018 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, 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
@@ -49,6 +49,8 @@
import static org.junit.Assert.assertTrue;
import java.lang.annotation.Annotation;
+import java.lang.reflect.AccessibleObject;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -762,6 +764,13 @@
if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(Class.class)) && f.getName().equals("classLoader")) {
return true;
}
+ if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(ClassLoader.class)) ||
+ f.getDeclaringClass().equals(metaAccess.lookupJavaType(AccessibleObject.class)) ||
+ f.getDeclaringClass().equals(metaAccess.lookupJavaType(Constructor.class)) ||
+ f.getDeclaringClass().equals(metaAccess.lookupJavaType(Field.class)) ||
+ f.getDeclaringClass().equals(metaAccess.lookupJavaType(Method.class))) {
+ return true;
+ }
return false;
}
--- a/test/jdk/java/lang/reflect/callerCache/AccessTest.java Wed Sep 19 12:14:53 2018 +0530
+++ b/test/jdk/java/lang/reflect/callerCache/AccessTest.java Wed Sep 19 08:49:07 2018 +0100
@@ -113,25 +113,17 @@
if (!Modifier.isFinal(f.getModifiers())) {
throw new RuntimeException("not a final field");
}
- makeFinalNonFinal(f);
}
public Void call() throws Exception {
Members obj = isStatic ? null : new Members();
try {
f.set(obj, 20);
- checkValue(obj, 20);
+ throw new RuntimeException("IllegalAccessException expected");
} catch (IllegalAccessException e) {
- throw e;
+ // expected
}
return null;
}
-
- void checkValue(Object obj, int expected) throws Exception {
- int value = (int) f.get(obj);
- if (value != expected) {
- throw new RuntimeException("unexpectd value: " + value);
- }
- }
}
public static class PublicFinalField extends FinalField {
@@ -157,15 +149,4 @@
super("privateStaticFinalField");
}
}
-
- private static void makeFinalNonFinal(Field f) throws ReflectiveOperationException {
- Field modifiers = Field.class.getDeclaredField("modifiers");
- modifiers.setAccessible(true);
- modifiers.set(f, modifiers.getInt(f) & ~Modifier.FINAL);
- f.setAccessible(true);
-
- if (Modifier.isFinal(f.getModifiers())) {
- throw new RuntimeException("should be a non-final field");
- }
- }
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/jdk/internal/reflect/Reflection/Filtering.java Wed Sep 19 08:49:07 2018 +0100
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2018, 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.
+ *
+ * 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.
+ */
+
+/**
+ * @test
+ * @bug 8210496
+ * @modules java.base/jdk.internal.reflect
+ * @run testng Filtering
+ * @summary Test that security sensitive fields that filtered by core reflection
+ */
+
+import java.lang.reflect.*;
+import java.lang.invoke.MethodHandles.Lookup;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import static org.testng.Assert.assertTrue;
+
+public class Filtering {
+
+ @DataProvider(name = "sensitiveClasses")
+ private Object[][] sensitiveClasses() {
+ return new Object[][]{
+ { jdk.internal.reflect.Reflection.class, null },
+ { AccessibleObject.class, null },
+ { ClassLoader.class, null },
+ { Constructor.class, null },
+ { Field.class, null },
+ { Method.class, null },
+ };
+ }
+
+ @DataProvider(name = "sensitiveFields")
+ private Object[][] sensitiveFields() {
+ return new Object[][]{
+ { AccessibleObject.class, "override" },
+ { Class.class, "classLoader" },
+ { ClassLoader.class, "parent" },
+ { Field.class, "clazz" },
+ { Field.class, "modifiers" },
+ { Lookup.class, "lookupClass" },
+ { Lookup.class, "allowedModes" },
+ { Method.class, "clazz" },
+ { Method.class, "modifiers" },
+ { System.class, "security" },
+ };
+ }
+
+ @Test(dataProvider = "sensitiveClasses")
+ public void testClass(Class<?> clazz, Object ignore) throws Exception {
+ Field[] fields = clazz.getDeclaredFields();
+ assertTrue(fields.length == 0);
+ }
+
+ @Test(dataProvider = "sensitiveFields",
+ expectedExceptions = NoSuchFieldException.class)
+ public void testField(Class<?> clazz, String name) throws Exception {
+ clazz.getDeclaredField(name);
+ }
+
+}