Replacement of sun.reflect.Reflection#getCallerClass

[David M. Lloyd]

On 09/19/2013 07:03 AM, Peter Levart wrote:
> On 09/18/2013 05:21 PM, David M. Lloyd wrote:
>> On 09/03/2013 12:16 PM, Peter Levart wrote:
>> [...]
>>> *AND* that Reflection.getCallerClass() can only be called from within
>>>  methods annotated with @CallerSensitive.
>>>
>>> Now for that part, the public API equivalent
>>> (StackTraceFrame.getCallerClass() or whatever it is called) need not
>>> be restricted to methods annotated with any annotation, but that
>>> means that this public API should not be used to implement security
>>> decisions since MethodHandles API allows caller to be spoofed unless
>>> looking-up a method annotated with @CallerSensitive...
>>
>> Peter, can you please elaborate on this a bit?  I could find nothing
>> in the MethodHandles API or its associated classes that would seem to
>> give the ability to call another method with a spoofed caller.  Yes
>> you can set up a Lookup for another class but I don't see how that
>> would affect the ability of (say) a security manager to make access
>> decisions based on the call stack/class context?
>>
>
> Hi David,
>
> The thing with method handles is that they perform all security checks
> not at invoke-time (like reflection), but at lookup-time. If you have a
> hold on a MethodHandle object, you can always invoke it. All security
> checks must have been performed at lookup-time. This includes security
> checks that are based on what the "caller" of the method is. In case of
> method handles, the "caller" is the class associated with the Lookup
> object - the lookup class.
>
> Try this example:
>
> // --- the following be loaded by the bootstrap class loader
> package sys;
>
> import sun.reflect.CallerSensitive;
> import sun.reflect.Reflection;
>
> import java.lang.invoke.MethodHandles;
>
> public class CSUtil
> {
>      public static final MethodHandles.Lookup lookup =
> MethodHandles.lookup();
>
>      @CallerSensitive
>      public static void printCallerClassLoader(String prefix) {
>          Class<?> cc = Reflection.getCallerClass();
>          System.err.println(prefix + ":\n  "
>                             + cc + "\n  loaded by "
>                             + cc.getClassLoader() + "\n");
>      }
>
>      public static class Nested {}
> }
>
>
> // ---the following be loaded by application class loader
> package usr;
>
> import sys.CSUtil;
>
> import java.lang.invoke.MethodHandle;
> import java.lang.invoke.MethodHandles;
> import java.lang.invoke.MethodType;
>
> public class CSTest
> {
>      public static final MethodHandles.Lookup lookup =
> MethodHandles.lookup();
>
>      public static void main(String[] args) throws Throwable
>      {
>          MethodHandle mh1  = CSTest.lookup.findStatic(
>              CSUtil.class,
>              "printCallerClassLoader",
>              MethodType.methodType(void.class, String.class)
>          );
>
>          mh1.invokeExact("mh1");
>
>          MethodHandle mh2  = CSUtil.lookup.findStatic(
>              CSUtil.class,
>              "printCallerClassLoader",
>              MethodType.methodType(void.class, String.class)
>          );
>
>          mh2.invokeExact("mh2");
>
>          MethodHandle mh3  =
> CSUtil.lookup.in(CSUtil.Nested.class).findStatic(
>              CSUtil.class,
>              "printCallerClassLoader",
>              MethodType.methodType(void.class, String.class)
>          );
>
>          mh3.invokeExact("mh3");
>
>          MethodHandle mh4  = CSUtil.lookup.in(CSTest.class).findStatic(
>              CSUtil.class,
>              "printCallerClassLoader",
>              MethodType.methodType(void.class, String.class)
>          );
>
>          mh4.invokeExact("mh4");
>      }
> }
>
>
> ...which prints:
>
> mh1:
>    class java.lang.invoke.MethodHandleImpl$BindCaller$T/523429237
>    loaded by sun.misc.Launcher$AppClassLoader at 15db9742
>
> mh2:
>    class java.lang.invoke.MethodHandleImpl$BindCaller$T/1450495309
>    loaded by null
>
> mh3:
>    class java.lang.invoke.MethodHandleImpl$BindCaller$T/2001049719
>    loaded by null
>
> Exception in thread "main" java.lang.IllegalAccessException: Attempt to
> lookup caller-sensitive method using restricted lookup object
>      at
> java.lang.invoke.MethodHandles$Lookup.findBoundCallerClass(MethodHandles.java:1123)
>
>      at
> java.lang.invoke.MethodHandles$Lookup.findStatic(MethodHandles.java:619)
>      at usr.CSTest.main(CSTest.java:39)
>
>
> You can see that the "caller" class it not actually the class doing
> lookup, but some anonymous class which is loaded by the same class
> loader as the lookup class and maybe it shares some other properties
> with it (it seems that this is sufficient for security checks). The mh3
> is interesting in that the "caller" is different from that of mh2. The
> Lookup.in(Class) method returns a Lookup object with different lookup
> class and so the "caller" class is different too. Although the
> printCallerClassLoader is a public method in a public class, the mh4 can
> not be looked-up, because this could be used to "spoof" the caller class
> loader for any such public method. Imagine:
>
>
>          // this throws IllegalAccessException. If it didn't...
>          MethodHandle mh = MethodHandles.lookup()
>              .in(String.class)
>              .findVirtual(
>                  Field.class,
>                  "get",
>                  MethodType.methodType(Object.class, Object.class)
>              );
>
>          Field valueField = String.class.getDeclaredField("value");
>
>          String abc = "abc";
>
>          // ... then the following would invoke: valueField.get(abc) and
> pretend that
>          // it was invoked from the String class, which would succeed...
>          char[] abcArray = (char[])(Object) mh.invokeExact(valueField,
> (Object) abc);
>
>          // ...and you could observe the following:
>          abcArray[0] = 'A';
>          assert abc.charAt(0) == 'A';

This must have changed since JDK 7 because I don't see any of this 
behavior with the classic getCallerClass(int) method.  In other words 
using "in" does not appear to affect the frames reported by 
getCallerClass(int) (or SecurityManager.getClassContext() or 
Thread.getStackTrace()) at all.

It looks like this new behavior is a result of @CallerSensitive and 
changes associated with it (in particular those weird pseudo-class names 
are not reported in 7); I'd say the fatal flaw exposed here is in those 
changes, not in the original method behavior.  It seems like we've gone 
way off course if this is the case.

Especially consider that SecurityManager.getClassContext() is a 
supported, non-deprecated API designed for the express purpose of access 
checking, which (as far as I can test) works similarly to 
getCallerClass(int).  If method handles could spoof callers in this way 
then we've blown yet another massive hole in Java security.
-- 
- DML