8210496: Improve filtering for classes with security sensitive fields

Reviewed-by: plevart, mchung
This commit is contained in:
Alan Bateman 2018-09-19 08:49:07 +01:00
parent a17816f881
commit 9c70e26c14
8 changed files with 138 additions and 66 deletions

View File

@ -54,6 +54,7 @@ import java.util.BitSet;
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 @@ public class MethodHandles {
/** 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

View File

@ -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 @@ public class ConstantPool {
//
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)

View File

@ -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 @@ public class Reflection {
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 @@ public class Reflection {
// 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 @@ public class Reflection {
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;
}
}

View File

@ -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 @@ package jdk.internal.reflect;
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 @@ import jdk.internal.misc.Unsafe;
abstract class UnsafeStaticFieldAccessorImpl extends UnsafeFieldAccessorImpl {
static {
Reflection.registerFieldsToFilter(UnsafeStaticFieldAccessorImpl.class,
new String[] { "base" });
Set.of("base"));
}
protected final Object base; // base

View File

@ -33,6 +33,7 @@ import jdk.internal.reflect.Reflection;
import sun.nio.ch.DirectBuffer;
import java.lang.reflect.Field;
import java.util.Set;
/**
@ -56,7 +57,7 @@ import java.lang.reflect.Field;
public final class Unsafe {
static {
Reflection.registerMethodsToFilter(Unsafe.class, "getUnsafe");
Reflection.registerMethodsToFilter(Unsafe.class, Set.of("getUnsafe"));
}
private Unsafe() {}

View File

@ -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.assertNull;
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 @@ public class TestResolvedJavaType extends TypeUniverse {
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;
}

View File

@ -113,25 +113,17 @@ public class AccessTest {
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 @@ public class AccessTest {
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");
}
}
}

View File

@ -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);
}
}