8210496: Improve filtering for classes with security sensitive fields

[Peter Levart]

Hi Alan,

Just a few comments about implementation.

In Reflection:

  292             boolean shouldSkip = false;
  293             for (String filteredName : filteredNames) {
  294                 if (member.getName() == filteredName) {
  295                     shouldSkip = true;
  296                     break;
  297                 }
  298             }

...now that filteredNames is a Set<String>, you could simplify this loop to:

         shouldSkip = filteredNames.contains(member.getName());

The filter method then becomes:

     private static Member[] filter(Member[] members, Set<String> 
filteredNames) {
         if ((filteredNames == null) || (members.length == 0)) {
             return members;
         }
         if (filteredNames.contains(WILDCARD)) {
             return (Member[]) Array.newInstance(members[0].getClass(), 0);
         }
         int numNewMembers = 0;
         for (Member member : members) {
             if (!filteredNames.contains(member.getName())) {
                 ++numNewMembers;
             }
         }
         Member[] newMembers =
             (Member[])Array.newInstance(members[0].getClass(), 
numNewMembers);
         int destIdx = 0;
         for (Member member : members) {
             if (!filteredNames.contains(member.getName())) {
                 newMembers[destIdx++] = member;
             }
         }
         return newMembers;
     }


I don't know if it's better, but you could use just the immutable Map 
without HashMap. Instead of:

  262         map = new HashMap<>(map);
  263         map.put(containingClass, Set.copyOf(names));
  264         return map;

you could do this:

         @SuppressWarnings("unchecked")
         Map.Entry<Class<?>, Set<String>>[] entries = 
map.entrySet().toArray(new Map.Entry[map.size() + 1]);
         entries[entries.length - 1] = Map.entry(containingClass, 
Set.copyOf(names));
         return Map.ofEntries(entries);

This should work because the registerFilter method is called at most 
once per map/containingClass.

Regards, Peter

On 09/14/2018 07:57 PM, Alan Bateman wrote:
>
>
> On 14/09/2018 18:52, Alan Bateman wrote:
>> Core reflection has a filtering mechanism to hide a number of fields 
>> that are critical to security or the integrity of the runtime. It's a 
>> bit of a band aid but it helps to reduce hacking on fields such as 
>> java.lang.System.security and java.lang.Class.classLoder. I'd like to 
>> extend the filters to hide a few additional fields from 
>> integrity-sensitive (and non-serializable) classes in 
>> java.lang.reflect and java.lang.invoke. There are of course a number 
>> of nasty hacks around that might break due to this but these hacks 
>> would be broken anyway with simple rename or other innocent 
>> refactoring (we had some of this during JDK 11 when Mandy fixed 
>> JDK-8202113 for example).
>>
>> The webrev with the changes is here:
>>    https://bugs.openjdk.java.net/browse/JDK-8210496
> Sorry, that is the JBS issue, the webrev is here:
>    http://cr.openjdk.java.net/~alanb/8210496/webrev/index.html
>
> -Alan